On Mon, Sep 18, 2023 at 3:06 PM Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx> wrote: > > > On 9/18/23 17:52, Zack Rusin wrote: > > On Mon, 2023-09-18 at 17:13 +0200, Thomas Hellström wrote: > >> Hi, > >> > >> On 9/18/23 16:56, Thomas Hellström wrote: > >>> Hi Zack, Christian > >>> > >>> On 9/18/23 13:36, Christian König wrote: > >>>> Hi Zack, > >>>> > >>>> adding Thomas and Daniel. > >>>> > >>>> I briefly remember that I talked with Thomas and some other people > >>>> about that quite a while ago as well, but I don't fully remember the > >>>> outcome. > >>> Found one old thread, but didn't read it: > >>> > >>> https://lists.freedesktop.org/archives/dri-devel/2019-September/234100.html > >>> > >>> > >>> /Thomas > >>> > >>> > >> Ugh. Now starting to read that thread I have a vague recollection it all > >> ended with not supporting mapping any device pages whatsoever when SEV > >> was enabled, but rather resorting to llvmpipe and VM-local bos. > > Hi, Thomas. > > > > Thanks for finding this! I'd (of course) like to solve it properly and get vmwgfx > > running with 3d support with SEV-ES active instead of essentially disabling the > > driver when SEV-ES is active. > > > > I think there are two separate discussions there, the non-controversial one and the > > controversial one: > > 1) The non-controversial: is there a case where drivers would want encrypted memory > > for TT pages but not for io mem mappings? Because if not then as Christian pointed > > out we could just add pgprot_decrypted to ttm_io_prot and be essentially done. The > > current method of decrypting io mem but leaving sys mem mappings encrypted is a bit > > weird anyway. > > > > If the answer to that question is "yes, some driver does want the TT mappings to be > > encrypted" then your "[PATCH v2 3/4] drm/ttm, drm/vmwgfx: Correctly support support > > AMD memory encryption" solves that. I think getting one of those two in makes sense > > regardless of everything else, agreed? > > Well, there is more to it I think. > > IIRC, the AMD SME encryption mode has a way for a device to have the > memory controller (?) encrypt / decrypt device traffic by using an > address range alias, so in theory it supports encrypted TT pages, and > the dma-layer may indeed hand encrypted DMA pages to TTM on such systems > depending on the device's DMA mask. That's why I think that > force_dma_unencrypted() export was needed, and If the amdgpu driver > accesses TT memory in SME mode *without* pgprot_decrypted() and it still > works, then I think that mode is actually used. How could it otherwise work? For SME, as long as the encrypted bit is set in the physical address used for DMA, the memory controller will handle the encrypt/decrypt for the device. For devices with a limited dma mask, you need to use the IOMMU so that the encrypted bit is retained when the address hits the memory controller. Alex > > But anyway, I agree SEV-ES mode would *always* want pgprot_decrypted for > dma allocated memory, whether it's in PL_SYSTEM or in PL_TT, but I guess > the above SME case need to be sorted out first. > > > > > 2) The controversial part of your series seems to be exporting of > > force_dma_unencrypted and its usage within ttm. Personally I'm perfectly ok with > > ignoring that for now. Getting #1 in is still a million times (rounded up to the > > nearest million) faster than just using llvmpipe in the guest. And getting #1 also > > fixes modern presentation on vmwgfx. > > > > > > So #1 is "make the driver work", while #2 is an optimisation - I think we should > > treat them as separate things/series. Does that make sense? > > I guess that depends on the tradeoff between security and speed for > people running SEV-enabled VMs. But giving them a choice is probably not > a bad idea. > > /Thomas > > > > > > z > > > > P.S. I forgot to cc dri-devel on the initial email, that wasn't by design. Sorry. > > I'm ok with moving the discussion to dri-devel.
