As per Intel TDX Virtual Firmware Design Guide, sec titled "AP
initialization in OS" (index 4.3.5) and sec titled "Hotplug Device"
(index 9.4), all unused CPUs are put in spinning state by TDVF until
OS requests for CPU bring-up via mailbox address passed by ACPI MADT
table. Since by default all unused CPUs are always in spinning state,
there is no point in supporting dynamic CPU online/offline feature. So
current generation of TDVF does not support CPU hotplug feature. It may
be supported in next generation.
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
Reviewed-by: Andi Kleen <[email protected]>
Reviewed-by: Tony Luck <[email protected]>
---
Changes since v6:
* None
arch/x86/kernel/tdx.c | 16 ++++++++++++++++
arch/x86/kernel/topology.c | 3 ++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
index a66520405109..203deb57c4c9 100644
--- a/arch/x86/kernel/tdx.c
+++ b/arch/x86/kernel/tdx.c
@@ -4,6 +4,8 @@
#undef pr_fmt
#define pr_fmt(fmt) "tdx: " fmt
+#include <linux/cpuhotplug.h>
+
#include <asm/tdx.h>
#include <asm/vmx.h>
#include <asm/insn.h>
@@ -307,6 +309,17 @@ static int tdx_handle_mmio(struct pt_regs *regs, struct ve_info *ve)
return insn.length;
}
+static int tdx_cpu_offline_prepare(unsigned int cpu)
+{
+ /*
+ * Per Intel TDX Virtual Firmware Design Guide,
+ * sec 4.3.5 and sec 9.4, Hotplug is not supported
+ * in TDX platforms. So don't support CPU
+ * offline feature once it is turned on.
+ */
+ return -EOPNOTSUPP;
+}
+
unsigned long tdx_get_ve_info(struct ve_info *ve)
{
struct tdx_module_output out = {0};
@@ -451,5 +464,8 @@ void __init tdx_early_init(void)
pv_ops.irq.safe_halt = tdx_safe_halt;
pv_ops.irq.halt = tdx_halt;
+ cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "tdx:cpu_hotplug",
+ NULL, tdx_cpu_offline_prepare);
+
pr_info("Guest initialized\n");
}
diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index bd83748e2bde..ded34eda5bac 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -32,6 +32,7 @@
#include <linux/init.h>
#include <linux/smp.h>
#include <linux/irq.h>
+#include <linux/cc_platform.h>
#include <asm/io_apic.h>
#include <asm/cpu.h>
@@ -130,7 +131,7 @@ int arch_register_cpu(int num)
}
}
}
- if (num || cpu0_hotpluggable)
+ if ((num || cpu0_hotpluggable) && !cc_platform_has(CC_ATTR_GUEST_TDX))
per_cpu(cpu_devices, num).cpu.hotpluggable = 1;
return register_cpu(&per_cpu(cpu_devices, num).cpu, num);
--
2.25.1
On Tue, Oct 05 2021 at 16:05, Kuppuswamy Sathyanarayanan wrote:
>
> +static int tdx_cpu_offline_prepare(unsigned int cpu)
> +{
> + /*
> + * Per Intel TDX Virtual Firmware Design Guide,
> + * sec 4.3.5 and sec 9.4, Hotplug is not supported
> + * in TDX platforms. So don't support CPU
> + * offline feature once it is turned on.
> + */
> + return -EOPNOTSUPP;
> +}
> +
> unsigned long tdx_get_ve_info(struct ve_info *ve)
> {
> struct tdx_module_output out = {0};
> @@ -451,5 +464,8 @@ void __init tdx_early_init(void)
> pv_ops.irq.safe_halt = tdx_safe_halt;
> pv_ops.irq.halt = tdx_halt;
>
> + cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "tdx:cpu_hotplug",
> + NULL, tdx_cpu_offline_prepare);
Seriously? This lets the unplug start, which starts to kick off tasks
from the CPU just to make it fail a few steps later?
The obvious place to prevent this is the CPU hotplug code itself, right?
Thanks,
tglx
---
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 192e43a87407..c544eb6c79d3 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1178,6 +1178,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_HOTPLUG_DISABLED))
+ return -ENOTSUPP;
if (cpu_hotplug_disabled)
return -EBUSY;
return _cpu_down(cpu, 0, target);
On 10/17/21 12:23 PM, Thomas Gleixner wrote:
> Seriously? This lets the unplug start, which starts to kick off tasks
> from the CPU just to make it fail a few steps later?
>
> The obvious place to prevent this is the CPU hotplug code itself, right?
>
> Thanks,
>
> tglx
> ---
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 192e43a87407..c544eb6c79d3 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1178,6 +1178,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_HOTPLUG_DISABLED))
> + return -ENOTSUPP;
> if (cpu_hotplug_disabled)
> return -EBUSY;
> return _cpu_down(cpu, 0, target);
Makes sense. I will use it in next version.
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer