On Wed Jun 28, 2023 at 1:25 AM AEST, Fabiano Rosas wrote:
> Nicholas Piggin <npig...@gmail.com> writes:
>
> > attn is an implementation-specific instruction that on POWER (and G5/
> > 970) can be enabled with a HID bit (disabled = illegal), and executing
> > it causes the host processor to stop and the service processor to be
> > notified. Generally used for debugging.
> >
> > Implement attn and make it checkstop the system, which should be good
> > enough for QEMU debugging.
> >
> > Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> > ---
> > Since v1:
> > - New patch that also uses checkstop function. Works with skiboot.
> >
> > target/ppc/cpu.h | 2 ++
> > target/ppc/excp_helper.c | 28 ++++++++++++++++++++++++++++
> > target/ppc/helper.h | 2 ++
> > target/ppc/translate.c | 7 +++++++
> > 4 files changed, 39 insertions(+)
> >
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index 94497aa115..f6e93dec5f 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -2116,6 +2116,8 @@ void ppc_compat_add_property(Object *obj, const char
> > *name,
> > #define HID0_NAP (1 << 22) /* pre-2.06 */
> > #define HID0_HILE PPC_BIT(19) /* POWER8 */
> > #define HID0_POWER9_HILE PPC_BIT(4)
> > +#define HID0_ENABLE_ATTN PPC_BIT(31) /* POWER8 */
> > +#define HID0_POWER9_ENABLE_ATTN PPC_BIT(3)
> >
> >
> > /*****************************************************************************/
> > /* PowerPC Instructions types definitions
> > */
> > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > index 28d8a9b212..f46fdd2ee6 100644
> > --- a/target/ppc/excp_helper.c
> > +++ b/target/ppc/excp_helper.c
> > @@ -208,6 +208,34 @@ static void powerpc_checkstop(CPUPPCState *env, const
> > char *reason)
> > }
> >
> > #if defined(TARGET_PPC64)
> > +void helper_attn(CPUPPCState *env)
> > +{
> > + CPUState *cs = env_cpu(env);
> > + target_ulong hid0_attn = 0;
> > +
> > + switch (env->excp_model) {
> > + case POWERPC_EXCP_970:
> > + case POWERPC_EXCP_POWER7:
> > + case POWERPC_EXCP_POWER8:
> > + hid0_attn = HID0_ENABLE_ATTN;
> > + break;
> > + case POWERPC_EXCP_POWER9:
> > + case POWERPC_EXCP_POWER10:
> > + hid0_attn = HID0_POWER9_ENABLE_ATTN;
> > + break;
> > + default:
> > + break;
> > + }
>
> There's some precedent for checking HID bits using a cpu class
> function. See pcc->check_pow, check_pow_hid0() and
> check_pow_hid0_74xx(). I find it a bit nicer because the class carries
> all the data so it's easier to move code around.
Good suggestion, thanks.
> > +
> > + if (env->spr[SPR_HID0] & hid0_attn) {
> > + powerpc_checkstop(env, "host executed attn");
> > + cpu_loop_exit_noexc(cs);
> > + } else {
> > + raise_exception_err(env, POWERPC_EXCP_HV_EMU,
> > + POWERPC_EXCP_INVAL | POWERPC_EXCP_INVAL_INVAL);
> > + }
> > +}
> > +
> > static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
> > target_ulong *msr)
> > {
> > diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> > index fda40b8a60..50bb105c08 100644
> > --- a/target/ppc/helper.h
> > +++ b/target/ppc/helper.h
> > @@ -812,3 +812,5 @@ DEF_HELPER_4(DSCLIQ, void, env, fprp, fprp, i32)
> >
> > DEF_HELPER_1(tbegin, void, env)
> > DEF_HELPER_FLAGS_1(fixup_thrm, TCG_CALL_NO_RWG, void, env)
> > +
> > +DEF_HELPER_1(attn, void, env)
> > diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> > index 372ee600b2..4e9e606d77 100644
> > --- a/target/ppc/translate.c
> > +++ b/target/ppc/translate.c
> > @@ -6382,6 +6382,12 @@ static void gen_dform3D(DisasContext *ctx)
> > }
> >
> > #if defined(TARGET_PPC64)
> > +/* attn */
> > +static void gen_attn(DisasContext *ctx)
> > +{
> > + gen_helper_attn(cpu_env);
>
> In another incarnation of this patch, Cédric had a check for the
> privilege level and linux-user:
>
> +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
> +}
User only could be checked... On priv, I thought that attn is
unprivileged (so long as it is enabled in HID). I could check
what hardware does.
>
> > +}
> > +
> > /* brd */
> > static void gen_brd(DisasContext *ctx)
> > {
> > @@ -6413,6 +6419,7 @@ static void gen_brh(DisasContext *ctx)
> >
> > static opcode_t opcodes[] = {
> > #if defined(TARGET_PPC64)
> > +GEN_HANDLER_E(attn, 0x00, 0x00, 0x08, 0xFFFFFDFF, PPC_NONE,
> > PPC2_ISA207S),
>
> Aren't you missing the 970 with this? Maybe worth a insns_flag2 flag
> just for the attn?
Yes good catch, I started out just doing powernv but found 970 manual
and it has attn. Will update.
Thanks,
Nick
