Re: [PATCH v1 1/1] ARM: Select DMA_DIRECT_REMAP to fix restricted DMA

On Thu, Sep 28, 2023 at 11:17 AM Arnd Bergmann [off-list ref] wrote:

On Thu, Sep 28, 2023, at 10:00, Jim Quinlan wrote:

quoted

On Thu, Sep 28, 2023 at 9:32 AM Arnd Bergmann [off-list ref] wrote:

quoted

On Thu, Sep 28, 2023, at 08:07, Jim Quinlan wrote:

quoted

On Wed, Sep 27, 2023 at 7:10 PM Linus Walleij [off-list ref] wrote:

quoted

Clearly if you want to do this, surely the ARM-specific
arch/arm/mm/dma-mapping.c and arch/arm/mm/dma-mapping-nommu.c
needs to be removed at the same time?

Yes, this is the reason I used "RFC" as the fix looked too easy to be viable :-)
I debugged it enough to see that the host driver's
writes to the dma_alloc_coherent() region  were not appearing in
memory, and that
led me to DMA_DIRECT_REMAP.

Usually when you see a mismatch between the data observed by the
device and the CPU, the problem is an incorrect "dma-coherent"
property in the DT: either the device is coherent and accesses
the cache but the CPU tries to bypass it because the property
is missing, or there is an extraneous property and the CPU
goes the through the cache but the devices bypasses it.

I just searched, there are no "dt-coherent" properties in our device tree.
Also, even if we did have them, wouldn't things also fail when not using
restricted DMA?

Correct, it should be independent of restricted DMA, but it might
work by chance that way even if it's still wrong. If your DT
is marked as non-coherent (note: the property to look for
is "dma-coherent", not "dt-coherent"), can you check the
datasheet of the SoC to if that is actually correct?

Our PCIe RC device is not dma-coherent.

If the chip is designed to support high-speed devices on
PCIe, it's likely that the PCIe root complex is either coherent
with the caches, or can (and should) be configured that way
for performance reasons.

Our RC is definitely not coherent with the ARM/ARM64 caches.

quoted

quoted

It could also be a driver bug if the device  mixes up the
address spaces, e.g. passing virt_to_phys(pointer) rather
than the DMA address returned by dma_alloc_coherent().

This is an Intel 7260 part using the iwlwifi driver, I doubt it has
errors of that kind.

It's unlikely but not impossible, as the driver has some
unusual constructs, using a lot of coherent mappings that
might otherwise be streaming mappings, and relying on
dma_sync_single_for_device(..., DMA_BIDIRECTIONAL) for other
data, but without the corresponding dma_sync_single_for_cpu().
If all the testing happens on x86, this might easily lead
to a bug that only shows up on non-coherent systems but
is never seen during testing.

If the problem is not the "dma-coherent" property, can you
double-check if using a different PCIe device works, or narrow
down which specific buffer you saw get corrupted?

I've done some testing, below are the results.  The new two devices, a
USB controller
and an M2 NVMe stick, behave the same as iwlwifi.

Note that I'm not advocating that "select DMA_DIRECT_REMAP" is the
anser, I'm just
showing that it fixes my examples.

Regards,
Jim Quinlan
Broadcom STB/CM


VER      PCI-DEV                       <--------- RESTRICTED DMA --------->
                      ARM64    ARM     ARM64    ARM    ARM+DMA_DIRECT_REMAP
5.15     iwlwifi        P       P        P       F             P
5.15     nvme           P       P        P       F             P
5.15     usb            P       P        P       F             P

6.1      iwlwifi        P       P        P       F             P
6.1      nvme           P       P        P       F             P
6.1      usb            P        P       P       F             P

Upstrm   iwlwifi        P       P        F       F             F
Upstrm   nvme           P       P        F       F             F
Upstrm   usb            P       P        F       F             F
                      ARM64    ARM     ARM64    ARM    ARM+DMA_DIRECT_REMAP
VER      PCI-DEV                       <--------- RESTRICTED DMA --------->

LEGEND:
  P       := pass, driver probe and some functional test performed
  F       := fail, usually when probe is called; impossible to do
functional test
  Upstrm  := 633b47cb009d "Merge tag 'scsi-fixes' of
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi"

  iwlwifi := 7260 Wifi 8086:08b1
  nvme    := 1e95:1007
  usb     := Supahub, 1912:0014

     Arnd