Re: [PATCH v3 9/9] PCI: endpoint: Set prefetch when allocating memory for 64-bit BARs

[Niklas Cassel <cassel@kernel.org>]

Hello Arnd,

On Mon, Mar 18, 2024 at 08:25:36AM +0100, Arnd Bergmann wrote:
> 
> I'm not sure I follow this logic: If the device wants the
> buffer to act like "normal memory", then it can be marked
> as prefetchable and mapped into the host as write-combining,
> but I think in this case you *don't* want it to be coherent
> on the endpoint side either but use a streaming mapping with
> explicit cache management instead.
> 
> Conversely, if the endpoint side requires a coherent mapping,
> then I think you will want a strictly ordered (non-wc,
> non-frefetchable) mapping on the host side as well.
> 
> It would be helpful to have actual endpoint function drivers
> in the kernel rather than just the test drivers to see what type
> of serialization you actually want for best performance on
> both sides.

Yes, that would be nice.

This specific API, pci_epf_alloc_space(), is only used by the
following drivers:
drivers/pci/endpoint/functions/pci-epf-test.c
drivers/pci/endpoint/functions/pci-epf-ntb.c
drivers/pci/endpoint/functions/pci-epf-vntb.c

pci_epf_alloc_space() is only used to allocate backing
memory for the BARs.


> 
> Can you give a specific example of an endpoint that you are
> actually interested in, maybe just one that we have a host-side
> device driver for in tree?

I personally just care about pci-epf-test, but obviously I don't
want to regress any other user of pci_epf_alloc_space().

Looking at the endpoint side driver:
drivers/pci/endpoint/functions/pci-epf-test.c
and the host side driver:
drivers/misc/pci_endpoint_test.c

On the RC side, allocating buffers that the EP will DMA to is
done using: kzalloc() + dma_map_single().

On EP side:
drivers/pci/endpoint/functions/pci-epf-test.c
uses dma_map_single() when using DMA, and signals completion using MSI.

On EP side:
When reading/writing to the BARs, it simply does:
READ_ONCE()/WRITE_ONCE():
https://github.com/torvalds/linux/blob/v6.8/drivers/pci/endpoint/functions/pci-epf-test.c#L643-L648

There is no dma_sync(), so the pci-test-epf driver currently seems to
depend on the backing memory being allocated by dma_alloc_coherent().


> If you don't care about ordering on that level, I would use
> dma_map_sg() on the endpoint side and prefetchable mapping on
> the host side, with the endpoint using dma_sync_*() to pass
> buffer ownership between the two sides, as controlled by some
> other communication method (non-prefetchable BAR, MSI, ...).

I don't think that there is no big reason why pci-epf-test is
implemented using dma_alloc_coherent() rather than dma_sync()
for the memory backing the BARs, but that is the way it is.

Since I don't feel like totally rewriting pci-epf-test, and since
you say that we shouldn't use dma_alloc_coherent() for the memory
backing the BARs together with exporting the BAR as prefetchable,
I will drop this patch from the series in the next revision.


Kind regards,
Niklas