Re: [PATCH v2 08/10] s390x: smp: Wait for sigp completion

Janosch Frank
2020-04-24T11:40:39Z

[-- Attachment #1.1: Type: text/plain, Size: 2559 bytes --]

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@linux.ibm.com>
>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>> 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]