From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [78.32.30.218]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 158948121F; Tue, 23 Jan 2024 18:20:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=78.32.30.218 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706034012; cv=none; b=euX7/Aih/U+fvQOSMcvtxaZWapTGewlvjKoevNcAxIw3e4IFZGBL9yqda7B3PTpA9iPdUAOwnc+/N/iDuLNiPazOYKv+Ro/njgEZ2P64dSLxRbZA5/GEQ7eHbC0Jdvk7oI9Lu0AtjEiS564zir0Q9KO4GWDY5go60y0I7FX0n9M= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706034012; c=relaxed/simple; bh=Kz6Sx2HwHUtMwThwQvcZ1SzVhfnaGraII0WfWtXei18=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=PtfYcpttXVFaObKvDsKL53yAUMti5CPdQ9PR63IjD34WYxp1zc8mqP/9LhGtHisvk2O7fG/Mdxpur0KnB5ZIF3l6RBwfUA0OevfZeXtGnjZcQ1y9OV1W60WTLTtxIlJxi+pezkoFqLNqhfJaRnTkOrHSh4i0vkSRdM7eu4jEkpw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=armlinux.org.uk; spf=none smtp.mailfrom=armlinux.org.uk; dkim=pass (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b=KfMZkqSB; arc=none smtp.client-ip=78.32.30.218 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=armlinux.org.uk Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=armlinux.org.uk Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="KfMZkqSB" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=xJtDwsEzxC2wRkKiDTxPnJF7Bcyhg3afuw16GcINCwM=; b=KfMZkqSBpSMz1Up8pIHrEoOmK2 gS72067GJZOQOPUTd0wTWjKykok1kTE4dCcBEAtrgrvvb7VXsM9FqTwumi5Ntqb8yCVazVOZ6Wp0n SWrK5q4QA2TOsTegfudAQLyRGgqxNF9QlzA33ibCCQrZVHvdeIFQQuCEl66Vo0dmF0OFYnFiMyhzF heFGFU0q325UHEoWKsnoe/rL3eAw4o4e6t/0qFi5Zao5+PrxjeOs+vVIdYacZNE3H5wdKxLx6gUvX ldr2VrwRpbatnDdYCIf9HvSw8Mjum3/eXOGXrGqv9MXGK0aslVEdvI8jm2wuMDfSjExEkTjy3Irtk X7HVzNGQ==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:34546) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1rSLNQ-0002u1-2h; Tue, 23 Jan 2024 18:20:00 +0000 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1rSLNM-00029h-Kk; Tue, 23 Jan 2024 18:19:56 +0000 Date: Tue, 23 Jan 2024 18:19:56 +0000 From: "Russell King (Oracle)" To: "Rafael J. Wysocki" Cc: Jonathan Cameron , 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 , Jean-Philippe Brucker , jianyong.wu@arm.com, justin.he@arm.com, James Morse Subject: Re: [PATCH RFC v3 05/21] ACPI: Rename ACPI_HOTPLUG_CPU to include 'present' Message-ID: References: <20240122180013.000016d5@Huawei.com> Precedence: bulk X-Mailing-List: linux-csky@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Sender: Russell King (Oracle) 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) > 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) > > > 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" wrote: > > > > > > > > > > > On Wed, Dec 13, 2023 at 1:49 PM Russell King wrote: > > > > > > > > > > > > > > From: James Morse > > > > > > > > > > > > > > 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. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!