* [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
* 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
* [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 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).