linux-kernel - Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86

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]

Date:   Wed, 23 Jan 2019 10:08:22 +0100
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     Alex Deucher <alexdeucher@...il.com>,
        Michel Dänzer <michel@...nzer.net>,
        Maxime Ripard <maxime.ripard@...tlin.com>,
        Will Deacon <will.deacon@....com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        amd-gfx list <amd-gfx@...ts.freedesktop.org>,
        David Airlie <airlied@...ux.ie>, Huang Rui <ray.huang@....com>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Junwei Zhang <Jerry.Zhang@....com>,
        Alex Deucher <alexander.deucher@....com>,
        Sean Paul <sean@...rly.run>,
        Christian Koenig <christian.koenig@....com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH] drm: disable WC optimization for cache coherent
 devices on non-x86

On Wed, 23 Jan 2019 at 08:15, Christoph Hellwig <hch@...radead.org> wrote:
>
> On Tue, Jan 22, 2019 at 10:07:07PM +0100, Ard Biesheuvel wrote:
> > Yes, so much was clear. And the reason this breaks on some arm64
> > systems is because
> > a) non-snooped PCIe TLP attributes may be ignored, and
> > b) non-x86 CPUs do not snoop the caches when accessing uncached mappings.
> >
> > I don't think there is actually any disagreement on this part. And I
> > think my patch is reasonable, only Christoph is objecting to it on the
> > grounds that drivers should not go around the DMA API and create
> > vmap()s of DMA pages with self chosen attributes.
>
> I object to it on various grounds.  While the above is correct it really
> is a mid to long-term fix.
>
> But even in the short term your patch just maintains a random list of
> idefs in a random driver, pokes into the dma-mapping internals and lacks
> any comments in the code explaining on what is going on, leading to
> futher cargo culting.  So it very clearly is not acceptable.

Fair enough. It would be helpful, though, if you could give more
guidance on what you do find acceptable. You haven't answered my
question on whether you think drivers should be allowed to reason at
all about the device's cache coherence.

In any case, I think we (you and I) could easily agree on the correct fix being:
- introduce DMA_ATTR_NOSNOOP
- implement it as a no-op for non-cache coherent devices (since
nosnoop is implied)
- permit non-x86 architectures to override/ignore it if not supported
- rip out all the explicit vmap()/kmap()s of DMA pages from the DRM
drivers, and instead, set the DMA_ATTR_NOSNOOP attribute where
desired, and always use the mappings provided by the DMA API.

The problem is that we will need the DRM/radeon/amdgpu maintainers on
board with this, and until this happens, I am stuck in the middle with
a broken arm64 system.

So, given the above, what /would/ you find acceptable?