[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0jV8ODcxuLL+iSpYbW7w=GFtUSakN-n8CO5Zmun3K-Erg@mail.gmail.com>
Date: Wed, 22 Jun 2022 13:42:01 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Kai Huang <kai.huang@...el.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
kvm-devel <kvm@...r.kernel.org>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
Sean Christopherson <seanjc@...gle.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Dave Hansen <dave.hansen@...el.com>,
Len Brown <len.brown@...el.com>,
Tony Luck <tony.luck@...el.com>,
Rafael Wysocki <rafael.j.wysocki@...el.com>,
Reinette Chatre <reinette.chatre@...el.com>,
Dan Williams <dan.j.williams@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Andi Kleen <ak@...ux.intel.com>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@...ux.intel.com>,
isaku.yamahata@...el.com, Tom Lendacky <thomas.lendacky@....com>,
Tianyu.Lan@...rosoft.com, Randy Dunlap <rdunlap@...radead.org>,
"Jason A. Donenfeld" <Jason@...c4.com>,
Juri Lelli <juri.lelli@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Frederic Weisbecker <frederic@...nel.org>,
Yue Haibing <yuehaibing@...wei.com>, dongli.zhang@...cle.com
Subject: Re: [PATCH v5 02/22] cc_platform: Add new attribute to prevent ACPI
CPU hotplug
On Wed, Jun 22, 2022 at 1:16 PM Kai Huang <kai.huang@...el.com> wrote:
>
> Platforms with confidential computing technology may not support ACPI
> CPU hotplug when such technology is enabled by the BIOS. Examples
> include Intel platforms which support Intel Trust Domain Extensions
> (TDX).
>
> If the kernel ever receives ACPI CPU hotplug event, it is likely a BIOS
> bug. For ACPI CPU hot-add, the kernel should speak out this is a BIOS
> bug and reject the new CPU. For hot-removal, for simplicity just assume
> the kernel cannot continue to work normally, and BUG().
>
> Add a new attribute CC_ATTR_ACPI_CPU_HOTPLUG_DISABLED to indicate the
> platform doesn't support ACPI CPU hotplug, so that kernel can handle
> ACPI CPU hotplug events for such platform. The existing attribute
> CC_ATTR_HOTPLUG_DISABLED is for software CPU hotplug thus doesn't fit.
>
> In acpi_processor_{add|remove}(), add early check against this attribute
> and handle accordingly if it is set.
>
> Also take this chance to rename existing CC_ATTR_HOTPLUG_DISABLED to
> CC_ATTR_CPU_HOTPLUG_DISABLED as it is for software CPU hotplug.
>
> Signed-off-by: Kai Huang <kai.huang@...el.com>
> ---
> arch/x86/coco/core.c | 2 +-
> drivers/acpi/acpi_processor.c | 23 +++++++++++++++++++++++
> include/linux/cc_platform.h | 15 +++++++++++++--
> kernel/cpu.c | 2 +-
> 4 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
> index 4320fadae716..1bde1af75296 100644
> --- a/arch/x86/coco/core.c
> +++ b/arch/x86/coco/core.c
> @@ -20,7 +20,7 @@ static bool intel_cc_platform_has(enum cc_attr attr)
> {
> switch (attr) {
> case CC_ATTR_GUEST_UNROLL_STRING_IO:
> - case CC_ATTR_HOTPLUG_DISABLED:
> + case CC_ATTR_CPU_HOTPLUG_DISABLED:
> case CC_ATTR_GUEST_MEM_ENCRYPT:
> case CC_ATTR_MEM_ENCRYPT:
> return true;
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 6737b1cbf6d6..b960db864cd4 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -15,6 +15,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> +#include <linux/cc_platform.h>
>
> #include <acpi/processor.h>
>
> @@ -357,6 +358,17 @@ static int acpi_processor_add(struct acpi_device *device,
> struct device *dev;
> int result = 0;
>
> + /*
> + * If the confidential computing platform doesn't support ACPI
> + * memory hotplug, the BIOS should never deliver such event to
> + * the kernel. Report ACPI CPU hot-add as a BIOS bug and ignore
> + * the new CPU.
> + */
> + if (cc_platform_has(CC_ATTR_ACPI_CPU_HOTPLUG_DISABLED)) {
This will affect initialization, not just hotplug AFAICS.
You should reset the .hotplug.enabled flag in processor_handler to
false instead.
> + dev_err(&device->dev, "[BIOS bug]: Platform doesn't support ACPI CPU hotplug. New CPU ignored.\n");
> + return -EINVAL;
> + }
> +
> pr = kzalloc(sizeof(struct acpi_processor), GFP_KERNEL);
> if (!pr)
> return -ENOMEM;
> @@ -434,6 +446,17 @@ static void acpi_processor_remove(struct acpi_device *device)
> if (!device || !acpi_driver_data(device))
> return;
>
> + /*
> + * The confidential computing platform is broken if ACPI memory
> + * hot-removal isn't supported but it happened anyway. Assume
> + * it's not guaranteed that the kernel can continue to work
> + * normally. Just BUG().
> + */
> + if (cc_platform_has(CC_ATTR_ACPI_CPU_HOTPLUG_DISABLED)) {
> + dev_err(&device->dev, "Platform doesn't support ACPI CPU hotplug. BUG().\n");
> + BUG();
> + }
> +
> pr = acpi_driver_data(device);
> if (pr->id >= nr_cpu_ids)
> goto out;
> diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
> index 691494bbaf5a..9ce9256facc8 100644
> --- a/include/linux/cc_platform.h
> +++ b/include/linux/cc_platform.h
> @@ -74,14 +74,25 @@ enum cc_attr {
> CC_ATTR_GUEST_UNROLL_STRING_IO,
>
> /**
> - * @CC_ATTR_HOTPLUG_DISABLED: Hotplug is not supported or disabled.
> + * @CC_ATTR_CPU_HOTPLUG_DISABLED: CPU hotplug is not supported or
> + * disabled.
> *
> * The platform/OS is running as a guest/virtual machine does not
> * support CPU hotplug feature.
> *
> * Examples include TDX Guest.
> */
> - CC_ATTR_HOTPLUG_DISABLED,
> + CC_ATTR_CPU_HOTPLUG_DISABLED,
> +
> + /**
> + * @CC_ATTR_ACPI_CPU_HOTPLUG_DISABLED: ACPI CPU hotplug is not
> + * supported.
> + *
> + * The platform/OS does not support ACPI CPU hotplug.
> + *
> + * Examples include TDX platform.
> + */
> + CC_ATTR_ACPI_CPU_HOTPLUG_DISABLED,
> };
>
> #ifdef CONFIG_ARCH_HAS_CC_PLATFORM
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index edb8c199f6a3..966772cce063 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1191,7 +1191,7 @@ static int cpu_down_maps_locked(unsigned int cpu, enum cpuhp_state target)
> * If the platform does not support hotplug, report it explicitly to
> * differentiate it from a transient offlining failure.
> */
> - if (cc_platform_has(CC_ATTR_HOTPLUG_DISABLED))
> + if (cc_platform_has(CC_ATTR_CPU_HOTPLUG_DISABLED))
> return -EOPNOTSUPP;
> if (cpu_hotplug_disabled)
> return -EBUSY;
> --
> 2.36.1
>
Powered by blists - more mailing lists