Page MenuHomeFreeBSD

Add necessary bits for Linux KPI to work correctly on powerpc
ClosedPublic

Authored by jhibbits on Jul 28 2019, 9:03 PM.

Details

Summary

PowerPC, and possibly other architectures, use different address ranges
for PCI space vs physical address space, which is only mapped at
resource activation time, when the BAR gets written. The DRM kernel
modules do not activate the rman resources, soas not to waste KVA,
instead only mapping parts of the PCI memory at a time. This introduces
a BUS_TRANSLATE_RESOURCE() method, implemented in the Open Firmware/FDT
PCI driver, to perform this necessary translation without activating the
resource.

In addition to system KPI changes, LinuxKPI is updated to handle a
big-endian host, by adding proper endian swaps to the I/O functions.

Work largely done by Matt Macy.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhibbits created this revision.Jul 28 2019, 9:03 PM

I'll have a closer look tomorrow. Most of this looks good.

Is this patch also needed for powerpc?

D21008

I'll have a closer look tomorrow. Most of this looks good.
Is this patch also needed for powerpc?
D21008

It might be. I'm still building xf86-video-ati to test X11 with drm-current-kmod on powerpc64, but console loaded fine (log shows acceleration disabled though). The log shows:

[drm] Driver supports precise vblank timestamp query.
drmn0: radeon: MSI limited to 32-bit
[drm] radeon: irq initialized.
[drm:r600_ring_test] radeon: ring 0 test failed (scratch(0x850C)=0xCAFEDEAD)
drmn0: disabling GPU acceleration

In short, MSI isn't required for powerpc64 but would probably help.

hselasky added inline comments.Jul 29 2019, 2:43 PM
sys/compat/linuxkpi/common/include/linux/io.h
52 ↗(On Diff #60220)

Did you check all architectures use rmb macros and not static inline functions ?

sys/dev/ofw/ofwpci.c
512 ↗(On Diff #60220)

Style: bad indent closing curly bracket.

sys/dev/pci/vga_pci.c
269 ↗(On Diff #60220)

spelling: rom -> ROM

sys/kern/bus_if.m
424 ↗(On Diff #60220)

Can we specify a default here, instead of adding the nexus translate resource?

Like:

METHOD int translate_resource {
device_t _dev;
int _type;
rman_res_t _start;
rman_res_t *_newstart;
} DEFAULT device_translate_resource_none;

sys/powerpc/powerpc/nexus.c
272 ↗(On Diff #60220)

Can't this be the default function, if the device_translate_resource method is not specified?

jhibbits marked 2 inline comments as done.Jul 29 2019, 9:34 PM
jhibbits added inline comments.
sys/compat/linuxkpi/common/include/linux/io.h
52 ↗(On Diff #60220)

No, I didn't check. This code came from mmacy, and I only made it compile with head.

sys/kern/bus_if.m
424 ↗(On Diff #60220)

Yeah, that makes sense. Would it make sense to make this default more like bus_generic_translate_resource(), but return 0 for the root, and remove bus_generic_translate_resource()? Sort of like what null_remap_intr() does?

hselasky added inline comments.Jul 29 2019, 10:23 PM
sys/kern/bus_if.m
424 ↗(On Diff #60220)

I think that's a good idea.

@jhibbits : Can you update the patch with the agreed upon changes?

@hselasky yes, I'm working on it, but got sidetracked doing other things while working on it. I'll have an update either tonight or tomorrow.

jhibbits updated this revision to Diff 60389.Aug 2 2019, 2:11 PM

Address @hselasky's comments.

hselasky accepted this revision.Aug 4 2019, 1:26 PM

Looks good. Maybe you should also bump the __FreeBSD_version when you commit this.

This revision is now accepted and ready to land.Aug 4 2019, 1:26 PM
This revision was automatically updated to reflect the committed changes.