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

From: Julien Grall <julien.grall@arm.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>, WeiLiu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] [PATCH 1/3] x86/ACPI: re-park previously parked CPUs upon resume from S3
Date: Mon, 17 Jun 2019 09:12:55 +0100	[thread overview]
Message-ID: <0b1f2e30-e037-e21a-0d89-3aed35f4daa7@arm.com> (raw)
In-Reply-To: <5D0735D20200007800238B1B@prv1-mh.provo.novell.com>

Hi Jan,

On 6/17/19 7:40 AM, Jan Beulich wrote:
>>>> On 14.06.19 at 18:52, <julien.grall@arm.com> 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).

Bear in mind that I have nearly no knowledge of x86, so trying to write 
a commit message fully the x86 way is not going to help me understand 
why this makes sense for everyone (today or in the future).

>>> @@ -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. 

Calling and building are two separate things... A function may be built 
even if it is not called.

> 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".

Looking again, we are defining park_offline_cpus to false on Arm (see
a6448adfd3 "xen/cpu: Fix ARM build following c/s 597fbb8"). So there are 
no build issue as I first thought.

Cheers,

-- 
Julien Grall

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