lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+-6iNx1YennqtbvNTRi8dQ=RK4LrOngQTipWkWz1Om6-CFzbA@mail.gmail.com>
Date:   Fri, 29 Sep 2023 17:13:00 -0400
From:   Jim Quinlan <james.quinlan@...adcom.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Christoph Hellwig <hch@....de>,
        bcm-kernel-feedback-list@...adcom.com, jim2101024@...il.com,
        Russell King <linux@...linux.org.uk>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Russell King <rmk+kernel@...linux.org.uk>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jonathan Corbet <corbet@....net>,
        Thomas Gleixner <tglx@...utronix.de>,
        Sebastian Reichel <sebastian.reichel@...labora.com>,
        Mike Rapoport <rppt@...nel.org>,
        Eric DeVolder <eric.devolder@...cle.com>,
        Nathan Chancellor <nathan@...nel.org>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Christophe Leroy <christophe.leroy@...roup.eu>,
        "moderated list:ARM PORT" <linux-arm-kernel@...ts.infradead.org>,
        open list <linux-kernel@...r.kernel.org>,
        Claire Chang <tientzu@...omium.org>,
        Robin Murphy <robin.murphy@....com>
Subject: Re: [PATCH v1 1/1] ARM: Select DMA_DIRECT_REMAP to fix restricted DMA

On Fri, Sep 29, 2023 at 3:52 PM Arnd Bergmann <arnd@...db.de> wrote:
>
> On Fri, Sep 29, 2023, at 15:24, Jim Quinlan wrote:
> > On Thu, Sep 28, 2023 at 11:17 AM Arnd Bergmann <arnd@...db.de> 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.

Yes I plan to do that.

There's also one other datapoint I have but have only tried it on 6.1
and 5.15 ARM  builds.
I can set a board to run its PCIe  HW in EP mode, and then connect it
to a board running its
PCIe HW in RC mode.

On the RC side, I've written a small PCIe host driver [1].  On the EP
board, I just run this shell
command when I get the prompt:

while true ; do
    for i in `seq 0 15` ; do
        devmem $(printf "0x%x" $((0x50a000000 + ($i << 16))))
    done
    echo
    sleep 1
done

You will have to take my word that I've configured the PCIe window on
the EP board at 0x5_0a000000
to correspond to the restricted memory region on the RC side (0x4a000000).
With ARM+restricted DMA  I see something like this as the output [2].
The values should appear
all at once or at least in order -- as they do on ARM64 -- but they do
not for ARM.

FWIW,
Jim Quinlan
Broadcom STB/CM

[1]
#define NUM_DMA_REGIONS 16

for (i = 0; i < NUM_DMA_REGIONS; i++)
        va[i] = dma_alloc_coherent(dev, size, &dma_addr, flags);

for (i = 0; i < NUM_DMA_REGIONS; i++)
        memset(va[i], 0, size/sizeof(u32));

printk("\n================ START READING AT EP  ======================\n");
msleep(4000);
for (i = 0; i < NUM_DMA_REGIONS; i++) {
        pu32 = va[i];
        for (j = 0; j < size/sizeof(u32); j += 64) {
                uint32_t x = pu32[j];

                mdelay(3);
                pu32[j] = 0x12340000 + (i << 12) + j + (x & 0x0f00);
                wmb();
        }
}

[2]
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000

0x00000000
0x00000000
0x12342000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000

0x12340000
0x00000000
0x12342000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000

0x12340000
0x00000000
0x12342000
0x12343000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000

0x12340000
0x00000000
0x12342000
0x12343000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000

0x12340000
0x12341000
0x12342000
0x12343000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000

0x12340000
0x12341000
0x12342000
0x12343000
0x00000000
0x00000000
0x00000000
0x12347000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000

0x12340000
0x12341000
0x12342000
0x12343000
0x00000000
0x00000000
0x00000000
0x12347000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000
0x00000000

 ^C
>
>
> 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

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4210 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ