| Message ID | alpine.DEB.2.21.2204271207590.9383@angie.orcam.me.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISC-V: PCI: Avoid handing out address 0 to devices | expand |
On Wed, 27 Apr 2022, Maciej W. Rozycki wrote: > Therefore avoid handing out address 0, by bumping the lowest address > available to PCI via PCIBIOS_MIN_IO and PCIBIOS_MIN_MEM up by 4 and 16 > respectively, which is the minimum allocation size for I/O and memory > BARs. Ping for: <https://lore.kernel.org/lkml/alpine.DEB.2.21.2204271207590.9383@angie.orcam.me.uk/> Maciej
On Wed, 27 Apr 2022, Maciej W. Rozycki wrote: > Therefore avoid handing out address 0, by bumping the lowest address > available to PCI via PCIBIOS_MIN_IO and PCIBIOS_MIN_MEM up by 4 and 16 > respectively, which is the minimum allocation size for I/O and memory > BARs. Ping for: <https://lore.kernel.org/lkml/alpine.DEB.2.21.2204271207590.9383@angie.orcam.me.uk/> Maciej
On Wed, 22 Jun 2022 04:04:09 PDT (-0700), macro@orcam.me.uk wrote: > On Wed, 27 Apr 2022, Maciej W. Rozycki wrote: > >> Therefore avoid handing out address 0, by bumping the lowest address >> available to PCI via PCIBIOS_MIN_IO and PCIBIOS_MIN_MEM up by 4 and 16 >> respectively, which is the minimum allocation size for I/O and memory >> BARs. > > Ping for: > <https://lore.kernel.org/lkml/alpine.DEB.2.21.2204271207590.9383@angie.orcam.me.uk/> Sorry, I got this mixed up with the non-RISC-V patch. David poked me about it, this is on for-next. It's passing my tests, but they're just QEMU so probably not all that exciting here.
On Wed, 22 Jun 2022, Palmer Dabbelt wrote: > > > Therefore avoid handing out address 0, by bumping the lowest address > > > available to PCI via PCIBIOS_MIN_IO and PCIBIOS_MIN_MEM up by 4 and 16 > > > respectively, which is the minimum allocation size for I/O and memory > > > BARs. > > > > Ping for: > > <https://lore.kernel.org/lkml/alpine.DEB.2.21.2204271207590.9383@angie.orcam.me.uk/> > > Sorry, I got this mixed up with the non-RISC-V patch. If you mean this: <https://lore.kernel.org/lkml/alpine.DEB.2.21.2202260044180.25061@angie.orcam.me.uk/> then we just don't have consensus to move forward. If we ever do for a generic change, then we can revert the RISC-V platform solution, as it's merely an internal implementation detail and not a part of the ABI or something. > David poked me about > it, this is on for-next. It's passing my tests, but they're just QEMU so > probably not all that exciting here. Thanks! I don't know offhand what QEMU supports as far as the RISC-V architecture is concerned; I guess you can't just enable a PCI port-I/O serial port in the simulator and see if it works with Linux or not. Anyway it's just number shuffling, so the change should be reasonably safe. Maciej
Index: linux-macro/arch/riscv/include/asm/pci.h =================================================================== --- linux-macro.orig/arch/riscv/include/asm/pci.h +++ linux-macro/arch/riscv/include/asm/pci.h @@ -12,8 +12,8 @@ #include <asm/io.h> -#define PCIBIOS_MIN_IO 0 -#define PCIBIOS_MIN_MEM 0 +#define PCIBIOS_MIN_IO 4 +#define PCIBIOS_MIN_MEM 16 /* RISC-V shim does not initialize PCI bus */ #define pcibios_assign_all_busses() 1
For RISC-V platforms we permit assigning addresses from 0 to PCI devices, both in the memory and the I/O bus space, and we happily do so if there is no conflict, e.g.: pci 0000:07:00.0: BAR 0: assigned [io 0x0000-0x0007] pci 0000:07:00.1: BAR 0: assigned [io 0x0008-0x000f] pci 0000:06:01.0: PCI bridge to [bus 07] pci 0000:06:01.0: bridge window [io 0x0000-0x0fff] (with the SiFive HiFive Unmatched RISC-V board and a dual serial port option card based on the OxSemi OXPCIe952 device wired for the legacy UART mode). Address 0 is treated specially however in many places, for example in `pci_iomap_range' and `pci_iomap_wc_range' we require that the start address is non-zero, and even if we let such an address through, then individual device drivers could reject a request to handle a device at such an address, such as in `uart_configure_port'. Consequently given devices configured as shown above only one is actually usable: Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled serial 0000:07:00.0: enabling device (0000 -> 0001) serial: probe of 0000:07:00.0 failed with error -12 serial 0000:07:00.1: enabling device (0000 -> 0001) serial 0000:07:00.1: detected caps 00000700 should be 00000500 0000:07:00.1: ttyS0 at I/O 0x8 (irq = 39, base_baud = 15625000) is a 16C950/954 Therefore avoid handing out address 0, by bumping the lowest address available to PCI via PCIBIOS_MIN_IO and PCIBIOS_MIN_MEM up by 4 and 16 respectively, which is the minimum allocation size for I/O and memory BARs. With this in place the system in question we have: pci 0000:07:00.0: BAR 0: assigned [io 0x1000-0x1007] pci 0000:07:00.1: BAR 0: assigned [io 0x1008-0x100f] pci 0000:06:01.0: PCI bridge to [bus 07] pci 0000:06:01.0: bridge window [io 0x1000-0x1fff] and then devices work correctly: Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled serial 0000:07:00.0: enabling device (0000 -> 0001) serial 0000:07:00.0: detected caps 00000700 should be 00000500 0000:07:00.0: ttyS0 at I/O 0x1000 (irq = 38, base_baud = 15625000) is a 16C950/954 serial 0000:07:00.1: enabling device (0000 -> 0001) serial 0000:07:00.1: detected caps 00000700 should be 00000500 0000:07:00.1: ttyS1 at I/O 0x1008 (irq = 39, base_baud = 15625000) is a 16C950/954 Especially I/O space ranges are particularly valuable, because bridges only decode bits from 12 up and consequently where 16-bit addressing is in effect, as few as 16 separate ranges can be assigned to individual buses only, however a generic change to avoid handing out address 0 only has turned out controversial as per the discussion referred via the link below. Conversely sorting this out in platform code has been standard practice since forever to avoid a clash with legacy devices subtractively decoded by the southbridge where present. This can be revised should a generic solution be adopted sometime. Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk> Link: https://lore.kernel.org/r/alpine.DEB.2.21.2202260044180.25061@angie.orcam.me.uk --- Hi, NB I have an OxSemi OXPCIe952 based card that can be wired to either the native or the legacy mode via a jumper block and I am so glad that I have checked whether it works in the legacy mode as well. I guess there are so few legacy-free platforms still for nobody else to notice this issue yet. Maciej --- arch/riscv/include/asm/pci.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) linux-riscv-pcibios-min.diff