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

[Arnd Bergmann]

On Fri, Sep 29, 2023, at 15:24, Jim Quinlan wrote:
> On Thu, Sep 28, 2023 at 11:17 AM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>> On Thu, Sep 28, 2023, at 10:00, Jim Quinlan wrote:
>
> Our RC is definitely not coherent with the ARM/ARM64 caches.

Ok, thanks for the confirmation.

>> 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.

Ok, so I think we can stop looking at the device drivers for
bugs then.

> 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

Thanks for the thorough testing, that looks very useful, even though
I don't have an answer immediately. Maybe Robin can see something
here that I don't.

It's particularly interesting how arm64 only started failing
on fairly recent kernels, so if nothing else helps you could
always try bisecting the history between 6.1 and 633b47cb009d,
hoping that the commit that broke it points us to the arm32
problem.

The only change I see in that time frame that seems related
is 7bd6680b47fa ("Revert "Revert "arm64: dma: Drop cache
invalidation from arch_dma_prep_coherent()"""), so you could
start by reverting that. However, it's probably something
else since this is for the coherent mappings, not the
streaming ones.

       Arnd