Re: [PATCH] KVM: x86: do not reset microcode version on INIT or RESET

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Tue, Feb 11, 2020 at 07:04:39PM +0100, Paolo Bonzini wrote:
> The microcode version should be set just once, since it is essentially
> a CPU feature; so do it on vCPU creation rather than reset.

I wouldn't call it a CPU feature, CPU features generally can't be
arbitrarily changed while running.  I'd prefer to have a changelog that
at least somewhat ties the change to hardware behavior. 

  Do not initialize the microcode version at RESET or INIT.   Microcode
  updates are not lost during INIT, and exact behavior across a warm RESET
  is microarchitectural, i.e. defer to userspace to emulate behavior for
  RESET as it sees fit.

For the code:

Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>

> Userspace can tie the fix to the availability of MSR_IA32_UCODE_REV in
> the list of emulated MSRs.
> 
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/svm.c     | 2 +-
>  arch/x86/kvm/vmx/vmx.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index a7e63b613837..280f6d024e84 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -2185,7 +2185,6 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>  	u32 dummy;
>  	u32 eax = 1;
>  
> -	vcpu->arch.microcode_version = 0x01000065;
>  	svm->spec_ctrl = 0;
>  	svm->virt_spec_ctrl = 0;
>  
> @@ -2276,6 +2275,7 @@ static int svm_create_vcpu(struct kvm_vcpu *vcpu)
>  	init_vmcb(svm);
>  
>  	svm_init_osvw(vcpu);
> +	vcpu->arch.microcode_version = 0x01000065;
>  
>  	return 0;
>  
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 9a6664886f2e..d625b4b0e7b4 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4238,7 +4238,6 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>  
>  	vmx->msr_ia32_umwait_control = 0;
>  
> -	vcpu->arch.microcode_version = 0x100000000ULL;
>  	vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val();
>  	vmx->hv_deadline_tsc = -1;
>  	kvm_set_cr8(vcpu, 0);
> @@ -6763,6 +6762,7 @@ static int vmx_create_vcpu(struct kvm_vcpu *vcpu)
>  	vmx->nested.posted_intr_nv = -1;
>  	vmx->nested.current_vmptr = -1ull;
>  
> +	vcpu->arch.microcode_version = 0x100000000ULL;
>  	vmx->msr_ia32_feature_control_valid_bits = FEAT_CTL_LOCKED;
>  
>  	/*
> -- 
> 1.8.3.1
>