Hi Prabhakar-san,
> From: Lad, Prabhakar, Sent: Wednesday, April 1, 2020 5:00 PM
<snip>
> > > root@hihope-rzg2m:~# pcitest -r -s 2176
> > <snip>
> > > [ 528.556991] pci-endpoint-test 0000:01:00.0: pci_endpoint_test_read
> > > ffff0004b61f9000 7e951000 910c690d=910c690d
> >
> > I'd like to know how to print these crc values. Your report on the case 1
> > mentioned on pci-epf-test.c like below though.
> >
> > > + dev_err(dev, "%s %llx %llx csum: %x = %x\n", __func__, reg->dst_addr,
> > > + phys_addr, reg->checksum, crc32_le(~0, dst_addr, reg->size));
> >
> > Also, as I mentioned on previous email, this is possible to cause timing issue.
> > So, I'd like to where you added the hexdump on pci_endpoint_test.c.
> > Perhaps, copy and pasting your whole debug diff is useful to understand about it.
> >
>
> Following is the complete diff:
Thanks!
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 3c49514b4813..e7bf58a1fee8 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -561,6 +561,23 @@ static bool pci_endpoint_test_write(struct
> pci_endpoint_test *test,
> return ret;
> }
>
> +static void print_buffer(struct device *dev, void *buff_addr, size_t size)
> +{
> + size_t i;
> + u64 *addr = buff_addr;
> +
> + for(i = 0; i < size; i += sizeof(addr))
> + dev_err(dev, "buf[%zu] : %llx\n", i, *addr);
> +
> + for(i = 0; i < size; i +=1) {
> + if (0x910c690d == crc32_le(~0, buff_addr, i))
> + dev_err(dev, "%x\n",
> + crc32_le(~0, buff_addr, i));
> + }
> +
> + dev_err(dev, "\n\n\n\n");
> +}
> +
> static bool pci_endpoint_test_read(struct pci_endpoint_test *test,
> unsigned long arg)
> {
> @@ -608,7 +625,7 @@ static bool pci_endpoint_test_read(struct
> pci_endpoint_test *test,
> }
I'd like to allocate temporary buffer here to copy data later...
void *tmp;
...
tmp = kzalloc(size + alignment, GFP_KERNEL);
> orig_phys_addr = dma_map_single(dev, orig_addr, size + alignment,
> - DMA_FROM_DEVICE);
> + DMA_BIDIRECTIONAL);
> if (dma_mapping_error(dev, orig_phys_addr)) {
> dev_err(dev, "failed to map source buffer address\n");
> ret = false;
> @@ -640,12 +657,17 @@ static bool pci_endpoint_test_read(struct
> pci_endpoint_test *test,
> wait_for_completion(&test->irq_raised);
>
> dma_unmap_single(dev, orig_phys_addr, size + alignment,
> - DMA_FROM_DEVICE);
> + DMA_BIDIRECTIONAL);
And then, I'd like to copy addr buffer to the tmp here.
memcpy(tmp, addr, size);
> crc32 = crc32_le(~0, addr, size);
And addr is replaced with tmp;
> if (crc32 == pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_CHECKSUM))
> ret = true;
>
> + print_buffer(dev, addr, size);
This addr is also replaced with tmp;
> + dev_err(dev, "%s %px %llx %x=%x\n", __func__, orig_addr,
> + orig_phys_addr, crc32,
> + pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_CHECKSUM));
The crc32 value was calculated before print the buffer.
This means that timing is difference between crc32_le(addr) and print_buffer(addr).
So, I'd like to copy the addr buffer to tmp and use the tmp for crc32_le() and print_buffer().
> +
> err_phys_addr:
> kfree(orig_addr);
> err:
>
> Note: I have added DMA_BIDIRECTIONAL that is because I am also
> printing the buffer on ep
> and calulating the crc
>
> > > READ ( 2176 bytes): OKAY
> > > root@hihope-rzg2m:~# pcitest -r -s 2176
> > <snip>
> > > [ 533.457921] pci-endpoint-test 0000:01:00.0: pci_endpoint_test_read
> > > ffff0004b61f9000 7e959800 ce535039=910c690d
> > > READ ( 2176 bytes): NOT OKAY
> > >
> > > Note: for failure case the crc is always ce535039
> >
> > The value of ce535039 is from pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_CHECKSUM)?
> > If so, it's strange because all 0xdf values with 2176 bytes should be 910c690d by crc32_le().
> >
> The value ce535039 is the one which is calculated from the buffer and
> value 910c690d is the one
> which is passed from the BAR memory which is correct.
I got it. If my guess is correct, using the tmp buffer above can print
the buffer with wrong value(s).
By the way, your PCIe host environment (RZ/G2N) output the following log?
[ 0.000000] software IO TLB: mapped [mem 0x7bfff000-0x7ffff000] (64MB)
If so, I guess this issue is related to this software IO TLB behavior.
IIUC, if we use coherent buffer or GPF_DMA buffer, the kernel will not
use software IO TLB for these buffers.
Best regards,
Yoshihiro Shimoda
> Cheers,
> --Prabhakar
