All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4]target-riscv: support vector extension part 1
@ 2020-02-10  8:12 ` LIU Zhiwei
  0 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-10  8:12 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768, LIU Zhiwei

This is the first part of v4 patchset. The changelog of v4 is only coverd
the part1.

Features:
  * support specification riscv-v-spec-0.7.1.
  * support basic vector extension.
  * support Zvlsseg.
  * support Zvamo.
  * not support Zvediv as it is changing.
  * fixed SLEN 128bit.
  * element width support 8bit, 16bit, 32bit, 64bit.

Changelog:

v4
  * adjust max vlen to 512 bits.
  * check maximum on elen(64bits).
  * check minimum on vlen(128bits).
  * check if rs1 is x0 in vsetvl/vsetvli.
  * use gen_goto_tb in vsetvli instead of exit_tb.
  * fixup fetch vlmax from rs2, not env->vext.type.
v3
  * support VLEN configure from qemu command line.
  * support ELEN configure from qemu command line.
  * support vector specification version configure from qemu command line.
  * only default on for "any" cpu, others turn on from command line.
  * use a continous memory block for vector register description.
V2
  * use float16_compare{_quiet}
  * only use GETPC() in outer most helper
  * add ctx.ext_v Property

LIU Zhiwei (4):
  target/riscv: add vector extension field in CPURISCVState
  target/riscv: configure and turn on vector extension from command line
  target/riscv: support vector extension csr
  target/riscv: add vector configure instruction

 MAINTAINERS                             |  1 +
 target/riscv/Makefile.objs              |  2 +-
 target/riscv/cpu.c                      | 48 ++++++++++++++-
 target/riscv/cpu.h                      | 82 ++++++++++++++++++++++---
 target/riscv/cpu_bits.h                 | 15 +++++
 target/riscv/csr.c                      | 72 +++++++++++++++++++++-
 target/riscv/helper.h                   |  2 +
 target/riscv/insn32.decode              |  5 ++
 target/riscv/insn_trans/trans_rvv.inc.c | 69 +++++++++++++++++++++
 target/riscv/translate.c                | 17 ++++-
 target/riscv/vector_helper.c            | 49 +++++++++++++++
 11 files changed, 346 insertions(+), 16 deletions(-)
 create mode 100644 target/riscv/insn_trans/trans_rvv.inc.c
 create mode 100644 target/riscv/vector_helper.c

-- 
2.23.0



^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v4 0/4]target-riscv: support vector extension part 1
@ 2020-02-10  8:12 ` LIU Zhiwei
  0 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-10  8:12 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv, LIU Zhiwei

This is the first part of v4 patchset. The changelog of v4 is only coverd
the part1.

Features:
  * support specification riscv-v-spec-0.7.1.
  * support basic vector extension.
  * support Zvlsseg.
  * support Zvamo.
  * not support Zvediv as it is changing.
  * fixed SLEN 128bit.
  * element width support 8bit, 16bit, 32bit, 64bit.

Changelog:

v4
  * adjust max vlen to 512 bits.
  * check maximum on elen(64bits).
  * check minimum on vlen(128bits).
  * check if rs1 is x0 in vsetvl/vsetvli.
  * use gen_goto_tb in vsetvli instead of exit_tb.
  * fixup fetch vlmax from rs2, not env->vext.type.
v3
  * support VLEN configure from qemu command line.
  * support ELEN configure from qemu command line.
  * support vector specification version configure from qemu command line.
  * only default on for "any" cpu, others turn on from command line.
  * use a continous memory block for vector register description.
V2
  * use float16_compare{_quiet}
  * only use GETPC() in outer most helper
  * add ctx.ext_v Property

LIU Zhiwei (4):
  target/riscv: add vector extension field in CPURISCVState
  target/riscv: configure and turn on vector extension from command line
  target/riscv: support vector extension csr
  target/riscv: add vector configure instruction

 MAINTAINERS                             |  1 +
 target/riscv/Makefile.objs              |  2 +-
 target/riscv/cpu.c                      | 48 ++++++++++++++-
 target/riscv/cpu.h                      | 82 ++++++++++++++++++++++---
 target/riscv/cpu_bits.h                 | 15 +++++
 target/riscv/csr.c                      | 72 +++++++++++++++++++++-
 target/riscv/helper.h                   |  2 +
 target/riscv/insn32.decode              |  5 ++
 target/riscv/insn_trans/trans_rvv.inc.c | 69 +++++++++++++++++++++
 target/riscv/translate.c                | 17 ++++-
 target/riscv/vector_helper.c            | 49 +++++++++++++++
 11 files changed, 346 insertions(+), 16 deletions(-)
 create mode 100644 target/riscv/insn_trans/trans_rvv.inc.c
 create mode 100644 target/riscv/vector_helper.c

-- 
2.23.0



^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v4 1/4] target/riscv: add vector extension field in CPURISCVState
  2020-02-10  8:12 ` LIU Zhiwei
@ 2020-02-10  8:12   ` LIU Zhiwei
  -1 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-10  8:12 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768, LIU Zhiwei

The 32 vector registers will be viewed as a continuous memory block.
It avoids the convension between element index and (regno,offset).
Thus elements can be directly accessed by offset from the first vector
base address.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index de0a8d893a..07e63016a7 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -93,9 +93,22 @@ typedef struct CPURISCVState CPURISCVState;
 
 #include "pmp.h"
 
+#define RV_VLEN_MAX 512
+
 struct CPURISCVState {
     target_ulong gpr[32];
     uint64_t fpr[32]; /* assume both F and D extensions */
+
+    /* vector coprocessor state. */
+    struct {
+         uint64_t vreg[32 * RV_VLEN_MAX / 64] QEMU_ALIGNED(16);
+         target_ulong vxrm;
+         target_ulong vxsat;
+         target_ulong vl;
+         target_ulong vstart;
+         target_ulong vtype;
+    } vext;
+
     target_ulong pc;
     target_ulong load_res;
     target_ulong load_val;
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v4 1/4] target/riscv: add vector extension field in CPURISCVState
@ 2020-02-10  8:12   ` LIU Zhiwei
  0 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-10  8:12 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv, LIU Zhiwei

The 32 vector registers will be viewed as a continuous memory block.
It avoids the convension between element index and (regno,offset).
Thus elements can be directly accessed by offset from the first vector
base address.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index de0a8d893a..07e63016a7 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -93,9 +93,22 @@ typedef struct CPURISCVState CPURISCVState;
 
 #include "pmp.h"
 
+#define RV_VLEN_MAX 512
+
 struct CPURISCVState {
     target_ulong gpr[32];
     uint64_t fpr[32]; /* assume both F and D extensions */
+
+    /* vector coprocessor state. */
+    struct {
+         uint64_t vreg[32 * RV_VLEN_MAX / 64] QEMU_ALIGNED(16);
+         target_ulong vxrm;
+         target_ulong vxsat;
+         target_ulong vl;
+         target_ulong vstart;
+         target_ulong vtype;
+    } vext;
+
     target_ulong pc;
     target_ulong load_res;
     target_ulong load_val;
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
  2020-02-10  8:12 ` LIU Zhiwei
@ 2020-02-10  8:12   ` LIU Zhiwei
  -1 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-10  8:12 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768, LIU Zhiwei

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 <zhiwei_liu@c-sky.com>
---
 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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
@ 2020-02-10  8:12   ` LIU Zhiwei
  0 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-10  8:12 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv, LIU Zhiwei

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 <zhiwei_liu@c-sky.com>
---
 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



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v4 3/4] target/riscv: support vector extension csr
  2020-02-10  8:12 ` LIU Zhiwei
@ 2020-02-10  8:12   ` LIU Zhiwei
  -1 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-10  8:12 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768, LIU Zhiwei

The v0.7.1 specification does not define vector status within mstatus.
A future revision will define the privileged portion of the vector status.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu_bits.h | 15 +++++++++
 target/riscv/csr.c      | 72 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index e99834856c..1f588ebc14 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -29,6 +29,14 @@
 #define FSR_NXA             (FPEXC_NX << FSR_AEXC_SHIFT)
 #define FSR_AEXC            (FSR_NVA | FSR_OFA | FSR_UFA | FSR_DZA | FSR_NXA)
 
+/* Vector Fixed-Point round model */
+#define FSR_VXRM_SHIFT      9
+#define FSR_VXRM            (0x3 << FSR_VXRM_SHIFT)
+
+/* Vector Fixed-Point saturation flag */
+#define FSR_VXSAT_SHIFT     8
+#define FSR_VXSAT           (0x1 << FSR_VXSAT_SHIFT)
+
 /* Control and Status Registers */
 
 /* User Trap Setup */
@@ -48,6 +56,13 @@
 #define CSR_FRM             0x002
 #define CSR_FCSR            0x003
 
+/* User Vector CSRs */
+#define CSR_VSTART          0x008
+#define CSR_VXSAT           0x009
+#define CSR_VXRM            0x00a
+#define CSR_VL              0xc20
+#define CSR_VTYPE           0xc21
+
 /* User Timers and Counters */
 #define CSR_CYCLE           0xc00
 #define CSR_TIME            0xc01
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0e34c292c5..4696c8c180 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -46,6 +46,10 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 static int fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
+    /* loose check condition for fcsr in vector extension */
+    if ((csrno == CSR_FCSR) && (env->misa & RVV)) {
+        return 0;
+    }
     if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
@@ -53,6 +57,11 @@ static int fs(CPURISCVState *env, int csrno)
     return 0;
 }
 
+static int vs(CPURISCVState *env, int csrno)
+{
+    return 0;
+}
+
 static int ctr(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
@@ -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;
 }
 
@@ -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;
 }
 
+static int read_vtype(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vext.vtype;
+    return 0;
+}
+
+static int read_vl(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vext.vl;
+    return 0;
+}
+
+static int read_vxrm(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vext.vxrm;
+    return 0;
+}
+
+static int read_vxsat(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vext.vxsat;
+    return 0;
+}
+
+static int read_vstart(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vext.vstart;
+    return 0;
+}
+
+static int write_vxrm(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vext.vxrm = val;
+    return 0;
+}
+
+static int write_vxsat(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vext.vxsat = val;
+    return 0;
+}
+
+static int write_vstart(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vext.vstart = val;
+    return 0;
+}
+
 /* User Timers and Counters */
 static int read_instret(CPURISCVState *env, int csrno, target_ulong *val)
 {
@@ -877,7 +938,12 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_FFLAGS] =              { fs,   read_fflags,      write_fflags      },
     [CSR_FRM] =                 { fs,   read_frm,         write_frm         },
     [CSR_FCSR] =                { fs,   read_fcsr,        write_fcsr        },
-
+    /* Vector CSRs */
+    [CSR_VSTART] =              { vs,   read_vstart,      write_vstart      },
+    [CSR_VXSAT] =               { vs,   read_vxsat,       write_vxsat       },
+    [CSR_VXRM] =                { vs,   read_vxrm,        write_vxrm        },
+    [CSR_VL] =                  { vs,   read_vl                             },
+    [CSR_VTYPE] =               { vs,   read_vtype                          },
     /* User Timers and Counters */
     [CSR_CYCLE] =               { ctr,  read_instret                        },
     [CSR_INSTRET] =             { ctr,  read_instret                        },
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v4 3/4] target/riscv: support vector extension csr
@ 2020-02-10  8:12   ` LIU Zhiwei
  0 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-10  8:12 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv, LIU Zhiwei

The v0.7.1 specification does not define vector status within mstatus.
A future revision will define the privileged portion of the vector status.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu_bits.h | 15 +++++++++
 target/riscv/csr.c      | 72 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index e99834856c..1f588ebc14 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -29,6 +29,14 @@
 #define FSR_NXA             (FPEXC_NX << FSR_AEXC_SHIFT)
 #define FSR_AEXC            (FSR_NVA | FSR_OFA | FSR_UFA | FSR_DZA | FSR_NXA)
 
+/* Vector Fixed-Point round model */
+#define FSR_VXRM_SHIFT      9
+#define FSR_VXRM            (0x3 << FSR_VXRM_SHIFT)
+
+/* Vector Fixed-Point saturation flag */
+#define FSR_VXSAT_SHIFT     8
+#define FSR_VXSAT           (0x1 << FSR_VXSAT_SHIFT)
+
 /* Control and Status Registers */
 
 /* User Trap Setup */
@@ -48,6 +56,13 @@
 #define CSR_FRM             0x002
 #define CSR_FCSR            0x003
 
+/* User Vector CSRs */
+#define CSR_VSTART          0x008
+#define CSR_VXSAT           0x009
+#define CSR_VXRM            0x00a
+#define CSR_VL              0xc20
+#define CSR_VTYPE           0xc21
+
 /* User Timers and Counters */
 #define CSR_CYCLE           0xc00
 #define CSR_TIME            0xc01
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0e34c292c5..4696c8c180 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -46,6 +46,10 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 static int fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
+    /* loose check condition for fcsr in vector extension */
+    if ((csrno == CSR_FCSR) && (env->misa & RVV)) {
+        return 0;
+    }
     if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
@@ -53,6 +57,11 @@ static int fs(CPURISCVState *env, int csrno)
     return 0;
 }
 
+static int vs(CPURISCVState *env, int csrno)
+{
+    return 0;
+}
+
 static int ctr(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
@@ -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;
 }
 
@@ -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;
 }
 
+static int read_vtype(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vext.vtype;
+    return 0;
+}
+
+static int read_vl(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vext.vl;
+    return 0;
+}
+
+static int read_vxrm(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vext.vxrm;
+    return 0;
+}
+
+static int read_vxsat(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vext.vxsat;
+    return 0;
+}
+
+static int read_vstart(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vext.vstart;
+    return 0;
+}
+
+static int write_vxrm(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vext.vxrm = val;
+    return 0;
+}
+
+static int write_vxsat(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vext.vxsat = val;
+    return 0;
+}
+
+static int write_vstart(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vext.vstart = val;
+    return 0;
+}
+
 /* User Timers and Counters */
 static int read_instret(CPURISCVState *env, int csrno, target_ulong *val)
 {
@@ -877,7 +938,12 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_FFLAGS] =              { fs,   read_fflags,      write_fflags      },
     [CSR_FRM] =                 { fs,   read_frm,         write_frm         },
     [CSR_FCSR] =                { fs,   read_fcsr,        write_fcsr        },
-
+    /* Vector CSRs */
+    [CSR_VSTART] =              { vs,   read_vstart,      write_vstart      },
+    [CSR_VXSAT] =               { vs,   read_vxsat,       write_vxsat       },
+    [CSR_VXRM] =                { vs,   read_vxrm,        write_vxrm        },
+    [CSR_VL] =                  { vs,   read_vl                             },
+    [CSR_VTYPE] =               { vs,   read_vtype                          },
     /* User Timers and Counters */
     [CSR_CYCLE] =               { ctr,  read_instret                        },
     [CSR_INSTRET] =             { ctr,  read_instret                        },
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v4 4/4] target/riscv: add vector configure instruction
  2020-02-10  8:12 ` LIU Zhiwei
@ 2020-02-10  8:12   ` LIU Zhiwei
  -1 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-10  8:12 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768, LIU Zhiwei

vsetvl and vsetvli are two configure instructions for vl, vtype. TB flags
should update after configure instructions. The (ill, lmul, sew ) of vtype
and the bit of (VSTART == 0 && VL == VLMAX) will be placed within tb_flags.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 MAINTAINERS                             |  1 +
 target/riscv/Makefile.objs              |  2 +-
 target/riscv/cpu.h                      | 61 +++++++++++++++++++---
 target/riscv/helper.h                   |  2 +
 target/riscv/insn32.decode              |  5 ++
 target/riscv/insn_trans/trans_rvv.inc.c | 69 +++++++++++++++++++++++++
 target/riscv/translate.c                | 17 +++++-
 target/riscv/vector_helper.c            | 49 ++++++++++++++++++
 8 files changed, 195 insertions(+), 11 deletions(-)
 create mode 100644 target/riscv/insn_trans/trans_rvv.inc.c
 create mode 100644 target/riscv/vector_helper.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e72b5e5f69..015e9239b5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -266,6 +266,7 @@ M: Palmer Dabbelt <palmer@dabbelt.com>
 M: Alistair Francis <Alistair.Francis@wdc.com>
 M: Sagar Karandikar <sagark@eecs.berkeley.edu>
 M: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
+M: LIU Zhiwei <zhiwei_liu@c-sky.com>
 L: qemu-riscv@nongnu.org
 S: Supported
 F: target/riscv/
diff --git a/target/riscv/Makefile.objs b/target/riscv/Makefile.objs
index ff651f69f6..ff38df6219 100644
--- a/target/riscv/Makefile.objs
+++ b/target/riscv/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += translate.o op_helper.o cpu_helper.o cpu.o csr.o fpu_helper.o gdbstub.o
+obj-y += translate.o op_helper.o cpu_helper.o cpu.o csr.o fpu_helper.o vector_helper.o gdbstub.o
 obj-$(CONFIG_SOFTMMU) += pmp.o
 
 ifeq ($(CONFIG_SOFTMMU),y)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index bf2b4b55af..f857845285 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -21,6 +21,7 @@
 #define RISCV_CPU_H
 
 #include "hw/core/cpu.h"
+#include "hw/registerfields.h"
 #include "exec/cpu-defs.h"
 #include "fpu/softfloat-types.h"
 
@@ -98,6 +99,10 @@ typedef struct CPURISCVState CPURISCVState;
 
 #define RV_VLEN_MAX 512
 
+FIELD(VTYPE, LMUL, 0, 2)
+FIELD(VTYPE, SEW, 2, 3)
+FIELD(VTYPE, VILL, sizeof(target_ulong) * 8 - 2, 1)
+
 struct CPURISCVState {
     target_ulong gpr[32];
     uint64_t fpr[32]; /* assume both F and D extensions */
@@ -306,16 +311,61 @@ void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
 #define TB_FLAGS_MMU_MASK   3
 #define TB_FLAGS_MSTATUS_FS MSTATUS_FS
 
+typedef CPURISCVState CPUArchState;
+typedef RISCVCPU ArchCPU;
+#include "exec/cpu-all.h"
+
+FIELD(TB_FLAGS, VL_EQ_VLMAX, 2, 1)
+FIELD(TB_FLAGS, LMUL, 3, 2)
+FIELD(TB_FLAGS, SEW, 5, 3)
+FIELD(TB_FLAGS, VILL, 8, 1)
+
+/*
+ * A simplification for VLMAX
+ * = (1 << LMUL) * VLEN / (8 * (1 << SEW))
+ * = (VLEN << LMUL) / (8 << SEW)
+ * = (VLEN << LMUL) >> (SEW + 3)
+ * = VLEN >> (SEW + 3 - LMUL)
+ */
+static inline uint32_t vext_get_vlmax(RISCVCPU *cpu, target_ulong vtype)
+{
+    uint8_t sew, lmul;
+
+    sew = FIELD_EX64(vtype, VTYPE, SEW);
+    lmul = FIELD_EX64(vtype, VTYPE, LMUL);
+    return cpu->cfg.vlen >> (sew + 3 - lmul);
+}
+
 static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
-                                        target_ulong *cs_base, uint32_t *flags)
+                                        target_ulong *cs_base, uint32_t *pflags)
 {
+    uint32_t flags = 0;
+    uint32_t vlmax;
+    uint8_t vl_eq_vlmax;
+
     *pc = env->pc;
     *cs_base = 0;
+
+    if (env->misa & RVV) {
+        vlmax = vext_get_vlmax(env_archcpu(env), env->vext.vtype);
+        vl_eq_vlmax = (env->vext.vstart == 0) && (vlmax == env->vext.vl);
+        flags = FIELD_DP32(flags, TB_FLAGS, VILL,
+                    FIELD_EX64(env->vext.vtype, VTYPE, VILL));
+        flags = FIELD_DP32(flags, TB_FLAGS, SEW,
+                    FIELD_EX64(env->vext.vtype, VTYPE, SEW));
+        flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
+                    FIELD_EX64(env->vext.vtype, VTYPE, LMUL));
+        flags = FIELD_DP32(flags, TB_FLAGS, VL_EQ_VLMAX, vl_eq_vlmax);
+    } else {
+        flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1);
+    }
+
 #ifdef CONFIG_USER_ONLY
-    *flags = TB_FLAGS_MSTATUS_FS;
+    flags |= TB_FLAGS_MSTATUS_FS;
 #else
-    *flags = cpu_mmu_index(env, 0) | (env->mstatus & MSTATUS_FS);
+    flags |= cpu_mmu_index(env, 0) | (env->mstatus & MSTATUS_FS);
 #endif
+    *pflags = flags;
 }
 
 int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
@@ -356,9 +406,4 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
 
 void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
 
-typedef CPURISCVState CPUArchState;
-typedef RISCVCPU ArchCPU;
-
-#include "exec/cpu-all.h"
-
 #endif /* RISCV_CPU_H */
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index debb22a480..3c28c7e407 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -76,3 +76,5 @@ DEF_HELPER_2(mret, tl, env, tl)
 DEF_HELPER_1(wfi, void, env)
 DEF_HELPER_1(tlb_flush, void, env)
 #endif
+/* Vector functions */
+DEF_HELPER_3(vsetvl, tl, env, tl, tl)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 77f794ed70..5dc009c3cd 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -62,6 +62,7 @@
 @r_rm    .......   ..... ..... ... ..... ....... %rs2 %rs1 %rm %rd
 @r2_rm   .......   ..... ..... ... ..... ....... %rs1 %rm %rd
 @r2      .......   ..... ..... ... ..... ....... %rs1 %rd
+@r2_zimm . zimm:11  ..... ... ..... ....... %rs1 %rd
 
 @sfence_vma ....... ..... .....   ... ..... ....... %rs2 %rs1
 @sfence_vm  ....... ..... .....   ... ..... ....... %rs1
@@ -203,3 +204,7 @@ fcvt_w_d   1100001  00000 ..... ... ..... 1010011 @r2_rm
 fcvt_wu_d  1100001  00001 ..... ... ..... 1010011 @r2_rm
 fcvt_d_w   1101001  00000 ..... ... ..... 1010011 @r2_rm
 fcvt_d_wu  1101001  00001 ..... ... ..... 1010011 @r2_rm
+
+# *** RV32V Extension ***
+vsetvli         0 ........... ..... 111 ..... 1010111  @r2_zimm
+vsetvl          1000000 ..... ..... 111 ..... 1010111  @r
diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
new file mode 100644
index 0000000000..da82c72bbf
--- /dev/null
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -0,0 +1,69 @@
+/*
+ * RISC-V translation routines for the RVV Standard Extension.
+ *
+ * Copyright (c) 2020 C-SKY Limited. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+static bool trans_vsetvl(DisasContext *ctx, arg_vsetvl * a)
+{
+    TCGv s1, s2, dst;
+    s2 = tcg_temp_new();
+    dst = tcg_temp_new();
+
+    /* Using x0 as the rs1 register specifier, encodes an infinite AVL */
+    if (a->rs1 == 0) {
+        /* As the mask is at least one bit, RV_VLEN_MAX is >= VLMAX */
+        s1 = tcg_const_tl(RV_VLEN_MAX);
+    } else {
+        s1 = tcg_temp_new();
+        gen_get_gpr(s1, a->rs1);
+    }
+    gen_get_gpr(s2, a->rs2);
+    gen_helper_vsetvl(dst, cpu_env, s1, s2);
+    gen_set_gpr(a->rd, dst);
+    tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn);
+    exit_tb(ctx);
+    ctx->base.is_jmp = DISAS_NORETURN;
+
+    tcg_temp_free(s1);
+    tcg_temp_free(s2);
+    tcg_temp_free(dst);
+    return true;
+}
+
+static bool trans_vsetvli(DisasContext *ctx, arg_vsetvli * a)
+{
+    TCGv s1, s2, dst;
+    s2 = tcg_const_tl(a->zimm);
+    dst = tcg_temp_new();
+
+    /* Using x0 as the rs1 register specifier, encodes an infinite AVL */
+    if (a->rs1 == 0) {
+        /* As the mask is at least one bit, RV_VLEN_MAX is >= VLMAX */
+        s1 = tcg_const_tl(RV_VLEN_MAX);
+    } else {
+        s1 = tcg_temp_new();
+        gen_get_gpr(s1, a->rs1);
+    }
+    gen_helper_vsetvl(dst, cpu_env, s1, s2);
+    gen_set_gpr(a->rd, dst);
+    gen_goto_tb(ctx, 0, ctx->pc_succ_insn);
+    ctx->base.is_jmp = DISAS_NORETURN;
+
+    tcg_temp_free(s1);
+    tcg_temp_free(s2);
+    tcg_temp_free(dst);
+    return true;
+}
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 14dc71156b..cc356aabd8 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -55,6 +55,12 @@ typedef struct DisasContext {
        to reset this known value.  */
     int frm;
     bool ext_ifencei;
+    /* vector extension */
+    bool vill;
+    uint8_t lmul;
+    uint8_t sew;
+    uint16_t vlen;
+    bool vl_eq_vlmax;
 } DisasContext;
 
 #ifdef TARGET_RISCV64
@@ -704,6 +710,7 @@ static bool gen_shift(DisasContext *ctx, arg_r *a,
 #include "insn_trans/trans_rva.inc.c"
 #include "insn_trans/trans_rvf.inc.c"
 #include "insn_trans/trans_rvd.inc.c"
+#include "insn_trans/trans_rvv.inc.c"
 #include "insn_trans/trans_privileged.inc.c"
 
 /* Include the auto-generated decoder for 16 bit insn */
@@ -735,14 +742,20 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     CPURISCVState *env = cs->env_ptr;
     RISCVCPU *cpu = RISCV_CPU(cs);
+    uint32_t tb_flags = ctx->base.tb->flags;
 
     ctx->pc_succ_insn = ctx->base.pc_first;
-    ctx->mem_idx = ctx->base.tb->flags & TB_FLAGS_MMU_MASK;
-    ctx->mstatus_fs = ctx->base.tb->flags & TB_FLAGS_MSTATUS_FS;
+    ctx->mem_idx = tb_flags & TB_FLAGS_MMU_MASK;
+    ctx->mstatus_fs = tb_flags & TB_FLAGS_MSTATUS_FS;
     ctx->priv_ver = env->priv_ver;
     ctx->misa = env->misa;
     ctx->frm = -1;  /* unknown rounding mode */
     ctx->ext_ifencei = cpu->cfg.ext_ifencei;
+    ctx->vlen = cpu->cfg.vlen;
+    ctx->vill = FIELD_EX32(tb_flags, TB_FLAGS, VILL);
+    ctx->sew = FIELD_EX32(tb_flags, TB_FLAGS, SEW);
+    ctx->lmul = FIELD_EX32(tb_flags, TB_FLAGS, LMUL);
+    ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
 }
 
 static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
new file mode 100644
index 0000000000..e0f2415345
--- /dev/null
+++ b/target/riscv/vector_helper.c
@@ -0,0 +1,49 @@
+/*
+ * RISC-V Vector Extension Helpers for QEMU.
+ *
+ * Copyright (c) 2020 C-SKY Limited. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "exec/exec-all.h"
+#include "exec/helper-proto.h"
+#include <math.h>
+
+target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
+     target_ulong s2)
+{
+    int vlmax, vl;
+    RISCVCPU *cpu = env_archcpu(env);
+    uint16_t sew = 1 << FIELD_EX64(s2, VTYPE, SEW);
+
+    if (sew > cpu->cfg.elen) { /* only set vill bit. */
+        env->vext.vtype = FIELD_DP64(0, VTYPE, VILL, 1);
+        env->vext.vl = 0;
+        env->vext.vstart = 0;
+        return 0;
+    }
+
+    vlmax = vext_get_vlmax(cpu, s2);
+    if (s1 <= vlmax) {
+        vl = s1;
+    } else {
+        vl = vlmax;
+    }
+    env->vext.vl = vl;
+    env->vext.vtype = s2;
+    env->vext.vstart = 0;
+    return vl;
+}
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v4 4/4] target/riscv: add vector configure instruction
@ 2020-02-10  8:12   ` LIU Zhiwei
  0 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-10  8:12 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv, LIU Zhiwei

vsetvl and vsetvli are two configure instructions for vl, vtype. TB flags
should update after configure instructions. The (ill, lmul, sew ) of vtype
and the bit of (VSTART == 0 && VL == VLMAX) will be placed within tb_flags.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 MAINTAINERS                             |  1 +
 target/riscv/Makefile.objs              |  2 +-
 target/riscv/cpu.h                      | 61 +++++++++++++++++++---
 target/riscv/helper.h                   |  2 +
 target/riscv/insn32.decode              |  5 ++
 target/riscv/insn_trans/trans_rvv.inc.c | 69 +++++++++++++++++++++++++
 target/riscv/translate.c                | 17 +++++-
 target/riscv/vector_helper.c            | 49 ++++++++++++++++++
 8 files changed, 195 insertions(+), 11 deletions(-)
 create mode 100644 target/riscv/insn_trans/trans_rvv.inc.c
 create mode 100644 target/riscv/vector_helper.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e72b5e5f69..015e9239b5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -266,6 +266,7 @@ M: Palmer Dabbelt <palmer@dabbelt.com>
 M: Alistair Francis <Alistair.Francis@wdc.com>
 M: Sagar Karandikar <sagark@eecs.berkeley.edu>
 M: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
+M: LIU Zhiwei <zhiwei_liu@c-sky.com>
 L: qemu-riscv@nongnu.org
 S: Supported
 F: target/riscv/
diff --git a/target/riscv/Makefile.objs b/target/riscv/Makefile.objs
index ff651f69f6..ff38df6219 100644
--- a/target/riscv/Makefile.objs
+++ b/target/riscv/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += translate.o op_helper.o cpu_helper.o cpu.o csr.o fpu_helper.o gdbstub.o
+obj-y += translate.o op_helper.o cpu_helper.o cpu.o csr.o fpu_helper.o vector_helper.o gdbstub.o
 obj-$(CONFIG_SOFTMMU) += pmp.o
 
 ifeq ($(CONFIG_SOFTMMU),y)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index bf2b4b55af..f857845285 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -21,6 +21,7 @@
 #define RISCV_CPU_H
 
 #include "hw/core/cpu.h"
+#include "hw/registerfields.h"
 #include "exec/cpu-defs.h"
 #include "fpu/softfloat-types.h"
 
@@ -98,6 +99,10 @@ typedef struct CPURISCVState CPURISCVState;
 
 #define RV_VLEN_MAX 512
 
+FIELD(VTYPE, LMUL, 0, 2)
+FIELD(VTYPE, SEW, 2, 3)
+FIELD(VTYPE, VILL, sizeof(target_ulong) * 8 - 2, 1)
+
 struct CPURISCVState {
     target_ulong gpr[32];
     uint64_t fpr[32]; /* assume both F and D extensions */
@@ -306,16 +311,61 @@ void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
 #define TB_FLAGS_MMU_MASK   3
 #define TB_FLAGS_MSTATUS_FS MSTATUS_FS
 
+typedef CPURISCVState CPUArchState;
+typedef RISCVCPU ArchCPU;
+#include "exec/cpu-all.h"
+
+FIELD(TB_FLAGS, VL_EQ_VLMAX, 2, 1)
+FIELD(TB_FLAGS, LMUL, 3, 2)
+FIELD(TB_FLAGS, SEW, 5, 3)
+FIELD(TB_FLAGS, VILL, 8, 1)
+
+/*
+ * A simplification for VLMAX
+ * = (1 << LMUL) * VLEN / (8 * (1 << SEW))
+ * = (VLEN << LMUL) / (8 << SEW)
+ * = (VLEN << LMUL) >> (SEW + 3)
+ * = VLEN >> (SEW + 3 - LMUL)
+ */
+static inline uint32_t vext_get_vlmax(RISCVCPU *cpu, target_ulong vtype)
+{
+    uint8_t sew, lmul;
+
+    sew = FIELD_EX64(vtype, VTYPE, SEW);
+    lmul = FIELD_EX64(vtype, VTYPE, LMUL);
+    return cpu->cfg.vlen >> (sew + 3 - lmul);
+}
+
 static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
-                                        target_ulong *cs_base, uint32_t *flags)
+                                        target_ulong *cs_base, uint32_t *pflags)
 {
+    uint32_t flags = 0;
+    uint32_t vlmax;
+    uint8_t vl_eq_vlmax;
+
     *pc = env->pc;
     *cs_base = 0;
+
+    if (env->misa & RVV) {
+        vlmax = vext_get_vlmax(env_archcpu(env), env->vext.vtype);
+        vl_eq_vlmax = (env->vext.vstart == 0) && (vlmax == env->vext.vl);
+        flags = FIELD_DP32(flags, TB_FLAGS, VILL,
+                    FIELD_EX64(env->vext.vtype, VTYPE, VILL));
+        flags = FIELD_DP32(flags, TB_FLAGS, SEW,
+                    FIELD_EX64(env->vext.vtype, VTYPE, SEW));
+        flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
+                    FIELD_EX64(env->vext.vtype, VTYPE, LMUL));
+        flags = FIELD_DP32(flags, TB_FLAGS, VL_EQ_VLMAX, vl_eq_vlmax);
+    } else {
+        flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1);
+    }
+
 #ifdef CONFIG_USER_ONLY
-    *flags = TB_FLAGS_MSTATUS_FS;
+    flags |= TB_FLAGS_MSTATUS_FS;
 #else
-    *flags = cpu_mmu_index(env, 0) | (env->mstatus & MSTATUS_FS);
+    flags |= cpu_mmu_index(env, 0) | (env->mstatus & MSTATUS_FS);
 #endif
+    *pflags = flags;
 }
 
 int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
@@ -356,9 +406,4 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
 
 void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
 
-typedef CPURISCVState CPUArchState;
-typedef RISCVCPU ArchCPU;
-
-#include "exec/cpu-all.h"
-
 #endif /* RISCV_CPU_H */
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index debb22a480..3c28c7e407 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -76,3 +76,5 @@ DEF_HELPER_2(mret, tl, env, tl)
 DEF_HELPER_1(wfi, void, env)
 DEF_HELPER_1(tlb_flush, void, env)
 #endif
+/* Vector functions */
+DEF_HELPER_3(vsetvl, tl, env, tl, tl)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 77f794ed70..5dc009c3cd 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -62,6 +62,7 @@
 @r_rm    .......   ..... ..... ... ..... ....... %rs2 %rs1 %rm %rd
 @r2_rm   .......   ..... ..... ... ..... ....... %rs1 %rm %rd
 @r2      .......   ..... ..... ... ..... ....... %rs1 %rd
+@r2_zimm . zimm:11  ..... ... ..... ....... %rs1 %rd
 
 @sfence_vma ....... ..... .....   ... ..... ....... %rs2 %rs1
 @sfence_vm  ....... ..... .....   ... ..... ....... %rs1
@@ -203,3 +204,7 @@ fcvt_w_d   1100001  00000 ..... ... ..... 1010011 @r2_rm
 fcvt_wu_d  1100001  00001 ..... ... ..... 1010011 @r2_rm
 fcvt_d_w   1101001  00000 ..... ... ..... 1010011 @r2_rm
 fcvt_d_wu  1101001  00001 ..... ... ..... 1010011 @r2_rm
+
+# *** RV32V Extension ***
+vsetvli         0 ........... ..... 111 ..... 1010111  @r2_zimm
+vsetvl          1000000 ..... ..... 111 ..... 1010111  @r
diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c
new file mode 100644
index 0000000000..da82c72bbf
--- /dev/null
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -0,0 +1,69 @@
+/*
+ * RISC-V translation routines for the RVV Standard Extension.
+ *
+ * Copyright (c) 2020 C-SKY Limited. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+static bool trans_vsetvl(DisasContext *ctx, arg_vsetvl * a)
+{
+    TCGv s1, s2, dst;
+    s2 = tcg_temp_new();
+    dst = tcg_temp_new();
+
+    /* Using x0 as the rs1 register specifier, encodes an infinite AVL */
+    if (a->rs1 == 0) {
+        /* As the mask is at least one bit, RV_VLEN_MAX is >= VLMAX */
+        s1 = tcg_const_tl(RV_VLEN_MAX);
+    } else {
+        s1 = tcg_temp_new();
+        gen_get_gpr(s1, a->rs1);
+    }
+    gen_get_gpr(s2, a->rs2);
+    gen_helper_vsetvl(dst, cpu_env, s1, s2);
+    gen_set_gpr(a->rd, dst);
+    tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn);
+    exit_tb(ctx);
+    ctx->base.is_jmp = DISAS_NORETURN;
+
+    tcg_temp_free(s1);
+    tcg_temp_free(s2);
+    tcg_temp_free(dst);
+    return true;
+}
+
+static bool trans_vsetvli(DisasContext *ctx, arg_vsetvli * a)
+{
+    TCGv s1, s2, dst;
+    s2 = tcg_const_tl(a->zimm);
+    dst = tcg_temp_new();
+
+    /* Using x0 as the rs1 register specifier, encodes an infinite AVL */
+    if (a->rs1 == 0) {
+        /* As the mask is at least one bit, RV_VLEN_MAX is >= VLMAX */
+        s1 = tcg_const_tl(RV_VLEN_MAX);
+    } else {
+        s1 = tcg_temp_new();
+        gen_get_gpr(s1, a->rs1);
+    }
+    gen_helper_vsetvl(dst, cpu_env, s1, s2);
+    gen_set_gpr(a->rd, dst);
+    gen_goto_tb(ctx, 0, ctx->pc_succ_insn);
+    ctx->base.is_jmp = DISAS_NORETURN;
+
+    tcg_temp_free(s1);
+    tcg_temp_free(s2);
+    tcg_temp_free(dst);
+    return true;
+}
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 14dc71156b..cc356aabd8 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -55,6 +55,12 @@ typedef struct DisasContext {
        to reset this known value.  */
     int frm;
     bool ext_ifencei;
+    /* vector extension */
+    bool vill;
+    uint8_t lmul;
+    uint8_t sew;
+    uint16_t vlen;
+    bool vl_eq_vlmax;
 } DisasContext;
 
 #ifdef TARGET_RISCV64
@@ -704,6 +710,7 @@ static bool gen_shift(DisasContext *ctx, arg_r *a,
 #include "insn_trans/trans_rva.inc.c"
 #include "insn_trans/trans_rvf.inc.c"
 #include "insn_trans/trans_rvd.inc.c"
+#include "insn_trans/trans_rvv.inc.c"
 #include "insn_trans/trans_privileged.inc.c"
 
 /* Include the auto-generated decoder for 16 bit insn */
@@ -735,14 +742,20 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     CPURISCVState *env = cs->env_ptr;
     RISCVCPU *cpu = RISCV_CPU(cs);
+    uint32_t tb_flags = ctx->base.tb->flags;
 
     ctx->pc_succ_insn = ctx->base.pc_first;
-    ctx->mem_idx = ctx->base.tb->flags & TB_FLAGS_MMU_MASK;
-    ctx->mstatus_fs = ctx->base.tb->flags & TB_FLAGS_MSTATUS_FS;
+    ctx->mem_idx = tb_flags & TB_FLAGS_MMU_MASK;
+    ctx->mstatus_fs = tb_flags & TB_FLAGS_MSTATUS_FS;
     ctx->priv_ver = env->priv_ver;
     ctx->misa = env->misa;
     ctx->frm = -1;  /* unknown rounding mode */
     ctx->ext_ifencei = cpu->cfg.ext_ifencei;
+    ctx->vlen = cpu->cfg.vlen;
+    ctx->vill = FIELD_EX32(tb_flags, TB_FLAGS, VILL);
+    ctx->sew = FIELD_EX32(tb_flags, TB_FLAGS, SEW);
+    ctx->lmul = FIELD_EX32(tb_flags, TB_FLAGS, LMUL);
+    ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
 }
 
 static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
new file mode 100644
index 0000000000..e0f2415345
--- /dev/null
+++ b/target/riscv/vector_helper.c
@@ -0,0 +1,49 @@
+/*
+ * RISC-V Vector Extension Helpers for QEMU.
+ *
+ * Copyright (c) 2020 C-SKY Limited. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "exec/exec-all.h"
+#include "exec/helper-proto.h"
+#include <math.h>
+
+target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
+     target_ulong s2)
+{
+    int vlmax, vl;
+    RISCVCPU *cpu = env_archcpu(env);
+    uint16_t sew = 1 << FIELD_EX64(s2, VTYPE, SEW);
+
+    if (sew > cpu->cfg.elen) { /* only set vill bit. */
+        env->vext.vtype = FIELD_DP64(0, VTYPE, VILL, 1);
+        env->vext.vl = 0;
+        env->vext.vstart = 0;
+        return 0;
+    }
+
+    vlmax = vext_get_vlmax(cpu, s2);
+    if (s1 <= vlmax) {
+        vl = s1;
+    } else {
+        vl = vlmax;
+    }
+    env->vext.vl = vl;
+    env->vext.vtype = s2;
+    env->vext.vstart = 0;
+    return vl;
+}
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 1/4] target/riscv: add vector extension field in CPURISCVState
  2020-02-10  8:12   ` LIU Zhiwei
@ 2020-02-11 15:53     ` Richard Henderson
  -1 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2020-02-11 15:53 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768

On 2/10/20 8:12 AM, LIU Zhiwei wrote:
> The 32 vector registers will be viewed as a continuous memory block.
> It avoids the convension between element index and (regno,offset).
> Thus elements can be directly accessed by offset from the first vector
> base address.
> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>  target/riscv/cpu.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

I still don't think you need to put stuff into a sub-structure.  These register
names are unique in the manual, and not subdivided there.


r~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 1/4] target/riscv: add vector extension field in CPURISCVState
@ 2020-02-11 15:53     ` Richard Henderson
  0 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2020-02-11 15:53 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv

On 2/10/20 8:12 AM, LIU Zhiwei wrote:
> The 32 vector registers will be viewed as a continuous memory block.
> It avoids the convension between element index and (regno,offset).
> Thus elements can be directly accessed by offset from the first vector
> base address.
> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>  target/riscv/cpu.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

I still don't think you need to put stuff into a sub-structure.  These register
names are unique in the manual, and not subdivided there.


r~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
  2020-02-10  8:12   ` LIU Zhiwei
@ 2020-02-11 15:56     ` Richard Henderson
  -1 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2020-02-11 15:56 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768

On 2/10/20 8:12 AM, LIU Zhiwei wrote:
> +            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;
> +            }

ELEN should use the same "only supports ELEN in the range" language as VLEN.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
@ 2020-02-11 15:56     ` Richard Henderson
  0 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2020-02-11 15:56 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv

On 2/10/20 8:12 AM, LIU Zhiwei wrote:
> +            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;
> +            }

ELEN should use the same "only supports ELEN in the range" language as VLEN.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 3/4] target/riscv: support vector extension csr
  2020-02-10  8:12   ` LIU Zhiwei
@ 2020-02-11 16:11     ` Richard Henderson
  -1 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2020-02-11 16:11 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768

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~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 3/4] target/riscv: support vector extension csr
@ 2020-02-11 16:11     ` Richard Henderson
  0 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2020-02-11 16:11 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv

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~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 4/4] target/riscv: add vector configure instruction
  2020-02-10  8:12   ` LIU Zhiwei
@ 2020-02-11 16:56     ` Richard Henderson
  -1 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2020-02-11 16:56 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768

On 2/10/20 8:12 AM, LIU Zhiwei wrote:
>  static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
> -                                        target_ulong *cs_base, uint32_t *flags)
> +                                        target_ulong *cs_base, uint32_t *pflags)
>  {
> +    uint32_t flags = 0;
> +    uint32_t vlmax;
> +    uint8_t vl_eq_vlmax;

bool.

> +
>      *pc = env->pc;
>      *cs_base = 0;
> +
> +    if (env->misa & RVV) {
> +        vlmax = vext_get_vlmax(env_archcpu(env), env->vext.vtype);
> +        vl_eq_vlmax = (env->vext.vstart == 0) && (vlmax == env->vext.vl);

You might as well move the variable declarations inside this block.

> +target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
> +     target_ulong s2)

Indentation.

> +{
> +    int vlmax, vl;
> +    RISCVCPU *cpu = env_archcpu(env);
> +    uint16_t sew = 1 << FIELD_EX64(s2, VTYPE, SEW);
> +
> +    if (sew > cpu->cfg.elen) { /* only set vill bit. */
> +        env->vext.vtype = FIELD_DP64(0, VTYPE, VILL, 1);
> +        env->vext.vl = 0;
> +        env->vext.vstart = 0;
> +        return 0;
> +    }

You're missing checks against EDIV, VILL and the RESERVED field == 0.

> +
> +    vlmax = vext_get_vlmax(cpu, s2);
> +    if (s1 <= vlmax) {
> +        vl = s1;
> +    } else {
> +        vl = vlmax;
> +    }
> +    env->vext.vl = vl;
> +    env->vext.vtype = s2;
> +    env->vext.vstart = 0;
> +    return vl;
> +}
> 


r~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 4/4] target/riscv: add vector configure instruction
@ 2020-02-11 16:56     ` Richard Henderson
  0 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2020-02-11 16:56 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv

On 2/10/20 8:12 AM, LIU Zhiwei wrote:
>  static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
> -                                        target_ulong *cs_base, uint32_t *flags)
> +                                        target_ulong *cs_base, uint32_t *pflags)
>  {
> +    uint32_t flags = 0;
> +    uint32_t vlmax;
> +    uint8_t vl_eq_vlmax;

bool.

> +
>      *pc = env->pc;
>      *cs_base = 0;
> +
> +    if (env->misa & RVV) {
> +        vlmax = vext_get_vlmax(env_archcpu(env), env->vext.vtype);
> +        vl_eq_vlmax = (env->vext.vstart == 0) && (vlmax == env->vext.vl);

You might as well move the variable declarations inside this block.

> +target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
> +     target_ulong s2)

Indentation.

> +{
> +    int vlmax, vl;
> +    RISCVCPU *cpu = env_archcpu(env);
> +    uint16_t sew = 1 << FIELD_EX64(s2, VTYPE, SEW);
> +
> +    if (sew > cpu->cfg.elen) { /* only set vill bit. */
> +        env->vext.vtype = FIELD_DP64(0, VTYPE, VILL, 1);
> +        env->vext.vl = 0;
> +        env->vext.vstart = 0;
> +        return 0;
> +    }

You're missing checks against EDIV, VILL and the RESERVED field == 0.

> +
> +    vlmax = vext_get_vlmax(cpu, s2);
> +    if (s1 <= vlmax) {
> +        vl = s1;
> +    } else {
> +        vl = vlmax;
> +    }
> +    env->vext.vl = vl;
> +    env->vext.vtype = s2;
> +    env->vext.vstart = 0;
> +    return vl;
> +}
> 


r~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 1/4] target/riscv: add vector extension field in CPURISCVState
  2020-02-11 15:53     ` Richard Henderson
@ 2020-02-12  7:17       ` LIU Zhiwei
  -1 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-12  7:17 UTC (permalink / raw)
  To: Richard Henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768



On 2020/2/11 23:53, Richard Henderson wrote:
> On 2/10/20 8:12 AM, LIU Zhiwei wrote:
>> The 32 vector registers will be viewed as a continuous memory block.
>> It avoids the convension between element index and (regno,offset).
>> Thus elements can be directly accessed by offset from the first vector
>> base address.
>>
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>> ---
>>   target/riscv/cpu.h | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> I still don't think you need to put stuff into a sub-structure.  These register
> names are unique in the manual, and not subdivided there.
OK. I will scatter these registers next patch.
>
> r~



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 1/4] target/riscv: add vector extension field in CPURISCVState
@ 2020-02-12  7:17       ` LIU Zhiwei
  0 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-12  7:17 UTC (permalink / raw)
  To: Richard Henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv



On 2020/2/11 23:53, Richard Henderson wrote:
> On 2/10/20 8:12 AM, LIU Zhiwei wrote:
>> The 32 vector registers will be viewed as a continuous memory block.
>> It avoids the convension between element index and (regno,offset).
>> Thus elements can be directly accessed by offset from the first vector
>> base address.
>>
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>> ---
>>   target/riscv/cpu.h | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> I still don't think you need to put stuff into a sub-structure.  These register
> names are unique in the manual, and not subdivided there.
OK. I will scatter these registers next patch.
>
> r~



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
  2020-02-11 15:56     ` Richard Henderson
@ 2020-02-12  7:19       ` LIU Zhiwei
  -1 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-12  7:19 UTC (permalink / raw)
  To: Richard Henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768



On 2020/2/11 23:56, Richard Henderson wrote:
> On 2/10/20 8:12 AM, LIU Zhiwei wrote:
>> +            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;
>> +            }
> ELEN should use the same "only supports ELEN in the range" language as VLEN.
OK. I will printf "only supports ELEN in the range[8, 64]".
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>
> r~



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
@ 2020-02-12  7:19       ` LIU Zhiwei
  0 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-12  7:19 UTC (permalink / raw)
  To: Richard Henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv



On 2020/2/11 23:56, Richard Henderson wrote:
> On 2/10/20 8:12 AM, LIU Zhiwei wrote:
>> +            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;
>> +            }
> ELEN should use the same "only supports ELEN in the range" language as VLEN.
OK. I will printf "only supports ELEN in the range[8, 64]".
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>
> r~



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 3/4] target/riscv: support vector extension csr
  2020-02-11 16:11     ` Richard Henderson
@ 2020-02-12  7:23       ` LIU Zhiwei
  -1 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-12  7:23 UTC (permalink / raw)
  To: Richard Henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768



On 2020/2/12 0:11, Richard Henderson wrote:
> 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().
Testing RVV is ok.

  I'm not quite understand "a -1a smode()" here. Could you give more 
details? Thanks.
> 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().
OK.
>> @@ -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.
OK.
>
>
> r~



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 3/4] target/riscv: support vector extension csr
@ 2020-02-12  7:23       ` LIU Zhiwei
  0 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-12  7:23 UTC (permalink / raw)
  To: Richard Henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv



On 2020/2/12 0:11, Richard Henderson wrote:
> 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().
Testing RVV is ok.

  I'm not quite understand "a -1a smode()" here. Could you give more 
details? Thanks.
> 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().
OK.
>> @@ -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.
OK.
>
>
> r~



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 4/4] target/riscv: add vector configure instruction
  2020-02-11 16:56     ` Richard Henderson
@ 2020-02-12  8:09       ` LIU Zhiwei
  -1 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-12  8:09 UTC (permalink / raw)
  To: Richard Henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768



On 2020/2/12 0:56, Richard Henderson wrote:
> On 2/10/20 8:12 AM, LIU Zhiwei wrote:
>>   static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>> -                                        target_ulong *cs_base, uint32_t *flags)
>> +                                        target_ulong *cs_base, uint32_t *pflags)
>>   {
>> +    uint32_t flags = 0;
>> +    uint32_t vlmax;
>> +    uint8_t vl_eq_vlmax;
> bool.
OK.

Is it clearer to use "bool" here? Or it's wrong to use "uint8_t "?
>
>> +
>>       *pc = env->pc;
>>       *cs_base = 0;
>> +
>> +    if (env->misa & RVV) {
>> +        vlmax = vext_get_vlmax(env_archcpu(env), env->vext.vtype);
>> +        vl_eq_vlmax = (env->vext.vstart == 0) && (vlmax == env->vext.vl);
> You might as well move the variable declarations inside this block.
OK.
>
>> +target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
>> +     target_ulong s2)
> Indentation.
OK.
>
>> +{
>> +    int vlmax, vl;
>> +    RISCVCPU *cpu = env_archcpu(env);
>> +    uint16_t sew = 1 << FIELD_EX64(s2, VTYPE, SEW);
>> +
>> +    if (sew > cpu->cfg.elen) { /* only set vill bit. */
>> +        env->vext.vtype = FIELD_DP64(0, VTYPE, VILL, 1);
>> +        env->vext.vl = 0;
>> +        env->vext.vstart = 0;
>> +        return 0;
>> +    }
> You're missing checks against EDIV, VILL and the RESERVED field == 0.
This implementation does not support "Zvediv" . So I did not check it. 
I'm not sure if I should check(ediv==0).

I missed check  "VILL" filed.  Fix up it next patch.

I'm not quite sure if I should set VILL if  the RESERVED field != 0.

>
>> +
>> +    vlmax = vext_get_vlmax(cpu, s2);
>> +    if (s1 <= vlmax) {
>> +        vl = s1;
>> +    } else {
>> +        vl = vlmax;
>> +    }
>> +    env->vext.vl = vl;
>> +    env->vext.vtype = s2;
>> +    env->vext.vstart = 0;
>> +    return vl;
>> +}
>>
>
> r~



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 4/4] target/riscv: add vector configure instruction
@ 2020-02-12  8:09       ` LIU Zhiwei
  0 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-12  8:09 UTC (permalink / raw)
  To: Richard Henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv



On 2020/2/12 0:56, Richard Henderson wrote:
> On 2/10/20 8:12 AM, LIU Zhiwei wrote:
>>   static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>> -                                        target_ulong *cs_base, uint32_t *flags)
>> +                                        target_ulong *cs_base, uint32_t *pflags)
>>   {
>> +    uint32_t flags = 0;
>> +    uint32_t vlmax;
>> +    uint8_t vl_eq_vlmax;
> bool.
OK.

Is it clearer to use "bool" here? Or it's wrong to use "uint8_t "?
>
>> +
>>       *pc = env->pc;
>>       *cs_base = 0;
>> +
>> +    if (env->misa & RVV) {
>> +        vlmax = vext_get_vlmax(env_archcpu(env), env->vext.vtype);
>> +        vl_eq_vlmax = (env->vext.vstart == 0) && (vlmax == env->vext.vl);
> You might as well move the variable declarations inside this block.
OK.
>
>> +target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
>> +     target_ulong s2)
> Indentation.
OK.
>
>> +{
>> +    int vlmax, vl;
>> +    RISCVCPU *cpu = env_archcpu(env);
>> +    uint16_t sew = 1 << FIELD_EX64(s2, VTYPE, SEW);
>> +
>> +    if (sew > cpu->cfg.elen) { /* only set vill bit. */
>> +        env->vext.vtype = FIELD_DP64(0, VTYPE, VILL, 1);
>> +        env->vext.vl = 0;
>> +        env->vext.vstart = 0;
>> +        return 0;
>> +    }
> You're missing checks against EDIV, VILL and the RESERVED field == 0.
This implementation does not support "Zvediv" . So I did not check it. 
I'm not sure if I should check(ediv==0).

I missed check  "VILL" filed.  Fix up it next patch.

I'm not quite sure if I should set VILL if  the RESERVED field != 0.

>
>> +
>> +    vlmax = vext_get_vlmax(cpu, s2);
>> +    if (s1 <= vlmax) {
>> +        vl = s1;
>> +    } else {
>> +        vl = vlmax;
>> +    }
>> +    env->vext.vl = vl;
>> +    env->vext.vtype = s2;
>> +    env->vext.vstart = 0;
>> +    return vl;
>> +}
>>
>
> r~



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 4/4] target/riscv: add vector configure instruction
  2020-02-12  8:09       ` LIU Zhiwei
@ 2020-02-12 19:28         ` Richard Henderson
  -1 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2020-02-12 19:28 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768

On 2/12/20 12:09 AM, LIU Zhiwei wrote:
> 
> 
> On 2020/2/12 0:56, Richard Henderson wrote:
>> On 2/10/20 8:12 AM, LIU Zhiwei wrote:
>>>   static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>>> -                                        target_ulong *cs_base, uint32_t
>>> *flags)
>>> +                                        target_ulong *cs_base, uint32_t
>>> *pflags)
>>>   {
>>> +    uint32_t flags = 0;
>>> +    uint32_t vlmax;
>>> +    uint8_t vl_eq_vlmax;
>> bool.
> OK.
> 
> Is it clearer to use "bool" here? Or it's wrong to use "uint8_t "?

It is clearer.  Using uint8_t makes me wonder what else you were going to put
in that variable, but the answer from the code below is nothing.

>>> +    if (sew > cpu->cfg.elen) { /* only set vill bit. */
>>> +        env->vext.vtype = FIELD_DP64(0, VTYPE, VILL, 1);
>>> +        env->vext.vl = 0;
>>> +        env->vext.vstart = 0;
>>> +        return 0;
>>> +    }
>> You're missing checks against EDIV, VILL and the RESERVED field == 0.
> This implementation does not support "Zvediv" . So I did not check it. I'm not
> sure if I should check(ediv==0).
> 
> I missed check  "VILL" filed.  Fix up it next patch.
> 
> I'm not quite sure if I should set VILL if  the RESERVED field != 0.


The manual says

  # If the vtype setting is not supported by the implementation,
  # then the vill bit is set in vtype, the remaining bits in
  # vtype are set to zero, and the vl register is also set
  # to zero.

So yes, you most certainly have to check ediv == 0.

By extension, I believe the entire RESERVED field should be checked.
Otherwise, we don't get the same forward compatible behaviour for the next
vector extension beyond Zvediv.


r~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 4/4] target/riscv: add vector configure instruction
@ 2020-02-12 19:28         ` Richard Henderson
  0 siblings, 0 replies; 34+ messages in thread
From: Richard Henderson @ 2020-02-12 19:28 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv

On 2/12/20 12:09 AM, LIU Zhiwei wrote:
> 
> 
> On 2020/2/12 0:56, Richard Henderson wrote:
>> On 2/10/20 8:12 AM, LIU Zhiwei wrote:
>>>   static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>>> -                                        target_ulong *cs_base, uint32_t
>>> *flags)
>>> +                                        target_ulong *cs_base, uint32_t
>>> *pflags)
>>>   {
>>> +    uint32_t flags = 0;
>>> +    uint32_t vlmax;
>>> +    uint8_t vl_eq_vlmax;
>> bool.
> OK.
> 
> Is it clearer to use "bool" here? Or it's wrong to use "uint8_t "?

It is clearer.  Using uint8_t makes me wonder what else you were going to put
in that variable, but the answer from the code below is nothing.

>>> +    if (sew > cpu->cfg.elen) { /* only set vill bit. */
>>> +        env->vext.vtype = FIELD_DP64(0, VTYPE, VILL, 1);
>>> +        env->vext.vl = 0;
>>> +        env->vext.vstart = 0;
>>> +        return 0;
>>> +    }
>> You're missing checks against EDIV, VILL and the RESERVED field == 0.
> This implementation does not support "Zvediv" . So I did not check it. I'm not
> sure if I should check(ediv==0).
> 
> I missed check  "VILL" filed.  Fix up it next patch.
> 
> I'm not quite sure if I should set VILL if  the RESERVED field != 0.


The manual says

  # If the vtype setting is not supported by the implementation,
  # then the vill bit is set in vtype, the remaining bits in
  # vtype are set to zero, and the vl register is also set
  # to zero.

So yes, you most certainly have to check ediv == 0.

By extension, I believe the entire RESERVED field should be checked.
Otherwise, we don't get the same forward compatible behaviour for the next
vector extension beyond Zvediv.


r~


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
  2020-02-10  8:12   ` LIU Zhiwei
@ 2020-02-18 22:34     ` Alistair Francis
  -1 siblings, 0 replies; 34+ messages in thread
From: Alistair Francis @ 2020-02-18 22:34 UTC (permalink / raw)
  To: LIU Zhiwei
  Cc: open list:RISC-V, Richard Henderson,
	qemu-devel@nongnu.org Developers, wxy194768, Chih-Min Chao,
	wenmeng_zhang, Palmer Dabbelt

On Mon, Feb 10, 2020 at 12:12 AM LIU Zhiwei <zhiwei_liu@c-sky.com> 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 <zhiwei_liu@c-sky.com>

This looks fine to me. Shouldn't this be the last patch though? 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.

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
>


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
@ 2020-02-18 22:34     ` Alistair Francis
  0 siblings, 0 replies; 34+ messages in thread
From: Alistair Francis @ 2020-02-18 22:34 UTC (permalink / raw)
  To: LIU Zhiwei
  Cc: Richard Henderson, Chih-Min Chao, Palmer Dabbelt, wenmeng_zhang,
	wxy194768, qemu-devel@nongnu.org Developers, open list:RISC-V

On Mon, Feb 10, 2020 at 12:12 AM LIU Zhiwei <zhiwei_liu@c-sky.com> 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 <zhiwei_liu@c-sky.com>

This looks fine to me. Shouldn't this be the last patch though? 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.

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
>


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
  2020-02-18 22:34     ` Alistair Francis
@ 2020-02-19  0:46       ` LIU Zhiwei
  -1 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-19  0:46 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Richard Henderson,
	qemu-devel@nongnu.org Developers, wxy194768, Chih-Min Chao,
	wenmeng_zhang, Palmer Dabbelt

Hi, Alistair

On 2020/2/19 6:34, Alistair Francis wrote:
> On Mon, Feb 10, 2020 at 12:12 AM LIU Zhiwei <zhiwei_liu@c-sky.com> 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 <zhiwei_liu@c-sky.com>
> 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
>>



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
@ 2020-02-19  0:46       ` LIU Zhiwei
  0 siblings, 0 replies; 34+ messages in thread
From: LIU Zhiwei @ 2020-02-19  0:46 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Richard Henderson, Chih-Min Chao, Palmer Dabbelt, wenmeng_zhang,
	wxy194768, qemu-devel@nongnu.org Developers, open list:RISC-V

Hi, Alistair

On 2020/2/19 6:34, Alistair Francis wrote:
> On Mon, Feb 10, 2020 at 12:12 AM LIU Zhiwei <zhiwei_liu@c-sky.com> 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 <zhiwei_liu@c-sky.com>
> 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
>>



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
  2020-02-19  0:46       ` LIU Zhiwei
@ 2020-02-19  1:05         ` Alistair Francis
  -1 siblings, 0 replies; 34+ messages in thread
From: Alistair Francis @ 2020-02-19  1:05 UTC (permalink / raw)
  To: LIU Zhiwei
  Cc: open list:RISC-V, Richard Henderson,
	qemu-devel@nongnu.org Developers, wxy194768, Chih-Min Chao,
	wenmeng_zhang, Palmer Dabbelt

On Tue, Feb 18, 2020 at 4:46 PM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> Hi, Alistair
>
> On 2020/2/19 6:34, Alistair Francis wrote:
> > On Mon, Feb 10, 2020 at 12:12 AM LIU Zhiwei <zhiwei_liu@c-sky.com> 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 <zhiwei_liu@c-sky.com>
> > 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.

Yes, I think that is a good idea.

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

I understand, but I don't think anyone is going to want to test the
extension half way through it being added to QEMU. This way we can
start to merge patches even without full support as users can't turn
it on.

Alistair

>
> > 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
> >>
>


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line
@ 2020-02-19  1:05         ` Alistair Francis
  0 siblings, 0 replies; 34+ messages in thread
From: Alistair Francis @ 2020-02-19  1:05 UTC (permalink / raw)
  To: LIU Zhiwei
  Cc: Richard Henderson, Chih-Min Chao, Palmer Dabbelt, wenmeng_zhang,
	wxy194768, qemu-devel@nongnu.org Developers, open list:RISC-V

On Tue, Feb 18, 2020 at 4:46 PM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> Hi, Alistair
>
> On 2020/2/19 6:34, Alistair Francis wrote:
> > On Mon, Feb 10, 2020 at 12:12 AM LIU Zhiwei <zhiwei_liu@c-sky.com> 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 <zhiwei_liu@c-sky.com>
> > 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.

Yes, I think that is a good idea.

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

I understand, but I don't think anyone is going to want to test the
extension half way through it being added to QEMU. This way we can
start to merge patches even without full support as users can't turn
it on.

Alistair

>
> > 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
> >>
>


^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2020-02-19  1:13 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-10  8:12 [PATCH v4 0/4]target-riscv: support vector extension part 1 LIU Zhiwei
2020-02-10  8:12 ` LIU Zhiwei
2020-02-10  8:12 ` [PATCH v4 1/4] target/riscv: add vector extension field in CPURISCVState LIU Zhiwei
2020-02-10  8:12   ` LIU Zhiwei
2020-02-11 15:53   ` Richard Henderson
2020-02-11 15:53     ` Richard Henderson
2020-02-12  7:17     ` LIU Zhiwei
2020-02-12  7:17       ` LIU Zhiwei
2020-02-10  8:12 ` [PATCH v4 2/4] target/riscv: configure and turn on vector extension from command line LIU Zhiwei
2020-02-10  8:12   ` LIU Zhiwei
2020-02-11 15:56   ` Richard Henderson
2020-02-11 15:56     ` Richard Henderson
2020-02-12  7:19     ` LIU Zhiwei
2020-02-12  7:19       ` LIU Zhiwei
2020-02-18 22:34   ` Alistair Francis
2020-02-18 22:34     ` Alistair Francis
2020-02-19  0:46     ` LIU Zhiwei
2020-02-19  0:46       ` LIU Zhiwei
2020-02-19  1:05       ` Alistair Francis
2020-02-19  1:05         ` Alistair Francis
2020-02-10  8:12 ` [PATCH v4 3/4] target/riscv: support vector extension csr LIU Zhiwei
2020-02-10  8:12   ` LIU Zhiwei
2020-02-11 16:11   ` Richard Henderson
2020-02-11 16:11     ` Richard Henderson
2020-02-12  7:23     ` LIU Zhiwei
2020-02-12  7:23       ` LIU Zhiwei
2020-02-10  8:12 ` [PATCH v4 4/4] target/riscv: add vector configure instruction LIU Zhiwei
2020-02-10  8:12   ` LIU Zhiwei
2020-02-11 16:56   ` Richard Henderson
2020-02-11 16:56     ` Richard Henderson
2020-02-12  8:09     ` LIU Zhiwei
2020-02-12  8:09       ` LIU Zhiwei
2020-02-12 19:28       ` Richard Henderson
2020-02-12 19:28         ` Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.