Bibo Mao <[email protected]> writes:

> On 2026/5/29 下午2:38, Xianglai Li wrote:
>> Set the CSR_CRMD, CSR_ESTAT, CSR_MSGIS and CSR_PERFCTRL registers to the
>> reset state when the CPU is reset.
> Hi Xianglai,
>
> Thanks for the patch.
>
> Could you add some description how do you find this issue? such as by
> code reviewing or there is a real problem.

Also Loongarch seems to be one of the few that doesn't just do:

  memset(env, 0, offsetof(CPUFooState, end_reset_fields));

in the reset function.

>
>> Signed-off-by: Xianglai Li <[email protected]>
>> ---
>> Cc: Bibo Mao <[email protected]>
>> Cc: Song Gao <[email protected]>
>>   target/loongarch/cpu-csr.h |  7 +++++++
>>   target/loongarch/cpu.c     | 19 ++++++++++++++++++-
>>   2 files changed, 25 insertions(+), 1 deletion(-)
>> diff --git a/target/loongarch/cpu-csr.h b/target/loongarch/cpu-csr.h
>> index d860417af2..d936af8e57 100644
>> --- a/target/loongarch/cpu-csr.h
>> +++ b/target/loongarch/cpu-csr.h
>> @@ -211,6 +211,13 @@ FIELD(CSR_DMW_64, VSEG, 60, 4)
>>   #define LOONGARCH_CSR_PERFCTRL(N)    (0x200 + 2 * N)
>>   #define LOONGARCH_CSR_PERFCNTR(N)    (0x201 + 2 * N)
>>   +FIELD(CSR_PERFCTRL, EV, 0, 10)
>> +FIELD(CSR_PERFCTRL, PLV0, 16, 1)
>> +FIELD(CSR_PERFCTRL, PLV1, 17, 1)
>> +FIELD(CSR_PERFCTRL, PLV2, 18, 1)
>> +FIELD(CSR_PERFCTRL, PLV3, 19, 1)
>> +FIELD(CSR_PERFCTRL, PMIE, 20, 1)
>> +
>>   /* Debug CSRs */
>>   #define LOONGARCH_CSR_DBG            0x500 /* debug config */
>>   FIELD(CSR_DBG, DST, 0, 1)
>> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
>> index 8f277f7696..7f82c4c46c 100644
>> --- a/target/loongarch/cpu.c
>> +++ b/target/loongarch/cpu.c
>> @@ -622,6 +622,7 @@ static void loongarch_cpu_reset_hold(Object *obj, 
>> ResetType type)
>>       env->CSR_CRMD = FIELD_DP64(env->CSR_CRMD, CSR_CRMD, PG, 0);
>>       env->CSR_CRMD = FIELD_DP64(env->CSR_CRMD, CSR_CRMD, DATF, 0);
>>       env->CSR_CRMD = FIELD_DP64(env->CSR_CRMD, CSR_CRMD, DATM, 0);
>> +    env->CSR_CRMD = FIELD_DP64(env->CSR_CRMD, CSR_CRMD, WE, 0);
>>         env->CSR_EUEN = FIELD_DP64(env->CSR_EUEN, CSR_EUEN, FPE, 0);
>>       env->CSR_EUEN = FIELD_DP64(env->CSR_EUEN, CSR_EUEN, SXE, 0);
>> @@ -633,7 +634,7 @@ static void loongarch_cpu_reset_hold(Object *obj, 
>> ResetType type)
>>       env->CSR_ECFG = FIELD_DP64(env->CSR_ECFG, CSR_ECFG, VS, 0);
>>       env->CSR_ECFG = FIELD_DP64(env->CSR_ECFG, CSR_ECFG, LIE, 0);
>>   -    env->CSR_ESTAT = env->CSR_ESTAT & (~MAKE_64BIT_MASK(0, 2));
>> +    env->CSR_ESTAT = 0;
>>       env->CSR_RVACFG = FIELD_DP64(env->CSR_RVACFG, CSR_RVACFG, RBITS, 0);
>>       env->CSR_CPUID = cs->cpu_index;
>>       env->CSR_TCFG = FIELD_DP64(env->CSR_TCFG, CSR_TCFG, EN, 0);
>> @@ -641,6 +642,22 @@ static void loongarch_cpu_reset_hold(Object *obj, 
>> ResetType type)
>>       env->CSR_TLBRERA = FIELD_DP64(env->CSR_TLBRERA, CSR_TLBRERA, ISTLBR, 
>> 0);
>>       env->CSR_MERRCTL = FIELD_DP64(env->CSR_MERRCTL, CSR_MERRCTL, ISMERR, 
>> 0);
>>       env->CSR_TID = cs->cpu_index;
>> +
>> +    memset(env->CSR_MSGIS, 0, sizeof(env->CSR_MSGIS));
>> +    env->CSR_DBG = FIELD_DP64(env->CSR_DBG, CSR_DBG, DST, 0);
>> +    for (n = 0; n < MAX_PERF_EVENTS; n++) {
> how about for (n = 0; n < env.perf_event_num; n++) {  ?
>
> Regards
> Bibo Mao
>> +        env->CSR_PERFCTRL[n] = FIELD_DP64(env->CSR_PERFCTRL[n], 
>> CSR_PERFCTRL,
>> +                                          PLV0, 0);
>> +        env->CSR_PERFCTRL[n] = FIELD_DP64(env->CSR_PERFCTRL[n], 
>> CSR_PERFCTRL,
>> +                                          PLV1, 0);
>> +        env->CSR_PERFCTRL[n] = FIELD_DP64(env->CSR_PERFCTRL[n], 
>> CSR_PERFCTRL,
>> +                                          PLV2, 0);
>> +        env->CSR_PERFCTRL[n] = FIELD_DP64(env->CSR_PERFCTRL[n], 
>> CSR_PERFCTRL,
>> +                                          PLV3, 0);
>> +        env->CSR_PERFCTRL[n] = FIELD_DP64(env->CSR_PERFCTRL[n], 
>> CSR_PERFCTRL,
>> +                                          PMIE, 0);
>> +    }
>> +
>>       /*
>>        * Workaround for edk2-stable202408, CSR PGD register is set only if
>>        * its value is equal to zero for boot cpu, it causes reboot issue.
>> 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Reply via email to