1. Patchew
  2. QEMU
  3. [RFC PATCH 0/4] Smstateen FCSR implementation
  4. View patch

[RFC PATCH 3/4] target/riscv: check smstateen fcsr flag

Mayuresh Chitale posted 4 patches 2 years, 10 months ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
[RFC PATCH 3/4] target/riscv: check smstateen fcsr flag
Posted by Mayuresh Chitale 2 years, 10 months ago 
If misa.F and smstateen_fcsr_ok flag are clear then all the floating
point instructions must generate an appropriate exception.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 target/riscv/insn_trans/trans_rvf.c.inc   | 24 ++++++++++++++++++++---
 target/riscv/insn_trans/trans_rvzfh.c.inc |  4 ++++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
index 052408f45c..6173dace46 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -24,9 +24,27 @@
             return false; \
 } while (0)
 
-#define REQUIRE_ZFINX_OR_F(ctx) do {\
-    if (!ctx->cfg_ptr->ext_zfinx) { \
-        REQUIRE_EXT(ctx, RVF); \
+#ifndef CONFIG_USER_ONLY
+#define smstateen_fcsr_check(ctx) do { \
+    if (!ctx->smstateen_fcsr_ok) { \
+        if (ctx->virt_enabled) { \
+            generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT); \
+        } else { \
+            generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST); \
+        } \
+        return true; \
+    } \
+} while (0)
+#else
+#define smstateen_fcsr_check(ctx)
+#endif
+
+#define REQUIRE_ZFINX_OR_F(ctx) do { \
+    if (!has_ext(ctx, RVF)) { \
+        if (!ctx->cfg_ptr->ext_zfinx) { \
+            return false; \
+        } \
+        smstateen_fcsr_check(ctx); \
     } \
 } while (0)
 
diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc
index 74dde37ff7..304bee1002 100644
--- a/target/riscv/insn_trans/trans_rvzfh.c.inc
+++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
@@ -20,24 +20,28 @@
     if (!ctx->cfg_ptr->ext_zfh) {      \
         return false;         \
     }                         \
+    smstateen_fcsr_check(ctx); \
 } while (0)
 
 #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
     if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
         return false;                  \
     }                                  \
+    smstateen_fcsr_check(ctx); \
 } while (0)
 
 #define REQUIRE_ZFHMIN(ctx) do {              \
     if (!ctx->cfg_ptr->ext_zfhmin) {          \
         return false;                         \
     }                                         \
+    smstateen_fcsr_check(ctx); \
 } while (0)
 
 #define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {                 \
     if (!(ctx->cfg_ptr->ext_zfhmin || ctx->cfg_ptr->ext_zhinxmin)) { \
         return false;                                        \
     }                                                        \
+    smstateen_fcsr_check(ctx); \
 } while (0)
 
 static bool trans_flh(DisasContext *ctx, arg_flh *a)
-- 
2.34.1
Re: [RFC PATCH 3/4] target/riscv: check smstateen fcsr flag
Posted by Richard Henderson 2 years, 10 months ago 
On 4/10/23 07:13, Mayuresh Chitale wrote:
> +#ifndef CONFIG_USER_ONLY
> +#define smstateen_fcsr_check(ctx) do { \
> +    if (!ctx->smstateen_fcsr_ok) { \
> +        if (ctx->virt_enabled) { \
> +            generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT); \
> +        } else { \
> +            generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST); \
> +        } \
> +        return true; \
> +    } \
> +} while (0)
> +#else
> +#define smstateen_fcsr_check(ctx)
> +#endif
> +
> +#define REQUIRE_ZFINX_OR_F(ctx) do { \
> +    if (!has_ext(ctx, RVF)) { \
> +        if (!ctx->cfg_ptr->ext_zfinx) { \
> +            return false; \
> +        } \
> +        smstateen_fcsr_check(ctx); \
>       } \
>   } while (0)

As a matter of style, I strongly object to a *nested* macro returning from the calling 
function.  These should all be changed to normal functions of the form

     if (!require_xyz(ctx) || !require_abc(ctx)) {
         return something;
     }

etc.  insn_trans/trans_rvv.c.inc is much much cleaner in this respect.


r~
Re: [RFC PATCH 3/4] target/riscv: check smstateen fcsr flag
Posted by Mayuresh Chitale 2 years, 10 months ago 
On Tue, Apr 11, 2023 at 7:23 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/10/23 07:13, Mayuresh Chitale wrote:
> > +#ifndef CONFIG_USER_ONLY
> > +#define smstateen_fcsr_check(ctx) do { \
> > +    if (!ctx->smstateen_fcsr_ok) { \
> > +        if (ctx->virt_enabled) { \
> > +            generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT); \
> > +        } else { \
> > +            generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST); \
> > +        } \
> > +        return true; \
> > +    } \
> > +} while (0)
> > +#else
> > +#define smstateen_fcsr_check(ctx)
> > +#endif
> > +
> > +#define REQUIRE_ZFINX_OR_F(ctx) do { \
> > +    if (!has_ext(ctx, RVF)) { \
> > +        if (!ctx->cfg_ptr->ext_zfinx) { \
> > +            return false; \
> > +        } \
> > +        smstateen_fcsr_check(ctx); \
> >       } \
> >   } while (0)
>
> As a matter of style, I strongly object to a *nested* macro returning from the calling
> function.  These should all be changed to normal functions of the form
>
>      if (!require_xyz(ctx) || !require_abc(ctx)) {
>          return something;
>      }
>
> etc.  insn_trans/trans_rvv.c.inc is much much cleaner in this respect.
Ok. I will change smstateen_fcsr_check to a function.
>
>
> r~
Re: [RFC PATCH 3/4] target/riscv: check smstateen fcsr flag
Posted by liweiwei 2 years, 10 months ago 
On 2023/4/10 22:13, Mayuresh Chitale wrote:
> If misa.F and smstateen_fcsr_ok flag are clear then all the floating
> point instructions must generate an appropriate exception.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
>   target/riscv/insn_trans/trans_rvf.c.inc   | 24 ++++++++++++++++++++---
>   target/riscv/insn_trans/trans_rvzfh.c.inc |  4 ++++
>   2 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
> index 052408f45c..6173dace46 100644
> --- a/target/riscv/insn_trans/trans_rvf.c.inc
> +++ b/target/riscv/insn_trans/trans_rvf.c.inc
> @@ -24,9 +24,27 @@
>               return false; \
>   } while (0)
>   
> -#define REQUIRE_ZFINX_OR_F(ctx) do {\
> -    if (!ctx->cfg_ptr->ext_zfinx) { \
> -        REQUIRE_EXT(ctx, RVF); \
> +#ifndef CONFIG_USER_ONLY
> +#define smstateen_fcsr_check(ctx) do { \
> +    if (!ctx->smstateen_fcsr_ok) { \
> +        if (ctx->virt_enabled) { \
> +            generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT); \
> +        } else { \
> +            generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST); \
> +        } \

We can setctx->virt_inst_excp = ctx->virt_enabledand return false here.

Or we need store current opcode to bins before generate_exception.

>   
> +        return true; \
> +    } \
> +} while (0)
> +#else
> +#define smstateen_fcsr_check(ctx)
> +#endif
> +
> +#define REQUIRE_ZFINX_OR_F(ctx) do { \
> +    if (!has_ext(ctx, RVF)) { \
> +        if (!ctx->cfg_ptr->ext_zfinx) { \
> +            return false; \
> +        } \
> +        smstateen_fcsr_check(ctx); \
>       } \
>   } while (0)
>   
> diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc
> index 74dde37ff7..304bee1002 100644
> --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
> @@ -20,24 +20,28 @@
>       if (!ctx->cfg_ptr->ext_zfh) {      \
>           return false;         \
>       }                         \
> +    smstateen_fcsr_check(ctx); \
>   } while (0)
>   
>   #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
>       if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
>           return false;                  \
>       }                                  \
> +    smstateen_fcsr_check(ctx); \

It's better to remain "\" alignment here.

Similar to following cases.

Regards,

Weiwei Li

>   } while (0)
>   
>   #define REQUIRE_ZFHMIN(ctx) do {              \
>       if (!ctx->cfg_ptr->ext_zfhmin) {          \
>           return false;                         \
>       }                                         \
> +    smstateen_fcsr_check(ctx); \
>   } while (0)
>   
>   #define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {                 \
>       if (!(ctx->cfg_ptr->ext_zfhmin || ctx->cfg_ptr->ext_zhinxmin)) { \
>           return false;                                        \
>       }                                                        \
> +    smstateen_fcsr_check(ctx); \
>   } while (0)
>   
>   static bool trans_flh(DisasContext *ctx, arg_flh *a)
Re: [RFC PATCH 3/4] target/riscv: check smstateen fcsr flag
Posted by Mayuresh Chitale 2 years, 10 months ago 
On Mon, Apr 10, 2023 at 8:00 PM liweiwei <liweiwei@iscas.ac.cn> wrote:
>
>
> On 2023/4/10 22:13, Mayuresh Chitale wrote:
> > If misa.F and smstateen_fcsr_ok flag are clear then all the floating
> > point instructions must generate an appropriate exception.
> >
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > ---
> >   target/riscv/insn_trans/trans_rvf.c.inc   | 24 ++++++++++++++++++++---
> >   target/riscv/insn_trans/trans_rvzfh.c.inc |  4 ++++
> >   2 files changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
> > index 052408f45c..6173dace46 100644
> > --- a/target/riscv/insn_trans/trans_rvf.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvf.c.inc
> > @@ -24,9 +24,27 @@
> >               return false; \
> >   } while (0)
> >
> > -#define REQUIRE_ZFINX_OR_F(ctx) do {\
> > -    if (!ctx->cfg_ptr->ext_zfinx) { \
> > -        REQUIRE_EXT(ctx, RVF); \
> > +#ifndef CONFIG_USER_ONLY
> > +#define smstateen_fcsr_check(ctx) do { \
> > +    if (!ctx->smstateen_fcsr_ok) { \
> > +        if (ctx->virt_enabled) { \
> > +            generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT); \
> > +        } else { \
> > +            generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST); \
> > +        } \
>
> We can setctx->virt_inst_excp = ctx->virt_enabledand return false here.
Ok.
>
> Or we need store current opcode to bins before generate_exception.
>
> >
> > +        return true; \
> > +    } \
> > +} while (0)
> > +#else
> > +#define smstateen_fcsr_check(ctx)
> > +#endif
> > +
> > +#define REQUIRE_ZFINX_OR_F(ctx) do { \
> > +    if (!has_ext(ctx, RVF)) { \
> > +        if (!ctx->cfg_ptr->ext_zfinx) { \
> > +            return false; \
> > +        } \
> > +        smstateen_fcsr_check(ctx); \
> >       } \
> >   } while (0)
> >
> > diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc
> > index 74dde37ff7..304bee1002 100644
> > --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
> > @@ -20,24 +20,28 @@
> >       if (!ctx->cfg_ptr->ext_zfh) {      \
> >           return false;         \
> >       }                         \
> > +    smstateen_fcsr_check(ctx); \
> >   } while (0)
> >
> >   #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
> >       if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
> >           return false;                  \
> >       }                                  \
> > +    smstateen_fcsr_check(ctx); \
>
> It's better to remain "\" alignment here.
Ok.

>
> Similar to following cases.
>
> Regards,
>
> Weiwei Li
>
> >   } while (0)
> >
> >   #define REQUIRE_ZFHMIN(ctx) do {              \
> >       if (!ctx->cfg_ptr->ext_zfhmin) {          \
> >           return false;                         \
> >       }                                         \
> > +    smstateen_fcsr_check(ctx); \
> >   } while (0)
> >
> >   #define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {                 \
> >       if (!(ctx->cfg_ptr->ext_zfhmin || ctx->cfg_ptr->ext_zhinxmin)) { \
> >           return false;                                        \
> >       }                                                        \
> > +    smstateen_fcsr_check(ctx); \
> >   } while (0)
> >
> >   static bool trans_flh(DisasContext *ctx, arg_flh *a)
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.