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

[Julien Grall <julien.grall@arm.com>]

Hi Jan,

The title and commit message are a bit odd to read because you are modifying 
common code but everything is geared towards x86.

On 14/06/2019 12:37, Jan Beulich wrote:
> Aiui when resuming from S3, CPUs come back out of RESET/INIT. Therefore
> they need to undergo the same procedure as was added elsewhere by
> commits d8f974f1a6 ("x86: command line option to avoid use of secondary
> hyper-threads") and 8797d20a6e ("x86: possibly bring up all CPUs even
> if not all are supposed to be used").
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- 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? If so, could you please mention it in the commit message?

>           on_selected_cpus(cpumask_of(cpu), _take_cpu_down, NULL, true);
>       else if ( (err = stop_machine_run(take_cpu_down, NULL, cpu)) < 0 )
>           goto fail;
> @@ -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.

> +                               : !cpumask_test_cpu(cpu, &frozen_cpus) )
> +            continue;
>           if ( (error = cpu_up(cpu)) )
>           {
>               printk("Error bringing CPU%d up: %d\n", cpu, error);
>               BUG_ON(error == -EBUSY);
>           }
> -        else
> -            __cpumask_clear_cpu(cpu, &frozen_cpus);
> +        else if ( !__cpumask_test_and_clear_cpu(cpu, &frozen_cpus) &&
> +                  (error = cpu_down(cpu)) )
> +            printk("Error re-offlining CPU%d: %d\n", cpu, error);
>       }
>   
>       for_each_cpu ( cpu, &frozen_cpus )

Cheers,

-- 
Julien Grall

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