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

Re: [Xen-devel] [PATCH v3 6/7] x86/tlb: allow disabling the TLB clock


  • To: Wei Liu <wl@xxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 6 Feb 2020 14:46:10 +0100
  • Authentication-results: esa6.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, Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Delivery-date: Thu, 06 Feb 2020 13:46:25 +0000
  • Ironport-sdr: JQTmWNg7ufZ6lwsYjgRS8us60exYrT/XrOC98WujkfEvY69ypAdPXLopDKfQ3ttvE5x/A/pDU8 I+miZgHOBd9sjDBjeGdqtXd1r3rrCy82jJhAJ9yoZrd6CPC7M9IHuewNsrRDS/KITxDjcdFQLZ A6ulo4efj9zhPMDErvW/JcheSoJdMl67jbJrjhadDVB4/mw9akummsrtFrtiIlPDEOZYivNNXi EXFuRymCxP0AVepK7Ad3m/boS6ksUxlPAqF7u4S+0E3sq4lT5uZE1LVpqmsd1ru9PEXBrTV3A8 4Qs=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Feb 06, 2020 at 01:31:34PM +0000, Wei Liu wrote:
> On Mon, Jan 27, 2020 at 07:11:14PM +0100, Roger Pau Monne wrote:
> > The TLB clock is helpful when running Xen on bare metal because when
> > doing a TLB flush each CPU is IPI'ed and can keep a timestamp of the
> > last flush.
> > 
> > This is not the case however when Xen is running virtualized, and the
> > underlying hypervisor provides mechanism to assist in performing TLB
> > flushes: Xen itself for example offers a HVMOP_flush_tlbs hypercall in
> > order to perform a TLB flush without having to IPI each CPU. When
> > using such mechanisms it's no longer possible to keep a timestamp of
> > the flushes on each CPU, as they are performed by the underlying
> > hypervisor.
> > 
> > Offer a boolean in order to signal Xen that the timestamped TLB
> > shouldn't be used. This avoids keeping the timestamps of the flushes,
> > and also forces NEED_FLUSH to always return true.
> > 
> > No functional change intended, as this change doesn't introduce any
> > user that disables the timestamped TLB.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/flushtlb.c        | 19 +++++++++++++------
> >  xen/include/asm-x86/flushtlb.h | 17 ++++++++++++++++-
> >  2 files changed, 29 insertions(+), 7 deletions(-)
> > 
> > diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
> > index e7ccd4ec7b..3649900793 100644
> > --- a/xen/arch/x86/flushtlb.c
> > +++ b/xen/arch/x86/flushtlb.c
> > @@ -32,6 +32,9 @@
> >  u32 tlbflush_clock = 1U;
> >  DEFINE_PER_CPU(u32, tlbflush_time);
> >  
> > +/* Signals whether the TLB flush clock is in use. */
> > +bool __read_mostly tlb_clk_enabled = true;
> > +
> >  /*
> >   * pre_flush(): Increment the virtual TLB-flush clock. Returns new clock 
> > value.
> >   * 
> > @@ -82,12 +85,13 @@ static void post_flush(u32 t)
> >  static void do_tlb_flush(void)
> >  {
> >      unsigned long flags, cr4;
> > -    u32 t;
> > +    u32 t = 0;
> >  
> >      /* This non-reentrant function is sometimes called in interrupt 
> > context. */
> >      local_irq_save(flags);
> >  
> > -    t = pre_flush();
> > +    if ( tlb_clk_enabled )
> > +        t = pre_flush();
> 
> I think it makes more sense to push the check to pre_flush and
> post_flush -- they are the ones that care about the clock, after all.
> 
> This also has the effect of making this patch a bit shorter, I think.

I added the check here in order to avoid the call just to return 0.
This is a hot path, so I thought that a check here would be less
expensive that a function call when the TLB stamps are disabled.

I don't mind moving it inside {pre/post}_flush functions if that's the
consensus.

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