Re: [PATCH v1 10/23] KVM: VMX: Add support for FRED context save/restore

Chao Gao
2023-11-13T05:24:58Z

>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 2c924075f6f1..c5a55810647f 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -1451,6 +1451,9 @@ static const u32 msrs_to_save_base[] = {
> MSR_STAR,
> #ifdef CONFIG_X86_64
> MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
>+ MSR_IA32_FRED_RSP0, MSR_IA32_FRED_RSP1, MSR_IA32_FRED_RSP2,
>+ MSR_IA32_FRED_RSP3, MSR_IA32_FRED_STKLVLS, MSR_IA32_FRED_SSP1,
>+ MSR_IA32_FRED_SSP2, MSR_IA32_FRED_SSP3, MSR_IA32_FRED_CONFIG,

Need to handle the case where FRED MSRs are valid but KVM cannot virtualize
FRED, see kvm_probe_msr_to_save().

> #endif
> MSR_IA32_TSC, MSR_IA32_CR_PAT, MSR_VM_HSAVE_PA,
> MSR_IA32_FEAT_CTL, MSR_IA32_BNDCFGS, MSR_TSC_AUX,
>@@ -1890,6 +1893,16 @@ static int __kvm_set_msr(struct kvm_vcpu *vcpu, u32 index, u64 data,
>
> data = (u32)data;
> break;
>+ case MSR_IA32_FRED_RSP0 ... MSR_IA32_FRED_CONFIG:
>+ if (host_initiated || guest_cpuid_has(vcpu, X86_FEATURE_FRED))
>+ break;

Nothing guarantees FRED MSRs/VMCS fields exist on the hardware here. Probably use
guest_cpu_cap_has()*.

*: https://lore.kernel.org/kvm/20231110235528.1561679-1-seanjc@google.com

>+
>+ /*
>+ * Inject #GP upon FRED MSRs accesses from a non-FRED guest to
>+ * make sure no malicious guest can write to FRED MSRs thus to
>+ * corrupt host FRED MSRs.
>+ */

I think injecting #GP here is simply because KVM should emulate hardware
behavior. To me, preventing guest from corrupting FRED MSRs is at most a
byproduct. I prefer to drop the comment.

>+ return 1;
> }
>
> msr.data = data;
>@@ -1933,6 +1946,16 @@ int __kvm_get_msr(struct kvm_vcpu *vcpu, u32 index, u64 *data,
> !guest_cpuid_has(vcpu, X86_FEATURE_RDPID))
> return 1;
> break;
>+ case MSR_IA32_FRED_RSP0 ... MSR_IA32_FRED_CONFIG:
>+ if (host_initiated || guest_cpuid_has(vcpu, X86_FEATURE_FRED))
>+ break;
>+
>+ /*
>+ * Inject #GP upon FRED MSRs accesses from a non-FRED guest to
>+ * make sure no malicious guest can write to FRED MSRs thus to
>+ * corrupt host FRED MSRs.
>+ */
>+ return 1;
> }
>
> msr.index = index;
>--
>2.42.0
>
>