Received-SPF: pass (google.com: domain of linux-kernel+bounces-35797-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <ZXmn46ptis59F0CO@shell.armlinux.org.uk> <E1rDOgD-00Dvk2-3h@rmk-PC.armlinux.org.uk>
 <CAJZ5v0g9nfLrEf9u4Ksw6BOWJQ9iv8Z-O8RsLU6jR5zk0ahxRw@mail.gmail.com>
 <20240122180013.000016d5@Huawei.com> <Za++/11n5KA1VS3p@shell.armlinux.org.uk>
 <CAJZ5v0h7wsLt8d3ZoLXsK1=crAx66T42WDKNoHcg8CiHpAjS8g@mail.gmail.com> <Za/q9jivG4OdZM0f@shell.armlinux.org.uk>
In-Reply-To: <Za/q9jivG4OdZM0f@shell.armlinux.org.uk>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Tue, 23 Jan 2024 18:43:59 +0100
Message-ID: <CAJZ5v0gwe02uzAQoX0QDHo35OTEozpbnqC6vukjM3aE6HMq9WQ@mail.gmail.com>
Subject: Re: [PATCH RFC v3 05/21] ACPI: Rename ACPI_HOTPLUG_CPU to include 'present'
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>, Jonathan Cameron <Jonathan.Cameron@huawei.com>, 
	linux-pm@vger.kernel.org, loongarch@lists.linux.dev, 
	linux-acpi@vger.kernel.org, linux-arch@vger.kernel.org, 
	linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, 
	linux-riscv@lists.infradead.org, kvmarm@lists.linux.dev, x86@kernel.org, 
	acpica-devel@lists.linuxfoundation.org, linux-csky@vger.kernel.org, 
	linux-doc@vger.kernel.org, linux-ia64@vger.kernel.org, 
	linux-parisc@vger.kernel.org, Salil Mehta <salil.mehta@huawei.com>, 
	Jean-Philippe Brucker <jean-philippe@linaro.org>, jianyong.wu@arm.com, justin.he@arm.com, 
	James Morse <james.morse@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Jan 23, 2024 at 5:36=E2=80=AFPM 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=E2=80=AFPM 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=E2=80=AFPM 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 be=
come
> > > > > > 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 imp=
lement 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-ec=
62f879ad95@arm.com/
> > > > is one of the relevant threads of the kernel side of that discussio=
n.
> > > >
> > > > 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 no=
t
> > > > provide an architecturally compliant way to do it for virtual CPU h=
otplug and
> > > > another means is needed (which is why this series doesn't use the p=
resent 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 t=
he 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 disabl=
ed CPUs
> > > > > > needs some additional terminology.
> > > > > >
> > > > > > Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to re=
flect
> > > > > > that it makes possible CPUs present.
> > > > >
> > > > > Honestly, I don't think that this change is necessary or even use=
ful.
> > > >
> > > > 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_CP=
U
> > > somehow enables hotplug CPU support is now no longer true.
> > >
> > > Keeping it as ACPI_HOTPLUG_CPU makes the code less obvious, because i=
t
> > > 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 ...