Re: [Qemu-devel] [PATCH for-4.2 14/24] target/arm: Simplify tlb_force_broadcast alternatives

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> Rather than call to a separate function and re-compute any
> parameters for the flush, simply use the correct flush
> function directly.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper.c | 52 +++++++++++++++++++++------------------------
>  1 file changed, 24 insertions(+), 28 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 7adbf51479..2b95fc763f 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -626,56 +626,54 @@ static void tlbiall_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                            uint64_t value)
>  {
>      /* Invalidate all (TLBIALL) */
> -    ARMCPU *cpu = env_archcpu(env);
> +    CPUState *cs = env_cpu(env);
>
>      if (tlb_force_broadcast(env)) {
> -        tlbiall_is_write(env, NULL, value);
> -        return;
> +        tlb_flush_all_cpus_synced(cs);
> +    } else {
> +        tlb_flush(cs);
>      }
> -
> -    tlb_flush(CPU(cpu));
>  }
>
>  static void tlbimva_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                            uint64_t value)
>  {
>      /* Invalidate single TLB entry by MVA and ASID (TLBIMVA) */
> -    ARMCPU *cpu = env_archcpu(env);
> +    CPUState *cs = env_cpu(env);
>
> +    value &= TARGET_PAGE_MASK;

I'm fairly sure this is superfluous (we certainly mask pages in the
cputlb code, don't know if we do at the translation end).

>      if (tlb_force_broadcast(env)) {
> -        tlbimva_is_write(env, NULL, value);
> -        return;
> +        tlb_flush_page_all_cpus_synced(cs, value);
> +    } else {
> +        tlb_flush_page(cs, value);
>      }
> -
> -    tlb_flush_page(CPU(cpu), value & TARGET_PAGE_MASK);
>  }
>
>  static void tlbiasid_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                             uint64_t value)
>  {
>      /* Invalidate by ASID (TLBIASID) */
> -    ARMCPU *cpu = env_archcpu(env);
> +    CPUState *cs = env_cpu(env);
>
>      if (tlb_force_broadcast(env)) {
> -        tlbiasid_is_write(env, NULL, value);
> -        return;
> +        tlb_flush_all_cpus_synced(cs);
> +    } else {
> +        tlb_flush(cs);
>      }
> -
> -    tlb_flush(CPU(cpu));
>  }
>
>  static void tlbimvaa_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                             uint64_t value)
>  {
>      /* Invalidate single entry by MVA, all ASIDs (TLBIMVAA) */
> -    ARMCPU *cpu = env_archcpu(env);
> +    CPUState *cs = env_cpu(env);
>
> +    value &= TARGET_PAGE_MASK;
>      if (tlb_force_broadcast(env)) {
> -        tlbimvaa_is_write(env, NULL, value);
> -        return;
> +        tlb_flush_page_all_cpus_synced(cs, value);
> +    } else {
> +        tlb_flush_page(cs, value);
>      }
> -
> -    tlb_flush_page(CPU(cpu), value & TARGET_PAGE_MASK);
>  }
>
>  static void tlbiall_nsnh_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -3926,11 +3924,10 @@ static void tlbi_aa64_vmalle1_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      int mask = vae1_tlbmask(env);
>
>      if (tlb_force_broadcast(env)) {
> -        tlbi_aa64_vmalle1is_write(env, NULL, value);
> -        return;
> +        tlb_flush_by_mmuidx_all_cpus_synced(cs, mask);
> +    } else {
> +        tlb_flush_by_mmuidx(cs, mask);
>      }
> -
> -    tlb_flush_by_mmuidx(cs, mask);
>  }
>
>  static int vmalle1_tlbmask(CPUARMState *env)
> @@ -4052,11 +4049,10 @@ static void tlbi_aa64_vae1_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      uint64_t pageaddr = sextract64(value << 12, 0, 56);
>
>      if (tlb_force_broadcast(env)) {
> -        tlbi_aa64_vae1is_write(env, NULL, value);
> -        return;
> +        tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, mask);
> +    } else {
> +        tlb_flush_page_by_mmuidx(cs, pageaddr, mask);
>      }
> -
> -    tlb_flush_page_by_mmuidx(cs, pageaddr, mask);
>  }
>
>  static void tlbi_aa64_vae2is_write(CPUARMState *env, const ARMCPRegInfo *ri,

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

--
Alex Bennée