Re: [PATCH for-8.1 v4 07/25] target/riscv: move pmp and epmp validations to validate_set_extensions()

[LIU Zhiwei]

On 2023/3/23 6:19, Daniel Henrique Barboza wrote:
> In the near future, write_misa() will use a variation of what we have
> now as riscv_cpu_validate_set_extensions(). The pmp and epmp validation
> will be required in write_misa()

I don't know why pmp and epmp should be checked in write_misa().
As write_misa can't alter the pmp and epmp setting, the check for 
pmp/epmp should only be at cpu object initialization time for one time.

Zhiwei

> and it's already required here in
> riscv_cpu_realize(), so move it to riscv_cpu_validate_set_extensions().
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   target/riscv/cpu.c | 19 +++++++++----------
>   1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 1a298e5e55..7458845fec 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -916,6 +916,15 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU 
> *cpu, Error **errp)
>       Error *local_err = NULL;
>       uint32_t ext = 0;
>   
> +    if (cpu->cfg.epmp && !cpu->cfg.pmp) {
> +        /*
> +         * Enhanced PMP should only be available
> +         * on harts with PMP support
> +         */
> +        error_setg(errp, "Invalid configuration: EPMP requires PMP support");
> +        return;
> +    }
> +
>       /* Do some ISA extension error checking */
>       if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
>                               cpu->cfg.ext_a && cpu->cfg.ext_f &&
> @@ -1228,16 +1237,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> **errp)
>           return;
>       }
>   
> -    if (cpu->cfg.epmp && !cpu->cfg.pmp) {
> -        /*
> -         * Enhanced PMP should only be available
> -         * on harts with PMP support
> -         */
> -        error_setg(errp, "Invalid configuration: EPMP requires PMP support");
> -        return;
> -    }
> -
> -
>   #ifndef CONFIG_USER_ONLY
>       if (cpu->cfg.ext_sstc) {
>           riscv_timer_init(cpu);