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

[Weiwei Li]

On 2023/4/29 00:52, 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 | 32 +++++++++++++++--------
>   3 files changed, 50 insertions(+), 19 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 b2de4fcf3f..e4d9834237 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 (!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..e228ae28a5 100644
> --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
> @@ -16,28 +16,38 @@
>    * this program.  If not, see <http://www.gnu.org/licenses/>.
>    */
>   
> -#define REQUIRE_ZFH(ctx) do { \
> +#define REQUIRE_ZFH(ctx) do {          \

This modification seems unnecessary.

Otherwise, Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Weiwei Li

>       if (!ctx->cfg_ptr->ext_zfh) {      \
> -        return false;         \
> -    }                         \
> -} while (0)
> -
> -#define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
> -    if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
>           return false;                  \
>       }                                  \
>   } while (0)
>   
> +#define REQUIRE_ZHINX_OR_ZFH(ctx) do {                \
> +    if (!ctx->cfg_ptr->ext_zfh) {                     \
> +        if (!ctx->cfg_ptr->ext_zhinx) {               \
> +            return false;                             \
> +        }                                             \
> +        if (!smstateen_fcsr_check(ctx)) {             \
> +            return false;                             \
> +        }                                             \
> +    }                                                 \
> +} while (0)
> +
>   #define REQUIRE_ZFHMIN(ctx) do {              \
>       if (!ctx->cfg_ptr->ext_zfhmin) {          \
>           return false;                         \
>       }                                         \
>   } while (0)
>   
> -#define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {                 \
> -    if (!(ctx->cfg_ptr->ext_zfhmin || ctx->cfg_ptr->ext_zhinxmin)) { \
> -        return false;                                        \
> -    }                                                        \
> +#define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {             \
> +    if (!ctx->cfg_ptr->ext_zfhmin) {                     \
> +        if (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)