* [PATCH 0/2] target/s390x: Fix LAE setting a wrong access register
@ 2024-01-09 23:22 Ilya Leoshkevich
2024-01-09 23:22 ` [PATCH 1/2] " Ilya Leoshkevich
2024-01-09 23:22 ` [PATCH 2/2] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED Ilya Leoshkevich
0 siblings, 2 replies; 7+ messages in thread
From: Ilya Leoshkevich @ 2024-01-09 23:22 UTC (permalink / raw)
To: Richard Henderson, David Hildenbrand, Thomas Huth
Cc: qemu-s390x, qemu-devel, Ilya Leoshkevich
Hi,
Ido has noticed that LAE sets a wrong access register and proposed a
fix. This series fixes the issue and adds a test.
Best regards,
Ilya
Ilya Leoshkevich (2):
target/s390x: Fix LAE setting a wrong access register
tests/tcg/s390x: Test LOAD ADDRESS EXTENDED
target/s390x/tcg/translate.c | 3 ++-
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/lae.c | 25 +++++++++++++++++++++++++
3 files changed, 28 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/s390x/lae.c
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] target/s390x: Fix LAE setting a wrong access register
2024-01-09 23:22 [PATCH 0/2] target/s390x: Fix LAE setting a wrong access register Ilya Leoshkevich
@ 2024-01-09 23:22 ` Ilya Leoshkevich
2024-01-10 9:11 ` David Hildenbrand
2024-01-09 23:22 ` [PATCH 2/2] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED Ilya Leoshkevich
1 sibling, 1 reply; 7+ messages in thread
From: Ilya Leoshkevich @ 2024-01-09 23:22 UTC (permalink / raw)
To: Richard Henderson, David Hildenbrand, Thomas Huth
Cc: qemu-s390x, qemu-devel, Ilya Leoshkevich, Ido Plat, qemu-stable
LAE should set the access register corresponding to the first operand,
instead, it always modifies access register 1.
Co-developed-by: Ido Plat <Ido.Plat@ibm.com>
Cc: qemu-stable@nongnu.org
Fixes: a1c7610a6879 ("target-s390x: implement LAY and LAEY instructions")
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
target/s390x/tcg/translate.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 62ab2be8b12..8df00b7df9f 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -3221,6 +3221,7 @@ static DisasJumpType op_mov2e(DisasContext *s, DisasOps *o)
{
int b2 = get_field(s, b2);
TCGv ar1 = tcg_temp_new_i64();
+ int r1 = get_field(s, r1);
o->out = o->in2;
o->in2 = NULL;
@@ -3244,7 +3245,7 @@ static DisasJumpType op_mov2e(DisasContext *s, DisasOps *o)
break;
}
- tcg_gen_st32_i64(ar1, tcg_env, offsetof(CPUS390XState, aregs[1]));
+ tcg_gen_st32_i64(ar1, tcg_env, offsetof(CPUS390XState, aregs[r1]));
return DISAS_NEXT;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED
2024-01-09 23:22 [PATCH 0/2] target/s390x: Fix LAE setting a wrong access register Ilya Leoshkevich
2024-01-09 23:22 ` [PATCH 1/2] " Ilya Leoshkevich
@ 2024-01-09 23:22 ` Ilya Leoshkevich
2024-01-10 16:50 ` Thomas Huth
2024-01-11 8:37 ` Thomas Huth
1 sibling, 2 replies; 7+ messages in thread
From: Ilya Leoshkevich @ 2024-01-09 23:22 UTC (permalink / raw)
To: Richard Henderson, David Hildenbrand, Thomas Huth
Cc: qemu-s390x, qemu-devel, Ilya Leoshkevich
Add a small test to prevent regressions. Userspace runs in primary
mode, so LAE should always set the access register to 0.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/lae.c | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+)
create mode 100644 tests/tcg/s390x/lae.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 0e670f3f8b9..30994dcf9c2 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -44,6 +44,7 @@ TESTS+=clgebr
TESTS+=clc
TESTS+=laalg
TESTS+=add-logical-with-carry
+TESTS+=lae
cdsg: CFLAGS+=-pthread
cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/lae.c b/tests/tcg/s390x/lae.c
new file mode 100644
index 00000000000..661e95f9978
--- /dev/null
+++ b/tests/tcg/s390x/lae.c
@@ -0,0 +1,25 @@
+/*
+ * Test the LOAD ADDRESS EXTENDED instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <assert.h>
+#include <stdlib.h>
+
+int main(void)
+{
+ unsigned long long ar = -1, b2 = 100000, r, x2 = 500;
+ int tmp;
+
+ asm("ear %[tmp],%[r]\n"
+ "lae %[r],42(%[x2],%[b2])\n"
+ "ear %[ar],%[r]\n"
+ "sar %[r],%[tmp]"
+ : [tmp] "=&r" (tmp), [r] "=&r" (r), [ar] "+r" (ar)
+ : [b2] "r" (b2), [x2] "r" (x2)
+ : "memory");
+ assert(ar == 0xffffffff00000000ULL);
+ assert(r == 100542);
+
+ return EXIT_SUCCESS;
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] target/s390x: Fix LAE setting a wrong access register
2024-01-09 23:22 ` [PATCH 1/2] " Ilya Leoshkevich
@ 2024-01-10 9:11 ` David Hildenbrand
0 siblings, 0 replies; 7+ messages in thread
From: David Hildenbrand @ 2024-01-10 9:11 UTC (permalink / raw)
To: Ilya Leoshkevich, Richard Henderson, Thomas Huth
Cc: qemu-s390x, qemu-devel, Ido Plat, qemu-stable
On 10.01.24 00:22, Ilya Leoshkevich wrote:
> LAE should set the access register corresponding to the first operand,
> instead, it always modifies access register 1.
>
> Co-developed-by: Ido Plat <Ido.Plat@ibm.com>
> Cc: qemu-stable@nongnu.org
> Fixes: a1c7610a6879 ("target-s390x: implement LAY and LAEY instructions")
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> target/s390x/tcg/translate.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 62ab2be8b12..8df00b7df9f 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -3221,6 +3221,7 @@ static DisasJumpType op_mov2e(DisasContext *s, DisasOps *o)
> {
> int b2 = get_field(s, b2);
> TCGv ar1 = tcg_temp_new_i64();
> + int r1 = get_field(s, r1);
>
> o->out = o->in2;
> o->in2 = NULL;
> @@ -3244,7 +3245,7 @@ static DisasJumpType op_mov2e(DisasContext *s, DisasOps *o)
> break;
> }
>
> - tcg_gen_st32_i64(ar1, tcg_env, offsetof(CPUS390XState, aregs[1]));
> + tcg_gen_st32_i64(ar1, tcg_env, offsetof(CPUS390XState, aregs[r1]));
> return DISAS_NEXT;
> }
>
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED
2024-01-09 23:22 ` [PATCH 2/2] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED Ilya Leoshkevich
@ 2024-01-10 16:50 ` Thomas Huth
2024-01-11 8:37 ` Thomas Huth
1 sibling, 0 replies; 7+ messages in thread
From: Thomas Huth @ 2024-01-10 16:50 UTC (permalink / raw)
To: Ilya Leoshkevich, Richard Henderson, David Hildenbrand
Cc: qemu-s390x, qemu-devel
On 10/01/2024 00.22, Ilya Leoshkevich wrote:
> Add a small test to prevent regressions. Userspace runs in primary
> mode, so LAE should always set the access register to 0.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tests/tcg/s390x/Makefile.target | 1 +
> tests/tcg/s390x/lae.c | 25 +++++++++++++++++++++++++
> 2 files changed, 26 insertions(+)
> create mode 100644 tests/tcg/s390x/lae.c
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED
2024-01-09 23:22 ` [PATCH 2/2] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED Ilya Leoshkevich
2024-01-10 16:50 ` Thomas Huth
@ 2024-01-11 8:37 ` Thomas Huth
2024-01-11 9:04 ` Ilya Leoshkevich
1 sibling, 1 reply; 7+ messages in thread
From: Thomas Huth @ 2024-01-11 8:37 UTC (permalink / raw)
To: Ilya Leoshkevich, Richard Henderson, David Hildenbrand
Cc: qemu-s390x, qemu-devel
On 10/01/2024 00.22, Ilya Leoshkevich wrote:
> Add a small test to prevent regressions. Userspace runs in primary
> mode, so LAE should always set the access register to 0.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tests/tcg/s390x/Makefile.target | 1 +
> tests/tcg/s390x/lae.c | 25 +++++++++++++++++++++++++
> 2 files changed, 26 insertions(+)
> create mode 100644 tests/tcg/s390x/lae.c
>
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index 0e670f3f8b9..30994dcf9c2 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -44,6 +44,7 @@ TESTS+=clgebr
> TESTS+=clc
> TESTS+=laalg
> TESTS+=add-logical-with-carry
> +TESTS+=lae
>
> cdsg: CFLAGS+=-pthread
> cdsg: LDFLAGS+=-pthread
> diff --git a/tests/tcg/s390x/lae.c b/tests/tcg/s390x/lae.c
> new file mode 100644
> index 00000000000..661e95f9978
> --- /dev/null
> +++ b/tests/tcg/s390x/lae.c
> @@ -0,0 +1,25 @@
> +/*
> + * Test the LOAD ADDRESS EXTENDED instruction.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <assert.h>
> +#include <stdlib.h>
> +
> +int main(void)
> +{
> + unsigned long long ar = -1, b2 = 100000, r, x2 = 500;
> + int tmp;
> +
> + asm("ear %[tmp],%[r]\n"
> + "lae %[r],42(%[x2],%[b2])\n"
> + "ear %[ar],%[r]\n"
> + "sar %[r],%[tmp]"
> + : [tmp] "=&r" (tmp), [r] "=&r" (r), [ar] "+r" (ar)
> + : [b2] "r" (b2), [x2] "r" (x2)
> + : "memory");
> + assert(ar == 0xffffffff00000000ULL);
> + assert(r == 100542);
> +
> + return EXIT_SUCCESS;
> +}
I'm sorry, but it fails when building with Clang (version 17):
.../qemu/tests/tcg/s390x/lae.c:14:9: error: invalid operand for instruction
14 | asm("ear %[tmp],%[r]\n"
| ^
<inline asm>:1:10: note: instantiated into assembly here
1 | ear %r2,%r1
| ^
.../qemu/tests/tcg/s390x/lae.c:16:10: error: invalid operand for instruction
16 | "ear %[ar],%[r]\n"
| ^
<inline asm>:3:9: note: instantiated into assembly here
3 | ear %r0,%r1
| ^
.../qemu/tests/tcg/s390x/lae.c:17:10: error: invalid operand for instruction
17 | "sar %[r],%[tmp]"
| ^
<inline asm>:4:5: note: instantiated into assembly here
4 | sar %r1,%r2
| ^
3 errors generated.
Any suggestions how to fix it best?
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED
2024-01-11 8:37 ` Thomas Huth
@ 2024-01-11 9:04 ` Ilya Leoshkevich
0 siblings, 0 replies; 7+ messages in thread
From: Ilya Leoshkevich @ 2024-01-11 9:04 UTC (permalink / raw)
To: Thomas Huth, Richard Henderson, David Hildenbrand; +Cc: qemu-s390x, qemu-devel
On Thu, 2024-01-11 at 09:37 +0100, Thomas Huth wrote:
> On 10/01/2024 00.22, Ilya Leoshkevich wrote:
> > Add a small test to prevent regressions. Userspace runs in primary
> > mode, so LAE should always set the access register to 0.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> > tests/tcg/s390x/Makefile.target | 1 +
> > tests/tcg/s390x/lae.c | 25 +++++++++++++++++++++++++
> > 2 files changed, 26 insertions(+)
> > create mode 100644 tests/tcg/s390x/lae.c
> >
> > diff --git a/tests/tcg/s390x/Makefile.target
> > b/tests/tcg/s390x/Makefile.target
> > index 0e670f3f8b9..30994dcf9c2 100644
> > --- a/tests/tcg/s390x/Makefile.target
> > +++ b/tests/tcg/s390x/Makefile.target
> > @@ -44,6 +44,7 @@ TESTS+=clgebr
> > TESTS+=clc
> > TESTS+=laalg
> > TESTS+=add-logical-with-carry
> > +TESTS+=lae
> >
> > cdsg: CFLAGS+=-pthread
> > cdsg: LDFLAGS+=-pthread
> > diff --git a/tests/tcg/s390x/lae.c b/tests/tcg/s390x/lae.c
> > new file mode 100644
> > index 00000000000..661e95f9978
> > --- /dev/null
> > +++ b/tests/tcg/s390x/lae.c
> > @@ -0,0 +1,25 @@
> > +/*
> > + * Test the LOAD ADDRESS EXTENDED instruction.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +#include <assert.h>
> > +#include <stdlib.h>
> > +
> > +int main(void)
> > +{
> > + unsigned long long ar = -1, b2 = 100000, r, x2 = 500;
> > + int tmp;
> > +
> > + asm("ear %[tmp],%[r]\n"
> > + "lae %[r],42(%[x2],%[b2])\n"
> > + "ear %[ar],%[r]\n"
> > + "sar %[r],%[tmp]"
> > + : [tmp] "=&r" (tmp), [r] "=&r" (r), [ar] "+r" (ar)
> > + : [b2] "r" (b2), [x2] "r" (x2)
> > + : "memory");
> > + assert(ar == 0xffffffff00000000ULL);
> > + assert(r == 100542);
> > +
> > + return EXIT_SUCCESS;
> > +}
>
> I'm sorry, but it fails when building with Clang (version 17):
>
> .../qemu/tests/tcg/s390x/lae.c:14:9: error: invalid operand for
> instruction
> 14 | asm("ear %[tmp],%[r]\n"
> | ^
> <inline asm>:1:10: note: instantiated into assembly here
> 1 | ear %r2,%r1
> | ^
> .../qemu/tests/tcg/s390x/lae.c:16:10: error: invalid operand for
> instruction
> 16 | "ear %[ar],%[r]\n"
> | ^
> <inline asm>:3:9: note: instantiated into assembly here
> 3 | ear %r0,%r1
> | ^
> .../qemu/tests/tcg/s390x/lae.c:17:10: error: invalid operand for
> instruction
> 17 | "sar %[r],%[tmp]"
> | ^
> <inline asm>:4:5: note: instantiated into assembly here
> 4 | sar %r1,%r2
> | ^
> 3 errors generated.
>
> Any suggestions how to fix it best?
>
> Thomas
>
clang wants %aN there, and I don't see a way to convert %rN to %aN.
Seems like I'll have to hardcode the register number. I'll send a v2.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-01-11 9:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-09 23:22 [PATCH 0/2] target/s390x: Fix LAE setting a wrong access register Ilya Leoshkevich
2024-01-09 23:22 ` [PATCH 1/2] " Ilya Leoshkevich
2024-01-10 9:11 ` David Hildenbrand
2024-01-09 23:22 ` [PATCH 2/2] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED Ilya Leoshkevich
2024-01-10 16:50 ` Thomas Huth
2024-01-11 8:37 ` Thomas Huth
2024-01-11 9:04 ` Ilya Leoshkevich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.