On Tue, Sep 20, 2022 at 2:12 PM Boris Ostrovsky <boris.ostrov...@oracle.com> wrote:
> > On 9/20/22 10:54 AM, Jan Beulich wrote: > > On 20.09.2022 16:26, Boris Ostrovsky wrote: > >> On 9/20/22 4:01 AM, Jan Beulich wrote: > >>> On 20.09.2022 00:42, Boris Ostrovsky wrote: > >>>> It is saving vpmu data from current pcpu's MSRs for a remote vcpu so > @v > >>>> in vmx_find_msr() is not @current: > >>>> > >>>> vpmu_load() > >>>> ... > >>>> prev = per_cpu(last_vcpu, pcpu); > >>>> vpmu_save_force(prev) > >>>> core2_vpmu_save() > >>>> __core2_vpmu_save() > >>>> vmx_read_guest_msr() > >>>> vmx_find_msr() > >>>> > >>>> > >>>> The call to vmx_find_msr() was introduced by 755087eb9b10c. I wonder > though whether > >>>> this call is needed when code path above is executed (i.e. when we > are saving > >>>> remove vcpu) > >>> How could it not be needed? We need to obtain the guest value. The > >>> thing I don't understand is why this forced saving is necessary, > >>> when context_switch() unconditionally calls vpmu_switch_from(). > >> > >> IIRC the logic is: > >> > >> 1. vcpuA runs on pcpu0 > >> 2. vcpuA is de-scheduled and is selected to run on pcpu1. It has not > yet called vpmu_load() from pcpu1 > > The calling of vpmu_load() shouldn't matter here. What does matter is > > that vpmu_save() was necessarily called already. > > > I thought we don't always save MSRs on context switch because we optimize > for case when the same vcpu gets scheduled again. But I am not sure I see > this now that I am looking at the code. > I see context_switch calling vpmu_save_from before __context_switch, but if that did actually save the vPMU state it would have reset VPMU_CONTEXT_LOADED, so by the time vpmu_load calls vpmu_save_force it would have just bailed before hitting the ASSERT failure in vmx_find_msrs. The context must still be loaded at that point (I'm trying to verify right now by adding some printks). Tamas
