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?
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.
