On Fri, Feb 10, 2023 at 12:31:32AM +0000, Sean Christopherson wrote:
> Disallow writes to feature MSRs after KVM_RUN to prevent userspace from
> changing the vCPU model after running the vCPU. Similar to guest CPUID,
> KVM uses feature MSRs to configure intercepts, determine what operations
> are/aren't allowed, etc. Changing the capabilities while the vCPU is
> active will at best yield unpredictable guest behavior, and at worst
> could be dangerous to KVM.
>
> Allow writing the current value, e.g. so that userspace can blindly set
> all MSRs when emulating RESET, and unconditionally allow writes to
> MSR_IA32_UCODE_REV so that userspace can emulate patch loads.
>
> Special case the VMX MSRs to keep the generic list small, i.e. so that
> KVM can do a linear walk of the generic list without incurring meaningful
> overhead.
>
> Cc: Like Xu <like.xu.linux@xxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> arch/x86/kvm/x86.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 7b73a0b45041..186cb6a81643 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1554,6 +1554,25 @@ static u32 msr_based_features[ARRAY_SIZE(msr_based_features_all_except_vmx) +
> (KVM_LAST_EMULATED_VMX_MSR - KVM_FIRST_EMULATED_VMX_MSR + 1)];
> static unsigned int num_msr_based_features;
>
> +/*
> + * All feature MSRs except uCode revID, which tracks the currently loaded uCode
> + * patch, are immutable once the vCPU model is defined.
> + */
> +static bool kvm_is_immutable_feature_msr(u32 msr)
> +{
> + int i;
> +
> + if (msr >= KVM_FIRST_EMULATED_VMX_MSR && msr <= KVM_LAST_EMULATED_VMX_MSR)
> + return true;
> +
> + for (i = 0; i < ARRAY_SIZE(msr_based_features_all_except_vmx); i++) {
> + if (msr == msr_based_features_all_except_vmx[i])
> + return msr != MSR_IA32_UCODE_REV;
> + }
> +
> + return false;
> +}
> +
> /*
> * Some IA32_ARCH_CAPABILITIES bits have dependencies on MSRs that KVM
> * does not yet virtualize. These include:
> @@ -2168,6 +2187,23 @@ static int do_get_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
>
> static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
> {
> + u64 val;
> +
> + /*
> + * Disallow writes to immutable feature MSRs after KVM_RUN. KVM does
> + * not support modifying the guest vCPU model on the fly, e.g. changing
> + * the nVMX capabilities while L2 is running is nonsensical. Ignore
> + * writes of the same value, e.g. to allow userspace to blindly stuff
> + * all MSRs when emulating RESET.
> + */
> + if (vcpu->arch.last_vmentry_cpu != -1 &&
Use kvm_vcpu_has_run(vcpu) here?
B.R.
Yu
