| Message ID | 20240223103221.1142518-5-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm: Implement FEAT_NMI and FEAT_GICv3_NMI | expand |
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. As previously noted, with ALLINT in env->pstate, you can implement this completely inline for EL[23], or EL1 with imm==0. No point in passing in "op" and extracting, because you know exactly what the value should be for all MSR ALLINT. r~
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. > > As previously noted, with ALLINT in env->pstate, you can implement this > completely inline for EL[23], or EL1 with imm==0. > > No point in passing in "op" and extracting, because you know exactly > what the value should be for all MSR ALLINT. > > > r~
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~
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 MSR_i_SVCR 1101 0101 0000 0 011 0100 0 mask:2 imm:1 011 11111 # MRS, MSR (register), SYS, SYSL. These are all essentially the diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c index ebaa7f00df..58fb28a6b3 100644 --- a/target/arm/tcg/helper-a64.c +++ b/target/arm/tcg/helper-a64.c @@ -66,6 +66,30 @@ void HELPER(msr_i_spsel)(CPUARMState *env, uint32_t imm) update_spsel(env, imm); } +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()); + if (imm == 1) { + env->allint |= PSTATE_ALLINT; + } else { + env->allint &= ~PSTATE_ALLINT; + } +} + static void daif_check(CPUARMState *env, uint32_t op, uint32_t imm, uintptr_t ra) { diff --git a/target/arm/tcg/helper-a64.h b/target/arm/tcg/helper-a64.h index 575a5dab7d..3aec703d4a 100644 --- a/target/arm/tcg/helper-a64.h +++ b/target/arm/tcg/helper-a64.h @@ -22,6 +22,7 @@ DEF_HELPER_FLAGS_1(rbit64, TCG_CALL_NO_RWG_SE, i64, i64) DEF_HELPER_2(msr_i_spsel, void, env, i32) DEF_HELPER_2(msr_i_daifset, void, env, i32) DEF_HELPER_2(msr_i_daifclear, void, env, i32) +DEF_HELPER_2(msr_i_allint, void, env, i32) DEF_HELPER_3(vfp_cmph_a64, i64, f16, f16, ptr) DEF_HELPER_3(vfp_cmpeh_a64, i64, f16, f16, ptr) DEF_HELPER_3(vfp_cmps_a64, i64, f32, f32, ptr) diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c index 340265beb0..f1800f7c71 100644 --- a/target/arm/tcg/translate-a64.c +++ b/target/arm/tcg/translate-a64.c @@ -2036,6 +2036,16 @@ static bool trans_MSR_i_DAIFCLEAR(DisasContext *s, arg_i *a) return true; } +static bool trans_MSR_i_ALLINT(DisasContext *s, arg_i *a) +{ + if (!dc_isar_feature(aa64_nmi, s) || s->current_el == 0) { + return false; + } + gen_helper_msr_i_allint(tcg_env, tcg_constant_i32(a->imm)); + s->base.is_jmp = DISAS_TOO_MANY; + return true; +} + static bool trans_MSR_i_SVCR(DisasContext *s, arg_MSR_i_SVCR *a) { if (!dc_isar_feature(aa64_sme, s) || a->mask == 0) {
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(+)