[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <07bb7d6a-f06d-7787-f76a-56a78cfb54e9@amd.com>
Date: Wed, 5 Jun 2024 09:42:25 +0530
From: "Nikunj A. Dadhania" <nikunj@....com>
To: Michael Roth <michael.roth@....com>, kvm@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
Sean Christopherson <seanjc@...gle.com>,
Ravi Bangoria <ravi.bangoria@....com>, Srikanth Aithal <sraithal@....com>
Subject: Re: [PATCH] KVM: SEV-ES: Fix svm_get_msr()/svm_set_msr() for
KVM_SEV_ES_INIT guests
On 6/5/2024 5:05 AM, Michael Roth wrote:
> With commit 27bd5fdc24c0 ("KVM: SEV-ES: Prevent MSR access post VMSA
> encryption"), older VMMs like QEMU 9.0 and older will fail when booting
> SEV-ES guests with something like the following error:
>
> qemu-system-x86_64: error: failed to get MSR 0x174
> qemu-system-x86_64: ../qemu.git/target/i386/kvm/kvm.c:3950: kvm_get_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
>
> This is because older VMMs that might still call
> svm_get_msr()/svm_set_msr() for SEV-ES guests after guest boot even if
> those interfaces were essentially just noops because of the vCPU state
> being encrypted and stored separately in the VMSA. Now those VMMs will
> get an -EINVAL and generally crash.
>
> Newer VMMs that are aware of KVM_SEV_INIT2 however are already aware of
> the stricter limitations of what vCPU state can be sync'd during
> guest run-time, so newer QEMU for instance will work both for legacy
> KVM_SEV_ES_INIT interface as well as KVM_SEV_INIT2.
>
> So when using KVM_SEV_INIT2 it's okay to assume userspace can deal with
> -EINVAL, whereas for legacy KVM_SEV_ES_INIT the kernel might be dealing
> with either an older VMM and so it needs to assume that returning
> -EINVAL might break the VMM.
>
> Address this by only returning -EINVAL if the guest was started with
> KVM_SEV_INIT2. Otherwise, just silently return.
>
> Cc: Ravi Bangoria <ravi.bangoria@....com>
> Cc: Nikunj A Dadhania <nikunj@....com>
> Reported-by: Srikanth Aithal <sraithal@....com>
> Closes: https://lore.kernel.org/lkml/37usuu4yu4ok7be2hqexhmcyopluuiqj3k266z4gajc2rcj4yo@eujb23qc3zcm/
> Fixes: 27bd5fdc24c0 ("KVM: SEV-ES: Prevent MSR access post VMSA encryption")
> Signed-off-by: Michael Roth <michael.roth@....com>
Reviewed-by: Nikunj A Dadhania <nikunj@....com>
> ---
> arch/x86/kvm/svm/svm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index b252a2732b6f..c58da281f14f 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2855,7 +2855,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>
> if (sev_es_prevent_msr_access(vcpu, msr_info)) {
> msr_info->data = 0;
> - return -EINVAL;
> + return vcpu->kvm->arch.has_protected_state ? -EINVAL : 0;
> }
>
> switch (msr_info->index) {
> @@ -3010,7 +3010,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
> u64 data = msr->data;
>
> if (sev_es_prevent_msr_access(vcpu, msr))
> - return -EINVAL;
> + return vcpu->kvm->arch.has_protected_state ? -EINVAL : 0;
>
> switch (ecx) {
> case MSR_AMD64_TSC_RATIO:
>
Powered by blists - more mailing lists