Page MenuHomeFreeBSD

Retire non-NEW_PCIB code and remove config option
AcceptedPublic

Authored by jrtc27 on Nov 12 2021, 1:17 AM.

Details

Reviewers
jhb
manu
imp
Summary

All of amd64, arm64, i386, powerpc and riscv enable NEW_PCIB in DEFAULTS,
leaving just arm (which still enables it for GENERIC and TEGRA124) and mips
(which still enables it for RT2880_FDT and various std.mediatek-using kernel
configs), so it's time they caught up and we removed the legacy code that no
longer sees much testing.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 42744
Build 39632: arc lint + arc unit

Event Timeline

This revision is now accepted and ready to land.Nov 12 2021, 2:01 AM

... leaving just arm (which still enables it for GENERIC and TEGRA124) and mips
(which still enables it for RT2880_FDT and various std.mediatek-using kernel
configs), ...

For everything that already had NEW_PCIB, this change is a no-op, obviously. But what about those configs which haven't had NEW_PCIB until now? Presumably the reason they didn't have it until now is because there was some incompatibility with NEW_PCIB, but I don't see anything which would resolve those issues.

sys/dev/pci/pci_pci.c
136

This drops the PCI_HP check in addition to NEW_PCIB; is that what you intended?

1575

These drop the PCI_HP check as well as the NEW_PCIB check; intentional?

... leaving just arm (which still enables it for GENERIC and TEGRA124) and mips
(which still enables it for RT2880_FDT and various std.mediatek-using kernel
configs), ...

For everything that already had NEW_PCIB, this change is a no-op, obviously. But what about those configs which haven't had NEW_PCIB until now? Presumably the reason they didn't have it until now is because there was some incompatibility with NEW_PCIB, but I don't see anything which would resolve those issues.

All the pcib drivers in sys/arm support NEW_PCIB as far as I can tell, by virtue of deriving from either ofw_pcib_driver or generic_pcie_fdt_driver, or implementing support themselves like mv_pci. And the only board not supposedly supported by GENERIC whose board-specific kernel config enables device pci is ALPINE, whose PCI controller is just a thin wrapper around generic_pcie_fdt_driver.

The only issue therefore is mips, which has some drivers that indeed appear to not be compatible. Testing a MALTA64 kernel in QEMU it's no longer able to attach to pci0 as it can't allocate a bus number; mips's resource.h unconditionally defines PCI_RES_BUS whenever NEW_PCIB is defined, but not all its pcib drivers support it. From what I can see, all of ar71xx_pci, ar724x_pci, qca955x_pci, octopci, gt_pci and xlp_pci will fail PCI_RES_BUS allocations, with just mtk_pcie supporting it, hence why that was the only config with it. So I guess we just wait until we're free from mips; I could fix gt_pci so MALTA64 keeps working (which, given it's what gets used for QEMU, is about the only kernel config anyone's actually using) but that would be a waste of time.

sys/dev/pci/pci_pci.c
136

Yes, because true || X is always true, it's only where it's an && that the RHS needs to stay

(the device pci but currently not NEW_PCIB arm kernel configs being just ALPINE, ARMADA38X and ARMADAXP)