1. Patchew
  2. QEMU
  3. [PATCH v1 0/2] target/s390x: Fix emulation of C(G)HRL
  4. View patch

[PATCH v1 1/2] target/s390x: Fix emulation of C(G)HRL

Nina Schoetterl-Glausch posted 2 patches 2 years, 11 months ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Thomas Huth <thuth@redhat.com>
There is a newer version of this series
[PATCH v1 1/2] target/s390x: Fix emulation of C(G)HRL
Posted by Nina Schoetterl-Glausch 2 years, 11 months ago 
The second operand of COMPARE HALFWORD RELATIVE LONG is a signed
halfword, it does not have the same size as the first operand.

Fixes: a7e836d5eb ("target-s390: Convert COMPARE, COMPARE LOGICAL")
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
 target/s390x/tcg/insn-data.h.inc | 4 ++--
 target/s390x/tcg/translate.c     | 7 +++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc
index 9d2d35f084..6fe8ca5143 100644
--- a/target/s390x/tcg/insn-data.h.inc
+++ b/target/s390x/tcg/insn-data.h.inc
@@ -199,8 +199,8 @@
     C(0xe55c, CHSI,    SIL,   GIE, m1_32s, i2, 0, 0, 0, cmps64)
     C(0xe558, CGHSI,   SIL,   GIE, m1_64, i2, 0, 0, 0, cmps64)
 /* COMPARE HALFWORD RELATIVE LONG */
-    C(0xc605, CHRL,    RIL_b, GIE, r1_o, mri2_32s, 0, 0, 0, cmps32)
-    C(0xc604, CGHRL,   RIL_b, GIE, r1_o, mri2_64, 0, 0, 0, cmps64)
+    C(0xc605, CHRL,    RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps32)
+    C(0xc604, CGHRL,   RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps64)
 /* COMPARE HIGH */
     C(0xb9cd, CHHR,    RRE,   HW,  r1_sr32, r2_sr32, 0, 0, 0, cmps32)
     C(0xb9dd, CHLR,    RRE,   HW,  r1_sr32, r2_o, 0, 0, 0, cmps32)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 811049ea28..60b1254bb2 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -5974,6 +5974,13 @@ static void in2_m2_64a(DisasContext *s, DisasOps *o)
 #define SPEC_in2_m2_64a 0
 #endif
 
+static void in2_mri2_16s(DisasContext *s, DisasOps *o)
+{
+    in2_ri2(s, o);
+    tcg_gen_qemu_ld16s(o->in2, o->in2, get_mem_index(s));
+}
+#define SPEC_in2_mri2_16s 0
+
 static void in2_mri2_16u(DisasContext *s, DisasOps *o)
 {
     in2_ri2(s, o);
-- 
2.37.2
Re: [PATCH v1 1/2] target/s390x: Fix emulation of C(G)HRL
Posted by Richard Henderson 2 years, 11 months ago 
On 3/6/23 12:55, Nina Schoetterl-Glausch wrote:
> The second operand of COMPARE HALFWORD RELATIVE LONG is a signed
> halfword, it does not have the same size as the first operand.
> 
> Fixes: a7e836d5eb ("target-s390: Convert COMPARE, COMPARE LOGICAL")
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> ---
>   target/s390x/tcg/insn-data.h.inc | 4 ++--
>   target/s390x/tcg/translate.c     | 7 +++++++
>   2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc
> index 9d2d35f084..6fe8ca5143 100644
> --- a/target/s390x/tcg/insn-data.h.inc
> +++ b/target/s390x/tcg/insn-data.h.inc
> @@ -199,8 +199,8 @@
>       C(0xe55c, CHSI,    SIL,   GIE, m1_32s, i2, 0, 0, 0, cmps64)
>       C(0xe558, CGHSI,   SIL,   GIE, m1_64, i2, 0, 0, 0, cmps64)
>   /* COMPARE HALFWORD RELATIVE LONG */
> -    C(0xc605, CHRL,    RIL_b, GIE, r1_o, mri2_32s, 0, 0, 0, cmps32)
> -    C(0xc604, CGHRL,   RIL_b, GIE, r1_o, mri2_64, 0, 0, 0, cmps64)
> +    C(0xc605, CHRL,    RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps32)
> +    C(0xc604, CGHRL,   RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps64)
>   /* COMPARE HIGH */
>       C(0xb9cd, CHHR,    RRE,   HW,  r1_sr32, r2_sr32, 0, 0, 0, cmps32)
>       C(0xb9dd, CHLR,    RRE,   HW,  r1_sr32, r2_o, 0, 0, 0, cmps32)
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 811049ea28..60b1254bb2 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -5974,6 +5974,13 @@ static void in2_m2_64a(DisasContext *s, DisasOps *o)
>   #define SPEC_in2_m2_64a 0
>   #endif
>   
> +static void in2_mri2_16s(DisasContext *s, DisasOps *o)
> +{
> +    in2_ri2(s, o);
> +    tcg_gen_qemu_ld16s(o->in2, o->in2, get_mem_index(s));
> +}
> +#define SPEC_in2_mri2_16s 0

This is incorrect.  See

https://patchew.org/QEMU/20230227054233.390271-1-richard.henderson@linaro.org/20230227054233.390271-51-richard.henderson@linaro.org/


r~
Re: [PATCH v1 1/2] target/s390x: Fix emulation of C(G)HRL
Posted by Nina Schoetterl-Glausch 2 years, 11 months ago 
On Mon, 2023-03-06 at 12:59 -0800, Richard Henderson wrote:
> On 3/6/23 12:55, Nina Schoetterl-Glausch wrote:
> > The second operand of COMPARE HALFWORD RELATIVE LONG is a signed
> > halfword, it does not have the same size as the first operand.
> > 
> > Fixes: a7e836d5eb ("target-s390: Convert COMPARE, COMPARE LOGICAL")
> > Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > ---
> >   target/s390x/tcg/insn-data.h.inc | 4 ++--
> >   target/s390x/tcg/translate.c     | 7 +++++++
> >   2 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc
> > index 9d2d35f084..6fe8ca5143 100644
> > --- a/target/s390x/tcg/insn-data.h.inc
> > +++ b/target/s390x/tcg/insn-data.h.inc
> > @@ -199,8 +199,8 @@
> >       C(0xe55c, CHSI,    SIL,   GIE, m1_32s, i2, 0, 0, 0, cmps64)
> >       C(0xe558, CGHSI,   SIL,   GIE, m1_64, i2, 0, 0, 0, cmps64)
> >   /* COMPARE HALFWORD RELATIVE LONG */
> > -    C(0xc605, CHRL,    RIL_b, GIE, r1_o, mri2_32s, 0, 0, 0, cmps32)
> > -    C(0xc604, CGHRL,   RIL_b, GIE, r1_o, mri2_64, 0, 0, 0, cmps64)
> > +    C(0xc605, CHRL,    RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps32)
> > +    C(0xc604, CGHRL,   RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps64)
> >   /* COMPARE HIGH */
> >       C(0xb9cd, CHHR,    RRE,   HW,  r1_sr32, r2_sr32, 0, 0, 0, cmps32)
> >       C(0xb9dd, CHLR,    RRE,   HW,  r1_sr32, r2_o, 0, 0, 0, cmps32)
> > diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> > index 811049ea28..60b1254bb2 100644
> > --- a/target/s390x/tcg/translate.c
> > +++ b/target/s390x/tcg/translate.c
> > @@ -5974,6 +5974,13 @@ static void in2_m2_64a(DisasContext *s, DisasOps *o)
> >   #define SPEC_in2_m2_64a 0
> >   #endif
> >   
> > +static void in2_mri2_16s(DisasContext *s, DisasOps *o)
> > +{
> > +    in2_ri2(s, o);
> > +    tcg_gen_qemu_ld16s(o->in2, o->in2, get_mem_index(s));
> > +}
> > +#define SPEC_in2_mri2_16s 0
> 
> This is incorrect.  See
> 
> https://patchew.org/QEMU/20230227054233.390271-1-richard.henderson@linaro.org/20230227054233.390271-51-richard.henderson@linaro.org/
> 

Ah, that got merged yesterday, didn't it?
base-commit: a2b5f8b8ab7b2c947823088103a40f0ff11fe06b
which is a bit older, I'll rebase.

> 
> r~
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.