* [PATCH v3 0/1] target/riscv: Add Zihintpause support @ 2022-06-09 4:40 Dao Lu 2022-06-09 4:40 ` [PATCH v3 1/1] " Dao Lu 0 siblings, 1 reply; 8+ messages in thread From: Dao Lu @ 2022-06-09 4:40 UTC (permalink / raw) To: qemu-devel Cc: Dao Lu, Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V TCG CPUs This patch adds RISC-V Zihintpause support. The extension is set to be enabled by default and opcode has been added to insn32.decode. Added trans_pause for TCG to mainly to break reservation and exit the TB. The change can also be found in: https://github.com/dlu42/qemu/tree/zihintpause_support_v1 Tested along with pause support added to cpu_relax function for linux, the changes I made to linux to test can be found here: https://github.com/dlu42/linux/tree/pause_support_v1 -------- Changelog: v1 -> v2 1. Pause now also exit the TB and return to main loop 2. Move the REQUIRE_ZIHINTPAUSE macro inside the trans_pause function v2 -> v3 No changes, v2 was lost from the list Dao Lu (1): Add Zihintpause support target/riscv/cpu.c | 2 ++ target/riscv/cpu.h | 1 + target/riscv/insn32.decode | 7 ++++++- target/riscv/insn_trans/trans_rvi.c.inc | 18 ++++++++++++++++++ 4 files changed, 27 insertions(+), 1 deletion(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/1] target/riscv: Add Zihintpause support 2022-06-09 4:40 [PATCH v3 0/1] target/riscv: Add Zihintpause support Dao Lu @ 2022-06-09 4:40 ` Dao Lu 2022-06-20 23:39 ` Alistair Francis 0 siblings, 1 reply; 8+ messages in thread From: Dao Lu @ 2022-06-09 4:40 UTC (permalink / raw) To: qemu-devel Cc: Dao Lu, Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V TCG CPUs, Heiko Stuebner Added support for RISC-V PAUSE instruction from Zihintpause extension, enabled by default. Tested-by: Heiko Stuebner <heiko@sntech.de> Signed-off-by: Dao Lu <daolu@rivosinc.com> --- target/riscv/cpu.c | 2 ++ target/riscv/cpu.h | 1 + target/riscv/insn32.decode | 7 ++++++- target/riscv/insn_trans/trans_rvi.c.inc | 18 ++++++++++++++++++ 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index ccacdee215..183fb37fdf 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -825,6 +825,7 @@ static Property riscv_cpu_properties[] = { DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true), DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true), DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true), + DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true), DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false), DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false), DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false), @@ -996,6 +997,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) * extensions by an underscore. */ struct isa_ext_data isa_edata_arr[] = { + ISA_EDATA_ENTRY(zihintpause, ext_zihintpause), ISA_EDATA_ENTRY(zfh, ext_zfh), ISA_EDATA_ENTRY(zfhmin, ext_zfhmin), ISA_EDATA_ENTRY(zfinx, ext_zfinx), diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index fe6c9a2c92..e466a04a59 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -394,6 +394,7 @@ struct RISCVCPUConfig { bool ext_counters; bool ext_ifencei; bool ext_icsr; + bool ext_zihintpause; bool ext_svinval; bool ext_svnapot; bool ext_svpbmt; diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode index 4033565393..595fdcdad8 100644 --- a/target/riscv/insn32.decode +++ b/target/riscv/insn32.decode @@ -149,7 +149,12 @@ srl 0000000 ..... ..... 101 ..... 0110011 @r sra 0100000 ..... ..... 101 ..... 0110011 @r or 0000000 ..... ..... 110 ..... 0110011 @r and 0000000 ..... ..... 111 ..... 0110011 @r -fence ---- pred:4 succ:4 ----- 000 ----- 0001111 + +{ + pause 0000 0001 0000 00000 000 00000 0001111 + fence ---- pred:4 succ:4 ----- 000 ----- 0001111 +} + fence_i ---- ---- ---- ----- 001 ----- 0001111 csrrw ............ ..... 001 ..... 1110011 @csr csrrs ............ ..... 010 ..... 1110011 @csr diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc index f1342f30f8..ca75e05f4b 100644 --- a/target/riscv/insn_trans/trans_rvi.c.inc +++ b/target/riscv/insn_trans/trans_rvi.c.inc @@ -796,6 +796,24 @@ static bool trans_srad(DisasContext *ctx, arg_srad *a) return gen_shift(ctx, a, EXT_SIGN, tcg_gen_sar_tl, NULL); } +static bool trans_pause(DisasContext *ctx, arg_pause *a) +{ + if (!ctx->cfg_ptr->ext_zihintpause) { + return false; + } + + /* + * PAUSE is a no-op in QEMU, + * however we need to clear the reservation, + * end the TB and return to main loop + */ + tcg_gen_movi_tl(load_res, -1); + gen_set_pc_imm(ctx, ctx->pc_succ_insn); + tcg_gen_exit_tb(NULL, 0); + ctx->base.is_jmp = DISAS_NORETURN; + + return true; +} static bool trans_fence(DisasContext *ctx, arg_fence *a) { -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] target/riscv: Add Zihintpause support 2022-06-09 4:40 ` [PATCH v3 1/1] " Dao Lu @ 2022-06-20 23:39 ` Alistair Francis 2022-06-21 16:17 ` Dao Lu 0 siblings, 1 reply; 8+ messages in thread From: Alistair Francis @ 2022-06-20 23:39 UTC (permalink / raw) To: Dao Lu Cc: qemu-devel@nongnu.org Developers, Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V TCG CPUs, Heiko Stuebner On Thu, Jun 9, 2022 at 2:42 PM Dao Lu <daolu@rivosinc.com> wrote: > > Added support for RISC-V PAUSE instruction from Zihintpause extension, > enabled by default. > > Tested-by: Heiko Stuebner <heiko@sntech.de> > Signed-off-by: Dao Lu <daolu@rivosinc.com> > --- > target/riscv/cpu.c | 2 ++ > target/riscv/cpu.h | 1 + > target/riscv/insn32.decode | 7 ++++++- > target/riscv/insn_trans/trans_rvi.c.inc | 18 ++++++++++++++++++ > 4 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index ccacdee215..183fb37fdf 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -825,6 +825,7 @@ static Property riscv_cpu_properties[] = { > DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true), > DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true), > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true), > + DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true), > DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false), > DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false), > DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false), > @@ -996,6 +997,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) > * extensions by an underscore. > */ > struct isa_ext_data isa_edata_arr[] = { > + ISA_EDATA_ENTRY(zihintpause, ext_zihintpause), > ISA_EDATA_ENTRY(zfh, ext_zfh), > ISA_EDATA_ENTRY(zfhmin, ext_zfhmin), > ISA_EDATA_ENTRY(zfinx, ext_zfinx), > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index fe6c9a2c92..e466a04a59 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -394,6 +394,7 @@ struct RISCVCPUConfig { > bool ext_counters; > bool ext_ifencei; > bool ext_icsr; > + bool ext_zihintpause; > bool ext_svinval; > bool ext_svnapot; > bool ext_svpbmt; > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode > index 4033565393..595fdcdad8 100644 > --- a/target/riscv/insn32.decode > +++ b/target/riscv/insn32.decode > @@ -149,7 +149,12 @@ srl 0000000 ..... ..... 101 ..... 0110011 @r > sra 0100000 ..... ..... 101 ..... 0110011 @r > or 0000000 ..... ..... 110 ..... 0110011 @r > and 0000000 ..... ..... 111 ..... 0110011 @r > -fence ---- pred:4 succ:4 ----- 000 ----- 0001111 > + > +{ > + pause 0000 0001 0000 00000 000 00000 0001111 > + fence ---- pred:4 succ:4 ----- 000 ----- 0001111 > +} > + > fence_i ---- ---- ---- ----- 001 ----- 0001111 > csrrw ............ ..... 001 ..... 1110011 @csr > csrrs ............ ..... 010 ..... 1110011 @csr > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc > index f1342f30f8..ca75e05f4b 100644 > --- a/target/riscv/insn_trans/trans_rvi.c.inc > +++ b/target/riscv/insn_trans/trans_rvi.c.inc > @@ -796,6 +796,24 @@ static bool trans_srad(DisasContext *ctx, arg_srad *a) > return gen_shift(ctx, a, EXT_SIGN, tcg_gen_sar_tl, NULL); > } > > +static bool trans_pause(DisasContext *ctx, arg_pause *a) > +{ > + if (!ctx->cfg_ptr->ext_zihintpause) { > + return false; > + } > + > + /* > + * PAUSE is a no-op in QEMU, > + * however we need to clear the reservation, > + * end the TB and return to main loop > + */ > + tcg_gen_movi_tl(load_res, -1); I'm not clear why we need to clear the load_res? We don't do it for fence instruction Alistair > + gen_set_pc_imm(ctx, ctx->pc_succ_insn); > + tcg_gen_exit_tb(NULL, 0); > + ctx->base.is_jmp = DISAS_NORETURN; > + > + return true; > +} > > static bool trans_fence(DisasContext *ctx, arg_fence *a) > { > -- > 2.25.1 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] target/riscv: Add Zihintpause support 2022-06-20 23:39 ` Alistair Francis @ 2022-06-21 16:17 ` Dao Lu 2022-06-27 5:12 ` Alistair Francis 0 siblings, 1 reply; 8+ messages in thread From: Dao Lu @ 2022-06-21 16:17 UTC (permalink / raw) To: Alistair Francis Cc: qemu-devel@nongnu.org Developers, Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V TCG CPUs, Heiko Stuebner From what I know that's generally the way reservations are handled: if the forward progress requirements aren't met then the implementation is free to break any outstanding reservations (the hardware is always free to do that to a degree, but once forward progress is gone it can always do that). So this is legal, as would be not breaking the reservation. I don't have a strong opinion on this and am fine about changing it if anyone does. Thanks, Dao On Mon, Jun 20, 2022 at 4:39 PM Alistair Francis <alistair23@gmail.com> wrote: > > On Thu, Jun 9, 2022 at 2:42 PM Dao Lu <daolu@rivosinc.com> wrote: > > > > Added support for RISC-V PAUSE instruction from Zihintpause extension, > > enabled by default. > > > > Tested-by: Heiko Stuebner <heiko@sntech.de> > > Signed-off-by: Dao Lu <daolu@rivosinc.com> > > --- > > target/riscv/cpu.c | 2 ++ > > target/riscv/cpu.h | 1 + > > target/riscv/insn32.decode | 7 ++++++- > > target/riscv/insn_trans/trans_rvi.c.inc | 18 ++++++++++++++++++ > > 4 files changed, 27 insertions(+), 1 deletion(-) > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > index ccacdee215..183fb37fdf 100644 > > --- a/target/riscv/cpu.c > > +++ b/target/riscv/cpu.c > > @@ -825,6 +825,7 @@ static Property riscv_cpu_properties[] = { > > DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true), > > DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true), > > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true), > > + DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true), > > DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false), > > DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false), > > DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false), > > @@ -996,6 +997,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) > > * extensions by an underscore. > > */ > > struct isa_ext_data isa_edata_arr[] = { > > + ISA_EDATA_ENTRY(zihintpause, ext_zihintpause), > > ISA_EDATA_ENTRY(zfh, ext_zfh), > > ISA_EDATA_ENTRY(zfhmin, ext_zfhmin), > > ISA_EDATA_ENTRY(zfinx, ext_zfinx), > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > > index fe6c9a2c92..e466a04a59 100644 > > --- a/target/riscv/cpu.h > > +++ b/target/riscv/cpu.h > > @@ -394,6 +394,7 @@ struct RISCVCPUConfig { > > bool ext_counters; > > bool ext_ifencei; > > bool ext_icsr; > > + bool ext_zihintpause; > > bool ext_svinval; > > bool ext_svnapot; > > bool ext_svpbmt; > > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode > > index 4033565393..595fdcdad8 100644 > > --- a/target/riscv/insn32.decode > > +++ b/target/riscv/insn32.decode > > @@ -149,7 +149,12 @@ srl 0000000 ..... ..... 101 ..... 0110011 @r > > sra 0100000 ..... ..... 101 ..... 0110011 @r > > or 0000000 ..... ..... 110 ..... 0110011 @r > > and 0000000 ..... ..... 111 ..... 0110011 @r > > -fence ---- pred:4 succ:4 ----- 000 ----- 0001111 > > + > > +{ > > + pause 0000 0001 0000 00000 000 00000 0001111 > > + fence ---- pred:4 succ:4 ----- 000 ----- 0001111 > > +} > > + > > fence_i ---- ---- ---- ----- 001 ----- 0001111 > > csrrw ............ ..... 001 ..... 1110011 @csr > > csrrs ............ ..... 010 ..... 1110011 @csr > > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc > > index f1342f30f8..ca75e05f4b 100644 > > --- a/target/riscv/insn_trans/trans_rvi.c.inc > > +++ b/target/riscv/insn_trans/trans_rvi.c.inc > > @@ -796,6 +796,24 @@ static bool trans_srad(DisasContext *ctx, arg_srad *a) > > return gen_shift(ctx, a, EXT_SIGN, tcg_gen_sar_tl, NULL); > > } > > > > +static bool trans_pause(DisasContext *ctx, arg_pause *a) > > +{ > > + if (!ctx->cfg_ptr->ext_zihintpause) { > > + return false; > > + } > > + > > + /* > > + * PAUSE is a no-op in QEMU, > > + * however we need to clear the reservation, > > + * end the TB and return to main loop > > + */ > > + tcg_gen_movi_tl(load_res, -1); > > I'm not clear why we need to clear the load_res? We don't do it for > fence instruction > > Alistair > > > + gen_set_pc_imm(ctx, ctx->pc_succ_insn); > > + tcg_gen_exit_tb(NULL, 0); > > + ctx->base.is_jmp = DISAS_NORETURN; > > + > > + return true; > > +} > > > > static bool trans_fence(DisasContext *ctx, arg_fence *a) > > { > > -- > > 2.25.1 > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] target/riscv: Add Zihintpause support 2022-06-21 16:17 ` Dao Lu @ 2022-06-27 5:12 ` Alistair Francis 2022-06-27 15:55 ` Dao Lu 0 siblings, 1 reply; 8+ messages in thread From: Alistair Francis @ 2022-06-27 5:12 UTC (permalink / raw) To: Dao Lu Cc: qemu-devel@nongnu.org Developers, Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V TCG CPUs, Heiko Stuebner On Wed, Jun 22, 2022 at 2:17 AM Dao Lu <daolu@rivosinc.com> wrote: > > From what I know that's generally the way reservations are handled: > if the forward progress requirements aren't met then the implementation > is free to break any outstanding reservations (the hardware is always > free to do that to a degree, but once forward progress is gone it can > always do that). So this is legal, as would be not breaking the reservation. I'm thinking let's not break the reservation. That way we are consistent with the fence instruction. If we do want to clear the reservation then we should do it for fence as well. Alistair > > I don't have a strong opinion on this and am fine about changing it if > anyone does. > > Thanks, > Dao > > On Mon, Jun 20, 2022 at 4:39 PM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Thu, Jun 9, 2022 at 2:42 PM Dao Lu <daolu@rivosinc.com> wrote: > > > > > > Added support for RISC-V PAUSE instruction from Zihintpause extension, > > > enabled by default. > > > > > > Tested-by: Heiko Stuebner <heiko@sntech.de> > > > Signed-off-by: Dao Lu <daolu@rivosinc.com> > > > --- > > > target/riscv/cpu.c | 2 ++ > > > target/riscv/cpu.h | 1 + > > > target/riscv/insn32.decode | 7 ++++++- > > > target/riscv/insn_trans/trans_rvi.c.inc | 18 ++++++++++++++++++ > > > 4 files changed, 27 insertions(+), 1 deletion(-) > > > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > > index ccacdee215..183fb37fdf 100644 > > > --- a/target/riscv/cpu.c > > > +++ b/target/riscv/cpu.c > > > @@ -825,6 +825,7 @@ static Property riscv_cpu_properties[] = { > > > DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true), > > > DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true), > > > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true), > > > + DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true), > > > DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false), > > > DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false), > > > DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false), > > > @@ -996,6 +997,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) > > > * extensions by an underscore. > > > */ > > > struct isa_ext_data isa_edata_arr[] = { > > > + ISA_EDATA_ENTRY(zihintpause, ext_zihintpause), > > > ISA_EDATA_ENTRY(zfh, ext_zfh), > > > ISA_EDATA_ENTRY(zfhmin, ext_zfhmin), > > > ISA_EDATA_ENTRY(zfinx, ext_zfinx), > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > > > index fe6c9a2c92..e466a04a59 100644 > > > --- a/target/riscv/cpu.h > > > +++ b/target/riscv/cpu.h > > > @@ -394,6 +394,7 @@ struct RISCVCPUConfig { > > > bool ext_counters; > > > bool ext_ifencei; > > > bool ext_icsr; > > > + bool ext_zihintpause; > > > bool ext_svinval; > > > bool ext_svnapot; > > > bool ext_svpbmt; > > > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode > > > index 4033565393..595fdcdad8 100644 > > > --- a/target/riscv/insn32.decode > > > +++ b/target/riscv/insn32.decode > > > @@ -149,7 +149,12 @@ srl 0000000 ..... ..... 101 ..... 0110011 @r > > > sra 0100000 ..... ..... 101 ..... 0110011 @r > > > or 0000000 ..... ..... 110 ..... 0110011 @r > > > and 0000000 ..... ..... 111 ..... 0110011 @r > > > -fence ---- pred:4 succ:4 ----- 000 ----- 0001111 > > > + > > > +{ > > > + pause 0000 0001 0000 00000 000 00000 0001111 > > > + fence ---- pred:4 succ:4 ----- 000 ----- 0001111 > > > +} > > > + > > > fence_i ---- ---- ---- ----- 001 ----- 0001111 > > > csrrw ............ ..... 001 ..... 1110011 @csr > > > csrrs ............ ..... 010 ..... 1110011 @csr > > > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc > > > index f1342f30f8..ca75e05f4b 100644 > > > --- a/target/riscv/insn_trans/trans_rvi.c.inc > > > +++ b/target/riscv/insn_trans/trans_rvi.c.inc > > > @@ -796,6 +796,24 @@ static bool trans_srad(DisasContext *ctx, arg_srad *a) > > > return gen_shift(ctx, a, EXT_SIGN, tcg_gen_sar_tl, NULL); > > > } > > > > > > +static bool trans_pause(DisasContext *ctx, arg_pause *a) > > > +{ > > > + if (!ctx->cfg_ptr->ext_zihintpause) { > > > + return false; > > > + } > > > + > > > + /* > > > + * PAUSE is a no-op in QEMU, > > > + * however we need to clear the reservation, > > > + * end the TB and return to main loop > > > + */ > > > + tcg_gen_movi_tl(load_res, -1); > > > > I'm not clear why we need to clear the load_res? We don't do it for > > fence instruction > > > > Alistair > > > > > + gen_set_pc_imm(ctx, ctx->pc_succ_insn); > > > + tcg_gen_exit_tb(NULL, 0); > > > + ctx->base.is_jmp = DISAS_NORETURN; > > > + > > > + return true; > > > +} > > > > > > static bool trans_fence(DisasContext *ctx, arg_fence *a) > > > { > > > -- > > > 2.25.1 > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] target/riscv: Add Zihintpause support 2022-06-27 5:12 ` Alistair Francis @ 2022-06-27 15:55 ` Dao Lu 0 siblings, 0 replies; 8+ messages in thread From: Dao Lu @ 2022-06-27 15:55 UTC (permalink / raw) To: Alistair Francis Cc: qemu-devel@nongnu.org Developers, Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V TCG CPUs, Heiko Stuebner That sounds reasonable to me. Will change in the next version. Thanks, Dao On Sun, Jun 26, 2022 at 10:13 PM Alistair Francis <alistair23@gmail.com> wrote: > > On Wed, Jun 22, 2022 at 2:17 AM Dao Lu <daolu@rivosinc.com> wrote: > > > > From what I know that's generally the way reservations are handled: > > if the forward progress requirements aren't met then the implementation > > is free to break any outstanding reservations (the hardware is always > > free to do that to a degree, but once forward progress is gone it can > > always do that). So this is legal, as would be not breaking the reservation. > > I'm thinking let's not break the reservation. That way we are > consistent with the fence instruction. If we do want to clear the > reservation then we should do it for fence as well. > > Alistair > > > > > I don't have a strong opinion on this and am fine about changing it if > > anyone does. > > > > Thanks, > > Dao > > > > On Mon, Jun 20, 2022 at 4:39 PM Alistair Francis <alistair23@gmail.com> wrote: > > > > > > On Thu, Jun 9, 2022 at 2:42 PM Dao Lu <daolu@rivosinc.com> wrote: > > > > > > > > Added support for RISC-V PAUSE instruction from Zihintpause extension, > > > > enabled by default. > > > > > > > > Tested-by: Heiko Stuebner <heiko@sntech.de> > > > > Signed-off-by: Dao Lu <daolu@rivosinc.com> > > > > --- > > > > target/riscv/cpu.c | 2 ++ > > > > target/riscv/cpu.h | 1 + > > > > target/riscv/insn32.decode | 7 ++++++- > > > > target/riscv/insn_trans/trans_rvi.c.inc | 18 ++++++++++++++++++ > > > > 4 files changed, 27 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > > > index ccacdee215..183fb37fdf 100644 > > > > --- a/target/riscv/cpu.c > > > > +++ b/target/riscv/cpu.c > > > > @@ -825,6 +825,7 @@ static Property riscv_cpu_properties[] = { > > > > DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true), > > > > DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true), > > > > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true), > > > > + DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true), > > > > DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false), > > > > DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false), > > > > DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false), > > > > @@ -996,6 +997,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) > > > > * extensions by an underscore. > > > > */ > > > > struct isa_ext_data isa_edata_arr[] = { > > > > + ISA_EDATA_ENTRY(zihintpause, ext_zihintpause), > > > > ISA_EDATA_ENTRY(zfh, ext_zfh), > > > > ISA_EDATA_ENTRY(zfhmin, ext_zfhmin), > > > > ISA_EDATA_ENTRY(zfinx, ext_zfinx), > > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > > > > index fe6c9a2c92..e466a04a59 100644 > > > > --- a/target/riscv/cpu.h > > > > +++ b/target/riscv/cpu.h > > > > @@ -394,6 +394,7 @@ struct RISCVCPUConfig { > > > > bool ext_counters; > > > > bool ext_ifencei; > > > > bool ext_icsr; > > > > + bool ext_zihintpause; > > > > bool ext_svinval; > > > > bool ext_svnapot; > > > > bool ext_svpbmt; > > > > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode > > > > index 4033565393..595fdcdad8 100644 > > > > --- a/target/riscv/insn32.decode > > > > +++ b/target/riscv/insn32.decode > > > > @@ -149,7 +149,12 @@ srl 0000000 ..... ..... 101 ..... 0110011 @r > > > > sra 0100000 ..... ..... 101 ..... 0110011 @r > > > > or 0000000 ..... ..... 110 ..... 0110011 @r > > > > and 0000000 ..... ..... 111 ..... 0110011 @r > > > > -fence ---- pred:4 succ:4 ----- 000 ----- 0001111 > > > > + > > > > +{ > > > > + pause 0000 0001 0000 00000 000 00000 0001111 > > > > + fence ---- pred:4 succ:4 ----- 000 ----- 0001111 > > > > +} > > > > + > > > > fence_i ---- ---- ---- ----- 001 ----- 0001111 > > > > csrrw ............ ..... 001 ..... 1110011 @csr > > > > csrrs ............ ..... 010 ..... 1110011 @csr > > > > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc > > > > index f1342f30f8..ca75e05f4b 100644 > > > > --- a/target/riscv/insn_trans/trans_rvi.c.inc > > > > +++ b/target/riscv/insn_trans/trans_rvi.c.inc > > > > @@ -796,6 +796,24 @@ static bool trans_srad(DisasContext *ctx, arg_srad *a) > > > > return gen_shift(ctx, a, EXT_SIGN, tcg_gen_sar_tl, NULL); > > > > } > > > > > > > > +static bool trans_pause(DisasContext *ctx, arg_pause *a) > > > > +{ > > > > + if (!ctx->cfg_ptr->ext_zihintpause) { > > > > + return false; > > > > + } > > > > + > > > > + /* > > > > + * PAUSE is a no-op in QEMU, > > > > + * however we need to clear the reservation, > > > > + * end the TB and return to main loop > > > > + */ > > > > + tcg_gen_movi_tl(load_res, -1); > > > > > > I'm not clear why we need to clear the load_res? We don't do it for > > > fence instruction > > > > > > Alistair > > > > > > > + gen_set_pc_imm(ctx, ctx->pc_succ_insn); > > > > + tcg_gen_exit_tb(NULL, 0); > > > > + ctx->base.is_jmp = DISAS_NORETURN; > > > > + > > > > + return true; > > > > +} > > > > > > > > static bool trans_fence(DisasContext *ctx, arg_fence *a) > > > > { > > > > -- > > > > 2.25.1 > > > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 0/1] target/riscv: Add Zihintpause support @ 2022-06-08 3:44 Dao Lu 2022-06-08 3:44 ` [PATCH v3 1/1] " Dao Lu 0 siblings, 1 reply; 8+ messages in thread From: Dao Lu @ 2022-06-08 3:44 UTC (permalink / raw) To: qemu-devel Cc: Dao Lu, Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V TCG CPUs This patch adds RISC-V Zihintpause support. The extension is set to be enabled by default and opcode has been added to insn32.decode. Added trans_pause for TCG to mainly to break reservation and exit the TB. The change can also be found in: https://github.com/dlu42/qemu/tree/zihintpause_support_v1 Tested along with pause support added to cpu_relax function for linux, the changes I made to linux to test can be found here: https://github.com/dlu42/linux/tree/pause_support_v1 -------- Changelog: v1 -> v2 1. Pause now also exit the TB and return to main loop 2. Move the REQUIRE_ZIHINTPAUSE macro inside the trans_pause function v2 -> v3 No changes, v2 was lost from the list Dao Lu (1): Add Zihintpause support target/riscv/cpu.c | 2 ++ target/riscv/cpu.h | 1 + target/riscv/insn32.decode | 7 ++++++- target/riscv/insn_trans/trans_rvi.c.inc | 18 ++++++++++++++++++ 4 files changed, 27 insertions(+), 1 deletion(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/1] target/riscv: Add Zihintpause support 2022-06-08 3:44 [PATCH v3 0/1] " Dao Lu @ 2022-06-08 3:44 ` Dao Lu 2022-06-08 18:26 ` Dao Lu 0 siblings, 1 reply; 8+ messages in thread From: Dao Lu @ 2022-06-08 3:44 UTC (permalink / raw) To: qemu-devel Cc: Dao Lu, Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V TCG CPUs, Heiko Stuebner Tested-by: Heiko Stuebner <heiko@sntech.de> Signed-off-by: Dao Lu <daolu@rivosinc.com> --- target/riscv/cpu.c | 2 ++ target/riscv/cpu.h | 1 + target/riscv/insn32.decode | 7 ++++++- target/riscv/insn_trans/trans_rvi.c.inc | 18 ++++++++++++++++++ 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index ccacdee215..183fb37fdf 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -825,6 +825,7 @@ static Property riscv_cpu_properties[] = { DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true), DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true), DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true), + DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true), DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false), DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false), DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false), @@ -996,6 +997,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) * extensions by an underscore. */ struct isa_ext_data isa_edata_arr[] = { + ISA_EDATA_ENTRY(zihintpause, ext_zihintpause), ISA_EDATA_ENTRY(zfh, ext_zfh), ISA_EDATA_ENTRY(zfhmin, ext_zfhmin), ISA_EDATA_ENTRY(zfinx, ext_zfinx), diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index fe6c9a2c92..e466a04a59 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -394,6 +394,7 @@ struct RISCVCPUConfig { bool ext_counters; bool ext_ifencei; bool ext_icsr; + bool ext_zihintpause; bool ext_svinval; bool ext_svnapot; bool ext_svpbmt; diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode index 4033565393..595fdcdad8 100644 --- a/target/riscv/insn32.decode +++ b/target/riscv/insn32.decode @@ -149,7 +149,12 @@ srl 0000000 ..... ..... 101 ..... 0110011 @r sra 0100000 ..... ..... 101 ..... 0110011 @r or 0000000 ..... ..... 110 ..... 0110011 @r and 0000000 ..... ..... 111 ..... 0110011 @r -fence ---- pred:4 succ:4 ----- 000 ----- 0001111 + +{ + pause 0000 0001 0000 00000 000 00000 0001111 + fence ---- pred:4 succ:4 ----- 000 ----- 0001111 +} + fence_i ---- ---- ---- ----- 001 ----- 0001111 csrrw ............ ..... 001 ..... 1110011 @csr csrrs ............ ..... 010 ..... 1110011 @csr diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc index f1342f30f8..ca75e05f4b 100644 --- a/target/riscv/insn_trans/trans_rvi.c.inc +++ b/target/riscv/insn_trans/trans_rvi.c.inc @@ -796,6 +796,24 @@ static bool trans_srad(DisasContext *ctx, arg_srad *a) return gen_shift(ctx, a, EXT_SIGN, tcg_gen_sar_tl, NULL); } +static bool trans_pause(DisasContext *ctx, arg_pause *a) +{ + if (!ctx->cfg_ptr->ext_zihintpause) { + return false; + } + + /* + * PAUSE is a no-op in QEMU, + * however we need to clear the reservation, + * end the TB and return to main loop + */ + tcg_gen_movi_tl(load_res, -1); + gen_set_pc_imm(ctx, ctx->pc_succ_insn); + tcg_gen_exit_tb(NULL, 0); + ctx->base.is_jmp = DISAS_NORETURN; + + return true; +} static bool trans_fence(DisasContext *ctx, arg_fence *a) { -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/1] target/riscv: Add Zihintpause support 2022-06-08 3:44 ` [PATCH v3 1/1] " Dao Lu @ 2022-06-08 18:26 ` Dao Lu 0 siblings, 0 replies; 8+ messages in thread From: Dao Lu @ 2022-06-08 18:26 UTC (permalink / raw) To: qemu-devel@nongnu.org Developers Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V TCG CPUs, Heiko Stuebner Please ignore, I have missed the commit message, will resend a v3. Sorry about that. Dao On Tue, Jun 7, 2022 at 8:44 PM Dao Lu <daolu@rivosinc.com> wrote: > > Tested-by: Heiko Stuebner <heiko@sntech.de> > Signed-off-by: Dao Lu <daolu@rivosinc.com> > --- > target/riscv/cpu.c | 2 ++ > target/riscv/cpu.h | 1 + > target/riscv/insn32.decode | 7 ++++++- > target/riscv/insn_trans/trans_rvi.c.inc | 18 ++++++++++++++++++ > 4 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index ccacdee215..183fb37fdf 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -825,6 +825,7 @@ static Property riscv_cpu_properties[] = { > DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true), > DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true), > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true), > + DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true), > DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false), > DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false), > DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false), > @@ -996,6 +997,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) > * extensions by an underscore. > */ > struct isa_ext_data isa_edata_arr[] = { > + ISA_EDATA_ENTRY(zihintpause, ext_zihintpause), > ISA_EDATA_ENTRY(zfh, ext_zfh), > ISA_EDATA_ENTRY(zfhmin, ext_zfhmin), > ISA_EDATA_ENTRY(zfinx, ext_zfinx), > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index fe6c9a2c92..e466a04a59 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -394,6 +394,7 @@ struct RISCVCPUConfig { > bool ext_counters; > bool ext_ifencei; > bool ext_icsr; > + bool ext_zihintpause; > bool ext_svinval; > bool ext_svnapot; > bool ext_svpbmt; > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode > index 4033565393..595fdcdad8 100644 > --- a/target/riscv/insn32.decode > +++ b/target/riscv/insn32.decode > @@ -149,7 +149,12 @@ srl 0000000 ..... ..... 101 ..... 0110011 @r > sra 0100000 ..... ..... 101 ..... 0110011 @r > or 0000000 ..... ..... 110 ..... 0110011 @r > and 0000000 ..... ..... 111 ..... 0110011 @r > -fence ---- pred:4 succ:4 ----- 000 ----- 0001111 > + > +{ > + pause 0000 0001 0000 00000 000 00000 0001111 > + fence ---- pred:4 succ:4 ----- 000 ----- 0001111 > +} > + > fence_i ---- ---- ---- ----- 001 ----- 0001111 > csrrw ............ ..... 001 ..... 1110011 @csr > csrrs ............ ..... 010 ..... 1110011 @csr > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc > index f1342f30f8..ca75e05f4b 100644 > --- a/target/riscv/insn_trans/trans_rvi.c.inc > +++ b/target/riscv/insn_trans/trans_rvi.c.inc > @@ -796,6 +796,24 @@ static bool trans_srad(DisasContext *ctx, arg_srad *a) > return gen_shift(ctx, a, EXT_SIGN, tcg_gen_sar_tl, NULL); > } > > +static bool trans_pause(DisasContext *ctx, arg_pause *a) > +{ > + if (!ctx->cfg_ptr->ext_zihintpause) { > + return false; > + } > + > + /* > + * PAUSE is a no-op in QEMU, > + * however we need to clear the reservation, > + * end the TB and return to main loop > + */ > + tcg_gen_movi_tl(load_res, -1); > + gen_set_pc_imm(ctx, ctx->pc_succ_insn); > + tcg_gen_exit_tb(NULL, 0); > + ctx->base.is_jmp = DISAS_NORETURN; > + > + return true; > +} > > static bool trans_fence(DisasContext *ctx, arg_fence *a) > { > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-06-27 16:01 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-09 4:40 [PATCH v3 0/1] target/riscv: Add Zihintpause support Dao Lu 2022-06-09 4:40 ` [PATCH v3 1/1] " Dao Lu 2022-06-20 23:39 ` Alistair Francis 2022-06-21 16:17 ` Dao Lu 2022-06-27 5:12 ` Alistair Francis 2022-06-27 15:55 ` Dao Lu -- strict thread matches above, loose matches on Subject: below -- 2022-06-08 3:44 [PATCH v3 0/1] " Dao Lu 2022-06-08 3:44 ` [PATCH v3 1/1] " Dao Lu 2022-06-08 18:26 ` Dao Lu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.