[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 8 Jan 2020 18:37:54 +0100
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=roger.pau@xxxxxxxxxx; spf=Pass smtp.mailfrom=roger.pau@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, Wei Liu <wl@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Delivery-date: Wed, 08 Jan 2020 17:38:18 +0000
  • Ironport-sdr: j5BRNv2dujWw5o0Se9uNjDGoDapk7+IqtOZJMkhC6UYwo/UVTzE1bTG93Fx3UuxBx7yI/r64Su 14Jee1ARWFRx7g32YubwHaqwEf/h+4RBW9qOsP7vrvJNOi/CPFaewcbpvUsxAfbiG3bS6Ch6zO zZ5y/L6QlPYWdgDc1Je0zp83RS4YQsfKUfND4izdzeawzhMYGuP38sIl3JFe+RK0T84CPquHON fG+374m5nzg5yTiG9pKBPE4phFUNoONUrCEZX2FbhuYFppSAguBrJrdbKHIkxVGqo8yLf6zSXV 62I=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Fri, Jan 03, 2020 at 04:17:14PM +0100, Jan Beulich wrote:
> 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?

hap_update_cr3 does a "v->arch.hvm.hw_cr[3] = v->arch.hvm.guest_cr[3]"
unconditionally, and such access would be racy if the vCPU is running
and also modifying cr3 at the same time AFAICT.

Just pausing each vCPU before calling paging_update_cr3 should be fine
and would have a smaller performance penalty.

> 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.

Right, I guess just having a hook that does a flush would be enough.
Let me try to propose something slightly better.

Thanks, Roger.

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.