Re: [PATCH v3] KVM: VMX: Fix crash due to uninitialized current_vmcs

[Sean Christopherson <seanjc@google.com>]

On Mon, Jan 23, 2023, Vitaly Kuznetsov wrote:
> Alexandru Matei <alexandru.matei@uipath.com> writes:
> 
> > KVM enables 'Enlightened VMCS' and 'Enlightened MSR Bitmap' when running as
> > a nested hypervisor on top of Hyper-V. When MSR bitmap is updated,
> > evmcs_touch_msr_bitmap function uses current_vmcs per-cpu variable to mark
> > that the msr bitmap was changed.

...

> > @@ -219,7 +223,7 @@ static inline u64 evmcs_read64(unsigned long field) { return 0; }
> >  static inline u32 evmcs_read32(unsigned long field) { return 0; }
> >  static inline u16 evmcs_read16(unsigned long field) { return 0; }
> >  static inline void evmcs_load(u64 phys_addr) {}
> > -static inline void evmcs_touch_msr_bitmap(void) {}
> > +static inline void evmcs_touch_msr_bitmap(struct hv_enlightened_vmcs *evmcs) {}
> >  #endif /* IS_ENABLED(CONFIG_HYPERV) */
> >  
> >  #define EVMPTR_INVALID (-1ULL)
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index fe5615fd8295..1d482a80bca8 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -3869,7 +3869,7 @@ static void vmx_msr_bitmap_l01_changed(struct vcpu_vmx *vmx)
> >  	 * bitmap has changed.
> >  	 */
> >  	if (static_branch_unlikely(&enable_evmcs))
> > -		evmcs_touch_msr_bitmap();
> > +		evmcs_touch_msr_bitmap((struct hv_enlightened_vmcs *)vmx->vmcs01.vmcs);
> >  
> >  	vmx->nested.force_msr_bitmap_recalc = true;
> >  }
> 
> Just in case we decide to follow this path and not merge
> evmcs_touch_msr_bitmap() into vmx_msr_bitmap_l01_changed():

This is the only approach that I'm outright opposed to.  The evmcs_touch_msr_bitmap()
stub is a lie in that it should never be reached with CONFIG_HYPERV=n, i.e. should
really WARN.  Ditto for the WARN_ON_ONCE() in the actual helper; if vmx->vmcs01.vmcs
is NULL then KVM is completely hosed.

KVM already consumes hv_enlightenments_control.msr_bitmap in vmx.c and in nested.c,
shoving this case into hyperv.h but leaving those in VMX proper is odd/kludgy.