Re: [PATCH V2] KVM: SEV: Update SEV-ES shutdown intercepts with more metadata
From: Tom Lendacky
Date: Wed Sep 06 2023 - 16:19:59 EST
On 9/6/23 15:11, Sean Christopherson wrote:
On Wed, Sep 06, 2023, Tom Lendacky wrote:
On 9/6/23 10:14, Peter Gonda wrote:
Currently if an SEV-ES VM shuts down userspace sees KVM_RUN struct with
s/down userspace/down, userspace/
Heh, yeah, I read that the same way you did.
only the INVALID_ARGUMENT. This is a very limited amount of information
to debug the situation. Instead KVM can return a
KVM_EXIT_SHUTDOWN to alert userspace the VM is shutting down and
is not usable any further.
Signed-off-by: Peter Gonda <pgonda@xxxxxxxxxx>
Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Cc: Sean Christopherson <seanjc@xxxxxxxxxx>
Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
Cc: Joerg Roedel <joro@xxxxxxxxxx>
Cc: Borislav Petkov <bp@xxxxxxxxx>
Cc: x86@xxxxxxxxxx
Cc: kvm@xxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
---
arch/x86/kvm/svm/svm.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 956726d867aa..cecf6a528c9b 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2131,12 +2131,14 @@ static int shutdown_interception(struct kvm_vcpu *vcpu)
* The VM save area has already been encrypted so it
* cannot be reinitialized - just terminate.
*/
- if (sev_es_guest(vcpu->kvm))
- return -EINVAL;
+ if (sev_es_guest(vcpu->kvm)) {
+ kvm_run->exit_reason = KVM_EXIT_SHUTDOWN;
+ return 0;
+ }
Just a nit... feel free to ignore, but, since KVM_EXIT_SHUTDOWN is also set
at the end of the function and I don't think kvm_vcpu_reset() clears the
value from kvm_run, you could just set kvm_run->exit_reason on entry and
just return 0 early for an SEV-ES guest.
kvm_run is writable by userspace though, so KVM can't rely on kvm_run->exit_reason
for correctness.
And IIUC, the VMSA is also toast, i.e. doing anything other than marking the VM
dead is futile, no?
I was just saying that "kvm_run->exit_reason = KVM_EXIT_SHUTDOWN;" is in
the shutdown_interception() function twice now (at both exit points of the
function) and can probably just be moved to the top of the function and be
common for both exit points, now, right?
I'm not saying to get rid of it, just set it sooner.
Thanks,
Tom