On Wed, Aug 14, 2024, Vitaly Kuznetsov wrote: > Sean Christopherson <seanjc@xxxxxxxxxx> writes: > > > On Wed, Aug 14, 2024, Vitaly Kuznetsov wrote: > >> What I meant is something along these lines (untested): > >> > >> diff --git a/arch/x86/kvm/vmx/vmx_onhyperv.h b/arch/x86/kvm/vmx/vmx_onhyperv.h > >> index eb48153bfd73..e2d8c67d0cad 100644 > >> --- a/arch/x86/kvm/vmx/vmx_onhyperv.h > >> +++ b/arch/x86/kvm/vmx/vmx_onhyperv.h > >> @@ -104,6 +104,14 @@ static inline void evmcs_load(u64 phys_addr) > >> struct hv_vp_assist_page *vp_ap = > >> hv_get_vp_assist_page(smp_processor_id()); > >> > >> + /* > >> + * When enabling eVMCS, KVM verifies that every CPU has a valid hv_vp_assist_page() > >> + * and aborts enabling the feature otherwise. CPU onlining path is also checked in > >> + * vmx_hardware_enable(). With this, it is impossible to reach here with vp_ap == NULL > >> + * but compilers may still complain. > >> + */ > >> + BUG_ON(!vp_ap); > > > > A full BUG_ON() is overkill, and easily avoided. If we want to add a sanity > > check here and do more than just WARN, then it's easy enough to plumb in @vcpu > > and make this a KVM_BUG_ON() so that the VM dies, i.e. so that KVM doesn't risk > > corrupting the guest somehow. > > > > I'm still acting under the impression this is an absolutely impossible > situation :-) > > AFAICS, we only call evmcs_load() from vmcs_load() but this one doesn't > have @vcpu/@kvm either and I wasn't sure it's worth the effort to do the > plumbing (or am I missing an easy way to go back from @vmcs to > @vcpu?). On the other hand, vmcs_load() should not be called that ofter > so if we prefer to have @vcpu there for some other reason -- why not. kvm_get_running_vcpu(), though I honestly purposely didn't suggest it earlier because I am not a fan of using kvm_get_running_vcpu() unless it's absolutely necessary. But for this situation, I'd be fine with using it.
