Messages in this thread | | | From | Jim Quinlan <> | Date | Fri, 29 Sep 2023 15:24:27 -0400 | Subject | 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 <arnd@arndb.de> wrote: > > On Thu, Sep 28, 2023, at 10:00, Jim Quinlan wrote: > > On Thu, Sep 28, 2023 at 9:32 AM Arnd Bergmann <arnd@arndb.de> wrote: > >> > >> On Thu, Sep 28, 2023, at 08:07, Jim Quinlan wrote: > >> > On Wed, Sep 27, 2023 at 7:10 PM Linus Walleij <linus.walleij@linaro.org> wrote: > >> >> > >> >> 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.
> > >> 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 [unhandled content-type:application/pkcs7-signature] | |