From: Kuppuswamy Sathyanarayanan <[email protected]>
Unlike regular VMs, TDX guests use the firmware hand-off wakeup method
to wake up the APs during the boot process. This wakeup model uses a
mailbox to communicate with firmware to bring up the APs. As per the
design, this mailbox can only be used once for the given AP, which means
after the APs are booted, the same mailbox cannot be used to
offline/online the given AP. More details about this requirement can be
found in Intel TDX Virtual Firmware Design Guide, sec titled "AP
initialization in OS" and in sec titled "Hotplug Device".
Since the architecture does not support any method of offlining the
CPUs, disable CPU hotplug support in the kernel.
Since this hotplug disable feature can be re-used by other VM guests,
add a new CC attribute CC_ATTR_HOTPLUG_DISABLED and use it to disable
the hotplug support.
With hotplug disabled, /sys/devices/system/cpu/cpuX/online sysfs option
will not exist for TDX guests.
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
Reviewed-by: Andi Kleen <[email protected]>
Reviewed-by: Tony Luck <[email protected]>
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
arch/x86/kernel/cc_platform.c | 7 ++++++-
include/linux/cc_platform.h | 10 ++++++++++
kernel/cpu.c | 3 +++
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c
index 8da246ab4339..dcb31d6a7554 100644
--- a/arch/x86/kernel/cc_platform.c
+++ b/arch/x86/kernel/cc_platform.c
@@ -17,8 +17,13 @@
static bool intel_cc_platform_has(enum cc_attr attr)
{
- if (attr == CC_ATTR_GUEST_UNROLL_STRING_IO)
+ switch (attr) {
+ case CC_ATTR_GUEST_UNROLL_STRING_IO:
+ case CC_ATTR_HOTPLUG_DISABLED:
return true;
+ default:
+ return false;
+ }
return false;
}
diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
index efd8205282da..691494bbaf5a 100644
--- a/include/linux/cc_platform.h
+++ b/include/linux/cc_platform.h
@@ -72,6 +72,16 @@ enum cc_attr {
* Examples include TDX guest & SEV.
*/
CC_ATTR_GUEST_UNROLL_STRING_IO,
+
+ /**
+ * @CC_ATTR_HOTPLUG_DISABLED: 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,
};
#ifdef CONFIG_ARCH_HAS_CC_PLATFORM
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 407a2568f35e..58fd06ebc2c8 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -34,6 +34,7 @@
#include <linux/scs.h>
#include <linux/percpu-rwsem.h>
#include <linux/cpuset.h>
+#include <linux/cc_platform.h>
#include <trace/events/power.h>
#define CREATE_TRACE_POINTS
@@ -1185,6 +1186,8 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen,
static int cpu_down_maps_locked(unsigned int cpu, enum cpuhp_state target)
{
+ if (cc_platform_has(CC_ATTR_HOTPLUG_DISABLED))
+ return -EOPNOTSUPP;
if (cpu_hotplug_disabled)
return -EBUSY;
return _cpu_down(cpu, 0, target);
--
2.34.1
On Wed, Feb 02 2022 at 01:09, Thomas Gleixner wrote:
> On Mon, Jan 24 2022 at 18:02, Kirill A. Shutemov wrote:
>> static bool intel_cc_platform_has(enum cc_attr attr)
>> {
>> - if (attr == CC_ATTR_GUEST_UNROLL_STRING_IO)
>> + switch (attr) {
>> + case CC_ATTR_GUEST_UNROLL_STRING_IO:
>> + case CC_ATTR_HOTPLUG_DISABLED:
Not that I care much, but I faintly remember that I suggested that in
one of the gazillion of threads.
On Mon, Jan 24 2022 at 18:02, Kirill A. Shutemov wrote:
> static bool intel_cc_platform_has(enum cc_attr attr)
> {
> - if (attr == CC_ATTR_GUEST_UNROLL_STRING_IO)
> + switch (attr) {
> + case CC_ATTR_GUEST_UNROLL_STRING_IO:
> + case CC_ATTR_HOTPLUG_DISABLED:
> return true;
> + default:
> + return false;
> + }
>
> return false;
Sigh. If 'default:' returns false then this final return cannot be
reached, no?
Thanks,
tglx
On Wed, Feb 02, 2022 at 01:11:56AM +0100, Thomas Gleixner wrote:
> On Wed, Feb 02 2022 at 01:09, Thomas Gleixner wrote:
>
> > On Mon, Jan 24 2022 at 18:02, Kirill A. Shutemov wrote:
> >> static bool intel_cc_platform_has(enum cc_attr attr)
> >> {
> >> - if (attr == CC_ATTR_GUEST_UNROLL_STRING_IO)
> >> + switch (attr) {
> >> + case CC_ATTR_GUEST_UNROLL_STRING_IO:
> >> + case CC_ATTR_HOTPLUG_DISABLED:
>
> Not that I care much, but I faintly remember that I suggested that in
> one of the gazillion of threads.
Right, and yeah, adding a separate attribute is ok too but we already
have a hotplug disable method. Why can't this call
cpu_hotplug_disable()
on the TDX init path somewhere and have this be even simpler?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Thu, Feb 03 2022 at 16:00, Borislav Petkov wrote:
> On Wed, Feb 02, 2022 at 01:11:56AM +0100, Thomas Gleixner wrote:
>> On Wed, Feb 02 2022 at 01:09, Thomas Gleixner wrote:
>>
>> > On Mon, Jan 24 2022 at 18:02, Kirill A. Shutemov wrote:
>> >> static bool intel_cc_platform_has(enum cc_attr attr)
>> >> {
>> >> - if (attr == CC_ATTR_GUEST_UNROLL_STRING_IO)
>> >> + switch (attr) {
>> >> + case CC_ATTR_GUEST_UNROLL_STRING_IO:
>> >> + case CC_ATTR_HOTPLUG_DISABLED:
>>
>> Not that I care much, but I faintly remember that I suggested that in
>> one of the gazillion of threads.
>
> Right, and yeah, adding a separate attribute is ok too but we already
> have a hotplug disable method. Why can't this call
>
> cpu_hotplug_disable()
>
> on the TDX init path somewhere and have this be even simpler?
That's daft. I rather have this explicit control which makes it obvious
what's going on.
Thanks,
tglx