[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/3] x86/ACPI: re-park previously parked CPUs upon resume from S3



>>> On 14.06.19 at 18:52, <julien.grall@xxxxxxx> wrote:
> The title and commit message are a bit odd to read because you are modifying 
> common code but everything is geared towards x86.

Indeed. There's no caller of {en,dis}able_nonboot_cpus() in Arm code
at present, afaics. Hence the code changed (but not the file) is truly
x86-specific at the moment. I've explicitly thought about the
"inconsistency" between title and contents, but I've deliberately put it
as is: The change _is_ x86 / ACPI only, _despite_ touching common
code (and hence needing a REST maintainer ack).

>> --- a/xen/common/cpu.c
>> +++ b/xen/common/cpu.c
>> @@ -105,7 +105,7 @@ int cpu_down(unsigned int cpu)
>>       if ( err )
>>           goto fail;
>>   
>> -    if ( unlikely(system_state < SYS_STATE_active) )
>> +    if ( system_state < SYS_STATE_active || system_state == 
>> SYS_STATE_resume )
> 
> So this change here is necessary because enable_nonboot_cpus() may call 
> cpu_down(), am I right?

Yes (albeit likely s/necessary/wanted/).

> If so, could you please mention it in the commit message?

Hmm, I could. But this is just paralleling what we're already doing for
the boot path, so it didn't seem imperative to me to call it out. But
anyway, I've added a sentence.

>> @@ -207,15 +207,19 @@ void enable_nonboot_cpus(void)
>>   
>>       printk("Enabling non-boot CPUs  ...\n");
>>   
>> -    for_each_cpu ( cpu, &frozen_cpus )
>> +    for_each_present_cpu ( cpu )
>>       {
>> +        if ( park_offline_cpus ? cpu == smp_processor_id()
> 
> park_offline_cpus is x86 specific, so it will not build on Arm.

And that's intentional, even more so that (as said above) Arm doesn't
call here in the first place. And even if it did - whether to do things the
"new" way would then still (intentionally) depend on whether Arm had
any way of park_offline_cpus being "true".

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.