Differential D3406

Add domain support to PCI bus allocation
ClosedPublic
Actions

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

Details

Reviewers

jhb

Group Reviewers

arm64

Commits

rS287882: Add domain support to PCI bus allocation

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 - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

wma_semihalf.com updated this revision to Diff 8019.Aug 18 2015, 6:35 AM

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 - subversion.

Herald added subscribers: andrew, imp. · View Herald TranscriptAug 18 2015, 6:35 AM

Any comments?

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.