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
