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

[Gavin Shan]

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@xxxxxxxxxx>
> > > Reviewed-by: Gavin Shan <gshan@xxxxxxxxxx>
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > > ---
> > >    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