Re: [Qemu-ppc] [QEMU-PPC] [PATCH V4 06/11] target/ppc: Remove the function ppc_hash64_set_sdr1()

[David Gibson]

On Fri, Feb 24, 2017 at 12:05:12PM +1100, Suraj Jitindar Singh wrote:
> The function ppc_hash64_set_sdr1 basically checked the htabsize and set an
> error if it was too big, otherwise it just stored the value in SPR_SDR1.
> 
> Given that the only function which calls ppc_hash64_set_sdr1() is
> ppc_store_sdr1(), why not handle the checking in ppc_store_sdr1() to avoid
> the extra function call. Note that ppc_store_sdr1() already stores the
> value in SPR_SDR1 anyway, so we were doing it twice.
> 
> Signed-off-by: Suraj Jitindar Singh <address@hidden>

This is a sensible cleanup to follow on from my series.  One nit..



> 
> ---
> 
> V3 -> V4:
>  - Split patch from next patch in series
>  - Removed unnecessary function instead of renaming it
> ---
>  target/ppc/mmu-hash64.c | 18 ------------------
>  target/ppc/mmu-hash64.h |  3 ---
>  target/ppc/mmu_helper.c | 16 +++++++++-------
>  3 files changed, 9 insertions(+), 28 deletions(-)
> 
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index 0c6a1e7..7e91290 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -289,24 +289,6 @@ target_ulong helper_load_slb_vsid(CPUPPCState *env, 
> target_ulong rb)
>      return rt;
>  }
>  
> -/*
> - * 64-bit hash table MMU handling
> - */
> -void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value,
> -                         Error **errp)
> -{
> -    CPUPPCState *env = &cpu->env;
> -    target_ulong htabsize = value & SDR_64_HTABSIZE;
> -
> -    if (htabsize > 28) {
> -        error_setg(errp,
> -                   "Invalid HTABSIZE 0x" TARGET_FMT_lx" stored in SDR1",
> -                   htabsize);
> -        return;
> -    }
> -    env->spr[SPR_SDR1] = value;
> -}
> -
>  static int ppc_hash64_pte_prot(PowerPCCPU *cpu,
>                                 ppc_slb_t *slb, ppc_hash_pte64_t pte)
>  {
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index 9c33e9d..a2a6748 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -109,9 +109,6 @@ static inline hwaddr ppc_hash64_hpt_mask(PowerPCCPU *cpu)
>      return (1ULL << ((cpu->env.spr[SPR_SDR1] & SDR_64_HTABSIZE) + 18 - 7)) - 
> 1;
>  }
>  
> -void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value,
> -                         Error **errp);
> -
>  struct ppc_hash_pte64 {
>      uint64_t pte0, pte1;
>  };
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 0176ab6..eb5c35f 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -2006,13 +2006,15 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong 
> value)
>      assert(!cpu->vhyp);
>  #if defined(TARGET_PPC64)
>      if (env->mmu_model & POWERPC_MMU_64) {
> -        PowerPCCPU *cpu = ppc_env_get_cpu(env);
> -        Error *local_err = NULL;
> -
> -        ppc_hash64_set_sdr1(cpu, value, &local_err);
> -        if (local_err) {
> -            error_report_err(local_err);
> -            error_free(local_err);
> +        Error *errp = NULL;
> +        target_ulong htabsize = value & SDR_64_HTABSIZE;
> +
> +        if (htabsize > 28) {
> +            error_setg(&errp,
> +                       "Invalid HTABSIZE 0x" TARGET_FMT_lx" stored in SDR1",
> +                       htabsize);
> +            error_report_err(errp);
> +            error_free(errp);
>              return;

The above is basically equivalent to just an error_report() and return;

>          }
>      }

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature