Re: [PATCH 1/4] target/riscv: Remove misa_mxl validation

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2023/10/17 11:29, LIU Zhiwei wrote:
> 
> On 2023/10/12 13:42, Akihiko Odaki wrote:
>> It is initialized with a simple assignment and there is little room for
>> error. In fact, the validation is even more complex.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   target/riscv/cpu.c | 13 ++-----------
>>   1 file changed, 2 insertions(+), 11 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index f5572704de..550b357fb7 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -1042,7 +1042,7 @@ static void 
>> riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
>>       }
>>   }
>> -static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
>> +static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
>>   {
>>       RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
>>       CPUClass *cc = CPU_CLASS(mcc);
>> @@ -1062,11 +1062,6 @@ static void 
>> riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
>>       default:
>>           g_assert_not_reached();
>>       }
>> -
>> -    if (env->misa_mxl_max != env->misa_mxl) {
>> -        error_setg(errp, "misa_mxl_max must be equal to misa_mxl");
>> -        return;
>> -    }
>>   }
>>   /*
>> @@ -1447,11 +1442,7 @@ static void riscv_cpu_realize_tcg(DeviceState 
>> *dev, Error **errp)
>>           return;
>>       }
>> -    riscv_cpu_validate_misa_mxl(cpu, &local_err);
>> -    if (local_err != NULL) {
>> -        error_propagate(errp, local_err);
>> -        return;
>> -    }
>> +    riscv_cpu_validate_misa_mxl(cpu);
> 
> This it not right.  As we are still working on the supporting for MXL32 
> or SXL32, this validation is needed.

It's not preventing supporting MXL32 or SXL32. It's removing 
env->misa_mxl_max != env->misa_mxl just because it's initialized with a 
simple statement:
env->misa_mxl_max = env->misa_mxl = mxl;

It makes little sense to have a validation code that is more complex 
than the validated code.

> 
> And we can't ensure the all RISC-V cpus have the same misa_mxl_max or 
> misa_mxl,   it is not right to move it to class.
> For example, in the future, riscv64-softmmu can run 32-bit cpu and 
> 64-bit cpu. And maybe in heterogeneous SOC,
> we have 32-bit cpu and 64-bit cpu together.

This patch series does not touch misa_mxl. We don't need to ensure that 
all CPUs have the same misa_mxl_max, but we just need to ensure that 
CPUs in the same class do. Creating a heterogeneous SoC is still 
possible by combining e.g. TYPE_RISCV_CPU_SIFIVE_E31 and 
TYPE_RISCV_CPU_SIFIVE_E51, for example.