| Message ID | 20240704111218.712654-1-gaosong@loongson.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | target/loongarch: Set CSR_PRCFG1 and CSR_PRCFG2 values | expand |
On 2024/7/4 下午7:12, Song Gao wrote: > We set the value of register CSR_PRCFG3, but left out CSR_PRCFG1 > and CSR_PRCFG2. Set CSR_PRCFG1 and CSR_PRCFG2 according to the > default values of the physical machine. > > Signed-off-by: Song Gao <gaosong@loongson.cn> > --- > target/loongarch/cpu.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c > index 270f711f11..ad40750701 100644 > --- a/target/loongarch/cpu.c > +++ b/target/loongarch/cpu.c > @@ -538,6 +538,12 @@ static void loongarch_cpu_reset_hold(Object *obj, ResetType type) > env->CSR_MERRCTL = FIELD_DP64(env->CSR_MERRCTL, CSR_MERRCTL, ISMERR, 0); > env->CSR_TID = cs->cpu_index; > > + env->CSR_PRCFG1 = FIELD_DP64(env->CSR_PRCFG1, CSR_PRCFG1, SAVE_NUM, 8); > + env->CSR_PRCFG1 = FIELD_DP64(env->CSR_PRCFG1, CSR_PRCFG1, TIMER_BITS, 0x2f); > + env->CSR_PRCFG1 = FIELD_DP64(env->CSR_PRCFG1, CSR_PRCFG1, VSMAX, 7); > + > + env->CSR_PRCFG2 = 0x3ffff000; > + > env->CSR_PRCFG3 = FIELD_DP64(env->CSR_PRCFG3, CSR_PRCFG3, TLB_TYPE, 2); > env->CSR_PRCFG3 = FIELD_DP64(env->CSR_PRCFG3, CSR_PRCFG3, MTLB_ENTRY, 63); > env->CSR_PRCFG3 = FIELD_DP64(env->CSR_PRCFG3, CSR_PRCFG3, STLB_WAYS, 7); > For the function, it looks good to me. There are some nits: For PRCFG1-PRCFG3, it is constant. I thinks it had better be put at cpu instance_init() or instance_finalize(). Also it is strange that most of cpu_state should be cleared with 0 besides cpucfg/prcfg etc, such as end_reset_fields marker in other architectures. Maybe it will better if there is double check with cpu state change callback such as init/reset etc :) Regards Bibo Mao
在 2024/7/4 下午8:47, maobibo 写道: > > > On 2024/7/4 下午7:12, Song Gao wrote: >> We set the value of register CSR_PRCFG3, but left out CSR_PRCFG1 >> and CSR_PRCFG2. Set CSR_PRCFG1 and CSR_PRCFG2 according to the >> default values of the physical machine. >> >> Signed-off-by: Song Gao <gaosong@loongson.cn> >> --- >> target/loongarch/cpu.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c >> index 270f711f11..ad40750701 100644 >> --- a/target/loongarch/cpu.c >> +++ b/target/loongarch/cpu.c >> @@ -538,6 +538,12 @@ static void loongarch_cpu_reset_hold(Object >> *obj, ResetType type) >> env->CSR_MERRCTL = FIELD_DP64(env->CSR_MERRCTL, CSR_MERRCTL, >> ISMERR, 0); >> env->CSR_TID = cs->cpu_index; >> + env->CSR_PRCFG1 = FIELD_DP64(env->CSR_PRCFG1, CSR_PRCFG1, >> SAVE_NUM, 8); >> + env->CSR_PRCFG1 = FIELD_DP64(env->CSR_PRCFG1, CSR_PRCFG1, >> TIMER_BITS, 0x2f); >> + env->CSR_PRCFG1 = FIELD_DP64(env->CSR_PRCFG1, CSR_PRCFG1, VSMAX, >> 7); >> + >> + env->CSR_PRCFG2 = 0x3ffff000; >> + >> env->CSR_PRCFG3 = FIELD_DP64(env->CSR_PRCFG3, CSR_PRCFG3, >> TLB_TYPE, 2); >> env->CSR_PRCFG3 = FIELD_DP64(env->CSR_PRCFG3, CSR_PRCFG3, >> MTLB_ENTRY, 63); >> env->CSR_PRCFG3 = FIELD_DP64(env->CSR_PRCFG3, CSR_PRCFG3, >> STLB_WAYS, 7); >> > For the function, it looks good to me. There are some nits: > For PRCFG1-PRCFG3, it is constant. I thinks it had better be put at > cpu instance_init() or instance_finalize(). You are right, we should put them in instance_init(). > > Also it is strange that most of cpu_state should be cleared with 0 > besides cpucfg/prcfg etc, such as end_reset_fields marker in other > architectures. > > Maybe it will better if there is double check with cpu state change > callback such as init/reset etc :) > The CSRs that need to be reset are from chapter 6, section 4 of the manual, and they are not always reset to 0. I think maybe that's why end_reset_fields was not used at the time. We should add some comments to loongarch_cpu_reset_hold(). Thanks. Song Gao > Regards > Bibo Mao
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c index 270f711f11..ad40750701 100644 --- a/target/loongarch/cpu.c +++ b/target/loongarch/cpu.c @@ -538,6 +538,12 @@ static void loongarch_cpu_reset_hold(Object *obj, ResetType type) env->CSR_MERRCTL = FIELD_DP64(env->CSR_MERRCTL, CSR_MERRCTL, ISMERR, 0); env->CSR_TID = cs->cpu_index; + env->CSR_PRCFG1 = FIELD_DP64(env->CSR_PRCFG1, CSR_PRCFG1, SAVE_NUM, 8); + env->CSR_PRCFG1 = FIELD_DP64(env->CSR_PRCFG1, CSR_PRCFG1, TIMER_BITS, 0x2f); + env->CSR_PRCFG1 = FIELD_DP64(env->CSR_PRCFG1, CSR_PRCFG1, VSMAX, 7); + + env->CSR_PRCFG2 = 0x3ffff000; + env->CSR_PRCFG3 = FIELD_DP64(env->CSR_PRCFG3, CSR_PRCFG3, TLB_TYPE, 2); env->CSR_PRCFG3 = FIELD_DP64(env->CSR_PRCFG3, CSR_PRCFG3, MTLB_ENTRY, 63); env->CSR_PRCFG3 = FIELD_DP64(env->CSR_PRCFG3, CSR_PRCFG3, STLB_WAYS, 7);
We set the value of register CSR_PRCFG3, but left out CSR_PRCFG1 and CSR_PRCFG2. Set CSR_PRCFG1 and CSR_PRCFG2 according to the default values of the physical machine. Signed-off-by: Song Gao <gaosong@loongson.cn> --- target/loongarch/cpu.c | 6 ++++++ 1 file changed, 6 insertions(+)