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

From: Palmer Dabbelt
Date: Thu Apr 14 2022 - 18:12:48 EST


On Thu, 14 Apr 2022 13:22:42 PDT (-0700), macro@xxxxxxxxxxx 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.

I tried to find the RISC-V definition of inb(), but it's obfuscated
too much to be easily discoverable.

AFAICT the RISC-V port uses definitions from include/asm-generic/io.h.
Palmer, did I get this right?

I'd argue that asm-generic/io.h uses the definitions from RISC-V, but the result is the same ;).

The general idea is that the IO itself is pretty generic for a handful of ports, they just need to be decorated with some fences (or whatever the ISA calls them) before/after the load/store. Those are the __io_p{b,a}{r,w}() macros, which are in riscv's io.h. IIRC they stand for something like Port{Before,After}{Read,Write}.