Re: [PATCH v3] x86/PV: remove unnecessary toggle_guest_pt() overhead

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/05/2020 11:07, Jan Beulich wrote:
> On 21.05.2020 18:46, Andrew Cooper wrote:
>> On 05/05/2020 07:16, Jan Beulich wrote:
>>> While the mere updating of ->pv_cr3 and ->root_pgt_changed aren't overly
>>> expensive (but still needed only for the toggle_guest_mode() path), the
>>> effect of the latter on the exit-to-guest path is not insignificant.
>>> Move the logic into toggle_guest_mode(), on the basis that
>>> toggle_guest_pt() will always be invoked in pairs, yet we can't safely
>>> undo the setting of root_pgt_changed during the second of these
>>> invocations.
>>>
>>> While at it, add a comment ahead of toggle_guest_pt() to clarify its
>>> intended usage.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> I'm still of the opinion that the commit message wants rewriting to get
>> the important points across clearly.
>>
>> And those are that toggle_guest_pt() is called in pairs specifically to
>> read kernel data structures when emulating a userspace action, and that
>> this doesn't modify cr3 from the guests point of view, and therefore
>> doesn't need the resync on exit-to-guest path.
> Is this
>
> "toggle_guest_pt() is called in pairs, to read guest kernel data
>  structures when emulating a guest userspace action. Hence this doesn't
>  modify cr3 from the guest's point of view, and therefore doesn't need
>  any resync on the exit-to-guest path. Therefore move the updating of
>  ->pv_cr3 and ->root_pgt_changed into toggle_guest_mode(), since undoing
>  the changes during the second of these invocations wouldn't be a safe
>  thing to do."
>
> any better?

Yes - that will do.

Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>