* [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] target/riscv: Add Zihintpause support 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
* 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
* 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-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-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
* [PATCH v3 1/1] target/riscv: Add Zihintpause support
2022-06-09 4:40 [PATCH v3 0/1] " 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
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-08 3:44 [PATCH v3 0/1] target/riscv: Add Zihintpause support Dao Lu
2022-06-08 3:44 ` [PATCH v3 1/1] " Dao Lu
2022-06-08 18:26 ` Dao Lu
2022-06-09 4:40 [PATCH v3 0/1] " 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
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.