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

[Janosch Frank <frankja@linux.ibm.com>]

On 7/22/22 09:20, Nico Boehr wrote:
> Under PV, SIGP ecall requires some special handling by the hypervisor
> when the receiving CPU is in enabled wait. Hence, we should have
> coverage for the various SIGP call orders when the receiving CPU is in
> enabled wait.

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.

> 
> The ecall test currently fails under PV due to a KVM bug under
> investigation.

That shouldn't be true anymore

> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
>   s390x/smp.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 75 insertions(+)
> 
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 683b0e618a48..eed7aa3564de 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -347,6 +347,80 @@ static void test_calls(void)
>   	}
>   }
>   
> +static void call_in_wait_ext_int_fixup(struct stack_frame_int *stack)
> +{
> +	/* leave wait after returning */

Clear wait bit so we don't immediately wait again after the fixup

> +	lowcore.ext_old_psw.mask &= ~PSW_MASK_WAIT;
> +
> +	stack->crs[0] &= ~current_sigp_call_case->cr0_bit;

You need a mask but have a bit, no?

~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");
> +	ctl_clear_bit(0, current_sigp_call_case->cr0_bit);
> +	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_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_for_flag();
> +		smp_cpu_stop(1);
> +
> +		report_prefix_pop();
> +	}
> +	report_prefix_pop();
> +}
> +
>   static void test_sense_running(void)
>   {
>   	report_prefix_push("sense_running");
> @@ -465,6 +539,7 @@ int main(void)
>   	test_store_status();
>   	test_set_prefix();
>   	test_calls();
> +	test_calls_in_wait();
>   	test_sense_running();
>   	test_reset();
>   	test_reset_initial();