On 9/6/23 15:26, Sean Christopherson wrote:
On Wed, Sep 06, 2023, Tom Lendacky wrote:
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:
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.
Ah, I thought you were saying bail early from kvm_vcpu_reset(). I agree that not
having completely split logic would be ideal. What about this?
/*
* VMCB is undefined after a SHUTDOWN intercept. INIT the vCPU to put
* the VMCB in a known good state. Unfortuately, KVM doesn't have
* KVM_MP_STATE_SHUTDOWN and can't add it without potentially breaking
* userspace. At a platform view, INIT is acceptable behavior as
* there exist bare metal platforms that automatically INIT the CPU
* in response to shutdown.
*
* The VM save area for SEV-ES guests has already been encrypted so it
* cannot be reinitialized, i.e. synthesizing INIT is futile.
*/
if (!sev_es_guest(vcpu->kvm)) {
clear_page(svm->vmcb);
kvm_vcpu_reset(vcpu, true);
}
kvm_run->exit_reason = KVM_EXIT_SHUTDOWN;
return 0;
That looks good to me!
Thanks,
Tom
