[kvm-unit-tests PATCH v5] s390x: Add tests for execute-type instructions

[kvm-unit-tests PATCH v5] s390x: Add tests for execute-type instructions

[Nina Schoetterl-Glausch]

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.

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---


v4 -> v5:
 * word align the execute-type instruction, preventing a specification
   exception if the address calculation is wrong, since LLGFRL requires
   word alignment
 * change wording of comment

v3 -> v4:
 * fix nits (thanks Janosch)
 * pickup R-b (thanks Janosch)

v2 -> v3:
 * add some comments (thanks Janosch)
 * add two new tests (drop Nico's R-b)
 * push prefix

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.
Everything that has an operand that is relative to the instruction given by
the immediate in the instruction and goes through in2_ri2 in TCG has this
problem, because in2_ri2 does the calculation relative to pc_next which is the
address of the EX(RL).
That should make fixing it easier tho.


Range-diff against v4:
1:  f29ef634 ! 1:  57f8f256 s390x: Add tests for execute-type instructions
    @@ s390x/ex.c (new)
     +		"	.popsection\n"
     +
     +		"	llgfrl	%[target],0b\n"
    ++		//align (pad with nop), in case the wrong operand is used
    ++		"	.balignw 4,0x0707\n"
     +		"	exrl	0,0b\n"
     +		: [target] "=d" (target),
     +		  [value] "=d" (value)
    @@ s390x/ex.c (new)
     +		"	.popsection\n"
     +
     +		"	lrl	%[crl_word],0b\n"
    -+		//align (pad with nop), in case the wrong bad operand is used
    ++		//align (pad with nop), in case the wrong operand is used
     +		"	.balignw 4,0x0707\n"
     +		"	exrl	0,0b\n"
     +		"	ipm	%[program_mask]\n"

 s390x/Makefile      |   1 +
 s390x/ex.c          | 172 ++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |   3 +
 .gitlab-ci.yml      |   1 +
 4 files changed, 177 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..f05f8f90
--- /dev/null
+++ b/s390x/ex.c
@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright IBM Corp. 2023
+ *
+ * Test EXECUTE (RELATIVE LONG).
+ * These instructions execute a target instruction. The target instruction is formed
+ * by reading an instruction from memory and optionally modifying some of its bits.
+ * The execution of the target instruction is the same as if it was executed
+ * normally as part of the instruction sequence, except for the instruction
+ * address and the instruction-length code.
+ */
+
+#include <libcflat.h>
+
+/*
+ * BRANCH AND SAVE, register register variant.
+ * Saves the next instruction address (address from PSW + length of instruction)
+ * to the first register. No branch is taken in this test, because 0 is
+ * specified as target.
+ * BASR does *not* perform a relative address calculation with an intermediate.
+ */
+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();
+}
+
+/*
+ * BRANCH RELATIVE AND SAVE.
+ * According to PoP (Branch-Address Generation), the address calculated relative
+ * to the instruction address is relative to BRAS when it is the target of an
+ * execute-type instruction, not relative to the execute-type instruction.
+ */
+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();
+}
+
+/*
+ * LOAD ADDRESS RELATIVE LONG.
+ * If it is the target of an execute-type instruction, the address is relative
+ * to the LARL.
+ */
+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();
+}
+
+/* LOAD LOGICAL RELATIVE LONG.
+ * If it is the target of an execute-type instruction, the address is relative
+ * to the LLGFRL.
+ */
+static void test_llgfrl(void)
+{
+	uint64_t target, value;
+
+	report_prefix_push("LLGFRL");
+	asm volatile ( ".pushsection .rodata\n"
+		"	.balign	4\n"
+		"0:	llgfrl	%[value],0\n"
+		"	.popsection\n"
+
+		"	llgfrl	%[target],0b\n"
+		//align (pad with nop), in case the wrong operand is used
+		"	.balignw 4,0x0707\n"
+		"	exrl	0,0b\n"
+		: [target] "=d" (target),
+		  [value] "=d" (value)
+	);
+
+	report(target == value, "loaded correct value");
+	report_prefix_pop();
+}
+
+/*
+ * COMPARE RELATIVE LONG
+ * If it is the target of an execute-type instruction, the address is relative
+ * to the CRL.
+ */
+static void test_crl(void)
+{
+	uint32_t program_mask, cc, crl_word;
+
+	report_prefix_push("CRL");
+	asm volatile ( ".pushsection .rodata\n"
+		 //operand of crl must be word aligned
+		 "	.balign	4\n"
+		"0:	crl	%[crl_word],0\n"
+		"	.popsection\n"
+
+		"	lrl	%[crl_word],0b\n"
+		//align (pad with nop), in case the wrong operand is used
+		"	.balignw 4,0x0707\n"
+		"	exrl	0,0b\n"
+		"	ipm	%[program_mask]\n"
+		: [program_mask] "=d" (program_mask),
+		  [crl_word] "=d" (crl_word)
+		:: "cc"
+	);
+
+	cc = program_mask >> 28;
+	report(!cc, "operand compared to is relative to CRL");
+	report_prefix_pop();
+}
+
+int main(int argc, char **argv)
+{
+	report_prefix_push("ex");
+	test_basr();
+	test_bras();
+	test_larl();
+	test_llgfrl();
+	test_crl();
+	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
-- 
2.39.1

Re: [kvm-unit-tests PATCH v5] s390x: Add tests for execute-type instructions

[Claudio Imbrenda]

On Fri, 10 Mar 2023 19:11:31 +0100
Nina Schoetterl-Glausch <nsg@linux.ibm.com> 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.
> 
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> ---
> 
> 
> v4 -> v5:
>  * word align the execute-type instruction, preventing a specification
>    exception if the address calculation is wrong, since LLGFRL requires
>    word alignment
>  * change wording of comment
> 
> v3 -> v4:
>  * fix nits (thanks Janosch)
>  * pickup R-b (thanks Janosch)
> 
> v2 -> v3:
>  * add some comments (thanks Janosch)
>  * add two new tests (drop Nico's R-b)
>  * push prefix
> 
> 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.
> Everything that has an operand that is relative to the instruction given by
> the immediate in the instruction and goes through in2_ri2 in TCG has this
> problem, because in2_ri2 does the calculation relative to pc_next which is the
> address of the EX(RL).
> That should make fixing it easier tho.
> 
> 
> Range-diff against v4:
> 1:  f29ef634 ! 1:  57f8f256 s390x: Add tests for execute-type instructions
>     @@ s390x/ex.c (new)
>      +		"	.popsection\n"
>      +
>      +		"	llgfrl	%[target],0b\n"
>     ++		//align (pad with nop), in case the wrong operand is used
>     ++		"	.balignw 4,0x0707\n"
>      +		"	exrl	0,0b\n"
>      +		: [target] "=d" (target),
>      +		  [value] "=d" (value)
>     @@ s390x/ex.c (new)
>      +		"	.popsection\n"
>      +
>      +		"	lrl	%[crl_word],0b\n"
>     -+		//align (pad with nop), in case the wrong bad operand is used
>     ++		//align (pad with nop), in case the wrong operand is used
>      +		"	.balignw 4,0x0707\n"
>      +		"	exrl	0,0b\n"
>      +		"	ipm	%[program_mask]\n"
> 
>  s390x/Makefile      |   1 +
>  s390x/ex.c          | 172 ++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg |   3 +
>  .gitlab-ci.yml      |   1 +
>  4 files changed, 177 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..f05f8f90
> --- /dev/null
> +++ b/s390x/ex.c
> @@ -0,0 +1,172 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright IBM Corp. 2023
> + *
> + * Test EXECUTE (RELATIVE LONG).
> + * These instructions execute a target instruction. The target instruction is formed
> + * by reading an instruction from memory and optionally modifying some of its bits.
> + * The execution of the target instruction is the same as if it was executed
> + * normally as part of the instruction sequence, except for the instruction
> + * address and the instruction-length code.
> + */
> +
> +#include <libcflat.h>
> +
> +/*
> + * BRANCH AND SAVE, register register variant.
> + * Saves the next instruction address (address from PSW + length of instruction)
> + * to the first register. No branch is taken in this test, because 0 is
> + * specified as target.
> + * BASR does *not* perform a relative address calculation with an intermediate.
> + */
> +static void test_basr(void)
> +{
> +	uint64_t ret_addr, after_ex;
> +
> +	report_prefix_push("BASR");
> +	asm volatile ( ".pushsection .rodata\n"

you use .text.ex_bras in the next test, why not something like that here
(and everywhere else) too?

> +		"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();
> +}
> +
> +/*
> + * BRANCH RELATIVE AND SAVE.
> + * According to PoP (Branch-Address Generation), the address calculated relative
> + * to the instruction address is relative to BRAS when it is the target of an
> + * execute-type instruction, not relative to the execute-type instruction.
> + */
> +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"

please explain why briefly (i.e. if the wrong target is executed and
the offset mismatches Bad Things™ happen)

> +		"	.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();
> +}
> +
> +/*
> + * LOAD ADDRESS RELATIVE LONG.
> + * If it is the target of an execute-type instruction, the address is relative
> + * to the LARL.
> + */
> +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();
> +}
> +
> +/* LOAD LOGICAL RELATIVE LONG.
> + * If it is the target of an execute-type instruction, the address is relative
> + * to the LLGFRL.
> + */
> +static void test_llgfrl(void)
> +{
> +	uint64_t target, value;
> +
> +	report_prefix_push("LLGFRL");
> +	asm volatile ( ".pushsection .rodata\n"
> +		"	.balign	4\n"

explain the alignment (like you did in the test below)

> +		"0:	llgfrl	%[value],0\n"
> +		"	.popsection\n"
> +
> +		"	llgfrl	%[target],0b\n"
> +		//align (pad with nop), in case the wrong operand is used
> +		"	.balignw 4,0x0707\n"
> +		"	exrl	0,0b\n"
> +		: [target] "=d" (target),
> +		  [value] "=d" (value)
> +	);
> +
> +	report(target == value, "loaded correct value");
> +	report_prefix_pop();
> +}
> +
> +/*
> + * COMPARE RELATIVE LONG
> + * If it is the target of an execute-type instruction, the address is relative
> + * to the CRL.
> + */
> +static void test_crl(void)
> +{
> +	uint32_t program_mask, cc, crl_word;
> +
> +	report_prefix_push("CRL");
> +	asm volatile ( ".pushsection .rodata\n"
> +		 //operand of crl must be word aligned
> +		 "	.balign	4\n"
> +		"0:	crl	%[crl_word],0\n"
> +		"	.popsection\n"
> +
> +		"	lrl	%[crl_word],0b\n"
> +		//align (pad with nop), in case the wrong operand is used
> +		"	.balignw 4,0x0707\n"
> +		"	exrl	0,0b\n"
> +		"	ipm	%[program_mask]\n"
> +		: [program_mask] "=d" (program_mask),
> +		  [crl_word] "=d" (crl_word)
> +		:: "cc"
> +	);
> +
> +	cc = program_mask >> 28;
> +	report(!cc, "operand compared to is relative to CRL");
> +	report_prefix_pop();
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	report_prefix_push("ex");
> +	test_basr();
> +	test_bras();
> +	test_larl();
> +	test_llgfrl();
> +	test_crl();
> +	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

Re: [kvm-unit-tests PATCH v5] s390x: Add tests for execute-type instructions

[Nina Schoetterl-Glausch]

On Mon, 2023-03-13 at 19:16 +0100, Claudio Imbrenda wrote:
> On Fri, 10 Mar 2023 19:11:31 +0100
> Nina Schoetterl-Glausch <nsg@linux.ibm.com> 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.
> > 
> > Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> > Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > ---
> > 
> > 
> > v4 -> v5:
> >  * word align the execute-type instruction, preventing a specification
> >    exception if the address calculation is wrong, since LLGFRL requires
> >    word alignment
> >  * change wording of comment
> > 
> > v3 -> v4:
> >  * fix nits (thanks Janosch)
> >  * pickup R-b (thanks Janosch)
> > 
> > v2 -> v3:
> >  * add some comments (thanks Janosch)
> >  * add two new tests (drop Nico's R-b)
> >  * push prefix
> > 
> > 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.
> > Everything that has an operand that is relative to the instruction given by
> > the immediate in the instruction and goes through in2_ri2 in TCG has this
> > problem, because in2_ri2 does the calculation relative to pc_next which is the
> > address of the EX(RL).
> > That should make fixing it easier tho.
> > 
> > 
> > Range-diff against v4:
> > 1:  f29ef634 ! 1:  57f8f256 s390x: Add tests for execute-type instructions
> >     @@ s390x/ex.c (new)
> >      +		"	.popsection\n"
> >      +
> >      +		"	llgfrl	%[target],0b\n"
> >     ++		//align (pad with nop), in case the wrong operand is used
> >     ++		"	.balignw 4,0x0707\n"
> >      +		"	exrl	0,0b\n"
> >      +		: [target] "=d" (target),
> >      +		  [value] "=d" (value)
> >     @@ s390x/ex.c (new)
> >      +		"	.popsection\n"
> >      +
> >      +		"	lrl	%[crl_word],0b\n"
> >     -+		//align (pad with nop), in case the wrong bad operand is used
> >     ++		//align (pad with nop), in case the wrong operand is used
> >      +		"	.balignw 4,0x0707\n"
> >      +		"	exrl	0,0b\n"
> >      +		"	ipm	%[program_mask]\n"
> > 
> >  s390x/Makefile      |   1 +
> >  s390x/ex.c          | 172 ++++++++++++++++++++++++++++++++++++++++++++
> >  s390x/unittests.cfg |   3 +
> >  .gitlab-ci.yml      |   1 +
> >  4 files changed, 177 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..f05f8f90
> > --- /dev/null
> > +++ b/s390x/ex.c
> > @@ -0,0 +1,172 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright IBM Corp. 2023
> > + *
> > + * Test EXECUTE (RELATIVE LONG).
> > + * These instructions execute a target instruction. The target instruction is formed
> > + * by reading an instruction from memory and optionally modifying some of its bits.
> > + * The execution of the target instruction is the same as if it was executed
> > + * normally as part of the instruction sequence, except for the instruction
> > + * address and the instruction-length code.
> > + */
> > +
> > +#include <libcflat.h>
> > +
> > +/*
> > + * BRANCH AND SAVE, register register variant.
> > + * Saves the next instruction address (address from PSW + length of instruction)
> > + * to the first register. No branch is taken in this test, because 0 is
> > + * specified as target.
> > + * BASR does *not* perform a relative address calculation with an intermediate.
> > + */
> > +static void test_basr(void)
> > +{
> > +	uint64_t ret_addr, after_ex;
> > +
> > +	report_prefix_push("BASR");
> > +	asm volatile ( ".pushsection .rodata\n"
> 
> you use .text.ex_bras in the next test, why not something like that here
> (and everywhere else) too?

In the test below we branch to the code in .text.ex_bras.
In all other tests the instruction in .rodata is just an operand of the execute instruction,
and it doesn't get modified.
As for the bras test having a suffix, I guess it's pretty arbitrary, but since it's a handful
of instructions instead of just one, it felt substantial enough to warrant one.

> 
> > +		"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();
> > +}
> > +
> > +/*
> > + * BRANCH RELATIVE AND SAVE.
> > + * According to PoP (Branch-Address Generation), the address calculated relative
> > + * to the instruction address is relative to BRAS when it is the target of an
> > + * execute-type instruction, not relative to the execute-type instruction.
> > + */
> > +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"
/*
 * In case the address calculation is correct, we jump by the relative offset 1b-0b from 0b to 1b.
 * In case the address calculation is relative to the exrl (i.e. a test failure),
 * put a valid instruction at the same relative offset from the exrl, so the test continues in a
 * controlled manner.
 */
> > +		"3:	larl	%[branch_addr],0\n"
> > +		"4:\n"
> > +
> > +		"	.if (1b - 0b) != (3b - 2b)\n"
> > +		"	.error	\"right and wrong target must have same offset\"\n"
> 
> please explain why briefly (i.e. if the wrong target is executed and
> the offset mismatches Bad Things™ happen)

Ok, see above.

[...]

Re: [kvm-unit-tests PATCH v5] s390x: Add tests for execute-type instructions

[Claudio Imbrenda]

On Mon, 13 Mar 2023 23:45:33 +0100
Nina Schoetterl-Glausch <nsg@linux.ibm.com> wrote:


[...]

> > > +/*
> > > + * BRANCH AND SAVE, register register variant.
> > > + * Saves the next instruction address (address from PSW + length of instruction)
> > > + * to the first register. No branch is taken in this test, because 0 is
> > > + * specified as target.
> > > + * BASR does *not* perform a relative address calculation with an intermediate.
> > > + */
> > > +static void test_basr(void)
> > > +{
> > > +	uint64_t ret_addr, after_ex;
> > > +
> > > +	report_prefix_push("BASR");
> > > +	asm volatile ( ".pushsection .rodata\n"  
> > 
> > you use .text.ex_bras in the next test, why not something like that here
> > (and everywhere else) too?  
> 
> In the test below we branch to the code in .text.ex_bras.
> In all other tests the instruction in .rodata is just an operand of the execute instruction,
> and it doesn't get modified.
> As for the bras test having a suffix, I guess it's pretty arbitrary, but since it's a handful
> of instructions instead of just one, it felt substantial enough to warrant one.
> 

we discussed this offline :)

> >   
> > > +		"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();
> > > +}
> > > +
> > > +/*
> > > + * BRANCH RELATIVE AND SAVE.
> > > + * According to PoP (Branch-Address Generation), the address calculated relative
> > > + * to the instruction address is relative to BRAS when it is the target of an
> > > + * execute-type instruction, not relative to the execute-type instruction.
> > > + */
> > > +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"  
> /*
>  * In case the address calculation is correct, we jump by the relative offset 1b-0b from 0b to 1b.
>  * In case the address calculation is relative to the exrl (i.e. a test failure),
>  * put a valid instruction at the same relative offset from the exrl, so the test continues in a
>  * controlled manner.
>  */

looks good

> > > +		"3:	larl	%[branch_addr],0\n"
> > > +		"4:\n"
> > > +
> > > +		"	.if (1b - 0b) != (3b - 2b)\n"
> > > +		"	.error	\"right and wrong target must have same offset\"\n"  
> > 
> > please explain why briefly (i.e. if the wrong target is executed and
> > the offset mismatches Bad Things™ happen)  
> 
> Ok, see above.
> 
> [...]
> 
>