Re: [PATCH v5 12/19] KVM:x86: Save and reload SSP to/from SMRAM

Chao Gao
2023-08-04T07:54:00Z

On Thu, Aug 03, 2023 at 12:27:25AM -0400, Yang Weijiang wrote:
>Save CET SSP to SMRAM on SMI and reload it on RSM.
>KVM emulates architectural behavior when guest enters/leaves SMM
>mode, i.e., save registers to SMRAM at the entry of SMM and reload
>them at the exit of SMM. Per SDM, SSP is defined as one of
>the fields in SMRAM for 64-bit mode, so handle the state accordingly.
>
>Check is_smm() to determine whether kvm_cet_is_msr_accessible()
>is called in SMM mode so that kvm_{set,get}_msr() works in SMM mode.
>
>Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>---
> arch/x86/kvm/smm.c | 11 +++++++++++
> arch/x86/kvm/smm.h | 2 +-
> arch/x86/kvm/x86.c | 11 ++++++++++-
> 3 files changed, 22 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/kvm/smm.c b/arch/x86/kvm/smm.c
>index b42111a24cc2..e0b62d211306 100644
>--- a/arch/x86/kvm/smm.c
>+++ b/arch/x86/kvm/smm.c
>@@ -309,6 +309,12 @@ void enter_smm(struct kvm_vcpu *vcpu)
>
> kvm_smm_changed(vcpu, true);
>
>+#ifdef CONFIG_X86_64
>+ if (guest_can_use(vcpu, X86_FEATURE_SHSTK) &&
>+ kvm_get_msr(vcpu, MSR_KVM_GUEST_SSP, &smram.smram64.ssp))
>+ goto error;
>+#endif

SSP save/load should go to enter_smm_save_state_64() and rsm_load_state_64(),
where other fields of SMRAM are handled.

>+
> if (kvm_vcpu_write_guest(vcpu, vcpu->arch.smbase + 0xfe00, &smram, sizeof(smram)))
> goto error;
>
>@@ -586,6 +592,11 @@ int emulator_leave_smm(struct x86_emulate_ctxt *ctxt)
> if ((vcpu->arch.hflags & HF_SMM_INSIDE_NMI_MASK) == 0)
> static_call(kvm_x86_set_nmi_mask)(vcpu, false);
>
>+#ifdef CONFIG_X86_64
>+ if (guest_can_use(vcpu, X86_FEATURE_SHSTK) &&
>+ kvm_set_msr(vcpu, MSR_KVM_GUEST_SSP, smram.smram64.ssp))
>+ return X86EMUL_UNHANDLEABLE;
>+#endif
> kvm_smm_changed(vcpu, false);
>
> /*
>diff --git a/arch/x86/kvm/smm.h b/arch/x86/kvm/smm.h
>index a1cf2ac5bd78..1e2a3e18207f 100644
>--- a/arch/x86/kvm/smm.h
>+++ b/arch/x86/kvm/smm.h
>@@ -116,8 +116,8 @@ struct kvm_smram_state_64 {
> u32 smbase;
> u32 reserved4[5];
>
>- /* ssp and svm_* fields below are not implemented by KVM */
> u64 ssp;
>+ /* svm_* fields below are not implemented by KVM */
> u64 svm_guest_pat;
> u64 svm_host_efer;
> u64 svm_host_cr4;
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 98f3ff6078e6..56aa5a3d3913 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -3644,8 +3644,17 @@ static bool kvm_cet_is_msr_accessible(struct kvm_vcpu *vcpu,
> if (!kvm_cpu_cap_has(X86_FEATURE_SHSTK))
> return false;
>
>- if (msr->index == MSR_KVM_GUEST_SSP)
>+ /*
>+ * This MSR is synthesized mainly for userspace access during
>+ * Live Migration, it also can be accessed in SMM mode by VMM.
>+ * Guest is not allowed to access this MSR.
>+ */
>+ if (msr->index == MSR_KVM_GUEST_SSP) {
>+ if (IS_ENABLED(CONFIG_X86_64) && is_smm(vcpu))
>+ return true;

On second thoughts, this is incorrect. We don't want guest in SMM
mode to read/write SSP via the synthesized MSR. Right?

You can
1. move set/get guest SSP into two helper functions, e.g., kvm_set/get_ssp()
2. call kvm_set/get_ssp() for host-initiated MSR accesses and SMM transitions.
3. refuse guest accesses to the synthesized MSR.

>+
> return msr->host_initiated;
>+ }
>
> return msr->host_initiated ||
> guest_cpuid_has(vcpu, X86_FEATURE_SHSTK);
>--
>2.27.0
>