Re: [PATCH for-8.1 v4 22/25] target/riscv: use misa_ext val in riscv_cpu_validate_extensions()

[liweiwei]

On 2023/3/23 06:20, Daniel Henrique Barboza wrote:
> Similar to what we did with riscv_cpu_validate_misa_ext(), let's read
> all MISA bits from a misa_ext val instead of reading from the cpu->cfg
> object.
>
> This will allow write_misa() to use riscv_cpu_validate_extensions().
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   target/riscv/cpu.c | 25 ++++++++++++++-----------
>   1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ed02332093..0e6b8fb45e 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1109,10 +1109,13 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, 
> Error **errp)
>   }
>   
>   /*
> - * Check consistency between chosen extensions. No changes
> - * in env->misa_ext are made.
> + * Check consistency between cpu->cfg extensions and a
> + * candidate misa_ext value. No changes in env->misa_ext
> + * are made.
>    */
> -static void riscv_cpu_validate_extensions(RISCVCPU *cpu, Error **errp)
> +static void riscv_cpu_validate_extensions(RISCVCPU *cpu,
> +                                          uint32_t misa_ext,
> +                                          Error **errp)
>   {
>       if (cpu->cfg.epmp && !cpu->cfg.pmp) {
>           /*
> @@ -1123,12 +1126,12 @@ static void riscv_cpu_validate_extensions(RISCVCPU 
> *cpu, Error **errp)
>           return;
>       }
>   
> -    if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
> +    if (misa_ext & RVF && !cpu->cfg.ext_icsr) {
>           error_setg(errp, "F extension requires Zicsr");
>           return;
>       }

This check needn't be done for write_misa. Similar to several other checks.

Maybe we can take them out as a new function to share between 
validate_extensions and write_misa.

In fact, only a few check should be done for write_misa when 
multi-letter extensions require single-letter extensions.

Regards,

Weiwei Li

>   
> -    if ((cpu->cfg.ext_zawrs) && !cpu->cfg.ext_a) {
> +    if ((cpu->cfg.ext_zawrs) && !(misa_ext & RVA)) {
>           error_setg(errp, "Zawrs extension requires A extension");
>           return;
>       }
> @@ -1137,13 +1140,13 @@ static void riscv_cpu_validate_extensions(RISCVCPU 
> *cpu, Error **errp)
>           cpu->cfg.ext_zfhmin = true;
>       }
>   
> -    if (cpu->cfg.ext_zfhmin && !cpu->cfg.ext_f) {
> +    if (cpu->cfg.ext_zfhmin && !(misa_ext & RVF)) {
>           error_setg(errp, "Zfh/Zfhmin extensions require F extension");
>           return;
>       }
>   
>       /* The V vector extension depends on the Zve64d extension */
> -    if (cpu->cfg.ext_v) {
> +    if (misa_ext & RVV) {
>           cpu->cfg.ext_zve64d = true;
>       }
>   
> @@ -1157,12 +1160,12 @@ static void riscv_cpu_validate_extensions(RISCVCPU *cpu, Error **errp)
>           cpu->cfg.ext_zve32f = true;
>       }
>   
> -    if (cpu->cfg.ext_zve64d && !cpu->cfg.ext_d) {
> +    if (cpu->cfg.ext_zve64d && !(misa_ext & RVD)) {
>           error_setg(errp, "Zve64d/V extensions require D extension");
>           return;
>       }
>   
> -    if (cpu->cfg.ext_zve32f && !cpu->cfg.ext_f) {
> +    if (cpu->cfg.ext_zve32f && !(misa_ext & RVF)) {
>           error_setg(errp, "Zve32f/Zve64f extensions require F extension");
>           return;
>       }
> @@ -1195,7 +1198,7 @@ static void riscv_cpu_validate_extensions(RISCVCPU *cpu, 
> Error **errp)
>               error_setg(errp, "Zfinx extension requires Zicsr");
>               return;
>           }
> -        if (cpu->cfg.ext_f) {
> +        if (misa_ext & RVF) {
>               error_setg(errp,
>                          "Zfinx cannot be supported together with F extension");
>               return;
> @@ -1367,7 +1370,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> **errp)
>           return;
>       }
>   
> -    riscv_cpu_validate_extensions(cpu, &local_err);
> +    riscv_cpu_validate_extensions(cpu, env->misa_ext, &local_err);
>       if (local_err != NULL) {
>           error_propagate(errp, local_err);
>           return;