On Tue, 24 Mar 2020 04:12:49 -0400
Janosch Frank <frankja@xxxxxxxxxxxxx> 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.
>
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
> ---
> lib/s390x/smp.c | 8 ++++++++
> lib/s390x/smp.h | 1 +
> s390x/smp.c | 4 ++++
> 3 files changed, 13 insertions(+)
>
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index 6ef0335954fd4832..2555bf4f5e73d762 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -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.
> +{
> + 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;
> diff --git a/lib/s390x/smp.h b/lib/s390x/smp.h
> index ce63a89880c045f3..a8b98c0fcf2b451c 100644
> --- a/lib/s390x/smp.h
> +++ b/lib/s390x/smp.h
> @@ -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);
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 74622113a2c4ad92..48321f4e346dc71d 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -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);
I'm wondering whether there's a place for a
sigp-and-wait-for-completion function. But that's probably overkill.
Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
