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 441A8C43334 for ; Mon, 27 Jun 2022 08:02:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232660AbiF0ICE (ORCPT ); Mon, 27 Jun 2022 04:02:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36930 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232487AbiF0ICD (ORCPT ); Mon, 27 Jun 2022 04:02:03 -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 182BA10D7 for ; Mon, 27 Jun 2022 01:02:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1656316921; 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=QB2L7UqYcbfJKvcItharUG0ucsLsKmoJv2WheSsmnT1V1bYODE4e7WqukuldHNlEJFWS02 O5r/++UIVGHLAvQvDs/hmRVdlRo2awHjMLddRSkQZOwlLOwEWG9bzMDRXXR99tjPvjVV8m 9n4NJF3WhrvJrYEYVZE3NGUl1wy9fjY= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-10-1Dl9JsPKOGW3LQo9aLyKug-1; Mon, 27 Jun 2022 04:01:59 -0400 X-MC-Unique: 1Dl9JsPKOGW3LQo9aLyKug-1 Received: by mail-wr1-f71.google.com with SMTP id m7-20020adfa3c7000000b0021b94088ba2so966793wrb.9 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=m4c1whRzrLqAJBNUx4Zj4vxrT2LBC0ynY9+FZnLKwfzoL1Ge0VgLfdYu4sPattGY1z yhpMu6+6yJ1uc81LkxqpamK5PnmAz1BlCuDDa7nt1QRWwNf649PZLVBPq57sc3+o05ap gL/ATfT6F7TdTAMt3+7jnibg0aIaXNJOG4JKLjwtoVZeWKyeDoQDx8SMjzPacxiWC+aa fMPPd0GukCZxzmmPhHERvE1/IdB47iT62po3Rw2DmlFAwsIvnuiVa8E+fzyZm0AeM65/ pRpTNa1ut9qDBesMV+9zS88obQL1v3P9q36gxqHqRaIjR8bMj8yOjAkK6gg8KQk1LbQv 1LuQ== X-Gm-Message-State: AJIora8fVz2e4aXL9+yAOqcYcGqJytLXNeT2+azGToSoQIVkmM4EjxXb dkNSp2ZLOpudHdlpdB/E7PHayuHIDx+uuweqGSQ+Eb75fPOeUktmIMQXUuOSOxr673jqfLq+g/o bLmhNwV1rzEQCIL1zQZihlQ== X-Received: by 2002:a05:600c:4f81:b0:39c:809c:8a9e with SMTP id n1-20020a05600c4f8100b0039c809c8a9emr19091655wmq.39.1656316918107; 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-acpi@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; > > > + } > > > + >