On 4/24/20 12:11 PM, David Hildenbrand wrote: > On 23.04.20 11:10, Janosch Frank 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> >> Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx> >> --- >> 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 6ef0335..2555bf4 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) >> +{ >> + 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 ce63a89..a8b98c0 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 7462211..48321f4 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); > > Just curious: Would it make sense to add that inside > smp_cpu_stop_store_status() instead? > I think so, we also wait for stop and start to finish, so why not for this order code.
Attachment:
signature.asc
Description: OpenPGP digital signature