| Message ID | 20230414160202.1298242-4-mchitale@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | Smstateen FCSR | expand |
On 2023/4/15 00:02, 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_rvd.c.inc | 13 ++++++++---- > target/riscv/insn_trans/trans_rvf.c.inc | 24 +++++++++++++++++++---- > target/riscv/insn_trans/trans_rvzfh.c.inc | 18 ++++++++++++++--- > 3 files changed, 44 insertions(+), 11 deletions(-) > > diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc > index 2c51e01c40..d9e0cf116f 100644 > --- a/target/riscv/insn_trans/trans_rvd.c.inc > +++ b/target/riscv/insn_trans/trans_rvd.c.inc > @@ -18,10 +18,15 @@ > * this program. If not, see <http://www.gnu.org/licenses/>. > */ > > -#define REQUIRE_ZDINX_OR_D(ctx) do { \ > - if (!ctx->cfg_ptr->ext_zdinx) { \ > - REQUIRE_EXT(ctx, RVD); \ > - } \ > +#define REQUIRE_ZDINX_OR_D(ctx) do { \ > + if (!has_ext(ctx, RVD)) { \ > + if (!ctx->cfg_ptr->ext_zdinx) { \ > + return false; \ > + } \ > + if (!smstateen_fcsr_check(ctx)) { \ > + return false; \ > + } \ > + } \ > } while (0) > > #define REQUIRE_EVEN(ctx, reg) do { \ > diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc > index 9e9fa2087a..6bf6fe16be 100644 > --- a/target/riscv/insn_trans/trans_rvf.c.inc > +++ b/target/riscv/insn_trans/trans_rvf.c.inc > @@ -24,10 +24,26 @@ > return false; \ > } while (0) > > -#define REQUIRE_ZFINX_OR_F(ctx) do {\ > - if (!ctx->cfg_ptr->ext_zfinx) { \ > - REQUIRE_EXT(ctx, RVF); \ > - } \ > +static inline bool smstateen_fcsr_check(DisasContext *ctx) > +{ > +#ifndef CONFIG_USER_ONLY > + if (!has_ext(ctx, RVF) && !ctx->smstateen_fcsr_ok) { We needn't check RVF here. Two reasons: 1. This check only be done when RVF is diabled. 2. ctx->smstateen_fcsr_ok is always be true (set in last patch) when RVF is enabled > + ctx->virt_inst_excp = ctx->virt_enabled; > + return false; > + } > +#endif > + return true; > +} > + > +#define REQUIRE_ZFINX_OR_F(ctx) do { \ > + if (!has_ext(ctx, RVF)) { \ > + if (!ctx->cfg_ptr->ext_zfinx) { \ > + return false; \ > + } \ > + if (!smstateen_fcsr_check(ctx)) { \ > + return false; \ > + } \ > + } \ > } while (0) > > #define REQUIRE_ZCF(ctx) do { \ > diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc > index 74dde37ff7..74a125e4c0 100644 > --- a/target/riscv/insn_trans/trans_rvzfh.c.inc > +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc > @@ -16,28 +16,40 @@ > * this program. If not, see <http://www.gnu.org/licenses/>. > */ > > -#define REQUIRE_ZFH(ctx) do { \ > +#define REQUIRE_ZFH(ctx) do { \ > if (!ctx->cfg_ptr->ext_zfh) { \ > - return false; \ > - } \ > + return false; \ > + } \ > + if (!smstateen_fcsr_check(ctx)) { \ > + return false; \ > + } \ This is unnecessary here. This check is only for Zhinx. Similar to REQUIRE_ZFHMIN. > } while (0) > > #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \ > if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \ > return false; \ > } \ > + if (!smstateen_fcsr_check(ctx)) { \ > + return false; \ > + } \ This check is only for Zhinx here. So it's better to take the similar way as REQUIRE_ZFINX_OR_F. Similar to REQUIRE_ZFHMIN_OR_ZHINXMIN. Regards, Weiwei Li > } while (0) > > #define REQUIRE_ZFHMIN(ctx) do { \ > if (!ctx->cfg_ptr->ext_zfhmin) { \ > return false; \ > } \ > + if (!smstateen_fcsr_check(ctx)) { \ > + return false; \ > + } \ > } while (0) > > #define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do { \ > if (!(ctx->cfg_ptr->ext_zfhmin || ctx->cfg_ptr->ext_zhinxmin)) { \ > return false; \ > } \ > + if (!smstateen_fcsr_check(ctx)) { \ > + return false; \ > + } \ > } while (0) > > static bool trans_flh(DisasContext *ctx, arg_flh *a)
On Sat, Apr 15, 2023 at 6:55 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > > On 2023/4/15 00:02, 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_rvd.c.inc | 13 ++++++++---- > > target/riscv/insn_trans/trans_rvf.c.inc | 24 +++++++++++++++++++---- > > target/riscv/insn_trans/trans_rvzfh.c.inc | 18 ++++++++++++++--- > > 3 files changed, 44 insertions(+), 11 deletions(-) > > > > diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc > > index 2c51e01c40..d9e0cf116f 100644 > > --- a/target/riscv/insn_trans/trans_rvd.c.inc > > +++ b/target/riscv/insn_trans/trans_rvd.c.inc > > @@ -18,10 +18,15 @@ > > * this program. If not, see <http://www.gnu.org/licenses/>. > > */ > > > > -#define REQUIRE_ZDINX_OR_D(ctx) do { \ > > - if (!ctx->cfg_ptr->ext_zdinx) { \ > > - REQUIRE_EXT(ctx, RVD); \ > > - } \ > > +#define REQUIRE_ZDINX_OR_D(ctx) do { \ > > + if (!has_ext(ctx, RVD)) { \ > > + if (!ctx->cfg_ptr->ext_zdinx) { \ > > + return false; \ > > + } \ > > + if (!smstateen_fcsr_check(ctx)) { \ > > + return false; \ > > + } \ > > + } \ > > } while (0) > > > > #define REQUIRE_EVEN(ctx, reg) do { \ > > diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc > > index 9e9fa2087a..6bf6fe16be 100644 > > --- a/target/riscv/insn_trans/trans_rvf.c.inc > > +++ b/target/riscv/insn_trans/trans_rvf.c.inc > > @@ -24,10 +24,26 @@ > > return false; \ > > } while (0) > > > > -#define REQUIRE_ZFINX_OR_F(ctx) do {\ > > - if (!ctx->cfg_ptr->ext_zfinx) { \ > > - REQUIRE_EXT(ctx, RVF); \ > > - } \ > > +static inline bool smstateen_fcsr_check(DisasContext *ctx) > > +{ > > +#ifndef CONFIG_USER_ONLY > > + if (!has_ext(ctx, RVF) && !ctx->smstateen_fcsr_ok) { > > We needn't check RVF here. Two reasons: > > 1. This check only be done when RVF is diabled. > > 2. ctx->smstateen_fcsr_ok is always be true (set in last patch) when RVF > is enabled Ok. > > > + ctx->virt_inst_excp = ctx->virt_enabled; > > + return false; > > + } > > +#endif > > + return true; > > +} > > + > > +#define REQUIRE_ZFINX_OR_F(ctx) do { \ > > + if (!has_ext(ctx, RVF)) { \ > > + if (!ctx->cfg_ptr->ext_zfinx) { \ > > + return false; \ > > + } \ > > + if (!smstateen_fcsr_check(ctx)) { \ > > + return false; \ > > + } \ > > + } \ > > } while (0) > > > > #define REQUIRE_ZCF(ctx) do { \ > > diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc > > index 74dde37ff7..74a125e4c0 100644 > > --- a/target/riscv/insn_trans/trans_rvzfh.c.inc > > +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc > > @@ -16,28 +16,40 @@ > > * this program. If not, see <http://www.gnu.org/licenses/>. > > */ > > > > -#define REQUIRE_ZFH(ctx) do { \ > > +#define REQUIRE_ZFH(ctx) do { \ > > if (!ctx->cfg_ptr->ext_zfh) { \ > > - return false; \ > > - } \ > > + return false; \ > > + } \ > > + if (!smstateen_fcsr_check(ctx)) { \ > > + return false; \ > > + } \ > > This is unnecessary here. This check is only for Zhinx. > > Similar to REQUIRE_ZFHMIN. > > > } while (0) > > > > #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \ > > if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \ > > return false; \ > > } \ > > + if (!smstateen_fcsr_check(ctx)) { \ > > + return false; \ > > + } \ > > This check is only for Zhinx here. So it's better to take the similar > way as REQUIRE_ZFINX_OR_F. > > Similar to REQUIRE_ZFHMIN_OR_ZHINXMIN. I am not sure I follow the comments above. The FCSR check is applicable to all the extensions ie zfinx, zdinx, zhinx and zhinxmin. > > Regards, > > Weiwei Li > > > } while (0) > > > > #define REQUIRE_ZFHMIN(ctx) do { \ > > if (!ctx->cfg_ptr->ext_zfhmin) { \ > > return false; \ > > } \ > > + if (!smstateen_fcsr_check(ctx)) { \ > > + return false; \ > > + } \ > > } while (0) > > > > #define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do { \ > > if (!(ctx->cfg_ptr->ext_zfhmin || ctx->cfg_ptr->ext_zhinxmin)) { \ > > return false; \ > > } \ > > + if (!smstateen_fcsr_check(ctx)) { \ > > + return false; \ > > + } \ > > } while (0) > > > > static bool trans_flh(DisasContext *ctx, arg_flh *a) >
On 2023/4/27 01:18, Mayuresh Chitale wrote: > On Sat, Apr 15, 2023 at 6:55 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote: >> >> On 2023/4/15 00:02, 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_rvd.c.inc | 13 ++++++++---- >>> target/riscv/insn_trans/trans_rvf.c.inc | 24 +++++++++++++++++++---- >>> target/riscv/insn_trans/trans_rvzfh.c.inc | 18 ++++++++++++++--- >>> 3 files changed, 44 insertions(+), 11 deletions(-) >>> >>> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc >>> index 2c51e01c40..d9e0cf116f 100644 >>> --- a/target/riscv/insn_trans/trans_rvd.c.inc >>> +++ b/target/riscv/insn_trans/trans_rvd.c.inc >>> @@ -18,10 +18,15 @@ >>> * this program. If not, see <http://www.gnu.org/licenses/>. >>> */ >>> >>> -#define REQUIRE_ZDINX_OR_D(ctx) do { \ >>> - if (!ctx->cfg_ptr->ext_zdinx) { \ >>> - REQUIRE_EXT(ctx, RVD); \ >>> - } \ >>> +#define REQUIRE_ZDINX_OR_D(ctx) do { \ >>> + if (!has_ext(ctx, RVD)) { \ >>> + if (!ctx->cfg_ptr->ext_zdinx) { \ >>> + return false; \ >>> + } \ >>> + if (!smstateen_fcsr_check(ctx)) { \ >>> + return false; \ >>> + } \ >>> + } \ >>> } while (0) >>> >>> #define REQUIRE_EVEN(ctx, reg) do { \ >>> diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc >>> index 9e9fa2087a..6bf6fe16be 100644 >>> --- a/target/riscv/insn_trans/trans_rvf.c.inc >>> +++ b/target/riscv/insn_trans/trans_rvf.c.inc >>> @@ -24,10 +24,26 @@ >>> return false; \ >>> } while (0) >>> >>> -#define REQUIRE_ZFINX_OR_F(ctx) do {\ >>> - if (!ctx->cfg_ptr->ext_zfinx) { \ >>> - REQUIRE_EXT(ctx, RVF); \ >>> - } \ >>> +static inline bool smstateen_fcsr_check(DisasContext *ctx) >>> +{ >>> +#ifndef CONFIG_USER_ONLY >>> + if (!has_ext(ctx, RVF) && !ctx->smstateen_fcsr_ok) { >> We needn't check RVF here. Two reasons: >> >> 1. This check only be done when RVF is diabled. >> >> 2. ctx->smstateen_fcsr_ok is always be true (set in last patch) when RVF >> is enabled > Ok. >>> + ctx->virt_inst_excp = ctx->virt_enabled; >>> + return false; >>> + } >>> +#endif >>> + return true; >>> +} >>> + >>> +#define REQUIRE_ZFINX_OR_F(ctx) do { \ >>> + if (!has_ext(ctx, RVF)) { \ >>> + if (!ctx->cfg_ptr->ext_zfinx) { \ >>> + return false; \ >>> + } \ >>> + if (!smstateen_fcsr_check(ctx)) { \ >>> + return false; \ >>> + } \ >>> + } \ >>> } while (0) >>> >>> #define REQUIRE_ZCF(ctx) do { \ >>> diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc >>> index 74dde37ff7..74a125e4c0 100644 >>> --- a/target/riscv/insn_trans/trans_rvzfh.c.inc >>> +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc >>> @@ -16,28 +16,40 @@ >>> * this program. If not, see <http://www.gnu.org/licenses/>. >>> */ >>> >>> -#define REQUIRE_ZFH(ctx) do { \ >>> +#define REQUIRE_ZFH(ctx) do { \ >>> if (!ctx->cfg_ptr->ext_zfh) { \ >>> - return false; \ >>> - } \ >>> + return false; \ >>> + } \ >>> + if (!smstateen_fcsr_check(ctx)) { \ >>> + return false; \ >>> + } \ >> This is unnecessary here. This check is only for Zhinx. >> >> Similar to REQUIRE_ZFHMIN. >> >>> } while (0) >>> >>> #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \ >>> if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \ >>> return false; \ >>> } \ >>> + if (!smstateen_fcsr_check(ctx)) { \ >>> + return false; \ >>> + } \ >> This check is only for Zhinx here. So it's better to take the similar >> way as REQUIRE_ZFINX_OR_F. >> >> Similar to REQUIRE_ZFHMIN_OR_ZHINXMIN. > I am not sure I follow the comments above. > The FCSR check is applicable to all the extensions ie zfinx, zdinx, > zhinx and zhinxmin. Yeah. FCSR check is applicable to all Z*inx extensions, but unnecessary for Zfh. So I think it's better to be: if (!ctx->cfg_ptr->ext_zfh) { \ if (!ctx->cfg_ptr->ext_zhinx)) { \ return false; \ } \ if (!smstateen_fcsr_check(ctx)) { \ return false; \ } \ } Regards, Weiwei Li >> Regards, >> >> Weiwei Li >> >>> } while (0) >>> >>> #define REQUIRE_ZFHMIN(ctx) do { \ >>> if (!ctx->cfg_ptr->ext_zfhmin) { \ >>> return false; \ >>> } \ >>> + if (!smstateen_fcsr_check(ctx)) { \ >>> + return false; \ >>> + } \ >>> } while (0) >>> >>> #define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do { \ >>> if (!(ctx->cfg_ptr->ext_zfhmin || ctx->cfg_ptr->ext_zhinxmin)) { \ >>> return false; \ >>> } \ >>> + if (!smstateen_fcsr_check(ctx)) { \ >>> + return false; \ >>> + } \ >>> } while (0) >>> >>> static bool trans_flh(DisasContext *ctx, arg_flh *a)
diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc index 2c51e01c40..d9e0cf116f 100644 --- a/target/riscv/insn_trans/trans_rvd.c.inc +++ b/target/riscv/insn_trans/trans_rvd.c.inc @@ -18,10 +18,15 @@ * this program. If not, see <http://www.gnu.org/licenses/>. */ -#define REQUIRE_ZDINX_OR_D(ctx) do { \ - if (!ctx->cfg_ptr->ext_zdinx) { \ - REQUIRE_EXT(ctx, RVD); \ - } \ +#define REQUIRE_ZDINX_OR_D(ctx) do { \ + if (!has_ext(ctx, RVD)) { \ + if (!ctx->cfg_ptr->ext_zdinx) { \ + return false; \ + } \ + if (!smstateen_fcsr_check(ctx)) { \ + return false; \ + } \ + } \ } while (0) #define REQUIRE_EVEN(ctx, reg) do { \ diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc index 9e9fa2087a..6bf6fe16be 100644 --- a/target/riscv/insn_trans/trans_rvf.c.inc +++ b/target/riscv/insn_trans/trans_rvf.c.inc @@ -24,10 +24,26 @@ return false; \ } while (0) -#define REQUIRE_ZFINX_OR_F(ctx) do {\ - if (!ctx->cfg_ptr->ext_zfinx) { \ - REQUIRE_EXT(ctx, RVF); \ - } \ +static inline bool smstateen_fcsr_check(DisasContext *ctx) +{ +#ifndef CONFIG_USER_ONLY + if (!has_ext(ctx, RVF) && !ctx->smstateen_fcsr_ok) { + ctx->virt_inst_excp = ctx->virt_enabled; + return false; + } +#endif + return true; +} + +#define REQUIRE_ZFINX_OR_F(ctx) do { \ + if (!has_ext(ctx, RVF)) { \ + if (!ctx->cfg_ptr->ext_zfinx) { \ + return false; \ + } \ + if (!smstateen_fcsr_check(ctx)) { \ + return false; \ + } \ + } \ } while (0) #define REQUIRE_ZCF(ctx) do { \ diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc index 74dde37ff7..74a125e4c0 100644 --- a/target/riscv/insn_trans/trans_rvzfh.c.inc +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc @@ -16,28 +16,40 @@ * this program. If not, see <http://www.gnu.org/licenses/>. */ -#define REQUIRE_ZFH(ctx) do { \ +#define REQUIRE_ZFH(ctx) do { \ if (!ctx->cfg_ptr->ext_zfh) { \ - return false; \ - } \ + return false; \ + } \ + if (!smstateen_fcsr_check(ctx)) { \ + return false; \ + } \ } while (0) #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \ if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \ return false; \ } \ + if (!smstateen_fcsr_check(ctx)) { \ + return false; \ + } \ } while (0) #define REQUIRE_ZFHMIN(ctx) do { \ if (!ctx->cfg_ptr->ext_zfhmin) { \ return false; \ } \ + if (!smstateen_fcsr_check(ctx)) { \ + return false; \ + } \ } while (0) #define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do { \ if (!(ctx->cfg_ptr->ext_zfhmin || ctx->cfg_ptr->ext_zhinxmin)) { \ return false; \ } \ + if (!smstateen_fcsr_check(ctx)) { \ + return false; \ + } \ } while (0) static bool trans_flh(DisasContext *ctx, arg_flh *a)
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_rvd.c.inc | 13 ++++++++---- target/riscv/insn_trans/trans_rvf.c.inc | 24 +++++++++++++++++++---- target/riscv/insn_trans/trans_rvzfh.c.inc | 18 ++++++++++++++--- 3 files changed, 44 insertions(+), 11 deletions(-)