Hotpluggable CPUs MUST be exposed as 'online-capable' according to the new
change. However, cold-booted CPUs, if marked as 'online-capable' during boot
time, might not be detected by legacy operating systems. This could cause
compatibility problems.

Original Change Link: https://bugzilla.tianocore.org/show_bug.cgi?id=3706

Since updating the specification might take time, it is necessary to disable the
support for unplugging any cold-booted CPUs to preserve compatibility with
legacy operating systems.

Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 hw/arm/virt-acpi-build.c | 29 ++++++++++++++++++++---------
 hw/arm/virt.c            | 16 ++++++++++++++++
 include/hw/core/cpu.h    |  2 ++
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 6cb613103f..322ed8e35b 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -667,17 +667,28 @@ static uint32_t virt_acpi_get_gicc_flags(CPUState *cpu)
     }
 
     /*
-     * ARM GIC CPU Interface can be 'online-capable' or 'enabled' at boot
-     * We MUST set 'online-capable' bit for all hotpluggable CPUs except the
-     * first/boot CPU. Cold-booted CPUs without 'Id' can also be unplugged.
-     * Though as-of-now this is only used as a debugging feature.
+     * The ARM GIC CPU Interface can be either 'online-capable' or 'enabled' at
+     * boot. We MUST set the 'online-capable' bit for all hotpluggable CPUs.
      *
-     *   UEFI ACPI Specification 6.5
-     *   Section: 5.2.12.14. GIC CPU Interface (GICC) Structure
-     *   Table:   5.37 GICC CPU Interface Flags
-     *   Link: https://uefi.org/specs/ACPI/6.5
+     * Change Link: https://bugzilla.tianocore.org/show_bug.cgi?id=3706
+     *
+     * Refer to the UEFI ACPI Specification 6.5:
+     * Section: 5.2.12.14. GIC CPU Interface (GICC) Structure
+     * Table: 5.37 GICC CPU Interface Flags
+     * Link: https://uefi.org/specs/ACPI/6.5
+     *
+     * Cold-booted CPUs, except for the first/boot CPU, SHOULD be allowed to be
+     * hot(un)plugged as well. However, for this to happen, these CPUs MUST have
+     * the 'online-capable' bit set. This creates a compatibility problem with
+     * legacy OS, as it might ignore 'online-capable' bits during boot time, and
+     * hence some CPUs might not get detected.
+     *
+     * To fix this, the MADT GIC CPU interface flag should allow both
+     * 'online-capable' and 'enabled' bits to be set together. This change will
+     * require an update to the UEFI ACPI standard. Until this update occurs,
+     * all cold-booted CPUs should be exposed as 'enabled' only.
      */
-    return cpu && !cpu->cpu_index ? 1 : (1 << 3);
+    return cpu && cpu->cold_booted ? 1 : (1 << 3);
 }
 
 static void
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a2200099a1..770b599acf 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3163,6 +3163,10 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
      * This shall be used during the init of ACPI Hotplug state and hot-unplug
      */
      cs->acpi_persistent = true;
+
+    if (!dev->hotplugged) {
+        cs->cold_booted = true;
+    }
 }
 
 static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
@@ -3223,6 +3227,18 @@ static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,
         return;
     }
 
+    /*
+     * UEFI ACPI standard change is required to make both 'enabled' and the
+     * 'online-capable' bit co-exist instead of being mutually exclusive.
+     * check virt_acpi_get_gicc_flags() for more details.
+     *
+     * Disable the unplugging of cold-booted vCPUs as a temporary mitigation.
+     */
+    if (cs->cold_booted) {
+        error_setg(errp, "Hot-unplug of cold-booted CPU not supported!");
+        return;
+    }
+
     if (cs->cpu_index == first_cpu->cpu_index) {
         error_setg(errp, "Boot CPU(id%d=%d:%d:%d:%d) hot-unplug not supported",
                    first_cpu->cpu_index, cpu->socket_id, cpu->cluster_id,
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index e13e542177..99b699b47f 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -556,6 +556,8 @@ struct CPUState {
     uint32_t halted;
     int32_t exception_index;
 
+    bool cold_booted;
+
     AccelCPUState *accel;
 
     /* Used to keep track of an outstanding cpu throttle thread for migration
-- 
2.34.1