On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjin...@huawei.com> wrote:
>
> Support ALLINT msr access as follow:
> mrs <xt>, ALLINT // read allint
> msr ALLINT, <xt> // write allint with imm
>
> Signed-off-by: Jinjie Ruan <ruanjin...@huawei.com>
> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> v5:
> - Add Reviewed-by.
> v4:
> - Remove arm_is_el2_enabled() check in allint_check().
> - Change to env->pstate instead of env->allint.
> v3:
> - Remove EL0 check in aa64_allint_access() which alreay checks in .access
> PL1_RW.
> - Use arm_hcrx_el2_eff() in aa64_allint_access() instead of
> env->cp15.hcrx_el2.
> - Make ALLINT msr access function controlled by aa64_nmi.
> ---
> target/arm/helper.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
If you configure with --target-list=aarch64-softmmu,arm-softmmu
you'll find this fails to build:
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index b19a0178ce..aa0151c775 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -4752,6 +4752,36 @@ static void aa64_daif_write(CPUARMState *env, const
> ARMCPRegInfo *ri,
> env->daif = value & PSTATE_DAIF;
> }
>
> +static void aa64_allint_write(CPUARMState *env, const ARMCPRegInfo *ri,
> + uint64_t value)
> +{
> + env->pstate = (env->pstate & ~PSTATE_ALLINT) | (value & PSTATE_ALLINT);
> +}
> +
> +static uint64_t aa64_allint_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> + return env->pstate & PSTATE_ALLINT;
> +}
> +
> +static CPAccessResult aa64_allint_access(CPUARMState *env,
> + const ARMCPRegInfo *ri, bool isread)
> +{
> + if (arm_current_el(env) == 1 && (arm_hcrx_el2_eff(env) & HCRX_TALLINT)) {
> + return CP_ACCESS_TRAP_EL2;
> + }
> + return CP_ACCESS_OK;
> +}
> +
> +static const ARMCPRegInfo nmi_reginfo[] = {
> + { .name = "ALLINT", .state = ARM_CP_STATE_AA64,
> + .opc0 = 3, .opc1 = 0, .opc2 = 0, .crn = 4, .crm = 3,
> + .type = ARM_CP_NO_RAW,
> + .access = PL1_RW, .accessfn = aa64_allint_access,
> + .fieldoffset = offsetof(CPUARMState, pstate),
> + .writefn = aa64_allint_write, .readfn = aa64_allint_read,
> + .resetfn = arm_cp_reset_ignore },
> +};
These functions and the array have been put in a bit of the
file that is built whether TARGET_AARCH64 is defined or not...
> +
> static uint64_t aa64_pan_read(CPUARMState *env, const ARMCPRegInfo *ri)
> {
> return env->pstate & PSTATE_PAN;
> @@ -9889,6 +9919,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> if (cpu_isar_feature(aa64_nv2, cpu)) {
> define_arm_cp_regs(cpu, nv2_reginfo);
> }
> +
> + if (cpu_isar_feature(aa64_nmi, cpu)) {
> + define_arm_cp_regs(cpu, nmi_reginfo);
> + }
> #endif
...but the only reference to them is inside an ifdef TARGET_AARCH64.
Moving the nmi_reginfo[] and the functions so they are next to
some other TARGET_AARCH64-only reginfo array inside one of the
existing ifdef blocks is probably the nicest fix.
>
> if (cpu_isar_feature(any_predinv, cpu)) {
> --
thanks
-- PMM
