[PATCH v5 0/4] target-riscv: support vector extension part 1

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v5 0/4] target-riscv: support vector extension part 1
@ 2020-02-21  9:45 ` LIU Zhiwei
  0 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-21  9:45 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 v5 patchset. The changelog of v5 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.
  * SLEN always equals VLEN.
  * element width support 8bit, 16bit, 32bit, 64bit.

Changelog:

v5
  * vector registers as direct fields in RISCVCPUState.
  * mov the properties to last patch.
  * check RVV in vs().
  * check if rs1 is x0 in vsetvl/vsetvli.
  * check VILL, EDIV, RESERVED fileds in vsetvl.
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: implementation-defined constant parameters
  target/riscv: support vector extension csr
  target/riscv: add vector configure instruction

 MAINTAINERS                             |  1 +
 target/riscv/Makefile.objs              |  2 +-
 target/riscv/cpu.c                      |  7 +++
 target/riscv/cpu.h                      | 78 ++++++++++++++++++++++---
 target/riscv/cpu_bits.h                 | 15 +++++
 target/riscv/csr.c                      | 75 +++++++++++++++++++++++-
 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            | 53 +++++++++++++++++
 11 files changed, 312 insertions(+), 12 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] 37+ messages in thread

* [PATCH v5 0/4] target-riscv: support vector extension part 1
@ 2020-02-21  9:45 ` LIU Zhiwei
  0 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-21  9:45 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 v5 patchset. The changelog of v5 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.
  * SLEN always equals VLEN.
  * element width support 8bit, 16bit, 32bit, 64bit.

Changelog:

v5
  * vector registers as direct fields in RISCVCPUState.
  * mov the properties to last patch.
  * check RVV in vs().
  * check if rs1 is x0 in vsetvl/vsetvli.
  * check VILL, EDIV, RESERVED fileds in vsetvl.
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: implementation-defined constant parameters
  target/riscv: support vector extension csr
  target/riscv: add vector configure instruction

 MAINTAINERS                             |  1 +
 target/riscv/Makefile.objs              |  2 +-
 target/riscv/cpu.c                      |  7 +++
 target/riscv/cpu.h                      | 78 ++++++++++++++++++++++---
 target/riscv/cpu_bits.h                 | 15 +++++
 target/riscv/csr.c                      | 75 +++++++++++++++++++++++-
 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            | 53 +++++++++++++++++
 11 files changed, 312 insertions(+), 12 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] 37+ messages in thread

* [PATCH v5 1/4] target/riscv: add vector extension field in CPURISCVState
  2020-02-21  9:45 ` LIU Zhiwei
@ 2020-02-21  9:45   ` LIU Zhiwei
  -1 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-21  9:45 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 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index de0a8d893a..2e8d01c155 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')
@@ -93,9 +94,20 @@ 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. */
+    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;
+
     target_ulong pc;
     target_ulong load_res;
     target_ulong load_val;
-- 
2.23.0



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

* [PATCH v5 1/4] target/riscv: add vector extension field in CPURISCVState
@ 2020-02-21  9:45   ` LIU Zhiwei
  0 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-21  9:45 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 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index de0a8d893a..2e8d01c155 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')
@@ -93,9 +94,20 @@ 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. */
+    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;
+
     target_ulong pc;
     target_ulong load_res;
     target_ulong load_val;
-- 
2.23.0



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

* Re: [PATCH v5 1/4] target/riscv: add vector extension field in CPURISCVState
  2020-02-21  9:45   ` LIU Zhiwei
@ 2020-02-26 18:03     ` Alistair Francis
  -1 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 18:03 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 Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> 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>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index de0a8d893a..2e8d01c155 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')
> @@ -93,9 +94,20 @@ 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. */
> +    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;
> +
>      target_ulong pc;
>      target_ulong load_res;
>      target_ulong load_val;
> --
> 2.23.0
>


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

* Re: [PATCH v5 1/4] target/riscv: add vector extension field in CPURISCVState
@ 2020-02-26 18:03     ` Alistair Francis
  0 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 18:03 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 Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> 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>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index de0a8d893a..2e8d01c155 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')
> @@ -93,9 +94,20 @@ 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. */
> +    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;
> +
>      target_ulong pc;
>      target_ulong load_res;
>      target_ulong load_val;
> --
> 2.23.0
>


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

* Re: [PATCH v5 1/4] target/riscv: add vector extension field in CPURISCVState
  2020-02-21  9:45   ` LIU Zhiwei
@ 2020-02-27 20:32     ` Richard Henderson
  -1 siblings, 0 replies; 37+ messages in thread
From: Richard Henderson @ 2020-02-27 20:32 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768

On 2/21/20 1:45 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 | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

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


r~


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

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

On 2/21/20 1:45 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 | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

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


r~


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

* [PATCH v5 2/4] target/riscv: implementation-defined constant parameters
  2020-02-21  9:45 ` LIU Zhiwei
@ 2020-02-21  9:45   ` LIU Zhiwei
  -1 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-21  9:45 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768, LIU Zhiwei

vlen is the vector register length in bits.
elen is the max element size in bits.
vext_spec is the vector specification version, default value is v0.7.1.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu.c | 7 +++++++
 target/riscv/cpu.h | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 8c86ebc109..6900714432 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);
@@ -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;
 
@@ -345,6 +351,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
     }
 
     set_priv_version(env, priv_version);
+    set_vext_version(env, vext_version);
     set_resetvec(env, DEFAULT_RSTVEC);
 
     if (cpu->cfg.mmu) {
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 2e8d01c155..748bd557f9 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -83,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
@@ -117,6 +119,7 @@ struct CPURISCVState {
     target_ulong badaddr;
 
     target_ulong priv_ver;
+    target_ulong vext_ver;
     target_ulong misa;
     target_ulong misa_mask;
 
@@ -231,6 +234,8 @@ typedef struct RISCVCPU {
 
         char *priv_spec;
         char *user_spec;
+        uint16_t vlen;
+        uint16_t elen;
         bool mmu;
         bool pmp;
     } cfg;
-- 
2.23.0



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

* [PATCH v5 2/4] target/riscv: implementation-defined constant parameters
@ 2020-02-21  9:45   ` LIU Zhiwei
  0 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-21  9:45 UTC (permalink / raw)
  To: richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv, LIU Zhiwei

vlen is the vector register length in bits.
elen is the max element size in bits.
vext_spec is the vector specification version, default value is v0.7.1.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu.c | 7 +++++++
 target/riscv/cpu.h | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 8c86ebc109..6900714432 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);
@@ -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;
 
@@ -345,6 +351,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
     }
 
     set_priv_version(env, priv_version);
+    set_vext_version(env, vext_version);
     set_resetvec(env, DEFAULT_RSTVEC);
 
     if (cpu->cfg.mmu) {
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 2e8d01c155..748bd557f9 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -83,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
@@ -117,6 +119,7 @@ struct CPURISCVState {
     target_ulong badaddr;
 
     target_ulong priv_ver;
+    target_ulong vext_ver;
     target_ulong misa;
     target_ulong misa_mask;
 
@@ -231,6 +234,8 @@ typedef struct RISCVCPU {
 
         char *priv_spec;
         char *user_spec;
+        uint16_t vlen;
+        uint16_t elen;
         bool mmu;
         bool pmp;
     } cfg;
-- 
2.23.0



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

* Re: [PATCH v5 2/4] target/riscv: implementation-defined constant parameters
  2020-02-21  9:45   ` LIU Zhiwei
@ 2020-02-26 18:05     ` Alistair Francis
  -1 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 18: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 Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> vlen is the vector register length in bits.
> elen is the max element size in bits.
> vext_spec is the vector specification version, default value is v0.7.1.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c | 7 +++++++
>  target/riscv/cpu.h | 5 +++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 8c86ebc109..6900714432 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);
> @@ -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;
>
> @@ -345,6 +351,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>      }
>
>      set_priv_version(env, priv_version);
> +    set_vext_version(env, vext_version);
>      set_resetvec(env, DEFAULT_RSTVEC);
>
>      if (cpu->cfg.mmu) {
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 2e8d01c155..748bd557f9 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -83,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
> @@ -117,6 +119,7 @@ struct CPURISCVState {
>      target_ulong badaddr;
>
>      target_ulong priv_ver;
> +    target_ulong vext_ver;
>      target_ulong misa;
>      target_ulong misa_mask;
>
> @@ -231,6 +234,8 @@ typedef struct RISCVCPU {
>
>          char *priv_spec;
>          char *user_spec;
> +        uint16_t vlen;
> +        uint16_t elen;
>          bool mmu;
>          bool pmp;
>      } cfg;
> --
> 2.23.0
>


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

* Re: [PATCH v5 2/4] target/riscv: implementation-defined constant parameters
@ 2020-02-26 18:05     ` Alistair Francis
  0 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 18: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 Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> vlen is the vector register length in bits.
> elen is the max element size in bits.
> vext_spec is the vector specification version, default value is v0.7.1.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c | 7 +++++++
>  target/riscv/cpu.h | 5 +++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 8c86ebc109..6900714432 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);
> @@ -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;
>
> @@ -345,6 +351,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>      }
>
>      set_priv_version(env, priv_version);
> +    set_vext_version(env, vext_version);
>      set_resetvec(env, DEFAULT_RSTVEC);
>
>      if (cpu->cfg.mmu) {
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 2e8d01c155..748bd557f9 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -83,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
> @@ -117,6 +119,7 @@ struct CPURISCVState {
>      target_ulong badaddr;
>
>      target_ulong priv_ver;
> +    target_ulong vext_ver;
>      target_ulong misa;
>      target_ulong misa_mask;
>
> @@ -231,6 +234,8 @@ typedef struct RISCVCPU {
>
>          char *priv_spec;
>          char *user_spec;
> +        uint16_t vlen;
> +        uint16_t elen;
>          bool mmu;
>          bool pmp;
>      } cfg;
> --
> 2.23.0
>


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

* Re: [PATCH v5 2/4] target/riscv: implementation-defined constant parameters
  2020-02-21  9:45   ` LIU Zhiwei
@ 2020-02-27 20:33     ` Richard Henderson
  -1 siblings, 0 replies; 37+ messages in thread
From: Richard Henderson @ 2020-02-27 20:33 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768

On 2/21/20 1:45 AM, LIU Zhiwei wrote:
> vlen is the vector register length in bits.
> elen is the max element size in bits.
> vext_spec is the vector specification version, default value is v0.7.1.
> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>  target/riscv/cpu.c | 7 +++++++
>  target/riscv/cpu.h | 5 +++++
>  2 files changed, 12 insertions(+)

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


r~


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

* Re: [PATCH v5 2/4] target/riscv: implementation-defined constant parameters
@ 2020-02-27 20:33     ` Richard Henderson
  0 siblings, 0 replies; 37+ messages in thread
From: Richard Henderson @ 2020-02-27 20:33 UTC (permalink / raw)
  To: LIU Zhiwei, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, wxy194768, qemu-devel, qemu-riscv

On 2/21/20 1:45 AM, LIU Zhiwei wrote:
> vlen is the vector register length in bits.
> elen is the max element size in bits.
> vext_spec is the vector specification version, default value is v0.7.1.
> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>  target/riscv/cpu.c | 7 +++++++
>  target/riscv/cpu.h | 5 +++++
>  2 files changed, 12 insertions(+)

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


r~


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

* [PATCH v5 3/4] target/riscv: support vector extension csr
  2020-02-21  9:45 ` LIU Zhiwei
@ 2020-02-21  9:45   ` LIU Zhiwei
  -1 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-21  9:45 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      | 75 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 89 insertions(+), 1 deletion(-)

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..9cd2b418bf 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,14 @@ static int fs(CPURISCVState *env, int csrno)
     return 0;
 }
 
+static int vs(CPURISCVState *env, int csrno)
+{
+    if (env->misa & RVV) {
+        return 0;
+    }
+    return -1;
+}
+
 static int ctr(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
@@ -160,6 +172,10 @@ static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
 #endif
     *val = (riscv_cpu_get_fflags(env) << FSR_AEXC_SHIFT)
         | (env->frm << FSR_RD_SHIFT);
+    if (vs(env, csrno) >= 0) {
+        *val |= (env->vxrm << FSR_VXRM_SHIFT)
+                | (env->vxsat << FSR_VXSAT_SHIFT);
+    }
     return 0;
 }
 
@@ -172,10 +188,62 @@ static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
     env->mstatus |= MSTATUS_FS;
 #endif
     env->frm = (val & FSR_RD) >> FSR_RD_SHIFT;
+    if (vs(env, csrno) >= 0) {
+        env->vxrm = (val & FSR_VXRM) >> FSR_VXRM_SHIFT;
+        env->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->vtype;
+    return 0;
+}
+
+static int read_vl(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vl;
+    return 0;
+}
+
+static int read_vxrm(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vxrm;
+    return 0;
+}
+
+static int read_vxsat(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vxsat;
+    return 0;
+}
+
+static int read_vstart(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vstart;
+    return 0;
+}
+
+static int write_vxrm(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vxrm = val;
+    return 0;
+}
+
+static int write_vxsat(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vxsat = val;
+    return 0;
+}
+
+static int write_vstart(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vstart = val;
+    return 0;
+}
+
 /* User Timers and Counters */
 static int read_instret(CPURISCVState *env, int csrno, target_ulong *val)
 {
@@ -877,7 +945,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] 37+ messages in thread

* [PATCH v5 3/4] target/riscv: support vector extension csr
@ 2020-02-21  9:45   ` LIU Zhiwei
  0 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-21  9:45 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      | 75 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 89 insertions(+), 1 deletion(-)

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..9cd2b418bf 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,14 @@ static int fs(CPURISCVState *env, int csrno)
     return 0;
 }
 
+static int vs(CPURISCVState *env, int csrno)
+{
+    if (env->misa & RVV) {
+        return 0;
+    }
+    return -1;
+}
+
 static int ctr(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
@@ -160,6 +172,10 @@ static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
 #endif
     *val = (riscv_cpu_get_fflags(env) << FSR_AEXC_SHIFT)
         | (env->frm << FSR_RD_SHIFT);
+    if (vs(env, csrno) >= 0) {
+        *val |= (env->vxrm << FSR_VXRM_SHIFT)
+                | (env->vxsat << FSR_VXSAT_SHIFT);
+    }
     return 0;
 }
 
@@ -172,10 +188,62 @@ static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
     env->mstatus |= MSTATUS_FS;
 #endif
     env->frm = (val & FSR_RD) >> FSR_RD_SHIFT;
+    if (vs(env, csrno) >= 0) {
+        env->vxrm = (val & FSR_VXRM) >> FSR_VXRM_SHIFT;
+        env->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->vtype;
+    return 0;
+}
+
+static int read_vl(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vl;
+    return 0;
+}
+
+static int read_vxrm(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vxrm;
+    return 0;
+}
+
+static int read_vxsat(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vxsat;
+    return 0;
+}
+
+static int read_vstart(CPURISCVState *env, int csrno, target_ulong *val)
+{
+    *val = env->vstart;
+    return 0;
+}
+
+static int write_vxrm(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vxrm = val;
+    return 0;
+}
+
+static int write_vxsat(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vxsat = val;
+    return 0;
+}
+
+static int write_vstart(CPURISCVState *env, int csrno, target_ulong val)
+{
+    env->vstart = val;
+    return 0;
+}
+
 /* User Timers and Counters */
 static int read_instret(CPURISCVState *env, int csrno, target_ulong *val)
 {
@@ -877,7 +945,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] 37+ messages in thread

* Re: [PATCH v5 3/4] target/riscv: support vector extension csr
  2020-02-21  9:45   ` LIU Zhiwei
@ 2020-02-26 18:42     ` Alistair Francis
  -1 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 18:42 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 Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> 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      | 75 ++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 89 insertions(+), 1 deletion(-)
>
> 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)

Shouldn't these be FSCR_*?

> +
> +/* Vector Fixed-Point saturation flag */
> +#define FSR_VXSAT_SHIFT     8
> +#define FSR_VXSAT           (0x1 << FSR_VXSAT_SHIFT)

Same here, FCSR_*

> +
>  /* 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..9cd2b418bf 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,14 @@ static int fs(CPURISCVState *env, int csrno)
>      return 0;
>  }
>
> +static int vs(CPURISCVState *env, int csrno)
> +{
> +    if (env->misa & RVV) {
> +        return 0;
> +    }
> +    return -1;
> +}
> +
>  static int ctr(CPURISCVState *env, int csrno)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> @@ -160,6 +172,10 @@ static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
>  #endif
>      *val = (riscv_cpu_get_fflags(env) << FSR_AEXC_SHIFT)
>          | (env->frm << FSR_RD_SHIFT);
> +    if (vs(env, csrno) >= 0) {
> +        *val |= (env->vxrm << FSR_VXRM_SHIFT)
> +                | (env->vxsat << FSR_VXSAT_SHIFT);
> +    }
>      return 0;
>  }
>
> @@ -172,10 +188,62 @@ static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
>      env->mstatus |= MSTATUS_FS;
>  #endif
>      env->frm = (val & FSR_RD) >> FSR_RD_SHIFT;
> +    if (vs(env, csrno) >= 0) {
> +        env->vxrm = (val & FSR_VXRM) >> FSR_VXRM_SHIFT;
> +        env->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->vtype;
> +    return 0;
> +}
> +
> +static int read_vl(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> +    *val = env->vl;
> +    return 0;
> +}
> +
> +static int read_vxrm(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> +    *val = env->vxrm;
> +    return 0;
> +}
> +
> +static int read_vxsat(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> +    *val = env->vxsat;
> +    return 0;
> +}
> +
> +static int read_vstart(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> +    *val = env->vstart;
> +    return 0;
> +}
> +
> +static int write_vxrm(CPURISCVState *env, int csrno, target_ulong val)
> +{
> +    env->vxrm = val;
> +    return 0;
> +}
> +
> +static int write_vxsat(CPURISCVState *env, int csrno, target_ulong val)
> +{
> +    env->vxsat = val;
> +    return 0;
> +}
> +
> +static int write_vstart(CPURISCVState *env, int csrno, target_ulong val)
> +{
> +    env->vstart = val;
> +    return 0;
> +}

Can you keep these in read/write order? So read_vxrm() then
write_vxrm() for example.

Otherwise the patch looks good :)

Alistair

> +
>  /* User Timers and Counters */
>  static int read_instret(CPURISCVState *env, int csrno, target_ulong *val)
>  {
> @@ -877,7 +945,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	[flat|nested] 37+ messages in thread

* Re: [PATCH v5 3/4] target/riscv: support vector extension csr
@ 2020-02-26 18:42     ` Alistair Francis
  0 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 18:42 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 Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> 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      | 75 ++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 89 insertions(+), 1 deletion(-)
>
> 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)

Shouldn't these be FSCR_*?

> +
> +/* Vector Fixed-Point saturation flag */
> +#define FSR_VXSAT_SHIFT     8
> +#define FSR_VXSAT           (0x1 << FSR_VXSAT_SHIFT)

Same here, FCSR_*

> +
>  /* 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..9cd2b418bf 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,14 @@ static int fs(CPURISCVState *env, int csrno)
>      return 0;
>  }
>
> +static int vs(CPURISCVState *env, int csrno)
> +{
> +    if (env->misa & RVV) {
> +        return 0;
> +    }
> +    return -1;
> +}
> +
>  static int ctr(CPURISCVState *env, int csrno)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> @@ -160,6 +172,10 @@ static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
>  #endif
>      *val = (riscv_cpu_get_fflags(env) << FSR_AEXC_SHIFT)
>          | (env->frm << FSR_RD_SHIFT);
> +    if (vs(env, csrno) >= 0) {
> +        *val |= (env->vxrm << FSR_VXRM_SHIFT)
> +                | (env->vxsat << FSR_VXSAT_SHIFT);
> +    }
>      return 0;
>  }
>
> @@ -172,10 +188,62 @@ static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
>      env->mstatus |= MSTATUS_FS;
>  #endif
>      env->frm = (val & FSR_RD) >> FSR_RD_SHIFT;
> +    if (vs(env, csrno) >= 0) {
> +        env->vxrm = (val & FSR_VXRM) >> FSR_VXRM_SHIFT;
> +        env->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->vtype;
> +    return 0;
> +}
> +
> +static int read_vl(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> +    *val = env->vl;
> +    return 0;
> +}
> +
> +static int read_vxrm(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> +    *val = env->vxrm;
> +    return 0;
> +}
> +
> +static int read_vxsat(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> +    *val = env->vxsat;
> +    return 0;
> +}
> +
> +static int read_vstart(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> +    *val = env->vstart;
> +    return 0;
> +}
> +
> +static int write_vxrm(CPURISCVState *env, int csrno, target_ulong val)
> +{
> +    env->vxrm = val;
> +    return 0;
> +}
> +
> +static int write_vxsat(CPURISCVState *env, int csrno, target_ulong val)
> +{
> +    env->vxsat = val;
> +    return 0;
> +}
> +
> +static int write_vstart(CPURISCVState *env, int csrno, target_ulong val)
> +{
> +    env->vstart = val;
> +    return 0;
> +}

Can you keep these in read/write order? So read_vxrm() then
write_vxrm() for example.

Otherwise the patch looks good :)

Alistair

> +
>  /* User Timers and Counters */
>  static int read_instret(CPURISCVState *env, int csrno, target_ulong *val)
>  {
> @@ -877,7 +945,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	[flat|nested] 37+ messages in thread

* Re: [PATCH v5 3/4] target/riscv: support vector extension csr
  2020-02-26 18:42     ` Alistair Francis
@ 2020-02-27  0:41       ` LIU Zhiwei
  -1 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-27  0:41 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



On 2020/2/27 2:42, Alistair Francis wrote:
> On Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>> 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      | 75 ++++++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 89 insertions(+), 1 deletion(-)
>>
>> 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)
> Shouldn't these be FSCR_*?
Like other fields in fcsr, they all have been named by FSR_*, so I just 
name it like before.
>> +
>> +/* Vector Fixed-Point saturation flag */
>> +#define FSR_VXSAT_SHIFT     8
>> +#define FSR_VXSAT           (0x1 << FSR_VXSAT_SHIFT)
> Same here, FCSR_*
>
>> +
>>   /* 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..9cd2b418bf 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,14 @@ static int fs(CPURISCVState *env, int csrno)
>>       return 0;
>>   }
>>
>> +static int vs(CPURISCVState *env, int csrno)
>> +{
>> +    if (env->misa & RVV) {
>> +        return 0;
>> +    }
>> +    return -1;
>> +}
>> +
>>   static int ctr(CPURISCVState *env, int csrno)
>>   {
>>   #if !defined(CONFIG_USER_ONLY)
>> @@ -160,6 +172,10 @@ static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
>>   #endif
>>       *val = (riscv_cpu_get_fflags(env) << FSR_AEXC_SHIFT)
>>           | (env->frm << FSR_RD_SHIFT);
>> +    if (vs(env, csrno) >= 0) {
>> +        *val |= (env->vxrm << FSR_VXRM_SHIFT)
>> +                | (env->vxsat << FSR_VXSAT_SHIFT);
>> +    }
>>       return 0;
>>   }
>>
>> @@ -172,10 +188,62 @@ static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
>>       env->mstatus |= MSTATUS_FS;
>>   #endif
>>       env->frm = (val & FSR_RD) >> FSR_RD_SHIFT;
>> +    if (vs(env, csrno) >= 0) {
>> +        env->vxrm = (val & FSR_VXRM) >> FSR_VXRM_SHIFT;
>> +        env->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->vtype;
>> +    return 0;
>> +}
>> +
>> +static int read_vl(CPURISCVState *env, int csrno, target_ulong *val)
>> +{
>> +    *val = env->vl;
>> +    return 0;
>> +}
>> +
>> +static int read_vxrm(CPURISCVState *env, int csrno, target_ulong *val)
>> +{
>> +    *val = env->vxrm;
>> +    return 0;
>> +}
>> +
>> +static int read_vxsat(CPURISCVState *env, int csrno, target_ulong *val)
>> +{
>> +    *val = env->vxsat;
>> +    return 0;
>> +}
>> +
>> +static int read_vstart(CPURISCVState *env, int csrno, target_ulong *val)
>> +{
>> +    *val = env->vstart;
>> +    return 0;
>> +}
>> +
>> +static int write_vxrm(CPURISCVState *env, int csrno, target_ulong val)
>> +{
>> +    env->vxrm = val;
>> +    return 0;
>> +}
>> +
>> +static int write_vxsat(CPURISCVState *env, int csrno, target_ulong val)
>> +{
>> +    env->vxsat = val;
>> +    return 0;
>> +}
>> +
>> +static int write_vstart(CPURISCVState *env, int csrno, target_ulong val)
>> +{
>> +    env->vstart = val;
>> +    return 0;
>> +}
> Can you keep these in read/write order? So read_vxrm() then
> write_vxrm() for example.
OK.
> Otherwise the patch looks good :)
>
> Alistair
>
>> +
>>   /* User Timers and Counters */
>>   static int read_instret(CPURISCVState *env, int csrno, target_ulong *val)
>>   {
>> @@ -877,7 +945,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	[flat|nested] 37+ messages in thread

* Re: [PATCH v5 3/4] target/riscv: support vector extension csr
@ 2020-02-27  0:41       ` LIU Zhiwei
  0 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-27  0:41 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



On 2020/2/27 2:42, Alistair Francis wrote:
> On Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>> 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      | 75 ++++++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 89 insertions(+), 1 deletion(-)
>>
>> 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)
> Shouldn't these be FSCR_*?
Like other fields in fcsr, they all have been named by FSR_*, so I just 
name it like before.
>> +
>> +/* Vector Fixed-Point saturation flag */
>> +#define FSR_VXSAT_SHIFT     8
>> +#define FSR_VXSAT           (0x1 << FSR_VXSAT_SHIFT)
> Same here, FCSR_*
>
>> +
>>   /* 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..9cd2b418bf 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,14 @@ static int fs(CPURISCVState *env, int csrno)
>>       return 0;
>>   }
>>
>> +static int vs(CPURISCVState *env, int csrno)
>> +{
>> +    if (env->misa & RVV) {
>> +        return 0;
>> +    }
>> +    return -1;
>> +}
>> +
>>   static int ctr(CPURISCVState *env, int csrno)
>>   {
>>   #if !defined(CONFIG_USER_ONLY)
>> @@ -160,6 +172,10 @@ static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
>>   #endif
>>       *val = (riscv_cpu_get_fflags(env) << FSR_AEXC_SHIFT)
>>           | (env->frm << FSR_RD_SHIFT);
>> +    if (vs(env, csrno) >= 0) {
>> +        *val |= (env->vxrm << FSR_VXRM_SHIFT)
>> +                | (env->vxsat << FSR_VXSAT_SHIFT);
>> +    }
>>       return 0;
>>   }
>>
>> @@ -172,10 +188,62 @@ static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
>>       env->mstatus |= MSTATUS_FS;
>>   #endif
>>       env->frm = (val & FSR_RD) >> FSR_RD_SHIFT;
>> +    if (vs(env, csrno) >= 0) {
>> +        env->vxrm = (val & FSR_VXRM) >> FSR_VXRM_SHIFT;
>> +        env->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->vtype;
>> +    return 0;
>> +}
>> +
>> +static int read_vl(CPURISCVState *env, int csrno, target_ulong *val)
>> +{
>> +    *val = env->vl;
>> +    return 0;
>> +}
>> +
>> +static int read_vxrm(CPURISCVState *env, int csrno, target_ulong *val)
>> +{
>> +    *val = env->vxrm;
>> +    return 0;
>> +}
>> +
>> +static int read_vxsat(CPURISCVState *env, int csrno, target_ulong *val)
>> +{
>> +    *val = env->vxsat;
>> +    return 0;
>> +}
>> +
>> +static int read_vstart(CPURISCVState *env, int csrno, target_ulong *val)
>> +{
>> +    *val = env->vstart;
>> +    return 0;
>> +}
>> +
>> +static int write_vxrm(CPURISCVState *env, int csrno, target_ulong val)
>> +{
>> +    env->vxrm = val;
>> +    return 0;
>> +}
>> +
>> +static int write_vxsat(CPURISCVState *env, int csrno, target_ulong val)
>> +{
>> +    env->vxsat = val;
>> +    return 0;
>> +}
>> +
>> +static int write_vstart(CPURISCVState *env, int csrno, target_ulong val)
>> +{
>> +    env->vstart = val;
>> +    return 0;
>> +}
> Can you keep these in read/write order? So read_vxrm() then
> write_vxrm() for example.
OK.
> Otherwise the patch looks good :)
>
> Alistair
>
>> +
>>   /* User Timers and Counters */
>>   static int read_instret(CPURISCVState *env, int csrno, target_ulong *val)
>>   {
>> @@ -877,7 +945,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	[flat|nested] 37+ messages in thread

* Re: [PATCH v5 3/4] target/riscv: support vector extension csr
  2020-02-21  9:45   ` LIU Zhiwei
  (?)
  (?)
@ 2020-02-26 20:16   ` Jim Wilson
  -1 siblings, 0 replies; 37+ messages in thread
From: Jim Wilson @ 2020-02-26 20:16 UTC (permalink / raw)
  To: LIU Zhiwei, richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768

On 2/21/20 1:45 AM, LIU Zhiwei wrote:
> +/* 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)

These bits have been moved into the new VCSR register.  And the offsets 
are wrong because these bits are no longer above the FP status bits in 
the FCSR.  So this needs to be rvv 0.7.1 specific.

> +/* User Vector CSRs */
> +#define CSR_VSTART          0x008
> +#define CSR_VXSAT           0x009
> +#define CSR_VXRM            0x00a
> +#define CSR_VL              0xc20
> +#define CSR_VTYPE           0xc21

This is missing two new CSRs, VCSR and VLENB.

> +    /* loose check condition for fcsr in vector extension */
> +    if ((csrno == CSR_FCSR) && (env->misa & RVV)) {
> +        return 0;
> +    }

This is wrong for the current spec, because the vector status bits 
aren't in the FCSR anymore.  So this also needs to be rvv 0.7.1 specific.

Jim


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

* [PATCH v5 4/4] target/riscv: add vector configure instruction
  2020-02-21  9:45 ` LIU Zhiwei
@ 2020-02-21  9:45   ` LIU Zhiwei
  -1 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-21  9:45 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            | 53 +++++++++++++++++++
 8 files changed, 199 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 1740a4fddc..cd2e200db9 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 748bd557f9..f7003edb86 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,12 @@ typedef struct CPURISCVState CPURISCVState;
 
 #define RV_VLEN_MAX 512
 
+FIELD(VTYPE, LMUL, 0, 2)
+FIELD(VTYPE, SEW, 2, 3)
+FIELD(VTYPE, EDIV, 5, 2)
+FIELD(VTYPE, RESERVED, 7, sizeof(target_ulong) * 8 - 9)
+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 */
@@ -302,16 +309,59 @@ 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;
+
     *pc = env->pc;
     *cs_base = 0;
+
+    if (env->misa & RVV) {
+        uint32_t vlmax = vext_get_vlmax(env_archcpu(env), env->vtype);
+        bool vl_eq_vlmax = (env->vstart == 0) && (vlmax == env->vl);
+        flags = FIELD_DP32(flags, TB_FLAGS, VILL,
+                    FIELD_EX64(env->vtype, VTYPE, VILL));
+        flags = FIELD_DP32(flags, TB_FLAGS, SEW,
+                    FIELD_EX64(env->vtype, VTYPE, SEW));
+        flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
+                    FIELD_EX64(env->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,
@@ -352,9 +402,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..07db704656
--- /dev/null
+++ b/target/riscv/vector_helper.c
@@ -0,0 +1,53 @@
+/*
+ * 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);
+    uint8_t ediv = FIELD_EX64(s2, VTYPE, EDIV);
+    bool vill = FIELD_EX64(s2, VTYPE, VILL);
+    target_ulong reserved = FIELD_EX64(s2, VTYPE, RESERVED);
+
+    if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved != 0)) {
+        /* only set vill bit. */
+        env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
+        env->vl = 0;
+        env->vstart = 0;
+        return 0;
+    }
+
+    vlmax = vext_get_vlmax(cpu, s2);
+    if (s1 <= vlmax) {
+        vl = s1;
+    } else {
+        vl = vlmax;
+    }
+    env->vl = vl;
+    env->vtype = s2;
+    env->vstart = 0;
+    return vl;
+}
-- 
2.23.0



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

* [PATCH v5 4/4] target/riscv: add vector configure instruction
@ 2020-02-21  9:45   ` LIU Zhiwei
  0 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-21  9:45 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            | 53 +++++++++++++++++++
 8 files changed, 199 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 1740a4fddc..cd2e200db9 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 748bd557f9..f7003edb86 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,12 @@ typedef struct CPURISCVState CPURISCVState;
 
 #define RV_VLEN_MAX 512
 
+FIELD(VTYPE, LMUL, 0, 2)
+FIELD(VTYPE, SEW, 2, 3)
+FIELD(VTYPE, EDIV, 5, 2)
+FIELD(VTYPE, RESERVED, 7, sizeof(target_ulong) * 8 - 9)
+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 */
@@ -302,16 +309,59 @@ 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;
+
     *pc = env->pc;
     *cs_base = 0;
+
+    if (env->misa & RVV) {
+        uint32_t vlmax = vext_get_vlmax(env_archcpu(env), env->vtype);
+        bool vl_eq_vlmax = (env->vstart == 0) && (vlmax == env->vl);
+        flags = FIELD_DP32(flags, TB_FLAGS, VILL,
+                    FIELD_EX64(env->vtype, VTYPE, VILL));
+        flags = FIELD_DP32(flags, TB_FLAGS, SEW,
+                    FIELD_EX64(env->vtype, VTYPE, SEW));
+        flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
+                    FIELD_EX64(env->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,
@@ -352,9 +402,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..07db704656
--- /dev/null
+++ b/target/riscv/vector_helper.c
@@ -0,0 +1,53 @@
+/*
+ * 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);
+    uint8_t ediv = FIELD_EX64(s2, VTYPE, EDIV);
+    bool vill = FIELD_EX64(s2, VTYPE, VILL);
+    target_ulong reserved = FIELD_EX64(s2, VTYPE, RESERVED);
+
+    if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved != 0)) {
+        /* only set vill bit. */
+        env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
+        env->vl = 0;
+        env->vstart = 0;
+        return 0;
+    }
+
+    vlmax = vext_get_vlmax(cpu, s2);
+    if (s1 <= vlmax) {
+        vl = s1;
+    } else {
+        vl = vlmax;
+    }
+    env->vl = vl;
+    env->vtype = s2;
+    env->vstart = 0;
+    return vl;
+}
-- 
2.23.0



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

* Re: [PATCH v5 4/4] target/riscv: add vector configure instruction
  2020-02-21  9:45   ` LIU Zhiwei
@ 2020-02-26 19:20     ` Alistair Francis
  -1 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 19:20 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 Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> 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            | 53 +++++++++++++++++++
>  8 files changed, 199 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 1740a4fddc..cd2e200db9 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>

I don't think you should add yourself here. MAINTAINERS is more for
people doing active patch review.

RISC-V QEMU can really do with more maintainers though, so if you do
want to be involved you could help review patches.

>  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 748bd557f9..f7003edb86 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,12 @@ typedef struct CPURISCVState CPURISCVState;
>
>  #define RV_VLEN_MAX 512
>
> +FIELD(VTYPE, LMUL, 0, 2)

Shouldn't this be VLMUL?

> +FIELD(VTYPE, SEW, 2, 3)

VSEW?

> +FIELD(VTYPE, EDIV, 5, 2)

VEDIV?

> +FIELD(VTYPE, RESERVED, 7, sizeof(target_ulong) * 8 - 9)
> +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 */
> @@ -302,16 +309,59 @@ 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"

Why do you need this? Shouldn't the TB_FLAGS fields work without this.

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

These should probably be defined with the other TB_FLAGS (or if you
need them here you can move the others up here).


> +
> +/*
> + * 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);

Shouldn't we assert this isn't over RV_VLEN_MAX?

Alistair

> +}
> +
>  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;
> +
>      *pc = env->pc;
>      *cs_base = 0;
> +
> +    if (env->misa & RVV) {
> +        uint32_t vlmax = vext_get_vlmax(env_archcpu(env), env->vtype);
> +        bool vl_eq_vlmax = (env->vstart == 0) && (vlmax == env->vl);
> +        flags = FIELD_DP32(flags, TB_FLAGS, VILL,
> +                    FIELD_EX64(env->vtype, VTYPE, VILL));
> +        flags = FIELD_DP32(flags, TB_FLAGS, SEW,
> +                    FIELD_EX64(env->vtype, VTYPE, SEW));
> +        flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
> +                    FIELD_EX64(env->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,
> @@ -352,9 +402,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..07db704656
> --- /dev/null
> +++ b/target/riscv/vector_helper.c
> @@ -0,0 +1,53 @@
> +/*
> + * 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);
> +    uint8_t ediv = FIELD_EX64(s2, VTYPE, EDIV);
> +    bool vill = FIELD_EX64(s2, VTYPE, VILL);
> +    target_ulong reserved = FIELD_EX64(s2, VTYPE, RESERVED);
> +
> +    if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved != 0)) {
> +        /* only set vill bit. */
> +        env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
> +        env->vl = 0;
> +        env->vstart = 0;
> +        return 0;
> +    }
> +
> +    vlmax = vext_get_vlmax(cpu, s2);
> +    if (s1 <= vlmax) {
> +        vl = s1;
> +    } else {
> +        vl = vlmax;
> +    }
> +    env->vl = vl;
> +    env->vtype = s2;
> +    env->vstart = 0;
> +    return vl;
> +}
> --
> 2.23.0
>


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

* Re: [PATCH v5 4/4] target/riscv: add vector configure instruction
@ 2020-02-26 19:20     ` Alistair Francis
  0 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 19:20 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 Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> 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            | 53 +++++++++++++++++++
>  8 files changed, 199 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 1740a4fddc..cd2e200db9 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>

I don't think you should add yourself here. MAINTAINERS is more for
people doing active patch review.

RISC-V QEMU can really do with more maintainers though, so if you do
want to be involved you could help review patches.

>  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 748bd557f9..f7003edb86 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,12 @@ typedef struct CPURISCVState CPURISCVState;
>
>  #define RV_VLEN_MAX 512
>
> +FIELD(VTYPE, LMUL, 0, 2)

Shouldn't this be VLMUL?

> +FIELD(VTYPE, SEW, 2, 3)

VSEW?

> +FIELD(VTYPE, EDIV, 5, 2)

VEDIV?

> +FIELD(VTYPE, RESERVED, 7, sizeof(target_ulong) * 8 - 9)
> +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 */
> @@ -302,16 +309,59 @@ 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"

Why do you need this? Shouldn't the TB_FLAGS fields work without this.

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

These should probably be defined with the other TB_FLAGS (or if you
need them here you can move the others up here).


> +
> +/*
> + * 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);

Shouldn't we assert this isn't over RV_VLEN_MAX?

Alistair

> +}
> +
>  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;
> +
>      *pc = env->pc;
>      *cs_base = 0;
> +
> +    if (env->misa & RVV) {
> +        uint32_t vlmax = vext_get_vlmax(env_archcpu(env), env->vtype);
> +        bool vl_eq_vlmax = (env->vstart == 0) && (vlmax == env->vl);
> +        flags = FIELD_DP32(flags, TB_FLAGS, VILL,
> +                    FIELD_EX64(env->vtype, VTYPE, VILL));
> +        flags = FIELD_DP32(flags, TB_FLAGS, SEW,
> +                    FIELD_EX64(env->vtype, VTYPE, SEW));
> +        flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
> +                    FIELD_EX64(env->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,
> @@ -352,9 +402,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..07db704656
> --- /dev/null
> +++ b/target/riscv/vector_helper.c
> @@ -0,0 +1,53 @@
> +/*
> + * 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);
> +    uint8_t ediv = FIELD_EX64(s2, VTYPE, EDIV);
> +    bool vill = FIELD_EX64(s2, VTYPE, VILL);
> +    target_ulong reserved = FIELD_EX64(s2, VTYPE, RESERVED);
> +
> +    if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved != 0)) {
> +        /* only set vill bit. */
> +        env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
> +        env->vl = 0;
> +        env->vstart = 0;
> +        return 0;
> +    }
> +
> +    vlmax = vext_get_vlmax(cpu, s2);
> +    if (s1 <= vlmax) {
> +        vl = s1;
> +    } else {
> +        vl = vlmax;
> +    }
> +    env->vl = vl;
> +    env->vtype = s2;
> +    env->vstart = 0;
> +    return vl;
> +}
> --
> 2.23.0
>


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

* Re: [PATCH v5 4/4] target/riscv: add vector configure instruction
  2020-02-26 19:20     ` Alistair Francis
@ 2020-02-27  1:41       ` LIU Zhiwei
  -1 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-27  1:41 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



On 2020/2/27 3:20, Alistair Francis wrote:
>   On Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>> 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            | 53 +++++++++++++++++++
>>   8 files changed, 199 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 1740a4fddc..cd2e200db9 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>
> I don't think you should add yourself here. MAINTAINERS is more for
> people doing active patch review.
OK.
> RISC-V QEMU can really do with more maintainers though, so if you do
> want to be involved you could help review patches.
Actually my main job is to maintain and develop QEMU code，so I'd like to 
review target/riscv code,
however vector upstream takes a lot time .
>>   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 748bd557f9..f7003edb86 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,12 @@ typedef struct CPURISCVState CPURISCVState;
>>
>>   #define RV_VLEN_MAX 512
>>
>> +FIELD(VTYPE, LMUL, 0, 2)
> Shouldn't this be VLMUL?
OK. The same with VSEW and VEDIV.
>
>> +FIELD(VTYPE, SEW, 2, 3)
> VSEW?
>
>> +FIELD(VTYPE, EDIV, 5, 2)
> VEDIV?
>
>> +FIELD(VTYPE, RESERVED, 7, sizeof(target_ulong) * 8 - 9)
>> +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 */
>> @@ -302,16 +309,59 @@ 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"
> Why do you need this? Shouldn't the TB_FLAGS fields work without this.
Because env_archcpu in cpu_get_tb_cpu_state will use it.
>> +
>> +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)
> These should probably be defined with the other TB_FLAGS (or if you
> need them here you can move the others up here).
I'd like to put other TB_FLAGS in other separate patch.
>
>> +
>> +/*
>> + * 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);
> Shouldn't we assert this isn't over RV_VLEN_MAX?
I don't think so.  VLEN is vector register length in bits. It is checked
against RV_VLEN_MAX in cpu realize function. If it is over RV_VLEN_MAX,
it will exits before translate any tb.

Zhiwei

>
> Alistair
>
>> +}
>> +
>>   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;
>> +
>>       *pc = env->pc;
>>       *cs_base = 0;
>> +
>> +    if (env->misa & RVV) {
>> +        uint32_t vlmax = vext_get_vlmax(env_archcpu(env), env->vtype);
>> +        bool vl_eq_vlmax = (env->vstart == 0) && (vlmax == env->vl);
>> +        flags = FIELD_DP32(flags, TB_FLAGS, VILL,
>> +                    FIELD_EX64(env->vtype, VTYPE, VILL));
>> +        flags = FIELD_DP32(flags, TB_FLAGS, SEW,
>> +                    FIELD_EX64(env->vtype, VTYPE, SEW));
>> +        flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
>> +                    FIELD_EX64(env->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,
>> @@ -352,9 +402,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..07db704656
>> --- /dev/null
>> +++ b/target/riscv/vector_helper.c
>> @@ -0,0 +1,53 @@
>> +/*
>> + * 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);
>> +    uint8_t ediv = FIELD_EX64(s2, VTYPE, EDIV);
>> +    bool vill = FIELD_EX64(s2, VTYPE, VILL);
>> +    target_ulong reserved = FIELD_EX64(s2, VTYPE, RESERVED);
>> +
>> +    if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved != 0)) {
>> +        /* only set vill bit. */
>> +        env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
>> +        env->vl = 0;
>> +        env->vstart = 0;
>> +        return 0;
>> +    }
>> +
>> +    vlmax = vext_get_vlmax(cpu, s2);
>> +    if (s1 <= vlmax) {
>> +        vl = s1;
>> +    } else {
>> +        vl = vlmax;
>> +    }
>> +    env->vl = vl;
>> +    env->vtype = s2;
>> +    env->vstart = 0;
>> +    return vl;
>> +}
>> --
>> 2.23.0
>>



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

* Re: [PATCH v5 4/4] target/riscv: add vector configure instruction
@ 2020-02-27  1:41       ` LIU Zhiwei
  0 siblings, 0 replies; 37+ messages in thread
From: LIU Zhiwei @ 2020-02-27  1:41 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



On 2020/2/27 3:20, Alistair Francis wrote:
>   On Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>> 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            | 53 +++++++++++++++++++
>>   8 files changed, 199 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 1740a4fddc..cd2e200db9 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>
> I don't think you should add yourself here. MAINTAINERS is more for
> people doing active patch review.
OK.
> RISC-V QEMU can really do with more maintainers though, so if you do
> want to be involved you could help review patches.
Actually my main job is to maintain and develop QEMU code，so I'd like to 
review target/riscv code,
however vector upstream takes a lot time .
>>   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 748bd557f9..f7003edb86 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,12 @@ typedef struct CPURISCVState CPURISCVState;
>>
>>   #define RV_VLEN_MAX 512
>>
>> +FIELD(VTYPE, LMUL, 0, 2)
> Shouldn't this be VLMUL?
OK. The same with VSEW and VEDIV.
>
>> +FIELD(VTYPE, SEW, 2, 3)
> VSEW?
>
>> +FIELD(VTYPE, EDIV, 5, 2)
> VEDIV?
>
>> +FIELD(VTYPE, RESERVED, 7, sizeof(target_ulong) * 8 - 9)
>> +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 */
>> @@ -302,16 +309,59 @@ 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"
> Why do you need this? Shouldn't the TB_FLAGS fields work without this.
Because env_archcpu in cpu_get_tb_cpu_state will use it.
>> +
>> +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)
> These should probably be defined with the other TB_FLAGS (or if you
> need them here you can move the others up here).
I'd like to put other TB_FLAGS in other separate patch.
>
>> +
>> +/*
>> + * 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);
> Shouldn't we assert this isn't over RV_VLEN_MAX?
I don't think so.  VLEN is vector register length in bits. It is checked
against RV_VLEN_MAX in cpu realize function. If it is over RV_VLEN_MAX,
it will exits before translate any tb.

Zhiwei

>
> Alistair
>
>> +}
>> +
>>   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;
>> +
>>       *pc = env->pc;
>>       *cs_base = 0;
>> +
>> +    if (env->misa & RVV) {
>> +        uint32_t vlmax = vext_get_vlmax(env_archcpu(env), env->vtype);
>> +        bool vl_eq_vlmax = (env->vstart == 0) && (vlmax == env->vl);
>> +        flags = FIELD_DP32(flags, TB_FLAGS, VILL,
>> +                    FIELD_EX64(env->vtype, VTYPE, VILL));
>> +        flags = FIELD_DP32(flags, TB_FLAGS, SEW,
>> +                    FIELD_EX64(env->vtype, VTYPE, SEW));
>> +        flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
>> +                    FIELD_EX64(env->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,
>> @@ -352,9 +402,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..07db704656
>> --- /dev/null
>> +++ b/target/riscv/vector_helper.c
>> @@ -0,0 +1,53 @@
>> +/*
>> + * 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);
>> +    uint8_t ediv = FIELD_EX64(s2, VTYPE, EDIV);
>> +    bool vill = FIELD_EX64(s2, VTYPE, VILL);
>> +    target_ulong reserved = FIELD_EX64(s2, VTYPE, RESERVED);
>> +
>> +    if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved != 0)) {
>> +        /* only set vill bit. */
>> +        env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
>> +        env->vl = 0;
>> +        env->vstart = 0;
>> +        return 0;
>> +    }
>> +
>> +    vlmax = vext_get_vlmax(cpu, s2);
>> +    if (s1 <= vlmax) {
>> +        vl = s1;
>> +    } else {
>> +        vl = vlmax;
>> +    }
>> +    env->vl = vl;
>> +    env->vtype = s2;
>> +    env->vstart = 0;
>> +    return vl;
>> +}
>> --
>> 2.23.0
>>



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

* Re: [PATCH v5 4/4] target/riscv: add vector configure instruction
  2020-02-27  1:41       ` LIU Zhiwei
@ 2020-02-27 21:48         ` Alistair Francis
  -1 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-27 21:48 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 Wed, Feb 26, 2020 at 5:41 PM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
>
>
> On 2020/2/27 3:20, Alistair Francis wrote:
> >   On Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
> >> 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            | 53 +++++++++++++++++++
> >>   8 files changed, 199 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 1740a4fddc..cd2e200db9 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>
> > I don't think you should add yourself here. MAINTAINERS is more for
> > people doing active patch review.
> OK.
> > RISC-V QEMU can really do with more maintainers though, so if you do
> > want to be involved you could help review patches.
> Actually my main job is to maintain and develop QEMU code，so I'd like to
> review target/riscv code,
> however vector upstream takes a lot time .

Great! I know reviewing code can be touch and time consuming but it
really helps the project. Just as upstreaming can be time consuming
but it's worth it.

Just try to help review what you can, every little bit helps a lot :)

Anyone can review code (you don't have to be a maintainer) so it's a
good place to start. Once you are activley reviewing patches we can
add you as a RISC-V maintainer.

> >>   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 748bd557f9..f7003edb86 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,12 @@ typedef struct CPURISCVState CPURISCVState;
> >>
> >>   #define RV_VLEN_MAX 512
> >>
> >> +FIELD(VTYPE, LMUL, 0, 2)
> > Shouldn't this be VLMUL?
> OK. The same with VSEW and VEDIV.
> >
> >> +FIELD(VTYPE, SEW, 2, 3)
> > VSEW?
> >
> >> +FIELD(VTYPE, EDIV, 5, 2)
> > VEDIV?
> >
> >> +FIELD(VTYPE, RESERVED, 7, sizeof(target_ulong) * 8 - 9)
> >> +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 */
> >> @@ -302,16 +309,59 @@ 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"
> > Why do you need this? Shouldn't the TB_FLAGS fields work without this.
> Because env_archcpu in cpu_get_tb_cpu_state will use it.

Ah fair enough.

> >> +
> >> +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)
> > These should probably be defined with the other TB_FLAGS (or if you
> > need them here you can move the others up here).
> I'd like to put other TB_FLAGS in other separate patch.
> >
> >> +
> >> +/*
> >> + * 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);
> > Shouldn't we assert this isn't over RV_VLEN_MAX?
> I don't think so.  VLEN is vector register length in bits. It is checked
> against RV_VLEN_MAX in cpu realize function. If it is over RV_VLEN_MAX,
> it will exits before translate any tb.

Ah ok. I see. This looks good then.

Alistair

>
> Zhiwei
>
> >
> > Alistair
> >
> >> +}
> >> +
> >>   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;
> >> +
> >>       *pc = env->pc;
> >>       *cs_base = 0;
> >> +
> >> +    if (env->misa & RVV) {
> >> +        uint32_t vlmax = vext_get_vlmax(env_archcpu(env), env->vtype);
> >> +        bool vl_eq_vlmax = (env->vstart == 0) && (vlmax == env->vl);
> >> +        flags = FIELD_DP32(flags, TB_FLAGS, VILL,
> >> +                    FIELD_EX64(env->vtype, VTYPE, VILL));
> >> +        flags = FIELD_DP32(flags, TB_FLAGS, SEW,
> >> +                    FIELD_EX64(env->vtype, VTYPE, SEW));
> >> +        flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
> >> +                    FIELD_EX64(env->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,
> >> @@ -352,9 +402,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..07db704656
> >> --- /dev/null
> >> +++ b/target/riscv/vector_helper.c
> >> @@ -0,0 +1,53 @@
> >> +/*
> >> + * 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);
> >> +    uint8_t ediv = FIELD_EX64(s2, VTYPE, EDIV);
> >> +    bool vill = FIELD_EX64(s2, VTYPE, VILL);
> >> +    target_ulong reserved = FIELD_EX64(s2, VTYPE, RESERVED);
> >> +
> >> +    if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved != 0)) {
> >> +        /* only set vill bit. */
> >> +        env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
> >> +        env->vl = 0;
> >> +        env->vstart = 0;
> >> +        return 0;
> >> +    }
> >> +
> >> +    vlmax = vext_get_vlmax(cpu, s2);
> >> +    if (s1 <= vlmax) {
> >> +        vl = s1;
> >> +    } else {
> >> +        vl = vlmax;
> >> +    }
> >> +    env->vl = vl;
> >> +    env->vtype = s2;
> >> +    env->vstart = 0;
> >> +    return vl;
> >> +}
> >> --
> >> 2.23.0
> >>
>


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

* Re: [PATCH v5 4/4] target/riscv: add vector configure instruction
@ 2020-02-27 21:48         ` Alistair Francis
  0 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-27 21:48 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 Wed, Feb 26, 2020 at 5:41 PM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
>
>
> On 2020/2/27 3:20, Alistair Francis wrote:
> >   On Fri, Feb 21, 2020 at 1:45 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
> >> 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            | 53 +++++++++++++++++++
> >>   8 files changed, 199 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 1740a4fddc..cd2e200db9 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>
> > I don't think you should add yourself here. MAINTAINERS is more for
> > people doing active patch review.
> OK.
> > RISC-V QEMU can really do with more maintainers though, so if you do
> > want to be involved you could help review patches.
> Actually my main job is to maintain and develop QEMU code，so I'd like to
> review target/riscv code,
> however vector upstream takes a lot time .

Great! I know reviewing code can be touch and time consuming but it
really helps the project. Just as upstreaming can be time consuming
but it's worth it.

Just try to help review what you can, every little bit helps a lot :)

Anyone can review code (you don't have to be a maintainer) so it's a
good place to start. Once you are activley reviewing patches we can
add you as a RISC-V maintainer.

> >>   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 748bd557f9..f7003edb86 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,12 @@ typedef struct CPURISCVState CPURISCVState;
> >>
> >>   #define RV_VLEN_MAX 512
> >>
> >> +FIELD(VTYPE, LMUL, 0, 2)
> > Shouldn't this be VLMUL?
> OK. The same with VSEW and VEDIV.
> >
> >> +FIELD(VTYPE, SEW, 2, 3)
> > VSEW?
> >
> >> +FIELD(VTYPE, EDIV, 5, 2)
> > VEDIV?
> >
> >> +FIELD(VTYPE, RESERVED, 7, sizeof(target_ulong) * 8 - 9)
> >> +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 */
> >> @@ -302,16 +309,59 @@ 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"
> > Why do you need this? Shouldn't the TB_FLAGS fields work without this.
> Because env_archcpu in cpu_get_tb_cpu_state will use it.

Ah fair enough.

> >> +
> >> +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)
> > These should probably be defined with the other TB_FLAGS (or if you
> > need them here you can move the others up here).
> I'd like to put other TB_FLAGS in other separate patch.
> >
> >> +
> >> +/*
> >> + * 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);
> > Shouldn't we assert this isn't over RV_VLEN_MAX?
> I don't think so.  VLEN is vector register length in bits. It is checked
> against RV_VLEN_MAX in cpu realize function. If it is over RV_VLEN_MAX,
> it will exits before translate any tb.

Ah ok. I see. This looks good then.

Alistair

>
> Zhiwei
>
> >
> > Alistair
> >
> >> +}
> >> +
> >>   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;
> >> +
> >>       *pc = env->pc;
> >>       *cs_base = 0;
> >> +
> >> +    if (env->misa & RVV) {
> >> +        uint32_t vlmax = vext_get_vlmax(env_archcpu(env), env->vtype);
> >> +        bool vl_eq_vlmax = (env->vstart == 0) && (vlmax == env->vl);
> >> +        flags = FIELD_DP32(flags, TB_FLAGS, VILL,
> >> +                    FIELD_EX64(env->vtype, VTYPE, VILL));
> >> +        flags = FIELD_DP32(flags, TB_FLAGS, SEW,
> >> +                    FIELD_EX64(env->vtype, VTYPE, SEW));
> >> +        flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
> >> +                    FIELD_EX64(env->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,
> >> @@ -352,9 +402,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..07db704656
> >> --- /dev/null
> >> +++ b/target/riscv/vector_helper.c
> >> @@ -0,0 +1,53 @@
> >> +/*
> >> + * 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);
> >> +    uint8_t ediv = FIELD_EX64(s2, VTYPE, EDIV);
> >> +    bool vill = FIELD_EX64(s2, VTYPE, VILL);
> >> +    target_ulong reserved = FIELD_EX64(s2, VTYPE, RESERVED);
> >> +
> >> +    if ((sew > cpu->cfg.elen) || vill || (ediv != 0) || (reserved != 0)) {
> >> +        /* only set vill bit. */
> >> +        env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
> >> +        env->vl = 0;
> >> +        env->vstart = 0;
> >> +        return 0;
> >> +    }
> >> +
> >> +    vlmax = vext_get_vlmax(cpu, s2);
> >> +    if (s1 <= vlmax) {
> >> +        vl = s1;
> >> +    } else {
> >> +        vl = vlmax;
> >> +    }
> >> +    env->vl = vl;
> >> +    env->vtype = s2;
> >> +    env->vstart = 0;
> >> +    return vl;
> >> +}
> >> --
> >> 2.23.0
> >>
>


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

* Re: [PATCH v5 4/4] target/riscv: add vector configure instruction
  2020-02-21  9:45   ` LIU Zhiwei
  (?)
  (?)
@ 2020-02-26 20:20   ` Jim Wilson
  -1 siblings, 0 replies; 37+ messages in thread
From: Jim Wilson @ 2020-02-26 20:20 UTC (permalink / raw)
  To: LIU Zhiwei, richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768

On 2/21/20 1:45 AM, LIU Zhiwei wrote:
> +    /* 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);

This is wrong for the current draft of the vector spec.  x0 now means 
don't change VL.  So this needs to be version specific.

Jim


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

* Re: [PATCH v5 0/4] target-riscv: support vector extension part 1
  2020-02-21  9:45 ` LIU Zhiwei
                   ` (4 preceding siblings ...)
  (?)
@ 2020-02-26 20:09 ` Jim Wilson
  2020-02-26 22:28     ` Alistair Francis
  -1 siblings, 1 reply; 37+ messages in thread
From: Jim Wilson @ 2020-02-26 20:09 UTC (permalink / raw)
  To: LIU Zhiwei, richard.henderson, alistair23, chihmin.chao, palmer
  Cc: wenmeng_zhang, qemu-riscv, qemu-devel, wxy194768

On 2/21/20 1:45 AM, LIU Zhiwei wrote:
> This is the first part of v5 patchset. The changelog of v5 is only coverd
> the part1.
> 
> Features:
>    * support specification riscv-v-spec-0.7.1.

I'm still concerned about versioning issues.  This implements an 
unofficial draft of the proposed RISC-V vector extension.  This draft is 
not compatible with the current draft, and will be even less compatible 
with the final official version of the vector spec.

The patch adds a version which is good, but there is only one check when 
qemu starts.  Probably something like 25% of these patches will be wrong 
for the official vector extension.  How are we going to handle this when 
someone submits patches for the official support?  It would be better if 
everything in these patches were conditional on the version number.  It 
might also be better if we stopped calling this the 'v' extension and 
maybe used another name like Xrvv071 to make it clear that it is an 
unofficial draft of the proposed vector spec.  Or maybe be we can use 
v0p7 but that isn't an officially supported extension name.

If this rvv 0.7.1 implementation is considered a temporary solution, 
maybe we can just remove all of this work when the official rvv spec if 
available?  But presumably it is better if we can have both this 
implementation and the official one, which means everything needs to be 
conditional or tied to an Xsomething extension name instead of the V 
extension name.

Jim

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

* Re: [PATCH v5 0/4] target-riscv: support vector extension part 1
  2020-02-26 20:09 ` [PATCH v5 0/4] target-riscv: support vector extension part 1 Jim Wilson
@ 2020-02-26 22:28     ` Alistair Francis
  0 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 22:28 UTC (permalink / raw)
  To: Jim Wilson
  Cc: open list:RISC-V, Richard Henderson,
	qemu-devel@nongnu.org Developers, wxy194768, Chih-Min Chao,
	wenmeng_zhang, Palmer Dabbelt, LIU Zhiwei

On Wed, Feb 26, 2020 at 12:09 PM Jim Wilson <jimw@sifive.com> wrote:
>
> On 2/21/20 1:45 AM, LIU Zhiwei wrote:
> > This is the first part of v5 patchset. The changelog of v5 is only coverd
> > the part1.
> >
> > Features:
> >    * support specification riscv-v-spec-0.7.1.
>
> I'm still concerned about versioning issues.  This implements an
> unofficial draft of the proposed RISC-V vector extension.  This draft is
> not compatible with the current draft, and will be even less compatible
> with the final official version of the vector spec.

I wouldn't say this is an unofficial draft. v0.7.1 is an official and
tagged draft spec.

It is a draft spec and there is a new version (v0.8.0) and eventually
there will be a full (not draft release).

>
> The patch adds a version which is good, but there is only one check when
> qemu starts.  Probably something like 25% of these patches will be wrong
> for the official vector extension.  How are we going to handle this when
> someone submits patches for the official support?  It would be better if

The current plan (for all draft extensions) is to support just the
latest draft in QEMU. That is QEMU will have experimental support for
draft extension x at v0.1. When draft extension x is updated to v0.2
we will update the QEMU implementation (when we can) and only support
the v0.2.

We will continue updating and supporting the latest until the final
spec release, in which case we will then maintain the feature
following QEMU's deprecation policy.

While the spec is in a draft state the feature will be considered
experimental (hence the subject to change).

This way we can support and enable users to test on draft extensions,
but not spend all our time supporting draft extensions.

> everything in these patches were conditional on the version number.  It
> might also be better if we stopped calling this the 'v' extension and
> maybe used another name like Xrvv071 to make it clear that it is an
> unofficial draft of the proposed vector spec.  Or maybe be we can use
> v0p7 but that isn't an officially supported extension name.
>
> If this rvv 0.7.1 implementation is considered a temporary solution,
> maybe we can just remove all of this work when the official rvv spec if
> available?  But presumably it is better if we can have both this

That is generally the plan. When the final spec comes out this will be
updated and we will only support that.

> implementation and the official one, which means everything needs to be
> conditional or tied to an Xsomething extension name instead of the V
> extension name.

I agree that would be nice, but that is a lot of extra maintenance to
support a draft spec.

Considering most other projects won't accept draft specs I don't see
the huge appeal to maintain draft specs.

Alistair

>
> Jim

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

* Re: [PATCH v5 0/4] target-riscv: support vector extension part 1
@ 2020-02-26 22:28     ` Alistair Francis
  0 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 22:28 UTC (permalink / raw)
  To: Jim Wilson
  Cc: LIU Zhiwei, Richard Henderson, Chih-Min Chao, Palmer Dabbelt,
	wenmeng_zhang, open list:RISC-V,
	qemu-devel@nongnu.org Developers, wxy194768

On Wed, Feb 26, 2020 at 12:09 PM Jim Wilson <jimw@sifive.com> wrote:
>
> On 2/21/20 1:45 AM, LIU Zhiwei wrote:
> > This is the first part of v5 patchset. The changelog of v5 is only coverd
> > the part1.
> >
> > Features:
> >    * support specification riscv-v-spec-0.7.1.
>
> I'm still concerned about versioning issues.  This implements an
> unofficial draft of the proposed RISC-V vector extension.  This draft is
> not compatible with the current draft, and will be even less compatible
> with the final official version of the vector spec.

I wouldn't say this is an unofficial draft. v0.7.1 is an official and
tagged draft spec.

It is a draft spec and there is a new version (v0.8.0) and eventually
there will be a full (not draft release).

>
> The patch adds a version which is good, but there is only one check when
> qemu starts.  Probably something like 25% of these patches will be wrong
> for the official vector extension.  How are we going to handle this when
> someone submits patches for the official support?  It would be better if

The current plan (for all draft extensions) is to support just the
latest draft in QEMU. That is QEMU will have experimental support for
draft extension x at v0.1. When draft extension x is updated to v0.2
we will update the QEMU implementation (when we can) and only support
the v0.2.

We will continue updating and supporting the latest until the final
spec release, in which case we will then maintain the feature
following QEMU's deprecation policy.

While the spec is in a draft state the feature will be considered
experimental (hence the subject to change).

This way we can support and enable users to test on draft extensions,
but not spend all our time supporting draft extensions.

> everything in these patches were conditional on the version number.  It
> might also be better if we stopped calling this the 'v' extension and
> maybe used another name like Xrvv071 to make it clear that it is an
> unofficial draft of the proposed vector spec.  Or maybe be we can use
> v0p7 but that isn't an officially supported extension name.
>
> If this rvv 0.7.1 implementation is considered a temporary solution,
> maybe we can just remove all of this work when the official rvv spec if
> available?  But presumably it is better if we can have both this

That is generally the plan. When the final spec comes out this will be
updated and we will only support that.

> implementation and the official one, which means everything needs to be
> conditional or tied to an Xsomething extension name instead of the V
> extension name.

I agree that would be nice, but that is a lot of extra maintenance to
support a draft spec.

Considering most other projects won't accept draft specs I don't see
the huge appeal to maintain draft specs.

Alistair

>
> Jim

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

* Re: [PATCH v5 0/4] target-riscv: support vector extension part 1
  2020-02-26 22:28     ` Alistair Francis
@ 2020-02-26 23:39       ` Jim Wilson
  -1 siblings, 0 replies; 37+ messages in thread
From: Jim Wilson @ 2020-02-26 23:39 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, LIU Zhiwei

On Wed, Feb 26, 2020 at 2:36 PM Alistair Francis <alistair23@gmail.com> wrote:
> On Wed, Feb 26, 2020 at 12:09 PM Jim Wilson <jimw@sifive.com> wrote:
> > If this rvv 0.7.1 implementation is considered a temporary solution,
> > maybe we can just remove all of this work when the official rvv spec if
> > available?  But presumably it is better if we can have both this
>
> That is generally the plan. When the final spec comes out this will be
> updated and we will only support that.

This solves my problem, but maybe creates one for Alibaba.  They have
apparently fabbed a chip using the 0.7.1 draft of the vector spec
proposal.  So for qemu to properly support their asic, the 0.7.1 draft
support will have to be retained.  But I think SiFive and everyone
else is waiting for the official spec before building asics with
vector support.  If Alibaba will update their processor as the spec
evolves, then maybe this isn't a problem for them.

Jim

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

* Re: [PATCH v5 0/4] target-riscv: support vector extension part 1
@ 2020-02-26 23:39       ` Jim Wilson
  0 siblings, 0 replies; 37+ messages in thread
From: Jim Wilson @ 2020-02-26 23:39 UTC (permalink / raw)
  To: Alistair Francis
  Cc: LIU Zhiwei, Richard Henderson, Chih-Min Chao, Palmer Dabbelt,
	wenmeng_zhang, open list:RISC-V,
	qemu-devel@nongnu.org Developers, wxy194768

On Wed, Feb 26, 2020 at 2:36 PM Alistair Francis <alistair23@gmail.com> wrote:
> On Wed, Feb 26, 2020 at 12:09 PM Jim Wilson <jimw@sifive.com> wrote:
> > If this rvv 0.7.1 implementation is considered a temporary solution,
> > maybe we can just remove all of this work when the official rvv spec if
> > available?  But presumably it is better if we can have both this
>
> That is generally the plan. When the final spec comes out this will be
> updated and we will only support that.

This solves my problem, but maybe creates one for Alibaba.  They have
apparently fabbed a chip using the 0.7.1 draft of the vector spec
proposal.  So for qemu to properly support their asic, the 0.7.1 draft
support will have to be retained.  But I think SiFive and everyone
else is waiting for the official spec before building asics with
vector support.  If Alibaba will update their processor as the spec
evolves, then maybe this isn't a problem for them.

Jim

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

* Re: [PATCH v5 0/4] target-riscv: support vector extension part 1
  2020-02-26 23:39       ` Jim Wilson
@ 2020-02-26 23:46         ` Alistair Francis
  -1 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 23:46 UTC (permalink / raw)
  To: Jim Wilson
  Cc: open list:RISC-V, Richard Henderson,
	qemu-devel@nongnu.org Developers, wxy194768, Chih-Min Chao,
	wenmeng_zhang, Palmer Dabbelt, LIU Zhiwei

On Wed, Feb 26, 2020 at 3:40 PM Jim Wilson <jimw@sifive.com> wrote:
>
> On Wed, Feb 26, 2020 at 2:36 PM Alistair Francis <alistair23@gmail.com> wrote:
> > On Wed, Feb 26, 2020 at 12:09 PM Jim Wilson <jimw@sifive.com> wrote:
> > > If this rvv 0.7.1 implementation is considered a temporary solution,
> > > maybe we can just remove all of this work when the official rvv spec if
> > > available?  But presumably it is better if we can have both this
> >
> > That is generally the plan. When the final spec comes out this will be
> > updated and we will only support that.
>
> This solves my problem, but maybe creates one for Alibaba.  They have
> apparently fabbed a chip using the 0.7.1 draft of the vector spec
> proposal.  So for qemu to properly support their asic, the 0.7.1 draft
> support will have to be retained.  But I think SiFive and everyone
> else is waiting for the official spec before building asics with
> vector support.  If Alibaba will update their processor as the spec
> evolves, then maybe this isn't a problem for them.

That does seem unfortunate. I don't see a way for us to be able to
support previous draft spec versions though. That seems like a huge
amount of maintenance.

It shouldn't actually be that bad. Here is kind of what I'm imagining.

 - Vector extensions v0.7.1 are merged to QEMU
 - QEMU 5.x ships with v0.7.1 vector extensions (it seems unlikely
someone will upstream v0.8.0 before the 5.x release)

At some point in the future someone will update the experimental
support in QEMU from v0.7.1 to a fully released v1.0. In which case
there is still a release with the older experimental support which can
be used for the v0.7.1 if required.

Alistair

>
> Jim


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

* Re: [PATCH v5 0/4] target-riscv: support vector extension part 1
@ 2020-02-26 23:46         ` Alistair Francis
  0 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-02-26 23:46 UTC (permalink / raw)
  To: Jim Wilson
  Cc: LIU Zhiwei, Richard Henderson, Chih-Min Chao, Palmer Dabbelt,
	wenmeng_zhang, open list:RISC-V,
	qemu-devel@nongnu.org Developers, wxy194768

On Wed, Feb 26, 2020 at 3:40 PM Jim Wilson <jimw@sifive.com> wrote:
>
> On Wed, Feb 26, 2020 at 2:36 PM Alistair Francis <alistair23@gmail.com> wrote:
> > On Wed, Feb 26, 2020 at 12:09 PM Jim Wilson <jimw@sifive.com> wrote:
> > > If this rvv 0.7.1 implementation is considered a temporary solution,
> > > maybe we can just remove all of this work when the official rvv spec if
> > > available?  But presumably it is better if we can have both this
> >
> > That is generally the plan. When the final spec comes out this will be
> > updated and we will only support that.
>
> This solves my problem, but maybe creates one for Alibaba.  They have
> apparently fabbed a chip using the 0.7.1 draft of the vector spec
> proposal.  So for qemu to properly support their asic, the 0.7.1 draft
> support will have to be retained.  But I think SiFive and everyone
> else is waiting for the official spec before building asics with
> vector support.  If Alibaba will update their processor as the spec
> evolves, then maybe this isn't a problem for them.

That does seem unfortunate. I don't see a way for us to be able to
support previous draft spec versions though. That seems like a huge
amount of maintenance.

It shouldn't actually be that bad. Here is kind of what I'm imagining.

 - Vector extensions v0.7.1 are merged to QEMU
 - QEMU 5.x ships with v0.7.1 vector extensions (it seems unlikely
someone will upstream v0.8.0 before the 5.x release)

At some point in the future someone will update the experimental
support in QEMU from v0.7.1 to a fully released v1.0. In which case
there is still a release with the older experimental support which can
be used for the v0.7.1 if required.

Alistair

>
> Jim


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

end of thread, other threads:[~2020-02-27 21:57 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-21  9:45 [PATCH v5 0/4] target-riscv: support vector extension part 1 LIU Zhiwei
2020-02-21  9:45 ` LIU Zhiwei
2020-02-21  9:45 ` [PATCH v5 1/4] target/riscv: add vector extension field in CPURISCVState LIU Zhiwei
2020-02-21  9:45   ` LIU Zhiwei
2020-02-26 18:03   ` Alistair Francis
2020-02-26 18:03     ` Alistair Francis
2020-02-27 20:32   ` Richard Henderson
2020-02-27 20:32     ` Richard Henderson
2020-02-21  9:45 ` [PATCH v5 2/4] target/riscv: implementation-defined constant parameters LIU Zhiwei
2020-02-21  9:45   ` LIU Zhiwei
2020-02-26 18:05   ` Alistair Francis
2020-02-26 18:05     ` Alistair Francis
2020-02-27 20:33   ` Richard Henderson
2020-02-27 20:33     ` Richard Henderson
2020-02-21  9:45 ` [PATCH v5 3/4] target/riscv: support vector extension csr LIU Zhiwei
2020-02-21  9:45   ` LIU Zhiwei
2020-02-26 18:42   ` Alistair Francis
2020-02-26 18:42     ` Alistair Francis
2020-02-27  0:41     ` LIU Zhiwei
2020-02-27  0:41       ` LIU Zhiwei
2020-02-26 20:16   ` Jim Wilson
2020-02-21  9:45 ` [PATCH v5 4/4] target/riscv: add vector configure instruction LIU Zhiwei
2020-02-21  9:45   ` LIU Zhiwei
2020-02-26 19:20   ` Alistair Francis
2020-02-26 19:20     ` Alistair Francis
2020-02-27  1:41     ` LIU Zhiwei
2020-02-27  1:41       ` LIU Zhiwei
2020-02-27 21:48       ` Alistair Francis
2020-02-27 21:48         ` Alistair Francis
2020-02-26 20:20   ` Jim Wilson
2020-02-26 20:09 ` [PATCH v5 0/4] target-riscv: support vector extension part 1 Jim Wilson
2020-02-26 22:28   ` Alistair Francis
2020-02-26 22:28     ` Alistair Francis
2020-02-26 23:39     ` Jim Wilson
2020-02-26 23:39       ` Jim Wilson
2020-02-26 23:46       ` Alistair Francis
2020-02-26 23:46         ` Alistair Francis

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.