Re: [PATCH v4 3/4] target/riscv: support vector extension csr

[Richard Henderson]

On 2/10/20 8:12 AM, LIU Zhiwei wrote:
> +static int vs(CPURISCVState *env, int csrno)
> +{
> +    return 0;
> +}

This should at least be testing RVV, a-la smode().

You probably want to have all of the other tests vs RVV in this file use this
function, since this will need to grow the system mode enable test.

> @@ -158,8 +167,10 @@ static int read_fcsr(CPURISCVState *env, int csrno, 
> target_ulong *val)
>          return -1;
>      }
>  #endif
> -    *val = (riscv_cpu_get_fflags(env) << FSR_AEXC_SHIFT)
> -        | (env->frm << FSR_RD_SHIFT);
> +    *val = (env->vext.vxrm << FSR_VXRM_SHIFT)
> +            | (env->vext.vxsat << FSR_VXSAT_SHIFT)
> +            | (riscv_cpu_get_fflags(env) << FSR_AEXC_SHIFT)
> +            | (env->frm << FSR_RD_SHIFT);
>      return 0;
>  }

While we can be perfectly happy shifting 0's into place here, it would probably
be clearer to conditionalize on vs().

> @@ -172,10 +183,60 @@ static int write_fcsr(CPURISCVState *env, int csrno, 
> target_ulong val)
>      env->mstatus |= MSTATUS_FS;
>  #endif
>      env->frm = (val & FSR_RD) >> FSR_RD_SHIFT;
> +    env->vext.vxrm = (val & FSR_VXRM) >> FSR_VXRM_SHIFT;
> +    env->vext.vxsat = (val & FSR_VXSAT) >> FSR_VXSAT_SHIFT;
>      riscv_cpu_set_fflags(env, (val & FSR_AEXC) >> FSR_AEXC_SHIFT);
>      return 0;
>  }

You *must* test vs() here.


r~