public inbox for linux-s390@vger.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH v3] s390x: Add tests for execute-type instructions
@ 2023-02-28 20:44 Nina Schoetterl-Glausch
  2023-03-03 10:11 ` Janosch Frank
  0 siblings, 1 reply; 3+ messages in thread
From: Nina Schoetterl-Glausch @ 2023-02-28 20:44 UTC (permalink / raw)
  To: Thomas Huth, Janosch Frank, Claudio Imbrenda,
	Nina Schoetterl-Glausch
  Cc: David Hildenbrand, kvm, linux-s390

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.

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


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 v2:
1:  08bae04a ! 1:  136eb2a2 s390x: Add tests for execute-type instructions
    @@ Commit message
         relative to the target instruction, not the execute instruction.
     
         Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
    -    Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
     
      ## s390x/Makefile ##
     @@ s390x/Makefile: tests += $(TEST_DIR)/panic-loop-extint.elf
    @@ s390x/ex.c (new)
     + * Copyright IBM Corp. 2023
     + *
     + * Test EXECUTE (RELATIVE LONG).
    ++ * These instruction 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;
    @@ s390x/ex.c (new)
     +}
     +
     +/*
    -+ * According to PoP (Branch-Address Generation), the address is relative to
    -+ * BRAS when it is the target of an execute-type instruction.
    ++ * 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)
     +{
    @@ s390x/ex.c (new)
     +	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;
    @@ s390x/ex.c (new)
     +	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"
    ++		"	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"
    ++		"	.balign	4\n" //operand of crl must be word aligned
    ++		"0:	crl	%[crl_word],0\n"
    ++		"	.popsection\n"
    ++
    ++		"	lrl	%[crl_word],0b\n"
    ++		//align (pad with nop), in case the wrong bad 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();
     +}

 s390x/Makefile      |   1 +
 s390x/ex.c          | 169 ++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg |   3 +
 .gitlab-ci.yml      |   1 +
 4 files changed, 174 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..3a22e496
--- /dev/null
+++ b/s390x/ex.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright IBM Corp. 2023
+ *
+ * Test EXECUTE (RELATIVE LONG).
+ * These instruction 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"
+		"	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"
+		"	.balign	4\n" //operand of crl must be word aligned
+		"0:	crl	%[crl_word],0\n"
+		"	.popsection\n"
+
+		"	lrl	%[crl_word],0b\n"
+		//align (pad with nop), in case the wrong bad 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.36.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [kvm-unit-tests PATCH v3] s390x: Add tests for execute-type instructions
  2023-02-28 20:44 [kvm-unit-tests PATCH v3] s390x: Add tests for execute-type instructions Nina Schoetterl-Glausch
@ 2023-03-03 10:11 ` Janosch Frank
  2023-03-03 10:54   ` Nina Schoetterl-Glausch
  0 siblings, 1 reply; 3+ messages in thread
From: Janosch Frank @ 2023-03-03 10:11 UTC (permalink / raw)
  To: Nina Schoetterl-Glausch, Thomas Huth, Claudio Imbrenda
  Cc: David Hildenbrand, kvm, linux-s390

On 2/28/23 21:44, 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.
> 
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

Some small nits below, I can fix that up when picking.

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
> 
> 
[...]
>   s390x/Makefile      |   1 +
>   s390x/ex.c          | 169 ++++++++++++++++++++++++++++++++++++++++++++
>   s390x/unittests.cfg |   3 +
>   .gitlab-ci.yml      |   1 +
>   4 files changed, 174 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..3a22e496
> --- /dev/null
> +++ b/s390x/ex.c
> @@ -0,0 +1,169 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright IBM Corp. 2023
> + *
> + * Test EXECUTE (RELATIVE LONG).
> + * These instruction execute a target instruction. The target instruction is formed

s/instruction/instructions/ ?

> + * 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>
> +

[...]

> +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.
> + */

This is the only instruction where there's no comment about the execute 
behavior but it would only make sense that it follows the same address 
generation rules.

> +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"
> +		"	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"
> +		"	.balign	4\n" //operand of crl must be word aligned

Just put it on a new line so we don't mix commenting stiles.
Inline assembly is already hardly readable anyway.

> +		"0:	crl	%[crl_word],0\n"
> +		"	.popsection\n"
> +
> +		"	lrl	%[crl_word],0b\n"
> +		//align (pad with nop), in case the wrong bad 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [kvm-unit-tests PATCH v3] s390x: Add tests for execute-type instructions
  2023-03-03 10:11 ` Janosch Frank
@ 2023-03-03 10:54   ` Nina Schoetterl-Glausch
  0 siblings, 0 replies; 3+ messages in thread
From: Nina Schoetterl-Glausch @ 2023-03-03 10:54 UTC (permalink / raw)
  To: Janosch Frank, Thomas Huth, Claudio Imbrenda
  Cc: David Hildenbrand, kvm, linux-s390

On Fri, 2023-03-03 at 11:11 +0100, Janosch Frank wrote:
> On 2/28/23 21:44, 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.
> > 
> > Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> 
> Some small nits below, I can fix that up when picking.
> 
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Thanks.

> 
> > ---
> > 
> > 
> [...]
> >   s390x/Makefile      |   1 +
> >   s390x/ex.c          | 169 ++++++++++++++++++++++++++++++++++++++++++++
> >   s390x/unittests.cfg |   3 +
> >   .gitlab-ci.yml      |   1 +
> >   4 files changed, 174 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..3a22e496
> > --- /dev/null
> > +++ b/s390x/ex.c
> > @@ -0,0 +1,169 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright IBM Corp. 2023
> > + *
> > + * Test EXECUTE (RELATIVE LONG).
> > + * These instruction execute a target instruction. The target instruction is formed
> 
> s/instruction/instructions/ ?

Yes.
> 
> > + * 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>
> > +
> 
> [...]
> 
> > +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.
> > + */
> 
> This is the only instruction where there's no comment about the execute 
> behavior but it would only make sense that it follows the same address 
> generation rules.

I added CRL first because I overlooked that it *does* specify it's behavior and
decided to keep it in, because why not.
Then checked all the other instructions.
It's a bit wonky, the PoP says:

Operand-Address Generation
...

The signed binary inte-
ger specifies the number of halfwords that is added
to the address of the current instruction (or the
address of the execute-type instruction if the instruc-
tion having the RI2 field is the target of an execute-
type instruction) to form the intermediate value.

So it says *address* not *target* of the execute-type insn.

then:

Branch-Address Generation
...
The branch address is the number of half-
words designated by the RI2 field added to the
address of the relative-branch instruction.
...
In all formats, the sec-
ond addend is the 64-bit address of the branch
instruction. The address of the branch instruction is
the instruction address in the PSW before that
address is updated to address the next sequential
instruction, or it is the address of the target of the
execute-type instruction (EXECUTE or EXECUTE
RELATIVE LONG) if an execute-type instruction is
used.

And everything but LLGFRL explicitly mentions that it is relative to the target.
We could mention in the comment that the PoP doesn't explicitly state that,
but the realty is that that's what the machine does.
> 
> > +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"
> > +		"	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"
> > +		"	.balign	4\n" //operand of crl must be word aligned
> 
> Just put it on a new line so we don't mix commenting stiles.
> Inline assembly is already hardly readable anyway.

Could also put it in the asm "#operand of crl must be word aligned" but I haven't
seen that anywhere else, and the benefit is negligible.
> 
> > +		"0:	crl	%[crl_word],0\n"
> > +		"	.popsection\n"
> > +
> > +		"	lrl	%[crl_word],0b\n"
> > +		//align (pad with nop), in case the wrong bad 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
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-03-03 10:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-28 20:44 [kvm-unit-tests PATCH v3] s390x: Add tests for execute-type instructions Nina Schoetterl-Glausch
2023-03-03 10:11 ` Janosch Frank
2023-03-03 10:54   ` Nina Schoetterl-Glausch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox