Re: [Xen-devel] [PATCH 1/2] x86/hvm: improve performance of HVMOP_flush_tlbs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.12.2019 14:26, Roger Pau Monne wrote:
> There's no need to call paging_update_cr3 unless CR3 trapping is
> enabled, and that's only the case when using shadow paging or when
> requested for introspection purposes, otherwise there's no need to
> pause all the vCPUs of the domain in order to perform the flush.
> 
> Check whether CR3 trapping is currently in use in order to decide
> whether the vCPUs should be paused, otherwise just perform the flush.

First of all - with the commit introducing the pausing not saying
anything on the "why", you must have gained some understanding
there. Could you share this? I can't see why this was needed, and
sh_update_cr3() also doesn't look to have any respective ASSERT()
or alike. I'm having even more trouble seeing why in HAP mode the
pausing would be needed.

As a result I wonder whether, rather than determining whether
pausing is needed inside the function, this shouldn't be a flag
in struct paging_mode.

Next I seriously doubt introspection hooks should be called here.
Introspection should be about guest actions, and us calling
paging_update_cr3() is an implementation detail of Xen, not
something the guest controls. Even more, there not being any CR3
change here I wonder whether the call by the hooks to
hvm_update_guest_cr3() couldn't be suppressed altogether in this
case. Quite possibly in the shadow case there could be more
steps that aren't really needed, so perhaps a separate hook might
be on order.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel