Re: [RFC] Don't lookup full CPU state in the indirect branch fast path on AArch64 when running in user mode.

[Owen Anderson]

Ping

On Mon, Oct 12, 2020 at 1:52 PM Owen Anderson <oanderso@google.com> wrote:
>
> Ping.
>
> I'd like to get feedback on how/whether this could be developed into a
> landable version.
>
> Thanks,
>
> --Owen
>
> On Tue, Sep 29, 2020 at 2:32 PM Owen Anderson <oanderso@google.com> wrote:
> >
> > Hello,
> >
> > I would like to request feedback on the following patch, which I do
> > not believe should be applied to master as-is.  The idea here is to
> > avoid gathering the full CPU state in the fast path of an indirect
> > branch lookup when running in user mode on a platform where the flags
> > can only be changed in privileged mode.  I believe this is true on the
> > AArch64 scenario that I care about, but clearly not true in general.
> > I'm particularly seeking feedback on how to clean this up into a
> > version that checks the correct necessary and sufficient conditions to
> > allow all users that can benefit from it to do so.
> >
> > On the workload that I am targeting (aarch64 on x86), this patch
> > reduces execution wall time by approximately 20%, and eliminates
> > indirect branch lookups from the hot stack traces entirely.
> >
> > Thank you,
> >
> > --Owen
> >
> > From 3d96db17d3baacb92ef1bc5e70ef06b97d06a0ae Mon Sep 17 00:00:00 2001
> > From: Owen Anderson <oanderso@google.com>
> > Date: Tue, 29 Sep 2020 13:47:00 -0700
> > Subject: [RFC] Don't lookup full CPU state in the indirect branch fast path 
> > on
> >  AArch64 when running in user mode.
> >
> > Most of the CPU state can't be changed in user mode, so this is useless 
> > work.
> >
> > Signed-off-by: Owen Anderson <oanderso@google.com>
> > ---
> >  include/exec/tb-lookup.h | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/include/exec/tb-lookup.h b/include/exec/tb-lookup.h
> > index 9cf475bb03..f4ea0eb4c0 100644
> > --- a/include/exec/tb-lookup.h
> > +++ b/include/exec/tb-lookup.h
> > @@ -25,7 +25,15 @@ tb_lookup__cpu_state(CPUState *cpu, target_ulong
> > *pc, target_ulong *cs_base,
> >      TranslationBlock *tb;
> >      uint32_t hash;
> >
> > +#if !defined(TARGET_ARM) || !defined(CONFIG_USER_ONLY)
> >      cpu_get_tb_cpu_state(env, pc, cs_base, flags);
> > +#else
> > +    if (is_a64(env)) {
> > +      *pc = env->pc;
> > +    } else {
> > +      *pc = env->regs[15];
> > +    }
> > +#endif
> >      hash = tb_jmp_cache_hash_func(*pc);
> >      tb = qatomic_rcu_read(&cpu->tb_jmp_cache[hash]);
> >
> > @@ -34,12 +42,19 @@ tb_lookup__cpu_state(CPUState *cpu, target_ulong
> > *pc, target_ulong *cs_base,
> >
> >      if (likely(tb &&
> >                 tb->pc == *pc &&
> > +#if !defined(TARGET_ARM) || !defined(CONFIG_USER_ONLY)
> >                 tb->cs_base == *cs_base &&
> >                 tb->flags == *flags &&
> > +#endif
> >                 tb->trace_vcpu_dstate == *cpu->trace_dstate &&
> >                 (tb_cflags(tb) & (CF_HASH_MASK | CF_INVALID)) == cf_mask)) {
> >          return tb;
> >      }
> > +
> > +#ifdef CONFIG_USER_ONLY
> > +    cpu_get_tb_cpu_state(env, pc, cs_base, flags);
> > +#endif
> > +
> >      tb = tb_htable_lookup(cpu, *pc, *cs_base, *flags, cf_mask);
> >      if (tb == NULL) {
> >          return NULL;
> > --
> > 2.28.0.709.gb0816b6eb0-goog



-- 
--Owen