Re: [Qemu-devel] [PATCH] target-sparc: Store mmu index in TB flags

[Artyom Tarasenko]

Hi Richard,

On Tue, Aug 25, 2015 at 6:19 AM, Richard Henderson <address@hidden> wrote:
> Doing this instead of saving the raw PS_PRIV and TL.  This means
> that all nucleus mode TBs (TL > 0) can be shared.  This fixes a
> bug in that we didn't include HS_PRIV in the TB flags, and so could
> produce incorrect TB matches for hypervisor state.
>
> The LSU and DMMU states were unused by the translator.  Including
> them in TB flags meant unnecessary mismatches from tb_find_fast.
>
> Signed-off-by: Richard Henderson <address@hidden>
>
> ---
> Artyom and Dennis, I'm hoping that this will help with some of your
> translation performance problems.  I don't currently have a sparc64
> kernel set up for booting, but I did smoke test this with openbios,
> and even there it reduced the number of TBs created.

This patch indeed fixes a bug in sun4v emulation, and we'll need it once
we have a working sun4v machine (currently qemu doesn't implement
a minimal sun4v machine, for instance there is no sun4v mmu).

I haven't tried it, but it's unlikely it would impact the
sun4u emulation we are currently using for the tests:
the sun4u machine doesn't have a hypervisor.

Artyom

> ---
>  target-sparc/cpu.h       | 26 ++++++++++++--------------
>  target-sparc/translate.c |  2 +-
>  2 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 0522b65..23773f4 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -694,34 +694,32 @@ void cpu_tick_set_limit(CPUTimer *timer, uint64_t 
> limit);
>  trap_state* cpu_tsptr(CPUSPARCState* env);
>  #endif
>
> -#define TB_FLAG_FPU_ENABLED (1 << 4)
> -#define TB_FLAG_AM_ENABLED (1 << 5)
> +#define TB_FLAG_MMU_MASK     7
> +#define TB_FLAG_FPU_ENABLED  (1 << 4)
> +#define TB_FLAG_AM_ENABLED   (1 << 5)
>
>  static inline void cpu_get_tb_cpu_state(CPUSPARCState *env, target_ulong *pc,
> -                                        target_ulong *cs_base, int *flags)
> +                                        target_ulong *cs_base, int *pflags)
>  {
> +    int flags;
>      *pc = env->pc;
>      *cs_base = env->npc;
> +    flags = cpu_mmu_index(env);
>  #ifdef TARGET_SPARC64
> -    // AM . Combined FPU enable bits . PRIV . DMMU enabled . IMMU enabled
> -    *flags = (env->pstate & PS_PRIV)               /* 2 */
> -        | ((env->lsu & (DMMU_E | IMMU_E)) >> 2)    /* 1, 0 */
> -        | ((env->tl & 0xff) << 8)
> -        | (env->dmmu.mmu_primary_context << 16);   /* 16... */
>      if (env->pstate & PS_AM) {
> -        *flags |= TB_FLAG_AM_ENABLED;
> +        flags |= TB_FLAG_AM_ENABLED;
>      }
> -    if ((env->def->features & CPU_FEATURE_FLOAT) && (env->pstate & PS_PEF)
> +    if ((env->def->features & CPU_FEATURE_FLOAT)
> +        && (env->pstate & PS_PEF)
>          && (env->fprs & FPRS_FEF)) {
> -        *flags |= TB_FLAG_FPU_ENABLED;
> +        flags |= TB_FLAG_FPU_ENABLED;
>      }
>  #else
> -    // FPU enable . Supervisor
> -    *flags = env->psrs;
>      if ((env->def->features & CPU_FEATURE_FLOAT) && env->psref) {
> -        *flags |= TB_FLAG_FPU_ENABLED;
> +        flags |= TB_FLAG_FPU_ENABLED;
>      }
>  #endif
> +    *pflags = flags;
>  }
>
>  static inline bool tb_fpu_enabled(int tb_flags)
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 48fc2ab..8254a30 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -5234,7 +5234,7 @@ static inline void 
> gen_intermediate_code_internal(SPARCCPU *cpu,
>      last_pc = dc->pc;
>      dc->npc = (target_ulong) tb->cs_base;
>      dc->cc_op = CC_OP_DYNAMIC;
> -    dc->mem_idx = cpu_mmu_index(env);
> +    dc->mem_idx = tb->flags & TB_FLAG_MMU_MASK;
>      dc->def = env->def;
>      dc->fpu_enabled = tb_fpu_enabled(tb->flags);
>      dc->address_mask_32bit = tb_am_enabled(tb->flags);
> --
> 2.4.3
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu