[PATCH 00/10] KVM: SVM: Attempt to cleanup SEV_FEATURES

Sean Christopherson posted 10 patches 10 months ago
There is a newer version of this series
arch/x86/kvm/svm/sev.c | 218 +++++++++++++++++++----------------------
arch/x86/kvm/svm/svm.c |   7 +-
arch/x86/kvm/svm/svm.h |   2 +-
3 files changed, 106 insertions(+), 121 deletions(-)
[PATCH 00/10] KVM: SVM: Attempt to cleanup SEV_FEATURES
Posted by Sean Christopherson 10 months ago
This is a hastily thrown together series, barely above RFC, to try and
address the worst of the issues that arise with guest controlled SEV
features (thanks AP creation)[1].

In addition to the initial flaws with DebugSwap, I came across a variety
of issues when trying to figure out how best to handle SEV features in
general.  E.g. AFAICT, KVM doesn't guard against userspace manually making
a vCPU RUNNABLE after it has been DESTROYED (or after a failed CREATE).

This is essentially compile-tested only, as I don't have easy access to a
system with SNP enabled.  I ran the SEV-ES selftests, but that's not much
in the way of test coverage.

AMD folks, I would greatly appreciate reviews, testing, and most importantly,
confirmation that all of this actually works the way I think it does.

[1] https://lore.kernel.org/all/Z7TSef290IQxQhT2@google.com

Sean Christopherson (10):
  KVM: SVM: Save host DR masks but NOT DRs on CPUs with DebugSwap
  KVM: SVM: Don't rely on DebugSwap to restore host DR0..DR3
  KVM: SVM: Terminate the VM if a SEV-ES+ guest is run with an invalid
    VMSA
  KVM: SVM: Don't change target vCPU state on AP Creation VMGEXIT error
  KVM: SVM: Require AP's "requested" SEV_FEATURES to match KVM's view
  KVM: SVM: Simplify request+kick logic in SNP AP Creation handling
  KVM: SVM: Use guard(mutex) to simplify SNP AP Creation error handling
  KVM: SVM: Mark VMCB dirty before processing incoming snp_vmsa_gpa
  KVM: SVM: Use guard(mutex) to simplify SNP vCPU state updates
  KVM: SVM: Invalidate "next" SNP VMSA GPA even on failure

 arch/x86/kvm/svm/sev.c | 218 +++++++++++++++++++----------------------
 arch/x86/kvm/svm/svm.c |   7 +-
 arch/x86/kvm/svm/svm.h |   2 +-
 3 files changed, 106 insertions(+), 121 deletions(-)


base-commit: fed48e2967f402f561d80075a20c5c9e16866e53
-- 
2.48.1.601.g30ceb7b040-goog
Re: [PATCH 00/10] KVM: SVM: Attempt to cleanup SEV_FEATURES
Posted by Tom Lendacky 10 months ago
On 2/18/25 19:26, Sean Christopherson wrote:
> This is a hastily thrown together series, barely above RFC, to try and
> address the worst of the issues that arise with guest controlled SEV
> features (thanks AP creation)[1].
> 
> In addition to the initial flaws with DebugSwap, I came across a variety
> of issues when trying to figure out how best to handle SEV features in
> general.  E.g. AFAICT, KVM doesn't guard against userspace manually making
> a vCPU RUNNABLE after it has been DESTROYED (or after a failed CREATE).
> 
> This is essentially compile-tested only, as I don't have easy access to a
> system with SNP enabled.  I ran the SEV-ES selftests, but that's not much
> in the way of test coverage.
> 
> AMD folks, I would greatly appreciate reviews, testing, and most importantly,
> confirmation that all of this actually works the way I think it does.

A quick test of a 64 vCPU SNP guest booted successfully, so that's a
good start. I'll take a closer look at these patches over the next few days.

Thanks,
Tom

> 
> [1] https://lore.kernel.org/all/Z7TSef290IQxQhT2@google.com
> 
> Sean Christopherson (10):
>   KVM: SVM: Save host DR masks but NOT DRs on CPUs with DebugSwap
>   KVM: SVM: Don't rely on DebugSwap to restore host DR0..DR3
>   KVM: SVM: Terminate the VM if a SEV-ES+ guest is run with an invalid
>     VMSA
>   KVM: SVM: Don't change target vCPU state on AP Creation VMGEXIT error
>   KVM: SVM: Require AP's "requested" SEV_FEATURES to match KVM's view
>   KVM: SVM: Simplify request+kick logic in SNP AP Creation handling
>   KVM: SVM: Use guard(mutex) to simplify SNP AP Creation error handling
>   KVM: SVM: Mark VMCB dirty before processing incoming snp_vmsa_gpa
>   KVM: SVM: Use guard(mutex) to simplify SNP vCPU state updates
>   KVM: SVM: Invalidate "next" SNP VMSA GPA even on failure
> 
>  arch/x86/kvm/svm/sev.c | 218 +++++++++++++++++++----------------------
>  arch/x86/kvm/svm/svm.c |   7 +-
>  arch/x86/kvm/svm/svm.h |   2 +-
>  3 files changed, 106 insertions(+), 121 deletions(-)
> 
> 
> base-commit: fed48e2967f402f561d80075a20c5c9e16866e53
Re: [PATCH 00/10] KVM: SVM: Attempt to cleanup SEV_FEATURES
Posted by Tom Lendacky 9 months, 3 weeks ago
On 2/20/25 16:51, Tom Lendacky wrote:
> On 2/18/25 19:26, Sean Christopherson wrote:
>> This is a hastily thrown together series, barely above RFC, to try and
>> address the worst of the issues that arise with guest controlled SEV
>> features (thanks AP creation)[1].
>>
>> In addition to the initial flaws with DebugSwap, I came across a variety
>> of issues when trying to figure out how best to handle SEV features in
>> general.  E.g. AFAICT, KVM doesn't guard against userspace manually making
>> a vCPU RUNNABLE after it has been DESTROYED (or after a failed CREATE).
>>
>> This is essentially compile-tested only, as I don't have easy access to a
>> system with SNP enabled.  I ran the SEV-ES selftests, but that's not much
>> in the way of test coverage.
>>
>> AMD folks, I would greatly appreciate reviews, testing, and most importantly,
>> confirmation that all of this actually works the way I think it does.
> 
> A quick test of a 64 vCPU SNP guest booted successfully, so that's a
> good start. I'll take a closer look at these patches over the next few days.

Everything looks good. I'm going to try messing around with the
DebugSwap feature bit just to try some of those odd cases and make sure
everything does what it is supposed to. Should have results in a day or two.

Thanks,
Tom

> 
> Thanks,
> Tom
> 
>>
>> [1] https://lore.kernel.org/all/Z7TSef290IQxQhT2@google.com
>>
>> Sean Christopherson (10):
>>   KVM: SVM: Save host DR masks but NOT DRs on CPUs with DebugSwap
>>   KVM: SVM: Don't rely on DebugSwap to restore host DR0..DR3
>>   KVM: SVM: Terminate the VM if a SEV-ES+ guest is run with an invalid
>>     VMSA
>>   KVM: SVM: Don't change target vCPU state on AP Creation VMGEXIT error
>>   KVM: SVM: Require AP's "requested" SEV_FEATURES to match KVM's view
>>   KVM: SVM: Simplify request+kick logic in SNP AP Creation handling
>>   KVM: SVM: Use guard(mutex) to simplify SNP AP Creation error handling
>>   KVM: SVM: Mark VMCB dirty before processing incoming snp_vmsa_gpa
>>   KVM: SVM: Use guard(mutex) to simplify SNP vCPU state updates
>>   KVM: SVM: Invalidate "next" SNP VMSA GPA even on failure
>>
>>  arch/x86/kvm/svm/sev.c | 218 +++++++++++++++++++----------------------
>>  arch/x86/kvm/svm/svm.c |   7 +-
>>  arch/x86/kvm/svm/svm.h |   2 +-
>>  3 files changed, 106 insertions(+), 121 deletions(-)
>>
>>
>> base-commit: fed48e2967f402f561d80075a20c5c9e16866e53
Re: [PATCH 00/10] KVM: SVM: Attempt to cleanup SEV_FEATURES
Posted by Kim Phillips 9 months, 3 weeks ago
On 2/24/25 6:02 PM, Tom Lendacky wrote:
> On 2/20/25 16:51, Tom Lendacky wrote:
>> On 2/18/25 19:26, Sean Christopherson wrote:
>>> This is a hastily thrown together series, barely above RFC, to try and
>>> address the worst of the issues that arise with guest controlled SEV
>>> features (thanks AP creation)[1].
>>>
>>> In addition to the initial flaws with DebugSwap, I came across a variety
>>> of issues when trying to figure out how best to handle SEV features in
>>> general.  E.g. AFAICT, KVM doesn't guard against userspace manually making
>>> a vCPU RUNNABLE after it has been DESTROYED (or after a failed CREATE).
>>>
>>> This is essentially compile-tested only, as I don't have easy access to a
>>> system with SNP enabled.  I ran the SEV-ES selftests, but that's not much
>>> in the way of test coverage.
>>>
>>> AMD folks, I would greatly appreciate reviews, testing, and most importantly,
>>> confirmation that all of this actually works the way I think it does.
>>
>> A quick test of a 64 vCPU SNP guest booted successfully, so that's a
>> good start. I'll take a closer look at these patches over the next few days.
> 
> Everything looks good. I'm going to try messing around with the
> DebugSwap feature bit just to try some of those odd cases and make sure
> everything does what it is supposed to. Should have results in a day or two.

My host and guest kernels are based on kvm-x86/next and, following the
instructions under "Tested with:" [1], I don't see gdb stopping on the
watchpoint in the guest gdb session:

...
Reading symbols from a.out...
Hardware watchpoint 1: x
Starting program: /home/ubuntu/a.out
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[Inferior 1 (process 957) exited normally]

It happens regardless of the kvm_amd debug_swap= setting, and
regardless of whether this cleanup series is applied or not.

Doing a break on main and running interactively makes gdb stop at main(),
as it should.

Am I doing something wrong?  Does anyone know whether
DebugSwap under SEV-ES (not just SNP) was tested?

Thanks,

Kim

[1] https://lore.kernel.org/kvm/20230411125718.2297768-6-aik@amd.com/