Re: [PATCH v3 3/4] RISC-V: support vector extension csr

[Jim Wilson <jimw@sifive.com>]

On 1/2/20 7:33 PM, LIU Zhiwei wrote:
> Until v0.7.1 specification, vector status is still not defined for
> mstatus.

The v0.8 spec does define a VS bit in mstatus.

> @@ -107,11 +112,6 @@ static int pmp(CPURISCVState *env, int csrno)
>   /* User Floating-Point CSRs */
>   static int read_fflags(CPURISCVState *env, int csrno, target_ulong *val)
>   {
> -#if !defined(CONFIG_USER_ONLY)
> -    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> -        return -1;
> -    }
> -#endif
>       *val = riscv_cpu_get_fflags(env);
>       return 0;
>   }

This allows reads of fflags when it doesn't exist, and hence does not 
make much sense.  Instead of removing the code, you should add a check 
for the vector extension, since the vector extension requires that fcsr 
exist even if the base architecture doesn't include FP support.  Ideally 
this should use the VS bit, but if you don't have it then you can just 
check to see if the vector extension was enabled as a command line option.

While the vector spec says that fcsr must exist, it doesn't specify that 
the FP fields in fcsr are necessarily readable or writable when there is 
no FP.  It also doesn't specify whether the other FP related shadows of 
fcsr exist, like fflags.  This appears to have been left unspecified.  I 
don't think that you should be making fflags reads and writes work for a 
target with vector but without float.  I think it would make more sense 
to have fcsr behave 3 different ways depending on whether we have only 
F, only V, or both F and V.  And then we can support reads and writes of 
only the valid fields.

Jim