On 2/24/23 16:20, Nina Schoetterl-Glausch wrote:
Test the instruction address used by targets of an execute instruction.
When the target instruction calculates a relative address, the result is
relative to the target instruction, not the execute instruction.
For instructions like execute where the details matter it's a great idea
to have a lot of comments maybe even loose references to the PoP so
people can read up on the issue more easily.
Signed-off-by: Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx>
Reviewed-by: Nico Boehr <nrb@xxxxxxxxxxxxx>
---
v1 -> v2:
* add test to unittests.cfg and .gitlab-ci.yml
* pick up R-b (thanks Nico)
TCG does the address calculation relative to the execute instruction.
Always?
I.e. what are you telling me here?
s390x/Makefile | 1 +
s390x/ex.c | 92 +++++++++++++++++++++++++++++++++++++++++++++
s390x/unittests.cfg | 3 ++
.gitlab-ci.yml | 1 +
4 files changed, 97 insertions(+)
create mode 100644 s390x/ex.c
diff --git a/s390x/Makefile b/s390x/Makefile
index 97a61611..6cf8018b 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -39,6 +39,7 @@ tests += $(TEST_DIR)/panic-loop-extint.elf
tests += $(TEST_DIR)/panic-loop-pgm.elf
tests += $(TEST_DIR)/migration-sck.elf
tests += $(TEST_DIR)/exittime.elf
+tests += $(TEST_DIR)/ex.elf
pv-tests += $(TEST_DIR)/pv-diags.elf
diff --git a/s390x/ex.c b/s390x/ex.c
new file mode 100644
index 00000000..1bf4d8cd
--- /dev/null
+++ b/s390x/ex.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright IBM Corp. 2023
+ *
+ * Test EXECUTE (RELATIVE LONG).
+ */
+
+#include <libcflat.h>
+
Take my words with some salt, I never had a close look at the branch
instructions other than brc.
This is "branch and save" and the "r" in "basr" says that it's the RR
variant. It's not relative the way that "bras" is, right?
Hence ret_addr and after_ex both point to 1f.
I'd like to have a comment here that states that this is not a relative
branch at all. The r specifies the instruction format.
+static void test_basr(void)
+{
+ uint64_t ret_addr, after_ex;
+
+ report_prefix_push("BASR");
+ asm volatile ( ".pushsection .rodata\n"
+ "0: basr %[ret_addr],0\n"
+ " .popsection\n"
+
+ " larl %[after_ex],1f\n"
+ " exrl 0,0b\n"
+ "1:\n"
+ : [ret_addr] "=d" (ret_addr),
+ [after_ex] "=d" (after_ex)
+ );
+
+ report(ret_addr == after_ex, "return address after EX");
+ report_prefix_pop();
+}
+
+/*
+ * According to PoP (Branch-Address Generation), the address is relative to
+ * BRAS when it is the target of an execute-type instruction.
+ */
Is there any merit in testing the other br* instructions as well or are
they running through the same TCG function?
+static void test_bras(void)
+{
+ uint64_t after_target, ret_addr, after_ex, branch_addr;
+
+ report_prefix_push("BRAS");
+ asm volatile ( ".pushsection .text.ex_bras, \"x\"\n"
+ "0: bras %[ret_addr],1f\n"
+ " nopr %%r7\n"
+ "1: larl %[branch_addr],0\n"
+ " j 4f\n"
+ " .popsection\n"
+
+ " larl %[after_target],1b\n"
+ " larl %[after_ex],3f\n"
+ "2: exrl 0,0b\n"
+ "3: larl %[branch_addr],0\n"
+ "4:\n"
+
+ " .if (1b - 0b) != (3b - 2b)\n"
+ " .error \"right and wrong target must have same offset\"\n"
+ " .endif\n"
+ : [after_target] "=d" (after_target),
+ [ret_addr] "=d" (ret_addr),
+ [after_ex] "=d" (after_ex),
+ [branch_addr] "=d" (branch_addr)
+ );
+
+ report(after_target == branch_addr, "address calculated relative to BRAS");
+ report(ret_addr == after_ex, "return address after EX");
+ report_prefix_pop();
+}
+
Add:
/* larl follows the address generation of relative branch instructions */
+static void test_larl(void)
+{
+ uint64_t target, addr;
+
+ report_prefix_push("LARL");
+ asm volatile ( ".pushsection .rodata\n"
+ "0: larl %[addr],0\n"
+ " .popsection\n"
+
+ " larl %[target],0b\n"
+ " exrl 0,0b\n"
+ : [target] "=d" (target),
+ [addr] "=d" (addr)
+ );
+
+ report(target == addr, "address calculated relative to LARL");
+ report_prefix_pop();
+}
+
+int main(int argc, char **argv)
+{
We're missing push and pop around the test function block so that we
know which file generated the output.
report_prefix_push("execute");
+ test_basr();
+ test_bras();
+ test_larl();
report_prefix_pop();
+ return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index d97eb5e9..b61faf07 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -215,3 +215,6 @@ file = migration-skey.elf
smp = 2
groups = migration
extra_params = -append '--parallel'
+
+[execute]
+file = ex.elf
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ad7949c9..a999f64a 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -275,6 +275,7 @@ s390x-kvm:
- ACCEL=kvm ./run_tests.sh
selftest-setup intercept emulator sieve sthyi diag10 diag308 pfmf
cmm vector gs iep cpumodel diag288 stsi sclp-1g sclp-3g css skrf sie
+ execute
| tee results.txt
- grep -q PASS results.txt && ! grep -q FAIL results.txt
only:
base-commit: e3c5c3ef2524c58023073c0fadde2e8ae3c04ec6
