Re: PCI endpoint: pci-epf-test is broken on big-endian

[Damien Le Moal <dlemoal@kernel.org>]

On 10/22/24 12:26, Manivannan Sadhasivam wrote:
>>> Looking at pci-endpoint-test however, it does all its accesses using
>>> readl() and writel(), and if you look at the implementations of
>>> readl()/writel():
>>> https://github.com/torvalds/linux/blob/v6.12-rc4/include/asm-generic/io.h#L181-L184
>>>
>>> They convert to CPU native after reading, and convert to little-endian
>>> before writing, so pci-endpoint-test (RC side driver) is okay, it is
>>> just pci-epf-test (EP side driver) that is broken.
>>
>> That in itself is another problem. The use of readl/writel for things in the EPF
>> BAR memory is also *wrong*, because that memory is NOT a real mmio memory. We
>> should be using READ_ONCE()/WRITE_ONCE() to treat the BAR as volatile memory but
>> not use readl/writel.
>>
> 
> Not at all. The memory returned by pci_ioremap_bar() is annotated with __iomem,
> which means it should *only* be accessed with the relevant accessors like
> readl(), ioread32() etc... The memory is still treated as MMIO, so all the
> restrictions (alignment) applies to it also.

You are talking about the host (RC side) ? I was talking about the EP side...

pci_epf_alloc_space() returns a "void *" pointer, and the same is true for the
dma_alloc_coherent() call that actually allocates the BAR space. So on the EP, a
BAR memory should be treated as regular memory, but "volatile" since it can
change under the driver (due to the host writing to the BAR). Hence
READ_ONCE()/WRITE_ONCE() is the correct way to access a BAR on the endpoint.

And yes, readl() and friends are for the PCI RC (host) side, that I know.

Sorry about the confusing comment. I was thinking about the EP while reading
Niklas's comment :)

-- 
Damien Le Moal
Western Digital Research