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

[Wei Liu <wl@xxxxxxx>]

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.

Wei.

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