[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] x86/vpmu: fix race-condition in vpmu_load
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
|
