[kvm-unit-tests PATCH v3 0/3] s390x: add tests for SIGP call orders in enabled wait

[kvm-unit-tests PATCH v3 0/3] s390x: add tests for SIGP call orders in enabled wait

[Nico Boehr]

v2->v3:
---
* added some comments (thanks Janosch)
* fix bit-mask confusion (thanks Janosch)
* remove ctl_clear_bit() in call_in_wait_received() since it's handled
  in the interrupt cleanup already

v1->v2:
---
* rebase to latest master to align with Claudio's SMP changes, drop
  patch which adds the ext int clean up since it is already in Claudio's
  series
* make sure ctl0 register bit is cleared

When a CPU is in enabled wait, it can still receive SIGP calls from
other CPUs.

Since this requires some special handling in KVM, we should have tests
for it. This has already revealed a KVM bug with ecall under PV, which
is why this test currently fails there.

Some refactoring is done as part of this series to reduce code
duplication.

Nico Boehr (3):
  s390x: smp: move sigp calls with invalid cpu address to array
  s390x: smp: use an array for sigp calls
  s390x: smp: add tests for calls in wait state

 s390x/smp.c | 200 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 119 insertions(+), 81 deletions(-)

-- 
2.36.1

[kvm-unit-tests PATCH v3 1/3] s390x: smp: move sigp calls with invalid cpu address to array

[Nico Boehr]

We have the nice array to test SIGP calls with invalid CPU addresses.
Move the SIGP cases there to eliminate some of the duplicated code in
test_emcall and test_cond_emcall.

Since adding coverage for invalid CPU addresses in the ecall case is now
trivial, do that as well.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/smp.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/s390x/smp.c b/s390x/smp.c
index 0df4751f9cee..34ae91c3fe12 100644
--- a/s390x/smp.c
+++ b/s390x/smp.c
@@ -30,6 +30,9 @@ static const struct sigp_invalid_cases cases_invalid_cpu_addr[] = {
 	{ SIGP_STOP,                  "stop with invalid CPU address" },
 	{ SIGP_START,                 "start with invalid CPU address" },
 	{ SIGP_CPU_RESET,             "reset with invalid CPU address" },
+	{ SIGP_COND_EMERGENCY_SIGNAL, "conditional emcall with invalid CPU address" },
+	{ SIGP_EMERGENCY_SIGNAL,      "emcall with invalid CPU address" },
+	{ SIGP_EXTERNAL_CALL,         "ecall with invalid CPU address" },
 	{ INVALID_ORDER_CODE,         "invalid order code and CPU address" },
 	{ SIGP_SENSE,                 "sense with invalid CPU address" },
 	{ SIGP_STOP_AND_STORE_STATUS, "stop and store status with invalid CPU address" },
@@ -329,7 +332,6 @@ static void emcall(void)
 static void test_emcall(void)
 {
 	struct psw psw;
-	int cc;
 	psw.mask = extract_psw_mask();
 	psw.addr = (unsigned long)emcall;
 
@@ -343,13 +345,6 @@ static void test_emcall(void)
 	wait_for_flag();
 	smp_cpu_stop(1);
 
-	report_prefix_push("invalid CPU address");
-
-	cc = sigp(INVALID_CPU_ADDRESS, SIGP_EMERGENCY_SIGNAL, 0, NULL);
-	report(cc == 3, "CC = 3");
-
-	report_prefix_pop();
-
 	report_prefix_pop();
 }
 
@@ -368,13 +363,6 @@ static void test_cond_emcall(void)
 		goto out;
 	}
 
-	report_prefix_push("invalid CPU address");
-
-	cc = sigp(INVALID_CPU_ADDRESS, SIGP_COND_EMERGENCY_SIGNAL, 0, NULL);
-	report(cc == 3, "CC = 3");
-
-	report_prefix_pop();
-
 	report_prefix_push("success");
 	set_flag(0);
 
-- 
2.36.1

[kvm-unit-tests PATCH v3 2/3] s390x: smp: use an array for sigp calls

[Nico Boehr]

Tests for the SIGP calls are quite similar, so we have a lot of code
duplication right now. Since upcoming changes will add more cases,
refactor the code to iterate over an array, similarily as we already do
for test_invalid().

The receiving CPU is disabled for IO interrupts. This makes sure the
conditional emergency signal is accepted and doesn't hurt the other
orders.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/smp.c | 124 ++++++++++++++++++++--------------------------------
 1 file changed, 48 insertions(+), 76 deletions(-)

diff --git a/s390x/smp.c b/s390x/smp.c
index 34ae91c3fe12..12c40cadaed2 100644
--- a/s390x/smp.c
+++ b/s390x/smp.c
@@ -43,6 +43,20 @@ static const struct sigp_invalid_cases cases_valid_cpu_addr[] = {
 
 static uint32_t cpu1_prefix;
 
+struct sigp_call_cases {
+	char name[20];
+	int call;
+	uint16_t ext_int_expected_type;
+	uint32_t cr0_bit;
+	bool supports_pv;
+};
+static const struct sigp_call_cases cases_sigp_call[] = {
+	{ "emcall",      SIGP_EMERGENCY_SIGNAL,      0x1201, CTL0_EMERGENCY_SIGNAL, true },
+	{ "cond emcall", SIGP_COND_EMERGENCY_SIGNAL, 0x1201, CTL0_EMERGENCY_SIGNAL, false },
+	{ "ecall",       SIGP_EXTERNAL_CALL,         0x1202, CTL0_EXTERNAL_CALL,    true },
+};
+static const struct sigp_call_cases *current_sigp_call_case;
+
 static void test_invalid(void)
 {
 	const struct sigp_invalid_cases *c;
@@ -289,97 +303,57 @@ static void test_set_prefix(void)
 
 }
 
-static void ecall(void)
+static void call_received(void)
 {
 	expect_ext_int();
-	ctl_set_bit(0, CTL0_EXTERNAL_CALL);
-	psw_mask_set_bits(PSW_MASK_EXT);
-	set_flag(1);
-	while (lowcore.ext_int_code != 0x1202) { mb(); }
-	report_pass("received");
-	set_flag(1);
-}
+	ctl_set_bit(0, current_sigp_call_case->cr0_bit);
+	/* make sure conditional emergency is accepted by disabling IO interrupts */
+	psw_mask_clear_and_set_bits(PSW_MASK_IO, PSW_MASK_EXT);
 
-static void test_ecall(void)
-{
-	struct psw psw;
-	psw.mask = extract_psw_mask();
-	psw.addr = (unsigned long)ecall;
+	/* Indicate that we're ready to receive the call */
+	set_flag(1);
 
-	report_prefix_push("ecall");
-	set_flag(0);
+	while (lowcore.ext_int_code != current_sigp_call_case->ext_int_expected_type)
+		mb();
+	report_pass("received");
 
-	smp_cpu_start(1, psw);
-	wait_for_flag();
-	set_flag(0);
-	smp_sigp(1, SIGP_EXTERNAL_CALL, 0, NULL);
-	wait_for_flag();
-	smp_cpu_stop(1);
-	report_prefix_pop();
-}
+	ctl_clear_bit(0, current_sigp_call_case->cr0_bit);
 
-static void emcall(void)
-{
-	expect_ext_int();
-	ctl_set_bit(0, CTL0_EMERGENCY_SIGNAL);
-	psw_mask_set_bits(PSW_MASK_EXT);
-	set_flag(1);
-	while (lowcore.ext_int_code != 0x1201) { mb(); }
-	report_pass("received");
+	/* Indicate that we're done */
 	set_flag(1);
 }
 
-static void test_emcall(void)
+static void test_calls(void)
 {
+	int i;
 	struct psw psw;
-	psw.mask = extract_psw_mask();
-	psw.addr = (unsigned long)emcall;
 
-	report_prefix_push("emcall");
-	set_flag(0);
+	for (i = 0; i < ARRAY_SIZE(cases_sigp_call); i++) {
+		current_sigp_call_case = &cases_sigp_call[i];
 
-	smp_cpu_start(1, psw);
-	wait_for_flag();
-	set_flag(0);
-	smp_sigp(1, SIGP_EMERGENCY_SIGNAL, 0, NULL);
-	wait_for_flag();
-	smp_cpu_stop(1);
+		report_prefix_push(current_sigp_call_case->name);
+		if (!current_sigp_call_case->supports_pv && uv_os_is_guest()) {
+			report_skip("Not supported under PV");
+			report_prefix_pop();
+			continue;
+		}
 
-	report_prefix_pop();
-}
+		set_flag(0);
+		psw.mask = extract_psw_mask();
+		psw.addr = (unsigned long)call_received;
+		smp_cpu_start(1, psw);
 
-static void test_cond_emcall(void)
-{
-	uint32_t status = 0;
-	struct psw psw;
-	int cc;
-	psw.mask = extract_psw_mask() & ~PSW_MASK_IO;
-	psw.addr = (unsigned long)emcall;
+		/* Wait until the receiver has finished setup */
+		wait_for_flag();
+		set_flag(0);
 
-	report_prefix_push("conditional emergency call");
+		smp_sigp(1, current_sigp_call_case->call, 0, NULL);
 
-	if (uv_os_is_guest()) {
-		report_skip("unsupported under PV");
-		goto out;
+		/* Wait until the receiver has handled the call */
+		wait_for_flag();
+		smp_cpu_stop(1);
+		report_prefix_pop();
 	}
-
-	report_prefix_push("success");
-	set_flag(0);
-
-	smp_cpu_start(1, psw);
-	wait_for_flag();
-	set_flag(0);
-	cc = smp_sigp(1, SIGP_COND_EMERGENCY_SIGNAL, 0, &status);
-	report(!cc, "CC = 0");
-
-	wait_for_flag();
-	smp_cpu_stop(1);
-
-	report_prefix_pop();
-
-out:
-	report_prefix_pop();
-
 }
 
 static void test_sense_running(void)
@@ -499,9 +473,7 @@ int main(void)
 	test_stop_store_status();
 	test_store_status();
 	test_set_prefix();
-	test_ecall();
-	test_emcall();
-	test_cond_emcall();
+	test_calls();
 	test_sense_running();
 	test_reset();
 	test_reset_initial();
-- 
2.36.1

[kvm-unit-tests PATCH v3 3/3] s390x: smp: add tests for calls in wait state

[Nico Boehr]

When the SIGP interpretation facility is in use a SIGP external call to
a waiting CPU will result in an exit of the calling cpu. For non-pv
guests it's a code 56 (partial execution) exit otherwise its a code 108
(secure instruction notification) exit. Those exits are handled
differently from a normal SIGP instruction intercept that happens
without interpretation and hence need to be tested.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
---
 s390x/smp.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/s390x/smp.c b/s390x/smp.c
index 12c40cadaed2..d59ca38e7a37 100644
--- a/s390x/smp.c
+++ b/s390x/smp.c
@@ -356,6 +356,83 @@ static void test_calls(void)
 	}
 }
 
+static void call_in_wait_ext_int_fixup(struct stack_frame_int *stack)
+{
+	/* Clear wait bit so we don't immediately wait again after the fixup */
+	lowcore.ext_old_psw.mask &= ~PSW_MASK_WAIT;
+
+	stack->crs[0] &= ~BIT(current_sigp_call_case->cr0_bit);
+}
+
+static void call_in_wait_setup(void)
+{
+	expect_ext_int();
+	ctl_set_bit(0, current_sigp_call_case->cr0_bit);
+	register_ext_cleanup_func(call_in_wait_ext_int_fixup);
+
+	set_flag(1);
+}
+
+static void call_in_wait_received(void)
+{
+	report(lowcore.ext_int_code == current_sigp_call_case->ext_int_expected_type, "received");
+	register_ext_cleanup_func(NULL);
+
+	set_flag(1);
+}
+
+static void test_calls_in_wait(void)
+{
+	int i;
+	struct psw psw;
+
+	report_prefix_push("psw wait");
+	for (i = 0; i < ARRAY_SIZE(cases_sigp_call); i++) {
+		current_sigp_call_case = &cases_sigp_call[i];
+
+		report_prefix_push(current_sigp_call_case->name);
+		if (!current_sigp_call_case->supports_pv && uv_os_is_guest()) {
+			report_skip("Not supported under PV");
+			report_prefix_pop();
+			continue;
+		}
+
+		/* Let the secondary CPU setup the external mask and the external interrupt cleanup function */
+		set_flag(0);
+		psw.mask = extract_psw_mask();
+		psw.addr = (unsigned long)call_in_wait_setup;
+		smp_cpu_start(1, psw);
+
+		/* Wait until the receiver has finished setup */
+		wait_for_flag();
+		set_flag(0);
+
+		/*
+		 * To avoid races, we need to know that the secondary CPU has entered wait,
+		 * but the architecture provides no way to check whether the secondary CPU
+		 * is in wait.
+		 *
+		 * But since a waiting CPU is considered operating, simply stop the CPU, set
+		 * up the restart new PSW mask in wait, send the restart interrupt and then
+		 * wait until the CPU becomes operating (done by smp_cpu_start).
+		 */
+		smp_cpu_stop(1);
+		expect_ext_int();
+		psw.mask = extract_psw_mask() | PSW_MASK_EXT | PSW_MASK_WAIT;
+		psw.addr = (unsigned long)call_in_wait_received;
+		smp_cpu_start(1, psw);
+
+		smp_sigp(1, current_sigp_call_case->call, 0, NULL);
+
+		/* Wait until the receiver has handled the call */
+		wait_for_flag();
+		smp_cpu_stop(1);
+
+		report_prefix_pop();
+	}
+	report_prefix_pop();
+}
+
 static void test_sense_running(void)
 {
 	report_prefix_push("sense_running");
@@ -474,6 +551,7 @@ int main(void)
 	test_store_status();
 	test_set_prefix();
 	test_calls();
+	test_calls_in_wait();
 	test_sense_running();
 	test_reset();
 	test_reset_initial();
-- 
2.36.1

Re: [kvm-unit-tests PATCH v3 2/3] s390x: smp: use an array for sigp calls

[Claudio Imbrenda]

On Mon, 25 Jul 2022 17:54:19 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> Tests for the SIGP calls are quite similar, so we have a lot of code
> duplication right now. Since upcoming changes will add more cases,
> refactor the code to iterate over an array, similarily as we already do
> for test_invalid().
> 
> The receiving CPU is disabled for IO interrupts. This makes sure the
> conditional emergency signal is accepted and doesn't hurt the other
> orders.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  s390x/smp.c | 124 ++++++++++++++++++++--------------------------------
>  1 file changed, 48 insertions(+), 76 deletions(-)
> 
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 34ae91c3fe12..12c40cadaed2 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -43,6 +43,20 @@ static const struct sigp_invalid_cases cases_valid_cpu_addr[] = {
>  
>  static uint32_t cpu1_prefix;
>  
> +struct sigp_call_cases {
> +	char name[20];
> +	int call;
> +	uint16_t ext_int_expected_type;
> +	uint32_t cr0_bit;

does it need to be 32 bits? the range of valid values is 0 ~ 63
bonus, if you use an uint8_t, the whole struct will shrink by 8 bytes

with that fixed:

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

(unless there is a good enough reason to use uint32_t)

> +	bool supports_pv;
> +};
> +static const struct sigp_call_cases cases_sigp_call[] = {
> +	{ "emcall",      SIGP_EMERGENCY_SIGNAL,      0x1201, CTL0_EMERGENCY_SIGNAL, true },
> +	{ "cond emcall", SIGP_COND_EMERGENCY_SIGNAL, 0x1201, CTL0_EMERGENCY_SIGNAL, false },
> +	{ "ecall",       SIGP_EXTERNAL_CALL,         0x1202, CTL0_EXTERNAL_CALL,    true },
> +};
> +static const struct sigp_call_cases *current_sigp_call_case;
> +
>  static void test_invalid(void)
>  {
>  	const struct sigp_invalid_cases *c;
> @@ -289,97 +303,57 @@ static void test_set_prefix(void)
>  
>  }
>  
> -static void ecall(void)
> +static void call_received(void)
>  {
>  	expect_ext_int();
> -	ctl_set_bit(0, CTL0_EXTERNAL_CALL);
> -	psw_mask_set_bits(PSW_MASK_EXT);
> -	set_flag(1);
> -	while (lowcore.ext_int_code != 0x1202) { mb(); }
> -	report_pass("received");
> -	set_flag(1);
> -}
> +	ctl_set_bit(0, current_sigp_call_case->cr0_bit);
> +	/* make sure conditional emergency is accepted by disabling IO interrupts */
> +	psw_mask_clear_and_set_bits(PSW_MASK_IO, PSW_MASK_EXT);
>  
> -static void test_ecall(void)
> -{
> -	struct psw psw;
> -	psw.mask = extract_psw_mask();
> -	psw.addr = (unsigned long)ecall;
> +	/* Indicate that we're ready to receive the call */
> +	set_flag(1);
>  
> -	report_prefix_push("ecall");
> -	set_flag(0);
> +	while (lowcore.ext_int_code != current_sigp_call_case->ext_int_expected_type)
> +		mb();
> +	report_pass("received");
>  
> -	smp_cpu_start(1, psw);
> -	wait_for_flag();
> -	set_flag(0);
> -	smp_sigp(1, SIGP_EXTERNAL_CALL, 0, NULL);
> -	wait_for_flag();
> -	smp_cpu_stop(1);
> -	report_prefix_pop();
> -}
> +	ctl_clear_bit(0, current_sigp_call_case->cr0_bit);
>  
> -static void emcall(void)
> -{
> -	expect_ext_int();
> -	ctl_set_bit(0, CTL0_EMERGENCY_SIGNAL);
> -	psw_mask_set_bits(PSW_MASK_EXT);
> -	set_flag(1);
> -	while (lowcore.ext_int_code != 0x1201) { mb(); }
> -	report_pass("received");
> +	/* Indicate that we're done */
>  	set_flag(1);
>  }
>  
> -static void test_emcall(void)
> +static void test_calls(void)
>  {
> +	int i;
>  	struct psw psw;
> -	psw.mask = extract_psw_mask();
> -	psw.addr = (unsigned long)emcall;
>  
> -	report_prefix_push("emcall");
> -	set_flag(0);
> +	for (i = 0; i < ARRAY_SIZE(cases_sigp_call); i++) {
> +		current_sigp_call_case = &cases_sigp_call[i];
>  
> -	smp_cpu_start(1, psw);
> -	wait_for_flag();
> -	set_flag(0);
> -	smp_sigp(1, SIGP_EMERGENCY_SIGNAL, 0, NULL);
> -	wait_for_flag();
> -	smp_cpu_stop(1);
> +		report_prefix_push(current_sigp_call_case->name);
> +		if (!current_sigp_call_case->supports_pv && uv_os_is_guest()) {
> +			report_skip("Not supported under PV");
> +			report_prefix_pop();
> +			continue;
> +		}
>  
> -	report_prefix_pop();
> -}
> +		set_flag(0);
> +		psw.mask = extract_psw_mask();
> +		psw.addr = (unsigned long)call_received;
> +		smp_cpu_start(1, psw);
>  
> -static void test_cond_emcall(void)
> -{
> -	uint32_t status = 0;
> -	struct psw psw;
> -	int cc;
> -	psw.mask = extract_psw_mask() & ~PSW_MASK_IO;
> -	psw.addr = (unsigned long)emcall;
> +		/* Wait until the receiver has finished setup */
> +		wait_for_flag();
> +		set_flag(0);
>  
> -	report_prefix_push("conditional emergency call");
> +		smp_sigp(1, current_sigp_call_case->call, 0, NULL);
>  
> -	if (uv_os_is_guest()) {
> -		report_skip("unsupported under PV");
> -		goto out;
> +		/* Wait until the receiver has handled the call */
> +		wait_for_flag();
> +		smp_cpu_stop(1);
> +		report_prefix_pop();
>  	}
> -
> -	report_prefix_push("success");
> -	set_flag(0);
> -
> -	smp_cpu_start(1, psw);
> -	wait_for_flag();
> -	set_flag(0);
> -	cc = smp_sigp(1, SIGP_COND_EMERGENCY_SIGNAL, 0, &status);
> -	report(!cc, "CC = 0");
> -
> -	wait_for_flag();
> -	smp_cpu_stop(1);
> -
> -	report_prefix_pop();
> -
> -out:
> -	report_prefix_pop();
> -
>  }
>  
>  static void test_sense_running(void)
> @@ -499,9 +473,7 @@ int main(void)
>  	test_stop_store_status();
>  	test_store_status();
>  	test_set_prefix();
> -	test_ecall();
> -	test_emcall();
> -	test_cond_emcall();
> +	test_calls();
>  	test_sense_running();
>  	test_reset();
>  	test_reset_initial();

Re: [kvm-unit-tests PATCH v3 2/3] s390x: smp: use an array for sigp calls

[Nico Boehr]

Quoting Claudio Imbrenda (2022-07-28 17:47:35)
[...]
> > diff --git a/s390x/smp.c b/s390x/smp.c
> > index 34ae91c3fe12..12c40cadaed2 100644
> > --- a/s390x/smp.c
> > +++ b/s390x/smp.c
> > @@ -43,6 +43,20 @@ static const struct sigp_invalid_cases cases_valid_cpu_addr[] = {
> >  
> >  static uint32_t cpu1_prefix;
> >  
> > +struct sigp_call_cases {
> > +     char name[20];
> > +     int call;
> > +     uint16_t ext_int_expected_type;
> > +     uint32_t cr0_bit;
> 
> does it need to be 32 bits? the range of valid values is 0 ~ 63
> bonus, if you use an uint8_t, the whole struct will shrink by 8 bytes

uint32_t is clearly inappropriate here. Since I see little reason to optimize for size here, I would argue for int, but I am also ok with uint8_t.

Re: [kvm-unit-tests PATCH v3 3/3] s390x: smp: add tests for calls in wait state

[Claudio Imbrenda]

On Mon, 25 Jul 2022 17:54:20 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> When the SIGP interpretation facility is in use a SIGP external call to
> a waiting CPU will result in an exit of the calling cpu. For non-pv
> guests it's a code 56 (partial execution) exit otherwise its a code 108
> (secure instruction notification) exit. Those exits are handled
> differently from a normal SIGP instruction intercept that happens
> without interpretation and hence need to be tested.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
>  s390x/smp.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
> 
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 12c40cadaed2..d59ca38e7a37 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -356,6 +356,83 @@ static void test_calls(void)
>  	}
>  }
>  
> +static void call_in_wait_ext_int_fixup(struct stack_frame_int *stack)
> +{
> +	/* Clear wait bit so we don't immediately wait again after the fixup */
> +	lowcore.ext_old_psw.mask &= ~PSW_MASK_WAIT;
> +
> +	stack->crs[0] &= ~BIT(current_sigp_call_case->cr0_bit);
> +}
> +
> +static void call_in_wait_setup(void)
> +{
> +	expect_ext_int();
> +	ctl_set_bit(0, current_sigp_call_case->cr0_bit);
> +	register_ext_cleanup_func(call_in_wait_ext_int_fixup);
> +
> +	set_flag(1);
> +}
> +
> +static void call_in_wait_received(void)
> +{
> +	report(lowcore.ext_int_code == current_sigp_call_case->ext_int_expected_type, "received");
> +	register_ext_cleanup_func(NULL);
> +
> +	set_flag(1);
> +}
> +
> +static void test_calls_in_wait(void)
> +{
> +	int i;
> +	struct psw psw;
> +
> +	report_prefix_push("psw wait");
> +	for (i = 0; i < ARRAY_SIZE(cases_sigp_call); i++) {
> +		current_sigp_call_case = &cases_sigp_call[i];
> +
> +		report_prefix_push(current_sigp_call_case->name);
> +		if (!current_sigp_call_case->supports_pv && uv_os_is_guest()) {
> +			report_skip("Not supported under PV");
> +			report_prefix_pop();
> +			continue;
> +		}
> +
> +		/* Let the secondary CPU setup the external mask and the external interrupt cleanup function */
> +		set_flag(0);
> +		psw.mask = extract_psw_mask();
> +		psw.addr = (unsigned long)call_in_wait_setup;
> +		smp_cpu_start(1, psw);
> +
> +		/* Wait until the receiver has finished setup */
> +		wait_for_flag();
> +		set_flag(0);
> +
> +		/*
> +		 * To avoid races, we need to know that the secondary CPU has entered wait,
> +		 * but the architecture provides no way to check whether the secondary CPU
> +		 * is in wait.
> +		 *
> +		 * But since a waiting CPU is considered operating, simply stop the CPU, set
> +		 * up the restart new PSW mask in wait, send the restart interrupt and then
> +		 * wait until the CPU becomes operating (done by smp_cpu_start).
> +		 */
> +		smp_cpu_stop(1);
> +		expect_ext_int();

which external interrupt are you expecting on this CPU?

> +		psw.mask = extract_psw_mask() | PSW_MASK_EXT | PSW_MASK_WAIT;
> +		psw.addr = (unsigned long)call_in_wait_received;
> +		smp_cpu_start(1, psw);
> +
> +		smp_sigp(1, current_sigp_call_case->call, 0, NULL);
> +
> +		/* Wait until the receiver has handled the call */
> +		wait_for_flag();
> +		smp_cpu_stop(1);
> +
> +		report_prefix_pop();
> +	}
> +	report_prefix_pop();
> +}
> +
>  static void test_sense_running(void)
>  {
>  	report_prefix_push("sense_running");
> @@ -474,6 +551,7 @@ int main(void)
>  	test_store_status();
>  	test_set_prefix();
>  	test_calls();
> +	test_calls_in_wait();
>  	test_sense_running();
>  	test_reset();
>  	test_reset_initial();

Re: [kvm-unit-tests PATCH v3 3/3] s390x: smp: add tests for calls in wait state

[Nico Boehr]

Quoting Claudio Imbrenda (2022-08-02 18:06:05)
[...]
> > diff --git a/s390x/smp.c b/s390x/smp.c
> > index 12c40cadaed2..d59ca38e7a37 100644
> > --- a/s390x/smp.c
> > +++ b/s390x/smp.c
[...]
> > +static void test_calls_in_wait(void)
> > +{
[...]
> > +             /*
> > +              * To avoid races, we need to know that the secondary CPU has entered wait,
> > +              * but the architecture provides no way to check whether the secondary CPU
> > +              * is in wait.
> > +              *
> > +              * But since a waiting CPU is considered operating, simply stop the CPU, set
> > +              * up the restart new PSW mask in wait, send the restart interrupt and then
> > +              * wait until the CPU becomes operating (done by smp_cpu_start).
> > +              */
> > +             smp_cpu_stop(1);
> > +             expect_ext_int();
> 
> which external interrupt are you expecting on this CPU?

Right, leftover code, can be removed. Will be fixed.