| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0a29fef814e51a2aa0030ec9cc97366718859411.camel@intel.com>
Date: Mon, 23 Oct 2023 09:30:59 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
"x86@...nel.org" <x86@...nel.org>, "bp@...en8.de" <bp@...en8.de>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>
CC: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
"Reshetova, Elena" <elena.reshetova@...el.com>,
"Nakajima, Jun" <jun.nakajima@...el.com>,
"rafael@...nel.org" <rafael@...nel.org>,
"peterz@...radead.org" <peterz@...radead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"sathyanarayanan.kuppuswamy@...ux.intel.com"
<sathyanarayanan.kuppuswamy@...ux.intel.com>,
"Hunter, Adrian" <adrian.hunter@...el.com>,
"thomas.lendacky@....com" <thomas.lendacky@....com>,
"ashish.kalra@....com" <ashish.kalra@....com>,
"kexec@...ts.infradead.org" <kexec@...ts.infradead.org>,
"Christopherson,, Sean" <seanjc@...gle.com>,
"bhe@...hat.com" <bhe@...hat.com>,
"linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>
Subject: Re: [PATCHv2 02/13] kernel/cpu: Add support for declaring CPU
offlining not supported
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@...ux.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.
Powered by blists - more mailing lists