s390x: smp: Improve smp code part 2

All CRs are set to 0 and CRs 0 and 14 are set to pre-defined values, so we also need to test 1-13 and 15 for 0.

And while we're at it, let's also set some values to cr 1, 7 and 13, so we can actually be sure that they will be zeroed.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

Reviewed-by Christian Borntraeger <[email protected]>

---
 s390x/smp.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

s390x/smp.c +3 -0

index fa40753..8c9b98a 100644
@@ -182,16 +182,28 @@ static void test_emcall(void)
 	report_prefix_pop();
 }
 
+/* Used to dirty registers of cpu #1 before it is reset */
+static void test_func_initial(void)
+{

s390x/smp.c +3 -1

 	struct psw psw;
 
 	psw.mask = extract_psw_mask();
-	psw.addr = (unsigned long)test_func;
+	psw.addr = (unsigned long)test_func_initial;
 
 	report_prefix_push("reset initial");
+	set_flag(0);
 	smp_cpu_start(1, psw);
+	wait_for_flag();
 
 	sigp_retry(1, SIGP_INITIAL_CPU_RESET, 0, NULL);
 	sigp(1, SIGP_STORE_STATUS_AT_ADDRESS, (uintptr_t)status, NULL);

All CRs are set to 0 and CRs 0 and 14 are set to pre-defined values, so we also need to test 1-13 and 15 for 0.

And while we're at it, let's also set some values to cr 1, 7 and 13, so we can actually be sure that they will be zeroed.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

Reviewed-by Christian Borntraeger <[email protected]>

---
 s390x/smp.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

s390x/smp.c +13 -1

index fa40753..7144c9b 100644
@@ -182,16 +182,28 @@ static void test_emcall(void)
 	report_prefix_pop();
 }
 
+/* Used to dirty registers of cpu #1 before it is reset */
+static void test_func_initial(void)
+{
+	lctlg(1, 0x42000UL);
+	lctlg(7, 0x43000UL);
+	lctlg(13, 0x44000UL);
+	set_flag(1);
+}
+
 static void test_reset_initial(void)
 {
 	struct cpu_status *status = alloc_pages(0);
 	struct psw psw;
+	int i;
 
 	psw.mask = extract_psw_mask();
-	psw.addr = (unsigned long)test_func;
+	psw.addr = (unsigned long)test_func_initial;
 
 	report_prefix_push("reset initial");
+	set_flag(0);
 	smp_cpu_start(1, psw);
+	wait_for_flag();
 
 	sigp_retry(1, SIGP_INITIAL_CPU_RESET, 0, NULL);
 	sigp(1, SIGP_STORE_STATUS_AT_ADDRESS, (uintptr_t)status, NULL);

Let's also test the stop portion of the "stop and store status" sigp order.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 s390x/smp.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

s390x/smp.c +14 -0

index 95df8c4..8a6cd1d 100644
@@ -76,12 +76,26 @@ static void test_stop_store_status(void)
 	struct lowcore *lc = (void *)0x0;
 
 	report_prefix_push("stop store status");
+	report_prefix_push("running");
+	smp_cpu_restart(1);
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");
 	report(lc->grs_sa[15], "stack");
+	report(smp_cpu_stopped(1), "cpu stopped");
+	report_prefix_pop();
+
+	report_prefix_push("stopped");
+	lc->prefix_sa = 0;
+	lc->grs_sa[15] = 0;
+	smp_cpu_stop_store_status(1);
+	mb();
+	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");
+	report(lc->grs_sa[15], "stack");
+	report_prefix_pop();
+
 	report_prefix_pop();
 }
 

Local interrupts (external and emergency call) should be cleared after any cpu reset.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 s390x/smp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

s390x/smp.c +3 -0

index 8a6cd1d..a8e3dd7 100644
@@ -243,6 +243,20 @@ static void test_reset_initial(void)
 	report_prefix_pop();
 }
 
+static void test_local_ints(void)
+{
+	unsigned long mask;

s390x/smp.c +9 -0

+
 static void test_reset(void)
 {
 	struct psw psw;

@@ -251,10 +265,18 @@ static void test_reset(void)
 	psw.addr = (unsigned long)test_func;
 
 	report_prefix_push("cpu reset");
+	sigp(1, SIGP_EMERGENCY_SIGNAL, 0, NULL);
+	sigp(1, SIGP_EXTERNAL_CALL, 0, NULL);
 	smp_cpu_start(1, psw);
 
 	sigp_retry(1, SIGP_CPU_RESET, 0, NULL);
 	report(smp_cpu_stopped(1), "cpu stopped");
+
+	set_flag(0);
+	psw.addr = (unsigned long)test_local_ints;
+	smp_cpu_start(1, psw);
+	wait_for_flag();
+	report(true, "local interrupts cleared");
 	report_prefix_pop();
 }
 

Quoted 17 lines · Janosch Frank

Local interrupts (external and emergency call) should be cleared after any cpu reset.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 s390x/smp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

s390x/smp.c +0 -0

index 8a6cd1d..a8e3dd7 100644
@@ -243,6 +243,20 @@ static void test_reset_initial(void)
 	report_prefix_pop();
 }
 

s390x/smp.c static void test_reset_initial(void) · line 258

Quoted 13 lines

s390x/smp.c +13 -0

+static void test_local_ints(void)
+{
+	unsigned long mask;
+
+	expect_ext_int();
+	/* Open masks for ecall and emcall */
+	ctl_set_bit(0, 13);
+	ctl_set_bit(0, 14);
+	mask = extract_psw_mask();
+	mask |= PSW_MASK_EXT;
+	load_psw_mask(mask);
+	set_flag(1);
+}

I think last time I looked at this I got it all wrong. So, we actually don't expect that an interrupt triggers here, correct?

The SIGP_CPU_RESET should have cleared both interrupts on this cpu. So once we enable them, none should trigger.

Why do we have "expect_ext_int()" ?

Quoted 25 lines

s390x/smp.c +9 -0

+
 static void test_reset(void)
 {
 	struct psw psw;

@@ -251,10 +265,18 @@ static void test_reset(void)
 	psw.addr = (unsigned long)test_func;
 
 	report_prefix_push("cpu reset");
+	sigp(1, SIGP_EMERGENCY_SIGNAL, 0, NULL);
+	sigp(1, SIGP_EXTERNAL_CALL, 0, NULL);
 	smp_cpu_start(1, psw);
 
 	sigp_retry(1, SIGP_CPU_RESET, 0, NULL);
 	report(smp_cpu_stopped(1), "cpu stopped");
+
+	set_flag(0);
+	psw.addr = (unsigned long)test_local_ints;
+	smp_cpu_start(1, psw);
+	wait_for_flag();
+	report(true, "local interrupts cleared");
 	report_prefix_pop();
 }
 

Up to now a secondary cpu could have returned from the function it was executing and ending up somewhere in cstart64.S. This was mostly circumvented by an endless loop in the function that it executed.

Let's add a loop to the end of the cpu setup, so we don't have to rely on added loops in the tests.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 s390x/cstart64.S | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

s390x/cstart64.S +0 -0

index 9af6bb3..ecffbe0 100644

Up to now we ignored the psw mask and only used the psw address when bringing up a new cpu. For DAT we need to also load the mask, so let's do that.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c  | 2 ++
 s390x/cstart64.S | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

lib/s390x/smp.c +0 -0

index 3f86243..6ef0335 100644

Quoted 18 lines · Janosch Frank

Up to now we ignored the psw mask and only used the psw address when bringing up a new cpu. For DAT we need to also load the mask, so let's do that.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c  | 2 ++
 s390x/cstart64.S | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

lib/s390x/smp.c +0 -0

index 3f86243..6ef0335 100644
@@ -202,6 +202,8 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
 	cpu->stack = (uint64_t *)alloc_pages(2);
 

lib/s390x/smp.c int smp_cpu_setup(uint16_t addr, struct psw psw) · line 207

Quoted 4 lines

lib/s390x/smp.c +2 -0

 	/* Start without DAT and any other mask bits. */
+	cpu->lowcore->sw_int_psw.mask = psw.mask;
+	cpu->lowcore->sw_int_psw.addr = psw.addr;
 	cpu->lowcore->sw_int_grs[14] = psw.addr;

Do we still have to set sw_int_grs[14] ?

Quoted 19 lines · David Hildenbrand

Quoted 18 lines · Janosch Frank

Up to now we ignored the psw mask and only used the psw address when bringing up a new cpu. For DAT we need to also load the mask, so let's do that.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c  | 2 ++
 s390x/cstart64.S | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

lib/s390x/smp.c +0 -0

index 3f86243..6ef0335 100644
@@ -202,6 +202,8 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
 	cpu->stack = (uint64_t *)alloc_pages(2);
 

Quoted 7 lines

lib/s390x/smp.c int smp_cpu_setup(uint16_t addr, struct psw psw) · line 207

Quoted 4 lines

lib/s390x/smp.c +2 -0

 	/* Start without DAT and any other mask bits. */
+	cpu->lowcore->sw_int_psw.mask = psw.mask;
+	cpu->lowcore->sw_int_psw.addr = psw.addr;
 	cpu->lowcore->sw_int_grs[14] = psw.addr;

Do we still have to set sw_int_grs[14] ?

r14 is saved to restart_new so we don't go through setup twice. We could instead copy cpu->lowcore->sw_int_psw.addr to restart new, but that means more changes than the removal of one line.

Also, what about backtraces or plain old debug?
Having r14 is a good backup to have IMHO.

Quoted 31 lines · Janosch Frank

Quoted 19 lines · David Hildenbrand

Quoted 18 lines · Janosch Frank

Up to now we ignored the psw mask and only used the psw address when bringing up a new cpu. For DAT we need to also load the mask, so let's do that.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c  | 2 ++
 s390x/cstart64.S | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

lib/s390x/smp.c +0 -0

index 3f86243..6ef0335 100644
@@ -202,6 +202,8 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
 	cpu->stack = (uint64_t *)alloc_pages(2);
 

Quoted 7 lines

lib/s390x/smp.c int smp_cpu_setup(uint16_t addr, struct psw psw) · line 207

Quoted 4 lines

lib/s390x/smp.c +2 -0

 	/* Start without DAT and any other mask bits. */
+	cpu->lowcore->sw_int_psw.mask = psw.mask;
+	cpu->lowcore->sw_int_psw.addr = psw.addr;
 	cpu->lowcore->sw_int_grs[14] = psw.addr;

Do we still have to set sw_int_grs[14] ?

r14 is saved to restart_new so we don't go through setup twice. We could instead copy cpu->lowcore->sw_int_psw.addr to restart new, but that means more changes than the removal of one line.

Quoted 3 lines

Also, what about backtraces or plain old debug?
Having r14 is a good backup to have IMHO.

Fine with me

Reviewed-by David Hildenbrand <[email protected]>

Sigp orders are not necessarily finished when the processor finished the sigp instruction. We need to poll if the order has been finished before we continue.

For (re)start and stop we already use sigp sense running and sigp sense loops. But we still lack completion checks for stop and store status, as well as the cpu resets.

Let's add them.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c | 8 ++++++++
 lib/s390x/smp.h | 1 +
 s390x/smp.c     | 4 ++++
 3 files changed, 13 insertions(+)

lib/s390x/smp.c +8 -0

index 6ef0335..2555bf4 100644
@@ -154,6 +154,14 @@ int smp_cpu_start(uint16_t addr, struct psw psw)
 	return rc;
 }
 
+void smp_cpu_wait_for_completion(uint16_t addr)
+{
+	uint32_t status;
+
+	/* Loops when cc == 2, i.e. when the cpu is busy with a sigp order */
+	sigp_retry(1, SIGP_SENSE, 0, &status);
+}
+
 int smp_cpu_destroy(uint16_t addr)
 {
 	struct cpu *cpu;

lib/s390x/smp.h +1 -0

index ce63a89..a8b98c0 100644
@@ -45,6 +45,7 @@ int smp_cpu_restart(uint16_t addr);
 int smp_cpu_start(uint16_t addr, struct psw psw);
 int smp_cpu_stop(uint16_t addr);
 int smp_cpu_stop_store_status(uint16_t addr);
+void smp_cpu_wait_for_completion(uint16_t addr);
 int smp_cpu_destroy(uint16_t addr);
 int smp_cpu_setup(uint16_t addr, struct psw psw);
 void smp_teardown(void);

s390x/smp.c +1 -0

index 7462211..48321f4 100644
@@ -75,6 +75,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");
 	report(lc->grs_sa[15], "stack");

Quoted 60 lines · Janosch Frank

Sigp orders are not necessarily finished when the processor finished the sigp instruction. We need to poll if the order has been finished before we continue.

For (re)start and stop we already use sigp sense running and sigp sense loops. But we still lack completion checks for stop and store status, as well as the cpu resets.

Let's add them.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c | 8 ++++++++
 lib/s390x/smp.h | 1 +
 s390x/smp.c     | 4 ++++
 3 files changed, 13 insertions(+)

lib/s390x/smp.c +8 -0

index 6ef0335..2555bf4 100644
@@ -154,6 +154,14 @@ int smp_cpu_start(uint16_t addr, struct psw psw)
 	return rc;
 }
 
+void smp_cpu_wait_for_completion(uint16_t addr)
+{
+	uint32_t status;
+
+	/* Loops when cc == 2, i.e. when the cpu is busy with a sigp order */
+	sigp_retry(1, SIGP_SENSE, 0, &status);
+}
+
 int smp_cpu_destroy(uint16_t addr)
 {
 	struct cpu *cpu;

lib/s390x/smp.h +1 -0

index ce63a89..a8b98c0 100644
@@ -45,6 +45,7 @@ int smp_cpu_restart(uint16_t addr);
 int smp_cpu_start(uint16_t addr, struct psw psw);
 int smp_cpu_stop(uint16_t addr);
 int smp_cpu_stop_store_status(uint16_t addr);
+void smp_cpu_wait_for_completion(uint16_t addr);
 int smp_cpu_destroy(uint16_t addr);
 int smp_cpu_setup(uint16_t addr, struct psw psw);
 void smp_teardown(void);

s390x/smp.c +1 -0

index 7462211..48321f4 100644
@@ -75,6 +75,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

s390x/smp.c static void test_stop_store_status(void) · line 88

Quoted 5 lines

s390x/smp.c +0 -0

 	report(lc->grs_sa[15], "stack");

@@ -85,6 +86,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);

Just curious: Would it make sense to add that inside smp_cpu_stop_store_status() instead?

Quoted 61 lines · David Hildenbrand

Quoted 60 lines · Janosch Frank

Sigp orders are not necessarily finished when the processor finished the sigp instruction. We need to poll if the order has been finished before we continue.

For (re)start and stop we already use sigp sense running and sigp sense loops. But we still lack completion checks for stop and store status, as well as the cpu resets.

Let's add them.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c | 8 ++++++++
 lib/s390x/smp.h | 1 +
 s390x/smp.c     | 4 ++++
 3 files changed, 13 insertions(+)

lib/s390x/smp.c +8 -0

index 6ef0335..2555bf4 100644
@@ -154,6 +154,14 @@ int smp_cpu_start(uint16_t addr, struct psw psw)
 	return rc;
 }
 
+void smp_cpu_wait_for_completion(uint16_t addr)
+{
+	uint32_t status;
+
+	/* Loops when cc == 2, i.e. when the cpu is busy with a sigp order */
+	sigp_retry(1, SIGP_SENSE, 0, &status);
+}
+
 int smp_cpu_destroy(uint16_t addr)
 {
 	struct cpu *cpu;

lib/s390x/smp.h +1 -0

index ce63a89..a8b98c0 100644
@@ -45,6 +45,7 @@ int smp_cpu_restart(uint16_t addr);
 int smp_cpu_start(uint16_t addr, struct psw psw);
 int smp_cpu_stop(uint16_t addr);
 int smp_cpu_stop_store_status(uint16_t addr);
+void smp_cpu_wait_for_completion(uint16_t addr);
 int smp_cpu_destroy(uint16_t addr);
 int smp_cpu_setup(uint16_t addr, struct psw psw);
 void smp_teardown(void);

s390x/smp.c +1 -0

index 7462211..48321f4 100644
@@ -75,6 +75,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

Quoted 9 lines

s390x/smp.c static void test_stop_store_status(void) · line 88

Quoted 5 lines

s390x/smp.c +0 -0

 	report(lc->grs_sa[15], "stack");

@@ -85,6 +86,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);

Just curious: Would it make sense to add that inside smp_cpu_stop_store_status() instead?

I think so, we also wait for stop and start to finish, so why not for this order code.

Quoted 72 lines · Janosch Frank

Quoted 61 lines · David Hildenbrand

Quoted 60 lines · Janosch Frank

Sigp orders are not necessarily finished when the processor finished the sigp instruction. We need to poll if the order has been finished before we continue.

For (re)start and stop we already use sigp sense running and sigp sense loops. But we still lack completion checks for stop and store status, as well as the cpu resets.

Let's add them.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c | 8 ++++++++
 lib/s390x/smp.h | 1 +
 s390x/smp.c     | 4 ++++
 3 files changed, 13 insertions(+)

lib/s390x/smp.c +8 -0

index 6ef0335..2555bf4 100644
@@ -154,6 +154,14 @@ int smp_cpu_start(uint16_t addr, struct psw psw)
 	return rc;
 }
 
+void smp_cpu_wait_for_completion(uint16_t addr)
+{
+	uint32_t status;
+
+	/* Loops when cc == 2, i.e. when the cpu is busy with a sigp order */
+	sigp_retry(1, SIGP_SENSE, 0, &status);
+}
+
 int smp_cpu_destroy(uint16_t addr)
 {
 	struct cpu *cpu;

lib/s390x/smp.h +1 -0

index ce63a89..a8b98c0 100644
@@ -45,6 +45,7 @@ int smp_cpu_restart(uint16_t addr);
 int smp_cpu_start(uint16_t addr, struct psw psw);
 int smp_cpu_stop(uint16_t addr);
 int smp_cpu_stop_store_status(uint16_t addr);
+void smp_cpu_wait_for_completion(uint16_t addr);
 int smp_cpu_destroy(uint16_t addr);
 int smp_cpu_setup(uint16_t addr, struct psw psw);
 void smp_teardown(void);

s390x/smp.c +1 -0

index 7462211..48321f4 100644
@@ -75,6 +75,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

Quoted 9 lines

s390x/smp.c static void test_stop_store_status(void) · line 88

Quoted 5 lines

s390x/smp.c +0 -0

 	report(lc->grs_sa[15], "stack");

@@ -85,6 +86,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);

Just curious: Would it make sense to add that inside smp_cpu_stop_store_status() instead?

Quoted 3 lines

I think so, we also wait for stop and start to finish, so why not for this order code.

I've moved the waiting into the smp library and now the prefix check for stop and store status fails every so often if executed repeatedly.

I've tried making the lc ptr volatile, a print of the prefix before the report seems to fix the issue, a print after the report still shows the issue but according to the print both values are the same.

I'm currently at a loss...

Quoted 80 lines · Janosch Frank

Quoted 72 lines · Janosch Frank

Quoted 61 lines · David Hildenbrand

Quoted 60 lines · Janosch Frank

Sigp orders are not necessarily finished when the processor finished the sigp instruction. We need to poll if the order has been finished before we continue.

For (re)start and stop we already use sigp sense running and sigp sense loops. But we still lack completion checks for stop and store status, as well as the cpu resets.

Let's add them.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c | 8 ++++++++
 lib/s390x/smp.h | 1 +
 s390x/smp.c     | 4 ++++
 3 files changed, 13 insertions(+)

lib/s390x/smp.c +8 -0

index 6ef0335..2555bf4 100644
@@ -154,6 +154,14 @@ int smp_cpu_start(uint16_t addr, struct psw psw)
 	return rc;
 }
 
+void smp_cpu_wait_for_completion(uint16_t addr)
+{
+	uint32_t status;
+
+	/* Loops when cc == 2, i.e. when the cpu is busy with a sigp order */
+	sigp_retry(1, SIGP_SENSE, 0, &status);
+}
+
 int smp_cpu_destroy(uint16_t addr)
 {
 	struct cpu *cpu;

lib/s390x/smp.h +1 -0

index ce63a89..a8b98c0 100644
@@ -45,6 +45,7 @@ int smp_cpu_restart(uint16_t addr);
 int smp_cpu_start(uint16_t addr, struct psw psw);
 int smp_cpu_stop(uint16_t addr);
 int smp_cpu_stop_store_status(uint16_t addr);
+void smp_cpu_wait_for_completion(uint16_t addr);
 int smp_cpu_destroy(uint16_t addr);
 int smp_cpu_setup(uint16_t addr, struct psw psw);
 void smp_teardown(void);

s390x/smp.c +1 -0

index 7462211..48321f4 100644
@@ -75,6 +75,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

Quoted 9 lines

s390x/smp.c static void test_stop_store_status(void) · line 88

Quoted 5 lines

s390x/smp.c +0 -0

 	report(lc->grs_sa[15], "stack");

@@ -85,6 +86,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);

Just curious: Would it make sense to add that inside smp_cpu_stop_store_status() instead?

Quoted 3 lines

I think so, we also wait for stop and start to finish, so why not for this order code.

I've moved the waiting into the smp library and now the prefix check for stop and store status fails every so often if executed repeatedly.

Quoted 5 lines

I've tried making the lc ptr volatile, a print of the prefix before the report seems to fix the issue, a print after the report still shows the issue but according to the print both values are the same.

I'm currently at a loss...

Are you missing a barrier() somewhere?

Quoted 81 lines · David Hildenbrand

Quoted 80 lines · Janosch Frank

Quoted 72 lines · Janosch Frank

Quoted 61 lines · David Hildenbrand

Quoted 60 lines · Janosch Frank

Sigp orders are not necessarily finished when the processor finished the sigp instruction. We need to poll if the order has been finished before we continue.

For (re)start and stop we already use sigp sense running and sigp sense loops. But we still lack completion checks for stop and store status, as well as the cpu resets.

Let's add them.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c | 8 ++++++++
 lib/s390x/smp.h | 1 +
 s390x/smp.c     | 4 ++++
 3 files changed, 13 insertions(+)

lib/s390x/smp.c +8 -0

index 6ef0335..2555bf4 100644
@@ -154,6 +154,14 @@ int smp_cpu_start(uint16_t addr, struct psw psw)
 	return rc;
 }
 
+void smp_cpu_wait_for_completion(uint16_t addr)
+{
+	uint32_t status;
+
+	/* Loops when cc == 2, i.e. when the cpu is busy with a sigp order */
+	sigp_retry(1, SIGP_SENSE, 0, &status);
+}
+
 int smp_cpu_destroy(uint16_t addr)
 {
 	struct cpu *cpu;

lib/s390x/smp.h +1 -0

index ce63a89..a8b98c0 100644
@@ -45,6 +45,7 @@ int smp_cpu_restart(uint16_t addr);
 int smp_cpu_start(uint16_t addr, struct psw psw);
 int smp_cpu_stop(uint16_t addr);
 int smp_cpu_stop_store_status(uint16_t addr);
+void smp_cpu_wait_for_completion(uint16_t addr);
 int smp_cpu_destroy(uint16_t addr);
 int smp_cpu_setup(uint16_t addr, struct psw psw);
 void smp_teardown(void);

s390x/smp.c +1 -0

index 7462211..48321f4 100644
@@ -75,6 +75,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

Quoted 9 lines

s390x/smp.c static void test_stop_store_status(void) · line 88

Quoted 5 lines

s390x/smp.c +0 -0

 	report(lc->grs_sa[15], "stack");

@@ -85,6 +86,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);

Just curious: Would it make sense to add that inside smp_cpu_stop_store_status() instead?

Quoted 3 lines

I think so, we also wait for stop and start to finish, so why not for this order code.

I've moved the waiting into the smp library and now the prefix check for stop and store status fails every so often if executed repeatedly.

Quoted 8 lines

Quoted 5 lines

I've tried making the lc ptr volatile, a print of the prefix before the report seems to fix the issue, a print after the report still shows the issue but according to the print both values are the same.

I'm currently at a loss...

Are you missing a barrier() somewhere?

Maybe, but the question is where?

There's already one before the report:
smp_cpu_stop_store_status(1);
mb();
report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

Quoted 91 lines · Janosch Frank

Quoted 81 lines · David Hildenbrand

Quoted 80 lines · Janosch Frank

Quoted 72 lines · Janosch Frank

Quoted 61 lines · David Hildenbrand

Quoted 60 lines · Janosch Frank

Sigp orders are not necessarily finished when the processor finished the sigp instruction. We need to poll if the order has been finished before we continue.

For (re)start and stop we already use sigp sense running and sigp sense loops. But we still lack completion checks for stop and store status, as well as the cpu resets.

Let's add them.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c | 8 ++++++++
 lib/s390x/smp.h | 1 +
 s390x/smp.c     | 4 ++++
 3 files changed, 13 insertions(+)

lib/s390x/smp.c +8 -0

index 6ef0335..2555bf4 100644
@@ -154,6 +154,14 @@ int smp_cpu_start(uint16_t addr, struct psw psw)
 	return rc;
 }
 
+void smp_cpu_wait_for_completion(uint16_t addr)
+{
+	uint32_t status;
+
+	/* Loops when cc == 2, i.e. when the cpu is busy with a sigp order */
+	sigp_retry(1, SIGP_SENSE, 0, &status);
+}
+
 int smp_cpu_destroy(uint16_t addr)
 {
 	struct cpu *cpu;

lib/s390x/smp.h +1 -0

index ce63a89..a8b98c0 100644
@@ -45,6 +45,7 @@ int smp_cpu_restart(uint16_t addr);
 int smp_cpu_start(uint16_t addr, struct psw psw);
 int smp_cpu_stop(uint16_t addr);
 int smp_cpu_stop_store_status(uint16_t addr);
+void smp_cpu_wait_for_completion(uint16_t addr);
 int smp_cpu_destroy(uint16_t addr);
 int smp_cpu_setup(uint16_t addr, struct psw psw);
 void smp_teardown(void);

s390x/smp.c +1 -0

index 7462211..48321f4 100644
@@ -75,6 +75,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

Quoted 9 lines

s390x/smp.c static void test_stop_store_status(void) · line 88

Quoted 5 lines

s390x/smp.c +0 -0

 	report(lc->grs_sa[15], "stack");

@@ -85,6 +86,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);

Just curious: Would it make sense to add that inside smp_cpu_stop_store_status() instead?

Quoted 3 lines

I think so, we also wait for stop and start to finish, so why not for this order code.

I've moved the waiting into the smp library and now the prefix check for stop and store status fails every so often if executed repeatedly.

Quoted 8 lines

Quoted 5 lines

I've tried making the lc ptr volatile, a print of the prefix before the report seems to fix the issue, a print after the report still shows the issue but according to the print both values are the same.

I'm currently at a loss...

Are you missing a barrier() somewhere?

Quoted 6 lines

Maybe, but the question is where?

There's already one before the report:
smp_cpu_stop_store_status(1);
mb();
report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

The issue here is:

SIGP_SENSE is always handled in the kernel for KVM. Meaning, it will complete even before the target CPU executed the stop and store (in QEMU).

Reading the PoP:

"One of the following conditions exists at the
addressed CPU: ... A previously issued stop-
and-store-status ... has been accepted by the
addressed CPU, and execution of the func-
tion requested by the order has not yet been
completed.

"If the currently specified order is sense ... then the order is rejected, and condition code 2 is set."

So, in case of KVM, SENSE does not wait for completion of the previous order. I remember that was a performance improvements, because we wanted to avoid going to user space just to sense if another CPU is running. (and I remember that the documentation was inconsistent)

Let me guess, under TCG it works all the time?

Quoted 116 lines · David Hildenbrand

Quoted 91 lines · Janosch Frank

Quoted 81 lines · David Hildenbrand

Quoted 80 lines · Janosch Frank

Quoted 72 lines · Janosch Frank

Quoted 61 lines · David Hildenbrand

Quoted 60 lines · Janosch Frank

Sigp orders are not necessarily finished when the processor finished the sigp instruction. We need to poll if the order has been finished before we continue.

For (re)start and stop we already use sigp sense running and sigp sense loops. But we still lack completion checks for stop and store status, as well as the cpu resets.

Let's add them.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c | 8 ++++++++
 lib/s390x/smp.h | 1 +
 s390x/smp.c     | 4 ++++
 3 files changed, 13 insertions(+)

lib/s390x/smp.c +8 -0

index 6ef0335..2555bf4 100644
@@ -154,6 +154,14 @@ int smp_cpu_start(uint16_t addr, struct psw psw)
 	return rc;
 }
 
+void smp_cpu_wait_for_completion(uint16_t addr)
+{
+	uint32_t status;
+
+	/* Loops when cc == 2, i.e. when the cpu is busy with a sigp order */
+	sigp_retry(1, SIGP_SENSE, 0, &status);
+}
+
 int smp_cpu_destroy(uint16_t addr)
 {
 	struct cpu *cpu;

lib/s390x/smp.h +1 -0

index ce63a89..a8b98c0 100644
@@ -45,6 +45,7 @@ int smp_cpu_restart(uint16_t addr);
 int smp_cpu_start(uint16_t addr, struct psw psw);
 int smp_cpu_stop(uint16_t addr);
 int smp_cpu_stop_store_status(uint16_t addr);
+void smp_cpu_wait_for_completion(uint16_t addr);
 int smp_cpu_destroy(uint16_t addr);
 int smp_cpu_setup(uint16_t addr, struct psw psw);
 void smp_teardown(void);

s390x/smp.c +1 -0

index 7462211..48321f4 100644
@@ -75,6 +75,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

Quoted 9 lines

s390x/smp.c static void test_stop_store_status(void) · line 88

Quoted 5 lines

s390x/smp.c +0 -0

 	report(lc->grs_sa[15], "stack");

@@ -85,6 +86,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);

Just curious: Would it make sense to add that inside smp_cpu_stop_store_status() instead?

Quoted 3 lines

I think so, we also wait for stop and start to finish, so why not for this order code.

I've moved the waiting into the smp library and now the prefix check for stop and store status fails every so often if executed repeatedly.

Quoted 8 lines

Quoted 5 lines

I've tried making the lc ptr volatile, a print of the prefix before the report seems to fix the issue, a print after the report still shows the issue but according to the print both values are the same.

I'm currently at a loss...

Are you missing a barrier() somewhere?

Quoted 6 lines

Maybe, but the question is where?

There's already one before the report:
smp_cpu_stop_store_status(1);
mb();
report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

The issue here is:

SIGP_SENSE is always handled in the kernel for KVM. Meaning, it will complete even before the target CPU executed the stop and store (in QEMU).

Reading the PoP:

"One of the following conditions exists at the
addressed CPU: ... A previously issued stop-
and-store-status ... has been accepted by the
addressed CPU, and execution of the func-
tion requested by the order has not yet been
completed.

"If the currently specified order is sense ... then the order is rejected, and condition code 2 is set."

Quoted 4 lines

So, in case of KVM, SENSE does not wait for completion of the previous order. I remember that was a performance improvements, because we wanted to avoid going to user space just to sense if another CPU is running. (and I remember that the documentation was inconsistent)

So, KVM is not architectural compliant when it comes to SIGP SENSE? I guess I need to go back to looping until the prefix is > 0

Quoted 117 lines · Janosch Frank

Quoted 116 lines · David Hildenbrand

Quoted 91 lines · Janosch Frank

Quoted 81 lines · David Hildenbrand

Quoted 80 lines · Janosch Frank

Quoted 72 lines · Janosch Frank

Quoted 61 lines · David Hildenbrand

Quoted 60 lines · Janosch Frank

Sigp orders are not necessarily finished when the processor finished the sigp instruction. We need to poll if the order has been finished before we continue.

For (re)start and stop we already use sigp sense running and sigp sense loops. But we still lack completion checks for stop and store status, as well as the cpu resets.

Let's add them.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c | 8 ++++++++
 lib/s390x/smp.h | 1 +
 s390x/smp.c     | 4 ++++
 3 files changed, 13 insertions(+)

lib/s390x/smp.c +8 -0

index 6ef0335..2555bf4 100644
@@ -154,6 +154,14 @@ int smp_cpu_start(uint16_t addr, struct psw psw)
 	return rc;
 }
 
+void smp_cpu_wait_for_completion(uint16_t addr)
+{
+	uint32_t status;
+
+	/* Loops when cc == 2, i.e. when the cpu is busy with a sigp order */
+	sigp_retry(1, SIGP_SENSE, 0, &status);
+}
+
 int smp_cpu_destroy(uint16_t addr)
 {
 	struct cpu *cpu;

lib/s390x/smp.h +1 -0

index ce63a89..a8b98c0 100644
@@ -45,6 +45,7 @@ int smp_cpu_restart(uint16_t addr);
 int smp_cpu_start(uint16_t addr, struct psw psw);
 int smp_cpu_stop(uint16_t addr);
 int smp_cpu_stop_store_status(uint16_t addr);
+void smp_cpu_wait_for_completion(uint16_t addr);
 int smp_cpu_destroy(uint16_t addr);
 int smp_cpu_setup(uint16_t addr, struct psw psw);
 void smp_teardown(void);

s390x/smp.c +1 -0

index 7462211..48321f4 100644
@@ -75,6 +75,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

Quoted 9 lines

s390x/smp.c static void test_stop_store_status(void) · line 88

Quoted 5 lines

s390x/smp.c +0 -0

 	report(lc->grs_sa[15], "stack");

@@ -85,6 +86,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);

Just curious: Would it make sense to add that inside smp_cpu_stop_store_status() instead?

Quoted 3 lines

I think so, we also wait for stop and start to finish, so why not for this order code.

I've moved the waiting into the smp library and now the prefix check for stop and store status fails every so often if executed repeatedly.

Quoted 8 lines

Quoted 5 lines

I've tried making the lc ptr volatile, a print of the prefix before the report seems to fix the issue, a print after the report still shows the issue but according to the print both values are the same.

I'm currently at a loss...

Are you missing a barrier() somewhere?

Quoted 6 lines

Maybe, but the question is where?

There's already one before the report:
smp_cpu_stop_store_status(1);
mb();
report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

The issue here is:

SIGP_SENSE is always handled in the kernel for KVM. Meaning, it will complete even before the target CPU executed the stop and store (in QEMU).

Reading the PoP:

"One of the following conditions exists at the
addressed CPU: ... A previously issued stop-
and-store-status ... has been accepted by the
addressed CPU, and execution of the func-
tion requested by the order has not yet been
completed.

"If the currently specified order is sense ... then the order is rejected, and condition code 2 is set."

Quoted 7 lines

Quoted 4 lines

So, in case of KVM, SENSE does not wait for completion of the previous order. I remember that was a performance improvements, because we wanted to avoid going to user space just to sense if another CPU is running. (and I remember that the documentation was inconsistent)

So, KVM is not architectural compliant when it comes to SIGP SENSE? I guess I need to go back to looping until the prefix is > 0

Yeah, or fix SIGP_SENSE in KVM. Would need QEMU and KVM changes. I remember that a tricky part was checking if external calls are pending for a CPU from user space.

We could pass that information along with the intercept to QEMU.

AFAIKs, SIGP SENSE is not used on a hot path in Linux.

Quoted 132 lines · David Hildenbrand

Quoted 117 lines · Janosch Frank

Quoted 116 lines · David Hildenbrand

Quoted 91 lines · Janosch Frank

Quoted 81 lines · David Hildenbrand

Quoted 80 lines · Janosch Frank

Quoted 72 lines · Janosch Frank

Quoted 61 lines · David Hildenbrand

Quoted 60 lines · Janosch Frank

Sigp orders are not necessarily finished when the processor finished the sigp instruction. We need to poll if the order has been finished before we continue.

For (re)start and stop we already use sigp sense running and sigp sense loops. But we still lack completion checks for stop and store status, as well as the cpu resets.

Let's add them.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/smp.c | 8 ++++++++
 lib/s390x/smp.h | 1 +
 s390x/smp.c     | 4 ++++
 3 files changed, 13 insertions(+)

lib/s390x/smp.c +8 -0

index 6ef0335..2555bf4 100644
@@ -154,6 +154,14 @@ int smp_cpu_start(uint16_t addr, struct psw psw)
 	return rc;
 }
 
+void smp_cpu_wait_for_completion(uint16_t addr)
+{
+	uint32_t status;
+
+	/* Loops when cc == 2, i.e. when the cpu is busy with a sigp order */
+	sigp_retry(1, SIGP_SENSE, 0, &status);
+}
+
 int smp_cpu_destroy(uint16_t addr)
 {
 	struct cpu *cpu;

lib/s390x/smp.h +1 -0

index ce63a89..a8b98c0 100644
@@ -45,6 +45,7 @@ int smp_cpu_restart(uint16_t addr);
 int smp_cpu_start(uint16_t addr, struct psw psw);
 int smp_cpu_stop(uint16_t addr);
 int smp_cpu_stop_store_status(uint16_t addr);
+void smp_cpu_wait_for_completion(uint16_t addr);
 int smp_cpu_destroy(uint16_t addr);
 int smp_cpu_setup(uint16_t addr, struct psw psw);
 void smp_teardown(void);

s390x/smp.c +1 -0

index 7462211..48321f4 100644
@@ -75,6 +75,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

Quoted 9 lines

s390x/smp.c static void test_stop_store_status(void) · line 88

Quoted 5 lines

s390x/smp.c +0 -0

 	report(lc->grs_sa[15], "stack");

@@ -85,6 +86,7 @@ static void test_stop_store_status(void)
 	lc->prefix_sa = 0;
 	lc->grs_sa[15] = 0;
 	smp_cpu_stop_store_status(1);

Just curious: Would it make sense to add that inside smp_cpu_stop_store_status() instead?

Quoted 3 lines

I think so, we also wait for stop and start to finish, so why not for this order code.

I've moved the waiting into the smp library and now the prefix check for stop and store status fails every so often if executed repeatedly.

Quoted 8 lines

Quoted 5 lines

I've tried making the lc ptr volatile, a print of the prefix before the report seems to fix the issue, a print after the report still shows the issue but according to the print both values are the same.

I'm currently at a loss...

Are you missing a barrier() somewhere?

Quoted 6 lines

Maybe, but the question is where?

There's already one before the report:
smp_cpu_stop_store_status(1);
mb();
report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");

The issue here is:

SIGP_SENSE is always handled in the kernel for KVM. Meaning, it will complete even before the target CPU executed the stop and store (in QEMU).

Reading the PoP:

"One of the following conditions exists at the
addressed CPU: ... A previously issued stop-
and-store-status ... has been accepted by the
addressed CPU, and execution of the func-
tion requested by the order has not yet been
completed.

"If the currently specified order is sense ... then the order is rejected, and condition code 2 is set."

Quoted 7 lines

Quoted 4 lines

So, in case of KVM, SENSE does not wait for completion of the previous order. I remember that was a performance improvements, because we wanted to avoid going to user space just to sense if another CPU is running. (and I remember that the documentation was inconsistent)

So, KVM is not architectural compliant when it comes to SIGP SENSE? I guess I need to go back to looping until the prefix is > 0

Yeah, or fix SIGP_SENSE in KVM. Would need QEMU and KVM changes. I remember that a tricky part was checking if external calls are pending for a CPU from user space.

We could pass that information along with the intercept to QEMU.

Quoted 2 lines

AFAIKs, SIGP SENSE is not used on a hot path in Linux.

For now I'd rather have a workaround in the test until I can find cycles to find a solution in KVM/QEMU.

SIGP SENSE has been working quite well for Linux for the last few years, so I won't start running around now frantically fixing stuff.

Let's make sure we can restart a cpu that is already running. Restarting it if it is stopped is implicitely tested by the the other restart calls in the smp test.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 s390x/smp.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

s390x/smp.c +28 -0

index 48321f4..35ca9c7 100644
@@ -52,6 +52,34 @@ static void test_start(void)
 	report(1, "start");
 }
 
+/*
+ * Does only test restart when the target is running.
+ * The other tests do restarts when stopped multiple times already.
+ */
+static void test_restart(void)
+{
+	struct cpu *cpu = smp_cpu_from_addr(1);
+	struct lowcore *lc = cpu->lowcore;
+
+	lc->restart_new_psw.mask = extract_psw_mask();
+	lc->restart_new_psw.addr = (unsigned long)test_func;
+
+	/* Make sure cpu is running */
+	smp_cpu_stop(0);
+	set_flag(0);
+	smp_cpu_restart(1);
+	wait_for_flag();
+
+	/*
+	 * Wait until cpu 1 has set the flag because it executed the
+	 * restart function.
+	 */
+	set_flag(0);
+	smp_cpu_restart(1);
+	wait_for_flag();
+	report(1, "restart while running");
+}
+
 static void test_stop(void)
 {
 	smp_cpu_stop(1);

Seems like I uppercased the whole region instead of only the ULs when I added those definitions. Let's make the x lowercase again.

Signed-off-by Janosch Frank <[email protected]>

Reviewed-by Christian Borntraeger <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

---
 lib/s390x/asm/arch_def.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

lib/s390x/asm/arch_def.h +0 -1

index 15a4d49..1b3bb0c 100644
@@ -19,10 +19,10 @@ struct psw {
 #define PSW_MASK_DAT			0x0400000000000000UL
 #define PSW_MASK_PSTATE			0x0001000000000000UL
 
-#define CR0_EXTM_SCLP			0X0000000000000200UL