Re: [PATCH] misc: pci_endpoint_test: Use memcpy_toio()/memcpy_fromio() for BAR tests

[Niklas Cassel <cassel@kernel.org>]

Hello Arnd,

On Mon, Mar 18, 2024 at 09:02:21PM +0100, Arnd Bergmann wrote:
> On Mon, Mar 18, 2024, at 20:30, Niklas Cassel wrote:
> > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> > index 705029ad8eb5..cb6c9ccf3a5f 100644
> > --- a/drivers/misc/pci_endpoint_test.c
> > +++ b/drivers/misc/pci_endpoint_test.c
> > @@ -272,31 +272,59 @@ static const u32 bar_test_pattern[] = {
> >  	0xA5A5A5A5,
> >  };
> > 
> > +static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
> > +					enum pci_barno barno, int offset,
> > +					void *write_buf, void *read_buf,
> > +					int size)
> > +{
> > +	memset(write_buf, bar_test_pattern[barno], size);
> > +	memcpy_toio(test->bar[barno] + offset, write_buf, size);
> > +
> > +	/* Make sure that reads are performed after writes. */
> > +	mb();
> > +	memcpy_fromio(read_buf, test->bar[barno] + offset, size);
> 
> Did you see actual bugs without the barrier? On normal PCI
> semantics, a read will always force a write to be flushed first.

I'm aware that a Read Request must not pass a Posted Request under
normal PCI transaction ordering rules.
(As defined in PCIe 6.0, Table 2-42 Ordering Rules Summary)

I was more worried about the compiler or CPU reordering things:
https://github.com/torvalds/linux/blob/master/Documentation/memory-barriers.txt#L1876-L1878

I did try the patch without the barrier, and did not see any issues.


I did also see this comment:
https://github.com/torvalds/linux/blob/master/Documentation/memory-barriers.txt#L2785-L2790

Do you think that we need to perform any flushing after the memset(),
to ensure that the data written using memcpy_toio() is actually what
we expect it to me?


Kind regards,
Niklas