[RFC,v3,05/21] ACPI: Rename ACPI_HOTPLUG_CPU to include 'present'

From: James Morse <james.morse@arm.com>

The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
present. This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
CPUs can be taken offline as a power saving measure.

On arm64 an offline CPU may be disabled by firmware, preventing it from
being brought back online, but it remains present throughout.

Adding code to prevent user-space trying to online these disabled CPUs
needs some additional terminology.

Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
that it makes possible CPUs present.

HOTPLUG_CPU is untouched as this is only about the ACPI mechanism.

Signed-off-by: James Morse <james.morse@arm.com>
Tested-by: Miguel Luis <miguel.luis@oracle.com>
Tested-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>
Tested-by: Jianyong Wu <jianyong.wu@arm.com>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
Changes since RFC v2:
 * Add Loongarch update
Changes since RFC v3:
 * Dropped ia64 changes
---
 arch/loongarch/Kconfig                     |  2 +-
 arch/loongarch/configs/loongson3_defconfig |  2 +-
 arch/loongarch/kernel/acpi.c               |  4 ++--
 arch/x86/Kconfig                           |  2 +-
 arch/x86/kernel/acpi/boot.c                |  4 ++--
 drivers/acpi/Kconfig                       |  4 ++--
 drivers/acpi/acpi_processor.c              | 10 +++++-----
 include/linux/acpi.h                       |  6 +++---
 8 files changed, 17 insertions(+), 17 deletions(-)

On Wed, 13 Dec 2023 12:49:37 +0000
Russell King (Oracle) <rmk+kernel@armlinux.org.uk> wrote:

> From: James Morse <james.morse@arm.com>
> 
> The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> present. This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> CPUs can be taken offline as a power saving measure.
> 
> On arm64 an offline CPU may be disabled by firmware, preventing it from
> being brought back online, but it remains present throughout.
> 
> Adding code to prevent user-space trying to online these disabled CPUs
> needs some additional terminology.
> 
> Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> that it makes possible CPUs present.
> 
> HOTPLUG_CPU is untouched as this is only about the ACPI mechanism.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Tested-by: Miguel Luis <miguel.luis@oracle.com>
> Tested-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>
> Tested-by: Jianyong Wu <jianyong.wu@arm.com>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Formatting nitpick inline. Either way FWIW:
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4db54e928b36..36071bc11acd 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h

>  #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
>  int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr);
> @@ -629,7 +629,7 @@ static inline u32 acpi_osc_ctx_get_cxl_control(struct acpi_osc_context *context)
>  #define ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS	0x0000000F
>  
>  /* Enable _OST when all relevant hotplug operations are enabled */
> -#if defined(CONFIG_ACPI_HOTPLUG_CPU) &&			\
> +#if defined(CONFIG_ACPI_HOTPLUG_PRESENT_CPU) &&			\

Trivial but I think there is a tab to many before that \

>  	defined(CONFIG_ACPI_HOTPLUG_MEMORY) &&		\
>  	defined(CONFIG_ACPI_CONTAINER)
>  #define ACPI_HOTPLUG_OST

On Thu, Dec 14, 2023 at 05:41:07PM +0000, Jonathan Cameron wrote:
> > Signed-off-by: James Morse <james.morse@arm.com>
> > Tested-by: Miguel Luis <miguel.luis@oracle.com>
> > Tested-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>
> > Tested-by: Jianyong Wu <jianyong.wu@arm.com>
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> Formatting nitpick inline. Either way FWIW:
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Thanks, but you're absolutely correct about the nitpick, so I've fixed
that too!

On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> From: James Morse <james.morse@arm.com>
>
> The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> present.

Right.

> This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> CPUs can be taken offline as a power saving measure.

But still there is the case in which a non-present CPU can become
present, isn't it there?

> On arm64 an offline CPU may be disabled by firmware, preventing it from
> being brought back online, but it remains present throughout.
>
> Adding code to prevent user-space trying to online these disabled CPUs
> needs some additional terminology.
>
> Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> that it makes possible CPUs present.

Honestly, I don't think that this change is necessary or even useful.

On Mon, 18 Dec 2023 21:35:16 +0100
"Rafael J. Wysocki" <rafael@kernel.org> wrote:

> On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > From: James Morse <james.morse@arm.com>
> >
> > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > present.  
> 
> Right.
> 
> > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > CPUs can be taken offline as a power saving measure.  
> 
> But still there is the case in which a non-present CPU can become
> present, isn't it there?

Not yet defined by the architectures (and I'm assuming it probably never will be).

The original proposal we took to ARM was to do exactly that - they pushed
back hard on the basis there was no architecturally safe way to implement it.
Too much of the ARM arch has to exist from the start of time.

https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
is one of the relevant threads of the kernel side of that discussion.

Not to put specific words into the ARM architects mouths, but the
short description is that there is currently no demand for working
out how to make physical CPU hotplug possible, as such they will not
provide an architecturally compliant way to do it for virtual CPU hotplug and
another means is needed (which is why this series doesn't use the present bit
for that purpose and we have the Online capable bit in MADT/GICC)

It was a 'fun' dance of several years to get to that clarification.
As another fun fact, the same is defined for x86, but I don't think
anyone has used it yet (GICC for ARM has an online capable bit in the flags to
enable this, which was remarkably similar to the online capable bit in the
flags of the Local APIC entries as added fairly recently).

> 
> > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > being brought back online, but it remains present throughout.
> >
> > Adding code to prevent user-space trying to online these disabled CPUs
> > needs some additional terminology.
> >
> > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > that it makes possible CPUs present.  
> 
> Honestly, I don't think that this change is necessary or even useful.

Whilst it's an attempt to avoid future confusion, the rename is
not something I really care about so my advice to Russell is drop
it unless you are attached to it!

Jonathan


> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Mon, Jan 22, 2024 at 06:00:13PM +0000, Jonathan Cameron wrote:
> On Mon, 18 Dec 2023 21:35:16 +0100
> "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> 
> > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > >
> > > From: James Morse <james.morse@arm.com>
> > >
> > > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > > present.  
> > 
> > Right.
> > 
> > > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > > CPUs can be taken offline as a power saving measure.  
> > 
> > But still there is the case in which a non-present CPU can become
> > present, isn't it there?
> 
> Not yet defined by the architectures (and I'm assuming it probably never will be).
> 
> The original proposal we took to ARM was to do exactly that - they pushed
> back hard on the basis there was no architecturally safe way to implement it.
> Too much of the ARM arch has to exist from the start of time.
> 
> https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
> is one of the relevant threads of the kernel side of that discussion.
> 
> Not to put specific words into the ARM architects mouths, but the
> short description is that there is currently no demand for working
> out how to make physical CPU hotplug possible, as such they will not
> provide an architecturally compliant way to do it for virtual CPU hotplug and
> another means is needed (which is why this series doesn't use the present bit
> for that purpose and we have the Online capable bit in MADT/GICC)
> 
> It was a 'fun' dance of several years to get to that clarification.
> As another fun fact, the same is defined for x86, but I don't think
> anyone has used it yet (GICC for ARM has an online capable bit in the flags to
> enable this, which was remarkably similar to the online capable bit in the
> flags of the Local APIC entries as added fairly recently).
> 
> > 
> > > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > > being brought back online, but it remains present throughout.
> > >
> > > Adding code to prevent user-space trying to online these disabled CPUs
> > > needs some additional terminology.
> > >
> > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > > that it makes possible CPUs present.  
> > 
> > Honestly, I don't think that this change is necessary or even useful.
> 
> Whilst it's an attempt to avoid future confusion, the rename is
> not something I really care about so my advice to Russell is drop
> it unless you are attached to it!

While I agree that it isn't a necessity, I don't fully agree that it
isn't useful.

One of the issues will be that while Arm64 will support hotplug vCPU,
it won't be setting ACPI_HOTPLUG_CPU because it doesn't support
the present bit changing. So I can see why James decided to rename
it - because with Arm64's hotplug vCPU, the idea that ACPI_HOTPLUG_CPU
somehow enables hotplug CPU support is now no longer true.

Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because it
leads one to assume that it ought to be enabled for Arm64's
implementatinon, and that could well cause issues in the future if
people make the assumption that "ACPI_HOTPLUG_CPU" means hotplug CPU
is supported in ACPI. It doesn't anymore.

On Tue, Jan 23, 2024 at 2:28 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Jan 22, 2024 at 06:00:13PM +0000, Jonathan Cameron wrote:
> > On Mon, 18 Dec 2023 21:35:16 +0100
> > "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> >
> > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > >
> > > > From: James Morse <james.morse@arm.com>
> > > >
> > > > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > > > present.
> > >
> > > Right.
> > >
> > > > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > > > CPUs can be taken offline as a power saving measure.
> > >
> > > But still there is the case in which a non-present CPU can become
> > > present, isn't it there?
> >
> > Not yet defined by the architectures (and I'm assuming it probably never will be).
> >
> > The original proposal we took to ARM was to do exactly that - they pushed
> > back hard on the basis there was no architecturally safe way to implement it.
> > Too much of the ARM arch has to exist from the start of time.
> >
> > https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
> > is one of the relevant threads of the kernel side of that discussion.
> >
> > Not to put specific words into the ARM architects mouths, but the
> > short description is that there is currently no demand for working
> > out how to make physical CPU hotplug possible, as such they will not
> > provide an architecturally compliant way to do it for virtual CPU hotplug and
> > another means is needed (which is why this series doesn't use the present bit
> > for that purpose and we have the Online capable bit in MADT/GICC)
> >
> > It was a 'fun' dance of several years to get to that clarification.
> > As another fun fact, the same is defined for x86, but I don't think
> > anyone has used it yet (GICC for ARM has an online capable bit in the flags to
> > enable this, which was remarkably similar to the online capable bit in the
> > flags of the Local APIC entries as added fairly recently).
> >
> > >
> > > > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > > > being brought back online, but it remains present throughout.
> > > >
> > > > Adding code to prevent user-space trying to online these disabled CPUs
> > > > needs some additional terminology.
> > > >
> > > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > > > that it makes possible CPUs present.
> > >
> > > Honestly, I don't think that this change is necessary or even useful.
> >
> > Whilst it's an attempt to avoid future confusion, the rename is
> > not something I really care about so my advice to Russell is drop
> > it unless you are attached to it!
>
> While I agree that it isn't a necessity, I don't fully agree that it
> isn't useful.
>
> One of the issues will be that while Arm64 will support hotplug vCPU,
> it won't be setting ACPI_HOTPLUG_CPU because it doesn't support
> the present bit changing. So I can see why James decided to rename
> it - because with Arm64's hotplug vCPU, the idea that ACPI_HOTPLUG_CPU
> somehow enables hotplug CPU support is now no longer true.
>
> Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because it
> leads one to assume that it ought to be enabled for Arm64's
> implementatinon, and that could well cause issues in the future if
> people make the assumption that "ACPI_HOTPLUG_CPU" means hotplug CPU
> is supported in ACPI. It doesn't anymore.

On x86 there is no confusion AFAICS.  It's always meant "as long as
the platform supports it".

On Tue, Jan 23, 2024 at 05:15:54PM +0100, Rafael J. Wysocki wrote:
> On Tue, Jan 23, 2024 at 2:28 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Mon, Jan 22, 2024 at 06:00:13PM +0000, Jonathan Cameron wrote:
> > > On Mon, 18 Dec 2023 21:35:16 +0100
> > > "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> > >
> > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > >
> > > > > From: James Morse <james.morse@arm.com>
> > > > >
> > > > > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > > > > present.
> > > >
> > > > Right.
> > > >
> > > > > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > > > > CPUs can be taken offline as a power saving measure.
> > > >
> > > > But still there is the case in which a non-present CPU can become
> > > > present, isn't it there?
> > >
> > > Not yet defined by the architectures (and I'm assuming it probably never will be).
> > >
> > > The original proposal we took to ARM was to do exactly that - they pushed
> > > back hard on the basis there was no architecturally safe way to implement it.
> > > Too much of the ARM arch has to exist from the start of time.
> > >
> > > https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
> > > is one of the relevant threads of the kernel side of that discussion.
> > >
> > > Not to put specific words into the ARM architects mouths, but the
> > > short description is that there is currently no demand for working
> > > out how to make physical CPU hotplug possible, as such they will not
> > > provide an architecturally compliant way to do it for virtual CPU hotplug and
> > > another means is needed (which is why this series doesn't use the present bit
> > > for that purpose and we have the Online capable bit in MADT/GICC)
> > >
> > > It was a 'fun' dance of several years to get to that clarification.
> > > As another fun fact, the same is defined for x86, but I don't think
> > > anyone has used it yet (GICC for ARM has an online capable bit in the flags to
> > > enable this, which was remarkably similar to the online capable bit in the
> > > flags of the Local APIC entries as added fairly recently).
> > >
> > > >
> > > > > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > > > > being brought back online, but it remains present throughout.
> > > > >
> > > > > Adding code to prevent user-space trying to online these disabled CPUs
> > > > > needs some additional terminology.
> > > > >
> > > > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > > > > that it makes possible CPUs present.
> > > >
> > > > Honestly, I don't think that this change is necessary or even useful.
> > >
> > > Whilst it's an attempt to avoid future confusion, the rename is
> > > not something I really care about so my advice to Russell is drop
> > > it unless you are attached to it!
> >
> > While I agree that it isn't a necessity, I don't fully agree that it
> > isn't useful.
> >
> > One of the issues will be that while Arm64 will support hotplug vCPU,
> > it won't be setting ACPI_HOTPLUG_CPU because it doesn't support
> > the present bit changing. So I can see why James decided to rename
> > it - because with Arm64's hotplug vCPU, the idea that ACPI_HOTPLUG_CPU
> > somehow enables hotplug CPU support is now no longer true.
> >
> > Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because it
> > leads one to assume that it ought to be enabled for Arm64's
> > implementatinon, and that could well cause issues in the future if
> > people make the assumption that "ACPI_HOTPLUG_CPU" means hotplug CPU
> > is supported in ACPI. It doesn't anymore.
> 
> On x86 there is no confusion AFAICS.  It's always meant "as long as
> the platform supports it".

That's x86, which supports physical CPU hotplug. We're introducing
support for Arm64 here which doesn't support physical CPU hotplug.

						ACPI-based	Physical	Virtual
Arch	HOTPLUG_CPU	ACPI_HOTPLUG_CPU	Hotplug		Hotplug		Hotplug
Arm64	Y		N			Y		N		Y
x86	Y		Y			Y		Y		Y

So ACPI_HOTPLUG_CPU becomes totally misnamed with the introduction
of hotplug on Arm64.

If we want to just look at stuff from an x86 perspective, then yes,
it remains correct to call it ACPI_HOTPLUG_CPU. It isn't correct as
soon as we add Arm64, as I already said.

And honestly, a two line quip to my reasoned argument is not IMHO
an acceptable reply.

... getting close to throwing the rag in over this.

On Tue, Jan 23, 2024 at 5:36 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Tue, Jan 23, 2024 at 05:15:54PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Jan 23, 2024 at 2:28 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Mon, Jan 22, 2024 at 06:00:13PM +0000, Jonathan Cameron wrote:
> > > > On Mon, 18 Dec 2023 21:35:16 +0100
> > > > "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> > > >
> > > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > > >
> > > > > > From: James Morse <james.morse@arm.com>
> > > > > >
> > > > > > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > > > > > present.
> > > > >
> > > > > Right.
> > > > >
> > > > > > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > > > > > CPUs can be taken offline as a power saving measure.
> > > > >
> > > > > But still there is the case in which a non-present CPU can become
> > > > > present, isn't it there?
> > > >
> > > > Not yet defined by the architectures (and I'm assuming it probably never will be).
> > > >
> > > > The original proposal we took to ARM was to do exactly that - they pushed
> > > > back hard on the basis there was no architecturally safe way to implement it.
> > > > Too much of the ARM arch has to exist from the start of time.
> > > >
> > > > https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
> > > > is one of the relevant threads of the kernel side of that discussion.
> > > >
> > > > Not to put specific words into the ARM architects mouths, but the
> > > > short description is that there is currently no demand for working
> > > > out how to make physical CPU hotplug possible, as such they will not
> > > > provide an architecturally compliant way to do it for virtual CPU hotplug and
> > > > another means is needed (which is why this series doesn't use the present bit
> > > > for that purpose and we have the Online capable bit in MADT/GICC)
> > > >
> > > > It was a 'fun' dance of several years to get to that clarification.
> > > > As another fun fact, the same is defined for x86, but I don't think
> > > > anyone has used it yet (GICC for ARM has an online capable bit in the flags to
> > > > enable this, which was remarkably similar to the online capable bit in the
> > > > flags of the Local APIC entries as added fairly recently).
> > > >
> > > > >
> > > > > > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > > > > > being brought back online, but it remains present throughout.
> > > > > >
> > > > > > Adding code to prevent user-space trying to online these disabled CPUs
> > > > > > needs some additional terminology.
> > > > > >
> > > > > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > > > > > that it makes possible CPUs present.
> > > > >
> > > > > Honestly, I don't think that this change is necessary or even useful.
> > > >
> > > > Whilst it's an attempt to avoid future confusion, the rename is
> > > > not something I really care about so my advice to Russell is drop
> > > > it unless you are attached to it!
> > >
> > > While I agree that it isn't a necessity, I don't fully agree that it
> > > isn't useful.
> > >
> > > One of the issues will be that while Arm64 will support hotplug vCPU,
> > > it won't be setting ACPI_HOTPLUG_CPU because it doesn't support
> > > the present bit changing. So I can see why James decided to rename
> > > it - because with Arm64's hotplug vCPU, the idea that ACPI_HOTPLUG_CPU
> > > somehow enables hotplug CPU support is now no longer true.
> > >
> > > Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because it
> > > leads one to assume that it ought to be enabled for Arm64's
> > > implementatinon, and that could well cause issues in the future if
> > > people make the assumption that "ACPI_HOTPLUG_CPU" means hotplug CPU
> > > is supported in ACPI. It doesn't anymore.
> >
> > On x86 there is no confusion AFAICS.  It's always meant "as long as
> > the platform supports it".
>
> That's x86, which supports physical CPU hotplug. We're introducing
> support for Arm64 here which doesn't support physical CPU hotplug.
>
>                                                 ACPI-based      Physical        Virtual
> Arch    HOTPLUG_CPU     ACPI_HOTPLUG_CPU        Hotplug         Hotplug         Hotplug
> Arm64   Y               N                       Y               N               Y
> x86     Y               Y                       Y               Y               Y
>
> So ACPI_HOTPLUG_CPU becomes totally misnamed with the introduction
> of hotplug on Arm64.
>
> If we want to just look at stuff from an x86 perspective, then yes,
> it remains correct to call it ACPI_HOTPLUG_CPU. It isn't correct as
> soon as we add Arm64, as I already said.

And if you rename it, it becomes less confusing for ARM64, but more
confusing for x86, which basically is my point.

IMO "hotplug" covers both cases well enough and "hotplug present" is
only accurate for one of them.

> And honestly, a two line quip to my reasoned argument is not IMHO
> an acceptable reply.

Well, I'm not even sure how to respond to this ...

On Tue, Jan 23, 2024 at 06:43:59PM +0100, Rafael J. Wysocki wrote:
> On Tue, Jan 23, 2024 at 5:36 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Tue, Jan 23, 2024 at 05:15:54PM +0100, Rafael J. Wysocki wrote:
> > > On Tue, Jan 23, 2024 at 2:28 PM Russell King (Oracle)
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Mon, Jan 22, 2024 at 06:00:13PM +0000, Jonathan Cameron wrote:
> > > > > On Mon, 18 Dec 2023 21:35:16 +0100
> > > > > "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> > > > >
> > > > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > > > >
> > > > > > > From: James Morse <james.morse@arm.com>
> > > > > > >
> > > > > > > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > > > > > > present.
> > > > > >
> > > > > > Right.
> > > > > >
> > > > > > > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > > > > > > CPUs can be taken offline as a power saving measure.
> > > > > >
> > > > > > But still there is the case in which a non-present CPU can become
> > > > > > present, isn't it there?
> > > > >
> > > > > Not yet defined by the architectures (and I'm assuming it probably never will be).
> > > > >
> > > > > The original proposal we took to ARM was to do exactly that - they pushed
> > > > > back hard on the basis there was no architecturally safe way to implement it.
> > > > > Too much of the ARM arch has to exist from the start of time.
> > > > >
> > > > > https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
> > > > > is one of the relevant threads of the kernel side of that discussion.
> > > > >
> > > > > Not to put specific words into the ARM architects mouths, but the
> > > > > short description is that there is currently no demand for working
> > > > > out how to make physical CPU hotplug possible, as such they will not
> > > > > provide an architecturally compliant way to do it for virtual CPU hotplug and
> > > > > another means is needed (which is why this series doesn't use the present bit
> > > > > for that purpose and we have the Online capable bit in MADT/GICC)
> > > > >
> > > > > It was a 'fun' dance of several years to get to that clarification.
> > > > > As another fun fact, the same is defined for x86, but I don't think
> > > > > anyone has used it yet (GICC for ARM has an online capable bit in the flags to
> > > > > enable this, which was remarkably similar to the online capable bit in the
> > > > > flags of the Local APIC entries as added fairly recently).
> > > > >
> > > > > >
> > > > > > > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > > > > > > being brought back online, but it remains present throughout.
> > > > > > >
> > > > > > > Adding code to prevent user-space trying to online these disabled CPUs
> > > > > > > needs some additional terminology.
> > > > > > >
> > > > > > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > > > > > > that it makes possible CPUs present.
> > > > > >
> > > > > > Honestly, I don't think that this change is necessary or even useful.
> > > > >
> > > > > Whilst it's an attempt to avoid future confusion, the rename is
> > > > > not something I really care about so my advice to Russell is drop
> > > > > it unless you are attached to it!
> > > >
> > > > While I agree that it isn't a necessity, I don't fully agree that it
> > > > isn't useful.
> > > >
> > > > One of the issues will be that while Arm64 will support hotplug vCPU,
> > > > it won't be setting ACPI_HOTPLUG_CPU because it doesn't support
> > > > the present bit changing. So I can see why James decided to rename
> > > > it - because with Arm64's hotplug vCPU, the idea that ACPI_HOTPLUG_CPU
> > > > somehow enables hotplug CPU support is now no longer true.
> > > >
> > > > Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because it
> > > > leads one to assume that it ought to be enabled for Arm64's
> > > > implementatinon, and that could well cause issues in the future if
> > > > people make the assumption that "ACPI_HOTPLUG_CPU" means hotplug CPU
> > > > is supported in ACPI. It doesn't anymore.
> > >
> > > On x86 there is no confusion AFAICS.  It's always meant "as long as
> > > the platform supports it".
> >
> > That's x86, which supports physical CPU hotplug. We're introducing
> > support for Arm64 here which doesn't support physical CPU hotplug.
> >
> >                                                 ACPI-based      Physical        Virtual
> > Arch    HOTPLUG_CPU     ACPI_HOTPLUG_CPU        Hotplug         Hotplug         Hotplug
> > Arm64   Y               N                       Y               N               Y
> > x86     Y               Y                       Y               Y               Y
> >
> > So ACPI_HOTPLUG_CPU becomes totally misnamed with the introduction
> > of hotplug on Arm64.
> >
> > If we want to just look at stuff from an x86 perspective, then yes,
> > it remains correct to call it ACPI_HOTPLUG_CPU. It isn't correct as
> > soon as we add Arm64, as I already said.
> 
> And if you rename it, it becomes less confusing for ARM64, but more
> confusing for x86, which basically is my point.
> 
> IMO "hotplug" covers both cases well enough and "hotplug present" is
> only accurate for one of them.
> 
> > And honestly, a two line quip to my reasoned argument is not IMHO
> > an acceptable reply.
> 
> Well, I'm not even sure how to respond to this ...

The above explanation you give would have been useful...

I don't see how "hotplug" covers both cases. As I've tried to point
out many times now, ACPI_HOTPLUG_CPU is N for Arm64, yet it supports
ACPI based hotplug. How does ACPI_HOTPLUG_CPU cover Arm64 if it's
N there? IMHO it totally doesn't, and moreover, it goes against what
one would logically expect - and this is why I have a problem with
your effective NAK for this change. I believe you are basically
wrong on this for the reasons I've given - that ACPI_HOTPLUG_CPU
will be N for Arm64 despite it supporting ACPI-based CPU hotplug.

On Tue, Jan 23, 2024 at 7:20 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Tue, Jan 23, 2024 at 06:43:59PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Jan 23, 2024 at 5:36 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Tue, Jan 23, 2024 at 05:15:54PM +0100, Rafael J. Wysocki wrote:
> > > > On Tue, Jan 23, 2024 at 2:28 PM Russell King (Oracle)
> > > > <linux@armlinux.org.uk> wrote:
> > > > >
> > > > > On Mon, Jan 22, 2024 at 06:00:13PM +0000, Jonathan Cameron wrote:
> > > > > > On Mon, 18 Dec 2023 21:35:16 +0100
> > > > > > "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> > > > > >
> > > > > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > > > > >
> > > > > > > > From: James Morse <james.morse@arm.com>
> > > > > > > >
> > > > > > > > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > > > > > > > present.
> > > > > > >
> > > > > > > Right.
> > > > > > >
> > > > > > > > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > > > > > > > CPUs can be taken offline as a power saving measure.
> > > > > > >
> > > > > > > But still there is the case in which a non-present CPU can become
> > > > > > > present, isn't it there?
> > > > > >
> > > > > > Not yet defined by the architectures (and I'm assuming it probably never will be).
> > > > > >
> > > > > > The original proposal we took to ARM was to do exactly that - they pushed
> > > > > > back hard on the basis there was no architecturally safe way to implement it.
> > > > > > Too much of the ARM arch has to exist from the start of time.
> > > > > >
> > > > > > https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
> > > > > > is one of the relevant threads of the kernel side of that discussion.
> > > > > >
> > > > > > Not to put specific words into the ARM architects mouths, but the
> > > > > > short description is that there is currently no demand for working
> > > > > > out how to make physical CPU hotplug possible, as such they will not
> > > > > > provide an architecturally compliant way to do it for virtual CPU hotplug and
> > > > > > another means is needed (which is why this series doesn't use the present bit
> > > > > > for that purpose and we have the Online capable bit in MADT/GICC)
> > > > > >
> > > > > > It was a 'fun' dance of several years to get to that clarification.
> > > > > > As another fun fact, the same is defined for x86, but I don't think
> > > > > > anyone has used it yet (GICC for ARM has an online capable bit in the flags to
> > > > > > enable this, which was remarkably similar to the online capable bit in the
> > > > > > flags of the Local APIC entries as added fairly recently).
> > > > > >
> > > > > > >
> > > > > > > > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > > > > > > > being brought back online, but it remains present throughout.
> > > > > > > >
> > > > > > > > Adding code to prevent user-space trying to online these disabled CPUs
> > > > > > > > needs some additional terminology.
> > > > > > > >
> > > > > > > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > > > > > > > that it makes possible CPUs present.
> > > > > > >
> > > > > > > Honestly, I don't think that this change is necessary or even useful.
> > > > > >
> > > > > > Whilst it's an attempt to avoid future confusion, the rename is
> > > > > > not something I really care about so my advice to Russell is drop
> > > > > > it unless you are attached to it!
> > > > >
> > > > > While I agree that it isn't a necessity, I don't fully agree that it
> > > > > isn't useful.
> > > > >
> > > > > One of the issues will be that while Arm64 will support hotplug vCPU,
> > > > > it won't be setting ACPI_HOTPLUG_CPU because it doesn't support
> > > > > the present bit changing. So I can see why James decided to rename
> > > > > it - because with Arm64's hotplug vCPU, the idea that ACPI_HOTPLUG_CPU
> > > > > somehow enables hotplug CPU support is now no longer true.
> > > > >
> > > > > Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because it
> > > > > leads one to assume that it ought to be enabled for Arm64's
> > > > > implementatinon, and that could well cause issues in the future if
> > > > > people make the assumption that "ACPI_HOTPLUG_CPU" means hotplug CPU
> > > > > is supported in ACPI. It doesn't anymore.
> > > >
> > > > On x86 there is no confusion AFAICS.  It's always meant "as long as
> > > > the platform supports it".
> > >
> > > That's x86, which supports physical CPU hotplug. We're introducing
> > > support for Arm64 here which doesn't support physical CPU hotplug.
> > >
> > >                                                 ACPI-based      Physical        Virtual
> > > Arch    HOTPLUG_CPU     ACPI_HOTPLUG_CPU        Hotplug         Hotplug         Hotplug
> > > Arm64   Y               N                       Y               N               Y
> > > x86     Y               Y                       Y               Y               Y
> > >
> > > So ACPI_HOTPLUG_CPU becomes totally misnamed with the introduction
> > > of hotplug on Arm64.
> > >
> > > If we want to just look at stuff from an x86 perspective, then yes,
> > > it remains correct to call it ACPI_HOTPLUG_CPU. It isn't correct as
> > > soon as we add Arm64, as I already said.
> >
> > And if you rename it, it becomes less confusing for ARM64, but more
> > confusing for x86, which basically is my point.
> >
> > IMO "hotplug" covers both cases well enough and "hotplug present" is
> > only accurate for one of them.
> >
> > > And honestly, a two line quip to my reasoned argument is not IMHO
> > > an acceptable reply.
> >
> > Well, I'm not even sure how to respond to this ...
>
> The above explanation you give would have been useful...
>
> I don't see how "hotplug" covers both cases. As I've tried to point
> out many times now, ACPI_HOTPLUG_CPU is N for Arm64, yet it supports
> ACPI based hotplug. How does ACPI_HOTPLUG_CPU cover Arm64 if it's
> N there?

But IIUC this change is preliminary for changing it (or equivalent
option with a different name) to Y, isn't it?

> IMHO it totally doesn't, and moreover, it goes against what
> one would logically expect - and this is why I have a problem with
> your effective NAK for this change. I believe you are basically
> wrong on this for the reasons I've given - that ACPI_HOTPLUG_CPU
> will be N for Arm64 despite it supporting ACPI-based CPU hotplug.

So I still have to understand how renaming it for all architectures
(including x86) is supposed to help.

It will still be the same option under a different name.  How does
that change things technically?

On Tue, Jan 23, 2024 at 07:26:57PM +0100, Rafael J. Wysocki wrote:
> On Tue, Jan 23, 2024 at 7:20 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Tue, Jan 23, 2024 at 06:43:59PM +0100, Rafael J. Wysocki wrote:
> > > On Tue, Jan 23, 2024 at 5:36 PM Russell King (Oracle)
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Tue, Jan 23, 2024 at 05:15:54PM +0100, Rafael J. Wysocki wrote:
> > > > > On Tue, Jan 23, 2024 at 2:28 PM Russell King (Oracle)
> > > > > <linux@armlinux.org.uk> wrote:
> > > > > >
> > > > > > On Mon, Jan 22, 2024 at 06:00:13PM +0000, Jonathan Cameron wrote:
> > > > > > > On Mon, 18 Dec 2023 21:35:16 +0100
> > > > > > > "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> > > > > > >
> > > > > > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > > > > > >
> > > > > > > > > From: James Morse <james.morse@arm.com>
> > > > > > > > >
> > > > > > > > > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > > > > > > > > present.
> > > > > > > >
> > > > > > > > Right.
> > > > > > > >
> > > > > > > > > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > > > > > > > > CPUs can be taken offline as a power saving measure.
> > > > > > > >
> > > > > > > > But still there is the case in which a non-present CPU can become
> > > > > > > > present, isn't it there?
> > > > > > >
> > > > > > > Not yet defined by the architectures (and I'm assuming it probably never will be).
> > > > > > >
> > > > > > > The original proposal we took to ARM was to do exactly that - they pushed
> > > > > > > back hard on the basis there was no architecturally safe way to implement it.
> > > > > > > Too much of the ARM arch has to exist from the start of time.
> > > > > > >
> > > > > > > https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
> > > > > > > is one of the relevant threads of the kernel side of that discussion.
> > > > > > >
> > > > > > > Not to put specific words into the ARM architects mouths, but the
> > > > > > > short description is that there is currently no demand for working
> > > > > > > out how to make physical CPU hotplug possible, as such they will not
> > > > > > > provide an architecturally compliant way to do it for virtual CPU hotplug and
> > > > > > > another means is needed (which is why this series doesn't use the present bit
> > > > > > > for that purpose and we have the Online capable bit in MADT/GICC)
> > > > > > >
> > > > > > > It was a 'fun' dance of several years to get to that clarification.
> > > > > > > As another fun fact, the same is defined for x86, but I don't think
> > > > > > > anyone has used it yet (GICC for ARM has an online capable bit in the flags to
> > > > > > > enable this, which was remarkably similar to the online capable bit in the
> > > > > > > flags of the Local APIC entries as added fairly recently).
> > > > > > >
> > > > > > > >
> > > > > > > > > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > > > > > > > > being brought back online, but it remains present throughout.
> > > > > > > > >
> > > > > > > > > Adding code to prevent user-space trying to online these disabled CPUs
> > > > > > > > > needs some additional terminology.
> > > > > > > > >
> > > > > > > > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > > > > > > > > that it makes possible CPUs present.
> > > > > > > >
> > > > > > > > Honestly, I don't think that this change is necessary or even useful.
> > > > > > >
> > > > > > > Whilst it's an attempt to avoid future confusion, the rename is
> > > > > > > not something I really care about so my advice to Russell is drop
> > > > > > > it unless you are attached to it!
> > > > > >
> > > > > > While I agree that it isn't a necessity, I don't fully agree that it
> > > > > > isn't useful.
> > > > > >
> > > > > > One of the issues will be that while Arm64 will support hotplug vCPU,
> > > > > > it won't be setting ACPI_HOTPLUG_CPU because it doesn't support
> > > > > > the present bit changing. So I can see why James decided to rename
> > > > > > it - because with Arm64's hotplug vCPU, the idea that ACPI_HOTPLUG_CPU
> > > > > > somehow enables hotplug CPU support is now no longer true.
> > > > > >
> > > > > > Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because it
> > > > > > leads one to assume that it ought to be enabled for Arm64's
> > > > > > implementatinon, and that could well cause issues in the future if
> > > > > > people make the assumption that "ACPI_HOTPLUG_CPU" means hotplug CPU
> > > > > > is supported in ACPI. It doesn't anymore.
> > > > >
> > > > > On x86 there is no confusion AFAICS.  It's always meant "as long as
> > > > > the platform supports it".
> > > >
> > > > That's x86, which supports physical CPU hotplug. We're introducing
> > > > support for Arm64 here which doesn't support physical CPU hotplug.
> > > >
> > > >                                                 ACPI-based      Physical        Virtual
> > > > Arch    HOTPLUG_CPU     ACPI_HOTPLUG_CPU        Hotplug         Hotplug         Hotplug
> > > > Arm64   Y               N                       Y               N               Y
> > > > x86     Y               Y                       Y               Y               Y
> > > >
> > > > So ACPI_HOTPLUG_CPU becomes totally misnamed with the introduction
> > > > of hotplug on Arm64.
> > > >
> > > > If we want to just look at stuff from an x86 perspective, then yes,
> > > > it remains correct to call it ACPI_HOTPLUG_CPU. It isn't correct as
> > > > soon as we add Arm64, as I already said.
> > >
> > > And if you rename it, it becomes less confusing for ARM64, but more
> > > confusing for x86, which basically is my point.
> > >
> > > IMO "hotplug" covers both cases well enough and "hotplug present" is
> > > only accurate for one of them.
> > >
> > > > And honestly, a two line quip to my reasoned argument is not IMHO
> > > > an acceptable reply.
> > >
> > > Well, I'm not even sure how to respond to this ...
> >
> > The above explanation you give would have been useful...
> >
> > I don't see how "hotplug" covers both cases. As I've tried to point
> > out many times now, ACPI_HOTPLUG_CPU is N for Arm64, yet it supports
> > ACPI based hotplug. How does ACPI_HOTPLUG_CPU cover Arm64 if it's
> > N there?
> 
> But IIUC this change is preliminary for changing it (or equivalent
> option with a different name) to Y, isn't it?

No. As I keep saying, ACPI_HOTPLUG_CPU ends up N on Arm64 even when
it supports hotplug CPU via ACPI.

Even with the full Arm64 patch set here, under arch/ we still only
have:

arch/loongarch/Kconfig: select ACPI_HOTPLUG_PRESENT_CPU if ACPI_PROCESSOR && HOTPLUG_CPU
arch/x86/Kconfig:       select ACPI_HOTPLUG_PRESENT_CPU         if ACPI_PROCESSOR && HOTPLUG_CPU

To say it yet again, ACPI_HOTPLUG_(PRESENT_)CPU is *never* set on
Arm64.

> > IMHO it totally doesn't, and moreover, it goes against what
> > one would logically expect - and this is why I have a problem with
> > your effective NAK for this change. I believe you are basically
> > wrong on this for the reasons I've given - that ACPI_HOTPLUG_CPU
> > will be N for Arm64 despite it supporting ACPI-based CPU hotplug.
> 
> So I still have to understand how renaming it for all architectures
> (including x86) is supposed to help.
> 
> It will still be the same option under a different name.  How does
> that change things technically?

Do you think that it makes any sense to have support for ACPI-based
hotplug CPU *and* having it functional with a configuration symbol
named "ACPI_HOTPLUG_CPU" to be set to N ? That's essentially what
you are advocating for...

On Tue, Jan 23, 2024 at 7:59 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Tue, Jan 23, 2024 at 07:26:57PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Jan 23, 2024 at 7:20 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Tue, Jan 23, 2024 at 06:43:59PM +0100, Rafael J. Wysocki wrote:
> > > > On Tue, Jan 23, 2024 at 5:36 PM Russell King (Oracle)
> > > > <linux@armlinux.org.uk> wrote:
> > > > >
> > > > > On Tue, Jan 23, 2024 at 05:15:54PM +0100, Rafael J. Wysocki wrote:
> > > > > > On Tue, Jan 23, 2024 at 2:28 PM Russell King (Oracle)
> > > > > > <linux@armlinux.org.uk> wrote:
> > > > > > >
> > > > > > > On Mon, Jan 22, 2024 at 06:00:13PM +0000, Jonathan Cameron wrote:
> > > > > > > > On Mon, 18 Dec 2023 21:35:16 +0100
> > > > > > > > "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> > > > > > > >
> > > > > > > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > > > > > > >
> > > > > > > > > > From: James Morse <james.morse@arm.com>
> > > > > > > > > >
> > > > > > > > > > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > > > > > > > > > present.
> > > > > > > > >
> > > > > > > > > Right.
> > > > > > > > >
> > > > > > > > > > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > > > > > > > > > CPUs can be taken offline as a power saving measure.
> > > > > > > > >
> > > > > > > > > But still there is the case in which a non-present CPU can become
> > > > > > > > > present, isn't it there?
> > > > > > > >
> > > > > > > > Not yet defined by the architectures (and I'm assuming it probably never will be).
> > > > > > > >
> > > > > > > > The original proposal we took to ARM was to do exactly that - they pushed
> > > > > > > > back hard on the basis there was no architecturally safe way to implement it.
> > > > > > > > Too much of the ARM arch has to exist from the start of time.
> > > > > > > >
> > > > > > > > https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
> > > > > > > > is one of the relevant threads of the kernel side of that discussion.
> > > > > > > >
> > > > > > > > Not to put specific words into the ARM architects mouths, but the
> > > > > > > > short description is that there is currently no demand for working
> > > > > > > > out how to make physical CPU hotplug possible, as such they will not
> > > > > > > > provide an architecturally compliant way to do it for virtual CPU hotplug and
> > > > > > > > another means is needed (which is why this series doesn't use the present bit
> > > > > > > > for that purpose and we have the Online capable bit in MADT/GICC)
> > > > > > > >
> > > > > > > > It was a 'fun' dance of several years to get to that clarification.
> > > > > > > > As another fun fact, the same is defined for x86, but I don't think
> > > > > > > > anyone has used it yet (GICC for ARM has an online capable bit in the flags to
> > > > > > > > enable this, which was remarkably similar to the online capable bit in the
> > > > > > > > flags of the Local APIC entries as added fairly recently).
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > > > > > > > > > being brought back online, but it remains present throughout.
> > > > > > > > > >
> > > > > > > > > > Adding code to prevent user-space trying to online these disabled CPUs
> > > > > > > > > > needs some additional terminology.
> > > > > > > > > >
> > > > > > > > > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > > > > > > > > > that it makes possible CPUs present.
> > > > > > > > >
> > > > > > > > > Honestly, I don't think that this change is necessary or even useful.
> > > > > > > >
> > > > > > > > Whilst it's an attempt to avoid future confusion, the rename is
> > > > > > > > not something I really care about so my advice to Russell is drop
> > > > > > > > it unless you are attached to it!
> > > > > > >
> > > > > > > While I agree that it isn't a necessity, I don't fully agree that it
> > > > > > > isn't useful.
> > > > > > >
> > > > > > > One of the issues will be that while Arm64 will support hotplug vCPU,
> > > > > > > it won't be setting ACPI_HOTPLUG_CPU because it doesn't support
> > > > > > > the present bit changing. So I can see why James decided to rename
> > > > > > > it - because with Arm64's hotplug vCPU, the idea that ACPI_HOTPLUG_CPU
> > > > > > > somehow enables hotplug CPU support is now no longer true.
> > > > > > >
> > > > > > > Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because it
> > > > > > > leads one to assume that it ought to be enabled for Arm64's
> > > > > > > implementatinon, and that could well cause issues in the future if
> > > > > > > people make the assumption that "ACPI_HOTPLUG_CPU" means hotplug CPU
> > > > > > > is supported in ACPI. It doesn't anymore.
> > > > > >
> > > > > > On x86 there is no confusion AFAICS.  It's always meant "as long as
> > > > > > the platform supports it".
> > > > >
> > > > > That's x86, which supports physical CPU hotplug. We're introducing
> > > > > support for Arm64 here which doesn't support physical CPU hotplug.
> > > > >
> > > > >                                                 ACPI-based      Physical        Virtual
> > > > > Arch    HOTPLUG_CPU     ACPI_HOTPLUG_CPU        Hotplug         Hotplug         Hotplug
> > > > > Arm64   Y               N                       Y               N               Y
> > > > > x86     Y               Y                       Y               Y               Y
> > > > >
> > > > > So ACPI_HOTPLUG_CPU becomes totally misnamed with the introduction
> > > > > of hotplug on Arm64.
> > > > >
> > > > > If we want to just look at stuff from an x86 perspective, then yes,
> > > > > it remains correct to call it ACPI_HOTPLUG_CPU. It isn't correct as
> > > > > soon as we add Arm64, as I already said.
> > > >
> > > > And if you rename it, it becomes less confusing for ARM64, but more
> > > > confusing for x86, which basically is my point.
> > > >
> > > > IMO "hotplug" covers both cases well enough and "hotplug present" is
> > > > only accurate for one of them.
> > > >
> > > > > And honestly, a two line quip to my reasoned argument is not IMHO
> > > > > an acceptable reply.
> > > >
> > > > Well, I'm not even sure how to respond to this ...
> > >
> > > The above explanation you give would have been useful...
> > >
> > > I don't see how "hotplug" covers both cases. As I've tried to point
> > > out many times now, ACPI_HOTPLUG_CPU is N for Arm64, yet it supports
> > > ACPI based hotplug. How does ACPI_HOTPLUG_CPU cover Arm64 if it's
> > > N there?
> >
> > But IIUC this change is preliminary for changing it (or equivalent
> > option with a different name) to Y, isn't it?
>
> No. As I keep saying, ACPI_HOTPLUG_CPU ends up N on Arm64 even when
> it supports hotplug CPU via ACPI.
>
> Even with the full Arm64 patch set here, under arch/ we still only
> have:
>
> arch/loongarch/Kconfig: select ACPI_HOTPLUG_PRESENT_CPU if ACPI_PROCESSOR && HOTPLUG_CPU
> arch/x86/Kconfig:       select ACPI_HOTPLUG_PRESENT_CPU         if ACPI_PROCESSOR && HOTPLUG_CPU
>
> To say it yet again, ACPI_HOTPLUG_(PRESENT_)CPU is *never* set on
> Arm64.

Allright, so ARM64 is not going to use the code that is conditional on
ACPI_HOTPLUG_CPU today.

Fair enough.

> > > IMHO it totally doesn't, and moreover, it goes against what
> > > one would logically expect - and this is why I have a problem with
> > > your effective NAK for this change. I believe you are basically
> > > wrong on this for the reasons I've given - that ACPI_HOTPLUG_CPU
> > > will be N for Arm64 despite it supporting ACPI-based CPU hotplug.
> >
> > So I still have to understand how renaming it for all architectures
> > (including x86) is supposed to help.
> >
> > It will still be the same option under a different name.  How does
> > that change things technically?
>
> Do you think that it makes any sense to have support for ACPI-based
> hotplug CPU

So this is all about what you and I mean by "ACPI-based hotplug CPU".

> *and* having it functional with a configuration symbol
> named "ACPI_HOTPLUG_CPU" to be set to N ? That's essentially what
> you are advocating for...

Setting ACPI_HOTPLUG_CPU to N means that you are not going to compile
the code that is conditional on it.

That code allows the processor driver to be removed from CPUs and
arch_unregister_cpu() to be called from within acpi_bus_trim()  (among
other things).  On the way up, it allows arch_register_cpu() to be
called from within acpi_bus_scan().  If these things are not done,
what I mean by "ACPI-based hotplug CPU" is not supported.

On Tue, Jan 23, 2024 at 08:27:05PM +0100, Rafael J. Wysocki wrote:
> On Tue, Jan 23, 2024 at 7:59 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Tue, Jan 23, 2024 at 07:26:57PM +0100, Rafael J. Wysocki wrote:
> > > On Tue, Jan 23, 2024 at 7:20 PM Russell King (Oracle)
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Tue, Jan 23, 2024 at 06:43:59PM +0100, Rafael J. Wysocki wrote:
> > > > > On Tue, Jan 23, 2024 at 5:36 PM Russell King (Oracle)
> > > > > <linux@armlinux.org.uk> wrote:
> > > > > >
> > > > > > On Tue, Jan 23, 2024 at 05:15:54PM +0100, Rafael J. Wysocki wrote:
> > > > > > > On Tue, Jan 23, 2024 at 2:28 PM Russell King (Oracle)
> > > > > > > <linux@armlinux.org.uk> wrote:
> > > > > > > >
> > > > > > > > On Mon, Jan 22, 2024 at 06:00:13PM +0000, Jonathan Cameron wrote:
> > > > > > > > > On Mon, 18 Dec 2023 21:35:16 +0100
> > > > > > > > > "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> > > > > > > > >
> > > > > > > > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > > > > > > > >
> > > > > > > > > > > From: James Morse <james.morse@arm.com>
> > > > > > > > > > >
> > > > > > > > > > > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > > > > > > > > > > present.
> > > > > > > > > >
> > > > > > > > > > Right.
> > > > > > > > > >
> > > > > > > > > > > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > > > > > > > > > > CPUs can be taken offline as a power saving measure.
> > > > > > > > > >
> > > > > > > > > > But still there is the case in which a non-present CPU can become
> > > > > > > > > > present, isn't it there?
> > > > > > > > >
> > > > > > > > > Not yet defined by the architectures (and I'm assuming it probably never will be).
> > > > > > > > >
> > > > > > > > > The original proposal we took to ARM was to do exactly that - they pushed
> > > > > > > > > back hard on the basis there was no architecturally safe way to implement it.
> > > > > > > > > Too much of the ARM arch has to exist from the start of time.
> > > > > > > > >
> > > > > > > > > https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
> > > > > > > > > is one of the relevant threads of the kernel side of that discussion.
> > > > > > > > >
> > > > > > > > > Not to put specific words into the ARM architects mouths, but the
> > > > > > > > > short description is that there is currently no demand for working
> > > > > > > > > out how to make physical CPU hotplug possible, as such they will not
> > > > > > > > > provide an architecturally compliant way to do it for virtual CPU hotplug and
> > > > > > > > > another means is needed (which is why this series doesn't use the present bit
> > > > > > > > > for that purpose and we have the Online capable bit in MADT/GICC)
> > > > > > > > >
> > > > > > > > > It was a 'fun' dance of several years to get to that clarification.
> > > > > > > > > As another fun fact, the same is defined for x86, but I don't think
> > > > > > > > > anyone has used it yet (GICC for ARM has an online capable bit in the flags to
> > > > > > > > > enable this, which was remarkably similar to the online capable bit in the
> > > > > > > > > flags of the Local APIC entries as added fairly recently).
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > > > > > > > > > > being brought back online, but it remains present throughout.
> > > > > > > > > > >
> > > > > > > > > > > Adding code to prevent user-space trying to online these disabled CPUs
> > > > > > > > > > > needs some additional terminology.
> > > > > > > > > > >
> > > > > > > > > > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > > > > > > > > > > that it makes possible CPUs present.
> > > > > > > > > >
> > > > > > > > > > Honestly, I don't think that this change is necessary or even useful.
> > > > > > > > >
> > > > > > > > > Whilst it's an attempt to avoid future confusion, the rename is
> > > > > > > > > not something I really care about so my advice to Russell is drop
> > > > > > > > > it unless you are attached to it!
> > > > > > > >
> > > > > > > > While I agree that it isn't a necessity, I don't fully agree that it
> > > > > > > > isn't useful.
> > > > > > > >
> > > > > > > > One of the issues will be that while Arm64 will support hotplug vCPU,
> > > > > > > > it won't be setting ACPI_HOTPLUG_CPU because it doesn't support
> > > > > > > > the present bit changing. So I can see why James decided to rename
> > > > > > > > it - because with Arm64's hotplug vCPU, the idea that ACPI_HOTPLUG_CPU
> > > > > > > > somehow enables hotplug CPU support is now no longer true.
> > > > > > > >
> > > > > > > > Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because it
> > > > > > > > leads one to assume that it ought to be enabled for Arm64's
> > > > > > > > implementatinon, and that could well cause issues in the future if
> > > > > > > > people make the assumption that "ACPI_HOTPLUG_CPU" means hotplug CPU
> > > > > > > > is supported in ACPI. It doesn't anymore.
> > > > > > >
> > > > > > > On x86 there is no confusion AFAICS.  It's always meant "as long as
> > > > > > > the platform supports it".
> > > > > >
> > > > > > That's x86, which supports physical CPU hotplug. We're introducing
> > > > > > support for Arm64 here which doesn't support physical CPU hotplug.
> > > > > >
> > > > > >                                                 ACPI-based      Physical        Virtual
> > > > > > Arch    HOTPLUG_CPU     ACPI_HOTPLUG_CPU        Hotplug         Hotplug         Hotplug
> > > > > > Arm64   Y               N                       Y               N               Y
> > > > > > x86     Y               Y                       Y               Y               Y
> > > > > >
> > > > > > So ACPI_HOTPLUG_CPU becomes totally misnamed with the introduction
> > > > > > of hotplug on Arm64.
> > > > > >
> > > > > > If we want to just look at stuff from an x86 perspective, then yes,
> > > > > > it remains correct to call it ACPI_HOTPLUG_CPU. It isn't correct as
> > > > > > soon as we add Arm64, as I already said.
> > > > >
> > > > > And if you rename it, it becomes less confusing for ARM64, but more
> > > > > confusing for x86, which basically is my point.
> > > > >
> > > > > IMO "hotplug" covers both cases well enough and "hotplug present" is
> > > > > only accurate for one of them.
> > > > >
> > > > > > And honestly, a two line quip to my reasoned argument is not IMHO
> > > > > > an acceptable reply.
> > > > >
> > > > > Well, I'm not even sure how to respond to this ...
> > > >
> > > > The above explanation you give would have been useful...
> > > >
> > > > I don't see how "hotplug" covers both cases. As I've tried to point
> > > > out many times now, ACPI_HOTPLUG_CPU is N for Arm64, yet it supports
> > > > ACPI based hotplug. How does ACPI_HOTPLUG_CPU cover Arm64 if it's
> > > > N there?
> > >
> > > But IIUC this change is preliminary for changing it (or equivalent
> > > option with a different name) to Y, isn't it?
> >
> > No. As I keep saying, ACPI_HOTPLUG_CPU ends up N on Arm64 even when
> > it supports hotplug CPU via ACPI.
> >
> > Even with the full Arm64 patch set here, under arch/ we still only
> > have:
> >
> > arch/loongarch/Kconfig: select ACPI_HOTPLUG_PRESENT_CPU if ACPI_PROCESSOR && HOTPLUG_CPU
> > arch/x86/Kconfig:       select ACPI_HOTPLUG_PRESENT_CPU         if ACPI_PROCESSOR && HOTPLUG_CPU
> >
> > To say it yet again, ACPI_HOTPLUG_(PRESENT_)CPU is *never* set on
> > Arm64.
> 
> Allright, so ARM64 is not going to use the code that is conditional on
> ACPI_HOTPLUG_CPU today.
> 
> Fair enough.
> 
> > > > IMHO it totally doesn't, and moreover, it goes against what
> > > > one would logically expect - and this is why I have a problem with
> > > > your effective NAK for this change. I believe you are basically
> > > > wrong on this for the reasons I've given - that ACPI_HOTPLUG_CPU
> > > > will be N for Arm64 despite it supporting ACPI-based CPU hotplug.
> > >
> > > So I still have to understand how renaming it for all architectures
> > > (including x86) is supposed to help.
> > >
> > > It will still be the same option under a different name.  How does
> > > that change things technically?
> >
> > Do you think that it makes any sense to have support for ACPI-based
> > hotplug CPU
> 
> So this is all about what you and I mean by "ACPI-based hotplug CPU".
> 
> > *and* having it functional with a configuration symbol
> > named "ACPI_HOTPLUG_CPU" to be set to N ? That's essentially what
> > you are advocating for...
> 
> Setting ACPI_HOTPLUG_CPU to N means that you are not going to compile
> the code that is conditional on it.
> 
> That code allows the processor driver to be removed from CPUs and
> arch_unregister_cpu() to be called from within acpi_bus_trim()  (among
> other things).  On the way up, it allows arch_register_cpu() to be
> called from within acpi_bus_scan().  If these things are not done,
> what I mean by "ACPI-based hotplug CPU" is not supported.

Even on Arm64, arch_register_cpu() and arch_unregister_cpu() will be
called when the CPU in the VM is hot-removed or hot-added...

On Tue, Jan 23, 2024 at 9:09 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Tue, Jan 23, 2024 at 08:27:05PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Jan 23, 2024 at 7:59 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Tue, Jan 23, 2024 at 07:26:57PM +0100, Rafael J. Wysocki wrote:
> > > > On Tue, Jan 23, 2024 at 7:20 PM Russell King (Oracle)
> > > > <linux@armlinux.org.uk> wrote:
> > > > >
> > > > > On Tue, Jan 23, 2024 at 06:43:59PM +0100, Rafael J. Wysocki wrote:
> > > > > > On Tue, Jan 23, 2024 at 5:36 PM Russell King (Oracle)
> > > > > > <linux@armlinux.org.uk> wrote:
> > > > > > >
> > > > > > > On Tue, Jan 23, 2024 at 05:15:54PM +0100, Rafael J. Wysocki wrote:
> > > > > > > > On Tue, Jan 23, 2024 at 2:28 PM Russell King (Oracle)
> > > > > > > > <linux@armlinux.org.uk> wrote:
> > > > > > > > >
> > > > > > > > > On Mon, Jan 22, 2024 at 06:00:13PM +0000, Jonathan Cameron wrote:
> > > > > > > > > > On Mon, 18 Dec 2023 21:35:16 +0100
> > > > > > > > > > "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> > > > > > > > > >
> > > > > > > > > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > From: James Morse <james.morse@arm.com>
> > > > > > > > > > > >
> > > > > > > > > > > > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > > > > > > > > > > > present.
> > > > > > > > > > >
> > > > > > > > > > > Right.
> > > > > > > > > > >
> > > > > > > > > > > > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > > > > > > > > > > > CPUs can be taken offline as a power saving measure.
> > > > > > > > > > >
> > > > > > > > > > > But still there is the case in which a non-present CPU can become
> > > > > > > > > > > present, isn't it there?
> > > > > > > > > >
> > > > > > > > > > Not yet defined by the architectures (and I'm assuming it probably never will be).
> > > > > > > > > >
> > > > > > > > > > The original proposal we took to ARM was to do exactly that - they pushed
> > > > > > > > > > back hard on the basis there was no architecturally safe way to implement it.
> > > > > > > > > > Too much of the ARM arch has to exist from the start of time.
> > > > > > > > > >
> > > > > > > > > > https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
> > > > > > > > > > is one of the relevant threads of the kernel side of that discussion.
> > > > > > > > > >
> > > > > > > > > > Not to put specific words into the ARM architects mouths, but the
> > > > > > > > > > short description is that there is currently no demand for working
> > > > > > > > > > out how to make physical CPU hotplug possible, as such they will not
> > > > > > > > > > provide an architecturally compliant way to do it for virtual CPU hotplug and
> > > > > > > > > > another means is needed (which is why this series doesn't use the present bit
> > > > > > > > > > for that purpose and we have the Online capable bit in MADT/GICC)
> > > > > > > > > >
> > > > > > > > > > It was a 'fun' dance of several years to get to that clarification.
> > > > > > > > > > As another fun fact, the same is defined for x86, but I don't think
> > > > > > > > > > anyone has used it yet (GICC for ARM has an online capable bit in the flags to
> > > > > > > > > > enable this, which was remarkably similar to the online capable bit in the
> > > > > > > > > > flags of the Local APIC entries as added fairly recently).
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > > > > > > > > > > > being brought back online, but it remains present throughout.
> > > > > > > > > > > >
> > > > > > > > > > > > Adding code to prevent user-space trying to online these disabled CPUs
> > > > > > > > > > > > needs some additional terminology.
> > > > > > > > > > > >
> > > > > > > > > > > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > > > > > > > > > > > that it makes possible CPUs present.
> > > > > > > > > > >
> > > > > > > > > > > Honestly, I don't think that this change is necessary or even useful.
> > > > > > > > > >
> > > > > > > > > > Whilst it's an attempt to avoid future confusion, the rename is
> > > > > > > > > > not something I really care about so my advice to Russell is drop
> > > > > > > > > > it unless you are attached to it!
> > > > > > > > >
> > > > > > > > > While I agree that it isn't a necessity, I don't fully agree that it
> > > > > > > > > isn't useful.
> > > > > > > > >
> > > > > > > > > One of the issues will be that while Arm64 will support hotplug vCPU,
> > > > > > > > > it won't be setting ACPI_HOTPLUG_CPU because it doesn't support
> > > > > > > > > the present bit changing. So I can see why James decided to rename
> > > > > > > > > it - because with Arm64's hotplug vCPU, the idea that ACPI_HOTPLUG_CPU
> > > > > > > > > somehow enables hotplug CPU support is now no longer true.
> > > > > > > > >
> > > > > > > > > Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because it
> > > > > > > > > leads one to assume that it ought to be enabled for Arm64's
> > > > > > > > > implementatinon, and that could well cause issues in the future if
> > > > > > > > > people make the assumption that "ACPI_HOTPLUG_CPU" means hotplug CPU
> > > > > > > > > is supported in ACPI. It doesn't anymore.
> > > > > > > >
> > > > > > > > On x86 there is no confusion AFAICS.  It's always meant "as long as
> > > > > > > > the platform supports it".
> > > > > > >
> > > > > > > That's x86, which supports physical CPU hotplug. We're introducing
> > > > > > > support for Arm64 here which doesn't support physical CPU hotplug.
> > > > > > >
> > > > > > >                                                 ACPI-based      Physical        Virtual
> > > > > > > Arch    HOTPLUG_CPU     ACPI_HOTPLUG_CPU        Hotplug         Hotplug         Hotplug
> > > > > > > Arm64   Y               N                       Y               N               Y
> > > > > > > x86     Y               Y                       Y               Y               Y
> > > > > > >
> > > > > > > So ACPI_HOTPLUG_CPU becomes totally misnamed with the introduction
> > > > > > > of hotplug on Arm64.
> > > > > > >
> > > > > > > If we want to just look at stuff from an x86 perspective, then yes,
> > > > > > > it remains correct to call it ACPI_HOTPLUG_CPU. It isn't correct as
> > > > > > > soon as we add Arm64, as I already said.
> > > > > >
> > > > > > And if you rename it, it becomes less confusing for ARM64, but more
> > > > > > confusing for x86, which basically is my point.
> > > > > >
> > > > > > IMO "hotplug" covers both cases well enough and "hotplug present" is
> > > > > > only accurate for one of them.
> > > > > >
> > > > > > > And honestly, a two line quip to my reasoned argument is not IMHO
> > > > > > > an acceptable reply.
> > > > > >
> > > > > > Well, I'm not even sure how to respond to this ...
> > > > >
> > > > > The above explanation you give would have been useful...
> > > > >
> > > > > I don't see how "hotplug" covers both cases. As I've tried to point
> > > > > out many times now, ACPI_HOTPLUG_CPU is N for Arm64, yet it supports
> > > > > ACPI based hotplug. How does ACPI_HOTPLUG_CPU cover Arm64 if it's
> > > > > N there?
> > > >
> > > > But IIUC this change is preliminary for changing it (or equivalent
> > > > option with a different name) to Y, isn't it?
> > >
> > > No. As I keep saying, ACPI_HOTPLUG_CPU ends up N on Arm64 even when
> > > it supports hotplug CPU via ACPI.
> > >
> > > Even with the full Arm64 patch set here, under arch/ we still only
> > > have:
> > >
> > > arch/loongarch/Kconfig: select ACPI_HOTPLUG_PRESENT_CPU if ACPI_PROCESSOR && HOTPLUG_CPU
> > > arch/x86/Kconfig:       select ACPI_HOTPLUG_PRESENT_CPU         if ACPI_PROCESSOR && HOTPLUG_CPU
> > >
> > > To say it yet again, ACPI_HOTPLUG_(PRESENT_)CPU is *never* set on
> > > Arm64.
> >
> > Allright, so ARM64 is not going to use the code that is conditional on
> > ACPI_HOTPLUG_CPU today.
> >
> > Fair enough.
> >
> > > > > IMHO it totally doesn't, and moreover, it goes against what
> > > > > one would logically expect - and this is why I have a problem with
> > > > > your effective NAK for this change. I believe you are basically
> > > > > wrong on this for the reasons I've given - that ACPI_HOTPLUG_CPU
> > > > > will be N for Arm64 despite it supporting ACPI-based CPU hotplug.
> > > >
> > > > So I still have to understand how renaming it for all architectures
> > > > (including x86) is supposed to help.
> > > >
> > > > It will still be the same option under a different name.  How does
> > > > that change things technically?
> > >
> > > Do you think that it makes any sense to have support for ACPI-based
> > > hotplug CPU
> >
> > So this is all about what you and I mean by "ACPI-based hotplug CPU".
> >
> > > *and* having it functional with a configuration symbol
> > > named "ACPI_HOTPLUG_CPU" to be set to N ? That's essentially what
> > > you are advocating for...
> >
> > Setting ACPI_HOTPLUG_CPU to N means that you are not going to compile
> > the code that is conditional on it.
> >
> > That code allows the processor driver to be removed from CPUs and
> > arch_unregister_cpu() to be called from within acpi_bus_trim()  (among
> > other things).  On the way up, it allows arch_register_cpu() to be
> > called from within acpi_bus_scan().  If these things are not done,
> > what I mean by "ACPI-based hotplug CPU" is not supported.
>
> Even on Arm64, arch_register_cpu() and arch_unregister_cpu() will be
> called when the CPU in the VM is hot-removed or hot-added...

In a different way, however.

On Tue, Jan 23, 2024 at 08:57:08PM +0000, Russell King (Oracle) wrote:
> On Tue, Jan 23, 2024 at 09:17:18PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Jan 23, 2024 at 9:09 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Tue, Jan 23, 2024 at 08:27:05PM +0100, Rafael J. Wysocki wrote:
> > > > On Tue, Jan 23, 2024 at 7:59 PM Russell King (Oracle)
> > > > <linux@armlinux.org.uk> wrote:
> > > > >
> > > > > On Tue, Jan 23, 2024 at 07:26:57PM +0100, Rafael J. Wysocki wrote:
> > > > > > On Tue, Jan 23, 2024 at 7:20 PM Russell King (Oracle)
> > > > > > <linux@armlinux.org.uk> wrote:
> > > > > > >
> > > > > > > On Tue, Jan 23, 2024 at 06:43:59PM +0100, Rafael J. Wysocki wrote:
> > > > > > > > On Tue, Jan 23, 2024 at 5:36 PM Russell King (Oracle)
> > > > > > > > <linux@armlinux.org.uk> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Jan 23, 2024 at 05:15:54PM +0100, Rafael J. Wysocki wrote:
> > > > > > > > > > On Tue, Jan 23, 2024 at 2:28 PM Russell King (Oracle)
> > > > > > > > > > <linux@armlinux.org.uk> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Mon, Jan 22, 2024 at 06:00:13PM +0000, Jonathan Cameron wrote:
> > > > > > > > > > > > On Mon, 18 Dec 2023 21:35:16 +0100
> > > > > > > > > > > > "Rafael J. Wysocki" <rafael@kernel.org> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > From: James Morse <james.morse@arm.com>
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become
> > > > > > > > > > > > > > present.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Right.
> > > > > > > > > > > > >
> > > > > > > > > > > > > > This isn't the only use of HOTPLUG_CPU. On arm64 and riscv
> > > > > > > > > > > > > > CPUs can be taken offline as a power saving measure.
> > > > > > > > > > > > >
> > > > > > > > > > > > > But still there is the case in which a non-present CPU can become
> > > > > > > > > > > > > present, isn't it there?
> > > > > > > > > > > >
> > > > > > > > > > > > Not yet defined by the architectures (and I'm assuming it probably never will be).
> > > > > > > > > > > >
> > > > > > > > > > > > The original proposal we took to ARM was to do exactly that - they pushed
> > > > > > > > > > > > back hard on the basis there was no architecturally safe way to implement it.
> > > > > > > > > > > > Too much of the ARM arch has to exist from the start of time.
> > > > > > > > > > > >
> > > > > > > > > > > > https://lore.kernel.org/linux-arm-kernel/cbaa6d68-6143-e010-5f3c-ec62f879ad95@arm.com/
> > > > > > > > > > > > is one of the relevant threads of the kernel side of that discussion.
> > > > > > > > > > > >
> > > > > > > > > > > > Not to put specific words into the ARM architects mouths, but the
> > > > > > > > > > > > short description is that there is currently no demand for working
> > > > > > > > > > > > out how to make physical CPU hotplug possible, as such they will not
> > > > > > > > > > > > provide an architecturally compliant way to do it for virtual CPU hotplug and
> > > > > > > > > > > > another means is needed (which is why this series doesn't use the present bit
> > > > > > > > > > > > for that purpose and we have the Online capable bit in MADT/GICC)
> > > > > > > > > > > >
> > > > > > > > > > > > It was a 'fun' dance of several years to get to that clarification.
> > > > > > > > > > > > As another fun fact, the same is defined for x86, but I don't think
> > > > > > > > > > > > anyone has used it yet (GICC for ARM has an online capable bit in the flags to
> > > > > > > > > > > > enable this, which was remarkably similar to the online capable bit in the
> > > > > > > > > > > > flags of the Local APIC entries as added fairly recently).
> > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > > On arm64 an offline CPU may be disabled by firmware, preventing it from
> > > > > > > > > > > > > > being brought back online, but it remains present throughout.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Adding code to prevent user-space trying to online these disabled CPUs
> > > > > > > > > > > > > > needs some additional terminology.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect
> > > > > > > > > > > > > > that it makes possible CPUs present.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Honestly, I don't think that this change is necessary or even useful.
> > > > > > > > > > > >
> > > > > > > > > > > > Whilst it's an attempt to avoid future confusion, the rename is
> > > > > > > > > > > > not something I really care about so my advice to Russell is drop
> > > > > > > > > > > > it unless you are attached to it!
> > > > > > > > > > >
> > > > > > > > > > > While I agree that it isn't a necessity, I don't fully agree that it
> > > > > > > > > > > isn't useful.
> > > > > > > > > > >
> > > > > > > > > > > One of the issues will be that while Arm64 will support hotplug vCPU,
> > > > > > > > > > > it won't be setting ACPI_HOTPLUG_CPU because it doesn't support
> > > > > > > > > > > the present bit changing. So I can see why James decided to rename
> > > > > > > > > > > it - because with Arm64's hotplug vCPU, the idea that ACPI_HOTPLUG_CPU
> > > > > > > > > > > somehow enables hotplug CPU support is now no longer true.
> > > > > > > > > > >
> > > > > > > > > > > Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because it
> > > > > > > > > > > leads one to assume that it ought to be enabled for Arm64's
> > > > > > > > > > > implementatinon, and that could well cause issues in the future if
> > > > > > > > > > > people make the assumption that "ACPI_HOTPLUG_CPU" means hotplug CPU
> > > > > > > > > > > is supported in ACPI. It doesn't anymore.
> > > > > > > > > >
> > > > > > > > > > On x86 there is no confusion AFAICS.  It's always meant "as long as
> > > > > > > > > > the platform supports it".
> > > > > > > > >
> > > > > > > > > That's x86, which supports physical CPU hotplug. We're introducing
> > > > > > > > > support for Arm64 here which doesn't support physical CPU hotplug.
> > > > > > > > >
> > > > > > > > >                                                 ACPI-based      Physical        Virtual
> > > > > > > > > Arch    HOTPLUG_CPU     ACPI_HOTPLUG_CPU        Hotplug         Hotplug         Hotplug
> > > > > > > > > Arm64   Y               N                       Y               N               Y
> > > > > > > > > x86     Y               Y                       Y               Y               Y
> > > > > > > > >
> > > > > > > > > So ACPI_HOTPLUG_CPU becomes totally misnamed with the introduction
> > > > > > > > > of hotplug on Arm64.
> > > > > > > > >
> > > > > > > > > If we want to just look at stuff from an x86 perspective, then yes,
> > > > > > > > > it remains correct to call it ACPI_HOTPLUG_CPU. It isn't correct as
> > > > > > > > > soon as we add Arm64, as I already said.
> > > > > > > >
> > > > > > > > And if you rename it, it becomes less confusing for ARM64, but more
> > > > > > > > confusing for x86, which basically is my point.
> > > > > > > >
> > > > > > > > IMO "hotplug" covers both cases well enough and "hotplug present" is
> > > > > > > > only accurate for one of them.
> > > > > > > >
> > > > > > > > > And honestly, a two line quip to my reasoned argument is not IMHO
> > > > > > > > > an acceptable reply.
> > > > > > > >
> > > > > > > > Well, I'm not even sure how to respond to this ...
> > > > > > >
> > > > > > > The above explanation you give would have been useful...
> > > > > > >
> > > > > > > I don't see how "hotplug" covers both cases. As I've tried to point
> > > > > > > out many times now, ACPI_HOTPLUG_CPU is N for Arm64, yet it supports
> > > > > > > ACPI based hotplug. How does ACPI_HOTPLUG_CPU cover Arm64 if it's
> > > > > > > N there?
> > > > > >
> > > > > > But IIUC this change is preliminary for changing it (or equivalent
> > > > > > option with a different name) to Y, isn't it?
> > > > >
> > > > > No. As I keep saying, ACPI_HOTPLUG_CPU ends up N on Arm64 even when
> > > > > it supports hotplug CPU via ACPI.
> > > > >
> > > > > Even with the full Arm64 patch set here, under arch/ we still only
> > > > > have:
> > > > >
> > > > > arch/loongarch/Kconfig: select ACPI_HOTPLUG_PRESENT_CPU if ACPI_PROCESSOR && HOTPLUG_CPU
> > > > > arch/x86/Kconfig:       select ACPI_HOTPLUG_PRESENT_CPU         if ACPI_PROCESSOR && HOTPLUG_CPU
> > > > >
> > > > > To say it yet again, ACPI_HOTPLUG_(PRESENT_)CPU is *never* set on
> > > > > Arm64.
> > > >
> > > > Allright, so ARM64 is not going to use the code that is conditional on
> > > > ACPI_HOTPLUG_CPU today.
> > > >
> > > > Fair enough.
> > > >
> > > > > > > IMHO it totally doesn't, and moreover, it goes against what
> > > > > > > one would logically expect - and this is why I have a problem with
> > > > > > > your effective NAK for this change. I believe you are basically
> > > > > > > wrong on this for the reasons I've given - that ACPI_HOTPLUG_CPU
> > > > > > > will be N for Arm64 despite it supporting ACPI-based CPU hotplug.
> > > > > >
> > > > > > So I still have to understand how renaming it for all architectures
> > > > > > (including x86) is supposed to help.
> > > > > >
> > > > > > It will still be the same option under a different name.  How does
> > > > > > that change things technically?
> > > > >
> > > > > Do you think that it makes any sense to have support for ACPI-based
> > > > > hotplug CPU
> > > >
> > > > So this is all about what you and I mean by "ACPI-based hotplug CPU".
> > > >
> > > > > *and* having it functional with a configuration symbol
> > > > > named "ACPI_HOTPLUG_CPU" to be set to N ? That's essentially what
> > > > > you are advocating for...
> > > >
> > > > Setting ACPI_HOTPLUG_CPU to N means that you are not going to compile
> > > > the code that is conditional on it.
> > > >
> > > > That code allows the processor driver to be removed from CPUs and
> > > > arch_unregister_cpu() to be called from within acpi_bus_trim()  (among
> > > > other things).  On the way up, it allows arch_register_cpu() to be
> > > > called from within acpi_bus_scan().  If these things are not done,
> > > > what I mean by "ACPI-based hotplug CPU" is not supported.
> > >
> > > Even on Arm64, arch_register_cpu() and arch_unregister_cpu() will be
> > > called when the CPU in the VM is hot-removed or hot-added...
> > 
> > In a different way, however.
> 
> This is getting tiresome. The goal posts keep moving. This isn't a
> discussion, this is a "you're wrong and I'm going to keep changing my
> argument if you agree with me to make you always wrong".
> 
> Sorry, no point continuing this.

Let me be clear why I'm exhasperated by this.

I've been giving you a technical argument (Arm64 supporting ACPI
hotplug CPU, but ACPI_HOTPLUG_CPU=n) for many many emails. You
seemed to misunderstand that, expecting ACPI_HOTPLUG_CPU to become
Y later in the series.

When that became clear that it wasn't, you've changed tack. It then
became about whether two functions get called or not.

When I pointed out that they are still going to be called, oh no,
it's not about whether those two functions will be called but
how they get called.

Essentially, what this comes down to is that _you_ have no technical
argument against the change, just _you_ don't personally want it
and it doesn't matter what justification I come up with, you're
always going to tell me something different.

So why not state that you personally don't want it in the first
place? Why this game of cat and mouse and the constantly changing
arguments. I guess it's to waste developers time.

Well, I'm calling you out for this, because I'm that pissed off
at the amount of time you're causing to be wasted.

On Tue, Jan 23, 2024 at 10:12 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Tue, Jan 23, 2024 at 08:57:08PM +0000, Russell King (Oracle) wrote:
> > On Tue, Jan 23, 2024 at 09:17:18PM +0100, Rafael J. Wysocki wrote:
> > > On Tue, Jan 23, 2024 at 9:09 PM Russell King (Oracle)
> > > <linux@armlinux.org.uk> wrote:

[cut]

> > Sorry, no point continuing this.
>
> Let me be clear why I'm exhasperated by this.
>
> I've been giving you a technical argument (Arm64 supporting ACPI
> hotplug CPU, but ACPI_HOTPLUG_CPU=n) for many many emails. You
> seemed to misunderstand that, expecting ACPI_HOTPLUG_CPU to become
> Y later in the series.
>
> When that became clear that it wasn't, you've changed tack. It then
> became about whether two functions get called or not.
>
> When I pointed out that they are still going to be called, oh no,
> it's not about whether those two functions will be called but
> how they get called.

As I've said already in this thread, it is all about what "ACPI-based
CPU hotplug" means to each of us.

I know what it means to me: Running the code that is compiled when
ACPI_HOTPLUG_CPU is set via the processor scan handler.

I'm not entirely sure what it means to you.

You are saying that the config option name needs to be changed,
because it is going to stay N for ARM64 and it will support
"ACPI-based CPU hotplug" and I'm not sure what exactly you mean by
this.

To me, this just means that ARM64 is not going to use the processor
scan handler in the way it is used on x86.

> Essentially, what this comes down to is that _you_ have no technical
> argument against the change, just _you_ don't personally want it
> and it doesn't matter what justification I come up with, you're
> always going to tell me something different.

Sorry, but I'm just not convinced by your justification.

> So why not state that you personally don't want it in the first
> place? Why this game of cat and mouse and the constantly changing
> arguments. I guess it's to waste developers time.
>
> Well, I'm calling you out for this, because I'm that pissed off
> at the amount of time you're causing to be wasted.

And I don't have to suffer this kind of abuse.  Sorry.

On Tue, Jan 23, 2024 at 11:05:43PM +0100, Rafael J. Wysocki wrote:
> > So why not state that you personally don't want it in the first
> > place? Why this game of cat and mouse and the constantly changing
> > arguments. I guess it's to waste developers time.
> >
> > Well, I'm calling you out for this, because I'm that pissed off
> > at the amount of time you're causing to be wasted.
> 
> And I don't have to suffer this kind of abuse.  Sorry.

And I've had enough of this crap, so I'm not walking away. Good
riddance.