Decrypting tt maps in ttm

[Thomas Hellström]

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?

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.