On 12/17/21 12:08, Heiko Carstens wrote:
On Fri, Dec 17, 2021 at 11:31:37AM +0100, Nico Boehr wrote:
Use a more descriptive name instead of the magic number 424 (address of
restart new PSW in the lowcore).
In addition, add a comment to make it more obvious what the ASM snippet
does.
Signed-off-by: Nico Boehr <nrb@xxxxxxxxxxxxx>
---
s390x/diag288.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/s390x/diag288.c b/s390x/diag288.c
index da7b06c365bf..a2c263e38338 100644
--- a/s390x/diag288.c
+++ b/s390x/diag288.c
@@ -94,12 +94,15 @@ static void test_bite(void)
/* Arm watchdog */
lc->restart_new_psw.mask = extract_psw_mask() & ~PSW_MASK_EXT;
diag288(CODE_INIT, 15, ACTION_RESTART);
+ /* Wait for restart interruption */
asm volatile(" larl 0, 1f\n"
- " stg 0, 424\n"
+ " stg 0, %[restart_new_psw]\n"
"0: nop\n"
" j 0b\n"
"1:"
- : : : "0");
+ :
+ : [restart_new_psw] "T" (lc->restart_new_psw.addr)
Even though it was wrong and missing before: this is an output not an input
parameter. Also, older compilers might fail if only the "T" constraint is
given (see gcc commit 3e4be43f69da ("S/390: Memory constraint cleanup")).
Which means: "=RT" would be correct. To be on the safe side, and to avoid
that gcc optimizes any potential prior C code away, I'd recommend to use
"+RT" in this case.
Thanks for clearing that up, those intricate details are quite hard to
find/remember if you only write inline assembly every few months.
Also there is an ordering problem here: starting the time bomb before the
restart psw has been setup is racy. It is unlikely that this fails, but
still...
Correct would be to setup the restart psw, and then start the time
bomb. This would also allow to shorten the runtime of this test case to
1 second, instead of the 15 seconds it is running now.
While you are correct, the minimum value of the timer is 15s.
Racing that will be quite hard.
@Nico but yes, while you're at it you could switch that around so I
don't have to explain that a second time.
It was all like that before, I know. Just some comments ;)
