* [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions @ 2023-04-10 3:35 Weiwei Li 2023-04-10 3:35 ` [PATCH 1/2] target/riscv: Add set_implicit_extensions_from_ext() function Weiwei Li ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Weiwei Li @ 2023-04-10 3:35 UTC (permalink / raw) To: qemu-riscv, qemu-devel Cc: palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser, Weiwei Li The patch tries to separate the multi-letter extensions that may implicitly-enabled by misa.EXT from the explicitly-enabled cases, so that the misa.EXT can truely disabled by write_misa(). With this separation, the implicitly-enabled zve64d/f and zve32f extensions will no work if we clear misa.V. And clear misa.V will have no effect on the explicitly-enalbed zve64d/f and zve32f extensions. Weiwei Li (2): target/riscv: Add set_implicit_extensions_from_ext() function target/riscv: Add ext_z*_enabled for implicitly enabled extensions target/riscv/cpu.c | 73 +++++++++++++++---------- target/riscv/cpu.h | 8 +++ target/riscv/cpu_helper.c | 2 +- target/riscv/csr.c | 2 +- target/riscv/insn_trans/trans_rvd.c.inc | 2 +- target/riscv/insn_trans/trans_rvf.c.inc | 2 +- target/riscv/insn_trans/trans_rvi.c.inc | 5 +- target/riscv/insn_trans/trans_rvv.c.inc | 16 +++--- target/riscv/translate.c | 4 +- 9 files changed, 68 insertions(+), 46 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] target/riscv: Add set_implicit_extensions_from_ext() function 2023-04-10 3:35 [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions Weiwei Li @ 2023-04-10 3:35 ` Weiwei Li 2023-04-10 13:28 ` Daniel Henrique Barboza 2023-04-12 2:51 ` Alistair Francis 2023-04-10 3:35 ` [PATCH 2/2] target/riscv: Add ext_z*_enabled for implicitly enabled extensions Weiwei Li 2023-04-10 13:48 ` [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions Daniel Henrique Barboza 2 siblings, 2 replies; 11+ messages in thread From: Weiwei Li @ 2023-04-10 3:35 UTC (permalink / raw) To: qemu-riscv, qemu-devel Cc: palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser, Weiwei Li Move multi-letter extensions that may implicitly enabled from misa.EXT alone to prepare for following separation of implicitly enabled and explicitly enabled extensions. Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> --- target/riscv/cpu.c | 56 +++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index cb68916fce..abb65d41b1 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -809,6 +809,35 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info) } } +static void set_implicit_extensions_from_ext(RISCVCPU *cpu) +{ + + /* The V vector extension depends on the Zve64d extension */ + if (cpu->cfg.ext_v) { + cpu->cfg.ext_zve64d = true; + } + + /* The Zve64d extension depends on the Zve64f extension */ + if (cpu->cfg.ext_zve64d) { + cpu->cfg.ext_zve64f = true; + } + + /* The Zve64f extension depends on the Zve32f extension */ + if (cpu->cfg.ext_zve64f) { + cpu->cfg.ext_zve32f = true; + } + + if (cpu->cfg.ext_c) { + cpu->cfg.ext_zca = true; + if (cpu->cfg.ext_f && cpu->env.misa_mxl_max == MXL_RV32) { + cpu->cfg.ext_zcf = true; + } + if (cpu->cfg.ext_d) { + cpu->cfg.ext_zcd = true; + } + } +} + /* * Check consistency between chosen extensions while setting * cpu->cfg accordingly, doing a set_misa() in the end. @@ -833,6 +862,8 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) cpu->cfg.ext_ifencei = true; } + set_implicit_extensions_from_ext(cpu); + if (cpu->cfg.ext_i && cpu->cfg.ext_e) { error_setg(errp, "I and E extensions are incompatible"); @@ -886,21 +917,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) return; } - /* The V vector extension depends on the Zve64d extension */ - if (cpu->cfg.ext_v) { - cpu->cfg.ext_zve64d = true; - } - - /* The Zve64d extension depends on the Zve64f extension */ - if (cpu->cfg.ext_zve64d) { - cpu->cfg.ext_zve64f = true; - } - - /* The Zve64f extension depends on the Zve32f extension */ - if (cpu->cfg.ext_zve64f) { - cpu->cfg.ext_zve32f = true; - } - if (cpu->cfg.ext_zve64d && !cpu->cfg.ext_d) { error_setg(errp, "Zve64d/V extensions require D extension"); return; @@ -956,16 +972,6 @@ 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; -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] target/riscv: Add set_implicit_extensions_from_ext() function 2023-04-10 3:35 ` [PATCH 1/2] target/riscv: Add set_implicit_extensions_from_ext() function Weiwei Li @ 2023-04-10 13:28 ` Daniel Henrique Barboza 2023-04-12 2:51 ` Alistair Francis 1 sibling, 0 replies; 11+ messages in thread From: Daniel Henrique Barboza @ 2023-04-10 13:28 UTC (permalink / raw) To: Weiwei Li, qemu-riscv, qemu-devel Cc: palmer, alistair.francis, bin.meng, zhiwei_liu, wangjunqiang, lazyparser On 4/10/23 00:35, Weiwei Li wrote: > Move multi-letter extensions that may implicitly enabled from misa.EXT > alone to prepare for following separation of implicitly enabled and > explicitly enabled extensions. > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> > --- Regardless of what we end up doing with patch 2 I believe this is a good separation to have. Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > target/riscv/cpu.c | 56 +++++++++++++++++++++++++--------------------- > 1 file changed, 31 insertions(+), 25 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index cb68916fce..abb65d41b1 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -809,6 +809,35 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info) > } > } > > +static void set_implicit_extensions_from_ext(RISCVCPU *cpu) > +{ > + > + /* The V vector extension depends on the Zve64d extension */ > + if (cpu->cfg.ext_v) { > + cpu->cfg.ext_zve64d = true; > + } > + > + /* The Zve64d extension depends on the Zve64f extension */ > + if (cpu->cfg.ext_zve64d) { > + cpu->cfg.ext_zve64f = true; > + } > + > + /* The Zve64f extension depends on the Zve32f extension */ > + if (cpu->cfg.ext_zve64f) { > + cpu->cfg.ext_zve32f = true; > + } > + > + if (cpu->cfg.ext_c) { > + cpu->cfg.ext_zca = true; > + if (cpu->cfg.ext_f && cpu->env.misa_mxl_max == MXL_RV32) { > + cpu->cfg.ext_zcf = true; > + } > + if (cpu->cfg.ext_d) { > + cpu->cfg.ext_zcd = true; > + } > + } > +} > + > /* > * Check consistency between chosen extensions while setting > * cpu->cfg accordingly, doing a set_misa() in the end. > @@ -833,6 +862,8 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > cpu->cfg.ext_ifencei = true; > } > > + set_implicit_extensions_from_ext(cpu); > + > if (cpu->cfg.ext_i && cpu->cfg.ext_e) { > error_setg(errp, > "I and E extensions are incompatible"); > @@ -886,21 +917,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > return; > } > > - /* The V vector extension depends on the Zve64d extension */ > - if (cpu->cfg.ext_v) { > - cpu->cfg.ext_zve64d = true; > - } > - > - /* The Zve64d extension depends on the Zve64f extension */ > - if (cpu->cfg.ext_zve64d) { > - cpu->cfg.ext_zve64f = true; > - } > - > - /* The Zve64f extension depends on the Zve32f extension */ > - if (cpu->cfg.ext_zve64f) { > - cpu->cfg.ext_zve32f = true; > - } > - > if (cpu->cfg.ext_zve64d && !cpu->cfg.ext_d) { > error_setg(errp, "Zve64d/V extensions require D extension"); > return; > @@ -956,16 +972,6 @@ 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; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] target/riscv: Add set_implicit_extensions_from_ext() function 2023-04-10 3:35 ` [PATCH 1/2] target/riscv: Add set_implicit_extensions_from_ext() function Weiwei Li 2023-04-10 13:28 ` Daniel Henrique Barboza @ 2023-04-12 2:51 ` Alistair Francis 1 sibling, 0 replies; 11+ messages in thread From: Alistair Francis @ 2023-04-12 2:51 UTC (permalink / raw) To: Weiwei Li Cc: qemu-riscv, qemu-devel, palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser On Mon, Apr 10, 2023 at 1:36 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > Move multi-letter extensions that may implicitly enabled from misa.EXT > alone to prepare for following separation of implicitly enabled and > explicitly enabled extensions. > > 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 | 56 +++++++++++++++++++++++++--------------------- > 1 file changed, 31 insertions(+), 25 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index cb68916fce..abb65d41b1 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -809,6 +809,35 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info) > } > } > > +static void set_implicit_extensions_from_ext(RISCVCPU *cpu) > +{ > + > + /* The V vector extension depends on the Zve64d extension */ > + if (cpu->cfg.ext_v) { > + cpu->cfg.ext_zve64d = true; > + } > + > + /* The Zve64d extension depends on the Zve64f extension */ > + if (cpu->cfg.ext_zve64d) { > + cpu->cfg.ext_zve64f = true; > + } > + > + /* The Zve64f extension depends on the Zve32f extension */ > + if (cpu->cfg.ext_zve64f) { > + cpu->cfg.ext_zve32f = true; > + } > + > + if (cpu->cfg.ext_c) { > + cpu->cfg.ext_zca = true; > + if (cpu->cfg.ext_f && cpu->env.misa_mxl_max == MXL_RV32) { > + cpu->cfg.ext_zcf = true; > + } > + if (cpu->cfg.ext_d) { > + cpu->cfg.ext_zcd = true; > + } > + } > +} > + > /* > * Check consistency between chosen extensions while setting > * cpu->cfg accordingly, doing a set_misa() in the end. > @@ -833,6 +862,8 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > cpu->cfg.ext_ifencei = true; > } > > + set_implicit_extensions_from_ext(cpu); > + > if (cpu->cfg.ext_i && cpu->cfg.ext_e) { > error_setg(errp, > "I and E extensions are incompatible"); > @@ -886,21 +917,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > return; > } > > - /* The V vector extension depends on the Zve64d extension */ > - if (cpu->cfg.ext_v) { > - cpu->cfg.ext_zve64d = true; > - } > - > - /* The Zve64d extension depends on the Zve64f extension */ > - if (cpu->cfg.ext_zve64d) { > - cpu->cfg.ext_zve64f = true; > - } > - > - /* The Zve64f extension depends on the Zve32f extension */ > - if (cpu->cfg.ext_zve64f) { > - cpu->cfg.ext_zve32f = true; > - } > - > if (cpu->cfg.ext_zve64d && !cpu->cfg.ext_d) { > error_setg(errp, "Zve64d/V extensions require D extension"); > return; > @@ -956,16 +972,6 @@ 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; > -- > 2.25.1 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] target/riscv: Add ext_z*_enabled for implicitly enabled extensions 2023-04-10 3:35 [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions Weiwei Li 2023-04-10 3:35 ` [PATCH 1/2] target/riscv: Add set_implicit_extensions_from_ext() function Weiwei Li @ 2023-04-10 3:35 ` Weiwei Li 2023-04-10 13:48 ` [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions Daniel Henrique Barboza 2 siblings, 0 replies; 11+ messages in thread From: Weiwei Li @ 2023-04-10 3:35 UTC (permalink / raw) To: qemu-riscv, qemu-devel Cc: palmer, alistair.francis, bin.meng, dbarboza, zhiwei_liu, wangjunqiang, lazyparser, Weiwei Li For extensions that may implicitly enabled from misa.EXT: - ext_* is true if the extension is explicitly-enabled. - ext_*_enabled is true no matter the extension is implicitly-enabled or explicitly-enabled. And we change the check on the extension to it. Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> --- target/riscv/cpu.c | 29 +++++++++++++++---------- target/riscv/cpu.h | 8 +++++++ target/riscv/cpu_helper.c | 2 +- target/riscv/csr.c | 2 +- target/riscv/insn_trans/trans_rvd.c.inc | 2 +- target/riscv/insn_trans/trans_rvf.c.inc | 2 +- target/riscv/insn_trans/trans_rvi.c.inc | 5 +++-- target/riscv/insn_trans/trans_rvv.c.inc | 16 +++++++------- target/riscv/translate.c | 4 ++-- 9 files changed, 43 insertions(+), 27 deletions(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index abb65d41b1..815dd19f36 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -811,29 +811,35 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info) static void set_implicit_extensions_from_ext(RISCVCPU *cpu) { + cpu->cfg.ext_zve64d_enabled = cpu->cfg.ext_zve64d; + cpu->cfg.ext_zve64f_enabled = cpu->cfg.ext_zve64f; + cpu->cfg.ext_zve32f_enabled = cpu->cfg.ext_zve32f; + cpu->cfg.ext_zca_enabled = cpu->cfg.ext_zca; + cpu->cfg.ext_zcf_enabled = cpu->cfg.ext_zcf; + cpu->cfg.ext_zcd_enabled = cpu->cfg.ext_zcd; /* The V vector extension depends on the Zve64d extension */ if (cpu->cfg.ext_v) { - cpu->cfg.ext_zve64d = true; + cpu->cfg.ext_zve64d_enabled = true; } /* The Zve64d extension depends on the Zve64f extension */ if (cpu->cfg.ext_zve64d) { - cpu->cfg.ext_zve64f = true; + cpu->cfg.ext_zve64f_enabled = true; } /* The Zve64f extension depends on the Zve32f extension */ if (cpu->cfg.ext_zve64f) { - cpu->cfg.ext_zve32f = true; + cpu->cfg.ext_zve32f_enabled = true; } if (cpu->cfg.ext_c) { - cpu->cfg.ext_zca = true; + cpu->cfg.ext_zca_enabled = true; if (cpu->cfg.ext_f && cpu->env.misa_mxl_max == MXL_RV32) { - cpu->cfg.ext_zcf = true; + cpu->cfg.ext_zcf_enabled = true; } if (cpu->cfg.ext_d) { - cpu->cfg.ext_zcd = true; + cpu->cfg.ext_zcd_enabled = true; } } } @@ -917,12 +923,12 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) return; } - if (cpu->cfg.ext_zve64d && !cpu->cfg.ext_d) { + if (cpu->cfg.ext_zve64d_enabled && !cpu->cfg.ext_d) { error_setg(errp, "Zve64d/V extensions require D extension"); return; } - if (cpu->cfg.ext_zve32f && !cpu->cfg.ext_f) { + if (cpu->cfg.ext_zve32f_enabled && !cpu->cfg.ext_f) { error_setg(errp, "Zve32f/Zve64f extensions require F extension"); return; } @@ -931,7 +937,7 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) cpu->cfg.ext_zvfhmin = true; } - if (cpu->cfg.ext_zvfhmin && !cpu->cfg.ext_zve32f) { + if (cpu->cfg.ext_zvfhmin && !cpu->cfg.ext_zve32f_enabled) { error_setg(errp, "Zvfh/Zvfhmin extensions require Zve32f extension"); return; } @@ -988,13 +994,14 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) } 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) { + cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt) && + !cpu->cfg.ext_zca_enabled) { 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)) { + if (cpu->cfg.ext_zcd_enabled && (cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt)) { error_setg(errp, "Zcmp/Zcmt extensions are incompatible with " "Zcd extension"); return; diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 4af8ebc558..c59f099dd3 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -523,6 +523,14 @@ struct RISCVCPUConfig { bool debug; bool misa_w; + /* May implicitly enabled extensions by misa.EXT */ + bool ext_zca_enabled; + bool ext_zcf_enabled; + bool ext_zcd_enabled; + bool ext_zve32f_enabled; + bool ext_zve64f_enabled; + bool ext_zve64d_enabled; + bool short_isa_string; #ifndef CONFIG_USER_ONLY diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 433ea529b0..c9638ae905 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -51,7 +51,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, *pc = env->xl == MXL_RV32 ? env->pc & UINT32_MAX : env->pc; *cs_base = 0; - if (cpu->cfg.ext_zve32f) { + if (cpu->cfg.ext_zve32f_enabled) { /* * If env->vl equals to VLMAX, we can use generic vector operation * expanders (GVEC) to accerlate the vector operations. diff --git a/target/riscv/csr.c b/target/riscv/csr.c index f4d2dcfdc8..1639a1bdd6 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -88,7 +88,7 @@ static RISCVException fs(CPURISCVState *env, int csrno) static RISCVException vs(CPURISCVState *env, int csrno) { - if (riscv_cpu_cfg(env)->ext_zve32f) { + if (riscv_cpu_cfg(env)->ext_zve32f_enabled) { #if !defined(CONFIG_USER_ONLY) if (!env->debugger && !riscv_cpu_vector_enabled(env)) { return RISCV_EXCP_ILLEGAL_INST; diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc index 2c51e01c40..b0900a2274 100644 --- a/target/riscv/insn_trans/trans_rvd.c.inc +++ b/target/riscv/insn_trans/trans_rvd.c.inc @@ -32,7 +32,7 @@ } while (0) #define REQUIRE_ZCD(ctx) do { \ - if (!ctx->cfg_ptr->ext_zcd) { \ + if (!ctx->cfg_ptr->ext_zcd_enabled) { \ return false; \ } \ } while (0) diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc index 9e9fa2087a..6e810bab3c 100644 --- a/target/riscv/insn_trans/trans_rvf.c.inc +++ b/target/riscv/insn_trans/trans_rvf.c.inc @@ -31,7 +31,7 @@ } while (0) #define REQUIRE_ZCF(ctx) do { \ - if (!ctx->cfg_ptr->ext_zcf) { \ + if (!ctx->cfg_ptr->ext_zcf_enabled) { \ return false; \ } \ } while (0) diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc index c70c495fc5..8947176718 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 (!ctx->cfg_ptr->ext_zca) { + if (!ctx->cfg_ptr->ext_zca_enabled) { TCGv t0 = tcg_temp_new(); misaligned = gen_new_label(); @@ -169,7 +169,8 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond) gen_set_label(l); /* branch taken */ - if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) { + if (!ctx->cfg_ptr->ext_zca_enabled && + ((ctx->base.pc_next + a->imm) & 0x3)) { /* misaligned */ gen_exception_inst_addr_mis(ctx); } else { diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc index ca3c4c1a3d..fbd5b9b626 100644 --- a/target/riscv/insn_trans/trans_rvv.c.inc +++ b/target/riscv/insn_trans/trans_rvv.c.inc @@ -42,9 +42,9 @@ static bool require_rvf(DisasContext *s) case MO_16: return s->cfg_ptr->ext_zvfh; case MO_32: - return s->cfg_ptr->ext_zve32f; + return s->cfg_ptr->ext_zve32f_enabled; case MO_64: - return s->cfg_ptr->ext_zve64d; + return s->cfg_ptr->ext_zve64d_enabled; default: return false; } @@ -60,9 +60,9 @@ static bool require_scale_rvf(DisasContext *s) case MO_8: return s->cfg_ptr->ext_zvfh; case MO_16: - return s->cfg_ptr->ext_zve32f; + return s->cfg_ptr->ext_zve32f_enabled; case MO_32: - return s->cfg_ptr->ext_zve64d; + return s->cfg_ptr->ext_zve64d_enabled; default: return false; } @@ -78,9 +78,9 @@ static bool require_scale_rvfmin(DisasContext *s) case MO_8: return s->cfg_ptr->ext_zvfhmin; case MO_16: - return s->cfg_ptr->ext_zve32f; + return s->cfg_ptr->ext_zve32f_enabled; case MO_32: - return s->cfg_ptr->ext_zve64d; + return s->cfg_ptr->ext_zve64d_enabled; default: return false; } @@ -149,7 +149,7 @@ static bool do_vsetvl(DisasContext *s, int rd, int rs1, TCGv s2) { TCGv s1, dst; - if (!require_rvv(s) || !s->cfg_ptr->ext_zve32f) { + if (!require_rvv(s) || !s->cfg_ptr->ext_zve32f_enabled) { return false; } @@ -179,7 +179,7 @@ static bool do_vsetivli(DisasContext *s, int rd, TCGv s1, TCGv s2) { TCGv dst; - if (!require_rvv(s) || !s->cfg_ptr->ext_zve32f) { + if (!require_rvv(s) || !s->cfg_ptr->ext_zve32f_enabled) { return false; } diff --git a/target/riscv/translate.c b/target/riscv/translate.c index d0094922b6..3c0c5e58eb 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -551,7 +551,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm) /* check misaligned: */ next_pc = ctx->base.pc_next + imm; - if (!ctx->cfg_ptr->ext_zca) { + if (!ctx->cfg_ptr->ext_zca_enabled) { if ((next_pc & 0x3) != 0) { gen_exception_inst_addr_mis(ctx); return; @@ -1137,7 +1137,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t 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)) { + if (ctx->cfg_ptr->ext_zca_enabled && decode_insn16(ctx, opcode)) { return; } } else { -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions 2023-04-10 3:35 [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions Weiwei Li 2023-04-10 3:35 ` [PATCH 1/2] target/riscv: Add set_implicit_extensions_from_ext() function Weiwei Li 2023-04-10 3:35 ` [PATCH 2/2] target/riscv: Add ext_z*_enabled for implicitly enabled extensions Weiwei Li @ 2023-04-10 13:48 ` Daniel Henrique Barboza 2023-04-10 14:20 ` liweiwei 2023-04-11 0:15 ` liweiwei 2 siblings, 2 replies; 11+ messages in thread From: Daniel Henrique Barboza @ 2023-04-10 13:48 UTC (permalink / raw) To: Weiwei Li, qemu-riscv, qemu-devel Cc: palmer, alistair.francis, bin.meng, zhiwei_liu, wangjunqiang, lazyparser Hi, On 4/10/23 00:35, Weiwei Li wrote: > The patch tries to separate the multi-letter extensions that may implicitly-enabled by misa.EXT from the explicitly-enabled cases, so that the misa.EXT can truely disabled by write_misa(). > With this separation, the implicitly-enabled zve64d/f and zve32f extensions will no work if we clear misa.V. And clear misa.V will have no effect on the explicitly-enalbed zve64d/f and zve32f extensions. For this particular case of write_misa() I'm not sure if we need all that. If we want to grant user choice on write_misa(), let's say that the user wants to enable/disable RVV, we can enable/disable all RVV related Z-extensions by hand. It's just a matter of writing enable/disable code that write_misa() would use. In the end, write_misa() is also an user choice. If write_misa() wants to disable RVV, this means that the user wants to disable RVV, so it doesn't matter whether the user enabled zve32f on the command line or not - we disable zve32f as well. Same thing for RVC and its related Z-extensions. The reason why I didn't do this particular code for RVC and RVV is because we have pending work in the ML that I would like to get it merged first. And there's a few caveats we need to decide what to do, e.g. what if the user disables F but V is enabled? Do we refuse write_misa()? Do we disable RVV? All this said, patch 1 is still a good addition to make it easier to distinguish the Z-extensions we're enabling due to MISA bits. I believe we should use set_implicit_extensions_from_ext() in the future for all similar situations. Thanks, Daniel > > Weiwei Li (2): > target/riscv: Add set_implicit_extensions_from_ext() function > target/riscv: Add ext_z*_enabled for implicitly enabled extensions > > target/riscv/cpu.c | 73 +++++++++++++++---------- > target/riscv/cpu.h | 8 +++ > target/riscv/cpu_helper.c | 2 +- > target/riscv/csr.c | 2 +- > target/riscv/insn_trans/trans_rvd.c.inc | 2 +- > target/riscv/insn_trans/trans_rvf.c.inc | 2 +- > target/riscv/insn_trans/trans_rvi.c.inc | 5 +- > target/riscv/insn_trans/trans_rvv.c.inc | 16 +++--- > target/riscv/translate.c | 4 +- > 9 files changed, 68 insertions(+), 46 deletions(-) > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions 2023-04-10 13:48 ` [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions Daniel Henrique Barboza @ 2023-04-10 14:20 ` liweiwei 2023-04-10 18:35 ` Daniel Henrique Barboza 2023-04-11 0:15 ` liweiwei 1 sibling, 1 reply; 11+ messages in thread From: liweiwei @ 2023-04-10 14:20 UTC (permalink / raw) To: Daniel Henrique Barboza, qemu-riscv, qemu-devel Cc: liweiwei, palmer, alistair.francis, bin.meng, zhiwei_liu, wangjunqiang, lazyparser On 2023/4/10 21:48, Daniel Henrique Barboza wrote: > Hi, > > On 4/10/23 00:35, Weiwei Li wrote: >> The patch tries to separate the multi-letter extensions that may >> implicitly-enabled by misa.EXT from the explicitly-enabled cases, so >> that the misa.EXT can truely disabled by write_misa(). >> With this separation, the implicitly-enabled zve64d/f and zve32f >> extensions will no work if we clear misa.V. And clear misa.V will >> have no effect on the explicitly-enalbed zve64d/f and zve32f extensions. > > For this particular case of write_misa() I'm not sure if we need all > that. If we want > to grant user choice on write_misa(), let's say that the user wants to > enable/disable > RVV, we can enable/disable all RVV related Z-extensions by hand. It's > just a matter > of writing enable/disable code that write_misa() would use. > > In the end, write_misa() is also an user choice. If write_misa() wants > to disable RVV, > this means that the user wants to disable RVV, so it doesn't matter > whether the user > enabled zve32f on the command line or not - we disable zve32f as well. > Same thing for > RVC and its related Z-extensions. > Yeah. It's also a choice. It's a question whether we take C_Zca and C as the same user choice. If we consider them as different, then this patch works. And this patch can bypass the priv version problem. > The reason why I didn't do this particular code for RVC and RVV is > because we have > pending work in the ML that I would like to get it merged first. And > there's a few > caveats we need to decide what to do, e.g. what if the user disables F > but V is > enabled? Do we refuse write_misa()? Do we disable RVV? > In section 3.1.1 of privilege spec: "If an ISA feature x depends on an ISA feature y, then attempting to enable feature x but disable feature y results in both features being disabled." Even though there is also another description in the following content of the same section: "An implementation may impose additional constraints on the collective setting of two or more misa fields, in which case they function collectively as a single WARL field. An attempt to write an unsupported combination causes those bits to be set to some supported combination." I think the former description is more explicit. Regards, Weiwei Li > > All this said, patch 1 is still a good addition to make it easier to > distinguish > the Z-extensions we're enabling due to MISA bits. I believe we should use > set_implicit_extensions_from_ext() in the future for all similar > situations. > > > > Thanks, > > Daniel > > > >> >> Weiwei Li (2): >> target/riscv: Add set_implicit_extensions_from_ext() function >> target/riscv: Add ext_z*_enabled for implicitly enabled extensions >> >> target/riscv/cpu.c | 73 +++++++++++++++---------- >> target/riscv/cpu.h | 8 +++ >> target/riscv/cpu_helper.c | 2 +- >> target/riscv/csr.c | 2 +- >> target/riscv/insn_trans/trans_rvd.c.inc | 2 +- >> target/riscv/insn_trans/trans_rvf.c.inc | 2 +- >> target/riscv/insn_trans/trans_rvi.c.inc | 5 +- >> target/riscv/insn_trans/trans_rvv.c.inc | 16 +++--- >> target/riscv/translate.c | 4 +- >> 9 files changed, 68 insertions(+), 46 deletions(-) >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions 2023-04-10 14:20 ` liweiwei @ 2023-04-10 18:35 ` Daniel Henrique Barboza 2023-04-11 0:18 ` liweiwei 0 siblings, 1 reply; 11+ messages in thread From: Daniel Henrique Barboza @ 2023-04-10 18:35 UTC (permalink / raw) To: liweiwei, qemu-riscv, qemu-devel Cc: palmer, alistair.francis, bin.meng, zhiwei_liu, wangjunqiang, lazyparser On 4/10/23 11:20, liweiwei wrote: > > On 2023/4/10 21:48, Daniel Henrique Barboza wrote: >> Hi, >> >> On 4/10/23 00:35, Weiwei Li wrote: >>> The patch tries to separate the multi-letter extensions that may implicitly-enabled by misa.EXT from the explicitly-enabled cases, so that the misa.EXT can truely disabled by write_misa(). >>> With this separation, the implicitly-enabled zve64d/f and zve32f extensions will no work if we clear misa.V. And clear misa.V will have no effect on the explicitly-enalbed zve64d/f and zve32f extensions. >> >> For this particular case of write_misa() I'm not sure if we need all that. If we want >> to grant user choice on write_misa(), let's say that the user wants to enable/disable >> RVV, we can enable/disable all RVV related Z-extensions by hand. It's just a matter >> of writing enable/disable code that write_misa() would use. >> >> In the end, write_misa() is also an user choice. If write_misa() wants to disable RVV, >> this means that the user wants to disable RVV, so it doesn't matter whether the user >> enabled zve32f on the command line or not - we disable zve32f as well. Same thing for >> RVC and its related Z-extensions. >> > Yeah. It's also a choice. It's a question whether we take C_Zca and C as the same user choice. > > If we consider them as different, then this patch works. And this patch can bypass the priv version problem. > >> The reason why I didn't do this particular code for RVC and RVV is because we have >> pending work in the ML that I would like to get it merged first. And there's a few >> caveats we need to decide what to do, e.g. what if the user disables F but V is >> enabled? Do we refuse write_misa()? Do we disable RVV? >> > In section 3.1.1 of privilege spec: > > "If an ISA feature x depends on an ISA feature y, then attempting to enable feature x but disable > > feature y results in both features being disabled." > > Even though there is also another description in the following content of the same section: > > "An implementation may impose additional constraints on the collective setting of two or more misa > fields, in which case they function collectively as a single WARL field. An attempt to write an > unsupported combination causes those bits to be set to some supported combination." > > I think the former description is more explicit. Yeah. Disabling a MISA bit should disable all dependencies of the bit and there's not much to discuss about it. As far as the current write_misa() impl in the mailing list goes, we're refusing to disable the bit (e.g. the validation would fail if RVF is disabled while keeping all its dependencies, write_misa() becomes a no-op). If we decide to give users this kind of control I believe we should disregard all user choice during boot and enable/disable extensions as required. Thanks, Daniel > > Regards, > > Weiwei Li > >> >> All this said, patch 1 is still a good addition to make it easier to distinguish >> the Z-extensions we're enabling due to MISA bits. I believe we should use >> set_implicit_extensions_from_ext() in the future for all similar situations. >> >> >> >> Thanks, >> >> Daniel >> >> >> >>> >>> Weiwei Li (2): >>> target/riscv: Add set_implicit_extensions_from_ext() function >>> target/riscv: Add ext_z*_enabled for implicitly enabled extensions >>> >>> target/riscv/cpu.c | 73 +++++++++++++++---------- >>> target/riscv/cpu.h | 8 +++ >>> target/riscv/cpu_helper.c | 2 +- >>> target/riscv/csr.c | 2 +- >>> target/riscv/insn_trans/trans_rvd.c.inc | 2 +- >>> target/riscv/insn_trans/trans_rvf.c.inc | 2 +- >>> target/riscv/insn_trans/trans_rvi.c.inc | 5 +- >>> target/riscv/insn_trans/trans_rvv.c.inc | 16 +++--- >>> target/riscv/translate.c | 4 +- >>> 9 files changed, 68 insertions(+), 46 deletions(-) >>> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions 2023-04-10 18:35 ` Daniel Henrique Barboza @ 2023-04-11 0:18 ` liweiwei 0 siblings, 0 replies; 11+ messages in thread From: liweiwei @ 2023-04-11 0:18 UTC (permalink / raw) To: Daniel Henrique Barboza, qemu-riscv, qemu-devel Cc: liweiwei, palmer, alistair.francis, bin.meng, zhiwei_liu, wangjunqiang, lazyparser On 2023/4/11 02:35, Daniel Henrique Barboza wrote: > > > On 4/10/23 11:20, liweiwei wrote: >> >> On 2023/4/10 21:48, Daniel Henrique Barboza wrote: >>> Hi, >>> >>> On 4/10/23 00:35, Weiwei Li wrote: >>>> The patch tries to separate the multi-letter extensions that may >>>> implicitly-enabled by misa.EXT from the explicitly-enabled cases, >>>> so that the misa.EXT can truely disabled by write_misa(). >>>> With this separation, the implicitly-enabled zve64d/f and zve32f >>>> extensions will no work if we clear misa.V. And clear misa.V will >>>> have no effect on the explicitly-enalbed zve64d/f and zve32f >>>> extensions. >>> >>> For this particular case of write_misa() I'm not sure if we need all >>> that. If we want >>> to grant user choice on write_misa(), let's say that the user wants >>> to enable/disable >>> RVV, we can enable/disable all RVV related Z-extensions by hand. >>> It's just a matter >>> of writing enable/disable code that write_misa() would use. >>> >>> In the end, write_misa() is also an user choice. If write_misa() >>> wants to disable RVV, >>> this means that the user wants to disable RVV, so it doesn't matter >>> whether the user >>> enabled zve32f on the command line or not - we disable zve32f as >>> well. Same thing for >>> RVC and its related Z-extensions. >>> >> Yeah. It's also a choice. It's a question whether we take C_Zca and C >> as the same user choice. >> >> If we consider them as different, then this patch works. And this >> patch can bypass the priv version problem. >> >>> The reason why I didn't do this particular code for RVC and RVV is >>> because we have >>> pending work in the ML that I would like to get it merged first. And >>> there's a few >>> caveats we need to decide what to do, e.g. what if the user disables >>> F but V is >>> enabled? Do we refuse write_misa()? Do we disable RVV? >>> >> In section 3.1.1 of privilege spec: >> >> "If an ISA feature x depends on an ISA feature y, then attempting to >> enable feature x but disable >> >> feature y results in both features being disabled." >> >> Even though there is also another description in the following >> content of the same section: >> >> "An implementation may impose additional constraints on the >> collective setting of two or more misa >> fields, in which case they function collectively as a single WARL >> field. An attempt to write an >> unsupported combination causes those bits to be set to some supported >> combination." >> >> I think the former description is more explicit. > > Yeah. Disabling a MISA bit should disable all dependencies of the bit > and there's > not much to discuss about it. > > As far as the current write_misa() impl in the mailing list goes, > we're refusing to > disable the bit (e.g. the validation would fail if RVF is disabled > while keeping all > its dependencies, write_misa() becomes a no-op). > > If we decide to give users this kind of control I believe we should > disregard all user > choice during boot and enable/disable extensions as required. Sorry, I don't get your idea here. Why should we disregard all user choice during boot? Regards, Weiwei Li > > > > Thanks, > > Daniel > >> >> Regards, >> >> Weiwei Li >> >>> >>> All this said, patch 1 is still a good addition to make it easier to >>> distinguish >>> the Z-extensions we're enabling due to MISA bits. I believe we >>> should use >>> set_implicit_extensions_from_ext() in the future for all similar >>> situations. >>> >>> >>> >>> Thanks, >>> >>> Daniel >>> >>> >>> >>>> >>>> Weiwei Li (2): >>>> target/riscv: Add set_implicit_extensions_from_ext() function >>>> target/riscv: Add ext_z*_enabled for implicitly enabled extensions >>>> >>>> target/riscv/cpu.c | 73 >>>> +++++++++++++++---------- >>>> target/riscv/cpu.h | 8 +++ >>>> target/riscv/cpu_helper.c | 2 +- >>>> target/riscv/csr.c | 2 +- >>>> target/riscv/insn_trans/trans_rvd.c.inc | 2 +- >>>> target/riscv/insn_trans/trans_rvf.c.inc | 2 +- >>>> target/riscv/insn_trans/trans_rvi.c.inc | 5 +- >>>> target/riscv/insn_trans/trans_rvv.c.inc | 16 +++--- >>>> target/riscv/translate.c | 4 +- >>>> 9 files changed, 68 insertions(+), 46 deletions(-) >>>> >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions 2023-04-10 13:48 ` [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions Daniel Henrique Barboza 2023-04-10 14:20 ` liweiwei @ 2023-04-11 0:15 ` liweiwei 2023-04-11 23:48 ` Daniel Henrique Barboza 1 sibling, 1 reply; 11+ messages in thread From: liweiwei @ 2023-04-11 0:15 UTC (permalink / raw) To: Daniel Henrique Barboza, qemu-riscv, qemu-devel Cc: liweiwei, palmer, alistair.francis, bin.meng, zhiwei_liu, wangjunqiang, lazyparser On 2023/4/10 21:48, Daniel Henrique Barboza wrote: > Hi, > > On 4/10/23 00:35, Weiwei Li wrote: >> The patch tries to separate the multi-letter extensions that may >> implicitly-enabled by misa.EXT from the explicitly-enabled cases, so >> that the misa.EXT can truely disabled by write_misa(). >> With this separation, the implicitly-enabled zve64d/f and zve32f >> extensions will no work if we clear misa.V. And clear misa.V will >> have no effect on the explicitly-enalbed zve64d/f and zve32f extensions. > > For this particular case of write_misa() I'm not sure if we need all > that. If we want > to grant user choice on write_misa(), let's say that the user wants to > enable/disable > RVV, we can enable/disable all RVV related Z-extensions by hand. It's > just a matter > of writing enable/disable code that write_misa() would use. > > In the end, write_misa() is also an user choice. If write_misa() wants > to disable RVV, > this means that the user wants to disable RVV, so it doesn't matter > whether the user > enabled zve32f on the command line or not - we disable zve32f as well. > Same thing for > RVC and its related Z-extensions. I just find we should also separate the cases here. Zcmp/Zcmt require Zca. If we disable Zca when misa.C is cleared, whether we need disable Zcmp/Zcmt? If yes, then we need another new parameter(to indicate they are diabled by clearing misa.C ) to decide whether we should re-enable them when misa.C is set. If not, we should make clearing misa.C failed in this case. Regards, Weiwei Li > > The reason why I didn't do this particular code for RVC and RVV is > because we have > pending work in the ML that I would like to get it merged first. And > there's a few > caveats we need to decide what to do, e.g. what if the user disables F > but V is > enabled? Do we refuse write_misa()? Do we disable RVV? > > > All this said, patch 1 is still a good addition to make it easier to > distinguish > the Z-extensions we're enabling due to MISA bits. I believe we should use > set_implicit_extensions_from_ext() in the future for all similar > situations. > > > > Thanks, > > Daniel > > > >> >> Weiwei Li (2): >> target/riscv: Add set_implicit_extensions_from_ext() function >> target/riscv: Add ext_z*_enabled for implicitly enabled extensions >> >> target/riscv/cpu.c | 73 +++++++++++++++---------- >> target/riscv/cpu.h | 8 +++ >> target/riscv/cpu_helper.c | 2 +- >> target/riscv/csr.c | 2 +- >> target/riscv/insn_trans/trans_rvd.c.inc | 2 +- >> target/riscv/insn_trans/trans_rvf.c.inc | 2 +- >> target/riscv/insn_trans/trans_rvi.c.inc | 5 +- >> target/riscv/insn_trans/trans_rvv.c.inc | 16 +++--- >> target/riscv/translate.c | 4 +- >> 9 files changed, 68 insertions(+), 46 deletions(-) >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions 2023-04-11 0:15 ` liweiwei @ 2023-04-11 23:48 ` Daniel Henrique Barboza 0 siblings, 0 replies; 11+ messages in thread From: Daniel Henrique Barboza @ 2023-04-11 23:48 UTC (permalink / raw) To: liweiwei, qemu-riscv, qemu-devel Cc: palmer, alistair.francis, bin.meng, zhiwei_liu, wangjunqiang, lazyparser On 4/10/23 21:15, liweiwei wrote: > > On 2023/4/10 21:48, Daniel Henrique Barboza wrote: >> Hi, >> >> On 4/10/23 00:35, Weiwei Li wrote: >>> The patch tries to separate the multi-letter extensions that may implicitly-enabled by misa.EXT from the explicitly-enabled cases, so that the misa.EXT can truely disabled by write_misa(). >>> With this separation, the implicitly-enabled zve64d/f and zve32f extensions will no work if we clear misa.V. And clear misa.V will have no effect on the explicitly-enalbed zve64d/f and zve32f extensions. >> >> For this particular case of write_misa() I'm not sure if we need all that. If we want >> to grant user choice on write_misa(), let's say that the user wants to enable/disable >> RVV, we can enable/disable all RVV related Z-extensions by hand. It's just a matter >> of writing enable/disable code that write_misa() would use. >> >> In the end, write_misa() is also an user choice. If write_misa() wants to disable RVV, >> this means that the user wants to disable RVV, so it doesn't matter whether the user >> enabled zve32f on the command line or not - we disable zve32f as well. Same thing for >> RVC and its related Z-extensions. > > I just find we should also separate the cases here. Zcmp/Zcmt require Zca. > > If we disable Zca when misa.C is cleared, whether we need disable Zcmp/Zcmt? IMO write_misa() shouldn't be able to disable Z-extensions that it can't turn it back on. E.g. RVV disabling zve64d/f is ok because, if we re-enable RVV, they'll be re-enabled back. > > If yes, then we need another new parameter(to indicate they are diabled by > > clearing misa.C ) to decide whether we should re-enable them when misa.C is set. > > If not, we should make clearing misa.C failed in this case. I'd rather fail the operation. write_misa() should always make reversible operations. If a certain CSR write would put us in a state that we can't go back on, we should fail. For now I think I'll go back to that cleanup I made, rebase it, get one of Weiwei patches that fixes the sifive breakage I talked about in the other thread, and see if we can get that more simple version of write_misa() merged. We can continue these discussions on top of that code. Thanks, Daniel > > Regards, > > Weiwei Li > >> >> The reason why I didn't do this particular code for RVC and RVV is because we have >> pending work in the ML that I would like to get it merged first. And there's a few >> caveats we need to decide what to do, e.g. what if the user disables F but V is >> enabled? Do we refuse write_misa()? Do we disable RVV? >> >> >> All this said, patch 1 is still a good addition to make it easier to distinguish >> the Z-extensions we're enabling due to MISA bits. I believe we should use >> set_implicit_extensions_from_ext() in the future for all similar situations. >> >> >> >> Thanks, >> >> Daniel >> >> >> >>> >>> Weiwei Li (2): >>> target/riscv: Add set_implicit_extensions_from_ext() function >>> target/riscv: Add ext_z*_enabled for implicitly enabled extensions >>> >>> target/riscv/cpu.c | 73 +++++++++++++++---------- >>> target/riscv/cpu.h | 8 +++ >>> target/riscv/cpu_helper.c | 2 +- >>> target/riscv/csr.c | 2 +- >>> target/riscv/insn_trans/trans_rvd.c.inc | 2 +- >>> target/riscv/insn_trans/trans_rvf.c.inc | 2 +- >>> target/riscv/insn_trans/trans_rvi.c.inc | 5 +- >>> target/riscv/insn_trans/trans_rvv.c.inc | 16 +++--- >>> target/riscv/translate.c | 4 +- >>> 9 files changed, 68 insertions(+), 46 deletions(-) >>> > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-04-12 2:53 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-10 3:35 [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions Weiwei Li 2023-04-10 3:35 ` [PATCH 1/2] target/riscv: Add set_implicit_extensions_from_ext() function Weiwei Li 2023-04-10 13:28 ` Daniel Henrique Barboza 2023-04-12 2:51 ` Alistair Francis 2023-04-10 3:35 ` [PATCH 2/2] target/riscv: Add ext_z*_enabled for implicitly enabled extensions Weiwei Li 2023-04-10 13:48 ` [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions Daniel Henrique Barboza 2023-04-10 14:20 ` liweiwei 2023-04-10 18:35 ` Daniel Henrique Barboza 2023-04-11 0:18 ` liweiwei 2023-04-11 0:15 ` liweiwei 2023-04-11 23:48 ` Daniel Henrique Barboza
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).