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@xxxxxxx>
---
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.
