* [PATCH v12 00/10] support subsets of code size reduction extension
@ 2023-03-07 8:13 Weiwei Li
2023-03-07 8:13 ` [PATCH v12 01/10] target/riscv: add cfg properties for Zc* extension Weiwei Li
` (11 more replies)
0 siblings, 12 replies; 28+ messages in thread
From: Weiwei Li @ 2023-03-07 8:13 UTC (permalink / raw)
To: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
This patchset implements RISC-V Zc* extension v1.0.3-1 version instructions.
Specification:
https://github.com/riscv/riscv-code-size-reduction/tree/main/Zc-specification
The port is available here:
https://github.com/plctlab/plct-qemu/tree/plct-zce-upstream-v12
To test Zc* implementation, specify cpu argument with 'x-zca=true,x-zcb=true,x-zcf=true,f=true" and "x-zcd=true,d=true" (or "x-zcmp=true,x-zcmt=true" with c or d=false) to enable Zca/Zcb/Zcf and Zcd(or Zcmp,Zcmt) extensions support. We can also specify "x-zce=true,f=true" to enable Zca/Zcb/Zcmp/Zcmt and Zcf.
This implementation can pass the basic zc tests from https://github.com/yulong-plct/zc-test
v12
* add patch 10 to support zce property
* rebase on upstream master: reuse riscv_get_cfg() in patch 7 and remove tcg_temp_free in patch 6
v11
* update format and field name based on the latest spec in patch 5, 6, 7 (without other functional changes)
* rebase on riscv-to-apply.next
v10:
* rebase on Daniel's series(riscv-to-apply.next) and adjust riscv-tests to test on sifive related CPUs
v9:
* rebase on riscv-to-apply.next
v8:
* improve disas support in Patch 9
v7:
* Fix description for Zca
v6：
* fix base address for jump table in Patch 7
* rebase on riscv-to-apply.next
v5:
* fix exception unwind problem for cpu_ld*_code in helper of cm_jalt
v4:
* improve Zcmp suggested by Richard
* fix stateen related check for Zcmt
v3:
* update the solution for Zcf to the way of Zcd
* update Zcb to reuse gen_load/store
* use trans function instead of helper for push/pop
v2:
* add check for relationship between Zca/Zcf/Zcd with C/F/D based on related discussion in review of Zc* spec
* separate c.fld{sp}/fsd{sp} with fld{sp}/fsd{sp} before support of zcmp/zcmt
Weiwei Li (10):
target/riscv: add cfg properties for Zc* extension
target/riscv: add support for Zca extension
target/riscv: add support for Zcf extension
target/riscv: add support for Zcd extension
target/riscv: add support for Zcb extension
target/riscv: add support for Zcmp extension
target/riscv: add support for Zcmt extension
target/riscv: expose properties for Zc* extension
disas/riscv.c: add disasm support for Zc*
target/riscv: Add support for Zce
disas/riscv.c | 228 +++++++++++++++-
target/riscv/cpu.c | 69 +++++
target/riscv/cpu.h | 11 +
target/riscv/cpu_bits.h | 7 +
target/riscv/csr.c | 36 ++-
target/riscv/helper.h | 3 +
target/riscv/insn16.decode | 62 ++++-
target/riscv/insn_trans/trans_rvd.c.inc | 18 ++
target/riscv/insn_trans/trans_rvf.c.inc | 18 ++
target/riscv/insn_trans/trans_rvi.c.inc | 4 +-
target/riscv/insn_trans/trans_rvzce.c.inc | 311 ++++++++++++++++++++++
target/riscv/machine.c | 19 ++
target/riscv/meson.build | 3 +-
target/riscv/translate.c | 15 +-
target/riscv/zce_helper.c | 55 ++++
15 files changed, 843 insertions(+), 16 deletions(-)
create mode 100644 target/riscv/insn_trans/trans_rvzce.c.inc
create mode 100644 target/riscv/zce_helper.c
--
2.25.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v12 01/10] target/riscv: add cfg properties for Zc* extension
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
@ 2023-03-07 8:13 ` Weiwei Li
2023-03-07 8:13 ` [PATCH v12 02/10] target/riscv: add support for Zca extension Weiwei Li
` (10 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Weiwei Li @ 2023-03-07 8:13 UTC (permalink / raw)
To: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
Add properties for Zca,Zcb,Zcf,Zcd,Zcmp,Zcmt extension.
Add check for these properties.
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/cpu.c | 43 +++++++++++++++++++++++++++++++++++++++++++
target/riscv/cpu.h | 6 ++++++
2 files changed, 49 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1e97473af2..df7eed57af 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -939,6 +939,49 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
}
}
+ if (cpu->cfg.ext_c) {
+ cpu->cfg.ext_zca = true;
+ if (cpu->cfg.ext_f && env->misa_mxl_max == MXL_RV32) {
+ cpu->cfg.ext_zcf = true;
+ }
+ if (cpu->cfg.ext_d) {
+ cpu->cfg.ext_zcd = true;
+ }
+ }
+
+ if (env->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
+ error_setg(errp, "Zcf extension is only relevant to RV32");
+ return;
+ }
+
+ if (!cpu->cfg.ext_f && cpu->cfg.ext_zcf) {
+ error_setg(errp, "Zcf extension requires F extension");
+ return;
+ }
+
+ if (!cpu->cfg.ext_d && cpu->cfg.ext_zcd) {
+ error_setg(errp, "Zcd extension requires D extension");
+ return;
+ }
+
+ if ((cpu->cfg.ext_zcf || cpu->cfg.ext_zcd || cpu->cfg.ext_zcb ||
+ cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt) && !cpu->cfg.ext_zca) {
+ error_setg(errp, "Zcf/Zcd/Zcb/Zcmp/Zcmt extensions require Zca "
+ "extension");
+ return;
+ }
+
+ if (cpu->cfg.ext_zcd && (cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt)) {
+ error_setg(errp, "Zcmp/Zcmt extensions are incompatible with "
+ "Zcd extension");
+ return;
+ }
+
+ if (cpu->cfg.ext_zcmt && !cpu->cfg.ext_icsr) {
+ error_setg(errp, "Zcmt extension requires Zicsr extension");
+ return;
+ }
+
if (cpu->cfg.ext_zk) {
cpu->cfg.ext_zkn = true;
cpu->cfg.ext_zkr = true;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 638e47c75a..a2426ec479 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -438,6 +438,12 @@ struct RISCVCPUConfig {
bool ext_zbkc;
bool ext_zbkx;
bool ext_zbs;
+ bool ext_zca;
+ bool ext_zcb;
+ bool ext_zcd;
+ bool ext_zcf;
+ bool ext_zcmp;
+ bool ext_zcmt;
bool ext_zk;
bool ext_zkn;
bool ext_zknd;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
2023-03-07 8:13 ` [PATCH v12 01/10] target/riscv: add cfg properties for Zc* extension Weiwei Li
@ 2023-03-07 8:13 ` Weiwei Li
2023-04-06 20:22 ` Daniel Henrique Barboza
2023-03-07 8:13 ` [PATCH v12 03/10] target/riscv: add support for Zcf extension Weiwei Li
` (9 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Weiwei Li @ 2023-03-07 8:13 UTC (permalink / raw)
To: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li, Wilfred Mallawa
Modify the check for C extension to Zca (C implies Zca).
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
---
target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
target/riscv/translate.c | 8 ++++++--
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index 4ad54e8a49..c70c495fc5 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
gen_set_pc(ctx, cpu_pc);
- if (!has_ext(ctx, RVC)) {
+ if (!ctx->cfg_ptr->ext_zca) {
TCGv t0 = tcg_temp_new();
misaligned = gen_new_label();
@@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
gen_set_label(l); /* branch taken */
- if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
+ if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
/* misaligned */
gen_exception_inst_addr_mis(ctx);
} else {
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0ee8ee147d..d1fdd0c2d7 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
/* check misaligned: */
next_pc = ctx->base.pc_next + imm;
- if (!has_ext(ctx, RVC)) {
+ if (!ctx->cfg_ptr->ext_zca) {
if ((next_pc & 0x3) != 0) {
gen_exception_inst_addr_mis(ctx);
return;
@@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
if (insn_len(opcode) == 2) {
ctx->opcode = opcode;
ctx->pc_succ_insn = ctx->base.pc_next + 2;
- if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
+ /*
+ * The Zca extension is added as way to refer to instructions in the C
+ * extension that do not include the floating-point loads and stores
+ */
+ if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
return;
}
} else {
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v12 03/10] target/riscv: add support for Zcf extension
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
2023-03-07 8:13 ` [PATCH v12 01/10] target/riscv: add cfg properties for Zc* extension Weiwei Li
2023-03-07 8:13 ` [PATCH v12 02/10] target/riscv: add support for Zca extension Weiwei Li
@ 2023-03-07 8:13 ` Weiwei Li
2023-03-07 8:13 ` [PATCH v12 04/10] target/riscv: add support for Zcd extension Weiwei Li
` (8 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Weiwei Li @ 2023-03-07 8:13 UTC (permalink / raw)
To: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
Separate c_flw/c_fsw from flw/fsw to add check for Zcf extension.
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/insn16.decode | 8 ++++----
target/riscv/insn_trans/trans_rvf.c.inc | 18 ++++++++++++++++++
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index ccfe59f294..f3ea650325 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -109,11 +109,11 @@ sw 110 ... ... .. ... 00 @cs_w
# *** RV32C and RV64C specific Standard Extension (Quadrant 0) ***
{
ld 011 ... ... .. ... 00 @cl_d
- flw 011 ... ... .. ... 00 @cl_w
+ c_flw 011 ... ... .. ... 00 @cl_w
}
{
sd 111 ... ... .. ... 00 @cs_d
- fsw 111 ... ... .. ... 00 @cs_w
+ c_fsw 111 ... ... .. ... 00 @cs_w
}
# *** RV32/64C Standard Extension (Quadrant 1) ***
@@ -174,9 +174,9 @@ sw 110 . ..... ..... 10 @c_swsp
{
c64_illegal 011 - 00000 ----- 10 # c.ldsp, RES rd=0
ld 011 . ..... ..... 10 @c_ldsp
- flw 011 . ..... ..... 10 @c_lwsp
+ c_flw 011 . ..... ..... 10 @c_lwsp
}
{
sd 111 . ..... ..... 10 @c_sdsp
- fsw 111 . ..... ..... 10 @c_swsp
+ c_fsw 111 . ..... ..... 10 @c_swsp
}
diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
index 052408f45c..9e9fa2087a 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -30,6 +30,12 @@
} \
} while (0)
+#define REQUIRE_ZCF(ctx) do { \
+ if (!ctx->cfg_ptr->ext_zcf) { \
+ return false; \
+ } \
+} while (0)
+
static bool trans_flw(DisasContext *ctx, arg_flw *a)
{
TCGv_i64 dest;
@@ -61,6 +67,18 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a)
return true;
}
+static bool trans_c_flw(DisasContext *ctx, arg_flw *a)
+{
+ REQUIRE_ZCF(ctx);
+ return trans_flw(ctx, a);
+}
+
+static bool trans_c_fsw(DisasContext *ctx, arg_fsw *a)
+{
+ REQUIRE_ZCF(ctx);
+ return trans_fsw(ctx, a);
+}
+
static bool trans_fmadd_s(DisasContext *ctx, arg_fmadd_s *a)
{
REQUIRE_FPU;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v12 04/10] target/riscv: add support for Zcd extension
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
` (2 preceding siblings ...)
2023-03-07 8:13 ` [PATCH v12 03/10] target/riscv: add support for Zcf extension Weiwei Li
@ 2023-03-07 8:13 ` Weiwei Li
2023-03-07 8:13 ` [PATCH v12 05/10] target/riscv: add support for Zcb extension Weiwei Li
` (7 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Weiwei Li @ 2023-03-07 8:13 UTC (permalink / raw)
To: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
Separate c_fld/c_fsd from fld/fsd to add additional check for
c.fld{sp}/c.fsd{sp} which is useful for zcmp/zcmt to reuse
their encodings.
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/insn16.decode | 8 ++++----
target/riscv/insn_trans/trans_rvd.c.inc | 18 ++++++++++++++++++
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index f3ea650325..b62664b6af 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -97,12 +97,12 @@
}
{
lq 001 ... ... .. ... 00 @cl_q
- fld 001 ... ... .. ... 00 @cl_d
+ c_fld 001 ... ... .. ... 00 @cl_d
}
lw 010 ... ... .. ... 00 @cl_w
{
sq 101 ... ... .. ... 00 @cs_q
- fsd 101 ... ... .. ... 00 @cs_d
+ c_fsd 101 ... ... .. ... 00 @cs_d
}
sw 110 ... ... .. ... 00 @cs_w
@@ -148,7 +148,7 @@ addw 100 1 11 ... 01 ... 01 @cs_2
slli 000 . ..... ..... 10 @c_shift2
{
lq 001 ... ... .. ... 10 @c_lqsp
- fld 001 . ..... ..... 10 @c_ldsp
+ c_fld 001 . ..... ..... 10 @c_ldsp
}
{
illegal 010 - 00000 ----- 10 # c.lwsp, RES rd=0
@@ -166,7 +166,7 @@ slli 000 . ..... ..... 10 @c_shift2
}
{
sq 101 ... ... .. ... 10 @c_sqsp
- fsd 101 ...... ..... 10 @c_sdsp
+ c_fsd 101 ...... ..... 10 @c_sdsp
}
sw 110 . ..... ..... 10 @c_swsp
diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
index 1597bf31d8..2c51e01c40 100644
--- a/target/riscv/insn_trans/trans_rvd.c.inc
+++ b/target/riscv/insn_trans/trans_rvd.c.inc
@@ -31,6 +31,12 @@
} \
} while (0)
+#define REQUIRE_ZCD(ctx) do { \
+ if (!ctx->cfg_ptr->ext_zcd) { \
+ return false; \
+ } \
+} while (0)
+
static bool trans_fld(DisasContext *ctx, arg_fld *a)
{
TCGv addr;
@@ -59,6 +65,18 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
return true;
}
+static bool trans_c_fld(DisasContext *ctx, arg_fld *a)
+{
+ REQUIRE_ZCD(ctx);
+ return trans_fld(ctx, a);
+}
+
+static bool trans_c_fsd(DisasContext *ctx, arg_fsd *a)
+{
+ REQUIRE_ZCD(ctx);
+ return trans_fsd(ctx, a);
+}
+
static bool trans_fmadd_d(DisasContext *ctx, arg_fmadd_d *a)
{
REQUIRE_FPU;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v12 05/10] target/riscv: add support for Zcb extension
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
` (3 preceding siblings ...)
2023-03-07 8:13 ` [PATCH v12 04/10] target/riscv: add support for Zcd extension Weiwei Li
@ 2023-03-07 8:13 ` Weiwei Li
2023-03-07 8:13 ` [PATCH v12 06/10] target/riscv: add support for Zcmp extension Weiwei Li
` (6 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Weiwei Li @ 2023-03-07 8:13 UTC (permalink / raw)
To: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
Add encode and trans* functions support for Zcb instructions.
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/insn16.decode | 23 +++++
target/riscv/insn_trans/trans_rvzce.c.inc | 100 ++++++++++++++++++++++
target/riscv/translate.c | 2 +
3 files changed, 125 insertions(+)
create mode 100644 target/riscv/insn_trans/trans_rvzce.c.inc
diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index b62664b6af..ab780fa46a 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -43,6 +43,8 @@
%imm_addi16sp 12:s1 3:2 5:1 2:1 6:1 !function=ex_shift_4
%imm_lui 12:s1 2:5 !function=ex_shift_12
+%uimm_cl_b 5:1 6:1
+%uimm_cl_h 5:1 !function=ex_shift_1
# Argument sets imported from insn32.decode:
&empty !extern
@@ -53,6 +55,7 @@
&b imm rs2 rs1 !extern
&u imm rd !extern
&shift shamt rs1 rd !extern
+&r2 rd rs1 !extern
# Formats 16:
@@ -89,6 +92,12 @@
@c_andi ... . .. ... ..... .. &i imm=%imm_ci rs1=%rs1_3 rd=%rs1_3
+@cu ... ... ... .. ... .. &r2 rs1=%rs1_3 rd=%rs1_3
+@cl_b ... . .. ... .. ... .. &i imm=%uimm_cl_b rs1=%rs1_3 rd=%rs2_3
+@cl_h ... . .. ... .. ... .. &i imm=%uimm_cl_h rs1=%rs1_3 rd=%rs2_3
+@cs_b ... . .. ... .. ... .. &s imm=%uimm_cl_b rs1=%rs1_3 rs2=%rs2_3
+@cs_h ... . .. ... .. ... .. &s imm=%uimm_cl_h rs1=%rs1_3 rs2=%rs2_3
+
# *** RV32/64C Standard Extension (Quadrant 0) ***
{
# Opcode of all zeros is illegal; rd != 0, nzuimm == 0 is reserved.
@@ -180,3 +189,17 @@ sw 110 . ..... ..... 10 @c_swsp
sd 111 . ..... ..... 10 @c_sdsp
c_fsw 111 . ..... ..... 10 @c_swsp
}
+
+# *** RV64 and RV32 Zcb Extension ***
+c_zext_b 100 111 ... 11 000 01 @cu
+c_sext_b 100 111 ... 11 001 01 @cu
+c_zext_h 100 111 ... 11 010 01 @cu
+c_sext_h 100 111 ... 11 011 01 @cu
+c_zext_w 100 111 ... 11 100 01 @cu
+c_not 100 111 ... 11 101 01 @cu
+c_mul 100 111 ... 10 ... 01 @cs_2
+c_lbu 100 000 ... .. ... 00 @cl_b
+c_lhu 100 001 ... 0. ... 00 @cl_h
+c_lh 100 001 ... 1. ... 00 @cl_h
+c_sb 100 010 ... .. ... 00 @cs_b
+c_sh 100 011 ... 0. ... 00 @cs_h
diff --git a/target/riscv/insn_trans/trans_rvzce.c.inc b/target/riscv/insn_trans/trans_rvzce.c.inc
new file mode 100644
index 0000000000..de96c4afaf
--- /dev/null
+++ b/target/riscv/insn_trans/trans_rvzce.c.inc
@@ -0,0 +1,100 @@
+/*
+ * RISC-V translation routines for the Zcb Standard Extension.
+ *
+ * Copyright (c) 2021-2022 PLCT Lab
+ *
+ * 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/>.
+ */
+
+#define REQUIRE_ZCB(ctx) do { \
+ if (!ctx->cfg_ptr->ext_zcb) \
+ return false; \
+} while (0)
+
+static bool trans_c_zext_b(DisasContext *ctx, arg_c_zext_b *a)
+{
+ REQUIRE_ZCB(ctx);
+ return gen_unary(ctx, a, EXT_NONE, tcg_gen_ext8u_tl);
+}
+
+static bool trans_c_zext_h(DisasContext *ctx, arg_c_zext_h *a)
+{
+ REQUIRE_ZCB(ctx);
+ REQUIRE_ZBB(ctx);
+ return gen_unary(ctx, a, EXT_NONE, tcg_gen_ext16u_tl);
+}
+
+static bool trans_c_sext_b(DisasContext *ctx, arg_c_sext_b *a)
+{
+ REQUIRE_ZCB(ctx);
+ REQUIRE_ZBB(ctx);
+ return gen_unary(ctx, a, EXT_NONE, tcg_gen_ext8s_tl);
+}
+
+static bool trans_c_sext_h(DisasContext *ctx, arg_c_sext_h *a)
+{
+ REQUIRE_ZCB(ctx);
+ REQUIRE_ZBB(ctx);
+ return gen_unary(ctx, a, EXT_NONE, tcg_gen_ext16s_tl);
+}
+
+static bool trans_c_zext_w(DisasContext *ctx, arg_c_zext_w *a)
+{
+ REQUIRE_64BIT(ctx);
+ REQUIRE_ZCB(ctx);
+ REQUIRE_ZBA(ctx);
+ return gen_unary(ctx, a, EXT_NONE, tcg_gen_ext32u_tl);
+}
+
+static bool trans_c_not(DisasContext *ctx, arg_c_not *a)
+{
+ REQUIRE_ZCB(ctx);
+ return gen_unary(ctx, a, EXT_NONE, tcg_gen_not_tl);
+}
+
+static bool trans_c_mul(DisasContext *ctx, arg_c_mul *a)
+{
+ REQUIRE_ZCB(ctx);
+ REQUIRE_M_OR_ZMMUL(ctx);
+ return gen_arith(ctx, a, EXT_NONE, tcg_gen_mul_tl, NULL);
+}
+
+static bool trans_c_lbu(DisasContext *ctx, arg_c_lbu *a)
+{
+ REQUIRE_ZCB(ctx);
+ return gen_load(ctx, a, MO_UB);
+}
+
+static bool trans_c_lhu(DisasContext *ctx, arg_c_lhu *a)
+{
+ REQUIRE_ZCB(ctx);
+ return gen_load(ctx, a, MO_UW);
+}
+
+static bool trans_c_lh(DisasContext *ctx, arg_c_lh *a)
+{
+ REQUIRE_ZCB(ctx);
+ return gen_load(ctx, a, MO_SW);
+}
+
+static bool trans_c_sb(DisasContext *ctx, arg_c_sb *a)
+{
+ REQUIRE_ZCB(ctx);
+ return gen_store(ctx, a, MO_UB);
+}
+
+static bool trans_c_sh(DisasContext *ctx, arg_c_sh *a)
+{
+ REQUIRE_ZCB(ctx);
+ return gen_store(ctx, a, MO_UW);
+}
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index d1fdd0c2d7..3634137d85 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1091,6 +1091,8 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
/* Include the auto-generated decoder for 16 bit insn */
#include "decode-insn16.c.inc"
+#include "insn_trans/trans_rvzce.c.inc"
+
/* Include decoders for factored-out extensions */
#include "decode-XVentanaCondOps.c.inc"
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v12 06/10] target/riscv: add support for Zcmp extension
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
` (4 preceding siblings ...)
2023-03-07 8:13 ` [PATCH v12 05/10] target/riscv: add support for Zcb extension Weiwei Li
@ 2023-03-07 8:13 ` Weiwei Li
2023-03-07 8:14 ` [PATCH v12 07/10] target/riscv: add support for Zcmt extension Weiwei Li
` (5 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Weiwei Li @ 2023-03-07 8:13 UTC (permalink / raw)
To: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
Add encode, trans* functions for Zcmp instructions.
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/insn16.decode | 18 +++
target/riscv/insn_trans/trans_rvzce.c.inc | 187 +++++++++++++++++++++-
target/riscv/translate.c | 5 +
3 files changed, 209 insertions(+), 1 deletion(-)
diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index ab780fa46a..55c9574299 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -21,6 +21,8 @@
%rs1_3 7:3 !function=ex_rvc_register
%rs2_3 2:3 !function=ex_rvc_register
%rs2_5 2:5
+%r1s 7:3 !function=ex_sreg_register
+%r2s 2:3 !function=ex_sreg_register
# Immediates:
%imm_ci 12:s1 2:5
@@ -45,6 +47,8 @@
%uimm_cl_b 5:1 6:1
%uimm_cl_h 5:1 !function=ex_shift_1
+%spimm 2:2 !function=ex_shift_4
+%urlist 4:4
# Argument sets imported from insn32.decode:
&empty !extern
@@ -56,7 +60,9 @@
&u imm rd !extern
&shift shamt rs1 rd !extern
&r2 rd rs1 !extern
+&r2_s rs1 rs2 !extern
+&cmpp urlist spimm
# Formats 16:
@cr .... ..... ..... .. &r rs2=%rs2_5 rs1=%rd %rd
@@ -97,6 +103,8 @@
@cl_h ... . .. ... .. ... .. &i imm=%uimm_cl_h rs1=%rs1_3 rd=%rs2_3
@cs_b ... . .. ... .. ... .. &s imm=%uimm_cl_b rs1=%rs1_3 rs2=%rs2_3
@cs_h ... . .. ... .. ... .. &s imm=%uimm_cl_h rs1=%rs1_3 rs2=%rs2_3
+@cm_pp ... ... ........ .. &cmpp %urlist %spimm
+@cm_mv ... ... ... .. ... .. &r2_s rs2=%r2s rs1=%r1s
# *** RV32/64C Standard Extension (Quadrant 0) ***
{
@@ -176,6 +184,16 @@ slli 000 . ..... ..... 10 @c_shift2
{
sq 101 ... ... .. ... 10 @c_sqsp
c_fsd 101 ...... ..... 10 @c_sdsp
+
+ # *** RV64 and RV32 Zcmp Extension ***
+ [
+ cm_push 101 11000 .... .. 10 @cm_pp
+ cm_pop 101 11010 .... .. 10 @cm_pp
+ cm_popret 101 11110 .... .. 10 @cm_pp
+ cm_popretz 101 11100 .... .. 10 @cm_pp
+ cm_mva01s 101 011 ... 11 ... 10 @cm_mv
+ cm_mvsa01 101 011 ... 01 ... 10 @cm_mv
+ ]
}
sw 110 . ..... ..... 10 @c_swsp
diff --git a/target/riscv/insn_trans/trans_rvzce.c.inc b/target/riscv/insn_trans/trans_rvzce.c.inc
index de96c4afaf..a47959eb67 100644
--- a/target/riscv/insn_trans/trans_rvzce.c.inc
+++ b/target/riscv/insn_trans/trans_rvzce.c.inc
@@ -1,5 +1,5 @@
/*
- * RISC-V translation routines for the Zcb Standard Extension.
+ * RISC-V translation routines for the Zc[b,mp] Standard Extensions.
*
* Copyright (c) 2021-2022 PLCT Lab
*
@@ -21,6 +21,11 @@
return false; \
} while (0)
+#define REQUIRE_ZCMP(ctx) do { \
+ if (!ctx->cfg_ptr->ext_zcmp) \
+ return false; \
+} while (0)
+
static bool trans_c_zext_b(DisasContext *ctx, arg_c_zext_b *a)
{
REQUIRE_ZCB(ctx);
@@ -98,3 +103,183 @@ static bool trans_c_sh(DisasContext *ctx, arg_c_sh *a)
REQUIRE_ZCB(ctx);
return gen_store(ctx, a, MO_UW);
}
+
+#define X_S0 8
+#define X_S1 9
+#define X_Sn 16
+
+static uint32_t decode_push_pop_list(DisasContext *ctx, target_ulong rlist)
+{
+ uint32_t reg_bitmap = 0;
+
+ if (ctx->cfg_ptr->ext_e && rlist > 6) {
+ return 0;
+ }
+
+ switch (rlist) {
+ case 15:
+ reg_bitmap |= 1 << (X_Sn + 11) ;
+ reg_bitmap |= 1 << (X_Sn + 10) ;
+ /* FALL THROUGH */
+ case 14:
+ reg_bitmap |= 1 << (X_Sn + 9) ;
+ /* FALL THROUGH */
+ case 13:
+ reg_bitmap |= 1 << (X_Sn + 8) ;
+ /* FALL THROUGH */
+ case 12:
+ reg_bitmap |= 1 << (X_Sn + 7) ;
+ /* FALL THROUGH */
+ case 11:
+ reg_bitmap |= 1 << (X_Sn + 6) ;
+ /* FALL THROUGH */
+ case 10:
+ reg_bitmap |= 1 << (X_Sn + 5) ;
+ /* FALL THROUGH */
+ case 9:
+ reg_bitmap |= 1 << (X_Sn + 4) ;
+ /* FALL THROUGH */
+ case 8:
+ reg_bitmap |= 1 << (X_Sn + 3) ;
+ /* FALL THROUGH */
+ case 7:
+ reg_bitmap |= 1 << (X_Sn + 2) ;
+ /* FALL THROUGH */
+ case 6:
+ reg_bitmap |= 1 << X_S1 ;
+ /* FALL THROUGH */
+ case 5:
+ reg_bitmap |= 1 << X_S0;
+ /* FALL THROUGH */
+ case 4:
+ reg_bitmap |= 1 << xRA;
+ break;
+ default:
+ break;
+ }
+
+ return reg_bitmap;
+}
+
+static bool gen_pop(DisasContext *ctx, arg_cmpp *a, bool ret, bool ret_val)
+{
+ REQUIRE_ZCMP(ctx);
+
+ uint32_t reg_bitmap = decode_push_pop_list(ctx, a->urlist);
+ if (reg_bitmap == 0) {
+ return false;
+ }
+
+ MemOp memop = get_ol(ctx) == MXL_RV32 ? MO_TEUL : MO_TEUQ;
+ int reg_size = memop_size(memop);
+ target_ulong stack_adj = ROUND_UP(ctpop32(reg_bitmap) * reg_size, 16) +
+ a->spimm;
+ TCGv sp = dest_gpr(ctx, xSP);
+ TCGv addr = tcg_temp_new();
+ int i;
+
+ tcg_gen_addi_tl(addr, sp, stack_adj - reg_size);
+
+ for (i = X_Sn + 11; i >= 0; i--) {
+ if (reg_bitmap & (1 << i)) {
+ TCGv dest = dest_gpr(ctx, i);
+ tcg_gen_qemu_ld_tl(dest, addr, ctx->mem_idx, memop);
+ gen_set_gpr(ctx, i, dest);
+ tcg_gen_subi_tl(addr, addr, reg_size);
+ }
+ }
+
+ tcg_gen_addi_tl(sp, sp, stack_adj);
+ gen_set_gpr(ctx, xSP, sp);
+
+ if (ret_val) {
+ gen_set_gpr(ctx, xA0, ctx->zero);
+ }
+
+ if (ret) {
+ TCGv ret_addr = get_gpr(ctx, xRA, EXT_NONE);
+ gen_set_pc(ctx, ret_addr);
+ tcg_gen_lookup_and_goto_ptr();
+ ctx->base.is_jmp = DISAS_NORETURN;
+ }
+
+ return true;
+}
+
+static bool trans_cm_push(DisasContext *ctx, arg_cm_push *a)
+{
+ REQUIRE_ZCMP(ctx);
+
+ uint32_t reg_bitmap = decode_push_pop_list(ctx, a->urlist);
+ if (reg_bitmap == 0) {
+ return false;
+ }
+
+ MemOp memop = get_ol(ctx) == MXL_RV32 ? MO_TEUL : MO_TEUQ;
+ int reg_size = memop_size(memop);
+ target_ulong stack_adj = ROUND_UP(ctpop32(reg_bitmap) * reg_size, 16) +
+ a->spimm;
+ TCGv sp = dest_gpr(ctx, xSP);
+ TCGv addr = tcg_temp_new();
+ int i;
+
+ tcg_gen_subi_tl(addr, sp, reg_size);
+
+ for (i = X_Sn + 11; i >= 0; i--) {
+ if (reg_bitmap & (1 << i)) {
+ TCGv val = get_gpr(ctx, i, EXT_NONE);
+ tcg_gen_qemu_st_tl(val, addr, ctx->mem_idx, memop);
+ tcg_gen_subi_tl(addr, addr, reg_size);
+ }
+ }
+
+ tcg_gen_subi_tl(sp, sp, stack_adj);
+ gen_set_gpr(ctx, xSP, sp);
+
+ return true;
+}
+
+static bool trans_cm_pop(DisasContext *ctx, arg_cm_pop *a)
+{
+ return gen_pop(ctx, a, false, false);
+}
+
+static bool trans_cm_popret(DisasContext *ctx, arg_cm_popret *a)
+{
+ return gen_pop(ctx, a, true, false);
+}
+
+static bool trans_cm_popretz(DisasContext *ctx, arg_cm_popret *a)
+{
+ return gen_pop(ctx, a, true, true);
+}
+
+static bool trans_cm_mva01s(DisasContext *ctx, arg_cm_mva01s *a)
+{
+ REQUIRE_ZCMP(ctx);
+
+ TCGv src1 = get_gpr(ctx, a->rs1, EXT_NONE);
+ TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
+
+ gen_set_gpr(ctx, xA0, src1);
+ gen_set_gpr(ctx, xA1, src2);
+
+ return true;
+}
+
+static bool trans_cm_mvsa01(DisasContext *ctx, arg_cm_mvsa01 *a)
+{
+ REQUIRE_ZCMP(ctx);
+
+ if (a->rs1 == a->rs2) {
+ return false;
+ }
+
+ TCGv a0 = get_gpr(ctx, xA0, EXT_NONE);
+ TCGv a1 = get_gpr(ctx, xA1, EXT_NONE);
+
+ gen_set_gpr(ctx, a->rs1, a0);
+ gen_set_gpr(ctx, a->rs2, a1);
+
+ return true;
+}
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 3634137d85..6872d17fb9 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -757,6 +757,11 @@ static int ex_rvc_register(DisasContext *ctx, int reg)
return 8 + reg;
}
+static int ex_sreg_register(DisasContext *ctx, int reg)
+{
+ return reg < 2 ? reg + 8 : reg + 16;
+}
+
static int ex_rvc_shiftli(DisasContext *ctx, int imm)
{
/* For RV128 a shamt of 0 means a shift by 64. */
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v12 07/10] target/riscv: add support for Zcmt extension
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
` (5 preceding siblings ...)
2023-03-07 8:13 ` [PATCH v12 06/10] target/riscv: add support for Zcmp extension Weiwei Li
@ 2023-03-07 8:14 ` Weiwei Li
2023-03-07 8:14 ` [PATCH v12 08/10] target/riscv: expose properties for Zc* extension Weiwei Li
` (4 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Weiwei Li @ 2023-03-07 8:14 UTC (permalink / raw)
To: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
Add encode, trans* functions and helper functions support for Zcmt
instrutions.
Add support for jvt csr.
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/cpu.h | 4 ++
target/riscv/cpu_bits.h | 7 +++
target/riscv/csr.c | 36 ++++++++++++++-
target/riscv/helper.h | 3 ++
target/riscv/insn16.decode | 7 ++-
target/riscv/insn_trans/trans_rvzce.c.inc | 28 +++++++++++-
target/riscv/machine.c | 19 ++++++++
target/riscv/meson.build | 3 +-
target/riscv/zce_helper.c | 55 +++++++++++++++++++++++
9 files changed, 157 insertions(+), 5 deletions(-)
create mode 100644 target/riscv/zce_helper.c
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a2426ec479..351d5e3e79 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -176,6 +176,8 @@ struct CPUArchState {
/* 128-bit helpers upper part return value */
target_ulong retxh;
+ target_ulong jvt;
+
#ifdef CONFIG_USER_ONLY
uint32_t elf_flags;
#endif
@@ -619,6 +621,8 @@ void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, uint32_t priv,
target_ulong new_val,
target_ulong write_mask),
void *rmw_fn_arg);
+
+RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit);
#endif
void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv);
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index fca7ef0cef..a92313a06f 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -319,6 +319,7 @@
#define SMSTATEEN_MAX_COUNT 4
#define SMSTATEEN0_CS (1ULL << 0)
#define SMSTATEEN0_FCSR (1ULL << 1)
+#define SMSTATEEN0_JVT (1ULL << 2)
#define SMSTATEEN0_HSCONTXT (1ULL << 57)
#define SMSTATEEN0_IMSIC (1ULL << 58)
#define SMSTATEEN0_AIA (1ULL << 59)
@@ -523,6 +524,9 @@
/* Crypto Extension */
#define CSR_SEED 0x015
+/* Zcmt Extension */
+#define CSR_JVT 0x017
+
/* mstatus CSR bits */
#define MSTATUS_UIE 0x00000001
#define MSTATUS_SIE 0x00000002
@@ -898,4 +902,7 @@ typedef enum RISCVException {
#define MHPMEVENT_IDX_MASK 0xFFFFF
#define MHPMEVENT_SSCOF_RESVD 16
+/* JVT CSR bits */
+#define JVT_MODE 0x3F
+#define JVT_BASE (~0x3F)
#endif
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ab566639e5..d7bf83bc8c 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -42,8 +42,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
/* Predicates */
#if !defined(CONFIG_USER_ONLY)
-static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
- uint64_t bit)
+RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit)
{
bool virt = riscv_cpu_virt_enabled(env);
@@ -162,6 +161,22 @@ static RISCVException ctr32(CPURISCVState *env, int csrno)
return ctr(env, csrno);
}
+static RISCVException zcmt(CPURISCVState *env, int csrno)
+{
+ if (!riscv_cpu_cfg(env)->ext_zcmt) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+#if !defined(CONFIG_USER_ONLY)
+ RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_JVT);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+#endif
+
+ return RISCV_EXCP_NONE;
+}
+
#if !defined(CONFIG_USER_ONLY)
static RISCVException mctr(CPURISCVState *env, int csrno)
{
@@ -3990,6 +4005,20 @@ RISCVException riscv_csrrw_debug(CPURISCVState *env, int csrno,
return ret;
}
+static RISCVException read_jvt(CPURISCVState *env, int csrno,
+ target_ulong *val)
+{
+ *val = env->jvt;
+ return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_jvt(CPURISCVState *env, int csrno,
+ target_ulong val)
+{
+ env->jvt = val;
+ return RISCV_EXCP_NONE;
+}
+
/* Control and Status Register function table */
riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
/* User Floating-Point CSRs */
@@ -4020,6 +4049,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
/* Crypto Extension */
[CSR_SEED] = { "seed", seed, NULL, NULL, rmw_seed },
+ /* Zcmt Extension */
+ [CSR_JVT] = {"jvt", zcmt, read_jvt, write_jvt},
+
#if !defined(CONFIG_USER_ONLY)
/* Machine Timers and Counters */
[CSR_MCYCLE] = { "mcycle", any, read_hpmcounter,
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 37b54e0991..1880e95c50 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1142,3 +1142,6 @@ DEF_HELPER_FLAGS_1(aes64im, TCG_CALL_NO_RWG_SE, tl, tl)
DEF_HELPER_FLAGS_3(sm4ed, TCG_CALL_NO_RWG_SE, tl, tl, tl, tl)
DEF_HELPER_FLAGS_3(sm4ks, TCG_CALL_NO_RWG_SE, tl, tl, tl, tl)
+
+/* Zce helper */
+DEF_HELPER_FLAGS_2(cm_jalt, TCG_CALL_NO_WG, tl, env, i32)
diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index 55c9574299..b96c534e73 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -49,6 +49,7 @@
%uimm_cl_h 5:1 !function=ex_shift_1
%spimm 2:2 !function=ex_shift_4
%urlist 4:4
+%index 2:8
# Argument sets imported from insn32.decode:
&empty !extern
@@ -63,6 +64,7 @@
&r2_s rs1 rs2 !extern
&cmpp urlist spimm
+&cmjt index
# Formats 16:
@cr .... ..... ..... .. &r rs2=%rs2_5 rs1=%rd %rd
@@ -105,6 +107,7 @@
@cs_h ... . .. ... .. ... .. &s imm=%uimm_cl_h rs1=%rs1_3 rs2=%rs2_3
@cm_pp ... ... ........ .. &cmpp %urlist %spimm
@cm_mv ... ... ... .. ... .. &r2_s rs2=%r2s rs1=%r1s
+@cm_jt ... ... ........ .. &cmjt %index
# *** RV32/64C Standard Extension (Quadrant 0) ***
{
@@ -185,7 +188,7 @@ slli 000 . ..... ..... 10 @c_shift2
sq 101 ... ... .. ... 10 @c_sqsp
c_fsd 101 ...... ..... 10 @c_sdsp
- # *** RV64 and RV32 Zcmp Extension ***
+ # *** RV64 and RV32 Zcmp/Zcmt Extension ***
[
cm_push 101 11000 .... .. 10 @cm_pp
cm_pop 101 11010 .... .. 10 @cm_pp
@@ -193,6 +196,8 @@ slli 000 . ..... ..... 10 @c_shift2
cm_popretz 101 11100 .... .. 10 @cm_pp
cm_mva01s 101 011 ... 11 ... 10 @cm_mv
cm_mvsa01 101 011 ... 01 ... 10 @cm_mv
+
+ cm_jalt 101 000 ........ 10 @cm_jt
]
}
sw 110 . ..... ..... 10 @c_swsp
diff --git a/target/riscv/insn_trans/trans_rvzce.c.inc b/target/riscv/insn_trans/trans_rvzce.c.inc
index a47959eb67..d75acbc4a6 100644
--- a/target/riscv/insn_trans/trans_rvzce.c.inc
+++ b/target/riscv/insn_trans/trans_rvzce.c.inc
@@ -1,5 +1,5 @@
/*
- * RISC-V translation routines for the Zc[b,mp] Standard Extensions.
+ * RISC-V translation routines for the Zc[b,mp,mt] Standard Extensions.
*
* Copyright (c) 2021-2022 PLCT Lab
*
@@ -26,6 +26,11 @@
return false; \
} while (0)
+#define REQUIRE_ZCMT(ctx) do { \
+ if (!ctx->cfg_ptr->ext_zcmt) \
+ return false; \
+} while (0)
+
static bool trans_c_zext_b(DisasContext *ctx, arg_c_zext_b *a)
{
REQUIRE_ZCB(ctx);
@@ -283,3 +288,24 @@ static bool trans_cm_mvsa01(DisasContext *ctx, arg_cm_mvsa01 *a)
return true;
}
+
+static bool trans_cm_jalt(DisasContext *ctx, arg_cm_jalt *a)
+{
+ REQUIRE_ZCMT(ctx);
+
+ /*
+ * Update pc to current for the non-unwinding exception
+ * that might come from cpu_ld*_code() in the helper.
+ */
+ tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
+ gen_helper_cm_jalt(cpu_pc, cpu_env, tcg_constant_i32(a->index));
+
+ /* c.jt vs c.jalt depends on the index. */
+ if (a->index >= 32) {
+ gen_set_gpri(ctx, xRA, ctx->pc_succ_insn);
+ }
+
+ tcg_gen_lookup_and_goto_ptr();
+ ctx->base.is_jmp = DISAS_NORETURN;
+ return true;
+}
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 9c455931d8..27f430ad74 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -329,6 +329,24 @@ static const VMStateDescription vmstate_pmu_ctr_state = {
}
};
+static bool jvt_needed(void *opaque)
+{
+ RISCVCPU *cpu = opaque;
+
+ return cpu->cfg.ext_zcmt;
+}
+
+static const VMStateDescription vmstate_jvt = {
+ .name = "cpu/jvt",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = jvt_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINTTL(env.jvt, RISCVCPU),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
const VMStateDescription vmstate_riscv_cpu = {
.name = "cpu",
.version_id = 7,
@@ -395,6 +413,7 @@ const VMStateDescription vmstate_riscv_cpu = {
&vmstate_envcfg,
&vmstate_debug,
&vmstate_smstateen,
+ &vmstate_jvt,
NULL
}
};
diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index 5dee37a242..5b7f813a3e 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -19,7 +19,8 @@ riscv_ss.add(files(
'bitmanip_helper.c',
'translate.c',
'm128_helper.c',
- 'crypto_helper.c'
+ 'crypto_helper.c',
+ 'zce_helper.c'
))
riscv_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: files('kvm-stub.c'))
diff --git a/target/riscv/zce_helper.c b/target/riscv/zce_helper.c
new file mode 100644
index 0000000000..b433bda16d
--- /dev/null
+++ b/target/riscv/zce_helper.c
@@ -0,0 +1,55 @@
+/*
+ * RISC-V Zcmt Extension Helper for QEMU.
+ *
+ * Copyright (c) 2021-2022 PLCT Lab
+ *
+ * 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 "exec/cpu_ldst.h"
+
+target_ulong HELPER(cm_jalt)(CPURISCVState *env, uint32_t index)
+{
+
+#if !defined(CONFIG_USER_ONLY)
+ RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_JVT);
+ if (ret != RISCV_EXCP_NONE) {
+ riscv_raise_exception(env, ret, 0);
+ }
+#endif
+
+ target_ulong target;
+ target_ulong val = env->jvt;
+ int xlen = riscv_cpu_xlen(env);
+ uint8_t mode = get_field(val, JVT_MODE);
+ target_ulong base = val & JVT_BASE;
+ target_ulong t0;
+
+ if (mode != 0) {
+ riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, 0);
+ }
+
+ if (xlen == 32) {
+ t0 = base + (index << 2);
+ target = cpu_ldl_code(env, t0);
+ } else {
+ t0 = base + (index << 3);
+ target = cpu_ldq_code(env, t0);
+ }
+
+ return target & ~0x1;
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v12 08/10] target/riscv: expose properties for Zc* extension
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
` (6 preceding siblings ...)
2023-03-07 8:14 ` [PATCH v12 07/10] target/riscv: add support for Zcmt extension Weiwei Li
@ 2023-03-07 8:14 ` Weiwei Li
2023-03-07 8:14 ` [PATCH v12 09/10] disas/riscv.c: add disasm support for Zc* Weiwei Li
` (3 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Weiwei Li @ 2023-03-07 8:14 UTC (permalink / raw)
To: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
Expose zca,zcb,zcf,zcd,zcmp,zcmt properties.
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/cpu.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index df7eed57af..d17ae942bd 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -87,6 +87,12 @@ static const struct isa_ext_data isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zfhmin, true, PRIV_VERSION_1_12_0, ext_zfhmin),
ISA_EXT_DATA_ENTRY(zfinx, true, PRIV_VERSION_1_12_0, ext_zfinx),
ISA_EXT_DATA_ENTRY(zdinx, true, PRIV_VERSION_1_12_0, ext_zdinx),
+ ISA_EXT_DATA_ENTRY(zca, true, PRIV_VERSION_1_12_0, ext_zca),
+ ISA_EXT_DATA_ENTRY(zcb, true, PRIV_VERSION_1_12_0, ext_zcb),
+ ISA_EXT_DATA_ENTRY(zcf, true, PRIV_VERSION_1_12_0, ext_zcf),
+ ISA_EXT_DATA_ENTRY(zcd, true, PRIV_VERSION_1_12_0, ext_zcd),
+ ISA_EXT_DATA_ENTRY(zcmp, true, PRIV_VERSION_1_12_0, ext_zcmp),
+ ISA_EXT_DATA_ENTRY(zcmt, true, PRIV_VERSION_1_12_0, ext_zcmt),
ISA_EXT_DATA_ENTRY(zba, true, PRIV_VERSION_1_12_0, ext_zba),
ISA_EXT_DATA_ENTRY(zbb, true, PRIV_VERSION_1_12_0, ext_zbb),
ISA_EXT_DATA_ENTRY(zbc, true, PRIV_VERSION_1_12_0, ext_zbc),
@@ -1491,6 +1497,14 @@ static Property riscv_cpu_extensions[] = {
/* These are experimental so mark with 'x-' */
DEFINE_PROP_BOOL("x-zicond", RISCVCPU, cfg.ext_zicond, false),
DEFINE_PROP_BOOL("x-j", RISCVCPU, cfg.ext_j, false),
+
+ DEFINE_PROP_BOOL("x-zca", RISCVCPU, cfg.ext_zca, false),
+ DEFINE_PROP_BOOL("x-zcb", RISCVCPU, cfg.ext_zcb, false),
+ DEFINE_PROP_BOOL("x-zcd", RISCVCPU, cfg.ext_zcd, false),
+ DEFINE_PROP_BOOL("x-zcf", RISCVCPU, cfg.ext_zcf, false),
+ DEFINE_PROP_BOOL("x-zcmp", RISCVCPU, cfg.ext_zcmp, false),
+ DEFINE_PROP_BOOL("x-zcmt", RISCVCPU, cfg.ext_zcmt, false),
+
/* ePMP 0.9.3 */
DEFINE_PROP_BOOL("x-epmp", RISCVCPU, cfg.epmp, false),
DEFINE_PROP_BOOL("x-smaia", RISCVCPU, cfg.ext_smaia, false),
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v12 09/10] disas/riscv.c: add disasm support for Zc*
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
` (7 preceding siblings ...)
2023-03-07 8:14 ` [PATCH v12 08/10] target/riscv: expose properties for Zc* extension Weiwei Li
@ 2023-03-07 8:14 ` Weiwei Li
2023-03-07 8:14 ` [PATCH v12 10/10] target/riscv: Add support for Zce Weiwei Li
` (2 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Weiwei Li @ 2023-03-07 8:14 UTC (permalink / raw)
To: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
Zcmp/Zcmt instructions will override disasm for c.fld*/c.fsd*
instructions currently.
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---
disas/riscv.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 227 insertions(+), 1 deletion(-)
diff --git a/disas/riscv.c b/disas/riscv.c
index 54455aaaa8..189214c41f 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -163,6 +163,13 @@ typedef enum {
rv_codec_v_i,
rv_codec_vsetvli,
rv_codec_vsetivli,
+ rv_codec_zcb_ext,
+ rv_codec_zcb_mul,
+ rv_codec_zcb_lb,
+ rv_codec_zcb_lh,
+ rv_codec_zcmp_cm_pushpop,
+ rv_codec_zcmp_cm_mv,
+ rv_codec_zcmt_jt,
} rv_codec;
typedef enum {
@@ -935,6 +942,26 @@ typedef enum {
rv_op_vsetvli = 766,
rv_op_vsetivli = 767,
rv_op_vsetvl = 768,
+ rv_op_c_zext_b = 769,
+ rv_op_c_sext_b = 770,
+ rv_op_c_zext_h = 771,
+ rv_op_c_sext_h = 772,
+ rv_op_c_zext_w = 773,
+ rv_op_c_not = 774,
+ rv_op_c_mul = 775,
+ rv_op_c_lbu = 776,
+ rv_op_c_lhu = 777,
+ rv_op_c_lh = 778,
+ rv_op_c_sb = 779,
+ rv_op_c_sh = 780,
+ rv_op_cm_push = 781,
+ rv_op_cm_pop = 782,
+ rv_op_cm_popret = 783,
+ rv_op_cm_popretz = 784,
+ rv_op_cm_mva01s = 785,
+ rv_op_cm_mvsa01 = 786,
+ rv_op_cm_jt = 787,
+ rv_op_cm_jalt = 788,
} rv_op;
/* structures */
@@ -958,6 +985,7 @@ typedef struct {
uint8_t rnum;
uint8_t vm;
uint32_t vzimm;
+ uint8_t rlist;
} rv_decode;
typedef struct {
@@ -1070,6 +1098,10 @@ static const char rv_vreg_name_sym[32][4] = {
#define rv_fmt_vd_vm "O\tDm"
#define rv_fmt_vsetvli "O\t0,1,v"
#define rv_fmt_vsetivli "O\t0,u,v"
+#define rv_fmt_rs1_rs2_zce_ldst "O\t2,i(1)"
+#define rv_fmt_push_rlist "O\tx,-i"
+#define rv_fmt_pop_rlist "O\tx,i"
+#define rv_fmt_zcmt_index "O\ti"
/* pseudo-instruction constraints */
@@ -2065,7 +2097,27 @@ const rv_opcode_data opcode_data[] = {
{ "vsext.vf8", rv_codec_v_r, rv_fmt_vd_vs2_vm, NULL, rv_op_vsext_vf8, rv_op_vsext_vf8, 0 },
{ "vsetvli", rv_codec_vsetvli, rv_fmt_vsetvli, NULL, rv_op_vsetvli, rv_op_vsetvli, 0 },
{ "vsetivli", rv_codec_vsetivli, rv_fmt_vsetivli, NULL, rv_op_vsetivli, rv_op_vsetivli, 0 },
- { "vsetvl", rv_codec_r, rv_fmt_rd_rs1_rs2, NULL, rv_op_vsetvl, rv_op_vsetvl, 0 }
+ { "vsetvl", rv_codec_r, rv_fmt_rd_rs1_rs2, NULL, rv_op_vsetvl, rv_op_vsetvl, 0 },
+ { "c.zext.b", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+ { "c.sext.b", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+ { "c.zext.h", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+ { "c.sext.h", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+ { "c.zext.w", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+ { "c.not", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+ { "c.mul", rv_codec_zcb_mul, rv_fmt_rd_rs2, NULL, 0, 0 },
+ { "c.lbu", rv_codec_zcb_lb, rv_fmt_rs1_rs2_zce_ldst, NULL, 0, 0, 0 },
+ { "c.lhu", rv_codec_zcb_lh, rv_fmt_rs1_rs2_zce_ldst, NULL, 0, 0, 0 },
+ { "c.lh", rv_codec_zcb_lh, rv_fmt_rs1_rs2_zce_ldst, NULL, 0, 0, 0 },
+ { "c.sb", rv_codec_zcb_lb, rv_fmt_rs1_rs2_zce_ldst, NULL, 0, 0, 0 },
+ { "c.sh", rv_codec_zcb_lh, rv_fmt_rs1_rs2_zce_ldst, NULL, 0, 0, 0 },
+ { "cm.push", rv_codec_zcmp_cm_pushpop, rv_fmt_push_rlist, NULL, 0, 0 },
+ { "cm.pop", rv_codec_zcmp_cm_pushpop, rv_fmt_pop_rlist, NULL, 0, 0 },
+ { "cm.popret", rv_codec_zcmp_cm_pushpop, rv_fmt_pop_rlist, NULL, 0, 0, 0 },
+ { "cm.popretz", rv_codec_zcmp_cm_pushpop, rv_fmt_pop_rlist, NULL, 0, 0 },
+ { "cm.mva01s", rv_codec_zcmp_cm_mv, rv_fmt_rd_rs2, NULL, 0, 0, 0 },
+ { "cm.mvsa01", rv_codec_zcmp_cm_mv, rv_fmt_rd_rs2, NULL, 0, 0, 0 },
+ { "cm.jt", rv_codec_zcmt_jt, rv_fmt_zcmt_index, NULL, 0 },
+ { "cm.jalt", rv_codec_zcmt_jt, rv_fmt_zcmt_index, NULL, 0 },
};
/* CSR names */
@@ -2084,6 +2136,7 @@ static const char *csr_name(int csrno)
case 0x000a: return "vxrm";
case 0x000f: return "vcsr";
case 0x0015: return "seed";
+ case 0x0017: return "jvt";
case 0x0040: return "uscratch";
case 0x0041: return "uepc";
case 0x0042: return "ucause";
@@ -2306,6 +2359,24 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
op = rv_op_c_ld;
}
break;
+ case 4:
+ switch ((inst >> 10) & 0b111) {
+ case 0: op = rv_op_c_lbu; break;
+ case 1:
+ if (((inst >> 6) & 1) == 0) {
+ op = rv_op_c_lhu;
+ } else {
+ op = rv_op_c_lh;
+ }
+ break;
+ case 2: op = rv_op_c_sb; break;
+ case 3:
+ if (((inst >> 6) & 1) == 0) {
+ op = rv_op_c_sh;
+ }
+ break;
+ }
+ break;
case 5:
if (isa == rv128) {
op = rv_op_c_sq;
@@ -2362,6 +2433,17 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
case 3: op = rv_op_c_and; break;
case 4: op = rv_op_c_subw; break;
case 5: op = rv_op_c_addw; break;
+ case 6: op = rv_op_c_mul; break;
+ case 7:
+ switch ((inst >> 2) & 0b111) {
+ case 0: op = rv_op_c_zext_b; break;
+ case 1: op = rv_op_c_sext_b; break;
+ case 2: op = rv_op_c_zext_h; break;
+ case 3: op = rv_op_c_sext_h; break;
+ case 4: op = rv_op_c_zext_w; break;
+ case 5: op = rv_op_c_not; break;
+ }
+ break;
}
break;
}
@@ -2417,6 +2499,46 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
op = rv_op_c_sqsp;
} else {
op = rv_op_c_fsdsp;
+ if (((inst >> 12) & 0b01)) {
+ switch ((inst >> 8) & 0b01111) {
+ case 8:
+ if (((inst >> 4) & 0b01111) >= 4) {
+ op = rv_op_cm_push;
+ }
+ break;
+ case 10:
+ if (((inst >> 4) & 0b01111) >= 4) {
+ op = rv_op_cm_pop;
+ }
+ break;
+ case 12:
+ if (((inst >> 4) & 0b01111) >= 4) {
+ op = rv_op_cm_popretz;
+ }
+ break;
+ case 14:
+ if (((inst >> 4) & 0b01111) >= 4) {
+ op = rv_op_cm_popret;
+ }
+ break;
+ }
+ } else {
+ switch ((inst >> 10) & 0b011) {
+ case 0:
+ if (((inst >> 2) & 0xFF) >= 32) {
+ op = rv_op_cm_jalt;
+ } else {
+ op = rv_op_cm_jt;
+ }
+ break;
+ case 3:
+ switch ((inst >> 5) & 0b011) {
+ case 1: op = rv_op_cm_mvsa01; break;
+ case 3: op = rv_op_cm_mva01s; break;
+ }
+ break;
+ }
+ }
}
break;
case 6: op = rv_op_c_swsp; break;
@@ -3661,6 +3783,21 @@ static uint32_t operand_crs2q(rv_inst inst)
return (inst << 59) >> 61;
}
+static uint32_t calculate_xreg(uint32_t sreg)
+{
+ return sreg < 2 ? sreg + 8 : sreg + 16;
+}
+
+static uint32_t operand_sreg1(rv_inst inst)
+{
+ return calculate_xreg((inst << 54) >> 61);
+}
+
+static uint32_t operand_sreg2(rv_inst inst)
+{
+ return calculate_xreg((inst << 59) >> 61);
+}
+
static uint32_t operand_crd(rv_inst inst)
{
return (inst << 52) >> 59;
@@ -3883,6 +4020,46 @@ static uint32_t operand_vm(rv_inst inst)
return (inst << 38) >> 63;
}
+static uint32_t operand_uimm_c_lb(rv_inst inst)
+{
+ return (((inst << 58) >> 63) << 1) |
+ ((inst << 57) >> 63);
+}
+
+static uint32_t operand_uimm_c_lh(rv_inst inst)
+{
+ return (((inst << 58) >> 63) << 1);
+}
+
+static uint32_t operand_zcmp_spimm(rv_inst inst)
+{
+ return ((inst << 60) >> 62) << 4;
+}
+
+static uint32_t operand_zcmp_rlist(rv_inst inst)
+{
+ return ((inst << 56) >> 60);
+}
+
+static uint32_t calculate_stack_adj(rv_isa isa, uint32_t rlist, uint32_t spimm)
+{
+ int xlen_bytes_log2 = isa == rv64 ? 3 : 2;
+ int regs = rlist == 15 ? 13 : rlist - 3;
+ uint32_t stack_adj_base = ROUND_UP(regs << xlen_bytes_log2, 16);
+ return stack_adj_base + spimm;
+}
+
+static uint32_t operand_zcmp_stack_adj(rv_inst inst, rv_isa isa)
+{
+ return calculate_stack_adj(isa, operand_zcmp_rlist(inst),
+ operand_zcmp_spimm(inst));
+}
+
+static uint32_t operand_tbl_index(rv_inst inst)
+{
+ return ((inst << 54) >> 56);
+}
+
/* decode operands */
static void decode_inst_operands(rv_decode *dec, rv_isa isa)
@@ -4199,6 +4376,34 @@ static void decode_inst_operands(rv_decode *dec, rv_isa isa)
dec->imm = operand_vimm(inst);
dec->vzimm = operand_vzimm10(inst);
break;
+ case rv_codec_zcb_lb:
+ dec->rs1 = operand_crs1q(inst) + 8;
+ dec->rs2 = operand_crs2q(inst) + 8;
+ dec->imm = operand_uimm_c_lb(inst);
+ break;
+ case rv_codec_zcb_lh:
+ dec->rs1 = operand_crs1q(inst) + 8;
+ dec->rs2 = operand_crs2q(inst) + 8;
+ dec->imm = operand_uimm_c_lh(inst);
+ break;
+ case rv_codec_zcb_ext:
+ dec->rd = operand_crs1q(inst) + 8;
+ break;
+ case rv_codec_zcb_mul:
+ dec->rd = operand_crs1rdq(inst) + 8;
+ dec->rs2 = operand_crs2q(inst) + 8;
+ break;
+ case rv_codec_zcmp_cm_pushpop:
+ dec->imm = operand_zcmp_stack_adj(inst, isa);
+ dec->rlist = operand_zcmp_rlist(inst);
+ break;
+ case rv_codec_zcmp_cm_mv:
+ dec->rd = operand_sreg1(inst);
+ dec->rs2 = operand_sreg2(inst);
+ break;
+ case rv_codec_zcmt_jt:
+ dec->imm = operand_tbl_index(inst);
+ break;
};
}
@@ -4358,6 +4563,9 @@ static void format_inst(char *buf, size_t buflen, size_t tab, rv_decode *dec)
case ')':
append(buf, ")", buflen);
break;
+ case '-':
+ append(buf, "-", buflen);
+ break;
case 'b':
snprintf(tmp, sizeof(tmp), "%d", dec->bs);
append(buf, tmp, buflen);
@@ -4541,6 +4749,24 @@ static void format_inst(char *buf, size_t buflen, size_t tab, rv_decode *dec)
append(buf, vma, buflen);
break;
}
+ case 'x': {
+ switch (dec->rlist) {
+ case 4:
+ snprintf(tmp, sizeof(tmp), "{ra}");
+ break;
+ case 5:
+ snprintf(tmp, sizeof(tmp), "{ra, s0}");
+ break;
+ case 15:
+ snprintf(tmp, sizeof(tmp), "{ra, s0-s11}");
+ break;
+ default:
+ snprintf(tmp, sizeof(tmp), "{ra, s0-s%d}", dec->rlist - 5);
+ break;
+ }
+ append(buf, tmp, buflen);
+ break;
+ }
default:
break;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v12 10/10] target/riscv: Add support for Zce
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
` (8 preceding siblings ...)
2023-03-07 8:14 ` [PATCH v12 09/10] disas/riscv.c: add disasm support for Zc* Weiwei Li
@ 2023-03-07 8:14 ` Weiwei Li
2023-04-05 4:15 ` Alistair Francis
2023-03-24 13:23 ` [PATCH v12 00/10] support subsets of code size reduction extension liweiwei
2023-04-05 4:17 ` Alistair Francis
11 siblings, 1 reply; 28+ messages in thread
From: Weiwei Li @ 2023-03-07 8:14 UTC (permalink / raw)
To: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
Add and expose property for Zce:
* Specifying Zce without F includes Zca, Zcb, Zcmp, Zcmt.
* Specifying Zce with F includes Zca, Zcb, Zcmp, Zcmt and Zcf.
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
---
target/riscv/cpu.c | 12 ++++++++++++
target/riscv/cpu.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d17ae942bd..3502d1e749 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -91,6 +91,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zcb, true, PRIV_VERSION_1_12_0, ext_zcb),
ISA_EXT_DATA_ENTRY(zcf, true, PRIV_VERSION_1_12_0, ext_zcf),
ISA_EXT_DATA_ENTRY(zcd, true, PRIV_VERSION_1_12_0, ext_zcd),
+ ISA_EXT_DATA_ENTRY(zce, true, PRIV_VERSION_1_12_0, ext_zce),
ISA_EXT_DATA_ENTRY(zcmp, true, PRIV_VERSION_1_12_0, ext_zcmp),
ISA_EXT_DATA_ENTRY(zcmt, true, PRIV_VERSION_1_12_0, ext_zcmt),
ISA_EXT_DATA_ENTRY(zba, true, PRIV_VERSION_1_12_0, ext_zba),
@@ -945,6 +946,16 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
}
}
+ if (cpu->cfg.ext_zce) {
+ cpu->cfg.ext_zca = true;
+ cpu->cfg.ext_zcb = true;
+ cpu->cfg.ext_zcmp = true;
+ cpu->cfg.ext_zcmt = true;
+ if (cpu->cfg.ext_f && env->misa_mxl_max == MXL_RV32) {
+ cpu->cfg.ext_zcf = true;
+ }
+ }
+
if (cpu->cfg.ext_c) {
cpu->cfg.ext_zca = true;
if (cpu->cfg.ext_f && env->misa_mxl_max == MXL_RV32) {
@@ -1501,6 +1512,7 @@ static Property riscv_cpu_extensions[] = {
DEFINE_PROP_BOOL("x-zca", RISCVCPU, cfg.ext_zca, false),
DEFINE_PROP_BOOL("x-zcb", RISCVCPU, cfg.ext_zcb, false),
DEFINE_PROP_BOOL("x-zcd", RISCVCPU, cfg.ext_zcd, false),
+ DEFINE_PROP_BOOL("x-zce", RISCVCPU, cfg.ext_zce, false),
DEFINE_PROP_BOOL("x-zcf", RISCVCPU, cfg.ext_zcf, false),
DEFINE_PROP_BOOL("x-zcmp", RISCVCPU, cfg.ext_zcmp, false),
DEFINE_PROP_BOOL("x-zcmt", RISCVCPU, cfg.ext_zcmt, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 351d5e3e79..9b76885166 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -443,6 +443,7 @@ struct RISCVCPUConfig {
bool ext_zca;
bool ext_zcb;
bool ext_zcd;
+ bool ext_zce;
bool ext_zcf;
bool ext_zcmp;
bool ext_zcmt;
--
2.25.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v12 00/10] support subsets of code size reduction extension
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
` (9 preceding siblings ...)
2023-03-07 8:14 ` [PATCH v12 10/10] target/riscv: Add support for Zce Weiwei Li
@ 2023-03-24 13:23 ` liweiwei
2023-04-05 4:17 ` Alistair Francis
11 siblings, 0 replies; 28+ messages in thread
From: liweiwei @ 2023-03-24 13:23 UTC (permalink / raw)
To: Weiwei Li, richard.henderson, palmer, alistair.francis, bin.meng,
qemu-riscv, qemu-devel, Daniel Henrique Barboza, LIU Zhiwei
Cc: wangjunqiang, lazyparser
Ping!
Several updates have been applied to the support of Zc* extensions after
v9 was dropped from the riscv-to-apply.next list.
Any new comments for them?
Regards,
Weiwei Li
On 2023/3/7 16:13, Weiwei Li wrote:
> This patchset implements RISC-V Zc* extension v1.0.3-1 version instructions.
>
> Specification:
> https://github.com/riscv/riscv-code-size-reduction/tree/main/Zc-specification
>
> The port is available here:
> https://github.com/plctlab/plct-qemu/tree/plct-zce-upstream-v12
>
> To test Zc* implementation, specify cpu argument with 'x-zca=true,x-zcb=true,x-zcf=true,f=true" and "x-zcd=true,d=true" (or "x-zcmp=true,x-zcmt=true" with c or d=false) to enable Zca/Zcb/Zcf and Zcd(or Zcmp,Zcmt) extensions support. We can also specify "x-zce=true,f=true" to enable Zca/Zcb/Zcmp/Zcmt and Zcf.
>
> This implementation can pass the basic zc tests from https://github.com/yulong-plct/zc-test
>
> v12
> * add patch 10 to support zce property
> * rebase on upstream master: reuse riscv_get_cfg() in patch 7 and remove tcg_temp_free in patch 6
>
> v11
> * update format and field name based on the latest spec in patch 5, 6, 7 (without other functional changes)
> * rebase on riscv-to-apply.next
>
> v10:
> * rebase on Daniel's series(riscv-to-apply.next) and adjust riscv-tests to test on sifive related CPUs
>
> v9:
> * rebase on riscv-to-apply.next
>
> v8:
> * improve disas support in Patch 9
>
> v7:
> * Fix description for Zca
>
> v6：
> * fix base address for jump table in Patch 7
> * rebase on riscv-to-apply.next
>
> v5:
> * fix exception unwind problem for cpu_ld*_code in helper of cm_jalt
>
> v4:
> * improve Zcmp suggested by Richard
> * fix stateen related check for Zcmt
>
> v3:
> * update the solution for Zcf to the way of Zcd
> * update Zcb to reuse gen_load/store
> * use trans function instead of helper for push/pop
>
> v2:
> * add check for relationship between Zca/Zcf/Zcd with C/F/D based on related discussion in review of Zc* spec
> * separate c.fld{sp}/fsd{sp} with fld{sp}/fsd{sp} before support of zcmp/zcmt
>
> Weiwei Li (10):
> target/riscv: add cfg properties for Zc* extension
> target/riscv: add support for Zca extension
> target/riscv: add support for Zcf extension
> target/riscv: add support for Zcd extension
> target/riscv: add support for Zcb extension
> target/riscv: add support for Zcmp extension
> target/riscv: add support for Zcmt extension
> target/riscv: expose properties for Zc* extension
> disas/riscv.c: add disasm support for Zc*
> target/riscv: Add support for Zce
>
> disas/riscv.c | 228 +++++++++++++++-
> target/riscv/cpu.c | 69 +++++
> target/riscv/cpu.h | 11 +
> target/riscv/cpu_bits.h | 7 +
> target/riscv/csr.c | 36 ++-
> target/riscv/helper.h | 3 +
> target/riscv/insn16.decode | 62 ++++-
> target/riscv/insn_trans/trans_rvd.c.inc | 18 ++
> target/riscv/insn_trans/trans_rvf.c.inc | 18 ++
> target/riscv/insn_trans/trans_rvi.c.inc | 4 +-
> target/riscv/insn_trans/trans_rvzce.c.inc | 311 ++++++++++++++++++++++
> target/riscv/machine.c | 19 ++
> target/riscv/meson.build | 3 +-
> target/riscv/translate.c | 15 +-
> target/riscv/zce_helper.c | 55 ++++
> 15 files changed, 843 insertions(+), 16 deletions(-)
> create mode 100644 target/riscv/insn_trans/trans_rvzce.c.inc
> create mode 100644 target/riscv/zce_helper.c
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 10/10] target/riscv: Add support for Zce
2023-03-07 8:14 ` [PATCH v12 10/10] target/riscv: Add support for Zce Weiwei Li
@ 2023-04-05 4:15 ` Alistair Francis
0 siblings, 0 replies; 28+ messages in thread
From: Alistair Francis @ 2023-04-05 4:15 UTC (permalink / raw)
To: Weiwei Li
Cc: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel, wangjunqiang, lazyparser
On Tue, Mar 7, 2023 at 6:16 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> Add and expose property for Zce:
> * Specifying Zce without F includes Zca, Zcb, Zcmp, Zcmt.
> * Specifying Zce with F includes Zca, Zcb, Zcmp, Zcmt and Zcf.
>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 12 ++++++++++++
> target/riscv/cpu.h | 1 +
> 2 files changed, 13 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index d17ae942bd..3502d1e749 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -91,6 +91,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(zcb, true, PRIV_VERSION_1_12_0, ext_zcb),
> ISA_EXT_DATA_ENTRY(zcf, true, PRIV_VERSION_1_12_0, ext_zcf),
> ISA_EXT_DATA_ENTRY(zcd, true, PRIV_VERSION_1_12_0, ext_zcd),
> + ISA_EXT_DATA_ENTRY(zce, true, PRIV_VERSION_1_12_0, ext_zce),
> ISA_EXT_DATA_ENTRY(zcmp, true, PRIV_VERSION_1_12_0, ext_zcmp),
> ISA_EXT_DATA_ENTRY(zcmt, true, PRIV_VERSION_1_12_0, ext_zcmt),
> ISA_EXT_DATA_ENTRY(zba, true, PRIV_VERSION_1_12_0, ext_zba),
> @@ -945,6 +946,16 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> }
> }
>
> + if (cpu->cfg.ext_zce) {
> + cpu->cfg.ext_zca = true;
> + cpu->cfg.ext_zcb = true;
> + cpu->cfg.ext_zcmp = true;
> + cpu->cfg.ext_zcmt = true;
> + if (cpu->cfg.ext_f && env->misa_mxl_max == MXL_RV32) {
> + cpu->cfg.ext_zcf = true;
> + }
> + }
> +
> if (cpu->cfg.ext_c) {
> cpu->cfg.ext_zca = true;
> if (cpu->cfg.ext_f && env->misa_mxl_max == MXL_RV32) {
> @@ -1501,6 +1512,7 @@ static Property riscv_cpu_extensions[] = {
> DEFINE_PROP_BOOL("x-zca", RISCVCPU, cfg.ext_zca, false),
> DEFINE_PROP_BOOL("x-zcb", RISCVCPU, cfg.ext_zcb, false),
> DEFINE_PROP_BOOL("x-zcd", RISCVCPU, cfg.ext_zcd, false),
> + DEFINE_PROP_BOOL("x-zce", RISCVCPU, cfg.ext_zce, false),
> DEFINE_PROP_BOOL("x-zcf", RISCVCPU, cfg.ext_zcf, false),
> DEFINE_PROP_BOOL("x-zcmp", RISCVCPU, cfg.ext_zcmp, false),
> DEFINE_PROP_BOOL("x-zcmt", RISCVCPU, cfg.ext_zcmt, false),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 351d5e3e79..9b76885166 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -443,6 +443,7 @@ struct RISCVCPUConfig {
> bool ext_zca;
> bool ext_zcb;
> bool ext_zcd;
> + bool ext_zce;
> bool ext_zcf;
> bool ext_zcmp;
> bool ext_zcmt;
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 00/10] support subsets of code size reduction extension
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
` (10 preceding siblings ...)
2023-03-24 13:23 ` [PATCH v12 00/10] support subsets of code size reduction extension liweiwei
@ 2023-04-05 4:17 ` Alistair Francis
11 siblings, 0 replies; 28+ messages in thread
From: Alistair Francis @ 2023-04-05 4:17 UTC (permalink / raw)
To: Weiwei Li
Cc: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel, wangjunqiang, lazyparser
On Tue, Mar 7, 2023 at 6:16 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> This patchset implements RISC-V Zc* extension v1.0.3-1 version instructions.
>
> Specification:
> https://github.com/riscv/riscv-code-size-reduction/tree/main/Zc-specification
>
> The port is available here:
> https://github.com/plctlab/plct-qemu/tree/plct-zce-upstream-v12
>
> To test Zc* implementation, specify cpu argument with 'x-zca=true,x-zcb=true,x-zcf=true,f=true" and "x-zcd=true,d=true" (or "x-zcmp=true,x-zcmt=true" with c or d=false) to enable Zca/Zcb/Zcf and Zcd(or Zcmp,Zcmt) extensions support. We can also specify "x-zce=true,f=true" to enable Zca/Zcb/Zcmp/Zcmt and Zcf.
>
> This implementation can pass the basic zc tests from https://github.com/yulong-plct/zc-test
>
> v12
> * add patch 10 to support zce property
> * rebase on upstream master: reuse riscv_get_cfg() in patch 7 and remove tcg_temp_free in patch 6
>
> v11
> * update format and field name based on the latest spec in patch 5, 6, 7 (without other functional changes)
> * rebase on riscv-to-apply.next
>
> v10:
> * rebase on Daniel's series(riscv-to-apply.next) and adjust riscv-tests to test on sifive related CPUs
>
> v9:
> * rebase on riscv-to-apply.next
>
> v8:
> * improve disas support in Patch 9
>
> v7:
> * Fix description for Zca
>
> v6：
> * fix base address for jump table in Patch 7
> * rebase on riscv-to-apply.next
>
> v5:
> * fix exception unwind problem for cpu_ld*_code in helper of cm_jalt
>
> v4:
> * improve Zcmp suggested by Richard
> * fix stateen related check for Zcmt
>
> v3:
> * update the solution for Zcf to the way of Zcd
> * update Zcb to reuse gen_load/store
> * use trans function instead of helper for push/pop
>
> v2:
> * add check for relationship between Zca/Zcf/Zcd with C/F/D based on related discussion in review of Zc* spec
> * separate c.fld{sp}/fsd{sp} with fld{sp}/fsd{sp} before support of zcmp/zcmt
>
> Weiwei Li (10):
> target/riscv: add cfg properties for Zc* extension
> target/riscv: add support for Zca extension
> target/riscv: add support for Zcf extension
> target/riscv: add support for Zcd extension
> target/riscv: add support for Zcb extension
> target/riscv: add support for Zcmp extension
> target/riscv: add support for Zcmt extension
> target/riscv: expose properties for Zc* extension
> disas/riscv.c: add disasm support for Zc*
> target/riscv: Add support for Zce
Thanks!
Applied to riscv-to-apply.next
Alistair
>
> disas/riscv.c | 228 +++++++++++++++-
> target/riscv/cpu.c | 69 +++++
> target/riscv/cpu.h | 11 +
> target/riscv/cpu_bits.h | 7 +
> target/riscv/csr.c | 36 ++-
> target/riscv/helper.h | 3 +
> target/riscv/insn16.decode | 62 ++++-
> target/riscv/insn_trans/trans_rvd.c.inc | 18 ++
> target/riscv/insn_trans/trans_rvf.c.inc | 18 ++
> target/riscv/insn_trans/trans_rvi.c.inc | 4 +-
> target/riscv/insn_trans/trans_rvzce.c.inc | 311 ++++++++++++++++++++++
> target/riscv/machine.c | 19 ++
> target/riscv/meson.build | 3 +-
> target/riscv/translate.c | 15 +-
> target/riscv/zce_helper.c | 55 ++++
> 15 files changed, 843 insertions(+), 16 deletions(-)
> create mode 100644 target/riscv/insn_trans/trans_rvzce.c.inc
> create mode 100644 target/riscv/zce_helper.c
>
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-03-07 8:13 ` [PATCH v12 02/10] target/riscv: add support for Zca extension Weiwei Li
@ 2023-04-06 20:22 ` Daniel Henrique Barboza
2023-04-07 1:14 ` liweiwei
2023-04-12 2:12 ` Alistair Francis
0 siblings, 2 replies; 28+ messages in thread
From: Daniel Henrique Barboza @ 2023-04-06 20:22 UTC (permalink / raw)
To: Weiwei Li, richard.henderson, palmer, alistair.francis, bin.meng,
qemu-riscv, qemu-devel
Cc: wangjunqiang, lazyparser, Wilfred Mallawa
Hi,
This patch is going to break the sifive_u boot if I rebase
"[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
on top of it, as it is the case today with the current riscv-to-apply.next.
The reason is that the priv spec version for Zca is marked as 1_12_0, and
the priv spec version for both sifive CPUs is 1_10_0, and both are enabling
RVC.
This patch from that series above:
"[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers"
Makes the disabling of the extension based on priv version to happen *after* we
do all the validations, instead of before as we're doing today. Zca (and Zcd) will
be manually enabled just to be disabled shortly after by the priv spec code. And
this will happen:
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
(--- hangs ---)
This means that the assumption made in this patch - that Zca implies RVC - is no
longer valid, and all these translations won't work.
Some possible solutions:
- Do not use Zca as a synonym for RVC, i.e. drop this patch. We would need to convert
all Zca checks to RVC checks in all translation code.
- Do not apply patch 5/9 from that series that moves the disable_ext code to the end
of validation. Also a possibility, but we would be sweeping the problem under the rug.
Zca still can't be used as a RVC replacement due to priv spec version constraints, but
we just won't disable Zca because we'll keep validating exts too early (which is the
problem that the patch addresses).
- change the priv spec of the sifive CPUs - and everyone that uses RVC - to 1_12_0. Not
sure if this makes sense.
- do not disable any extensions due to privilege spec version mismatch. This would make
all the priv_version related artifacts to be more "educational" than to be an actual
configuration we want to enforce. Not sure if that would do any good in the end, but
it's also a possibility.
I'll hold the rebase of that series until we sort this out. Thanks,
Daniel
On 3/7/23 05:13, Weiwei Li wrote:
> Modify the check for C extension to Zca (C implies Zca).
>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> ---
> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
> target/riscv/translate.c | 8 ++++++--
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
> index 4ad54e8a49..c70c495fc5 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>
> gen_set_pc(ctx, cpu_pc);
> - if (!has_ext(ctx, RVC)) {
> + if (!ctx->cfg_ptr->ext_zca) {
> TCGv t0 = tcg_temp_new();
>
> misaligned = gen_new_label();
> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
>
> gen_set_label(l); /* branch taken */
>
> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
> /* misaligned */
> gen_exception_inst_addr_mis(ctx);
> } else {
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 0ee8ee147d..d1fdd0c2d7 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
>
> /* check misaligned: */
> next_pc = ctx->base.pc_next + imm;
> - if (!has_ext(ctx, RVC)) {
> + if (!ctx->cfg_ptr->ext_zca) {
> if ((next_pc & 0x3) != 0) {
> gen_exception_inst_addr_mis(ctx);
> return;
> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
> if (insn_len(opcode) == 2) {
> ctx->opcode = opcode;
> ctx->pc_succ_insn = ctx->base.pc_next + 2;
> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
> + /*
> + * The Zca extension is added as way to refer to instructions in the C
> + * extension that do not include the floating-point loads and stores
> + */
> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
> return;
> }
> } else {
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-06 20:22 ` Daniel Henrique Barboza
@ 2023-04-07 1:14 ` liweiwei
2023-04-07 3:34 ` liweiwei
2023-04-07 10:28 ` Daniel Henrique Barboza
2023-04-12 2:12 ` Alistair Francis
1 sibling, 2 replies; 28+ messages in thread
From: liweiwei @ 2023-04-07 1:14 UTC (permalink / raw)
To: Daniel Henrique Barboza, richard.henderson, palmer,
alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: liweiwei, wangjunqiang, lazyparser, Wilfred Mallawa
On 2023/4/7 04:22, Daniel Henrique Barboza wrote:
> Hi,
>
> This patch is going to break the sifive_u boot if I rebase
>
> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
>
> on top of it, as it is the case today with the current
> riscv-to-apply.next.
>
> The reason is that the priv spec version for Zca is marked as 1_12_0, and
> the priv spec version for both sifive CPUs is 1_10_0, and both are
> enabling
> RVC.
>
> This patch from that series above:
>
> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec
> validate/disable_exts helpers"
>
> Makes the disabling of the extension based on priv version to happen
> *after* we
> do all the validations, instead of before as we're doing today. Zca
> (and Zcd) will
> be manually enabled just to be disabled shortly after by the priv spec
> code. And
> this will happen:
Yeah, I didn't take priv_version into consideration before.
This is a new problem if we disable them at the end and was not
triggered in my previous tests.
Not only Zca and Zcd, Zcf also has the same problem.
>
> qemu-system-riscv64: warning: disabling zca extension for hart
> 0x0000000000000000 because privilege spec version does not match
> qemu-system-riscv64: warning: disabling zca extension for hart
> 0x0000000000000001 because privilege spec version does not match
> qemu-system-riscv64: warning: disabling zcd extension for hart
> 0x0000000000000001 because privilege spec version does not match
> (--- hangs ---)
>
> This means that the assumption made in this patch - that Zca implies
> RVC - is no
> longer valid, and all these translations won't work.
>
As specified in Zc* spec, Zca is the subset of RVC. C & F include Zcf
in RV32. C & D include Zcd.
>
> Some possible solutions:
>
> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would
> need to convert
> all Zca checks to RVC checks in all translation code.
We should check both Zca and RVC in this way.
Similarly, we also should check both C&F and Zcf for Zcf instructions,
C&D and Zcd for Zcd instructions.
I can update this patchset or add a new patch for it if needed.
>
> - Do not apply patch 5/9 from that series that moves the disable_ext
> code to the end
> of validation. Also a possibility, but we would be sweeping the
> problem under the rug.
> Zca still can't be used as a RVC replacement due to priv spec version
> constraints, but
> we just won't disable Zca because we'll keep validating exts too early
> (which is the
> problem that the patch addresses).
>
> - change the priv spec of the sifive CPUs - and everyone that uses RVC
> - to 1_12_0. Not
> sure if this makes sense.
>
> - do not disable any extensions due to privilege spec version
> mismatch. This would make
> all the priv_version related artifacts to be more "educational" than
> to be an actual
> configuration we want to enforce. Not sure if that would do any good
> in the end, but
> it's also a possibility.
I prefer this way. For vendor-specific cpu types, the implicitly implied
extensions will have no effect on its function,
and this can be invisible to user if we mask them in isa_string exposed
to the kernel.
The question is whether we need constrain the configuration for general
cpu type.
Regards,
Weiwei Li
> I'll hold the rebase of that series until we sort this out. Thanks,
>
>
> Daniel
>
>
>
> On 3/7/23 05:13, Weiwei Li wrote:
>> Modify the check for C extension to Zca (C implies Zca).
>>
>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>> ---
>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>> target/riscv/translate.c | 8 ++++++--
>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
>> b/target/riscv/insn_trans/trans_rvi.c.inc
>> index 4ad54e8a49..c70c495fc5 100644
>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>> gen_set_pc(ctx, cpu_pc);
>> - if (!has_ext(ctx, RVC)) {
>> + if (!ctx->cfg_ptr->ext_zca) {
>> TCGv t0 = tcg_temp_new();
>> misaligned = gen_new_label();
>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b
>> *a, TCGCond cond)
>> gen_set_label(l); /* branch taken */
>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) &
>> 0x3)) {
>> /* misaligned */
>> gen_exception_inst_addr_mis(ctx);
>> } else {
>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>> index 0ee8ee147d..d1fdd0c2d7 100644
>> --- a/target/riscv/translate.c
>> +++ b/target/riscv/translate.c
>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd,
>> target_ulong imm)
>> /* check misaligned: */
>> next_pc = ctx->base.pc_next + imm;
>> - if (!has_ext(ctx, RVC)) {
>> + if (!ctx->cfg_ptr->ext_zca) {
>> if ((next_pc & 0x3) != 0) {
>> gen_exception_inst_addr_mis(ctx);
>> return;
>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env,
>> DisasContext *ctx, uint16_t opcode)
>> if (insn_len(opcode) == 2) {
>> ctx->opcode = opcode;
>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
>> + /*
>> + * The Zca extension is added as way to refer to
>> instructions in the C
>> + * extension that do not include the floating-point loads
>> and stores
>> + */
>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
>> return;
>> }
>> } else {
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-07 1:14 ` liweiwei
@ 2023-04-07 3:34 ` liweiwei
2023-04-07 19:25 ` Daniel Henrique Barboza
2023-04-07 10:28 ` Daniel Henrique Barboza
1 sibling, 1 reply; 28+ messages in thread
From: liweiwei @ 2023-04-07 3:34 UTC (permalink / raw)
To: Daniel Henrique Barboza, richard.henderson, palmer,
alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: liweiwei, wangjunqiang, lazyparser, Wilfred Mallawa
On 2023/4/7 09:14, liweiwei wrote:
>
> On 2023/4/7 04:22, Daniel Henrique Barboza wrote:
>> Hi,
>>
>> This patch is going to break the sifive_u boot if I rebase
>>
>> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
>>
>> on top of it, as it is the case today with the current
>> riscv-to-apply.next.
>>
>> The reason is that the priv spec version for Zca is marked as 1_12_0,
>> and
>> the priv spec version for both sifive CPUs is 1_10_0, and both are
>> enabling
>> RVC.
>>
>> This patch from that series above:
>>
>> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec
>> validate/disable_exts helpers"
>>
>> Makes the disabling of the extension based on priv version to happen
>> *after* we
>> do all the validations, instead of before as we're doing today. Zca
>> (and Zcd) will
>> be manually enabled just to be disabled shortly after by the priv
>> spec code. And
>> this will happen:
>
> Yeah, I didn't take priv_version into consideration before.
>
> This is a new problem if we disable them at the end and was not
> triggered in my previous tests.
>
> Not only Zca and Zcd, Zcf also has the same problem.
>
>>
>> qemu-system-riscv64: warning: disabling zca extension for hart
>> 0x0000000000000000 because privilege spec version does not match
>> qemu-system-riscv64: warning: disabling zca extension for hart
>> 0x0000000000000001 because privilege spec version does not match
>> qemu-system-riscv64: warning: disabling zcd extension for hart
>> 0x0000000000000001 because privilege spec version does not match
>> (--- hangs ---)
>>
>> This means that the assumption made in this patch - that Zca implies
>> RVC - is no
>> longer valid, and all these translations won't work.
>>
> As specified in Zc* spec, Zca is the subset of RVC. C & F include
> Zcf in RV32. C & D include Zcd.
>>
>> Some possible solutions:
>>
>> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would
>> need to convert
>> all Zca checks to RVC checks in all translation code.
>
> We should check both Zca and RVC in this way.
>
> Similarly, we also should check both C&F and Zcf for Zcf instructions,
> C&D and Zcd for Zcd instructions.
>
> I can update this patchset or add a new patch for it if needed.
>
>>
>> - Do not apply patch 5/9 from that series that moves the disable_ext
>> code to the end
>> of validation. Also a possibility, but we would be sweeping the
>> problem under the rug.
>> Zca still can't be used as a RVC replacement due to priv spec version
>> constraints, but
>> we just won't disable Zca because we'll keep validating exts too
>> early (which is the
>> problem that the patch addresses).
>>
>> - change the priv spec of the sifive CPUs - and everyone that uses
>> RVC - to 1_12_0. Not
>> sure if this makes sense.
>>
>> - do not disable any extensions due to privilege spec version
>> mismatch. This would make
>> all the priv_version related artifacts to be more "educational" than
>> to be an actual
>> configuration we want to enforce. Not sure if that would do any good
>> in the end, but
>> it's also a possibility.
>
> I prefer this way. For vendor-specific cpu types, the implicitly
> implied extensions will have no effect on its function,
>
> and this can be invisible to user if we mask them in isa_string
> exposed to the kernel.
>
> The question is whether we need constrain the configuration for
> general cpu type.
Subset extension for another extension is not a single case in RISC-V.
such as zaamo is subset of A. Zfhmin is subset of Zfh.
Maybe some of them don't have this problem. However, I think it's
better to take the related work away from the developer.
I think we can combine the two method if we want to constrain the
configuration for general cpu type:
- remain disable extensions due to privilege spec version mismatch
before validation to disable the extensions manually set by users
- mask the implicitly enabled extensions in isa_string to make them
invisible to users (I have sent a new patch to do this, you can pick it
into
your patchset if this way is acceptable).
Regards,
Weiwei Li
>
> Regards,
>
> Weiwei Li
>
>> I'll hold the rebase of that series until we sort this out. Thanks,
>>
>>
>> Daniel
>>
>>
>>
>> On 3/7/23 05:13, Weiwei Li wrote:
>>> Modify the check for C extension to Zca (C implies Zca).
>>>
>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>> ---
>>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>>> target/riscv/translate.c | 8 ++++++--
>>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
>>> b/target/riscv/insn_trans/trans_rvi.c.inc
>>> index 4ad54e8a49..c70c495fc5 100644
>>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
>>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
>>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr
>>> *a)
>>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>>> gen_set_pc(ctx, cpu_pc);
>>> - if (!has_ext(ctx, RVC)) {
>>> + if (!ctx->cfg_ptr->ext_zca) {
>>> TCGv t0 = tcg_temp_new();
>>> misaligned = gen_new_label();
>>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b
>>> *a, TCGCond cond)
>>> gen_set_label(l); /* branch taken */
>>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) &
>>> 0x3)) {
>>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) &
>>> 0x3)) {
>>> /* misaligned */
>>> gen_exception_inst_addr_mis(ctx);
>>> } else {
>>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>>> index 0ee8ee147d..d1fdd0c2d7 100644
>>> --- a/target/riscv/translate.c
>>> +++ b/target/riscv/translate.c
>>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd,
>>> target_ulong imm)
>>> /* check misaligned: */
>>> next_pc = ctx->base.pc_next + imm;
>>> - if (!has_ext(ctx, RVC)) {
>>> + if (!ctx->cfg_ptr->ext_zca) {
>>> if ((next_pc & 0x3) != 0) {
>>> gen_exception_inst_addr_mis(ctx);
>>> return;
>>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env,
>>> DisasContext *ctx, uint16_t opcode)
>>> if (insn_len(opcode) == 2) {
>>> ctx->opcode = opcode;
>>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
>>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
>>> + /*
>>> + * The Zca extension is added as way to refer to
>>> instructions in the C
>>> + * extension that do not include the floating-point loads
>>> and stores
>>> + */
>>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
>>> return;
>>> }
>>> } else {
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-07 1:14 ` liweiwei
2023-04-07 3:34 ` liweiwei
@ 2023-04-07 10:28 ` Daniel Henrique Barboza
2023-04-07 10:48 ` liweiwei
1 sibling, 1 reply; 28+ messages in thread
From: Daniel Henrique Barboza @ 2023-04-07 10:28 UTC (permalink / raw)
To: liweiwei, richard.henderson, palmer, alistair.francis, bin.meng,
qemu-riscv, qemu-devel
Cc: wangjunqiang, lazyparser, Wilfred Mallawa
On 4/6/23 22:14, liweiwei wrote:
>
> On 2023/4/7 04:22, Daniel Henrique Barboza wrote:
>> Hi,
>>
>> This patch is going to break the sifive_u boot if I rebase
>>
>> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
>>
>> on top of it, as it is the case today with the current riscv-to-apply.next.
>>
>> The reason is that the priv spec version for Zca is marked as 1_12_0, and
>> the priv spec version for both sifive CPUs is 1_10_0, and both are enabling
>> RVC.
>>
>> This patch from that series above:
>>
>> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers"
>>
>> Makes the disabling of the extension based on priv version to happen *after* we
>> do all the validations, instead of before as we're doing today. Zca (and Zcd) will
>> be manually enabled just to be disabled shortly after by the priv spec code. And
>> this will happen:
>
> Yeah, I didn't take priv_version into consideration before.
>
> This is a new problem if we disable them at the end and was not triggered in my previous tests.
>
> Not only Zca and Zcd, Zcf also has the same problem.
>
>>
>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
>> qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
>> (--- hangs ---)
>>
>> This means that the assumption made in this patch - that Zca implies RVC - is no
>> longer valid, and all these translations won't work.
>>
> As specified in Zc* spec, Zca is the subset of RVC. C & F include Zcf in RV32. C & D include Zcd.
>>
>> Some possible solutions:
>>
>> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would need to convert
>> all Zca checks to RVC checks in all translation code.
>
> We should check both Zca and RVC in this way.
>
> Similarly, we also should check both C&F and Zcf for Zcf instructions, C&D and Zcd for Zcd instructions.
>
> I can update this patchset or add a new patch for it if needed.
>
>>
>> - Do not apply patch 5/9 from that series that moves the disable_ext code to the end
>> of validation. Also a possibility, but we would be sweeping the problem under the rug.
>> Zca still can't be used as a RVC replacement due to priv spec version constraints, but
>> we just won't disable Zca because we'll keep validating exts too early (which is the
>> problem that the patch addresses).
>>
>> - change the priv spec of the sifive CPUs - and everyone that uses RVC - to 1_12_0. Not
>> sure if this makes sense.
>>
>> - do not disable any extensions due to privilege spec version mismatch. This would make
>> all the priv_version related artifacts to be more "educational" than to be an actual
>> configuration we want to enforce. Not sure if that would do any good in the end, but
>> it's also a possibility.
>
> I prefer this way. For vendor-specific cpu types, the implicitly implied extensions will have no effect on its function,
>
> and this can be invisible to user if we mask them in isa_string exposed to the kernel.
Problem is that, at least for now, we can't say whether a Z extension was enabled
by the user or by us. We'll end up masking user selection in the isa_string as
well.
>
> The question is whether we need constrain the configuration for general cpu type.
General CPU types aren't affected at all by these changes because they'll always run
with PRIV_VERSION_LATEST. This particular problem is something that affects only
named CPUs.
Thanks,
Daniel
>
> Regards,
>
> Weiwei Li
>
>> I'll hold the rebase of that series until we sort this out. Thanks,
>>
>>
>> Daniel
>>
>>
>>
>> On 3/7/23 05:13, Weiwei Li wrote:
>>> Modify the check for C extension to Zca (C implies Zca).
>>>
>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>> ---
>>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>>> target/riscv/translate.c | 8 ++++++--
>>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
>>> index 4ad54e8a49..c70c495fc5 100644
>>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
>>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
>>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
>>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>>> gen_set_pc(ctx, cpu_pc);
>>> - if (!has_ext(ctx, RVC)) {
>>> + if (!ctx->cfg_ptr->ext_zca) {
>>> TCGv t0 = tcg_temp_new();
>>> misaligned = gen_new_label();
>>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
>>> gen_set_label(l); /* branch taken */
>>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>> /* misaligned */
>>> gen_exception_inst_addr_mis(ctx);
>>> } else {
>>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>>> index 0ee8ee147d..d1fdd0c2d7 100644
>>> --- a/target/riscv/translate.c
>>> +++ b/target/riscv/translate.c
>>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
>>> /* check misaligned: */
>>> next_pc = ctx->base.pc_next + imm;
>>> - if (!has_ext(ctx, RVC)) {
>>> + if (!ctx->cfg_ptr->ext_zca) {
>>> if ((next_pc & 0x3) != 0) {
>>> gen_exception_inst_addr_mis(ctx);
>>> return;
>>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
>>> if (insn_len(opcode) == 2) {
>>> ctx->opcode = opcode;
>>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
>>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
>>> + /*
>>> + * The Zca extension is added as way to refer to instructions in the C
>>> + * extension that do not include the floating-point loads and stores
>>> + */
>>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
>>> return;
>>> }
>>> } else {
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-07 10:28 ` Daniel Henrique Barboza
@ 2023-04-07 10:48 ` liweiwei
0 siblings, 0 replies; 28+ messages in thread
From: liweiwei @ 2023-04-07 10:48 UTC (permalink / raw)
To: Daniel Henrique Barboza, liweiwei, richard.henderson, palmer,
alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, lazyparser, Wilfred Mallawa
On 2023/4/7 18:28, Daniel Henrique Barboza wrote:
>
>
> On 4/6/23 22:14, liweiwei wrote:
>>
>> On 2023/4/7 04:22, Daniel Henrique Barboza wrote:
>>> Hi,
>>>
>>> This patch is going to break the sifive_u boot if I rebase
>>>
>>> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
>>>
>>> on top of it, as it is the case today with the current
>>> riscv-to-apply.next.
>>>
>>> The reason is that the priv spec version for Zca is marked as
>>> 1_12_0, and
>>> the priv spec version for both sifive CPUs is 1_10_0, and both are
>>> enabling
>>> RVC.
>>>
>>> This patch from that series above:
>>>
>>> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec
>>> validate/disable_exts helpers"
>>>
>>> Makes the disabling of the extension based on priv version to happen
>>> *after* we
>>> do all the validations, instead of before as we're doing today. Zca
>>> (and Zcd) will
>>> be manually enabled just to be disabled shortly after by the priv
>>> spec code. And
>>> this will happen:
>>
>> Yeah, I didn't take priv_version into consideration before.
>>
>> This is a new problem if we disable them at the end and was not
>> triggered in my previous tests.
>>
>> Not only Zca and Zcd, Zcf also has the same problem.
>>
>>>
>>> qemu-system-riscv64: warning: disabling zca extension for hart
>>> 0x0000000000000000 because privilege spec version does not match
>>> qemu-system-riscv64: warning: disabling zca extension for hart
>>> 0x0000000000000001 because privilege spec version does not match
>>> qemu-system-riscv64: warning: disabling zcd extension for hart
>>> 0x0000000000000001 because privilege spec version does not match
>>> (--- hangs ---)
>>>
>>> This means that the assumption made in this patch - that Zca implies
>>> RVC - is no
>>> longer valid, and all these translations won't work.
>>>
>> As specified in Zc* spec, Zca is the subset of RVC. C & F include
>> Zcf in RV32. C & D include Zcd.
>>>
>>> Some possible solutions:
>>>
>>> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We
>>> would need to convert
>>> all Zca checks to RVC checks in all translation code.
>>
>> We should check both Zca and RVC in this way.
>>
>> Similarly, we also should check both C&F and Zcf for Zcf
>> instructions, C&D and Zcd for Zcd instructions.
>>
>> I can update this patchset or add a new patch for it if needed.
>>
>>>
>>> - Do not apply patch 5/9 from that series that moves the disable_ext
>>> code to the end
>>> of validation. Also a possibility, but we would be sweeping the
>>> problem under the rug.
>>> Zca still can't be used as a RVC replacement due to priv spec
>>> version constraints, but
>>> we just won't disable Zca because we'll keep validating exts too
>>> early (which is the
>>> problem that the patch addresses).
>>>
>>> - change the priv spec of the sifive CPUs - and everyone that uses
>>> RVC - to 1_12_0. Not
>>> sure if this makes sense.
>>>
>>> - do not disable any extensions due to privilege spec version
>>> mismatch. This would make
>>> all the priv_version related artifacts to be more "educational" than
>>> to be an actual
>>> configuration we want to enforce. Not sure if that would do any good
>>> in the end, but
>>> it's also a possibility.
>>
>> I prefer this way. For vendor-specific cpu types, the implicitly
>> implied extensions will have no effect on its function,
>>
>> and this can be invisible to user if we mask them in isa_string
>> exposed to the kernel.
>
> Problem is that, at least for now, we can't say whether a Z extension
> was enabled
> by the user or by us. We'll end up masking user selection in the
> isa_string as
> well.
No, for vendor-specific cpu, extension support is stable, and the Z*
extension related property isn't registered for them.
So they cannot be enabled by user.
>
>
>>
>> The question is whether we need constrain the configuration for
>> general cpu type.
>
> General CPU types aren't affected at all by these changes because
> they'll always run
> with PRIV_VERSION_LATEST. This particular problem is something that
> affects only
> named CPUs.
>
>
Yeah, the default priv version is PRIV_VERSION_LATEST, However User can
specify the priv_version they needs.
Regards,
Weiwei Li
> Thanks,
>
> Daniel
>
>>
>> Regards,
>>
>> Weiwei Li
>>
>>> I'll hold the rebase of that series until we sort this out. Thanks,
>>>
>>>
>>> Daniel
>>>
>>>
>>>
>>> On 3/7/23 05:13, Weiwei Li wrote:
>>>> Modify the check for C extension to Zca (C implies Zca).
>>>>
>>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>> ---
>>>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>>>> target/riscv/translate.c | 8 ++++++--
>>>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
>>>> b/target/riscv/insn_trans/trans_rvi.c.inc
>>>> index 4ad54e8a49..c70c495fc5 100644
>>>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
>>>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
>>>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx,
>>>> arg_jalr *a)
>>>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>>>> gen_set_pc(ctx, cpu_pc);
>>>> - if (!has_ext(ctx, RVC)) {
>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>> TCGv t0 = tcg_temp_new();
>>>> misaligned = gen_new_label();
>>>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b
>>>> *a, TCGCond cond)
>>>> gen_set_label(l); /* branch taken */
>>>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) &
>>>> 0x3)) {
>>>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) &
>>>> 0x3)) {
>>>> /* misaligned */
>>>> gen_exception_inst_addr_mis(ctx);
>>>> } else {
>>>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>>>> index 0ee8ee147d..d1fdd0c2d7 100644
>>>> --- a/target/riscv/translate.c
>>>> +++ b/target/riscv/translate.c
>>>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd,
>>>> target_ulong imm)
>>>> /* check misaligned: */
>>>> next_pc = ctx->base.pc_next + imm;
>>>> - if (!has_ext(ctx, RVC)) {
>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>> if ((next_pc & 0x3) != 0) {
>>>> gen_exception_inst_addr_mis(ctx);
>>>> return;
>>>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env,
>>>> DisasContext *ctx, uint16_t opcode)
>>>> if (insn_len(opcode) == 2) {
>>>> ctx->opcode = opcode;
>>>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
>>>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
>>>> + /*
>>>> + * The Zca extension is added as way to refer to
>>>> instructions in the C
>>>> + * extension that do not include the floating-point loads
>>>> and stores
>>>> + */
>>>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
>>>> return;
>>>> }
>>>> } else {
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-07 3:34 ` liweiwei
@ 2023-04-07 19:25 ` Daniel Henrique Barboza
2023-04-08 1:09 ` liweiwei
0 siblings, 1 reply; 28+ messages in thread
From: Daniel Henrique Barboza @ 2023-04-07 19:25 UTC (permalink / raw)
To: liweiwei, richard.henderson, palmer, alistair.francis, bin.meng,
qemu-riscv, qemu-devel
Cc: wangjunqiang, lazyparser, Wilfred Mallawa
On 4/7/23 00:34, liweiwei wrote:
>
> On 2023/4/7 09:14, liweiwei wrote:
>>
>> On 2023/4/7 04:22, Daniel Henrique Barboza wrote:
>>> Hi,
>>>
>>> This patch is going to break the sifive_u boot if I rebase
>>>
>>> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
>>>
>>> on top of it, as it is the case today with the current riscv-to-apply.next.
>>>
>>> The reason is that the priv spec version for Zca is marked as 1_12_0, and
>>> the priv spec version for both sifive CPUs is 1_10_0, and both are enabling
>>> RVC.
>>>
>>> This patch from that series above:
>>>
>>> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers"
>>>
>>> Makes the disabling of the extension based on priv version to happen *after* we
>>> do all the validations, instead of before as we're doing today. Zca (and Zcd) will
>>> be manually enabled just to be disabled shortly after by the priv spec code. And
>>> this will happen:
>>
>> Yeah, I didn't take priv_version into consideration before.
>>
>> This is a new problem if we disable them at the end and was not triggered in my previous tests.
>>
>> Not only Zca and Zcd, Zcf also has the same problem.
>>
>>>
>>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
>>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
>>> qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
>>> (--- hangs ---)
>>>
>>> This means that the assumption made in this patch - that Zca implies RVC - is no
>>> longer valid, and all these translations won't work.
>>>
>> As specified in Zc* spec, Zca is the subset of RVC. C & F include Zcf in RV32. C & D include Zcd.
>>>
>>> Some possible solutions:
>>>
>>> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would need to convert
>>> all Zca checks to RVC checks in all translation code.
>>
>> We should check both Zca and RVC in this way.
>>
>> Similarly, we also should check both C&F and Zcf for Zcf instructions, C&D and Zcd for Zcd instructions.
>>
>> I can update this patchset or add a new patch for it if needed.
>>
>>>
>>> - Do not apply patch 5/9 from that series that moves the disable_ext code to the end
>>> of validation. Also a possibility, but we would be sweeping the problem under the rug.
>>> Zca still can't be used as a RVC replacement due to priv spec version constraints, but
>>> we just won't disable Zca because we'll keep validating exts too early (which is the
>>> problem that the patch addresses).
>>>
>>> - change the priv spec of the sifive CPUs - and everyone that uses RVC - to 1_12_0. Not
>>> sure if this makes sense.
>>>
>>> - do not disable any extensions due to privilege spec version mismatch. This would make
>>> all the priv_version related artifacts to be more "educational" than to be an actual
>>> configuration we want to enforce. Not sure if that would do any good in the end, but
>>> it's also a possibility.
>>
>> I prefer this way. For vendor-specific cpu types, the implicitly implied extensions will have no effect on its function,
>>
>> and this can be invisible to user if we mask them in isa_string exposed to the kernel.
>>
>> The question is whether we need constrain the configuration for general cpu type.
>
> Subset extension for another extension is not a single case in RISC-V. such as zaamo is subset of A. Zfhmin is subset of Zfh.
>
> Maybe some of them don't have this problem. However, I think it's better to take the related work away from the developer.
>
> I think we can combine the two method if we want to constrain the configuration for general cpu type:
>
> - remain disable extensions due to privilege spec version mismatch before validation to disable the extensions manually set by users
>
> - mask the implicitly enabled extensions in isa_string to make them invisible to users (I have sent a new patch to do this, you can pick it into
>
> your patchset if this way is acceptable).
I tested that patch with my series. If we keep the disable extension code to be executed
before the validation, filtering the extensions that were user enabled only, it fixes
the problem I reported here.
It's worth noticing though that we would be making the intended, conscious decision of
hiding extensions from the isa_string that are actually enabled in the hart. And CPUs
such as SIFIVE_CPU will start working with Z extensions that are beyond their declared
priv spec. This wouldn't be a problem if we could guarantee that userland would always
read 'isa_string' before using an extension, but in reality we can't guarantee that.
Firing an instruction for a given extension and capturing SIGILL to see if the hart supports
it or not isn't forbidden by the ISA.
All this said, I don't mind the proposed solution, as long as we're on the same boat
w.r.t. the design changes and potential userspace impact this might have.
Thanks,
Daniel
>
> Regards,
>
> Weiwei Li
>
>>
>> Regards,
>>
>> Weiwei Li
>>
>>> I'll hold the rebase of that series until we sort this out. Thanks,
>>>
>>>
>>> Daniel
>>>
>>>
>>>
>>> On 3/7/23 05:13, Weiwei Li wrote:
>>>> Modify the check for C extension to Zca (C implies Zca).
>>>>
>>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>> ---
>>>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>>>> target/riscv/translate.c | 8 ++++++--
>>>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
>>>> index 4ad54e8a49..c70c495fc5 100644
>>>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
>>>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
>>>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
>>>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>>>> gen_set_pc(ctx, cpu_pc);
>>>> - if (!has_ext(ctx, RVC)) {
>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>> TCGv t0 = tcg_temp_new();
>>>> misaligned = gen_new_label();
>>>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
>>>> gen_set_label(l); /* branch taken */
>>>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>>> /* misaligned */
>>>> gen_exception_inst_addr_mis(ctx);
>>>> } else {
>>>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>>>> index 0ee8ee147d..d1fdd0c2d7 100644
>>>> --- a/target/riscv/translate.c
>>>> +++ b/target/riscv/translate.c
>>>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
>>>> /* check misaligned: */
>>>> next_pc = ctx->base.pc_next + imm;
>>>> - if (!has_ext(ctx, RVC)) {
>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>> if ((next_pc & 0x3) != 0) {
>>>> gen_exception_inst_addr_mis(ctx);
>>>> return;
>>>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
>>>> if (insn_len(opcode) == 2) {
>>>> ctx->opcode = opcode;
>>>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
>>>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
>>>> + /*
>>>> + * The Zca extension is added as way to refer to instructions in the C
>>>> + * extension that do not include the floating-point loads and stores
>>>> + */
>>>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
>>>> return;
>>>> }
>>>> } else {
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-07 19:25 ` Daniel Henrique Barboza
@ 2023-04-08 1:09 ` liweiwei
0 siblings, 0 replies; 28+ messages in thread
From: liweiwei @ 2023-04-08 1:09 UTC (permalink / raw)
To: Daniel Henrique Barboza, richard.henderson, palmer,
alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: liweiwei, wangjunqiang, lazyparser, Wilfred Mallawa
On 2023/4/8 03:25, Daniel Henrique Barboza wrote:
>
>
> On 4/7/23 00:34, liweiwei wrote:
>>
>> On 2023/4/7 09:14, liweiwei wrote:
>>>
>>> On 2023/4/7 04:22, Daniel Henrique Barboza wrote:
>>>> Hi,
>>>>
>>>> This patch is going to break the sifive_u boot if I rebase
>>>>
>>>> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
>>>>
>>>> on top of it, as it is the case today with the current
>>>> riscv-to-apply.next.
>>>>
>>>> The reason is that the priv spec version for Zca is marked as
>>>> 1_12_0, and
>>>> the priv spec version for both sifive CPUs is 1_10_0, and both are
>>>> enabling
>>>> RVC.
>>>>
>>>> This patch from that series above:
>>>>
>>>> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec
>>>> validate/disable_exts helpers"
>>>>
>>>> Makes the disabling of the extension based on priv version to
>>>> happen *after* we
>>>> do all the validations, instead of before as we're doing today. Zca
>>>> (and Zcd) will
>>>> be manually enabled just to be disabled shortly after by the priv
>>>> spec code. And
>>>> this will happen:
>>>
>>> Yeah, I didn't take priv_version into consideration before.
>>>
>>> This is a new problem if we disable them at the end and was not
>>> triggered in my previous tests.
>>>
>>> Not only Zca and Zcd, Zcf also has the same problem.
>>>
>>>>
>>>> qemu-system-riscv64: warning: disabling zca extension for hart
>>>> 0x0000000000000000 because privilege spec version does not match
>>>> qemu-system-riscv64: warning: disabling zca extension for hart
>>>> 0x0000000000000001 because privilege spec version does not match
>>>> qemu-system-riscv64: warning: disabling zcd extension for hart
>>>> 0x0000000000000001 because privilege spec version does not match
>>>> (--- hangs ---)
>>>>
>>>> This means that the assumption made in this patch - that Zca
>>>> implies RVC - is no
>>>> longer valid, and all these translations won't work.
>>>>
>>> As specified in Zc* spec, Zca is the subset of RVC. C & F include
>>> Zcf in RV32. C & D include Zcd.
>>>>
>>>> Some possible solutions:
>>>>
>>>> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We
>>>> would need to convert
>>>> all Zca checks to RVC checks in all translation code.
>>>
>>> We should check both Zca and RVC in this way.
>>>
>>> Similarly, we also should check both C&F and Zcf for Zcf
>>> instructions, C&D and Zcd for Zcd instructions.
>>>
>>> I can update this patchset or add a new patch for it if needed.
>>>
>>>>
>>>> - Do not apply patch 5/9 from that series that moves the
>>>> disable_ext code to the end
>>>> of validation. Also a possibility, but we would be sweeping the
>>>> problem under the rug.
>>>> Zca still can't be used as a RVC replacement due to priv spec
>>>> version constraints, but
>>>> we just won't disable Zca because we'll keep validating exts too
>>>> early (which is the
>>>> problem that the patch addresses).
>>>>
>>>> - change the priv spec of the sifive CPUs - and everyone that uses
>>>> RVC - to 1_12_0. Not
>>>> sure if this makes sense.
>>>>
>>>> - do not disable any extensions due to privilege spec version
>>>> mismatch. This would make
>>>> all the priv_version related artifacts to be more "educational"
>>>> than to be an actual
>>>> configuration we want to enforce. Not sure if that would do any
>>>> good in the end, but
>>>> it's also a possibility.
>>>
>>> I prefer this way. For vendor-specific cpu types, the implicitly
>>> implied extensions will have no effect on its function,
>>>
>>> and this can be invisible to user if we mask them in isa_string
>>> exposed to the kernel.
>>>
>>> The question is whether we need constrain the configuration for
>>> general cpu type.
>>
>> Subset extension for another extension is not a single case in
>> RISC-V. such as zaamo is subset of A. Zfhmin is subset of Zfh.
>>
>> Maybe some of them don't have this problem. However, I think it's
>> better to take the related work away from the developer.
>>
>> I think we can combine the two method if we want to constrain the
>> configuration for general cpu type:
>>
>> - remain disable extensions due to privilege spec version mismatch
>> before validation to disable the extensions manually set by users
>>
>> - mask the implicitly enabled extensions in isa_string to make them
>> invisible to users (I have sent a new patch to do this, you can pick
>> it into
>>
>> your patchset if this way is acceptable).
>
> I tested that patch with my series. If we keep the disable extension
> code to be executed
> before the validation, filtering the extensions that were user enabled
> only, it fixes
> the problem I reported here.
>
> It's worth noticing though that we would be making the intended,
> conscious decision of
> hiding extensions from the isa_string that are actually enabled in the
> hart. And CPUs
> such as SIFIVE_CPU will start working with Z extensions that are
> beyond their declared
> priv spec. This wouldn't be a problem if we could guarantee that
> userland would always
> read 'isa_string' before using an extension, but in reality we can't
> guarantee that.
> Firing an instruction for a given extension and capturing SIGILL to
> see if the hart supports
> it or not isn't forbidden by the ISA.
The implicitly enabled extensions are mostly subset of its super
extension, except zfinx (I think it's visible to user,
and we can change it to check zdinx/zhinx{min} requires it). So
enabling them when their super subset are enabled will
introduce no additional function to the cpu. So it's just another
internal implementation(C is replaced by Zca, Zcf, Zcd)
for SIFIVE_CPU without real function change (neither increase nor
decrease).
Implicitly enabled extensions may introduce new problem for write_misa:
C/V function cannot be disabled by it
(I have described before). However, this is another problem unrelated
to priv version.
Regards,
Weiwei Li
>
>
> All this said, I don't mind the proposed solution, as long as we're on
> the same boat
> w.r.t. the design changes and potential userspace impact this might have.
>
>
> Thanks,
>
>
> Daniel
>
>
>>
>> Regards,
>>
>> Weiwei Li
>>
>>>
>>> Regards,
>>>
>>> Weiwei Li
>>>
>>>> I'll hold the rebase of that series until we sort this out. Thanks,
>>>>
>>>>
>>>> Daniel
>>>>
>>>>
>>>>
>>>> On 3/7/23 05:13, Weiwei Li wrote:
>>>>> Modify the check for C extension to Zca (C implies Zca).
>>>>>
>>>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>> ---
>>>>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>>>>> target/riscv/translate.c | 8 ++++++--
>>>>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
>>>>> b/target/riscv/insn_trans/trans_rvi.c.inc
>>>>> index 4ad54e8a49..c70c495fc5 100644
>>>>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
>>>>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
>>>>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx,
>>>>> arg_jalr *a)
>>>>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>>>>> gen_set_pc(ctx, cpu_pc);
>>>>> - if (!has_ext(ctx, RVC)) {
>>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>>> TCGv t0 = tcg_temp_new();
>>>>> misaligned = gen_new_label();
>>>>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx,
>>>>> arg_b *a, TCGCond cond)
>>>>> gen_set_label(l); /* branch taken */
>>>>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) &
>>>>> 0x3)) {
>>>>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) &
>>>>> 0x3)) {
>>>>> /* misaligned */
>>>>> gen_exception_inst_addr_mis(ctx);
>>>>> } else {
>>>>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>>>>> index 0ee8ee147d..d1fdd0c2d7 100644
>>>>> --- a/target/riscv/translate.c
>>>>> +++ b/target/riscv/translate.c
>>>>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd,
>>>>> target_ulong imm)
>>>>> /* check misaligned: */
>>>>> next_pc = ctx->base.pc_next + imm;
>>>>> - if (!has_ext(ctx, RVC)) {
>>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>>> if ((next_pc & 0x3) != 0) {
>>>>> gen_exception_inst_addr_mis(ctx);
>>>>> return;
>>>>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env,
>>>>> DisasContext *ctx, uint16_t opcode)
>>>>> if (insn_len(opcode) == 2) {
>>>>> ctx->opcode = opcode;
>>>>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
>>>>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
>>>>> + /*
>>>>> + * The Zca extension is added as way to refer to
>>>>> instructions in the C
>>>>> + * extension that do not include the floating-point loads
>>>>> and stores
>>>>> + */
>>>>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
>>>>> return;
>>>>> }
>>>>> } else {
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-06 20:22 ` Daniel Henrique Barboza
2023-04-07 1:14 ` liweiwei
@ 2023-04-12 2:12 ` Alistair Francis
2023-04-12 2:55 ` Weiwei Li
1 sibling, 1 reply; 28+ messages in thread
From: Alistair Francis @ 2023-04-12 2:12 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Weiwei Li, richard.henderson, palmer, alistair.francis, bin.meng,
qemu-riscv, qemu-devel, wangjunqiang, lazyparser, Wilfred Mallawa
On Fri, Apr 7, 2023 at 6:23 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Hi,
>
> This patch is going to break the sifive_u boot if I rebase
>
> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
>
> on top of it, as it is the case today with the current riscv-to-apply.next.
>
> The reason is that the priv spec version for Zca is marked as 1_12_0, and
> the priv spec version for both sifive CPUs is 1_10_0, and both are enabling
> RVC.
>
> This patch from that series above:
>
> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers"
>
> Makes the disabling of the extension based on priv version to happen *after* we
> do all the validations, instead of before as we're doing today. Zca (and Zcd) will
> be manually enabled just to be disabled shortly after by the priv spec code. And
> this will happen:
>
> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
> qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
> (--- hangs ---)
>
> This means that the assumption made in this patch - that Zca implies RVC - is no
> longer valid, and all these translations won't work.
Thanks for catching and reporting this
>
>
> Some possible solutions:
>
> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would need to convert
> all Zca checks to RVC checks in all translation code.
This to me seems like the best solution
>
> - Do not apply patch 5/9 from that series that moves the disable_ext code to the end
> of validation. Also a possibility, but we would be sweeping the problem under the rug.
> Zca still can't be used as a RVC replacement due to priv spec version constraints, but
> we just won't disable Zca because we'll keep validating exts too early (which is the
> problem that the patch addresses).
>
> - change the priv spec of the sifive CPUs - and everyone that uses RVC - to 1_12_0. Not
> sure if this makes sense.
>
> - do not disable any extensions due to privilege spec version mismatch. This would make
> all the priv_version related artifacts to be more "educational" than to be an actual
> configuration we want to enforce. Not sure if that would do any good in the end, but
> it's also a possibility.
This also seems like something we can do. Printing a warning but
continuing on seems reasonable to me. That allows users to
enable/disable features even if the versions don't match.
I don't think this is the solution for this problem though
Alistair
>
>
> I'll hold the rebase of that series until we sort this out. Thanks,
>
>
> Daniel
>
>
>
> On 3/7/23 05:13, Weiwei Li wrote:
> > Modify the check for C extension to Zca (C implies Zca).
> >
> > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> > ---
> > target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
> > target/riscv/translate.c | 8 ++++++--
> > 2 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
> > index 4ad54e8a49..c70c495fc5 100644
> > --- a/target/riscv/insn_trans/trans_rvi.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> > @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
> > tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
> >
> > gen_set_pc(ctx, cpu_pc);
> > - if (!has_ext(ctx, RVC)) {
> > + if (!ctx->cfg_ptr->ext_zca) {
> > TCGv t0 = tcg_temp_new();
> >
> > misaligned = gen_new_label();
> > @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
> >
> > gen_set_label(l); /* branch taken */
> >
> > - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
> > + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
> > /* misaligned */
> > gen_exception_inst_addr_mis(ctx);
> > } else {
> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > index 0ee8ee147d..d1fdd0c2d7 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
> >
> > /* check misaligned: */
> > next_pc = ctx->base.pc_next + imm;
> > - if (!has_ext(ctx, RVC)) {
> > + if (!ctx->cfg_ptr->ext_zca) {
> > if ((next_pc & 0x3) != 0) {
> > gen_exception_inst_addr_mis(ctx);
> > return;
> > @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
> > if (insn_len(opcode) == 2) {
> > ctx->opcode = opcode;
> > ctx->pc_succ_insn = ctx->base.pc_next + 2;
> > - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
> > + /*
> > + * The Zca extension is added as way to refer to instructions in the C
> > + * extension that do not include the floating-point loads and stores
> > + */
> > + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
> > return;
> > }
> > } else {
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-12 2:12 ` Alistair Francis
@ 2023-04-12 2:55 ` Weiwei Li
2023-04-12 10:55 ` Alistair Francis
0 siblings, 1 reply; 28+ messages in thread
From: Weiwei Li @ 2023-04-12 2:55 UTC (permalink / raw)
To: Alistair Francis, Daniel Henrique Barboza
Cc: liweiwei, richard.henderson, palmer, alistair.francis, bin.meng,
qemu-riscv, qemu-devel, wangjunqiang, lazyparser, Wilfred Mallawa
On 2023/4/12 10:12, Alistair Francis wrote:
> On Fri, Apr 7, 2023 at 6:23 AM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>> Hi,
>>
>> This patch is going to break the sifive_u boot if I rebase
>>
>> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
>>
>> on top of it, as it is the case today with the current riscv-to-apply.next.
>>
>> The reason is that the priv spec version for Zca is marked as 1_12_0, and
>> the priv spec version for both sifive CPUs is 1_10_0, and both are enabling
>> RVC.
>>
>> This patch from that series above:
>>
>> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers"
>>
>> Makes the disabling of the extension based on priv version to happen *after* we
>> do all the validations, instead of before as we're doing today. Zca (and Zcd) will
>> be manually enabled just to be disabled shortly after by the priv spec code. And
>> this will happen:
>>
>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
>> qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
>> (--- hangs ---)
>>
>> This means that the assumption made in this patch - that Zca implies RVC - is no
>> longer valid, and all these translations won't work.
> Thanks for catching and reporting this
>
>>
>> Some possible solutions:
>>
>> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would need to convert
>> all Zca checks to RVC checks in all translation code.
> This to me seems like the best solution
I had also implemented a patch for this solution. I'll sent it later.
However, this will introduce additional check. i.e. check both Zca and C
or , both Zcf/d and CF/CD.
I think this is not very necessary. Implcitly-enabled extensions is
often the subsets of existed extension.
So enabling them will introduce no additional function to the cpus.
We can just make them invisible to user(mask them in the isa string)
instead of disabling them to be compatible with lower priv version.
Regards,
Weiwei Li
>
>> - Do not apply patch 5/9 from that series that moves the disable_ext code to the end
>> of validation. Also a possibility, but we would be sweeping the problem under the rug.
>> Zca still can't be used as a RVC replacement due to priv spec version constraints, but
>> we just won't disable Zca because we'll keep validating exts too early (which is the
>> problem that the patch addresses).
>>
>> - change the priv spec of the sifive CPUs - and everyone that uses RVC - to 1_12_0. Not
>> sure if this makes sense.
>>
>> - do not disable any extensions due to privilege spec version mismatch. This would make
>> all the priv_version related artifacts to be more "educational" than to be an actual
>> configuration we want to enforce. Not sure if that would do any good in the end, but
>> it's also a possibility.
> This also seems like something we can do. Printing a warning but
> continuing on seems reasonable to me. That allows users to
> enable/disable features even if the versions don't match.
>
> I don't think this is the solution for this problem though
>
> Alistair
>
>>
>> I'll hold the rebase of that series until we sort this out. Thanks,
>>
>>
>> Daniel
>>
>>
>>
>> On 3/7/23 05:13, Weiwei Li wrote:
>>> Modify the check for C extension to Zca (C implies Zca).
>>>
>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>> ---
>>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>>> target/riscv/translate.c | 8 ++++++--
>>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
>>> index 4ad54e8a49..c70c495fc5 100644
>>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
>>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
>>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
>>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>>>
>>> gen_set_pc(ctx, cpu_pc);
>>> - if (!has_ext(ctx, RVC)) {
>>> + if (!ctx->cfg_ptr->ext_zca) {
>>> TCGv t0 = tcg_temp_new();
>>>
>>> misaligned = gen_new_label();
>>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
>>>
>>> gen_set_label(l); /* branch taken */
>>>
>>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>> /* misaligned */
>>> gen_exception_inst_addr_mis(ctx);
>>> } else {
>>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>>> index 0ee8ee147d..d1fdd0c2d7 100644
>>> --- a/target/riscv/translate.c
>>> +++ b/target/riscv/translate.c
>>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
>>>
>>> /* check misaligned: */
>>> next_pc = ctx->base.pc_next + imm;
>>> - if (!has_ext(ctx, RVC)) {
>>> + if (!ctx->cfg_ptr->ext_zca) {
>>> if ((next_pc & 0x3) != 0) {
>>> gen_exception_inst_addr_mis(ctx);
>>> return;
>>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
>>> if (insn_len(opcode) == 2) {
>>> ctx->opcode = opcode;
>>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
>>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
>>> + /*
>>> + * The Zca extension is added as way to refer to instructions in the C
>>> + * extension that do not include the floating-point loads and stores
>>> + */
>>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
>>> return;
>>> }
>>> } else {
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-12 2:55 ` Weiwei Li
@ 2023-04-12 10:55 ` Alistair Francis
2023-04-12 11:35 ` Weiwei Li
0 siblings, 1 reply; 28+ messages in thread
From: Alistair Francis @ 2023-04-12 10:55 UTC (permalink / raw)
To: Weiwei Li
Cc: Daniel Henrique Barboza, richard.henderson, palmer,
alistair.francis, bin.meng, qemu-riscv, qemu-devel, wangjunqiang,
lazyparser, Wilfred Mallawa
On Wed, Apr 12, 2023 at 12:55 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
>
> On 2023/4/12 10:12, Alistair Francis wrote:
> > On Fri, Apr 7, 2023 at 6:23 AM Daniel Henrique Barboza
> > <dbarboza@ventanamicro.com> wrote:
> >> Hi,
> >>
> >> This patch is going to break the sifive_u boot if I rebase
> >>
> >> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
> >>
> >> on top of it, as it is the case today with the current riscv-to-apply.next.
> >>
> >> The reason is that the priv spec version for Zca is marked as 1_12_0, and
> >> the priv spec version for both sifive CPUs is 1_10_0, and both are enabling
> >> RVC.
> >>
> >> This patch from that series above:
> >>
> >> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers"
> >>
> >> Makes the disabling of the extension based on priv version to happen *after* we
> >> do all the validations, instead of before as we're doing today. Zca (and Zcd) will
> >> be manually enabled just to be disabled shortly after by the priv spec code. And
> >> this will happen:
> >>
> >> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
> >> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
> >> qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
> >> (--- hangs ---)
> >>
> >> This means that the assumption made in this patch - that Zca implies RVC - is no
> >> longer valid, and all these translations won't work.
> > Thanks for catching and reporting this
> >
> >>
> >> Some possible solutions:
> >>
> >> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would need to convert
> >> all Zca checks to RVC checks in all translation code.
> > This to me seems like the best solution
>
> I had also implemented a patch for this solution. I'll sent it later.
Thanks!
>
> However, this will introduce additional check. i.e. check both Zca and C
> or , both Zcf/d and CF/CD.
Is there a large performance penalty for that?
>
> I think this is not very necessary. Implcitly-enabled extensions is
> often the subsets of existed extension.
Yeah, I see what you are saying. It just becomes difficult to manage
though. It all worked fine until there are conflicts between the priv
specs.
>
> So enabling them will introduce no additional function to the cpus.
>
> We can just make them invisible to user(mask them in the isa string)
> instead of disabling them to be compatible with lower priv version.
I'm open to other options, but masking them like this seems like more
work and also seems confusing. The extension will end up enabled in
QEMU and potentially visible to external tools, but then just not
exposed to the guest. It seems prone to future bugs.
Alistair
>
> Regards,
>
> Weiwei Li
>
>
> >
> >> - Do not apply patch 5/9 from that series that moves the disable_ext code to the end
> >> of validation. Also a possibility, but we would be sweeping the problem under the rug.
> >> Zca still can't be used as a RVC replacement due to priv spec version constraints, but
> >> we just won't disable Zca because we'll keep validating exts too early (which is the
> >> problem that the patch addresses).
> >>
> >> - change the priv spec of the sifive CPUs - and everyone that uses RVC - to 1_12_0. Not
> >> sure if this makes sense.
> >>
> >> - do not disable any extensions due to privilege spec version mismatch. This would make
> >> all the priv_version related artifacts to be more "educational" than to be an actual
> >> configuration we want to enforce. Not sure if that would do any good in the end, but
> >> it's also a possibility.
> > This also seems like something we can do. Printing a warning but
> > continuing on seems reasonable to me. That allows users to
> > enable/disable features even if the versions don't match.
> >
> > I don't think this is the solution for this problem though
> >
> > Alistair
> >
> >>
> >> I'll hold the rebase of that series until we sort this out. Thanks,
> >>
> >>
> >> Daniel
> >>
> >>
> >>
> >> On 3/7/23 05:13, Weiwei Li wrote:
> >>> Modify the check for C extension to Zca (C implies Zca).
> >>>
> >>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> >>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> >>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> >>> ---
> >>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
> >>> target/riscv/translate.c | 8 ++++++--
> >>> 2 files changed, 8 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
> >>> index 4ad54e8a49..c70c495fc5 100644
> >>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> >>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> >>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
> >>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
> >>>
> >>> gen_set_pc(ctx, cpu_pc);
> >>> - if (!has_ext(ctx, RVC)) {
> >>> + if (!ctx->cfg_ptr->ext_zca) {
> >>> TCGv t0 = tcg_temp_new();
> >>>
> >>> misaligned = gen_new_label();
> >>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
> >>>
> >>> gen_set_label(l); /* branch taken */
> >>>
> >>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
> >>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
> >>> /* misaligned */
> >>> gen_exception_inst_addr_mis(ctx);
> >>> } else {
> >>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> >>> index 0ee8ee147d..d1fdd0c2d7 100644
> >>> --- a/target/riscv/translate.c
> >>> +++ b/target/riscv/translate.c
> >>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
> >>>
> >>> /* check misaligned: */
> >>> next_pc = ctx->base.pc_next + imm;
> >>> - if (!has_ext(ctx, RVC)) {
> >>> + if (!ctx->cfg_ptr->ext_zca) {
> >>> if ((next_pc & 0x3) != 0) {
> >>> gen_exception_inst_addr_mis(ctx);
> >>> return;
> >>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
> >>> if (insn_len(opcode) == 2) {
> >>> ctx->opcode = opcode;
> >>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
> >>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
> >>> + /*
> >>> + * The Zca extension is added as way to refer to instructions in the C
> >>> + * extension that do not include the floating-point loads and stores
> >>> + */
> >>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
> >>> return;
> >>> }
> >>> } else {
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-12 10:55 ` Alistair Francis
@ 2023-04-12 11:35 ` Weiwei Li
2023-04-12 17:24 ` Daniel Henrique Barboza
0 siblings, 1 reply; 28+ messages in thread
From: Weiwei Li @ 2023-04-12 11:35 UTC (permalink / raw)
To: Alistair Francis, Weiwei Li
Cc: Daniel Henrique Barboza, richard.henderson, palmer,
alistair.francis, bin.meng, qemu-riscv, qemu-devel, wangjunqiang,
lazyparser, Wilfred Mallawa
On 2023/4/12 18:55, Alistair Francis wrote:
> On Wed, Apr 12, 2023 at 12:55 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>>
>> On 2023/4/12 10:12, Alistair Francis wrote:
>>> On Fri, Apr 7, 2023 at 6:23 AM Daniel Henrique Barboza
>>> <dbarboza@ventanamicro.com> wrote:
>>>> Hi,
>>>>
>>>> This patch is going to break the sifive_u boot if I rebase
>>>>
>>>> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
>>>>
>>>> on top of it, as it is the case today with the current riscv-to-apply.next.
>>>>
>>>> The reason is that the priv spec version for Zca is marked as 1_12_0, and
>>>> the priv spec version for both sifive CPUs is 1_10_0, and both are enabling
>>>> RVC.
>>>>
>>>> This patch from that series above:
>>>>
>>>> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers"
>>>>
>>>> Makes the disabling of the extension based on priv version to happen *after* we
>>>> do all the validations, instead of before as we're doing today. Zca (and Zcd) will
>>>> be manually enabled just to be disabled shortly after by the priv spec code. And
>>>> this will happen:
>>>>
>>>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
>>>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
>>>> qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
>>>> (--- hangs ---)
>>>>
>>>> This means that the assumption made in this patch - that Zca implies RVC - is no
>>>> longer valid, and all these translations won't work.
>>> Thanks for catching and reporting this
>>>
>>>> Some possible solutions:
>>>>
>>>> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would need to convert
>>>> all Zca checks to RVC checks in all translation code.
>>> This to me seems like the best solution
>> I had also implemented a patch for this solution. I'll sent it later.
> Thanks!
>
>> However, this will introduce additional check. i.e. check both Zca and C
>> or , both Zcf/d and CF/CD.
> Is there a large performance penalty for that?
May not very large. Just one or two additional check in instruction
translation. You can see the patch at:
https://lore.kernel.org/qemu-devel/20230412030648.80470-1-liweiwei@iscas.ac.cn/
>
>> I think this is not very necessary. Implcitly-enabled extensions is
>> often the subsets of existed extension.
> Yeah, I see what you are saying. It just becomes difficult to manage
> though. It all worked fine until there are conflicts between the priv
> specs.
>
>> So enabling them will introduce no additional function to the cpus.
>>
>> We can just make them invisible to user(mask them in the isa string)
>> instead of disabling them to be compatible with lower priv version.
> I'm open to other options, but masking them like this seems like more
> work and also seems confusing. The extension will end up enabled in
> QEMU and potentially visible to external tools, but then just not
> exposed to the guest. It seems prone to future bugs.
Yeah. they may be visible to external tools if they read cfg directly.
Another way is to introduce another parameter instead of cfg.ext_zca to
indicate whether Zca/Zcf/Zcd
instructions are enabled. This is be done by patchset:
https://lore.kernel.org/qemu-devel/20230410033526.31708-1-liweiwei@iscas.ac.cn/
All of the three ways are acceptable to me.
Regards,
Weiwei Li
> Alistair
>
>> Regards,
>>
>> Weiwei Li
>>
>>
>>>> - Do not apply patch 5/9 from that series that moves the disable_ext code to the end
>>>> of validation. Also a possibility, but we would be sweeping the problem under the rug.
>>>> Zca still can't be used as a RVC replacement due to priv spec version constraints, but
>>>> we just won't disable Zca because we'll keep validating exts too early (which is the
>>>> problem that the patch addresses).
>>>>
>>>> - change the priv spec of the sifive CPUs - and everyone that uses RVC - to 1_12_0. Not
>>>> sure if this makes sense.
>>>>
>>>> - do not disable any extensions due to privilege spec version mismatch. This would make
>>>> all the priv_version related artifacts to be more "educational" than to be an actual
>>>> configuration we want to enforce. Not sure if that would do any good in the end, but
>>>> it's also a possibility.
>>> This also seems like something we can do. Printing a warning but
>>> continuing on seems reasonable to me. That allows users to
>>> enable/disable features even if the versions don't match.
>>>
>>> I don't think this is the solution for this problem though
>>>
>>> Alistair
>>>
>>>> I'll hold the rebase of that series until we sort this out. Thanks,
>>>>
>>>>
>>>> Daniel
>>>>
>>>>
>>>>
>>>> On 3/7/23 05:13, Weiwei Li wrote:
>>>>> Modify the check for C extension to Zca (C implies Zca).
>>>>>
>>>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>> ---
>>>>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>>>>> target/riscv/translate.c | 8 ++++++--
>>>>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
>>>>> index 4ad54e8a49..c70c495fc5 100644
>>>>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
>>>>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
>>>>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
>>>>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>>>>>
>>>>> gen_set_pc(ctx, cpu_pc);
>>>>> - if (!has_ext(ctx, RVC)) {
>>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>>> TCGv t0 = tcg_temp_new();
>>>>>
>>>>> misaligned = gen_new_label();
>>>>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
>>>>>
>>>>> gen_set_label(l); /* branch taken */
>>>>>
>>>>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>>>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>>>> /* misaligned */
>>>>> gen_exception_inst_addr_mis(ctx);
>>>>> } else {
>>>>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>>>>> index 0ee8ee147d..d1fdd0c2d7 100644
>>>>> --- a/target/riscv/translate.c
>>>>> +++ b/target/riscv/translate.c
>>>>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
>>>>>
>>>>> /* check misaligned: */
>>>>> next_pc = ctx->base.pc_next + imm;
>>>>> - if (!has_ext(ctx, RVC)) {
>>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>>> if ((next_pc & 0x3) != 0) {
>>>>> gen_exception_inst_addr_mis(ctx);
>>>>> return;
>>>>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
>>>>> if (insn_len(opcode) == 2) {
>>>>> ctx->opcode = opcode;
>>>>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
>>>>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
>>>>> + /*
>>>>> + * The Zca extension is added as way to refer to instructions in the C
>>>>> + * extension that do not include the floating-point loads and stores
>>>>> + */
>>>>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
>>>>> return;
>>>>> }
>>>>> } else {
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-12 11:35 ` Weiwei Li
@ 2023-04-12 17:24 ` Daniel Henrique Barboza
2023-04-17 2:35 ` Alistair Francis
0 siblings, 1 reply; 28+ messages in thread
From: Daniel Henrique Barboza @ 2023-04-12 17:24 UTC (permalink / raw)
To: Weiwei Li, Alistair Francis
Cc: richard.henderson, palmer, alistair.francis, bin.meng, qemu-riscv,
qemu-devel, wangjunqiang, lazyparser, Wilfred Mallawa
On 4/12/23 08:35, Weiwei Li wrote:
>
> On 2023/4/12 18:55, Alistair Francis wrote:
>> On Wed, Apr 12, 2023 at 12:55 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>>>
>>> On 2023/4/12 10:12, Alistair Francis wrote:
>>>> On Fri, Apr 7, 2023 at 6:23 AM Daniel Henrique Barboza
>>>> <dbarboza@ventanamicro.com> wrote:
>>>>> Hi,
>>>>>
>>>>> This patch is going to break the sifive_u boot if I rebase
>>>>>
>>>>> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
>>>>>
>>>>> on top of it, as it is the case today with the current riscv-to-apply.next.
>>>>>
>>>>> The reason is that the priv spec version for Zca is marked as 1_12_0, and
>>>>> the priv spec version for both sifive CPUs is 1_10_0, and both are enabling
>>>>> RVC.
>>>>>
>>>>> This patch from that series above:
>>>>>
>>>>> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers"
>>>>>
>>>>> Makes the disabling of the extension based on priv version to happen *after* we
>>>>> do all the validations, instead of before as we're doing today. Zca (and Zcd) will
>>>>> be manually enabled just to be disabled shortly after by the priv spec code. And
>>>>> this will happen:
>>>>>
>>>>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
>>>>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
>>>>> qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
>>>>> (--- hangs ---)
>>>>>
>>>>> This means that the assumption made in this patch - that Zca implies RVC - is no
>>>>> longer valid, and all these translations won't work.
>>>> Thanks for catching and reporting this
>>>>
>>>>> Some possible solutions:
>>>>>
>>>>> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would need to convert
>>>>> all Zca checks to RVC checks in all translation code.
>>>> This to me seems like the best solution
>>> I had also implemented a patch for this solution. I'll sent it later.
>> Thanks!
>>
>>> However, this will introduce additional check. i.e. check both Zca and C
>>> or , both Zcf/d and CF/CD.
>> Is there a large performance penalty for that?
>
> May not very large. Just one or two additional check in instruction translation. You can see the patch at:
>
> https://lore.kernel.org/qemu-devel/20230412030648.80470-1-liweiwei@iscas.ac.cn/
>
>>
>>> I think this is not very necessary. Implcitly-enabled extensions is
>>> often the subsets of existed extension.
>> Yeah, I see what you are saying. It just becomes difficult to manage
>> though. It all worked fine until there are conflicts between the priv
>> specs.
>>
>>> So enabling them will introduce no additional function to the cpus.
>>>
>>> We can just make them invisible to user(mask them in the isa string)
>>> instead of disabling them to be compatible with lower priv version.
>> I'm open to other options, but masking them like this seems like more
>> work and also seems confusing. The extension will end up enabled in
>> QEMU and potentially visible to external tools, but then just not
>> exposed to the guest. It seems prone to future bugs.
>
> Yeah. they may be visible to external tools if they read cfg directly.
>
> Another way is to introduce another parameter instead of cfg.ext_zca to indicate whether Zca/Zcf/Zcd
>
> instructions are enabled. This is be done by patchset:
>
> https://lore.kernel.org/qemu-devel/20230410033526.31708-1-liweiwei@iscas.ac.cn/
>
> All of the three ways are acceptable to me.
Earlier today I grabbed two Weiwei Li patches (isa_string changes and Zca/Zcf/Zcd
changes) in the "rework CPU extensions validation" series, but after reading these
last messages I guess I jumped the gun.
Alistair, I'm considering drop the patch that disables extensions via priv_spec later
during realize() (the one I mentioned in my first message) from that series until we
figure the best way of dealing with priv spec and Z-extensions being used as MISA bits
and so on. We can merge those cleanups and write_misa() changes that are already reviewed
in the meantime. Are you ok with that?
Thanks,
Daniel
>
> Regards,
>
> Weiwei Li
>
>
>> Alistair
>>
>>> Regards,
>>>
>>> Weiwei Li
>>>
>>>
>>>>> - Do not apply patch 5/9 from that series that moves the disable_ext code to the end
>>>>> of validation. Also a possibility, but we would be sweeping the problem under the rug.
>>>>> Zca still can't be used as a RVC replacement due to priv spec version constraints, but
>>>>> we just won't disable Zca because we'll keep validating exts too early (which is the
>>>>> problem that the patch addresses).
>>>>>
>>>>> - change the priv spec of the sifive CPUs - and everyone that uses RVC - to 1_12_0. Not
>>>>> sure if this makes sense.
>>>>>
>>>>> - do not disable any extensions due to privilege spec version mismatch. This would make
>>>>> all the priv_version related artifacts to be more "educational" than to be an actual
>>>>> configuration we want to enforce. Not sure if that would do any good in the end, but
>>>>> it's also a possibility.
>>>> This also seems like something we can do. Printing a warning but
>>>> continuing on seems reasonable to me. That allows users to
>>>> enable/disable features even if the versions don't match.
>>>>
>>>> I don't think this is the solution for this problem though
>>>>
>>>> Alistair
>>>>
>>>>> I'll hold the rebase of that series until we sort this out. Thanks,
>>>>>
>>>>>
>>>>> Daniel
>>>>>
>>>>>
>>>>>
>>>>> On 3/7/23 05:13, Weiwei Li wrote:
>>>>>> Modify the check for C extension to Zca (C implies Zca).
>>>>>>
>>>>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>>>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>>>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>>> ---
>>>>>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>>>>>> target/riscv/translate.c | 8 ++++++--
>>>>>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
>>>>>> index 4ad54e8a49..c70c495fc5 100644
>>>>>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
>>>>>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
>>>>>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
>>>>>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>>>>>>
>>>>>> gen_set_pc(ctx, cpu_pc);
>>>>>> - if (!has_ext(ctx, RVC)) {
>>>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>>>> TCGv t0 = tcg_temp_new();
>>>>>>
>>>>>> misaligned = gen_new_label();
>>>>>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
>>>>>>
>>>>>> gen_set_label(l); /* branch taken */
>>>>>>
>>>>>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>>>>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>>>>> /* misaligned */
>>>>>> gen_exception_inst_addr_mis(ctx);
>>>>>> } else {
>>>>>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>>>>>> index 0ee8ee147d..d1fdd0c2d7 100644
>>>>>> --- a/target/riscv/translate.c
>>>>>> +++ b/target/riscv/translate.c
>>>>>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
>>>>>>
>>>>>> /* check misaligned: */
>>>>>> next_pc = ctx->base.pc_next + imm;
>>>>>> - if (!has_ext(ctx, RVC)) {
>>>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>>>> if ((next_pc & 0x3) != 0) {
>>>>>> gen_exception_inst_addr_mis(ctx);
>>>>>> return;
>>>>>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
>>>>>> if (insn_len(opcode) == 2) {
>>>>>> ctx->opcode = opcode;
>>>>>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
>>>>>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
>>>>>> + /*
>>>>>> + * The Zca extension is added as way to refer to instructions in the C
>>>>>> + * extension that do not include the floating-point loads and stores
>>>>>> + */
>>>>>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
>>>>>> return;
>>>>>> }
>>>>>> } else {
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-12 17:24 ` Daniel Henrique Barboza
@ 2023-04-17 2:35 ` Alistair Francis
2023-04-17 11:00 ` Daniel Henrique Barboza
0 siblings, 1 reply; 28+ messages in thread
From: Alistair Francis @ 2023-04-17 2:35 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Weiwei Li, richard.henderson, palmer, alistair.francis, bin.meng,
qemu-riscv, qemu-devel, wangjunqiang, lazyparser, Wilfred Mallawa
On Thu, Apr 13, 2023 at 3:24 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 4/12/23 08:35, Weiwei Li wrote:
> >
> > On 2023/4/12 18:55, Alistair Francis wrote:
> >> On Wed, Apr 12, 2023 at 12:55 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
> >>>
> >>> On 2023/4/12 10:12, Alistair Francis wrote:
> >>>> On Fri, Apr 7, 2023 at 6:23 AM Daniel Henrique Barboza
> >>>> <dbarboza@ventanamicro.com> wrote:
> >>>>> Hi,
> >>>>>
> >>>>> This patch is going to break the sifive_u boot if I rebase
> >>>>>
> >>>>> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
> >>>>>
> >>>>> on top of it, as it is the case today with the current riscv-to-apply.next.
> >>>>>
> >>>>> The reason is that the priv spec version for Zca is marked as 1_12_0, and
> >>>>> the priv spec version for both sifive CPUs is 1_10_0, and both are enabling
> >>>>> RVC.
> >>>>>
> >>>>> This patch from that series above:
> >>>>>
> >>>>> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers"
> >>>>>
> >>>>> Makes the disabling of the extension based on priv version to happen *after* we
> >>>>> do all the validations, instead of before as we're doing today. Zca (and Zcd) will
> >>>>> be manually enabled just to be disabled shortly after by the priv spec code. And
> >>>>> this will happen:
> >>>>>
> >>>>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
> >>>>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
> >>>>> qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
> >>>>> (--- hangs ---)
> >>>>>
> >>>>> This means that the assumption made in this patch - that Zca implies RVC - is no
> >>>>> longer valid, and all these translations won't work.
> >>>> Thanks for catching and reporting this
> >>>>
> >>>>> Some possible solutions:
> >>>>>
> >>>>> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would need to convert
> >>>>> all Zca checks to RVC checks in all translation code.
> >>>> This to me seems like the best solution
> >>> I had also implemented a patch for this solution. I'll sent it later.
> >> Thanks!
> >>
> >>> However, this will introduce additional check. i.e. check both Zca and C
> >>> or , both Zcf/d and CF/CD.
> >> Is there a large performance penalty for that?
> >
> > May not very large. Just one or two additional check in instruction translation. You can see the patch at:
> >
> > https://lore.kernel.org/qemu-devel/20230412030648.80470-1-liweiwei@iscas.ac.cn/
This is my prefered way. I think it's pretty simple and explicitly
follows the spec.
> >
> >>
> >>> I think this is not very necessary. Implcitly-enabled extensions is
> >>> often the subsets of existed extension.
> >> Yeah, I see what you are saying. It just becomes difficult to manage
> >> though. It all worked fine until there are conflicts between the priv
> >> specs.
> >>
> >>> So enabling them will introduce no additional function to the cpus.
> >>>
> >>> We can just make them invisible to user(mask them in the isa string)
> >>> instead of disabling them to be compatible with lower priv version.
> >> I'm open to other options, but masking them like this seems like more
> >> work and also seems confusing. The extension will end up enabled in
> >> QEMU and potentially visible to external tools, but then just not
> >> exposed to the guest. It seems prone to future bugs.
> >
> > Yeah. they may be visible to external tools if they read cfg directly.
> >
> > Another way is to introduce another parameter instead of cfg.ext_zca to indicate whether Zca/Zcf/Zcd
> >
> > instructions are enabled. This is be done by patchset:
> >
> > https://lore.kernel.org/qemu-devel/20230410033526.31708-1-liweiwei@iscas.ac.cn/
I don't prefer this option, but if others feel strongly I'm not
completely opposed to it.
> >
> > All of the three ways are acceptable to me.
>
> Earlier today I grabbed two Weiwei Li patches (isa_string changes and Zca/Zcf/Zcd
> changes) in the "rework CPU extensions validation" series, but after reading these
> last messages I guess I jumped the gun.
My preference would be the "target/riscv: Update check for
Zca/zcf/zcd" patch, which I think is what you picked. So that seems
like the way to go
>
> Alistair, I'm considering drop the patch that disables extensions via priv_spec later
> during realize() (the one I mentioned in my first message) from that series until we
> figure the best way of dealing with priv spec and Z-extensions being used as MISA bits
> and so on. We can merge those cleanups and write_misa() changes that are already reviewed
> in the meantime. Are you ok with that?
That's also fine
Alistair
>
>
> Thanks,
>
>
> Daniel
>
>
> >
> > Regards,
> >
> > Weiwei Li
> >
> >
> >> Alistair
> >>
> >>> Regards,
> >>>
> >>> Weiwei Li
> >>>
> >>>
> >>>>> - Do not apply patch 5/9 from that series that moves the disable_ext code to the end
> >>>>> of validation. Also a possibility, but we would be sweeping the problem under the rug.
> >>>>> Zca still can't be used as a RVC replacement due to priv spec version constraints, but
> >>>>> we just won't disable Zca because we'll keep validating exts too early (which is the
> >>>>> problem that the patch addresses).
> >>>>>
> >>>>> - change the priv spec of the sifive CPUs - and everyone that uses RVC - to 1_12_0. Not
> >>>>> sure if this makes sense.
> >>>>>
> >>>>> - do not disable any extensions due to privilege spec version mismatch. This would make
> >>>>> all the priv_version related artifacts to be more "educational" than to be an actual
> >>>>> configuration we want to enforce. Not sure if that would do any good in the end, but
> >>>>> it's also a possibility.
> >>>> This also seems like something we can do. Printing a warning but
> >>>> continuing on seems reasonable to me. That allows users to
> >>>> enable/disable features even if the versions don't match.
> >>>>
> >>>> I don't think this is the solution for this problem though
> >>>>
> >>>> Alistair
> >>>>
> >>>>> I'll hold the rebase of that series until we sort this out. Thanks,
> >>>>>
> >>>>>
> >>>>> Daniel
> >>>>>
> >>>>>
> >>>>>
> >>>>> On 3/7/23 05:13, Weiwei Li wrote:
> >>>>>> Modify the check for C extension to Zca (C implies Zca).
> >>>>>>
> >>>>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> >>>>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> >>>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >>>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >>>>>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> >>>>>> ---
> >>>>>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
> >>>>>> target/riscv/translate.c | 8 ++++++--
> >>>>>> 2 files changed, 8 insertions(+), 4 deletions(-)
> >>>>>>
> >>>>>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
> >>>>>> index 4ad54e8a49..c70c495fc5 100644
> >>>>>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> >>>>>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> >>>>>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
> >>>>>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
> >>>>>>
> >>>>>> gen_set_pc(ctx, cpu_pc);
> >>>>>> - if (!has_ext(ctx, RVC)) {
> >>>>>> + if (!ctx->cfg_ptr->ext_zca) {
> >>>>>> TCGv t0 = tcg_temp_new();
> >>>>>>
> >>>>>> misaligned = gen_new_label();
> >>>>>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
> >>>>>>
> >>>>>> gen_set_label(l); /* branch taken */
> >>>>>>
> >>>>>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
> >>>>>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
> >>>>>> /* misaligned */
> >>>>>> gen_exception_inst_addr_mis(ctx);
> >>>>>> } else {
> >>>>>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> >>>>>> index 0ee8ee147d..d1fdd0c2d7 100644
> >>>>>> --- a/target/riscv/translate.c
> >>>>>> +++ b/target/riscv/translate.c
> >>>>>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
> >>>>>>
> >>>>>> /* check misaligned: */
> >>>>>> next_pc = ctx->base.pc_next + imm;
> >>>>>> - if (!has_ext(ctx, RVC)) {
> >>>>>> + if (!ctx->cfg_ptr->ext_zca) {
> >>>>>> if ((next_pc & 0x3) != 0) {
> >>>>>> gen_exception_inst_addr_mis(ctx);
> >>>>>> return;
> >>>>>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
> >>>>>> if (insn_len(opcode) == 2) {
> >>>>>> ctx->opcode = opcode;
> >>>>>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
> >>>>>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
> >>>>>> + /*
> >>>>>> + * The Zca extension is added as way to refer to instructions in the C
> >>>>>> + * extension that do not include the floating-point loads and stores
> >>>>>> + */
> >>>>>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
> >>>>>> return;
> >>>>>> }
> >>>>>> } else {
> >
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v12 02/10] target/riscv: add support for Zca extension
2023-04-17 2:35 ` Alistair Francis
@ 2023-04-17 11:00 ` Daniel Henrique Barboza
0 siblings, 0 replies; 28+ messages in thread
From: Daniel Henrique Barboza @ 2023-04-17 11:00 UTC (permalink / raw)
To: Alistair Francis
Cc: Weiwei Li, richard.henderson, palmer, alistair.francis, bin.meng,
qemu-riscv, qemu-devel, wangjunqiang, lazyparser, Wilfred Mallawa
On 4/16/23 23:35, Alistair Francis wrote:
> On Thu, Apr 13, 2023 at 3:24 AM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>>
>>
>>
>> On 4/12/23 08:35, Weiwei Li wrote:
>>>
>>> On 2023/4/12 18:55, Alistair Francis wrote:
>>>> On Wed, Apr 12, 2023 at 12:55 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>>>>>
>>>>> On 2023/4/12 10:12, Alistair Francis wrote:
>>>>>> On Fri, Apr 7, 2023 at 6:23 AM Daniel Henrique Barboza
>>>>>> <dbarboza@ventanamicro.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This patch is going to break the sifive_u boot if I rebase
>>>>>>>
>>>>>>> "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
>>>>>>>
>>>>>>> on top of it, as it is the case today with the current riscv-to-apply.next.
>>>>>>>
>>>>>>> The reason is that the priv spec version for Zca is marked as 1_12_0, and
>>>>>>> the priv spec version for both sifive CPUs is 1_10_0, and both are enabling
>>>>>>> RVC.
>>>>>>>
>>>>>>> This patch from that series above:
>>>>>>>
>>>>>>> "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts helpers"
>>>>>>>
>>>>>>> Makes the disabling of the extension based on priv version to happen *after* we
>>>>>>> do all the validations, instead of before as we're doing today. Zca (and Zcd) will
>>>>>>> be manually enabled just to be disabled shortly after by the priv spec code. And
>>>>>>> this will happen:
>>>>>>>
>>>>>>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000000 because privilege spec version does not match
>>>>>>> qemu-system-riscv64: warning: disabling zca extension for hart 0x0000000000000001 because privilege spec version does not match
>>>>>>> qemu-system-riscv64: warning: disabling zcd extension for hart 0x0000000000000001 because privilege spec version does not match
>>>>>>> (--- hangs ---)
>>>>>>>
>>>>>>> This means that the assumption made in this patch - that Zca implies RVC - is no
>>>>>>> longer valid, and all these translations won't work.
>>>>>> Thanks for catching and reporting this
>>>>>>
>>>>>>> Some possible solutions:
>>>>>>>
>>>>>>> - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would need to convert
>>>>>>> all Zca checks to RVC checks in all translation code.
>>>>>> This to me seems like the best solution
>>>>> I had also implemented a patch for this solution. I'll sent it later.
>>>> Thanks!
>>>>
>>>>> However, this will introduce additional check. i.e. check both Zca and C
>>>>> or , both Zcf/d and CF/CD.
>>>> Is there a large performance penalty for that?
>>>
>>> May not very large. Just one or two additional check in instruction translation. You can see the patch at:
>>>
>>> https://lore.kernel.org/qemu-devel/20230412030648.80470-1-liweiwei@iscas.ac.cn/
>
> This is my prefered way. I think it's pretty simple and explicitly
> follows the spec.
>
>>>
>>>>
>>>>> I think this is not very necessary. Implcitly-enabled extensions is
>>>>> often the subsets of existed extension.
>>>> Yeah, I see what you are saying. It just becomes difficult to manage
>>>> though. It all worked fine until there are conflicts between the priv
>>>> specs.
>>>>
>>>>> So enabling them will introduce no additional function to the cpus.
>>>>>
>>>>> We can just make them invisible to user(mask them in the isa string)
>>>>> instead of disabling them to be compatible with lower priv version.
>>>> I'm open to other options, but masking them like this seems like more
>>>> work and also seems confusing. The extension will end up enabled in
>>>> QEMU and potentially visible to external tools, but then just not
>>>> exposed to the guest. It seems prone to future bugs.
>>>
>>> Yeah. they may be visible to external tools if they read cfg directly.
>>>
>>> Another way is to introduce another parameter instead of cfg.ext_zca to indicate whether Zca/Zcf/Zcd
>>>
>>> instructions are enabled. This is be done by patchset:
>>>
>>> https://lore.kernel.org/qemu-devel/20230410033526.31708-1-liweiwei@iscas.ac.cn/
>
> I don't prefer this option, but if others feel strongly I'm not
> completely opposed to it.
>
>>>
>>> All of the three ways are acceptable to me.
>>
>> Earlier today I grabbed two Weiwei Li patches (isa_string changes and Zca/Zcf/Zcd
>> changes) in the "rework CPU extensions validation" series, but after reading these
>> last messages I guess I jumped the gun.
>
> My preference would be the "target/riscv: Update check for
> Zca/zcf/zcd" patch, which I think is what you picked. So that seems
> like the way to go
Ok! I'll send it over with Weiwei's v2.
Thanks,
Daniel
>
>>
>> Alistair, I'm considering drop the patch that disables extensions via priv_spec later
>> during realize() (the one I mentioned in my first message) from that series until we
>> figure the best way of dealing with priv spec and Z-extensions being used as MISA bits
>> and so on. We can merge those cleanups and write_misa() changes that are already reviewed
>> in the meantime. Are you ok with that?
>
> That's also fine
>
> Alistair
>
>>
>>
>> Thanks,
>>
>>
>> Daniel
>>
>>
>>>
>>> Regards,
>>>
>>> Weiwei Li
>>>
>>>
>>>> Alistair
>>>>
>>>>> Regards,
>>>>>
>>>>> Weiwei Li
>>>>>
>>>>>
>>>>>>> - Do not apply patch 5/9 from that series that moves the disable_ext code to the end
>>>>>>> of validation. Also a possibility, but we would be sweeping the problem under the rug.
>>>>>>> Zca still can't be used as a RVC replacement due to priv spec version constraints, but
>>>>>>> we just won't disable Zca because we'll keep validating exts too early (which is the
>>>>>>> problem that the patch addresses).
>>>>>>>
>>>>>>> - change the priv spec of the sifive CPUs - and everyone that uses RVC - to 1_12_0. Not
>>>>>>> sure if this makes sense.
>>>>>>>
>>>>>>> - do not disable any extensions due to privilege spec version mismatch. This would make
>>>>>>> all the priv_version related artifacts to be more "educational" than to be an actual
>>>>>>> configuration we want to enforce. Not sure if that would do any good in the end, but
>>>>>>> it's also a possibility.
>>>>>> This also seems like something we can do. Printing a warning but
>>>>>> continuing on seems reasonable to me. That allows users to
>>>>>> enable/disable features even if the versions don't match.
>>>>>>
>>>>>> I don't think this is the solution for this problem though
>>>>>>
>>>>>> Alistair
>>>>>>
>>>>>>> I'll hold the rebase of that series until we sort this out. Thanks,
>>>>>>>
>>>>>>>
>>>>>>> Daniel
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 3/7/23 05:13, Weiwei Li wrote:
>>>>>>>> Modify the check for C extension to Zca (C implies Zca).
>>>>>>>>
>>>>>>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>>>>>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>>>>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>>>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>>>>>> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>>>>> ---
>>>>>>>> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
>>>>>>>> target/riscv/translate.c | 8 ++++++--
>>>>>>>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
>>>>>>>> index 4ad54e8a49..c70c495fc5 100644
>>>>>>>> --- a/target/riscv/insn_trans/trans_rvi.c.inc
>>>>>>>> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
>>>>>>>> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
>>>>>>>> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>>>>>>>>
>>>>>>>> gen_set_pc(ctx, cpu_pc);
>>>>>>>> - if (!has_ext(ctx, RVC)) {
>>>>>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>>>>>> TCGv t0 = tcg_temp_new();
>>>>>>>>
>>>>>>>> misaligned = gen_new_label();
>>>>>>>> @@ -169,7 +169,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
>>>>>>>>
>>>>>>>> gen_set_label(l); /* branch taken */
>>>>>>>>
>>>>>>>> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>>>>>>> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
>>>>>>>> /* misaligned */
>>>>>>>> gen_exception_inst_addr_mis(ctx);
>>>>>>>> } else {
>>>>>>>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>>>>>>>> index 0ee8ee147d..d1fdd0c2d7 100644
>>>>>>>> --- a/target/riscv/translate.c
>>>>>>>> +++ b/target/riscv/translate.c
>>>>>>>> @@ -549,7 +549,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
>>>>>>>>
>>>>>>>> /* check misaligned: */
>>>>>>>> next_pc = ctx->base.pc_next + imm;
>>>>>>>> - if (!has_ext(ctx, RVC)) {
>>>>>>>> + if (!ctx->cfg_ptr->ext_zca) {
>>>>>>>> if ((next_pc & 0x3) != 0) {
>>>>>>>> gen_exception_inst_addr_mis(ctx);
>>>>>>>> return;
>>>>>>>> @@ -1122,7 +1122,11 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
>>>>>>>> if (insn_len(opcode) == 2) {
>>>>>>>> ctx->opcode = opcode;
>>>>>>>> ctx->pc_succ_insn = ctx->base.pc_next + 2;
>>>>>>>> - if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
>>>>>>>> + /*
>>>>>>>> + * The Zca extension is added as way to refer to instructions in the C
>>>>>>>> + * extension that do not include the floating-point loads and stores
>>>>>>>> + */
>>>>>>>> + if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
>>>>>>>> return;
>>>>>>>> }
>>>>>>>> } else {
>>>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2023-04-17 11:01 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-07 8:13 [PATCH v12 00/10] support subsets of code size reduction extension Weiwei Li
2023-03-07 8:13 ` [PATCH v12 01/10] target/riscv: add cfg properties for Zc* extension Weiwei Li
2023-03-07 8:13 ` [PATCH v12 02/10] target/riscv: add support for Zca extension Weiwei Li
2023-04-06 20:22 ` Daniel Henrique Barboza
2023-04-07 1:14 ` liweiwei
2023-04-07 3:34 ` liweiwei
2023-04-07 19:25 ` Daniel Henrique Barboza
2023-04-08 1:09 ` liweiwei
2023-04-07 10:28 ` Daniel Henrique Barboza
2023-04-07 10:48 ` liweiwei
2023-04-12 2:12 ` Alistair Francis
2023-04-12 2:55 ` Weiwei Li
2023-04-12 10:55 ` Alistair Francis
2023-04-12 11:35 ` Weiwei Li
2023-04-12 17:24 ` Daniel Henrique Barboza
2023-04-17 2:35 ` Alistair Francis
2023-04-17 11:00 ` Daniel Henrique Barboza
2023-03-07 8:13 ` [PATCH v12 03/10] target/riscv: add support for Zcf extension Weiwei Li
2023-03-07 8:13 ` [PATCH v12 04/10] target/riscv: add support for Zcd extension Weiwei Li
2023-03-07 8:13 ` [PATCH v12 05/10] target/riscv: add support for Zcb extension Weiwei Li
2023-03-07 8:13 ` [PATCH v12 06/10] target/riscv: add support for Zcmp extension Weiwei Li
2023-03-07 8:14 ` [PATCH v12 07/10] target/riscv: add support for Zcmt extension Weiwei Li
2023-03-07 8:14 ` [PATCH v12 08/10] target/riscv: expose properties for Zc* extension Weiwei Li
2023-03-07 8:14 ` [PATCH v12 09/10] disas/riscv.c: add disasm support for Zc* Weiwei Li
2023-03-07 8:14 ` [PATCH v12 10/10] target/riscv: Add support for Zce Weiwei Li
2023-04-05 4:15 ` Alistair Francis
2023-03-24 13:23 ` [PATCH v12 00/10] support subsets of code size reduction extension liweiwei
2023-04-05 4:17 ` Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).