Page MenuHomeFreeBSD

Add domain support to PCI bus allocation
ClosedPublic

Authored by wma_semihalf.com on Aug 18 2015, 6:35 AM.

Details

Summary
When the system has more than a single PCI domain, the bus numbers
are not unique, thus they cannot be used for "pci" device numbering.
Change bus numbers to -1 (i.e. to-be-determined automatically)
wherever the code did not care about domains.

Here is the list of files containing secondary bus attachments, with my comments.

GOOD ALREADY:
arm/mv/mv_pci.c: device_add_child(self, "pci", -1);
arm/xscale/ixp425/ixp425_pci.c: device_add_child(dev, "pci", -1);
boot/kshim/bsd_kernel.c: usb_pci_root = device_add_child(NULL, "pci", -1);
dev/pci/pci_host_generic.c: device_add_child(dev, "pci", -1);
dev/siba/siba_pcib.c: device_add_child(dev, "pci", -1);
mips/cavium/octopci.c: device_add_child(dev, "pci", device_get_unit(dev));
mips/sibyte/sb_zbpci.c: if (device_add_child(dev, "pci", 0) == NULL)
mips/sibyte/sb_zbpci.c: if (device_add_child(dev, "pci", 1) == NULL)
powerpc/mpc85xx/pci_mpc85xx_pcib.c: child = device_add_child(dev, "pci", -1);
powerpc/ofw/ofw_pcib_pci.c: device_add_child(dev, "pci", -1);
powerpc/ofw/ofw_pci.c: device_add_child(dev, "pci", device_get_unit(dev));
sparc64/pci/apb.c: device_add_child(dev, "pci", -1);
sparc64/pci/fire.c: device_add_child(dev, "pci", -1);
sparc64/pci/ofw_pcib.c: device_add_child(dev, "pci", -1);
sparc64/pci/psycho.c: device_add_child(dev, "pci", -1);
sparc64/pci/schizo.c: device_add_child(dev, "pci", -1);

FIXED:
arm/xscale/i80321/i80321_pci.c: device_add_child(dev, "pci",busno);
arm/xscale/i8134x/i81342_pci.c: device_add_child(dev, "pci", busno);
arm/versatile/versatile_pci.c: device_add_child(dev, "pci", 0);
dev/pci/pci_pci.c: child = device_add_child(dev, "pci", sc->bus.sec);
mips/adm5120/admpci.c: device_add_child(dev, "pci", busno);
mips/atheros/ar71xx_pci.c: device_add_child(dev, "pci", busno);
mips/atheros/ar724x_pci.c: device_add_child(dev, "pci", busno);
mips/atheros/qca955x_pci.c: device_add_child(dev, "pci", busno);
mips/idt/idtpci.c: device_add_child(dev, "pci", busno);
mips/malta/gt_pci.c: device_add_child(dev, "pci", busno);
mips/nlm/xlp_pci.c: device_add_child(dev, "pci", 0);
mips/rmi/xlr_pci.c: device_add_child(dev, "pci", 0);

NOT FIXED (I've got not enough expertise here)
dev/acpica/acpi_pcib.c: if (device_add_child(dev, "pci", busno) == NULL) {
dev/xen/pcifront/pcifront.c: device_add_child(dev, "pci", sc->bus);
x86/pci/pci_bus.c: device_add_child(dev, "pci", bus); (legacy stuff, too scared to touch)
x86/pci/qpi.c: device_add_child(dev, "pci", pcib_get_bus(dev)); (legacy stuff, too scared to touch)
x86/x86/mptable_pci.c: device_add_child(dev, "pci", pcib_get_bus(dev)); (legacy stuff, too scared to touch)

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

wma_semihalf.com retitled this revision from to Add domain support to PCI bus allocation.
wma_semihalf.com updated this object.
wma_semihalf.com edited the test plan for this revision. (Show Details)
wma_semihalf.com added reviewers: jhb, arm64.
wma_semihalf.com set the repository for this revision to rS FreeBSD src repository.
jhb edited edge metadata.Aug 28 2015, 10:15 PM

Sorry, have been moving house and largely offline the past few weeks.

If you fix pci_pci.c you need to fix all the rest. Otherwise you can end up with a pci_pci pcib device using a unit number via -1 that another driver expects to be able to use via a hardcoded unit number. (Suppose the firmware enumerates a host-PCI bridge with higher numbered buses first for example).

All of the ones in your NOT FIXED list need to be fixed. In addition I think these should also be looked at more closely from your GOOD list:

mips/cavium/octopci.c: device_add_child(dev, "pci", device_get_unit(dev));
mips/sibyte/sb_zbpci.c: if (device_add_child(dev, "pci", 0) == NULL)
mips/sibyte/sb_zbpci.c: if (device_add_child(dev, "pci", 1) == NULL)
powerpc/ofw/ofw_pci.c: device_add_child(dev, "pci", device_get_unit(dev));

The sb_zbpci.c driver is abusing the unit number of pci for the PCIB_IVAR_BUS ivar. While that does work, that is
really gross. However, it does mean that fixing it would be more involved as you'd have to fix the ivar. Perhaps too much work for now.

octopci.c looks like it can just use -1 instead of device_get_unit() and be fine.

ofw_pci.c should be fine to use -1 as well.

wma_semihalf.com edited edge metadata.

Fixed everything except sb_zbpci, as you suggested.

jhb accepted this revision.Sep 8 2015, 9:21 PM
jhb edited edge metadata.
This revision is now accepted and ready to land.Sep 8 2015, 9:21 PM
This revision was automatically updated to reflect the committed changes.