Re: [RFC PATCH v3] RISC-V: Add Zawrs ISA extension support

2022-06-29 Thread Alistair Francis
On Mon, Jun 27, 2022 at 6:16 PM Christoph Müllner
 wrote:
>
>
>
> On Mon, Jun 27, 2022 at 7:20 AM Alistair Francis  wrote:
>>
>> On Fri, Jun 24, 2022 at 1:31 AM Christoph Muellner
>>  wrote:
>> >
>> > This patch adds support for the Zawrs ISA extension.
>> > Given the current (incomplete) implementation of reservation sets
>> > there seems to be no way to provide a full emulation of the WRS
>> > instruction (wake on reservation set invalidation or timeout or
>> > interrupt). Therefore, we just pretend that an interrupt occured,
>> > exit the execution loop and finally continue execution.
>> >
>> > The specification can be found here:
>> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
>> >
>> > Note, that the Zawrs extension is not frozen or ratified yet.
>> > Therefore this patch is an RFC and not intended to get merged.
>> >
>> > Changes since v2:
>> > * Adjustments according to a specification change
>> > * Inline REQUIRE_ZAWRS() since it has only one user
>> >
>> > Changes since v1:
>> > * Adding zawrs to the ISA string that is passed to the kernel
>> >
>> > Signed-off-by: Christoph Müllner 
>> > ---
>> >  target/riscv/cpu.c  |  2 +
>> >  target/riscv/cpu.h  |  1 +
>> >  target/riscv/insn32.decode  |  4 ++
>> >  target/riscv/insn_trans/trans_rvzawrs.c.inc | 54 +
>> >  target/riscv/translate.c|  1 +
>> >  5 files changed, 62 insertions(+)
>> >  create mode 100644 target/riscv/insn_trans/trans_rvzawrs.c.inc
>> >
>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > index 05e6521351..6cb00fadff 100644
>> > --- a/target/riscv/cpu.c
>> > +++ b/target/riscv/cpu.c
>> > @@ -882,6 +882,7 @@ static Property riscv_cpu_extensions[] = {
>> >  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("zawrs", RISCVCPU, cfg.ext_zawrs, true),
>>
>> Would this be enabled by default?
>
>
> The "true" was a personal preference (I prefer to keep the argument list for 
> QEMU short)
> and I did not see any conflicts with existing behavior (no code should break).
> If you prefer otherwise or if I missed a policy I will change it.
>
>>
>>
>> >  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),
>> > @@ -1075,6 +1076,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char 
>> > **isa_str, int max_str_len)
>> >  ISA_EDATA_ENTRY(zicsr, ext_icsr),
>> >  ISA_EDATA_ENTRY(zifencei, ext_ifencei),
>> >  ISA_EDATA_ENTRY(zmmul, ext_zmmul),
>> > +ISA_EDATA_ENTRY(zawrs, ext_zawrs),
>> >  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 7d6397acdf..a22bc0fa09 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -380,6 +380,7 @@ struct RISCVCPUConfig {
>> >  bool ext_h;
>> >  bool ext_j;
>> >  bool ext_v;
>> > +bool ext_zawrs;
>> >  bool ext_zba;
>> >  bool ext_zbb;
>> >  bool ext_zbc;
>> > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
>> > index 4033565393..513ea227fe 100644
>> > --- a/target/riscv/insn32.decode
>> > +++ b/target/riscv/insn32.decode
>> > @@ -711,6 +711,10 @@ vsetvli 0 ... . 111 . 1010111 
>> >  @r2_zimm11
>> >  vsetivli11 .. . 111 . 1010111  @r2_zimm10
>> >  vsetvl  100 . . 111 . 1010111  @r
>> >
>> > +# *** Zawrs Standard Extension ***
>> > +wrs_nto1101 0 000 0 1110011
>> > +wrs_sto00011101 0 000 0 1110011
>> > +
>> >  # *** RV32 Zba Standard Extension ***
>> >  sh1add 001 .. 010 . 0110011 @r
>> >  sh2add 001 .. 100 . 0110011 @r
>> > diff --git a/target/riscv/insn_trans/trans_rvzawrs.c.inc 
>> > b/target/riscv/insn_trans/trans_rvzawrs.c.inc
>> > new file mode 100644
>> > index 00..d0df56378e
>> > --- /dev/null
>> > +++ b/target/riscv/insn_trans/trans_rvzawrs.c.inc
>> > @@ -0,0 +1,54 @@
>> > +/*
>> > + * RISC-V translation routines for the RISC-V Zawrs Extension.
>> > + *
>> > + * Copyright (c) 2022 Christoph Muellner, christoph.muell...@vrull.io
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify it
>> > + * under the terms and conditions of the GNU General Public License,
>> > + * version 2 or later, as published by the Free Software Foundation.
>> > + *
>> > + * This program is distributed in the hope it will be useful, but WITHOUT
>> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> > + * FITNESS 

Re: [RFC PATCH v3] RISC-V: Add Zawrs ISA extension support

2022-06-27 Thread Christoph Müllner
On Mon, Jun 27, 2022 at 7:20 AM Alistair Francis 
wrote:

> On Fri, Jun 24, 2022 at 1:31 AM Christoph Muellner
>  wrote:
> >
> > This patch adds support for the Zawrs ISA extension.
> > Given the current (incomplete) implementation of reservation sets
> > there seems to be no way to provide a full emulation of the WRS
> > instruction (wake on reservation set invalidation or timeout or
> > interrupt). Therefore, we just pretend that an interrupt occured,
> > exit the execution loop and finally continue execution.
> >
> > The specification can be found here:
> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Note, that the Zawrs extension is not frozen or ratified yet.
> > Therefore this patch is an RFC and not intended to get merged.
> >
> > Changes since v2:
> > * Adjustments according to a specification change
> > * Inline REQUIRE_ZAWRS() since it has only one user
> >
> > Changes since v1:
> > * Adding zawrs to the ISA string that is passed to the kernel
> >
> > Signed-off-by: Christoph Müllner 
> > ---
> >  target/riscv/cpu.c  |  2 +
> >  target/riscv/cpu.h  |  1 +
> >  target/riscv/insn32.decode  |  4 ++
> >  target/riscv/insn_trans/trans_rvzawrs.c.inc | 54 +
> >  target/riscv/translate.c|  1 +
> >  5 files changed, 62 insertions(+)
> >  create mode 100644 target/riscv/insn_trans/trans_rvzawrs.c.inc
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 05e6521351..6cb00fadff 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -882,6 +882,7 @@ static Property riscv_cpu_extensions[] = {
> >  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("zawrs", RISCVCPU, cfg.ext_zawrs, true),
>
> Would this be enabled by default?
>

The "true" was a personal preference (I prefer to keep the argument list
for QEMU short)
and I did not see any conflicts with existing behavior (no code should
break).
If you prefer otherwise or if I missed a policy I will change it.


>
> >  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),
> > @@ -1075,6 +1076,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu,
> char **isa_str, int max_str_len)
> >  ISA_EDATA_ENTRY(zicsr, ext_icsr),
> >  ISA_EDATA_ENTRY(zifencei, ext_ifencei),
> >  ISA_EDATA_ENTRY(zmmul, ext_zmmul),
> > +ISA_EDATA_ENTRY(zawrs, ext_zawrs),
> >  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 7d6397acdf..a22bc0fa09 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -380,6 +380,7 @@ struct RISCVCPUConfig {
> >  bool ext_h;
> >  bool ext_j;
> >  bool ext_v;
> > +bool ext_zawrs;
> >  bool ext_zba;
> >  bool ext_zbb;
> >  bool ext_zbc;
> > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> > index 4033565393..513ea227fe 100644
> > --- a/target/riscv/insn32.decode
> > +++ b/target/riscv/insn32.decode
> > @@ -711,6 +711,10 @@ vsetvli 0 ... . 111 .
> 1010111  @r2_zimm11
> >  vsetivli11 .. . 111 . 1010111  @r2_zimm10
> >  vsetvl  100 . . 111 . 1010111  @r
> >
> > +# *** Zawrs Standard Extension ***
> > +wrs_nto1101 0 000 0 1110011
> > +wrs_sto00011101 0 000 0 1110011
> > +
> >  # *** RV32 Zba Standard Extension ***
> >  sh1add 001 .. 010 . 0110011 @r
> >  sh2add 001 .. 100 . 0110011 @r
> > diff --git a/target/riscv/insn_trans/trans_rvzawrs.c.inc
> b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> > new file mode 100644
> > index 00..d0df56378e
> > --- /dev/null
> > +++ b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> > @@ -0,0 +1,54 @@
> > +/*
> > + * RISC-V translation routines for the RISC-V Zawrs Extension.
> > + *
> > + * Copyright (c) 2022 Christoph Muellner, christoph.muell...@vrull.io
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
> WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> along with
> > + 

Re: [RFC PATCH v3] RISC-V: Add Zawrs ISA extension support

2022-06-26 Thread Alistair Francis
On Fri, Jun 24, 2022 at 1:31 AM Christoph Muellner
 wrote:
>
> This patch adds support for the Zawrs ISA extension.
> Given the current (incomplete) implementation of reservation sets
> there seems to be no way to provide a full emulation of the WRS
> instruction (wake on reservation set invalidation or timeout or
> interrupt). Therefore, we just pretend that an interrupt occured,
> exit the execution loop and finally continue execution.
>
> The specification can be found here:
> https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
>
> Note, that the Zawrs extension is not frozen or ratified yet.
> Therefore this patch is an RFC and not intended to get merged.
>
> Changes since v2:
> * Adjustments according to a specification change
> * Inline REQUIRE_ZAWRS() since it has only one user
>
> Changes since v1:
> * Adding zawrs to the ISA string that is passed to the kernel
>
> Signed-off-by: Christoph Müllner 
> ---
>  target/riscv/cpu.c  |  2 +
>  target/riscv/cpu.h  |  1 +
>  target/riscv/insn32.decode  |  4 ++
>  target/riscv/insn_trans/trans_rvzawrs.c.inc | 54 +
>  target/riscv/translate.c|  1 +
>  5 files changed, 62 insertions(+)
>  create mode 100644 target/riscv/insn_trans/trans_rvzawrs.c.inc
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 05e6521351..6cb00fadff 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -882,6 +882,7 @@ static Property riscv_cpu_extensions[] = {
>  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("zawrs", RISCVCPU, cfg.ext_zawrs, true),

Would this be enabled by default?

>  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),
> @@ -1075,6 +1076,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char 
> **isa_str, int max_str_len)
>  ISA_EDATA_ENTRY(zicsr, ext_icsr),
>  ISA_EDATA_ENTRY(zifencei, ext_ifencei),
>  ISA_EDATA_ENTRY(zmmul, ext_zmmul),
> +ISA_EDATA_ENTRY(zawrs, ext_zawrs),
>  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 7d6397acdf..a22bc0fa09 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -380,6 +380,7 @@ struct RISCVCPUConfig {
>  bool ext_h;
>  bool ext_j;
>  bool ext_v;
> +bool ext_zawrs;
>  bool ext_zba;
>  bool ext_zbb;
>  bool ext_zbc;
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 4033565393..513ea227fe 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -711,6 +711,10 @@ vsetvli 0 ... . 111 . 1010111  
> @r2_zimm11
>  vsetivli11 .. . 111 . 1010111  @r2_zimm10
>  vsetvl  100 . . 111 . 1010111  @r
>
> +# *** Zawrs Standard Extension ***
> +wrs_nto1101 0 000 0 1110011
> +wrs_sto00011101 0 000 0 1110011
> +
>  # *** RV32 Zba Standard Extension ***
>  sh1add 001 .. 010 . 0110011 @r
>  sh2add 001 .. 100 . 0110011 @r
> diff --git a/target/riscv/insn_trans/trans_rvzawrs.c.inc 
> b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> new file mode 100644
> index 00..d0df56378e
> --- /dev/null
> +++ b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> @@ -0,0 +1,54 @@
> +/*
> + * RISC-V translation routines for the RISC-V Zawrs Extension.
> + *
> + * Copyright (c) 2022 Christoph Muellner, christoph.muell...@vrull.io
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +
> +static bool trans_wrs(DisasContext *ctx)
> +{
> +if (!ctx->cfg_ptr->ext_zawrs) {
> +return false;
> +}
> +
> +/*
> + * We may continue if one or more of the following conditions are met:
> + * a) The reservation set is invalid

Shouldn't this be valid?

> + * b) If WRS.STO, a short time since start of stall has elapsed
> + * c) An interrupt is observed
> + *
> + * A reservation set can 

Re: [RFC PATCH v3] RISC-V: Add Zawrs ISA extension support

2022-06-23 Thread Heiko Stübner
Am Donnerstag, 23. Juni 2022, 17:29:07 CEST schrieb Christoph Muellner:
> This patch adds support for the Zawrs ISA extension.
> Given the current (incomplete) implementation of reservation sets
> there seems to be no way to provide a full emulation of the WRS
> instruction (wake on reservation set invalidation or timeout or
> interrupt). Therefore, we just pretend that an interrupt occured,
> exit the execution loop and finally continue execution.
> 
> The specification can be found here:
> https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> 
> Note, that the Zawrs extension is not frozen or ratified yet.
> Therefore this patch is an RFC and not intended to get merged.
> 
> Changes since v2:
> * Adjustments according to a specification change
> * Inline REQUIRE_ZAWRS() since it has only one user
> 
> Changes since v1:
> * Adding zawrs to the ISA string that is passed to the kernel
> 
> Signed-off-by: Christoph Müllner 

on both rv64 and rv32
Tested-by: Heiko Stuebner 

> ---
>  target/riscv/cpu.c  |  2 +
>  target/riscv/cpu.h  |  1 +
>  target/riscv/insn32.decode  |  4 ++
>  target/riscv/insn_trans/trans_rvzawrs.c.inc | 54 +
>  target/riscv/translate.c|  1 +
>  5 files changed, 62 insertions(+)
>  create mode 100644 target/riscv/insn_trans/trans_rvzawrs.c.inc
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 05e6521351..6cb00fadff 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -882,6 +882,7 @@ static Property riscv_cpu_extensions[] = {
>  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("zawrs", RISCVCPU, cfg.ext_zawrs, 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),
> @@ -1075,6 +1076,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char 
> **isa_str, int max_str_len)
>  ISA_EDATA_ENTRY(zicsr, ext_icsr),
>  ISA_EDATA_ENTRY(zifencei, ext_ifencei),
>  ISA_EDATA_ENTRY(zmmul, ext_zmmul),
> +ISA_EDATA_ENTRY(zawrs, ext_zawrs),
>  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 7d6397acdf..a22bc0fa09 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -380,6 +380,7 @@ struct RISCVCPUConfig {
>  bool ext_h;
>  bool ext_j;
>  bool ext_v;
> +bool ext_zawrs;
>  bool ext_zba;
>  bool ext_zbb;
>  bool ext_zbc;
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 4033565393..513ea227fe 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -711,6 +711,10 @@ vsetvli 0 ... . 111 . 1010111  
> @r2_zimm11
>  vsetivli11 .. . 111 . 1010111  @r2_zimm10
>  vsetvl  100 . . 111 . 1010111  @r
>  
> +# *** Zawrs Standard Extension ***
> +wrs_nto1101 0 000 0 1110011
> +wrs_sto00011101 0 000 0 1110011
> +
>  # *** RV32 Zba Standard Extension ***
>  sh1add 001 .. 010 . 0110011 @r
>  sh2add 001 .. 100 . 0110011 @r
> diff --git a/target/riscv/insn_trans/trans_rvzawrs.c.inc 
> b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> new file mode 100644
> index 00..d0df56378e
> --- /dev/null
> +++ b/target/riscv/insn_trans/trans_rvzawrs.c.inc
> @@ -0,0 +1,54 @@
> +/*
> + * RISC-V translation routines for the RISC-V Zawrs Extension.
> + *
> + * Copyright (c) 2022 Christoph Muellner, christoph.muell...@vrull.io
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +
> +static bool trans_wrs(DisasContext *ctx)
> +{
> +if (!ctx->cfg_ptr->ext_zawrs) {
> +return false;
> +}
> +
> +/*
> + * We may continue if one or more of the following conditions are met:
> + * a) The reservation set is invalid
> + * b) If WRS.STO, a short time since start of stall has elapsed
> + * c) An interrupt is observed
> + *
> + * A reservation