Message ID | 20231020151242.1814-3-kirill.shutemov@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2010:b0:403:3b70:6f57 with SMTP id fe16csp1125319vqb; Fri, 20 Oct 2023 08:13:11 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFABnqsjia49n6BIgHo5kXovejjUeRCNqh2RhZrcSB7FI0jBCBG5CGe3+NT9ntVxFLkgncQ X-Received: by 2002:a05:6a00:428e:b0:690:9a5a:e34e with SMTP id bx14-20020a056a00428e00b006909a5ae34emr8367478pfb.12.1697814791448; Fri, 20 Oct 2023 08:13:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697814791; cv=none; d=google.com; s=arc-20160816; b=k7OKfsgiWzPBH//OVOmXeXntNXHFXS25qEM/tVD4M0YE4zX/Jsswf3IT+mr/WB+vho 7P2pn7QNJQYY86xMEWOqSuu6AeIeVWKclp22KIXCe5W6gUd9raSM05GRZKc9x3pQacM7 CZsLw9ojkj+w9Nr3+zbXdPB6hECy94CYRJhl89i/x+TQZGFqGqlMkOKZ1sFSti3NEMl3 VupFfldC7hhm0SXAt3K7GaJILcJfiq18cg86nHhp2aSSSYuTb32TTVgPCBjx1eWn4mHB +8cN2RbcKte3IL6y+f63NJlhj0aN6avCnvPw0MS0bFhADkwUop9ZZFR1kkRHROLI3cOQ xPqQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=ehhuYjx0ryiMpc8uCQyVeRIfHM9a/ooGcq+1+G/ZN1s=; fh=OvJRnOqsMTm9XoNmEwebcqh9Ud7yh1CTeKAP84ols98=; b=WJIv3T15npDyl0N+VIq1eaMhJQ9tvC8vRexDm8n0/tHqi/lFnBmL4U8E1KTdt0zSbK 24gS0V4Smo3MqjqKf1zcqVnq69HhJNsgSWI1r8ArttuFEmbU2hdEGk9pmu30shijy4LB 9zO2MPRMorB0G76EXm8UNcaYk+J5TYzOfw2Kevd+Ew8d/3qfg9NYPASHImKzGOZpuAc7 afWT0IVkm59O6hCEiBHZ7IV8Kw5p4CPu77nzHe4X3U8BWLvT05OjF7EC/R+vzs8MQOm0 TcBUbObOfZkU4oGTOKtwoPN3e+pWeMTaRgo2fEIPC6xuvocGd7H6AFSaPqikocq2A6RJ A+aA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=Qd3pJVk2; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id u14-20020a056a00098e00b006bddac4b018si2233550pfg.107.2023.10.20.08.13.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 20 Oct 2023 08:13:11 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=Qd3pJVk2; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id B477E82F7FA5; Fri, 20 Oct 2023 08:13:10 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1377664AbjJTPNG (ORCPT <rfc822;a1648639935@gmail.com> + 26 others); Fri, 20 Oct 2023 11:13:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57490 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1377631AbjJTPNA (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 20 Oct 2023 11:13:00 -0400 Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.20]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3825BD69 for <linux-kernel@vger.kernel.org>; Fri, 20 Oct 2023 08:12:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1697814778; x=1729350778; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=Bu2/tizz5ae3ipBcoXAV9deIpKDIFDrWSuBYbHVXXFM=; b=Qd3pJVk2/b7Zc/QCbM8mb9Osn9GW0bL5K6RKNf3P/PK5AS1S4ZwrrXca j+d8N8LdwzR4VQ+Eoxb05gpOJrIXBH4HkVj33URu46bSooRwfxf+2rPf5 LGMao3BsQcyqj0GzOSPH9ALg4ounaZso+VswdQxvZxm/0Nn8CM+3NXXRD 0uOli6JJ2hIPxaZijfGUquukFwZw813df7Pm3zU33pESJhsasVAygYWfy pjp4TxNTRzHN2BJ9cwCdt4cL9Bd+j35IYJNEOEEQ0ugPLP6bloZUTh44U +KsJe0MutvdGNIqxVGqdVkIyiPqTUc0fauNLC/3eQLO7NWJAMLnBO+hxG A==; X-IronPort-AV: E=McAfee;i="6600,9927,10869"; a="376893585" X-IronPort-AV: E=Sophos;i="6.03,239,1694761200"; d="scan'208";a="376893585" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Oct 2023 08:12:53 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10869"; a="761080240" X-IronPort-AV: E=Sophos;i="6.03,239,1694761200"; d="scan'208";a="761080240" Received: from dgutows1-mobl.ger.corp.intel.com (HELO box.shutemov.name) ([10.249.39.237]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Oct 2023 08:12:48 -0700 Received: by box.shutemov.name (Postfix, from userid 1000) id F19B010A294; Fri, 20 Oct 2023 18:12:44 +0300 (+03) From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> To: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org Cc: "Rafael J. Wysocki" <rafael@kernel.org>, Peter Zijlstra <peterz@infradead.org>, Adrian Hunter <adrian.hunter@intel.com>, Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>, Elena Reshetova <elena.reshetova@intel.com>, Jun Nakajima <jun.nakajima@intel.com>, Rick Edgecombe <rick.p.edgecombe@intel.com>, Tom Lendacky <thomas.lendacky@amd.com>, "Kalra, Ashish" <ashish.kalra@amd.com>, Sean Christopherson <seanjc@google.com>, "Huang, Kai" <kai.huang@intel.com>, Baoquan He <bhe@redhat.com>, kexec@lists.infradead.org, linux-coco@lists.linux.dev, linux-kernel@vger.kernel.org, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> Subject: [PATCHv2 02/13] kernel/cpu: Add support for declaring CPU offlining not supported Date: Fri, 20 Oct 2023 18:12:31 +0300 Message-ID: <20231020151242.1814-3-kirill.shutemov@linux.intel.com> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20231020151242.1814-1-kirill.shutemov@linux.intel.com> References: <20231020151242.1814-1-kirill.shutemov@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Fri, 20 Oct 2023 08:13:10 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780287842687969910 X-GMAIL-MSGID: 1780287842687969910 |
Series |
x86/tdx: Add kexec support
|
|
Commit Message
Kirill A. Shutemov
Oct. 20, 2023, 3:12 p.m. UTC
ACPI MADT doesn't allow to offline CPU after it got woke up.
Currently offlining hotplug prevented based on the confidential
computing attribute which is set for Intel TDX. But TDX is not
the only possible user of the wake up method.
Introduce cpu_hotplug_not_supported() that can be called to indicate
that CPU offlining should be disabled.
This function is going to replace CC_ATTR_HOTPLUG_DISABLED for ACPI
MADT.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
include/linux/cpu.h | 2 ++
kernel/cpu.c | 13 ++++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
Comments
On Fri, 2023-10-20 at 18:12 +0300, Kirill A. Shutemov wrote: > ACPI MADT doesn't allow to offline CPU after it got woke up. > > Currently offlining hotplug prevented based on the confidential > computing attribute which is set for Intel TDX. But TDX is not > the only possible user of the wake up method. > > Introduce cpu_hotplug_not_supported() that can be called to indicate > that CPU offlining should be disabled. > > This function is going to replace CC_ATTR_HOTPLUG_DISABLED for ACPI > MADT. > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > --- > include/linux/cpu.h | 2 ++ > kernel/cpu.c | 13 ++++++++++++- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/include/linux/cpu.h b/include/linux/cpu.h > index f19f56501809..97832ced939d 100644 > --- a/include/linux/cpu.h > +++ b/include/linux/cpu.h > @@ -132,6 +132,7 @@ extern void cpus_read_lock(void); > extern void cpus_read_unlock(void); > extern int cpus_read_trylock(void); > extern void lockdep_assert_cpus_held(void); > +extern void cpu_hotplug_disable_offlining(void); > extern void cpu_hotplug_disable(void); > extern void cpu_hotplug_enable(void); > void clear_tasks_mm_cpumask(int cpu); > @@ -147,6 +148,7 @@ static inline void cpus_read_lock(void) { } > static inline void cpus_read_unlock(void) { } > static inline int cpus_read_trylock(void) { return true; } > static inline void lockdep_assert_cpus_held(void) { } > +static inline void cpu_hotplug_disable_offlining(void) { } > static inline void cpu_hotplug_disable(void) { } > static inline void cpu_hotplug_enable(void) { } > static inline int remove_cpu(unsigned int cpu) { return -EPERM; } > diff --git a/kernel/cpu.c b/kernel/cpu.c > index 6de7c6bb74ee..faebee0050a2 100644 > --- a/kernel/cpu.c > +++ b/kernel/cpu.c > @@ -484,6 +484,8 @@ static int cpu_hotplug_disabled; > > DEFINE_STATIC_PERCPU_RWSEM(cpu_hotplug_lock); > > +static bool cpu_hotplug_offline_disabled; > + > void cpus_read_lock(void) > { > percpu_down_read(&cpu_hotplug_lock); > @@ -543,6 +545,14 @@ static void lockdep_release_cpus_lock(void) > rwsem_release(&cpu_hotplug_lock.dep_map, _THIS_IP_); > } > > +/* Declare CPU offlining not supported */ > +void cpu_hotplug_disable_offlining(void) > +{ > + cpu_maps_update_begin(); > + cpu_hotplug_offline_disabled = true; > + cpu_maps_update_done(); > +} > + > /* > * Wait for currently running CPU hotplug operations to complete (if any) and > * disable future CPU hotplug (from sysfs). The 'cpu_add_remove_lock' protects > @@ -1507,7 +1517,8 @@ 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_HOTPLUG_DISABLED) || > + cpu_hotplug_offline_disabled) > return -EOPNOTSUPP; > if (cpu_hotplug_disabled) > return -EBUSY; IMHO it's a little bit odd to have two mechanisms in place, even in this middle state patch. Is it better to completely replace CC_ATTR_HOTPLUG_DISABLED with the new cpu_hotplug_offline_disabled in this patch? You can explicitly call cpu_hotplug_disable_offlining() in tdx_early_init() so no functional change is done. Or I am wondering why cannot just merge this and the next patch together, with a proper justification? Btw, IMHO the changelog (this and next patch's) seems didn't explain the true reason to replace CC_ATTR_HOTPLUG_DISABLED. Currently hotplug prevented based on the confidential computing attribute which is set for Intel TDX. But TDX is not the only possible user of the wake up method. "TDX is not the only possible user of the wake up method" doesn't mean we need to replace CC_ATTR_HOTPLUG_DISABLED. E.g., other CoCo VM type can also select CC_ATTR_HOTPLUG_DISABLED if it uses MADT wake up method. To me the true reason is the new MADT wake up version actually brings the support of offlining cpu, thus it's more suitable to decide whether the CoCo VM needs to disable CPU offline based on the MADT wake up version, but not the CC_* attributes that is determined by CoCo VM type.
On Mon, Oct 23, 2023 at 09:30:59AM +0000, Huang, Kai wrote: > IMHO it's a little bit odd to have two mechanisms in place, even in this middle > state patch. Is it better to completely replace CC_ATTR_HOTPLUG_DISABLED with > the new cpu_hotplug_offline_disabled in this patch? You can explicitly call > cpu_hotplug_disable_offlining() in tdx_early_init() so no functional change is > done. I can. But I don't see how it makes a difference. > Or I am wondering why cannot just merge this and the next patch together, with a > proper justification? Because the very next thing reviewers would ask is to split them :P > Btw, IMHO the changelog (this and next patch's) seems didn't explain the true > reason to replace CC_ATTR_HOTPLUG_DISABLED. > > Currently hotplug prevented based on the confidential computing > attribute which is set for Intel TDX. But TDX is not the only possible > user of the wake up method. > > "TDX is not the only possible user of the wake up method" doesn't mean we need > to replace CC_ATTR_HOTPLUG_DISABLED. E.g., other CoCo VM type can also select > CC_ATTR_HOTPLUG_DISABLED if it uses MADT wake up method. > > To me the true reason is the new MADT wake up version actually brings the > support of offlining cpu, thus it's more suitable to decide whether the CoCo VM > needs to disable CPU offline based on the MADT wake up version, but not the CC_* > attributes that is determined by CoCo VM type. No. MADT is orthogonal to CoCo. It can be implemented outside of CoCo environment and CoCo platform can implement other wake up methods. It is not up to TDX/SEV/whatever to decide if offlining is supported. It is property of the wakeup method implemented on the platform.
On Mon, 2023-10-23 at 18:31 +0300, kirill.shutemov@linux.intel.com wrote: > On Mon, Oct 23, 2023 at 09:30:59AM +0000, Huang, Kai wrote: > > IMHO it's a little bit odd to have two mechanisms in place, even in this middle > > state patch. Is it better to completely replace CC_ATTR_HOTPLUG_DISABLED with > > the new cpu_hotplug_offline_disabled in this patch? You can explicitly call > > cpu_hotplug_disable_offlining() in tdx_early_init() so no functional change is > > done. > > I can. But I don't see how it makes a difference. Personally I think this is better because it is odd to have two mechanisms in place even temporarily especially when we can avoid it. But no hard opinion. Up to you. > > > Or I am wondering why cannot just merge this and the next patch together, with a > > proper justification? > > Because the very next thing reviewers would ask is to split them :P > > > Btw, IMHO the changelog (this and next patch's) seems didn't explain the true > > reason to replace CC_ATTR_HOTPLUG_DISABLED. > > > > Currently hotplug prevented based on the confidential computing > > attribute which is set for Intel TDX. But TDX is not the only possible > > user of the wake up method. > > > > "TDX is not the only possible user of the wake up method" doesn't mean we need > > to replace CC_ATTR_HOTPLUG_DISABLED. E.g., other CoCo VM type can also select > > CC_ATTR_HOTPLUG_DISABLED if it uses MADT wake up method. > > > > To me the true reason is the new MADT wake up version actually brings the > > support of offlining cpu, thus it's more suitable to decide whether the CoCo VM > > needs to disable CPU offline based on the MADT wake up version, but not the CC_* > > attributes that is determined by CoCo VM type. > > No. MADT is orthogonal to CoCo. It can be implemented outside of CoCo > environment and CoCo platform can implement other wake up methods. It is > not up to TDX/SEV/whatever to decide if offlining is supported. It is > property of the wakeup method implemented on the platform. > Yeah sure. Can we put this to changelog to make it clearer? :-)
On Mon, Oct 23 2023 at 22:07, Kai Huang wrote: > On Mon, 2023-10-23 at 18:31 +0300, kirill.shutemov@linux.intel.com wrote: >> On Mon, Oct 23, 2023 at 09:30:59AM +0000, Huang, Kai wrote: >> > IMHO it's a little bit odd to have two mechanisms in place, even in this middle >> > state patch. Is it better to completely replace CC_ATTR_HOTPLUG_DISABLED with >> > the new cpu_hotplug_offline_disabled in this patch? You can explicitly call >> > cpu_hotplug_disable_offlining() in tdx_early_init() so no functional change is >> > done. >> >> I can. But I don't see how it makes a difference. > > Personally I think this is better because it is odd to have two mechanisms in > place even temporarily especially when we can avoid it. But no hard opinion. > Up to you. It's not at all better because having this clear split makes it simpler to review and ponder the individual changes. Thanks, tglx
On Fri, Oct 20 2023 at 18:12, Kirill A. Shutemov wrote: Subject: cpu/hotplug: ... > ACPI MADT doesn't allow to offline CPU after it got woke up. ACPI MADT is a table ... This is about the MADT mailbox wakeup method, right? > Currently offlining hotplug prevented based on the confidential is prevented > computing attribute which is set for Intel TDX. But TDX is not > the only possible user of the wake up method. > > Introduce cpu_hotplug_not_supported() that can be called to indicate > that CPU offlining should be disabled. Otherwise the changes look good!
On Sat, Oct 28, 2023 at 04:12:10PM +0200, Thomas Gleixner wrote: > On Fri, Oct 20 2023 at 18:12, Kirill A. Shutemov wrote: > > Subject: cpu/hotplug: ... > > > ACPI MADT doesn't allow to offline CPU after it got woke up. > > ACPI MADT is a table ... > > This is about the MADT mailbox wakeup method, right? > > > Currently offlining hotplug prevented based on the confidential > > is prevented > > > computing attribute which is set for Intel TDX. But TDX is not > > the only possible user of the wake up method. > > > > Introduce cpu_hotplug_not_supported() that can be called to indicate > > that CPU offlining should be disabled. > > Otherwise the changes look good! Thanks! Thomas, could you take a look if the last patch in the series is sane?
diff --git a/include/linux/cpu.h b/include/linux/cpu.h index f19f56501809..97832ced939d 100644 --- a/include/linux/cpu.h +++ b/include/linux/cpu.h @@ -132,6 +132,7 @@ extern void cpus_read_lock(void); extern void cpus_read_unlock(void); extern int cpus_read_trylock(void); extern void lockdep_assert_cpus_held(void); +extern void cpu_hotplug_disable_offlining(void); extern void cpu_hotplug_disable(void); extern void cpu_hotplug_enable(void); void clear_tasks_mm_cpumask(int cpu); @@ -147,6 +148,7 @@ static inline void cpus_read_lock(void) { } static inline void cpus_read_unlock(void) { } static inline int cpus_read_trylock(void) { return true; } static inline void lockdep_assert_cpus_held(void) { } +static inline void cpu_hotplug_disable_offlining(void) { } static inline void cpu_hotplug_disable(void) { } static inline void cpu_hotplug_enable(void) { } static inline int remove_cpu(unsigned int cpu) { return -EPERM; } diff --git a/kernel/cpu.c b/kernel/cpu.c index 6de7c6bb74ee..faebee0050a2 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -484,6 +484,8 @@ static int cpu_hotplug_disabled; DEFINE_STATIC_PERCPU_RWSEM(cpu_hotplug_lock); +static bool cpu_hotplug_offline_disabled; + void cpus_read_lock(void) { percpu_down_read(&cpu_hotplug_lock); @@ -543,6 +545,14 @@ static void lockdep_release_cpus_lock(void) rwsem_release(&cpu_hotplug_lock.dep_map, _THIS_IP_); } +/* Declare CPU offlining not supported */ +void cpu_hotplug_disable_offlining(void) +{ + cpu_maps_update_begin(); + cpu_hotplug_offline_disabled = true; + cpu_maps_update_done(); +} + /* * Wait for currently running CPU hotplug operations to complete (if any) and * disable future CPU hotplug (from sysfs). The 'cpu_add_remove_lock' protects @@ -1507,7 +1517,8 @@ 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_HOTPLUG_DISABLED) || + cpu_hotplug_offline_disabled) return -EOPNOTSUPP; if (cpu_hotplug_disabled) return -EBUSY;