Re: [PATCH kvm-unit-tests v2 3/6] s390x: smp: Fix checks for SIGP STOP STORE STATUS

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Fri, 11 Mar 2022 18:38:19 +0100
Eric Farman <farman@linux.ibm.com> wrote:

> In the routine test_stop_store_status(), the "running" part of
> the test checks a few of the fields in lowcore (to verify the
> "STORE STATUS" part of the SIGP order), and then ensures that
> the CPU has stopped. But this is backwards, according to the
> Principles of Operation:
>   The addressed CPU performs the stop function, fol-
>   lowed by the store-status operation (see “Store Sta-
>   tus” on page 4-82).
> 
> If the CPU were not yet stopped, the contents of the lowcore
> fields would be unpredictable. It works today because the
> library functions wait on the stop function, so the CPU is
> stopped by the time it comes back. Let's first check that the
> CPU is stopped first, just to be clear.
> 
> While here, add the same check to the second part of the test,
> even though the CPU is explicitly stopped prior to the SIGP.
> 
> Fixes: fc67b07a4 ("s390x: smp: Test stop and store status on a running and stopped cpu")
> Signed-off-by: Eric Farman <farman@linux.ibm.com>

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

> ---
>  s390x/smp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 2f4af820..50811bd0 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -98,9 +98,9 @@ static void test_stop_store_status(void)
>  	lc->grs_sa[15] = 0;
>  	smp_cpu_stop_store_status(1);
>  	mb();
> +	report(smp_cpu_stopped(1), "cpu stopped");
>  	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");
> @@ -108,6 +108,7 @@ static void test_stop_store_status(void)
>  	lc->grs_sa[15] = 0;
>  	smp_cpu_stop_store_status(1);
>  	mb();
> +	report(smp_cpu_stopped(1), "cpu stopped");
>  	report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix");
>  	report(lc->grs_sa[15], "stack");
>  	report_prefix_pop();