Re: [PATCH v2 03/12] target/ppc: Implement attn instruction on BookS 64-bit processors

[Nicholas Piggin]

On Wed May 22, 2024 at 3:34 AM AEST, Richard Henderson wrote:
> On 5/20/24 18:30, Nicholas Piggin wrote:
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index c358927211..2532408be0 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -3025,6 +3031,12 @@ static inline int check_pow_nocheck(CPUPPCState *env)
> >       return 1;
> >   }
> >   
> > +/* attn enable check                                                       
> >   */
> > +static inline int check_attn_none(CPUPPCState *env)
> > +{
> > +    return 0;
> > +}
>
> No point in putting this here, as a static inline...
>
> > @@ -2138,6 +2158,7 @@ POWERPC_FAMILY(405)(ObjectClass *oc, void *data)
> >       dc->desc = "PowerPC 405";
> >       pcc->init_proc = init_proc_405;
> >       pcc->check_pow = check_pow_nocheck;
> > +    pcc->check_attn = check_attn_none;
>
> ... when the only uses force an out-of-line instance.

Yeah... I was following existing convention (although apparently not
naming convention :/).

>
> Alternately,
>
> > +#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
> > +void helper_attn(CPUPPCState *env)
> > +{
> > +    /* POWER attn is unprivileged when enabled by HID, otherwise illegal */
> > +    if ((*env->check_attn)(env)) {
> > +        powerpc_checkstop(env, "host executed attn");
>
> ... allow the hook to be null to indicate no attn.

I'll add a todo list to change this and check_pow to follow this
suggestion.

>
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks,
Nick