Re: [RFC PATCH 1/6] target/ppc: Add support for the Processor Attention instruction

[Fabiano Rosas <farosas@linux.ibm.com>]

Leandro Lupori <leandro.lupori@eldorado.org.br> writes:

> From: Cédric Le Goater <clg@kaod.org>
>
> Check the HID0 bit to send signal, currently modeled as a checkstop.
> The QEMU implementation adds an exit using the GPR[3] value (that's a
> hack for tests)
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
> ---
>  target/ppc/cpu.h         |  8 ++++++++
>  target/ppc/excp_helper.c | 27 +++++++++++++++++++++++++++
>  target/ppc/helper.h      |  1 +
>  target/ppc/translate.c   | 14 ++++++++++++++
>  4 files changed, 50 insertions(+)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 047b24ba50..12f9f3a880 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -173,6 +173,12 @@ enum {
>      POWERPC_EXCP_PRIV_REG      = 0x02,  /* Privileged register exception     */
>      /* Trap                                                                  */
>      POWERPC_EXCP_TRAP          = 0x40,
> +    /* Processor Attention                                                   */
> +    POWERPC_EXCP_ATTN          = 0x100,
> +    /*
> +     * NOTE: POWERPC_EXCP_ATTN uses values from 0x100 to 0x1ff to return
> +     *       error codes.
> +     */
>  };
>  
>  #define PPC_INPUT(env) ((env)->bus_model)
> @@ -2089,6 +2095,8 @@ void ppc_compat_add_property(Object *obj, const char *name,
>  #define HID0_DOZE           (1 << 23)           /* pre-2.06 */
>  #define HID0_NAP            (1 << 22)           /* pre-2.06 */
>  #define HID0_HILE           PPC_BIT(19) /* POWER8 */
> +#define HID0_ATTN           PPC_BIT(31) /* Processor Attention */
> +#define HID0_POWER9_ATTN    PPC_BIT(3)
>  #define HID0_POWER9_HILE    PPC_BIT(4)
>  
>  /*****************************************************************************/
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index d3e2cfcd71..b0c629905c 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1379,6 +1379,9 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>              }
>              cs->halted = 1;
>              cpu_interrupt_exittb(cs);
> +            if ((env->error_code & ~0xff) == POWERPC_EXCP_ATTN) {
> +                exit(env->error_code & 0xff);
> +            }
>          }
>          if (env->msr_mask & MSR_HVB) {
>              /*
> @@ -1971,6 +1974,30 @@ void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
>      env->resume_as_sreset = (insn != PPC_PM_STOP) ||
>          (env->spr[SPR_PSSCR] & PSSCR_EC);
>  }
> +
> +/*
> + * Processor Attention instruction (Implementation dependent)
> + */
> +void helper_attn(CPUPPCState *env, target_ulong r3)
> +{
> +    bool attn = false;
> +
> +    if (env->excp_model == POWERPC_EXCP_POWER8) {
> +        attn = !!(env->spr[SPR_HID0] & HID0_ATTN);
> +    } else if (env->excp_model == POWERPC_EXCP_POWER9 ||
> +               env->excp_model == POWERPC_EXCP_POWER10) {
> +        attn = !!(env->spr[SPR_HID0] & HID0_POWER9_ATTN);
> +    }

The excp_model is not a CPU identifier. This should ideally be a flag
set during init_proc. Something like HID0_ATTN_P8/HID0_ATTN_P9.

Maybe we should consider adding a hid0_mask similar to lpcr_mask.

> +
> +    if (attn) {
> +        raise_exception_err(env, POWERPC_EXCP_MCHECK,
> +                            POWERPC_EXCP_ATTN | (r3 & 0xff));
> +    } else {
> +        raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
> +                               POWERPC_EXCP_INVAL |
> +                               POWERPC_EXCP_INVAL_INVAL, GETPC());
> +    }
> +}
>  #endif /* defined(TARGET_PPC64) */
>  
>  static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 57da11c77e..9a2497569b 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -14,6 +14,7 @@ DEF_HELPER_1(rfmci, void, env)
>  #if defined(TARGET_PPC64)
>  DEF_HELPER_2(scv, noreturn, env, i32)
>  DEF_HELPER_2(pminsn, void, env, i32)
> +DEF_HELPER_2(attn, void, env, tl)
>  DEF_HELPER_1(rfid, void, env)
>  DEF_HELPER_1(rfscv, void, env)
>  DEF_HELPER_1(hrfid, void, env)
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 408ae26173..5ace6f3a29 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4123,6 +4123,19 @@ static void gen_rvwinkle(DisasContext *ctx)
>      gen_exception_nip(ctx, EXCP_HLT, ctx->base.pc_next);
>  #endif /* defined(CONFIG_USER_ONLY) */
>  }
> +
> +static void gen_attn(DisasContext *ctx)
> +{
> + #if defined(CONFIG_USER_ONLY)
> +    GEN_PRIV;
> +#else
> +    CHK_SV;
> +
> +    gen_helper_attn(cpu_env, cpu_gpr[3]);
> +    ctx->base.is_jmp = DISAS_NORETURN;
> +#endif
> +}
> +
>  #endif /* #if defined(TARGET_PPC64) */
>  
>  static inline void gen_update_cfar(DisasContext *ctx, target_ulong nip)
> @@ -6844,6 +6857,7 @@ GEN_HANDLER_E(nap, 0x13, 0x12, 0x0d, 0x03FFF801, PPC_NONE, PPC2_PM_ISA206),
>  GEN_HANDLER_E(sleep, 0x13, 0x12, 0x0e, 0x03FFF801, PPC_NONE, PPC2_PM_ISA206),
>  GEN_HANDLER_E(rvwinkle, 0x13, 0x12, 0x0f, 0x03FFF801, PPC_NONE, PPC2_PM_ISA206),
>  GEN_HANDLER(hrfid, 0x13, 0x12, 0x08, 0x03FF8001, PPC_64H),
> +GEN_HANDLER(attn, 0x0, 0x00, 0x8, 0xfffffdff, PPC_FLOW),
>  #endif
>  /* Top bit of opc2 corresponds with low bit of LEV, so use two handlers */
>  GEN_HANDLER(sc, 0x11, 0x11, 0xFF, 0x03FFF01D, PPC_FLOW),