From: Tom Lendacky <thomas.lendacky@amd.com>
To: Sean Christopherson <seanjc@google.com>, John Allen <john.allen@amd.com>
Cc: kvm@vger.kernel.org, weijiang.yang@intel.com,
rick.p.edgecombe@intel.com, bp@alien8.de, pbonzini@redhat.com,
mlevitsk@redhat.com, linux-kernel@vger.kernel.org,
x86@kernel.org
Subject: Re: [PATCH v2 6/9] KVM: SVM: Add MSR_IA32_XSS to the GHCB for hypervisor kernel
Date: Thu, 2 May 2024 12:46:02 -0500 [thread overview]
Message-ID: <9b05e2d7-ac1c-e60f-0f6e-f4befea06334@amd.com> (raw)
In-Reply-To: <ZjLTr0n0nwBrZW36@google.com>
On 5/1/24 18:43, Sean Christopherson wrote:
> On Mon, Feb 26, 2024, John Allen wrote:
>> When a guest issues a cpuid instruction for Fn0000000D_x0B
>> (CetUserOffset), KVM will intercept and need to access the guest
>> MSR_IA32_XSS value. For SEV-ES, this is encrypted and needs to be
>> included in the GHCB to be visible to the hypervisor.
>
> Heh, too many pronouns and implicit subjects. I read this, several times, as:
>
> When a guest issues a cpuid instruction for Fn0000000D_x0B
> (CetUserOffset), KVM will intercept MSR_IA32_XSS and need to access the
> guest MSR_IA32_XSS value.
>
> I think you mean this?
>
> When a vCPU executes CPUID.0xD.0xB (CetUserOffset), KVM will intercept
> and emulate CPUID. To emulate CPUID, KVM needs access to the vCPU's
> MSR_IA32_XSS value. For SEV-ES guests, XSS is encrypted, and so the guest
> must include its XSS value in the GHCB as part of the CPUID request.
>
> Hmm, I suspect that last sentence is wrong though. Question on that below.
>
>> Signed-off-by: John Allen <john.allen@amd.com>
>> ---
>> v2:
>> - Omit passing through XSS as this has already been properly
>> implemented in a26b7cd22546 ("KVM: SEV: Do not intercept
>> accesses to MSR_IA32_XSS for SEV-ES guests")
>> ---
>> arch/x86/include/asm/svm.h | 1 +
>> arch/x86/kvm/svm/sev.c | 9 +++++++--
>> arch/x86/kvm/svm/svm.h | 1 +
>> 3 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
>> index 728c98175b9c..44cd41e2fb68 100644
>> --- a/arch/x86/include/asm/svm.h
>> +++ b/arch/x86/include/asm/svm.h
>> @@ -673,5 +673,6 @@ DEFINE_GHCB_ACCESSORS(sw_exit_info_1)
>> DEFINE_GHCB_ACCESSORS(sw_exit_info_2)
>> DEFINE_GHCB_ACCESSORS(sw_scratch)
>> DEFINE_GHCB_ACCESSORS(xcr0)
>> +DEFINE_GHCB_ACCESSORS(xss)
>>
>> #endif
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index f06f9e51ad9d..c3060d2068eb 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -2458,8 +2458,13 @@ static void sev_es_sync_from_ghcb(struct vcpu_svm *svm)
>>
>> svm->vmcb->save.cpl = kvm_ghcb_get_cpl_if_valid(svm, ghcb);
>>
>> - if (kvm_ghcb_xcr0_is_valid(svm)) {
>> - vcpu->arch.xcr0 = ghcb_get_xcr0(ghcb);
>> + if (kvm_ghcb_xcr0_is_valid(svm) || kvm_ghcb_xss_is_valid(svm)) {
>> + if (kvm_ghcb_xcr0_is_valid(svm))
>> + vcpu->arch.xcr0 = ghcb_get_xcr0(ghcb);
>> +
>> + if (kvm_ghcb_xss_is_valid(svm))
>> + vcpu->arch.ia32_xss = ghcb_get_xss(ghcb);
>> +
>> kvm_update_cpuid_runtime(vcpu);
>
> Pre-existing code, but isn't updating CPUID runtime on every VMGEXIT super wasteful?
> Or is the guest behavior to mark XCR0 and XSS as valid only when changing XCR0/XSS?
It's not really on every VMGEXIT. It's only if those values have been
supplied in the GHCB will the CPUID runtime be updated. And the Linux
guest code supplies XCR0 and XSS only on a CPUID VMGEXIT.
Both sides of that can optimized. The guest can be optimized down to
just supplying the values on CPUID 0xD or even further to only supplying
the values if they have changed since the last time they were supplied.
The hypervisor side could be optimized to compare the value and only
update the CPUID runtime if those values are different.
Thanks,
Tom
> If so, the last sentence of the changelog should be something like:
>
> MSR_IA32_XSS value. For SEV-ES guests, XSS is encrypted, and so the guest
> must notify the host of XSS changes by performing a ??? VMGEXIT and
> providing its XSS value in the GHCB.
next prev parent reply other threads:[~2024-05-02 17:46 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-26 21:32 [PATCH v2 0/9] SVM guest shadow stack support John Allen
2024-02-26 21:32 ` [PATCH v2 1/9] x86/boot: Move boot_*msr helpers to asm/shared/msr.h John Allen
2024-02-27 19:45 ` Borislav Petkov
2024-02-26 21:32 ` [PATCH v2 2/9] KVM: x86: SVM: Emulate reads and writes to shadow stack MSRs John Allen
2024-02-26 21:32 ` [PATCH v2 3/9] KVM: x86: SVM: Update dump_vmcb with shadow stack save area additions John Allen
2024-02-26 21:32 ` [PATCH v2 4/9] KVM: x86: SVM: Pass through shadow stack MSRs John Allen
2024-02-26 21:32 ` [PATCH v2 5/9] KVM: SVM: Rename vmplX_ssp -> plX_ssp John Allen
2024-02-27 18:14 ` Sean Christopherson
2024-02-27 19:15 ` Tom Lendacky
2024-02-27 19:19 ` John Allen
2024-02-27 19:23 ` Sean Christopherson
2024-02-27 19:25 ` John Allen
2024-02-26 21:32 ` [PATCH v2 6/9] KVM: SVM: Add MSR_IA32_XSS to the GHCB for hypervisor kernel John Allen
2024-05-01 23:43 ` Sean Christopherson
2024-05-02 17:46 ` Tom Lendacky [this message]
2024-05-02 18:34 ` Sean Christopherson
2024-02-26 21:32 ` [PATCH v2 7/9] x86/sev-es: Include XSS value in GHCB CPUID request John Allen
2024-02-27 19:47 ` Borislav Petkov
2024-02-26 21:32 ` [PATCH v2 8/9] KVM: SVM: Use KVM-governed features to track SHSTK John Allen
2024-02-26 21:32 ` [PATCH v2 9/9] KVM: SVM: Add CET features to supported_xss John Allen
2024-05-01 23:47 ` Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9b05e2d7-ac1c-e60f-0f6e-f4befea06334@amd.com \
--to=thomas.lendacky@amd.com \
--cc=bp@alien8.de \
--cc=john.allen@amd.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mlevitsk@redhat.com \
--cc=pbonzini@redhat.com \
--cc=rick.p.edgecombe@intel.com \
--cc=seanjc@google.com \
--cc=weijiang.yang@intel.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).