Re: [Xen-devel] heavy P2M lock contention on guest HPET counter reads

[Andres Lagar Cavilla <andres@xxxxxxxxxxxxxxxx>]

On Wed, Jul 30, 2014 at 10:22 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 30.07.14 at 12:08, <JBeulich@xxxxxxxx> wrote:
> with 40+ vCPU Win2012R2 guests we're observing apparent guest live
> locks. The 'd' debug key reveals more than half of the vCPU-s doing an
> inlined HPET main counter read from KeQueryPerformanceCounter(),
> resulting in all of them racing for the lock at the beginning of
> __get_gfn_type_access(). Assuming it is really necessary to always
> take the write lock (rather than just the read one) here, would it perhaps
> be reasonable to introduce a bypass in hvm_hap_nested_page_fault()
> for the HPET page similar to the LAPIC one?

Like this (for 4.4, if it matters, and with no tester feedback yet),
raising the question whether we shouldn't just do this for all
internal MMIO handlers.

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1523,11 +1523,16 @@ int hvm_hap_nested_page_fault(paddr_t gp
    Â}
  Â}

- Â Â/* For the benefit of 32-bit WinXP (& older Windows) on AMD CPUs,
- Â Â * a fast path for LAPIC accesses, skipping the p2m lookup. */
+ Â Â/*
+ Â Â * For the benefit of 32-bit WinXP (& older Windows) on AMD CPUs,
+ Â Â * a fast path for LAPIC accesses, skipping the p2m lookup.
+ Â Â * Similarly for newer Windows (like Server 2012) a fast path for
+ Â Â * HPET accesses.
+ Â Â */
  Âif ( !nestedhvm_vcpu_in_guestmode(v)
     && is_hvm_vcpu(v)
- Â Â Â Â && gfn == PFN_DOWN(vlapic_base_address(vcpu_vlapic(v))) )
+ Â Â Â Â && (gfn == PFN_DOWN(vlapic_base_address(vcpu_vlapic(v)))
+ Â Â Â Â Â Â || hpet_mmio_handler.check_handler(v, gpa)) )

Â

This is reasonable. I suggest knee-capping the open coding and moving this block into its own little inline function (e.g. try_shortcut_hvm_mmio). I think soon enough we may run into the next Xen-implemented mmio range that is getting pounded. In any case it will do for better-looking code. Roughly:

int try_shortcut_hvm_mmio(struct vcpu *v, paddr_t gpa)

{

  int i;

  if ( nestedhvm_vcpu_in_guestmode(v)

    Â|| !is_hvm_vcpu(v))

    return 0;

  for ( i = 0; i < HVM_MMIO_HANDLER_NR; i++ )

    if ( hvm_mmio_handlers[i]->check_handler(v, gpa) )

      return 1;

}

Having said that, I don't know off the top of my head that is obviously correct to shortcut the p2m lookup for msix table and iommu. I think so, but...

AndresÂ

  Â{
    Âif ( !handle_mmio() )
      Âhvm_inject_hw_exception(TRAP_gp_fault, 0);

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel