Re: [Qemu-devel] [PATCH 02/32] ppc: Provide basic raise_exception_* functions

[David Gibson]

On Wed, Jul 27, 2016 at 08:20:56AM +1000, Benjamin Herrenschmidt wrote:
> Instead of using the same helpers called from translate.c, let's have
> a bunch of functions that take the various argument combinations,
> especially the retaddr which will be needed in subsequent patches,
> and leave the helpers to be just that, helpers for translate.c
> 
> We don't yet convert all users, we'll go through them in subsequent
> patches.
> 
> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
> ---
>  target-ppc/cpu.h         |  8 ++++++++
>  target-ppc/excp_helper.c | 51 
> ++++++++++++++++++++++++++++++++----------------
>  2 files changed, 42 insertions(+), 17 deletions(-)
> 
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 5fce1ff..2ee7a5e 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -2295,6 +2295,14 @@ static inline void cpu_get_tb_cpu_state(CPUPPCState 
> *env, target_ulong *pc,
>      *flags = env->hflags;
>  }
>  
> +void QEMU_NORETURN raise_exception(CPUPPCState *env, uint32_t exception);
> +void QEMU_NORETURN raise_exception_ra(CPUPPCState *env, uint32_t exception,
> +                                      uintptr_t raddr);
> +void QEMU_NORETURN raise_exception_err(CPUPPCState *env, uint32_t exception,
> +                                       uint32_t error_code);
> +void QEMU_NORETURN raise_exception_err_ra(CPUPPCState *env, uint32_t 
> exception,
> +                                          uint32_t error_code, uintptr_t 
> raddr);
> +
>  #if !defined(CONFIG_USER_ONLY)
>  static inline int booke206_tlbm_id(CPUPPCState *env, ppcmas_tlb_t *tlbm)
>  {
> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
> index d6e1678..f4b115e 100644
> --- a/target-ppc/excp_helper.c
> +++ b/target-ppc/excp_helper.c
> @@ -898,34 +898,53 @@ static void cpu_dump_rfi(target_ulong RA, target_ulong 
> msr)
>  
> /*****************************************************************************/
>  /* Exceptions processing helpers */
>  
> -void helper_raise_exception_err(CPUPPCState *env, uint32_t exception,
> -                                uint32_t error_code)
> +void raise_exception_err_ra(CPUPPCState *env, uint32_t exception,
> +                            uint32_t error_code, uintptr_t raddr)
>  {
>      CPUState *cs = CPU(ppc_env_get_cpu(env));
>  
> -#if 0
> -    printf("Raise exception %3x code : %d\n", exception, error_code);
> -#endif
>      cs->exception_index = exception;
>      env->error_code = error_code;
> -    cpu_loop_exit(cs);
> +    cpu_loop_exit_restore(cs, raddr);
> +}
> +
> +void raise_exception_err(CPUPPCState *env, uint32_t exception,
> +                         uint32_t error_code)
> +{
> +    raise_exception_err_ra(env, exception, error_code, 0);
> +}
> +
> +void raise_exception(CPUPPCState *env, uint32_t exception)
> +{
> +    raise_exception_err_ra(env, exception, 0, 0);
> +}
> +
> +void raise_exception_ra(CPUPPCState *env, uint32_t exception,
> +                        uintptr_t raddr)
> +{
> +    raise_exception_err_ra(env, exception, 0, 0);

This should pass raddr as the last argument, shouldn't it?

> +}
> +
> +void helper_raise_exception_err(CPUPPCState *env, uint32_t exception,
> +                                uint32_t error_code)
> +{
> +    raise_exception_err_ra(env, exception, error_code, 0);
>  }
>  
>  void helper_raise_exception(CPUPPCState *env, uint32_t exception)
>  {
> -    helper_raise_exception_err(env, exception, 0);
> +    raise_exception_err_ra(env, exception, 0, 0);
>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
>  void helper_store_msr(CPUPPCState *env, target_ulong val)
>  {
> -    CPUState *cs;
> +    uint32_t excp = hreg_store_msr(env, val, 0);
>  
> -    val = hreg_store_msr(env, val, 0);
> -    if (val != 0) {
> -        cs = CPU(ppc_env_get_cpu(env));
> +    if (excp != 0) {
> +        CPUState *cs = CPU(ppc_env_get_cpu(env));
>          cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> -        helper_raise_exception(env, val);
> +        raise_exception(env, excp);
>      }
>  }
>  
> @@ -951,7 +970,7 @@ void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t 
> insn)
>       * but this doesn't seem to be a problem.
>       */
>      env->msr |= (1ull << MSR_EE);
> -    helper_raise_exception(env, EXCP_HLT);
> +    raise_exception(env, EXCP_HLT);
>  }
>  #endif /* defined(TARGET_PPC64) */
>  
> @@ -1041,8 +1060,7 @@ void helper_tw(CPUPPCState *env, target_ulong arg1, 
> target_ulong arg2,
>                    ((int32_t)arg1 == (int32_t)arg2 && (flags & 0x04)) ||
>                    ((uint32_t)arg1 < (uint32_t)arg2 && (flags & 0x02)) ||
>                    ((uint32_t)arg1 > (uint32_t)arg2 && (flags & 0x01))))) {
> -        helper_raise_exception_err(env, POWERPC_EXCP_PROGRAM,
> -                                   POWERPC_EXCP_TRAP);
> +        raise_exception_err(env, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_TRAP);
>      }
>  }
>  
> @@ -1055,8 +1073,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1, 
> target_ulong arg2,
>                    ((int64_t)arg1 == (int64_t)arg2 && (flags & 0x04)) ||
>                    ((uint64_t)arg1 < (uint64_t)arg2 && (flags & 0x02)) ||
>                    ((uint64_t)arg1 > (uint64_t)arg2 && (flags & 0x01))))) {
> -        helper_raise_exception_err(env, POWERPC_EXCP_PROGRAM,
> -                                   POWERPC_EXCP_TRAP);
> +        raise_exception_err(env, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_TRAP);
>      }
>  }
>  #endif

-- 
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