Re: [RFC PATCH v3 04/21] target/arm: Implement ALLINT MSR (immediate)

[Richard Henderson]

On 2/25/24 16:22, Jinjie Ruan wrote:
> On 2024/2/24 3:03, Richard Henderson wrote:
> > On 2/23/24 00:32, Jinjie Ruan via wrote:
> > > Add ALLINT MSR (immediate) to decodetree. And the EL0 check is necessary
> > > to ALLINT. Avoid the unconditional write to pc and use raise_exception_ra
> > > to unwind.
> > >
> > > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> > > ---
> > > v3:
> > > - Remove EL0 check in allint_check().
> > > - Add TALLINT check for EL1 in allint_check().
> > > - Remove unnecessarily arm_rebuild_hflags() in msr_i_allint helper.
> > > ---
> > >    target/arm/tcg/a64.decode      |  1 +
> > >    target/arm/tcg/helper-a64.c    | 24 ++++++++++++++++++++++++
> > >    target/arm/tcg/helper-a64.h    |  1 +
> > >    target/arm/tcg/translate-a64.c | 10 ++++++++++
> > >    4 files changed, 36 insertions(+)
> > >
> > > diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
> > > index 8a20dce3c8..3588080024 100644
> > > --- a/target/arm/tcg/a64.decode
> > > +++ b/target/arm/tcg/a64.decode
> > > @@ -207,6 +207,7 @@ MSR_i_DIT       1101 0101 0000 0 011 0100 .... 010
> > > 11111 @msr_i
> > >    MSR_i_TCO       1101 0101 0000 0 011 0100 .... 100 11111 @msr_i
> > >    MSR_i_DAIFSET   1101 0101 0000 0 011 0100 .... 110 11111 @msr_i
> > >    MSR_i_DAIFCLEAR 1101 0101 0000 0 011 0100 .... 111 11111 @msr_i
> > > +MSR_i_ALLINT    1101 0101 0000 0 001 0100 .... 000 11111 @msr_i
> >
> > Decode is incorrect either here, or in trans_MSR_i_ALLINT, because CRm
> > != '000x' is UNDEFINED.
> >
> > MSR_i_ALLINT    1101 0101 0000 0 001 0100 000 imm:1 000 11111
> >
> > is perhaps the clearest implementation.
> >
> > > +static void allint_check(CPUARMState *env, uint32_t op,
> > > +                       uint32_t imm, uintptr_t ra)
> > > +{
> > > +    /* ALLINT update to PSTATE. */
> > > +    if (arm_current_el(env) == 1 && arm_is_el2_enabled(env) &&
> > > +        (arm_hcrx_el2_eff(env) & HCRX_TALLINT)) {
> > > +        raise_exception_ra(env, EXCP_UDEF,
> > > +                           syn_aa64_sysregtrap(0, extract32(op, 0, 3),
> > > +                                               extract32(op, 3, 3), 4,
> > > +                                               imm, 0x1f, 0),
> > > +                           exception_target_el(env), ra);
> > > +    }
> > > +}
> > > +
> > > +void HELPER(msr_i_allint)(CPUARMState *env, uint32_t imm)
> > > +{
> > > +    allint_check(env, 0x8, imm, GETPC());
> >
> > As previously noted, the check for MSR_i only applies to imm==1, not 0.
>
> Sorry! The hardware manual I looked at didn't say this.

In DDI0487J.a, C6.2.229 MSR (immediate), it is present in the pseudocode

  when PSTATEField_ALLINT
    if (PSTATE.EL == EL1 && IsHCRXEL2Enabled()
        && HCRX_EL2.TALLINT == '1' && CRm<0> == '1') then
      AArch64.SystemAccessTrap(EL2, 0x18);
    PSTATE.ALLINT = CRm<0>;

In D19.2.49 HCRX_EL2, it is present as text for the description of TALLINT.


r~