On Thu, 3 Mar 2022 22:04:24 +0100
Eric Farman <farman@xxxxxxxxxxxxx> wrote:
> The kvm-unit-tests infrastructure for a CPU restart waits for the
> SIGP RESTART to complete. In order to test the restart itself,
> create a variation that does not wait, and test the state of the
> CPU directly.
>
> While here, add some better report prefixes/messages, to clarify
> which condition is being examined (similar to test_stop_store_status()).
>
> Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
> ---
> lib/s390x/smp.c | 22 ++++++++++++++++++++++
> lib/s390x/smp.h | 1 +
> s390x/smp.c | 18 +++++++++++++++---
> 3 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index 84e536e8..85b046a5 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -192,6 +192,28 @@ int smp_cpu_restart(uint16_t idx)
> return rc;
> }
>
> +/*
> + * Functionally equivalent to smp_cpu_restart(), but without the
> + * elements that wait/serialize matters here in the test.
> + * Used to see if KVM itself is serialized correctly.
> + */
> +int smp_cpu_restart_nowait(uint16_t idx)
> +{
> + spin_lock(&lock);
> +
> + /* Don't suppress a CC2 with sigp_retry() */
> + if (smp_sigp(idx, SIGP_RESTART, 0, NULL)) {
> + spin_unlock(&lock);
> + return -1;
> + }
> +
> + cpus[idx].active = true;
> +
> + spin_unlock(&lock);
> +
> + return 0;
> +}
> +
> int smp_cpu_start(uint16_t idx, struct psw psw)
> {
> int rc;
> diff --git a/lib/s390x/smp.h b/lib/s390x/smp.h
> index bae03dfd..24a0e2e0 100644
> --- a/lib/s390x/smp.h
> +++ b/lib/s390x/smp.h
> @@ -42,6 +42,7 @@ uint16_t smp_cpu_addr(uint16_t idx);
> bool smp_cpu_stopped(uint16_t idx);
> bool smp_sense_running_status(uint16_t idx);
> int smp_cpu_restart(uint16_t idx);
> +int smp_cpu_restart_nowait(uint16_t idx);
> int smp_cpu_start(uint16_t idx, struct psw psw);
> int smp_cpu_stop(uint16_t idx);
> int smp_cpu_stop_nowait(uint16_t idx);
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 11c2c673..03160b80 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -55,23 +55,35 @@ static void test_restart(void)
> struct cpu *cpu = smp_cpu_from_idx(1);
> struct lowcore *lc = cpu->lowcore;
>
> + report_prefix_push("restart");
> + report_prefix_push("stopped");
> +
> lc->restart_new_psw.mask = extract_psw_mask();
> lc->restart_new_psw.addr = (unsigned long)test_func;
>
> /* Make sure cpu is stopped */
> smp_cpu_stop(1);
> set_flag(0);
> - smp_cpu_restart(1);
> + smp_cpu_restart_nowait(1);
> + report(!smp_cpu_stopped(1), "cpu started");
can this check ^ race?
we are using the flag to check if the CPU actually restarts, right?
> wait_for_flag();
> + report_pass("test flag");
> +
> + report_prefix_pop();
> + report_prefix_push("running");
>
> /*
> * Wait until cpu 1 has set the flag because it executed the
> * restart function.
> */
> set_flag(0);
> - smp_cpu_restart(1);
> + smp_cpu_restart_nowait(1);
> + report(!smp_cpu_stopped(1), "cpu started");
same here
> wait_for_flag();
> - report_pass("restart while running");
> + report_pass("test flag");
> +
> + report_prefix_pop();
> + report_prefix_pop();
> }
>
> static void test_stop(void)
