On 1/16/20 3:14 PM, Cornelia Huck wrote:
> On Thu, 16 Jan 2020 07:05:10 -0500
> Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:
>
>> sigp is not synchronous on all hypervisors, so we need to wait until
>
> "The architecture specifies that processing sigp orders may be
> asynchronous, and this is indeed the case on some hypervisors, so..."
>
> ? (Or is that overkill?)
>
>> the cpu runs until we return from the setup/start function.
>
> s/until we return/before we return/
>
>>
>> As there was a lot of duplicate code a common function for cpu
>
> s/code/code,/
>
>> restarts has been intropduced.
>
> s/intropduced/introduced/
>
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> ---
>> lib/s390x/smp.c | 45 ++++++++++++++++++++++++---------------------
>> 1 file changed, 24 insertions(+), 21 deletions(-)
>>
>> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
>> index f57f420..f984a34 100644
>> --- a/lib/s390x/smp.c
>> +++ b/lib/s390x/smp.c
>> @@ -104,35 +104,41 @@ int smp_cpu_stop_store_status(uint16_t addr)
>> return rc;
>> }
>>
>> +static int smp_cpu_restart_nolock(uint16_t addr, struct psw *psw)
>> +{
>> + int rc;
>> + struct cpu *cpu = smp_cpu_from_addr(addr);
>> +
>> + if (!cpu)
>> + return -1;
>> + if (psw) {
>> + cpu->lowcore->restart_new_psw.mask = psw->mask;
>> + cpu->lowcore->restart_new_psw.addr = psw->addr;
>> + }
>> + rc = sigp(addr, SIGP_RESTART, 0, NULL);
>> + if (rc)
>> + return rc;
>> + while (!smp_cpu_running(addr)) { mb(); }
>
> Maybe split this statement? Also, maybe add a comment
/* Wait until the target cpu is running */
?
This is not QEMU with two line ifs taking up 3 lines :)
>
> /*
> * The order has been accepted, but the actual restart may not
> * have been performed yet, so wait until the cpu is running.
> */
>
> ?
>
>> + cpu->active = true;
>> + return 0;
>> +}
>
> The changes look good to me AFAICS.
>
> Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
Thanks!
Attachment:
signature.asc
Description: OpenPGP digital signature
