Skip to contentSkip to conversation

s390x: smp: Improve smp code part 2

PATCH Superseded Janosch Frank 4 participants 30 messages Mar 24, 2020 – Apr 6, 2020

Subsystems (2)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover s390x: smp: Improve smp code part 2

Let's continue cleaning up the smp test and smp related functions.

We add:
* Test for external/emergency calls after reset
* Test SIGP restart while running
* SIGP stop and store status while running
* CR testing on reset

We fix:
* Proper check for sigp completion
* smp_cpu_setup_state() loop and return address in r14

GIT: https://github.com/frankjaa/kvm-unit-tests/tree/smp_cleanup2

Janosch Frank (10):
s390x: smp: Test all CRs on initial reset
s390x: smp: Dirty fpc before initial reset test
s390x: smp: Test stop and store status on a running and stopped cpu
s390x: smp: Test local interrupts after cpu reset
s390x: smp: Loop if secondary cpu returns into cpu setup again
s390x: smp: Remove unneeded cpu loops
s390x: smp: Use full PSW to bringup new cpu
s390x: smp: Wait for sigp completion
s390x: smp: Add restart when running test
s390x: Fix library constant definitions

 lib/s390x/asm/arch_def.h |  8 ++--
 lib/s390x/smp.c          | 10 +++++
 lib/s390x/smp.h          |  1 +
 s390x/cstart64.S         |  5 ++-
 s390x/smp.c              | 84 ++++++++++++++++++++++++++++++++++++----
 5 files changed, 95 insertions(+), 13 deletions(-)
Signature · 1 line
2.25.1
#1 lore raw

PATCH 1/10 s390x: smp: Test all CRs on initial reset

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.

---
 s390x/smp.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
s390x/smp.c +15 -1
index fa40753524f321d4..8c9b98aabd9e8222 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);
+	uint64_t nullp[12] = {};
 	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);
@@ -202,6 +214,8 @@ static void test_reset_initial(void)
 	report(!status->fpc, "fpc");
 	report(!status->cputm, "cpu timer");
 	report(!status->todpr, "todpr");
+	report(!memcmp(&status->crs[1], nullp, sizeof(status->crs[1]) * 12), "cr1-13 == 0");
+	report(status->crs[15] == 0, "cr15 == 0");
 	report_prefix_pop();
 
 	report_prefix_push("initialized");
Signature · 1 line
2.25.1
#2 lore raw

Re: [kvm-unit-tests PATCH 01/10] s390x: smp: Test all CRs on initial reset

Quoted 47 lines · Janosch Frank

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.

---
 s390x/smp.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
s390x/smp.c +13 -1
index fa40753524f321d4..8c9b98aabd9e8222 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);
+	uint64_t nullp[12] = {};
 	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);

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

Quoted 6 lines
s390x/smp.c +2 -0
@@ -202,6 +214,8 @@ static void test_reset_initial(void)
 	report(!status->fpc, "fpc");
 	report(!status->cputm, "cpu timer");
 	report(!status->todpr, "todpr");
+	report(!memcmp(&status->crs[1], nullp, sizeof(status->crs[1]) * 12), "cr1-13 == 0");
+	report(status->crs[15] == 0, "cr15 == 0");
 	report_prefix_pop();

Why not add a check for crs[0] == 0xe0
and crs[14] = 0xc2000000
?

#3 lore raw

Re: [kvm-unit-tests PATCH 01/10] s390x: smp: Test all CRs on initial reset

Quoted 47 lines · Christian Borntraeger
Quoted 44 lines · Janosch Frank

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.

---
 s390x/smp.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
s390x/smp.c +13 -1
index fa40753524f321d4..8c9b98aabd9e8222 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);
+	uint64_t nullp[12] = {};
 	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();
 
Quoted 12 lines

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

Quoted 9 lines
s390x/smp.c +2 -0
 	sigp_retry(1, SIGP_INITIAL_CPU_RESET, 0, NULL);
 	sigp(1, SIGP_STORE_STATUS_AT_ADDRESS, (uintptr_t)status, NULL);
@@ -202,6 +214,8 @@ static void test_reset_initial(void)
 	report(!status->fpc, "fpc");
 	report(!status->cputm, "cpu timer");
 	report(!status->todpr, "todpr");
+	report(!memcmp(&status->crs[1], nullp, sizeof(status->crs[1]) * 12), "cr1-13 == 0");
+	report(status->crs[15] == 0, "cr15 == 0");
 	report_prefix_pop();

Why not add a check for crs[0] == 0xe0
and crs[14] = 0xc2000000

Of course this can be a future patch. This patch itself looks good.

#4 lore raw

Re: [kvm-unit-tests PATCH 01/10] s390x: smp: Test all CRs on initial reset

In reply to Christian Borntraeger
Quoted 47 lines · Christian Borntraeger
Quoted 44 lines · Janosch Frank

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.

---
 s390x/smp.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
s390x/smp.c +13 -1
index fa40753524f321d4..8c9b98aabd9e8222 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);
+	uint64_t nullp[12] = {};
 	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();
 
Quoted 12 lines

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

Quoted 9 lines
s390x/smp.c +2 -0
 	sigp_retry(1, SIGP_INITIAL_CPU_RESET, 0, NULL);
 	sigp(1, SIGP_STORE_STATUS_AT_ADDRESS, (uintptr_t)status, NULL);
@@ -202,6 +214,8 @@ static void test_reset_initial(void)
 	report(!status->fpc, "fpc");
 	report(!status->cputm, "cpu timer");
 	report(!status->todpr, "todpr");
+	report(!memcmp(&status->crs[1], nullp, sizeof(status->crs[1]) * 12), "cr1-13 == 0");
+	report(status->crs[15] == 0, "cr15 == 0");
 	report_prefix_pop();

Why not add a check for crs[0] == 0xe0
and crs[14] = 0xc2000000

You mean the checks which are done a few lines below this? This patch just actually dirties registers which should be set to 0 so we can really be sure that they have been touched.

#5 lore raw

Re: [kvm-unit-tests PATCH 01/10] s390x: smp: Test all CRs on initial reset

Quoted 61 lines · Janosch Frank
Quoted 47 lines · Christian Borntraeger
Quoted 44 lines · Janosch Frank

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.

---
 s390x/smp.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
s390x/smp.c +13 -1
index fa40753524f321d4..8c9b98aabd9e8222 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);
+	uint64_t nullp[12] = {};
 	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();
 
Quoted 12 lines

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

Quoted 9 lines
s390x/smp.c +2 -0
 	sigp_retry(1, SIGP_INITIAL_CPU_RESET, 0, NULL);
 	sigp(1, SIGP_STORE_STATUS_AT_ADDRESS, (uintptr_t)status, NULL);
@@ -202,6 +214,8 @@ static void test_reset_initial(void)
 	report(!status->fpc, "fpc");
 	report(!status->cputm, "cpu timer");
 	report(!status->todpr, "todpr");
+	report(!memcmp(&status->crs[1], nullp, sizeof(status->crs[1]) * 12), "cr1-13 == 0");
+	report(status->crs[15] == 0, "cr15 == 0");
 	report_prefix_pop();

Why not add a check for crs[0] == 0xe0
and crs[14] = 0xc2000000

Quoted 3 lines

You mean the checks which are done a few lines below this? This patch just actually dirties registers which should be set to 0 so we can really be sure that they have been touched.

Right. So feel free to add my RB.

#6 lore raw

PATCH 2/10 s390x: smp: Dirty fpc before initial reset test

In reply to Janosch Frank

Let's dirty the fpc, before we test if the initial reset sets it to 0.

---
 s390x/smp.c | 1 +
 1 file changed, 1 insertion(+)
s390x/smp.c +1 -0
index 8c9b98aabd9e8222..95df8c49d2495391 100644
@@ -185,6 +185,7 @@ static void test_emcall(void)
 /* Used to dirty registers of cpu #1 before it is reset */
 static void test_func_initial(void)
 {
+	asm volatile("sfpc %0" :: "d" (0x11));
 	lctlg(1, 0x42000UL);
 	lctlg(7, 0x43000UL);
 	lctlg(13, 0x44000UL);
Signature · 1 line
2.25.1
#7 lore raw

PATCH 3/10 s390x: smp: Test stop and store status on a running and stopped cpu

In reply to Janosch Frank

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

---
 s390x/smp.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
s390x/smp.c +14 -0
index 95df8c49d2495391..8a6cd1d8b17d76c6 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();
 }
 
Signature · 1 line
2.25.1
Reviewed-by on v1 3/10
#8 lore raw

Re: [kvm-unit-tests PATCH 03/10] s390x: smp: Test stop and store status on a running and stopped cpu

Quoted 7 lines · On Tue, 24 Mar 2020 04:12:44 -0400 Janosch Frank <[email protected]> wrote:

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

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

PATCH 4/10 s390x: smp: Test local interrupts after cpu reset

In reply to Janosch Frank

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

---
 s390x/smp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
s390x/smp.c +22 -0
index 8a6cd1d8b17d76c6..a8e3dd7aac0c788c 100644
@@ -243,6 +243,20 @@ static void test_reset_initial(void)
 	report_prefix_pop();
 }
 
+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);
+}
+
 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();
 }
 
Signature · 1 line
2.25.1
Reviewed-by on v1 4/10
#10 lore raw

Re: [kvm-unit-tests PATCH 04/10] s390x: smp: Test local interrupts after cpu reset

Quoted 8 lines · On Tue, 24 Mar 2020 04:12:45 -0400 Janosch Frank <[email protected]> wrote:

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

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

Re: [kvm-unit-tests PATCH 04/10] s390x: smp: Test local interrupts after cpu reset

In reply to Janosch Frank
Quoted 39 lines · Janosch Frank

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

---
 s390x/smp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
s390x/smp.c +16 -0
index 8a6cd1d8b17d76c6..a8e3dd7aac0c788c 100644
@@ -243,6 +243,20 @@ static void test_reset_initial(void)
 	report_prefix_pop();
 }
 
+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);
+}
+
 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);

s390x/smp.c static void test_reset(void) · line 279

Quoted 10 lines
s390x/smp.c +6 -0
 	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");

How can you be sure they were actually cleared/delivered?

Signature · 3 lines
Thanks,

David / dhildenb
#12 lore raw

Re: [kvm-unit-tests PATCH 04/10] s390x: smp: Test local interrupts after cpu reset

Quoted 42 lines · David Hildenbrand
Quoted 41 lines · Janosch Frank

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

---
 s390x/smp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
s390x/smp.c +16 -0
index 8a6cd1d8b17d76c6..a8e3dd7aac0c788c 100644
@@ -243,6 +243,20 @@ static void test_reset_initial(void)
 	report_prefix_pop();
 }
 
+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);
+}
+
 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);
 
Quoted 12 lines

s390x/smp.c static void test_reset(void) · line 279

Quoted 8 lines
s390x/smp.c +6 -0
 	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");

How can you be sure they were actually cleared/delivered?

Because cpu 1 would get a ext int it didn't expect and would do a report_abort() as pecified in lib/s390x/interrupt.c

#13 lore raw

Re: [kvm-unit-tests PATCH 04/10] s390x: smp: Test local interrupts after cpu reset

Quoted 55 lines · Janosch Frank
Quoted 42 lines · David Hildenbrand
Quoted 41 lines · Janosch Frank

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

---
 s390x/smp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
s390x/smp.c +16 -0
index 8a6cd1d8b17d76c6..a8e3dd7aac0c788c 100644
@@ -243,6 +243,20 @@ static void test_reset_initial(void)
 	report_prefix_pop();
 }
 
+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);
+}
+
 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);
 
Quoted 12 lines

s390x/smp.c static void test_reset(void) · line 279

Quoted 8 lines
s390x/smp.c +6 -0
 	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");

How can you be sure they were actually cleared/delivered?

Quoted 2 lines

Because cpu 1 would get a ext int it didn't expect and would do a report_abort() as pecified in lib/s390x/interrupt.c

But what if it *didn't* get the interrupts delivered.

Then cpu 1 will simply (test_local_ints()) unlock interrupts, load the psw mask, set the flag and be done with it. What am I missing? How can you be sure the interrupts on cpu 1 were actually delivered?

Signature · 3 lines
Thanks,

David / dhildenb
#14 lore raw

Re: [kvm-unit-tests PATCH 04/10] s390x: smp: Test local interrupts after cpu reset

Quoted 56 lines · David Hildenbrand
Quoted 55 lines · Janosch Frank
Quoted 54 lines · David Hildenbrand
Quoted 39 lines · Janosch Frank

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

---
 s390x/smp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
s390x/smp.c +16 -0
index 8a6cd1d8b17d76c6..a8e3dd7aac0c788c 100644
@@ -243,6 +243,20 @@ static void test_reset_initial(void)
 	report_prefix_pop();
 }
 
+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);
+}
+
 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);

s390x/smp.c static void test_reset(void) · line 279

Quoted 10 lines
s390x/smp.c +6 -0
 	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");

How can you be sure they were actually cleared/delivered?

Quoted 4 lines
Quoted 2 lines

Because cpu 1 would get a ext int it didn't expect and would do a report_abort() as pecified in lib/s390x/interrupt.c

But what if it *didn't* get the interrupts delivered.

Well, the target is still looping until the reset initial test is executed and from personal experience I can say that this test bites rather fast.

We could execute an instruction with a mandatory exit which will prompt KVM to inject any remaining IRQs before setting the flag to 1.

Unfortunately we do not have a polling instruction for non-IO interrupts at the moment to verify this test.

Quoted 5 lines

Then cpu 1 will simply (test_local_ints()) unlock interrupts, load the psw mask, set the flag and be done with it. What am I missing? How can you be sure the interrupts on cpu 1 were actually delivered?

#15 lore raw

Re: [kvm-unit-tests PATCH 04/10] s390x: smp: Test local interrupts after cpu reset

Quoted 66 lines · Janosch Frank
Quoted 56 lines · David Hildenbrand
Quoted 55 lines · Janosch Frank
Quoted 54 lines · David Hildenbrand
Quoted 39 lines · Janosch Frank

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

---
 s390x/smp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
s390x/smp.c +16 -0
index 8a6cd1d8b17d76c6..a8e3dd7aac0c788c 100644
@@ -243,6 +243,20 @@ static void test_reset_initial(void)
 	report_prefix_pop();
 }
 
+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);
+}
+
 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);

s390x/smp.c static void test_reset(void) · line 279

Quoted 10 lines
s390x/smp.c +6 -0
 	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");

How can you be sure they were actually cleared/delivered?

Quoted 4 lines
Quoted 2 lines

Because cpu 1 would get a ext int it didn't expect and would do a report_abort() as pecified in lib/s390x/interrupt.c

But what if it *didn't* get the interrupts delivered.

Well, the target is still looping until the reset initial test is executed and from personal experience I can say that this test bites rather fast.

Quoted 5 lines

We could execute an instruction with a mandatory exit which will prompt KVM to inject any remaining IRQs before setting the flag to 1.

Unfortunately we do not have a polling instruction for non-IO interrupts at the moment to verify this test.

I'd expect you should do something like this in your test instead

static void test_local_ints(void)

{
	unsigned long mask;

	/* Enable external interrupts */
	mask = extract_psw_mask();
	mask |= PSW_MASK_EXT;
	load_psw_mask(mask);

	expect_ext_int();
	/* Open masks for external call */
	ctl_set_bit(0, 13);
	if (lc->ext_int_code != $TODO1)
	       return;

	expect_ext_int();
	/* Open masks for emergency call */
	ctl_set_bit(0, 14);
	if (lc->ext_int_code != $TODO2)
	       return;

	set_flag(1);
}
Signature · 3 lines
Thanks,

David / dhildenb
#16 lore raw

PATCH 5/10 s390x: smp: Loop if secondary cpu returns into cpu setup again

In reply to Janosch Frank

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.

---
 s390x/cstart64.S | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
s390x/cstart64.S +3 -1
index 9af6bb3f28399fbc..ecffbe05c707fd7c 100644
@@ -161,7 +161,9 @@ smp_cpu_setup_state:
 	lctlg   %c0, %c0, GEN_LC_SW_INT_CRS
 	/* We should only go once through cpu setup and not for every restart */
 	stg	%r14, GEN_LC_RESTART_NEW_PSW + 8
-	br	%r14
+	brasl	%r14, %r14
+	/* If the function returns, just loop here */
+0:	j	0
 
 pgm_int:
 	SAVE_REGS
Signature · 1 line
2.25.1
Reviewed-by on v1 5/10
#17 lore raw

Re: [kvm-unit-tests PATCH 05/10] s390x: smp: Loop if secondary cpu returns into cpu setup again

Quoted 11 lines · On Tue, 24 Mar 2020 04:12:46 -0400 Janosch Frank <[email protected]> wrote:

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.

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

PATCH 6/10 s390x: smp: Remove unneeded cpu loops

In reply to Janosch Frank

Now that we have a loop which is executed after we return from the main function of a secondary cpu, we can remove the surplus loops.

---
 s390x/smp.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
s390x/smp.c +1 -7
index a8e3dd7aac0c788c..74622113a2c4ad92 100644
@@ -35,15 +35,9 @@ static void set_flag(int val)
 	mb();
 }
 
-static void cpu_loop(void)
-{
-	for (;;) {}
-}
-
 static void test_func(void)
 {
 	set_flag(1);
-	cpu_loop();
 }
 
 static void test_start(void)
@@ -292,7 +286,7 @@ int main(void)
 
 	/* Setting up the cpu to give it a stack and lowcore */
 	psw.mask = extract_psw_mask();
-	psw.addr = (unsigned long)cpu_loop;
+	psw.addr = (unsigned long)test_func;
 	smp_cpu_setup(1, psw);
 	smp_cpu_stop(1);
 
Signature · 1 line
2.25.1
#19 lore raw

PATCH 7/10 s390x: smp: Use full PSW to bringup new cpu

In reply to 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.

---
 lib/s390x/smp.c  | 2 ++
 s390x/cstart64.S | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)
lib/s390x/smp.c +2 -0
index 3f8624318f1aa5a1..6ef0335954fd4832 100644
@@ -202,6 +202,8 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
 	cpu->stack = (uint64_t *)alloc_pages(2);
 
 	/* 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;
 	cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4);
 	lc->restart_new_psw.mask = 0x0000000180000000UL;
s390x/cstart64.S +2 -1
index ecffbe05c707fd7c..e084f1305aa90e45 100644
@@ -161,7 +161,8 @@ smp_cpu_setup_state:
 	lctlg   %c0, %c0, GEN_LC_SW_INT_CRS
 	/* We should only go once through cpu setup and not for every restart */
 	stg	%r14, GEN_LC_RESTART_NEW_PSW + 8
-	brasl	%r14, %r14
+	larl	%r14, 0f
+	lpswe	GEN_LC_SW_INT_PSW
 	/* If the function returns, just loop here */
 0:	j	0
 
Signature · 1 line
2.25.1
Reviewed-by on v1 7/10
#20 lore raw

Re: [kvm-unit-tests PATCH 07/10] s390x: smp: Use full PSW to bringup new cpu

Quoted 10 lines · On Tue, 24 Mar 2020 04:12:48 -0400 Janosch Frank <[email protected]> wrote:

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.

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

PATCH 8/10 s390x: smp: Wait for sigp completion

In reply to 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.

---
 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 6ef0335954fd4832..2555bf4f5e73d762 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 ce63a89880c045f3..a8b98c0fcf2b451c 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 +4 -0
index 74622113a2c4ad92..48321f4e346dc71d 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");
@@ -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);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");
 	report(lc->grs_sa[15], "stack");
@@ -215,6 +217,7 @@ static void test_reset_initial(void)
 	wait_for_flag();
 
 	sigp_retry(1, SIGP_INITIAL_CPU_RESET, 0, NULL);
+	smp_cpu_wait_for_completion(1);
 	sigp(1, SIGP_STORE_STATUS_AT_ADDRESS, (uintptr_t)status, NULL);
 
 	report_prefix_push("clear");
@@ -264,6 +267,7 @@ static void test_reset(void)
 	smp_cpu_start(1, psw);
 
 	sigp_retry(1, SIGP_CPU_RESET, 0, NULL);
+	smp_cpu_wait_for_completion(1);
 	report(smp_cpu_stopped(1), "cpu stopped");
 
 	set_flag(0);
Signature · 1 line
2.25.1
Reviewed-by on v1 8/10
#22 lore raw

Re: [kvm-unit-tests PATCH 08/10] s390x: smp: Wait for sigp completion

Quoted 22 lines · On Tue, 24 Mar 2020 04:12:49 -0400 Janosch Frank <[email protected]> wrote:

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.

---
 lib/s390x/smp.c | 8 ++++++++
 lib/s390x/smp.h | 1 +
 s390x/smp.c     | 4 ++++
 3 files changed, 13 insertions(+)
lib/s390x/smp.c +0 -0
index 6ef0335954fd4832..2555bf4f5e73d762 100644

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

Quoted 4 lines
lib/s390x/smp.c +1 -0
@@ -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)

Hm, that is more wait-for-idle than wait-for-completion, I guess? But only semantics, no need to change that.

Quoted 51 lines
lib/s390x/smp.c +7 -0
+{
+	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 ce63a89880c045f3..a8b98c0fcf2b451c 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 +3 -0
index 74622113a2c4ad92..48321f4e346dc71d 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");
@@ -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);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");
 	report(lc->grs_sa[15], "stack");
@@ -215,6 +217,7 @@ static void test_reset_initial(void)
 	wait_for_flag();
 
 	sigp_retry(1, SIGP_INITIAL_CPU_RESET, 0, NULL);
+	smp_cpu_wait_for_completion(1);
 	sigp(1, SIGP_STORE_STATUS_AT_ADDRESS, (uintptr_t)status, NULL);
 
 	report_prefix_push("clear");

s390x/smp.c static void test_reset(void) · line 273

Quoted 7 lines
s390x/smp.c +1 -0
@@ -264,6 +267,7 @@ static void test_reset(void)
 	smp_cpu_start(1, psw);
 
 	sigp_retry(1, SIGP_CPU_RESET, 0, NULL);
+	smp_cpu_wait_for_completion(1);
 	report(smp_cpu_stopped(1), "cpu stopped");
 
 	set_flag(0);

I'm wondering whether there's a place for a sigp-and-wait-for-completion function. But that's probably overkill.

#23 lore raw

Re: [kvm-unit-tests PATCH 08/10] s390x: smp: Wait for sigp completion

In reply to Janosch Frank
Quoted 5 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

Nope, hopefully no longer "sense running".

Quoted 5 lines

sense loops. But we still lack completion checks for stop and store status, as well as the cpu resets.

Let's add them.

[...]

Quoted 26 lines
s390x/smp.c +3 -0
@@ -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");
@@ -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);
+	smp_cpu_wait_for_completion(1);
 	mb();
 	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");
 	report(lc->grs_sa[15], "stack");
@@ -215,6 +217,7 @@ static void test_reset_initial(void)
 	wait_for_flag();
 
 	sigp_retry(1, SIGP_INITIAL_CPU_RESET, 0, NULL);
+	smp_cpu_wait_for_completion(1);
 	sigp(1, SIGP_STORE_STATUS_AT_ADDRESS, (uintptr_t)status, NULL);
 
 	report_prefix_push("clear");

s390x/smp.c static void test_reset(void) · line 273

Quoted 7 lines
s390x/smp.c +1 -0
@@ -264,6 +267,7 @@ static void test_reset(void)
 	smp_cpu_start(1, psw);
 
 	sigp_retry(1, SIGP_CPU_RESET, 0, NULL);
+	smp_cpu_wait_for_completion(1);
 	report(smp_cpu_stopped(1), "cpu stopped");
 
 	set_flag(0);

Looks sane to me.

Signature · 3 lines
Thanks,

David / dhildenb
#24 lore raw

PATCH 9/10 s390x: smp: Add restart when running test

In reply to Janosch Frank

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.

---
 s390x/smp.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
s390x/smp.c +19 -0
index 48321f4e346dc71d..79cdc1f6a4b0b491 100644
@@ -52,6 +52,24 @@ 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;
+
+	set_flag(0);
+	smp_cpu_restart(1);
+	wait_for_flag();
+	report(1, "restart while running");
+}
+
 static void test_stop(void)
 {
 	smp_cpu_stop(1);
@@ -295,6 +313,7 @@ int main(void)
 	smp_cpu_stop(1);
 
 	test_start();
+	test_restart();
 	test_stop();
 	test_stop_store_status();
 	test_store_status();
Signature · 1 line
2.25.1
Reviewed-by on v1 9/10
#25 lore raw

Re: [kvm-unit-tests PATCH 09/10] s390x: smp: Add restart when running test

Quoted 8 lines · On Tue, 24 Mar 2020 04:12:50 -0400 Janosch Frank <[email protected]> wrote:

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.

---
 s390x/smp.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
#26 lore raw

Re: [kvm-unit-tests PATCH 09/10] s390x: smp: Add restart when running test

In reply to Janosch Frank
Quoted 16 lines · Janosch Frank

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.

---
 s390x/smp.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
s390x/smp.c +0 -0
index 48321f4e346dc71d..79cdc1f6a4b0b491 100644
@@ -52,6 +52,24 @@ static void test_start(void)
 	report(1, "start");
 }

s390x/smp.c static void test_start(void) · line 63

Quoted 10 lines
s390x/smp.c +9 -0
 
+/*
+ * 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;
+

Maybe explicitly trigger a stop+start with a dummy here and wait until it was processed. This make the test independent of the other (previous) tests.

Quoted 21 lines
s390x/smp.c +10 -0
+	lc->restart_new_psw.mask = extract_psw_mask();
+	lc->restart_new_psw.addr = (unsigned long)test_func;
+
+	set_flag(0);
+	smp_cpu_restart(1);
+	wait_for_flag();
+	report(1, "restart while running");
+}
+
 static void test_stop(void)
 {
 	smp_cpu_stop(1);
@@ -295,6 +313,7 @@ int main(void)
 	smp_cpu_stop(1);
 
 	test_start();
+	test_restart();
 	test_stop();
 	test_stop_store_status();
 	test_store_status();
Signature · 3 lines
Thanks,

David / dhildenb
#27 lore raw

PATCH 10/10 s390x: Fix library constant definitions

In reply to Janosch Frank

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

---
 lib/s390x/asm/arch_def.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
lib/s390x/asm/arch_def.h +4 -4
index 15a4d49ca97c9964..1b3bb0c1e7e1b626 100644
@@ -19,10 +19,10 @@ struct psw {
 #define PSW_MASK_DAT			0x0400000000000000UL
 #define PSW_MASK_PSTATE			0x0001000000000000UL
 
-#define CR0_EXTM_SCLP			0X0000000000000200UL
-#define CR0_EXTM_EXTC			0X0000000000002000UL
-#define CR0_EXTM_EMGC			0X0000000000004000UL
-#define CR0_EXTM_MASK			0X0000000000006200UL
+#define CR0_EXTM_SCLP			0x0000000000000200UL
+#define CR0_EXTM_EXTC			0x0000000000002000UL
+#define CR0_EXTM_EMGC			0x0000000000004000UL
+#define CR0_EXTM_MASK			0x0000000000006200UL
 
 struct lowcore {
 	uint8_t		pad_0x0000[0x0080 - 0x0000];	/* 0x0000 */
Signature · 1 line
2.25.1
Reviewed-by on v1 10/10
#28 lore raw

Re: [kvm-unit-tests PATCH 10/10] s390x: Fix library constant definitions

Quoted 4 lines · Janosch Frank

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

Quoted 24 lines
---
 lib/s390x/asm/arch_def.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
lib/s390x/asm/arch_def.h +4 -4
index 15a4d49ca97c9964..1b3bb0c1e7e1b626 100644
@@ -19,10 +19,10 @@ struct psw {
 #define PSW_MASK_DAT			0x0400000000000000UL
 #define PSW_MASK_PSTATE			0x0001000000000000UL
 
-#define CR0_EXTM_SCLP			0X0000000000000200UL
-#define CR0_EXTM_EXTC			0X0000000000002000UL
-#define CR0_EXTM_EMGC			0X0000000000004000UL
-#define CR0_EXTM_MASK			0X0000000000006200UL
+#define CR0_EXTM_SCLP			0x0000000000000200UL
+#define CR0_EXTM_EXTC			0x0000000000002000UL
+#define CR0_EXTM_EMGC			0x0000000000004000UL
+#define CR0_EXTM_MASK			0x0000000000006200UL
 
 struct lowcore {
 	uint8_t		pad_0x0000[0x0080 - 0x0000];	/* 0x0000 */
Reviewed-by on v1 10/10
#29 lore raw

Re: [kvm-unit-tests PATCH 10/10] s390x: Fix library constant definitions

In reply to Janosch Frank
Quoted 2 lines · On Tue, 24 Mar 2020 04:12:51 -0400 Janosch Frank <[email protected]> wrote:

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

s/Lets/Let's/ :)

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