Re: [PATCH v10 17/19] arm64: Kconfig: Enable hotplug CPU on arm64 if ACPI_PROCESSOR is enabled.

[Gavin Shan <gshan@redhat.com>]

Hi Catalin,

On 6/30/24 7:26 PM, Catalin Marinas wrote:
> On Sun, Jun 30, 2024 at 10:39:04AM +1000, Gavin Shan wrote:
>> On 5/29/24 11:34 PM, Jonathan Cameron wrote:
>>> In order to move arch_register_cpu() to be called via the same path
>>> for initially present CPUs described by ACPI and hotplugged CPUs
>>> ACPI_HOTPLUG_CPU needs to be enabled.
>>>
>>> The protection against invalid IDs in acpi_map_cpu() is needed as
>>> at least one production BIOS is in the wild which reports entries
>>> in DSDT (with no _STA method, so assumed enabled and present)
>>> that don't match MADT.
>>>
>>> Tested-by: Miguel Luis <miguel.luis@oracle.com>
>>> Reviewed-by: Gavin Shan <gshan@redhat.com>
>>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>> ---
>>>    arch/arm64/Kconfig       |  1 +
>>>    arch/arm64/kernel/acpi.c | 22 ++++++++++++++++++++++
>>>    2 files changed, 23 insertions(+)
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index 5d91259ee7b5..e8f2ef2312db 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -5,6 +5,7 @@ config ARM64
>>>    	select ACPI_CCA_REQUIRED if ACPI
>>>    	select ACPI_GENERIC_GSI if ACPI
>>>    	select ACPI_GTDT if ACPI
>>> +	select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR
>>
>> ACPI_HOTPLUG_CPU depends on (ACPI_PROCESSOR && HOTPLUG_CPU). It needs to be:
>>
>> 	select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU
>>
>> Otherwise, we can have compiling error with the following configurations.
>>
>> CONFIG_ACPI_PROCESSOR=y
>> CONFIG_HOTPLUG_CPU=n
>> CONFIG_ACPI_HOTPLUG_CPU=y
>>
>> arch/arm64/kernel/smp.c: In function ‘arch_unregister_cpu’:
>> arch/arm64/kernel/smp.c:563:9: error: implicit declaration of function ‘unregister_cpu’; did you mean ‘register_cpu’? [-Werror=implicit-function-declaration]
>>    563 |         unregister_cpu(c);
>>        |         ^~~~~~~~~~~~~~
>>        |         register_cpu
> 
> Ah, I thought that in addition to the warning for unmet dependencies,
> kbuild would also leave the option off. I need to add SUSPEND=n and
> HIBERNATE=n to my build scripts.
> 
> The fix matches what x86 does, so I'm ok with it.
> 

Ok, thanks for your confirm.

>> Since the series has been queued to Catalin's "for-next/vcpu-hotplug" branch, I
>> guess the easiest way would be to fix it in place with Catalin's help.
> 
> I wasn't planning to rebase the branch unless there's something major.
> Since the doesn't happen with defconfig, it's doesn't affect bisection
> that much, so my preference would be for a fix on top of this branch
> (and with a Fixes: tag since the branch is stable).
> 

Sure, I've posted a fix, applicable to branch 'for-next/vcpu-hotplug'.

https://lists.infradead.org/pipermail/linux-arm-kernel/2024-June/939690.html

Thanks,
Gavin