linux-kernel - Re: [PATCH v5 1/3] drm/shmem: add support for per object caching flags.

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: <78eb099e-020f-91d1-672e-15176bf12cd4@shipmail.org>
Date:   Thu, 27 Feb 2020 14:44:49 +0100
From:   Thomas Hellström (VMware) 
        <thomas_os@...pmail.org>
To:     Gerd Hoffmann <kraxel@...hat.com>
Cc:     dri-devel@...ts.freedesktop.org, Guillaume.Gardet@....com,
        David Airlie <airlied@...ux.ie>,
        open list <linux-kernel@...r.kernel.org>,
        gurchetansingh@...omium.org, tzimmermann@...e.de, yuq825@...il.com,
        noralf@...nnes.org, robh@...nel.org
Subject: Re: [PATCH v5 1/3] drm/shmem: add support for per object caching
 flags.

Hi,

On 2/27/20 2:21 PM, Gerd Hoffmann wrote:
>    Hi,
>
>>> So I'd like to push patches 1+2 to -fixes and sort everything else later
>>> in -next.  OK?
>> OK with me.
> Done.
>
>>> [ context: why shmem helpers use pgprot_writecombine + pgprot_decrypted?
>>>             we get conflicting mappings because of that, linear kernel
>>>             map vs. gem object vmap/mmap ]
>> Do we have any idea what drivers are actually using
>> write-combine and decrypted?
> drivers/gpu/drm# find -name Kconfig* -print | xargs grep -l DRM_GEM_SHMEM_HELPER
> ./lima/Kconfig
> ./tiny/Kconfig
> ./cirrus/Kconfig
> ./Kconfig
> ./panfrost/Kconfig
> ./udl/Kconfig
> ./v3d/Kconfig
> ./virtio/Kconfig
>
> virtio needs cached.
> cirrus+udl should be ok with cached too.
>
> Not clue about the others (lima, tiny, panfrost, v3d).  Maybe they use
> write-combine just because this is what they got by default from
> drm_gem_mmap_obj().  Maybe they actually need that.  Trying to Cc:
> maintainters (and drop stable@).
>
> On decrypted: I guess that is only relevant for virtual machines, i.e.
> virtio-gpu and cirrus?
>
> virtio-gpu needs it, otherwise the host can't show the virtual display.
> cirrus bounces everything via blits to vram, so it should be ok without
> decrypted.  I guess that implies we should make decrypted configurable.

Decrypted here is clearly incorrect and violates the SEV spec, 
regardless of a config option.
The only correct way is currently to use dma_alloc_coherent() and 
mmap_coherent() to allocate decrypted memory and then use the 
pgprot_decrypted flag.

Since the same page is aliased with two physical addresses (one 
encrypted and one decrypted) switching memory from one to the other 
needs extensive handling even to flush stale vmaps...

So if virtio-gpu needs it for some bos, it should move away from shmem 
for those bos.

/Thomas



>
> cheers,
>    Gerd