From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 69584CCA473 for ; Mon, 27 Jun 2022 08:02:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232487AbiF0ICF (ORCPT ); Mon, 27 Jun 2022 04:02:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36916 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229463AbiF0ICC (ORCPT ); Mon, 27 Jun 2022 04:02:02 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id BBB29DB1 for ; Mon, 27 Jun 2022 01:02:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1656316920; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=NjQ/y3uzOB0VPioQUmdD2/aH8hZijcv/VXyRMlZuNQg=; b=H1BuI42k2n+fG5buBKhT495sSnOAW0RPjQhIplffEFA7SIChKL7BOkbGpvRKBJ1EayLiOE xIy8tRMeJ9r2icsYQ1cFfemCWEfjJBukjSbPyp8elFE7aiz2d5TvmQPtBO81BuHbmIMkWb 8CWiTLN+ob4JqNY2m069BdwrMlqOoiY= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-354-s7P3euU1MrCLo8mm5H49uw-1; Mon, 27 Jun 2022 04:01:59 -0400 X-MC-Unique: s7P3euU1MrCLo8mm5H49uw-1 Received: by mail-wm1-f71.google.com with SMTP id r65-20020a1c4444000000b003a02a3f0beeso5393472wma.3 for ; Mon, 27 Jun 2022 01:01:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=NjQ/y3uzOB0VPioQUmdD2/aH8hZijcv/VXyRMlZuNQg=; b=zfeULfabL9gBvWrfZ9wxRLPile/6R49OK+4nC4N/6V7e4wgPhHw3iywluAEqdHCCGv Yy/4IYYtDAtkwjtyh9bHUH3cO/Dbb2fnTMJ/rpMzuLPtcv/FSUo9QI5DsyCSInV+LU6T oFcFtuGpX7JTR2eNK9pBYUM44I39FxXrMT/5qk7GUpWcsYzJxxTcZ6snv943kaZUQLzs ANu+HEU1oYWpPV116x2yCcePbf0PBic43jL6vTJ6X/De6mr2Y7Ta1e7A5bmds2NlKFSw ZENLj9fkSkOzYnEk3VTYb2H0g8uTVGDERmC9eFvtXDPz2m9uGuXKR8T9vvcRrZinWk5k P+aA== X-Gm-Message-State: AJIora+uiOFClK6LkhmFYPv1jyHgMULAcolHNp4mNAthq5cqke1EbrLH Xha/xQhjr4f4s1tIuuNa8EcRLjHKEX95g68+HhaXhK/WFiDbobxl8Bxpe6h0rOYf+cMx4VujNzb iDj7wgzRgpiGaQ8VvZNIroY3z X-Received: by 2002:a05:600c:4f81:b0:39c:809c:8a9e with SMTP id n1-20020a05600c4f8100b0039c809c8a9emr19091650wmq.39.1656316918085; Mon, 27 Jun 2022 01:01:58 -0700 (PDT) X-Google-Smtp-Source: AGRyM1ucgAGSBXy8zDrJRdc3q59PcoYBheBIuCkSQZpixIvVUMq6Ty2yfJQnho9iegzNyMmUhooTwA== X-Received: by 2002:a05:600c:4f81:b0:39c:809c:8a9e with SMTP id n1-20020a05600c4f8100b0039c809c8a9emr19091606wmq.39.1656316917811; Mon, 27 Jun 2022 01:01:57 -0700 (PDT) Received: from localhost (nat-pool-brq-t.redhat.com. [213.175.37.10]) by smtp.gmail.com with ESMTPSA id w9-20020a5d6089000000b0020e5b4ebaecsm9700461wrt.4.2022.06.27.01.01.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Jun 2022 01:01:57 -0700 (PDT) Date: Mon, 27 Jun 2022 10:01:55 +0200 From: Igor Mammedov To: Kai Huang Cc: "Rafael J. Wysocki" , Linux Kernel Mailing List , kvm-devel , ACPI Devel Maling List , Sean Christopherson , Paolo Bonzini , Dave Hansen , Len Brown , Tony Luck , Rafael Wysocki , Reinette Chatre , Dan Williams , Peter Zijlstra , Andi Kleen , "Kirill A. Shutemov" , Kuppuswamy Sathyanarayanan , isaku.yamahata@intel.com, Tom Lendacky , Tianyu.Lan@microsoft.com, Randy Dunlap , "Jason A. Donenfeld" , Juri Lelli , Mark Rutland , Frederic Weisbecker , Yue Haibing , dongli.zhang@oracle.com Subject: Re: [PATCH v5 02/22] cc_platform: Add new attribute to prevent ACPI CPU hotplug Message-ID: <20220627100155.71a7b34c@redhat.com> In-Reply-To: References: X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 23 Jun 2022 12:01:48 +1200 Kai Huang wrote: > On Wed, 2022-06-22 at 13:42 +0200, Rafael J. Wysocki wrote: > > On Wed, Jun 22, 2022 at 1:16 PM Kai Huang 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 > > > --- > > > 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 > > > #include > > > #include > > > +#include > > > > > > #include > > > > > > @@ -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. > > Hi Rafael, > > Thanks for the review. By "affect initialization" did you mean this > acpi_processor_add() is also called during kernel boot when any logical cpu is > brought up? Or do you mean ACPI CPU hotplug can also happen during kernel boot > (after acpi_processor_init())? > > I see acpi_processor_init() calls acpi_processor_check_duplicates() which calls > acpi_evaluate_object() but I don't know details of ACPI so I don't know whether > this would trigger acpi_processor_add(). > > One thing is TDX doesn't support ACPI CPU hotplug is an architectural thing, so > it is illegal even if it happens during kernel boot. Dave's idea is the kernel > should speak out loudly if physical CPU hotplug indeed happened on (BIOS) TDX- > enabled platforms. Otherwise perhaps we can just give up initializing the ACPI > CPU hotplug in acpi_processor_init(), something like below? The thing is that by the time ACPI machinery kicks in, physical hotplug has already happened and in case of (kvm+qemu+ovmf hypervisor combo) firmware has already handled it somehow and handed it over to ACPI. If you say it's architectural thing then cpu hotplug is platform/firmware bug and should be disabled there instead of working around it in the kernel. Perhaps instead of 'preventing' hotplug, complain/panic and be done with it. > --- a/drivers/acpi/acpi_processor.c > +++ b/drivers/acpi/acpi_processor.c > @@ -707,6 +707,10 @@ bool acpi_duplicate_processor_id(int proc_id) > void __init acpi_processor_init(void) > { > acpi_processor_check_duplicates(); > + > + if (cc_platform_has(CC_ATTR_ACPI_CPU_HOTPLUG_DISABLED)) > + return; > + > acpi_scan_add_handler_with_hotplug(&processor_handler, "processor"); > acpi_scan_add_handler(&processor_container_handler); > } > > > > > > > + dev_err(&device->dev, "[BIOS bug]: Platform doesn't support ACPI CPU hotplug. New CPU ignored.\n"); > > > + return -EINVAL; > > > + } > > > + >