Message-ID: <88cd10e8-e9d4-4c11-a494-37e0cf83bad6@amd.com>
Date: Fri, 20 Dec 2024 13:52:20 -0600
From: "Kalra, Ashish" <ashish.kalra@....com>
To: Sean Christopherson <seanjc@...gle.com>,
 Daniel P. Berrangé <berrange@...hat.com>
Cc: Dionna Amalie Glaze <dionnaglaze@...gle.com>, pbonzini@...hat.com,
 tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
 thomas.lendacky@....com, john.allen@....com, herbert@...dor.apana.org.au,
 davem@...emloft.net, michael.roth@....com, kvm@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
 linux-coco@...ts.linux.dev
Subject: Re: [PATCH v2 0/9] Move initializing SEV/SNP functionality to KVM



On 12/20/2024 10:25 AM, Sean Christopherson wrote:
> On Fri, Dec 20, 2024, Daniel P. Berrangé wrote:
>> On Thu, Dec 19, 2024 at 04:04:45PM -0600, Kalra, Ashish wrote:
>>> On 12/18/2024 7:11 PM, Kalra, Ashish wrote:
>>>> But, i believe there is another alternative approach : 
>>>>
>>>> - PSP driver can call SEV Shutdown right before calling DLFW_EX and then do
>>>> a SEV INIT after successful DLFW_EX, in other words, we wrap DLFW_EX with 
>>>> SEV_SHUTDOWN prior to it and SEV INIT post it. This approach will also allow
>>>> us to do both SNP and SEV INIT at KVM module load time, there is no need to
>>>> do SEV INIT lazily or on demand before SEV/SEV-ES VM launch.
>>>>
>>>> This approach should work without any changes in qemu and also allow 
>>>> SEV firmware hotloading without having any concerns about SEV INIT state.
>>>>
>>>
>>> And to add here that SEV Shutdown will succeed with active SEV and SNP guests. 
>>>
>>> SEV Shutdown (internally) marks all SEV asids as invalid and decommission all
>>> SEV guests and does not affect SNP guests. 
>>>
>>> So any active SEV guests will be implicitly shutdown and SNP guests will not be 
>>> affected after SEV Shutdown right before doing SEV firmware hotloading and
>>> calling DLFW_EX command. 
>>>
>>> It should be fine to expect that there are no active SEV guests or any active
>>> SEV guests will be shutdown as part of SEV firmware hotloading while keeping 
>>> SNP guests running.
>>
>> That's a pretty subtle distinction that I don't think host admins will
>> be likely to either learn about or remember. IMHO if there are active
>> SEV guests, the kernel should refuse the run the operation, rather
>> than kill running guests. The host admin must decide whether it is
>> appropriate to shutdown the guests in order to be able to run the
>> upgrade.
> 
> +1 to this and what Dionna said.  Aside from being a horrible experience for
> userspace, trying to forcefully stop actions from within the kernel gets ugly.

Ok, SEV firmware hotloading will refuse the operation if there are active
SEV/SEV-ES guests.

SNP firmware hotloading/DLFW_EX is anyway transparent to SNP guests.

If there are no active SEV/SEV-ES guests, DLFW_EX will do SEV Shutdown
prior to it and SEV INIT post it, to work with the requirement of SEV
to be in UNINIT state to do DLFW_EX.

KVM module load time will do both SNP and SEV INIT. 

There is no reason to support lazy/on-demand SEV INIT when the first SEV VM
is launched, and that anyway can't be supported till qemu is changed to remove
the check for SEV INIT to be done before launching SEV/SEV-ES VMs.

Hopefully this should be the final design for SEV/SNP platform initialization
changes and SEV firmware hotloading support which i can go ahead and implement
and if someone has comments or concerns with the above please let me know.

Thanks,
Ashish

Powered by blists - more mailing lists