* [PATCH v3 0/2] Fix EXECUTE of relative long instructions
@ 2023-03-16 21:07 Ilya Leoshkevich
2023-03-16 21:07 ` [PATCH v3 1/2] target/s390x: " Ilya Leoshkevich
2023-03-16 21:07 ` [PATCH v3 2/2] tests/tcg/s390x: Add ex-relative-long.c Ilya Leoshkevich
0 siblings, 2 replies; 3+ messages in thread
From: Ilya Leoshkevich @ 2023-03-16 21:07 UTC (permalink / raw)
To: Richard Henderson, David Hildenbrand, Thomas Huth
Cc: qemu-s390x, qemu-devel, Ilya Leoshkevich
v2: https://lists.gnu.org/archive/html/qemu-devel/2023-03/msg04499.html
v2 -> v3: Make mem static (Nina).
Initialize cc with cr (Nina).
Drop long casts (Nina).
Move mask assignment outside of asm.
Use "a" constraints instead of "r" where necessary.
Drop unnecessary earlyclobbers.
v1: https://lists.gnu.org/archive/html/qemu-devel/2023-03/msg04316.html
v1 -> v2: Address the middle of an array in the test (Richard).
Rebase - not 100% trivial, so not carrying Reviewed-bys.
Hi,
This series fixes EXECUTE of instructions like LARL, LGLR, etc.
Currently the address calculation uses EXECUTE's address as a base,
while it should be using that of the target instruction.
Patch 1 fixes the issue, patch 2 adds a test.
Best regards,
Ilya
Ilya Leoshkevich (2):
target/s390x: Fix EXECUTE of relative long instructions
tests/tcg/s390x: Add ex-relative-long.c
target/s390x/cpu.h | 1 +
target/s390x/tcg/mem_helper.c | 1 +
target/s390x/tcg/translate.c | 13 ++-
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/ex-relative-long.c | 156 +++++++++++++++++++++++++++++
5 files changed, 171 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/s390x/ex-relative-long.c
--
2.39.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v3 1/2] target/s390x: Fix EXECUTE of relative long instructions
2023-03-16 21:07 [PATCH v3 0/2] Fix EXECUTE of relative long instructions Ilya Leoshkevich
@ 2023-03-16 21:07 ` Ilya Leoshkevich
2023-03-16 21:07 ` [PATCH v3 2/2] tests/tcg/s390x: Add ex-relative-long.c Ilya Leoshkevich
1 sibling, 0 replies; 3+ messages in thread
From: Ilya Leoshkevich @ 2023-03-16 21:07 UTC (permalink / raw)
To: Richard Henderson, David Hildenbrand, Thomas Huth
Cc: qemu-s390x, qemu-devel, Ilya Leoshkevich, Nina Schoetterl-Glausch
The code uses the wrong base for relative addressing: it should use the
target instruction address and not the EXECUTE's address.
Fix by storing the target instruction address in the new CPUS390XState
member and loading it from the code generated by gen_ri2().
Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
target/s390x/cpu.h | 1 +
target/s390x/tcg/mem_helper.c | 1 +
target/s390x/tcg/translate.c | 13 ++++++++++++-
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 7d6d01325b2..8aaf8dd5a3b 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -87,6 +87,7 @@ struct CPUArchState {
uint64_t cc_vr;
uint64_t ex_value;
+ uint64_t ex_target;
uint64_t __excp_addr;
uint64_t psa;
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 6835c26dda4..00afae2b640 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -2530,6 +2530,7 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t r1, uint64_t addr)
that ex_value is non-zero, which flags that we are in a state
that requires such execution. */
env->ex_value = insn | ilen;
+ env->ex_target = addr;
}
uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 14c3896d529..e938d8538f8 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -5747,7 +5747,18 @@ static void in2_a2(DisasContext *s, DisasOps *o)
static TCGv gen_ri2(DisasContext *s)
{
- return tcg_constant_i64(s->base.pc_next + (int64_t)get_field(s, i2) * 2);
+ int64_t delta = (int64_t)get_field(s, i2) * 2;
+ TCGv ri2;
+
+ if (unlikely(s->ex_value)) {
+ ri2 = tcg_temp_new_i64();
+ tcg_gen_ld_i64(ri2, cpu_env, offsetof(CPUS390XState, ex_target));
+ tcg_gen_addi_i64(ri2, ri2, delta);
+ } else {
+ ri2 = tcg_constant_i64(s->base.pc_next + delta);
+ }
+
+ return ri2;
}
static void in2_ri2(DisasContext *s, DisasOps *o)
--
2.39.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v3 2/2] tests/tcg/s390x: Add ex-relative-long.c
2023-03-16 21:07 [PATCH v3 0/2] Fix EXECUTE of relative long instructions Ilya Leoshkevich
2023-03-16 21:07 ` [PATCH v3 1/2] target/s390x: " Ilya Leoshkevich
@ 2023-03-16 21:07 ` Ilya Leoshkevich
1 sibling, 0 replies; 3+ messages in thread
From: Ilya Leoshkevich @ 2023-03-16 21:07 UTC (permalink / raw)
To: Richard Henderson, David Hildenbrand, Thomas Huth
Cc: qemu-s390x, qemu-devel, Ilya Leoshkevich, Nina Schoetterl-Glausch
Test EXECUTE and EXECUTE RELATIVE LONG with relative long instructions
as targets.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/ex-relative-long.c | 156 +++++++++++++++++++++++++++++
2 files changed, 157 insertions(+)
create mode 100644 tests/tcg/s390x/ex-relative-long.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index cf93b966862..90bc48227db 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -29,6 +29,7 @@ TESTS+=clst
TESTS+=long-double
TESTS+=cdsg
TESTS+=chrl
+TESTS+=ex-relative-long
cdsg: CFLAGS+=-pthread
cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/ex-relative-long.c b/tests/tcg/s390x/ex-relative-long.c
new file mode 100644
index 00000000000..21fbef62585
--- /dev/null
+++ b/tests/tcg/s390x/ex-relative-long.c
@@ -0,0 +1,156 @@
+/* Check EXECUTE with relative long instructions as targets. */
+#include <stdlib.h>
+#include <stdio.h>
+
+struct test {
+ const char *name;
+ long (*func)(long reg, long *cc);
+ long exp_reg;
+ long exp_mem;
+ long exp_cc;
+};
+
+/*
+ * Each test sets the MEM_IDXth element of the mem array to MEM and uses a
+ * single relative long instruction on it. The other elements remain zero.
+ * This is in order to prevent stumbling upon MEM in random memory in case
+ * there is an off-by-a-small-value bug.
+ *
+ * Note that while gcc supports the ZL constraint for relative long operands,
+ * clang doesn't, so the assembly code accesses mem[MEM_IDX] using MEM_ASM.
+ */
+static long mem[0x1000];
+#define MEM_IDX 0x800
+#define MEM_ASM "mem+0x800*8"
+
+/* Initial %r2 value. */
+#define REG 0x1234567887654321
+
+/* Initial mem[MEM_IDX] value. */
+#define MEM 0xfedcba9889abcdef
+
+/* Initial cc value. */
+#define CC 0
+
+/* Relative long instructions and their expected effects. */
+#define FOR_EACH_INSN(F) \
+ F(cgfrl, REG, MEM, 2) \
+ F(cghrl, REG, MEM, 2) \
+ F(cgrl, REG, MEM, 2) \
+ F(chrl, REG, MEM, 1) \
+ F(clgfrl, REG, MEM, 2) \
+ F(clghrl, REG, MEM, 2) \
+ F(clgrl, REG, MEM, 1) \
+ F(clhrl, REG, MEM, 2) \
+ F(clrl, REG, MEM, 1) \
+ F(crl, REG, MEM, 1) \
+ F(larl, (long)&mem[MEM_IDX], MEM, CC) \
+ F(lgfrl, 0xfffffffffedcba98, MEM, CC) \
+ F(lghrl, 0xfffffffffffffedc, MEM, CC) \
+ F(lgrl, MEM, MEM, CC) \
+ F(lhrl, 0x12345678fffffedc, MEM, CC) \
+ F(llghrl, 0x000000000000fedc, MEM, CC) \
+ F(llhrl, 0x123456780000fedc, MEM, CC) \
+ F(lrl, 0x12345678fedcba98, MEM, CC) \
+ F(stgrl, REG, REG, CC) \
+ F(sthrl, REG, 0x4321ba9889abcdef, CC) \
+ F(strl, REG, 0x8765432189abcdef, CC)
+
+/* Test functions. */
+#define DEFINE_EX_TEST(insn, exp_reg, exp_mem, exp_cc) \
+ static long test_ex_ ## insn(long reg, long *cc) \
+ { \
+ register long r2 asm("r2"); \
+ char mask = 0x20; /* make target use %r2 */ \
+ long pm, target; \
+ \
+ r2 = reg; \
+ asm("larl %[target],0f\n" \
+ "cr %%r0,%%r0\n" /* initial cc */ \
+ "ex %[mask],0(%[target])\n" \
+ "jg 1f\n" \
+ "0: " #insn " %%r0," MEM_ASM "\n" \
+ "1: ipm %[pm]\n" \
+ : [target] "=&a" (target), [r2] "+r" (r2), [pm] "=r" (pm) \
+ : [mask] "a" (mask) \
+ : "cc", "memory"); \
+ reg = r2; \
+ *cc = (pm >> 28) & 3; \
+ \
+ return reg; \
+ }
+
+#define DEFINE_EXRL_TEST(insn, exp_reg, exp_mem, exp_cc) \
+ static long test_exrl_ ## insn(long reg, long *cc) \
+ { \
+ register long r2 asm("r2"); \
+ char mask = 0x20; /* make target use %r2 */ \
+ long pm; \
+ \
+ r2 = reg; \
+ asm("cr %%r0,%%r0\n" /* initial cc */ \
+ "exrl %[mask],0f\n" \
+ "jg 1f\n" \
+ "0: " #insn " %%r0," MEM_ASM "\n" \
+ "1: ipm %[pm]\n" \
+ : [r2] "+r" (r2), [pm] "=r" (pm) \
+ : [mask] "a" (mask) \
+ : "cc", "memory"); \
+ reg = r2; \
+ *cc = (pm >> 28) & 3; \
+ \
+ return reg; \
+ }
+
+FOR_EACH_INSN(DEFINE_EX_TEST)
+FOR_EACH_INSN(DEFINE_EXRL_TEST)
+
+/* Test definitions. */
+#define REGISTER_EX_EXRL_TEST(ex_insn, insn, _exp_reg, _exp_mem, _exp_cc) \
+ { \
+ .name = #ex_insn " " #insn, \
+ .func = test_ ## ex_insn ## _ ## insn, \
+ .exp_reg = (_exp_reg), \
+ .exp_mem = (_exp_mem), \
+ .exp_cc = (_exp_cc), \
+ },
+
+#define REGISTER_EX_TEST(insn, exp_reg, exp_mem, exp_cc) \
+ REGISTER_EX_EXRL_TEST(ex, insn, exp_reg, exp_mem, exp_cc)
+
+#define REGISTER_EXRL_TEST(insn, exp_reg, exp_mem, exp_cc) \
+ REGISTER_EX_EXRL_TEST(exrl, insn, exp_reg, exp_mem, exp_cc)
+
+static const struct test tests[] = {
+ FOR_EACH_INSN(REGISTER_EX_TEST)
+ FOR_EACH_INSN(REGISTER_EXRL_TEST)
+};
+
+/* Loop over all tests and run them. */
+int main(void)
+{
+ const struct test *test;
+ int ret = EXIT_SUCCESS;
+ long reg, cc;
+ size_t i;
+
+ for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) {
+ test = &tests[i];
+ mem[MEM_IDX] = MEM;
+ cc = -1;
+ reg = test->func(REG, &cc);
+#define ASSERT_EQ(expected, actual) do { \
+ if (expected != actual) { \
+ fprintf(stderr, "%s: " #expected " (0x%lx) != " #actual " (0x%lx)\n", \
+ test->name, expected, actual); \
+ ret = EXIT_FAILURE; \
+ } \
+} while (0)
+ ASSERT_EQ(test->exp_reg, reg);
+ ASSERT_EQ(test->exp_mem, mem[MEM_IDX]);
+ ASSERT_EQ(test->exp_cc, cc);
+#undef ASSERT_EQ
+ }
+
+ return ret;
+}
--
2.39.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-03-16 21:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-16 21:07 [PATCH v3 0/2] Fix EXECUTE of relative long instructions Ilya Leoshkevich
2023-03-16 21:07 ` [PATCH v3 1/2] target/s390x: " Ilya Leoshkevich
2023-03-16 21:07 ` [PATCH v3 2/2] tests/tcg/s390x: Add ex-relative-long.c 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).