[PATCH v12 00/10] support subsets of code size reduction extension

[PATCH v12 00/10] support subsets of code size reduction extension

[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

[PATCH v12 01/10] target/riscv: add cfg properties for Zc* extension

[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

[PATCH v12 02/10] target/riscv: add support for Zca extension

[Weiwei Li]

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

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[Daniel Henrique Barboza]

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 {

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[liweiwei]

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 {

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[liweiwei]

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 {

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[Daniel Henrique Barboza]

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

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[liweiwei]

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

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[Daniel Henrique Barboza]

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

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[liweiwei]

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

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[Alistair Francis]

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

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[Weiwei Li]

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 {

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[Alistair Francis]

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

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[Weiwei Li]

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 {

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[Daniel Henrique Barboza]

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

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[Alistair Francis]

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

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[Daniel Henrique Barboza]

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

[PATCH v12 03/10] target/riscv: add support for Zcf extension

[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

[PATCH v12 04/10] target/riscv: add support for Zcd extension

[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

[PATCH v12 05/10] target/riscv: add support for Zcb extension

[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

[PATCH v12 06/10] target/riscv: add support for Zcmp extension

[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

[PATCH v12 07/10] target/riscv: add support for Zcmt extension

[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

[PATCH v12 08/10] target/riscv: expose properties for Zc* extension

[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

[PATCH v12 09/10] disas/riscv.c: add disasm support for Zc*

[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

[PATCH v12 10/10] target/riscv: Add support for Zce

[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

Re: [PATCH v12 10/10] target/riscv: Add support for Zce

[Alistair Francis]

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

Re: [PATCH v12 00/10] support subsets of code size reduction extension

[liweiwei]

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
>

Re: [PATCH v12 00/10] support subsets of code size reduction extension

[Alistair Francis]

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