On Mon, Jan 23, 2023, Vitaly Kuznetsov wrote:
> Alexandru Matei <alexandru.matei@xxxxxxxxxx> 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.
