On 3/21/23 00:25, liweiwei wrote:
>
> On 2023/3/19 04:04, Daniel Henrique Barboza wrote:
>> Allow write_misa() to enable RVG by changing riscv_cpu_enable_g()
>> slighty: instead of returning void, return the current env->misa_ext
>> value. This is then retrieved by 'val', which will add the RVG flag
>> itself, and then we'll skip validation and go right into commiting the
>> changes.
>>
>> The reason why it's ok to skip validation in this case is because we're
>> only allowing RVG (and its associated extensions/Z extensions) to be
>> enabled in the hart, and riscv_cpu_enable_g() already does its own
>> validation before enabling itself. Everything else is considered to be
>> already validated beforehand, so we don't need to repeat ourselves.
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>> target/riscv/cpu.c | 7 +++++--
>> target/riscv/cpu.h | 2 ++
>> target/riscv/csr.c | 15 +++++++++++++++
>> 3 files changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 2d2a354af3..73a5fa46ee 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -281,7 +281,8 @@ static uint32_t riscv_get_misa_ext_with_cpucfg(RISCVCPUConfig *cfg)
>> return ext;
>> }
>> -static void riscv_cpu_enable_g(RISCVCPU *cpu, Error **errp)
>> +
>> +target_ulong riscv_cpu_enable_g(RISCVCPU *cpu, Error **errp)
>> {
>> CPURISCVState *env = &cpu->env;
>> RISCVCPUConfig *cfg = &cpu->cfg;
>> @@ -289,7 +290,7 @@ static void riscv_cpu_enable_g(RISCVCPU *cpu, Error **errp)
>> if (cpu->cfg.ext_zfinx) {
>> error_setg(errp, "Unable to enable G: Zfinx is enabled, "
>> "so F can not be enabled");
>> - return;
>> + return 0;
>> }
>> if (!(cfg->ext_i && cfg->ext_m && cfg->ext_a &&
>> @@ -315,6 +316,8 @@ static void riscv_cpu_enable_g(RISCVCPU *cpu, Error **errp)
>> cfg->ext_icsr = true;
>> cfg->ext_ifencei = true;
>> }
>> +
>> + return env->misa_ext;
>> }
>> static void riscv_set_cpucfg_with_misa(RISCVCPUConfig *cfg,
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index befc3b8fff..3ca1d4903c 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -599,6 +599,8 @@ void riscv_cpu_validate_extensions(RISCVCPU *cpu, uint32_t misa_ext,
>> Error **errp);
>> void riscv_cpu_commit_cpu_cfg(RISCVCPU *cpu, uint32_t misa_ext);
>> +target_ulong riscv_cpu_enable_g(RISCVCPU *cpu, Error **errp);
>> +
>> #define cpu_list riscv_cpu_list
>> #define cpu_mmu_index riscv_cpu_mmu_index
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index 839862f1a8..4335398c19 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -1381,6 +1381,20 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
>> val &= RVE;
>> }
>> + if (val & RVG && !(env->misa_ext & RVG)) {
>> + /*
>> + * If the write wants to enable RVG, only RVG and
>> + * its dependencies will be updated in the CSR.
>> + */
>> + val = riscv_cpu_enable_g(cpu, &local_err);
>> + if (local_err != NULL) {
>> + return RISCV_EXCP_NONE;
>> + }
>> +
>> + val |= RVG;
>
> This assignment is not necessary, since RVG is already set in val.
>
> By the way, RVG is still not disabled if any some of included extensions are disabled by write_misa.
I'll include this use case in v4. And I'll also treat RVG as a regular extension since
all checks were already done in realize() time.
Daniel
>
> Regards,
>
> Weiwei Li
>
>> + goto commit;
>> + }
>> +
>> /*
>> * This flow is similar to what riscv_cpu_realize() does,
>> * with the difference that we will update env->misa_ext
>> @@ -1396,6 +1410,7 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
>> return RISCV_EXCP_NONE;
>> }
>> +commit:
>> riscv_cpu_commit_cpu_cfg(cpu, val);
>> if (!(val & RVF)) {
>