Re: [PATCH] PCI: Avoid handing out address 0 to devices

From: Bjorn Helgaas
Date: Thu Apr 14 2022 - 19:23:37 EST


On Thu, Apr 14, 2022 at 09:22:42PM +0100, Maciej W. Rozycki wrote:
> On Thu, 14 Apr 2022, Bjorn Helgaas wrote:
>
> > > > > 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:
> > > >
> > > > pci_iomap_range() tests the resource start, i.e., the CPU
> > > > address. I guess the implication is that on RISC-V, the
> > > > CPU-side port address is the same as the PCI bus port address?
> > >
> > > Umm, for all systems I came across except x86, which have
> > > native port I/O access machine instructions, a port I/O
> > > resource records PCI bus addresses of the device rather than
> > > its CPU addresses, which encode the location of an MMIO window
> > > the PCI port I/O space is accessed through.
> >
> > My point is only that it is not necessary for the PCI bus address
> > and the struct resource address, i.e., the argument to inb(), to
> > be the same.
>
> Sure, but I have yet to see a system where it is the case.
>
> Also in principle peer PCI buses could have their own port I/O
> address spaces each mapped via distinct MMIO windows in the CPU
> address space, but I haven't heard of such a system. That of
> course doesn't mean there's no such system in existence.

They do exist, but are probably rare. Even on x86 where multiple host
bridges are now fairly common, and the hardware probably supports a
separate 64K port space for each, the ones I've seen split up a single
64K I/O port space so each bridge only gets a fraction of it. I'm not
sure Linux would even support multiple spaces. I do know ia64
supports multiple port spaces (see __ia64_mk_io_addr()), so we could
have something like this:

pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
pci_bus 0001:00: root bus resource [io 0x10000-0x1ffff] (bus address [0x0000-0xffff])

I guess the question is whether we want to reserve port 0 and MMIO
address 0 as being "invalid". That makes the first space smaller than
the others, but it's not *much* smaller and it's an unlikely
configuration to begin with.

But at the same time, it adds another slightly weird special case in
the already full-of-special-cases alloc code, and I'm somewhat averse
to things like that.

We do have the IORESOURCE_UNSET flag bit that could possibly be used
in pci_iomap_range() instead of testing for "!start". Or maybe
there's a way to allocate address 0 instead of special-casing the
allocator? Just thinking out loud here.

Bjorn