On Tue, 2022-07-19 at 10:46 -0700, Dave Hansen wrote:
> On 7/13/22 04:09, Kai Huang wrote:
> ...
> > "TDX doesn’t support adding or removing CPUs from TDX security perimeter. The
> > BIOS should prevent CPUs from being hot-added or hot-removed after platform
> > boots."
>
> That's a start. It also probably needs to say that the security
> perimeter includes all logical CPUs, though.
To me it is kinda implied. But I have sent email to TDX spec owner to see
whether we can say it more explicitly.
>
> > static int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid)
> > {
> > @@ -819,6 +820,12 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid,
> > u32 acpi_id,
> > {
> > int cpu;
> >
> > + if (platform_tdx_enabled()) {
> > + pr_err("BIOS bug: CPU (physid %u) hot-added on TDX enabled
> > platform. Reject it.\n",
> > + physid);
> > + return -EINVAL;
> > + }
>
> Is this the right place? There are other sanity checks in
> acpi_processor_hotadd_init() and it seems like a better spot.
It has below additional check:
if (invalid_phys_cpuid(pr->phys_id))
return -ENODEV;
status = acpi_evaluate_integer(pr->handle, "_STA", NULL, &sta);
if (ACPI_FAILURE(status) || !(sta & ACPI_STA_DEVICE_PRESENT))
return -ENODEV;
I don't know exactly when will the first "invalid_phys_cpuid()" case happen, but
the CPU is enumerated as "present" only after the second check. I.e. if BIOS is
buggy and somehow sends a ACPI CPU hot-add event to kernel w/o having the CPU
being actually hot-added, the kernel just returns -ENODEV here.
So to me, adding to acpi_map_cpu() is more reasonable, because by reaching here,
it is sure that a real CPU is being hot-added.
>
> > cpu = acpi_register_lapic(physid, acpi_id, ACPI_MADT_ENABLED);
> > if (cpu < 0) {
> > pr_info("Unable to map lapic to logical cpu number\n");
> > @@ -835,6 +842,10 @@ EXPORT_SYMBOL(acpi_map_cpu);
> >
> > int acpi_unmap_cpu(int cpu)
> > {
> > + if (platform_tdx_enabled())
> > + pr_err("BIOS bug: CPU %d hot-removed on TDX enabled platform.
> > TDX is broken. Please reboot the machine.\n",
> > + cpu);
> > +
> > #ifdef CONFIG_ACPI_NUMA
> > set_apicid_to_node(per_cpu(x86_cpu_to_apicid, cpu), NUMA_NO_NODE);
> > #endif
>
