Re: [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line

[LIU Zhiwei]

Hi, Alistair

On 2020/2/19 6:34, Alistair Francis wrote:
> On Mon, Feb 10, 2020 at 12:12 AM LIU Zhiwei <address@hidden> wrote:
> > Vector extension is default on only for "any" cpu. It can be turned
> > on by command line "-cpu rv64,v=true,vlen=128,elen=64,vext_spec=v0.7.1".
> >
> > vlen is the vector register length, default value is 128 bit.
> > elen is the max operator size in bits, default value is 64 bit.
> > vext_spec is the vector specification version, default value is v0.7.1.
> > Thest properties and cpu can be specified with other values.
> >
> > Signed-off-by: LIU Zhiwei <address@hidden>
> This looks fine to me. Shouldn't this be the last patch though?
Yes， it should be the last patch.
> As in
> once the vector extension has been added to QEMU you can turn it on
> from the command line. Right now this turns it on but it isn't
> implemented.
Maybe I should just add fields in RISCVCPU structure. And never open the
vector extension on or add configure properties until the implementation 
is ready.

It's still a little awkward as the reviewers will not be able to test 
the patch until the
last patch.

> Alistair
>
> > ---
> >   target/riscv/cpu.c | 48 ++++++++++++++++++++++++++++++++++++++++++++--
> >   target/riscv/cpu.h |  8 ++++++++
> >   2 files changed, 54 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 8c86ebc109..95fdb6261e 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -98,6 +98,11 @@ static void set_priv_version(CPURISCVState *env, int 
> > priv_ver)
> >       env->priv_ver = priv_ver;
> >   }
> >
> > +static void set_vext_version(CPURISCVState *env, int vext_ver)
> > +{
> > +    env->vext_ver = vext_ver;
> > +}
> > +
> >   static void set_feature(CPURISCVState *env, int feature)
> >   {
> >       env->features |= (1ULL << feature);
> > @@ -113,7 +118,7 @@ static void set_resetvec(CPURISCVState *env, int resetvec)
> >   static void riscv_any_cpu_init(Object *obj)
> >   {
> >       CPURISCVState *env = &RISCV_CPU(obj)->env;
> > -    set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> > +    set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU | RVV);
> >       set_priv_version(env, PRIV_VERSION_1_11_0);
> >       set_resetvec(env, DEFAULT_RSTVEC);
> >   }
> > @@ -320,6 +325,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> > **errp)
> >       CPURISCVState *env = &cpu->env;
> >       RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
> >       int priv_version = PRIV_VERSION_1_11_0;
> > +    int vext_version = VEXT_VERSION_0_07_1;
> >       target_ulong target_misa = 0;
> >       Error *local_err = NULL;
> >
> > @@ -343,8 +349,18 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> > **errp)
> >               return;
> >           }
> >       }
> > -
> > +    if (cpu->cfg.vext_spec) {
> > +        if (!g_strcmp0(cpu->cfg.vext_spec, "v0.7.1")) {
> > +            vext_version = VEXT_VERSION_0_07_1;
> > +        } else {
> > +            error_setg(errp,
> > +                       "Unsupported vector spec version '%s'",
> > +                       cpu->cfg.vext_spec);
> > +            return;
> > +        }
> > +    }
> >       set_priv_version(env, priv_version);
> > +    set_vext_version(env, vext_version);
> >       set_resetvec(env, DEFAULT_RSTVEC);
> >
> >       if (cpu->cfg.mmu) {
> > @@ -409,6 +425,30 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> > **errp)
> >           if (cpu->cfg.ext_u) {
> >               target_misa |= RVU;
> >           }
> > +        if (cpu->cfg.ext_v) {
> > +            target_misa |= RVV;
> > +            if (!is_power_of_2(cpu->cfg.vlen)) {
> > +                error_setg(errp,
> > +                       "Vector extension VLEN must be power of 2");
> > +                return;
> > +            }
> > +            if (cpu->cfg.vlen > RV_VLEN_MAX || cpu->cfg.vlen < 128) {
> > +                error_setg(errp,
> > +                       "Vector extension implementation only supports VLEN "
> > +                       "in the range [128, %d]", RV_VLEN_MAX);
> > +                return;
> > +            }
> > +            if (!is_power_of_2(cpu->cfg.elen)) {
> > +                error_setg(errp,
> > +                       "Vector extension ELEN must be power of 2");
> > +                return;
> > +            }
> > +            if (cpu->cfg.elen > 64) {
> > +                error_setg(errp,
> > +                       "Vector extension ELEN must <= 64");
> > +                return;
> > +            }
> > +        }
> >
> >           set_misa(env, RVXLEN | target_misa);
> >       }
> > @@ -444,10 +484,14 @@ static Property riscv_cpu_properties[] = {
> >       DEFINE_PROP_BOOL("c", RISCVCPU, cfg.ext_c, true),
> >       DEFINE_PROP_BOOL("s", RISCVCPU, cfg.ext_s, true),
> >       DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
> > +    DEFINE_PROP_BOOL("v", RISCVCPU, cfg.ext_v, false),
> >       DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
> >       DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
> >       DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> >       DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
> > +    DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
> > +    DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> > +    DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
> >       DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
> >       DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
> >       DEFINE_PROP_END_OF_LIST(),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 07e63016a7..bf2b4b55af 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -64,6 +64,7 @@
> >   #define RVA RV('A')
> >   #define RVF RV('F')
> >   #define RVD RV('D')
> > +#define RVV RV('V')
> >   #define RVC RV('C')
> >   #define RVS RV('S')
> >   #define RVU RV('U')
> > @@ -82,6 +83,8 @@ enum {
> >   #define PRIV_VERSION_1_10_0 0x00011000
> >   #define PRIV_VERSION_1_11_0 0x00011100
> >
> > +#define VEXT_VERSION_0_07_1 0x00000701
> > +
> >   #define TRANSLATE_PMP_FAIL 2
> >   #define TRANSLATE_FAIL 1
> >   #define TRANSLATE_SUCCESS 0
> > @@ -118,6 +121,7 @@ struct CPURISCVState {
> >       target_ulong badaddr;
> >
> >       target_ulong priv_ver;
> > +    target_ulong vext_ver;
> >       target_ulong misa;
> >       target_ulong misa_mask;
> >
> > @@ -226,12 +230,16 @@ typedef struct RISCVCPU {
> >           bool ext_c;
> >           bool ext_s;
> >           bool ext_u;
> > +        bool ext_v;
> >           bool ext_counters;
> >           bool ext_ifencei;
> >           bool ext_icsr;
> >
> >           char *priv_spec;
> >           char *user_spec;
> > +        char *vext_spec;
> > +        uint16_t vlen;
> > +        uint16_t elen;
> >           bool mmu;
> >           bool pmp;
> >       } cfg;
> > --
> > 2.23.0