Re: [RFC PATCH v2 05/22] target/arm: Support MSR access to ALLINT

[Jinjie Ruan]

On 2024/2/22 3:28, Richard Henderson wrote:
> On 2/21/24 03:08, Jinjie Ruan via wrote:
>> Support ALLINT msr access as follow:
>>     mrs <xt>, ALLINT    // read allint
>>     msr ALLINT, <xt>    // write allint with imm
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>>   target/arm/helper.c | 32 ++++++++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index a3062cb2ad..211156d640 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -4618,6 +4618,31 @@ 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->allint = value & PSTATE_ALLINT;
>> +}
>> +
>> +static uint64_t aa64_allint_read(CPUARMState *env, const ARMCPRegInfo
>> *ri)
>> +{
>> +    return env->allint & PSTATE_ALLINT;
>> +}
>> +
>> +static CPAccessResult aa64_allint_access(CPUARMState *env,
>> +                                         const ARMCPRegInfo *ri, bool
>> isread)
>> +{
>> +    if (arm_current_el(env) == 0) {
>> +        return CP_ACCESS_TRAP_UNCATEGORIZED;
>> +    }
> 
> This is handled by .access PL1_RW.
> 
>> +
>> +    if (arm_current_el(env) == 1 && arm_is_el2_enabled(env) &&
>> +        cpu_isar_feature(aa64_hcx, env_archcpu(env)) &&
>> +        (env->cp15.hcrx_el2 & HCRX_TALLINT))
>> +        return CP_ACCESS_TRAP_EL2;
> 
> You should be using arm_hcrx_el2_eff(env).
> Missing braces.

I'll fix it, thank you!

> 
>> @@ -5437,6 +5462,13 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>>         .access = PL0_RW, .accessfn = aa64_daif_access,
>>         .fieldoffset = offsetof(CPUARMState, daif),
>>         .writefn = aa64_daif_write, .resetfn = arm_cp_reset_ignore },
>> +    { .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, allint),
>> +      .writefn = aa64_allint_write, .readfn = aa64_allint_read,
>> +      .resetfn = arm_cp_reset_ignore },
> 
> You cannot add ALLINT here in v8_cp_reginfo[].
> Compare fgt_reginfo[], and how it is registered.

I'll fix it, thank you!

> 
> 
> r~