[PULL v2 07/35] target/riscv: access cfg structure through DisasContext

Alistair Francis posted 35 patches 3 years, 10 months ago
There is a newer version of this series
[PULL v2 07/35] target/riscv: access cfg structure through DisasContext
Posted by Alistair Francis 3 years, 10 months ago
From: Philipp Tomsich <philipp.tomsich@vrull.eu>

The Zb[abcs] support code still uses the RISCV_CPU macros to access
the configuration information (i.e., check whether an extension is
available/enabled).  Now that we provide this information directly
from DisasContext, we can access this directly via the cfg_ptr field.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20220202005249.3566542-5-philipp.tomsich@vrull.eu>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/insn_trans/trans_rvb.c.inc | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvb.c.inc b/target/riscv/insn_trans/trans_rvb.c.inc
index 810431a1d6..f9bd3b7ec4 100644
--- a/target/riscv/insn_trans/trans_rvb.c.inc
+++ b/target/riscv/insn_trans/trans_rvb.c.inc
@@ -19,25 +19,25 @@
  */
 
 #define REQUIRE_ZBA(ctx) do {                    \
-    if (!RISCV_CPU(ctx->cs)->cfg.ext_zba) {      \
+    if (ctx->cfg_ptr->ext_zba) {                 \
         return false;                            \
     }                                            \
 } while (0)
 
 #define REQUIRE_ZBB(ctx) do {                    \
-    if (!RISCV_CPU(ctx->cs)->cfg.ext_zbb) {      \
+    if (ctx->cfg_ptr->ext_zbb) {                 \
         return false;                            \
     }                                            \
 } while (0)
 
 #define REQUIRE_ZBC(ctx) do {                    \
-    if (!RISCV_CPU(ctx->cs)->cfg.ext_zbc) {      \
+    if (ctx->cfg_ptr->ext_zbc) {                 \
         return false;                            \
     }                                            \
 } while (0)
 
 #define REQUIRE_ZBS(ctx) do {                    \
-    if (!RISCV_CPU(ctx->cs)->cfg.ext_zbs) {      \
+    if (ctx->cfg_ptr->ext_zbs) {                 \
         return false;                            \
     }                                            \
 } while (0)
-- 
2.34.1


Re: [PULL v2 07/35] target/riscv: access cfg structure through DisasContext
Posted by Philipp Tomsich 3 years, 10 months ago
Alistair,

This PULL seems not to include the fixup (which you had intended to
squash into it) for the regression introduced (i.e. the condition
being inverted):
  https://patchwork.kernel.org/project/qemu-devel/patch/20220203153946.2676353-1-philipp.tomsich@vrull.eu/
Without that change this will introduce a regression in Zb[abcs]
(i.e., it will be enabled when it shouldn't be, and will be disabled
when it should be on).

Please ignore, if I missed a later stand-alone patch (I just looked at
the series in Patchworks).

Thanks,
Philipp.


On Wed, 16 Feb 2022 at 07:29, Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Philipp Tomsich <philipp.tomsich@vrull.eu>
>
> The Zb[abcs] support code still uses the RISCV_CPU macros to access
> the configuration information (i.e., check whether an extension is
> available/enabled).  Now that we provide this information directly
> from DisasContext, we can access this directly via the cfg_ptr field.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20220202005249.3566542-5-philipp.tomsich@vrull.eu>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/insn_trans/trans_rvb.c.inc | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvb.c.inc b/target/riscv/insn_trans/trans_rvb.c.inc
> index 810431a1d6..f9bd3b7ec4 100644
> --- a/target/riscv/insn_trans/trans_rvb.c.inc
> +++ b/target/riscv/insn_trans/trans_rvb.c.inc
> @@ -19,25 +19,25 @@
>   */
>
>  #define REQUIRE_ZBA(ctx) do {                    \
> -    if (!RISCV_CPU(ctx->cs)->cfg.ext_zba) {      \
> +    if (ctx->cfg_ptr->ext_zba) {                 \
>          return false;                            \
>      }                                            \
>  } while (0)
>
>  #define REQUIRE_ZBB(ctx) do {                    \
> -    if (!RISCV_CPU(ctx->cs)->cfg.ext_zbb) {      \
> +    if (ctx->cfg_ptr->ext_zbb) {                 \
>          return false;                            \
>      }                                            \
>  } while (0)
>
>  #define REQUIRE_ZBC(ctx) do {                    \
> -    if (!RISCV_CPU(ctx->cs)->cfg.ext_zbc) {      \
> +    if (ctx->cfg_ptr->ext_zbc) {                 \
>          return false;                            \
>      }                                            \
>  } while (0)
>
>  #define REQUIRE_ZBS(ctx) do {                    \
> -    if (!RISCV_CPU(ctx->cs)->cfg.ext_zbs) {      \
> +    if (ctx->cfg_ptr->ext_zbs) {                 \
>          return false;                            \
>      }                                            \
>  } while (0)
> --
> 2.34.1
>

Re: [PULL v2 07/35] target/riscv: access cfg structure through DisasContext
Posted by Alistair Francis 3 years, 10 months ago
On Wed, Feb 16, 2022 at 8:24 PM Philipp Tomsich
<philipp.tomsich@vrull.eu> wrote:
>
> Alistair,
>
> This PULL seems not to include the fixup (which you had intended to
> squash into it) for the regression introduced (i.e. the condition
> being inverted):
>   https://patchwork.kernel.org/project/qemu-devel/patch/20220203153946.2676353-1-philipp.tomsich@vrull.eu/

Well....

It does not include it and I'm not really sure why it doesn't. The V1
PR didn't either.

I thought I had applied it, but I guess not. I have actually applied
it to riscv-to-apply.next now

Alistair


> Without that change this will introduce a regression in Zb[abcs]
> (i.e., it will be enabled when it shouldn't be, and will be disabled
> when it should be on).
>
> Please ignore, if I missed a later stand-alone patch (I just looked at
> the series in Patchworks).
>
> Thanks,
> Philipp.
>
>
> On Wed, 16 Feb 2022 at 07:29, Alistair Francis
> <alistair.francis@opensource.wdc.com> wrote:
> >
> > From: Philipp Tomsich <philipp.tomsich@vrull.eu>
> >
> > The Zb[abcs] support code still uses the RISCV_CPU macros to access
> > the configuration information (i.e., check whether an extension is
> > available/enabled).  Now that we provide this information directly
> > from DisasContext, we can access this directly via the cfg_ptr field.
> >
> > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > Message-Id: <20220202005249.3566542-5-philipp.tomsich@vrull.eu>
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  target/riscv/insn_trans/trans_rvb.c.inc | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/insn_trans/trans_rvb.c.inc b/target/riscv/insn_trans/trans_rvb.c.inc
> > index 810431a1d6..f9bd3b7ec4 100644
> > --- a/target/riscv/insn_trans/trans_rvb.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvb.c.inc
> > @@ -19,25 +19,25 @@
> >   */
> >
> >  #define REQUIRE_ZBA(ctx) do {                    \
> > -    if (!RISCV_CPU(ctx->cs)->cfg.ext_zba) {      \
> > +    if (ctx->cfg_ptr->ext_zba) {                 \
> >          return false;                            \
> >      }                                            \
> >  } while (0)
> >
> >  #define REQUIRE_ZBB(ctx) do {                    \
> > -    if (!RISCV_CPU(ctx->cs)->cfg.ext_zbb) {      \
> > +    if (ctx->cfg_ptr->ext_zbb) {                 \
> >          return false;                            \
> >      }                                            \
> >  } while (0)
> >
> >  #define REQUIRE_ZBC(ctx) do {                    \
> > -    if (!RISCV_CPU(ctx->cs)->cfg.ext_zbc) {      \
> > +    if (ctx->cfg_ptr->ext_zbc) {                 \
> >          return false;                            \
> >      }                                            \
> >  } while (0)
> >
> >  #define REQUIRE_ZBS(ctx) do {                    \
> > -    if (!RISCV_CPU(ctx->cs)->cfg.ext_zbs) {      \
> > +    if (ctx->cfg_ptr->ext_zbs) {                 \
> >          return false;                            \
> >      }                                            \
> >  } while (0)
> > --
> > 2.34.1
> >