| Message ID | 20230404113639.37544-12-nrb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests,GIT,PULL,v2,01/14] .gitignore: ignore `s390x/comm.key` file | expand |
On 04/04/2023 13.36, Nico Boehr wrote: > From: Nina Schoetterl-Glausch <nsg@linux.ibm.com> > > 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> > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > Link: https://lore.kernel.org/r/20230317112339.774659-1-nsg@linux.ibm.com > Signed-off-by: Nico Boehr <nrb@linux.ibm.com> > --- > s390x/Makefile | 1 + > s390x/ex.c | 188 ++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 3 + > .gitlab-ci.yml | 1 + > 4 files changed, 193 insertions(+) > create mode 100644 s390x/ex.c > > diff --git a/s390x/Makefile b/s390x/Makefile > index ab146eb..a80db53 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 0000000..dbd8030 > --- /dev/null > +++ b/s390x/ex.c > @@ -0,0 +1,188 @@ > +// 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> > + > +/* > + * Accesses to the operand of execute-type instructions are instruction fetches. > + * Minimum alignment is two, since the relative offset is specified by number of halfwords. > + */ > +asm ( ".pushsection .text.exrl_targets,\"x\"\n" > +" .balign 2\n" > +" .popsection\n" > +); > + > +/* > + * 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 .text.exrl_targets\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.exrl_targets\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" > + " .endif\n" FWIW, this is failing with Clang 15 for me: s390x/ex.c:81:4: error: expected absolute expression " .if (1b - 0b) != (3b - 2b)\n" ^ <inline asm>:12:6: note: instantiated into assembly here .if (1b - 0b) != (3b - 2b) ^ s390x/ex.c:82:4: error: right and wrong target must have same offset " .error \"right and wrong target must have same offset\"\n" ^ <inline asm>:13:2: note: instantiated into assembly here .error "right and wrong target must have same offset" ^ 2 errors generated. Any easy ways to fix this? Thomas
On Tue, 2023-04-04 at 16:15 +0200, Thomas Huth wrote: > On 04/04/2023 13.36, Nico Boehr wrote: > > From: Nina Schoetterl-Glausch <nsg@linux.ibm.com> > > > > 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> > > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > Link: https://lore.kernel.org/r/20230317112339.774659-1-nsg@linux.ibm.com > > Signed-off-by: Nico Boehr <nrb@linux.ibm.com> > > --- > > s390x/Makefile | 1 + > > s390x/ex.c | 188 ++++++++++++++++++++++++++++++++++++++++++++ > > s390x/unittests.cfg | 3 + > > .gitlab-ci.yml | 1 + > > 4 files changed, 193 insertions(+) > > create mode 100644 s390x/ex.c > > > > diff --git a/s390x/Makefile b/s390x/Makefile > > index ab146eb..a80db53 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 0000000..dbd8030 > > --- /dev/null > > +++ b/s390x/ex.c > > @@ -0,0 +1,188 @@ > > +// 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> > > + > > +/* > > + * Accesses to the operand of execute-type instructions are instruction fetches. > > + * Minimum alignment is two, since the relative offset is specified by number of halfwords. > > + */ > > +asm ( ".pushsection .text.exrl_targets,\"x\"\n" > > +" .balign 2\n" > > +" .popsection\n" > > +); > > + > > +/* > > + * 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 .text.exrl_targets\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.exrl_targets\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" > > + " .endif\n" > > FWIW, this is failing with Clang 15 for me: > > s390x/ex.c:81:4: error: expected absolute expression > " .if (1b - 0b) != (3b - 2b)\n" > ^ > <inline asm>:12:6: note: instantiated into assembly here > .if (1b - 0b) != (3b - 2b) Seems gcc is smarter here than clang. > ^ > s390x/ex.c:82:4: error: right and wrong target must have same offset > " .error \"right and wrong target must have same > offset\"\n" > ^ > <inline asm>:13:2: note: instantiated into assembly here > .error "right and wrong target must have same offset" > ^ > 2 errors generated. > > Any easy ways to fix this? Just deleting that .if block would work, it's basically only a static assert. What do you think? Other than that I can't think of anything. > > Thomas >
On 04/04/2023 16.54, Nina Schoetterl-Glausch wrote: > On Tue, 2023-04-04 at 16:15 +0200, Thomas Huth wrote: >> On 04/04/2023 13.36, Nico Boehr wrote: >>> From: Nina Schoetterl-Glausch <nsg@linux.ibm.com> >>> >>> 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> >>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >>> Link: https://lore.kernel.org/r/20230317112339.774659-1-nsg@linux.ibm.com >>> Signed-off-by: Nico Boehr <nrb@linux.ibm.com> >>> --- >>> s390x/Makefile | 1 + >>> s390x/ex.c | 188 ++++++++++++++++++++++++++++++++++++++++++++ >>> s390x/unittests.cfg | 3 + >>> .gitlab-ci.yml | 1 + >>> 4 files changed, 193 insertions(+) >>> create mode 100644 s390x/ex.c >>> >>> diff --git a/s390x/Makefile b/s390x/Makefile >>> index ab146eb..a80db53 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 0000000..dbd8030 >>> --- /dev/null >>> +++ b/s390x/ex.c >>> @@ -0,0 +1,188 @@ >>> +// 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> >>> + >>> +/* >>> + * Accesses to the operand of execute-type instructions are instruction fetches. >>> + * Minimum alignment is two, since the relative offset is specified by number of halfwords. >>> + */ >>> +asm ( ".pushsection .text.exrl_targets,\"x\"\n" >>> +" .balign 2\n" >>> +" .popsection\n" >>> +); >>> + >>> +/* >>> + * 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 .text.exrl_targets\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.exrl_targets\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" >>> + " .endif\n" >> >> FWIW, this is failing with Clang 15 for me: >> >> s390x/ex.c:81:4: error: expected absolute expression >> " .if (1b - 0b) != (3b - 2b)\n" >> ^ >> <inline asm>:12:6: note: instantiated into assembly here >> .if (1b - 0b) != (3b - 2b) > > Seems gcc is smarter here than clang. Yeah, the assembler from clang is quite a bit behind on s390x ... in the past I was only able to compile the k-u-t with Clang when using the "-no-integrated-as" option ... but at least in the most recent version it seems to have caught up now enough to be very close to compile it with the built-in assembler, so it would be great to get this problem here fixed somehow, too... > Just deleting that .if block would work, it's basically only a static assert. > What do you think? > Other than that I can't think of anything. Yes, either delete it ... or maybe you could return the two values (1b - 0b) and (3b - 2b) as output from the asm statement and do an assert() in C instead? Thomas
On Tue, 2023-04-04 at 17:05 +0200, Thomas Huth wrote: > On 04/04/2023 16.54, Nina Schoetterl-Glausch wrote: > > On Tue, 2023-04-04 at 16:15 +0200, Thomas Huth wrote: > > > On 04/04/2023 13.36, Nico Boehr wrote: > > > > From: Nina Schoetterl-Glausch <nsg@linux.ibm.com> > > > > > > > > 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> > > > > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > > > Link: https://lore.kernel.org/r/20230317112339.774659-1-nsg@linux.ibm.com > > > > Signed-off-by: Nico Boehr <nrb@linux.ibm.com> > > > > --- > > > > s390x/Makefile | 1 + > > > > s390x/ex.c | 188 ++++++++++++++++++++++++++++++++++++++++++++ > > > > s390x/unittests.cfg | 3 + > > > > .gitlab-ci.yml | 1 + > > > > 4 files changed, 193 insertions(+) > > > > create mode 100644 s390x/ex.c > > > > > > > > diff --git a/s390x/Makefile b/s390x/Makefile > > > > index ab146eb..a80db53 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 0000000..dbd8030 > > > > --- /dev/null > > > > +++ b/s390x/ex.c > > > > @@ -0,0 +1,188 @@ > > > > +// 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> > > > > + > > > > +/* > > > > + * Accesses to the operand of execute-type instructions are instruction fetches. > > > > + * Minimum alignment is two, since the relative offset is specified by number of halfwords. > > > > + */ > > > > +asm ( ".pushsection .text.exrl_targets,\"x\"\n" > > > > +" .balign 2\n" > > > > +" .popsection\n" > > > > +); > > > > + > > > > +/* > > > > + * 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 .text.exrl_targets\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.exrl_targets\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" > > > > + " .endif\n" > > > > > > FWIW, this is failing with Clang 15 for me: > > > > > > s390x/ex.c:81:4: error: expected absolute expression > > > " .if (1b - 0b) != (3b - 2b)\n" > > > ^ > > > <inline asm>:12:6: note: instantiated into assembly here > > > .if (1b - 0b) != (3b - 2b) > > > > Seems gcc is smarter here than clang. > > Yeah, the assembler from clang is quite a bit behind on s390x ... in the > past I was only able to compile the k-u-t with Clang when using the > "-no-integrated-as" option ... but at least in the most recent version it > seems to have caught up now enough to be very close to compile it with the > built-in assembler, so it would be great to get this problem here fixed > somehow, too... > > > Just deleting that .if block would work, it's basically only a static assert. > > What do you think? > > Other than that I can't think of anything. > > Yes, either delete it ... or maybe you could return the two values (1b - 0b) > and (3b - 2b) as output from the asm statement and do an assert() in C instead? No, that's too late, it'd crash before if the invariant doesn't hold. Could do a runtime check in asm but I don't think it's worth it. So lets go for deletion. Do you wan't to fix it up when pulling or do you want a new version and pull request? > > Thomas >
Quoting Thomas Huth (2023-04-04 17:05:02) [...] > >> FWIW, this is failing with Clang 15 for me: > >> > >> s390x/ex.c:81:4: error: expected absolute expression > >> " .if (1b - 0b) != (3b - 2b)\n" > >> ^ > >> <inline asm>:12:6: note: instantiated into assembly here > >> .if (1b - 0b) != (3b - 2b) > > > > Seems gcc is smarter here than clang. > > Yeah, the assembler from clang is quite a bit behind on s390x ... in the > past I was only able to compile the k-u-t with Clang when using the > "-no-integrated-as" option ... but at least in the most recent version it > seems to have caught up now enough to be very close to compile it with the > built-in assembler, so it would be great to get this problem here fixed > somehow, too... Bringing up another option: Can we maybe guard this section from Clang so we still have the assertion when compiling with GCC?
On Tue, 2023-04-04 at 19:12 +0200, Nico Boehr wrote: > Quoting Thomas Huth (2023-04-04 17:05:02) > [...] > > > > FWIW, this is failing with Clang 15 for me: > > > > > > > > s390x/ex.c:81:4: error: expected absolute expression > > > > " .if (1b - 0b) != (3b - 2b)\n" > > > > ^ > > > > <inline asm>:12:6: note: instantiated into assembly here > > > > .if (1b - 0b) != (3b - 2b) > > > > > > Seems gcc is smarter here than clang. > > > > Yeah, the assembler from clang is quite a bit behind on s390x ... in the > > past I was only able to compile the k-u-t with Clang when using the > > "-no-integrated-as" option ... but at least in the most recent version it > > seems to have caught up now enough to be very close to compile it with the > > built-in assembler, so it would be great to get this problem here fixed > > somehow, too... > > Bringing up another option: Can we maybe guard this section from Clang so we still have the assertion when compiling with GCC? I considered this, but only from the asm, where I don't think it's possible. But putting #ifndef __clang__ around it works. Until you compile with gcc and assemble with clang. Not something we need to care about IMO.
On 04/04/2023 20.06, Nina Schoetterl-Glausch wrote: > On Tue, 2023-04-04 at 19:12 +0200, Nico Boehr wrote: >> Quoting Thomas Huth (2023-04-04 17:05:02) >> [...] >>>>> FWIW, this is failing with Clang 15 for me: >>>>> >>>>> s390x/ex.c:81:4: error: expected absolute expression >>>>> " .if (1b - 0b) != (3b - 2b)\n" >>>>> ^ >>>>> <inline asm>:12:6: note: instantiated into assembly here >>>>> .if (1b - 0b) != (3b - 2b) >>>> >>>> Seems gcc is smarter here than clang. >>> >>> Yeah, the assembler from clang is quite a bit behind on s390x ... in the >>> past I was only able to compile the k-u-t with Clang when using the >>> "-no-integrated-as" option ... but at least in the most recent version it >>> seems to have caught up now enough to be very close to compile it with the >>> built-in assembler, so it would be great to get this problem here fixed >>> somehow, too... >> >> Bringing up another option: Can we maybe guard this section from Clang so we still have the assertion when compiling with GCC? > > I considered this, but only from the asm, where I don't think it's possible. > But putting #ifndef __clang__ around it works. Until you compile with gcc and assemble with clang. > Not something we need to care about IMO. Right. So if the #ifndef works, let's go with that! Nico, could you fix it up in the pull request? Thanks, Thomas
diff --git a/s390x/Makefile b/s390x/Makefile index ab146eb..a80db53 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 0000000..dbd8030 --- /dev/null +++ b/s390x/ex.c @@ -0,0 +1,188 @@ +// 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> + +/* + * Accesses to the operand of execute-type instructions are instruction fetches. + * Minimum alignment is two, since the relative offset is specified by number of halfwords. + */ +asm ( ".pushsection .text.exrl_targets,\"x\"\n" +" .balign 2\n" +" .popsection\n" +); + +/* + * 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 .text.exrl_targets\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.exrl_targets\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" + " .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 .text.exrl_targets\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 .text.exrl_targets\n" + " .balign 4\n" + //operand of llgfrl must be word aligned + "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 .text.exrl_targets\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 d97eb5e..b61faf0 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 ad7949c..a999f64 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: