[PATCH for v11.0] hw/acpi: Do not save/load cpuhp state unconditionally

Zhao Liu posted 1 patch 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260330053008.2721532-1-zhao1.liu@intel.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>
hw/acpi/ich9.c  | 8 ++++++++
hw/acpi/piix4.c | 8 ++++++++
2 files changed, 16 insertions(+)
[PATCH for v11.0] hw/acpi: Do not save/load cpuhp state unconditionally
Posted by Zhao Liu 3 days ago
Commit 7aa563630b6b ("pc: Start with modern CPU hotplug interface
by default") removed the .needed callback (vmstate_test_use_cpuhp)
from vmstate_cpuhp_state in both piix4.c and ich9.c.

However, PIIX4 is also used by non-PC boards - MIPS Malta, which does
not select CONFIG_ACPI_CPU_HOTPLUG. For MIPS Malta, the linker resolves
vmstate_cpu_hotplug to the stub one in acpi-cpu-hotplug-stub.c, which is
a zero-initialized VMStateDescription with .fields == NULL.

Before commit 7aa563630b6b, .needed() of PIIX4's vmstate_cpuhp_state
returned false for MIPS Malta since PIIX4PMState always initialized the
field cpu_hotplug_legacy as true. Malta implicitly relies on this
initial value to bypass vmstate_cpuhp_state. However, this is unstable
because Malta itself does not support CPU hotplugging, whether via the
legacy way or the modern way.

Commit 7aa563630b6b removed .needed() check for vmstate_cpuhp_state,
this broke the existing dependency that Malta had relied on, forcing
Malta to save and load vmstate_cpuhp_state during the save/load process,
which in turn caused a segmentation fault due to NULL fields in the
stub-compiled code.

Fix this by bringing back the .needed = cpuhp_needed callback for
vmstate_cpuhp_state of PIIX4, that checks
MachineClass::has_hotpluggable_cpus. Boards that do not support CPU
hotplug (only MIPS Malta) will skip this subsection entirely, which
is both correct and consistent with the previous behavior.

At the same time, add a similar .needed() check to ICH9. Although no
boards with ICH9 are affected by this issue, this helps avoid potential
issues in the future.

Reproducer (MIPS Malta):
  $ qemu-img create -f qcow2 dummy.qcow2 32M
  $ qemu-system-mipsel -nographic \
      -drive if=none,format=qcow2,file=dummy.qcow2
  [Type "C-a c" to get the "(qemu)" monitor prompt)]
  (qemu) savevm foo    # segfault

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: 7aa563630b6b ("pc: Start with modern CPU hotplug interface by default")
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Tested with the following cases:
 * savevm for MIPS Malta
 * i386 migration:
   pc v10.2 (before 7aa563) <-> pc v10.2 (with this fix)
   q35 v10.2 (before 7aa563) <-> q35 v10.2 (with this fix)
   pc v10.2 (w/o this fix) <-> pc v10.2 (with this fix)
   q35 v10.2 (w/o this fix) <-> q35 v10.2 (with this fix)
   pc v11.0 (w/o this fix) <-> pc v11.0 (with this fix)
   q35 v11.0 (w/o this fix) <-> q35 v11.0 (with this fix)
---
 hw/acpi/ich9.c  | 8 ++++++++
 hw/acpi/piix4.c | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index bbb1bd60a206..5c7dfb2c69db 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -184,10 +184,18 @@ static const VMStateDescription vmstate_tco_io_state = {
     }
 };
 
+static bool cpuhp_needed(void *opaque)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+
+    return mc->has_hotpluggable_cpus;
+}
+
 static const VMStateDescription vmstate_cpuhp_state = {
     .name = "ich9_pm/cpuhp",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = cpuhp_needed,
     .fields = (const VMStateField[]) {
         VMSTATE_CPU_HOTPLUG(cpuhp_state, ICH9LPCPMRegs),
         VMSTATE_END_OF_LIST()
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 43860d122780..9b7f50c7afac 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -195,10 +195,18 @@ static const VMStateDescription vmstate_memhp_state = {
     }
 };
 
+static bool cpuhp_needed(void *opaque)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+
+    return mc->has_hotpluggable_cpus;
+}
+
 static const VMStateDescription vmstate_cpuhp_state = {
     .name = "piix4_pm/cpuhp",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = cpuhp_needed,
     .fields = (const VMStateField[]) {
         VMSTATE_CPU_HOTPLUG(cpuhp_state, PIIX4PMState),
         VMSTATE_END_OF_LIST()
-- 
2.34.1
Re: [PATCH for v11.0] hw/acpi: Do not save/load cpuhp state unconditionally
Posted by Philippe Mathieu-Daudé 2 days, 14 hours ago
On 30/3/26 07:30, Zhao Liu wrote:
> Commit 7aa563630b6b ("pc: Start with modern CPU hotplug interface
> by default") removed the .needed callback (vmstate_test_use_cpuhp)
> from vmstate_cpuhp_state in both piix4.c and ich9.c.
> 
> However, PIIX4 is also used by non-PC boards - MIPS Malta, which does
> not select CONFIG_ACPI_CPU_HOTPLUG. For MIPS Malta, the linker resolves
> vmstate_cpu_hotplug to the stub one in acpi-cpu-hotplug-stub.c, which is
> a zero-initialized VMStateDescription with .fields == NULL.
> 
> Before commit 7aa563630b6b, .needed() of PIIX4's vmstate_cpuhp_state
> returned false for MIPS Malta since PIIX4PMState always initialized the
> field cpu_hotplug_legacy as true. Malta implicitly relies on this
> initial value to bypass vmstate_cpuhp_state. However, this is unstable
> because Malta itself does not support CPU hotplugging, whether via the
> legacy way or the modern way.
> 
> Commit 7aa563630b6b removed .needed() check for vmstate_cpuhp_state,
> this broke the existing dependency that Malta had relied on, forcing
> Malta to save and load vmstate_cpuhp_state during the save/load process,
> which in turn caused a segmentation fault due to NULL fields in the
> stub-compiled code.
> 
> Fix this by bringing back the .needed = cpuhp_needed callback for
> vmstate_cpuhp_state of PIIX4, that checks
> MachineClass::has_hotpluggable_cpus. Boards that do not support CPU
> hotplug (only MIPS Malta) will skip this subsection entirely, which
> is both correct and consistent with the previous behavior.
> 
> At the same time, add a similar .needed() check to ICH9. Although no
> boards with ICH9 are affected by this issue, this helps avoid potential
> issues in the future.
> 
> Reproducer (MIPS Malta):
>    $ qemu-img create -f qcow2 dummy.qcow2 32M
>    $ qemu-system-mipsel -nographic \
>        -drive if=none,format=qcow2,file=dummy.qcow2
>    [Type "C-a c" to get the "(qemu)" monitor prompt)]
>    (qemu) savevm foo    # segfault
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: 7aa563630b6b ("pc: Start with modern CPU hotplug interface by default")
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Tested with the following cases:
>   * savevm for MIPS Malta
>   * i386 migration:
>     pc v10.2 (before 7aa563) <-> pc v10.2 (with this fix)
>     q35 v10.2 (before 7aa563) <-> q35 v10.2 (with this fix)
>     pc v10.2 (w/o this fix) <-> pc v10.2 (with this fix)
>     q35 v10.2 (w/o this fix) <-> q35 v10.2 (with this fix)
>     pc v11.0 (w/o this fix) <-> pc v11.0 (with this fix)
>     q35 v11.0 (w/o this fix) <-> q35 v11.0 (with this fix)
> ---
>   hw/acpi/ich9.c  | 8 ++++++++
>   hw/acpi/piix4.c | 8 ++++++++
>   2 files changed, 16 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH for v11.0] hw/acpi: Do not save/load cpuhp state unconditionally
Posted by Paolo Bonzini 2 days, 20 hours ago
Queued, thanks.

Paolo
Re: [PATCH for v11.0] hw/acpi: Do not save/load cpuhp state unconditionally
Posted by Peter Maydell 2 days, 21 hours ago
On Mon, 30 Mar 2026 at 06:04, Zhao Liu <zhao1.liu@intel.com> wrote:
>
> Commit 7aa563630b6b ("pc: Start with modern CPU hotplug interface
> by default") removed the .needed callback (vmstate_test_use_cpuhp)
> from vmstate_cpuhp_state in both piix4.c and ich9.c.
>
> However, PIIX4 is also used by non-PC boards - MIPS Malta, which does
> not select CONFIG_ACPI_CPU_HOTPLUG. For MIPS Malta, the linker resolves
> vmstate_cpu_hotplug to the stub one in acpi-cpu-hotplug-stub.c, which is
> a zero-initialized VMStateDescription with .fields == NULL.
>
> Before commit 7aa563630b6b, .needed() of PIIX4's vmstate_cpuhp_state
> returned false for MIPS Malta since PIIX4PMState always initialized the
> field cpu_hotplug_legacy as true. Malta implicitly relies on this
> initial value to bypass vmstate_cpuhp_state. However, this is unstable
> because Malta itself does not support CPU hotplugging, whether via the
> legacy way or the modern way.
>
> Commit 7aa563630b6b removed .needed() check for vmstate_cpuhp_state,
> this broke the existing dependency that Malta had relied on, forcing
> Malta to save and load vmstate_cpuhp_state during the save/load process,
> which in turn caused a segmentation fault due to NULL fields in the
> stub-compiled code.
>
> Fix this by bringing back the .needed = cpuhp_needed callback for
> vmstate_cpuhp_state of PIIX4, that checks
> MachineClass::has_hotpluggable_cpus. Boards that do not support CPU
> hotplug (only MIPS Malta) will skip this subsection entirely, which
> is both correct and consistent with the previous behavior.
>
> At the same time, add a similar .needed() check to ICH9. Although no
> boards with ICH9 are affected by this issue, this helps avoid potential
> issues in the future.
>
> Reproducer (MIPS Malta):
>   $ qemu-img create -f qcow2 dummy.qcow2 32M
>   $ qemu-system-mipsel -nographic \
>       -drive if=none,format=qcow2,file=dummy.qcow2
>   [Type "C-a c" to get the "(qemu)" monitor prompt)]
>   (qemu) savevm foo    # segfault
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: 7aa563630b6b ("pc: Start with modern CPU hotplug interface by default")
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Tested with the following cases:
>  * savevm for MIPS Malta
>  * i386 migration:
>    pc v10.2 (before 7aa563) <-> pc v10.2 (with this fix)
>    q35 v10.2 (before 7aa563) <-> q35 v10.2 (with this fix)
>    pc v10.2 (w/o this fix) <-> pc v10.2 (with this fix)
>    q35 v10.2 (w/o this fix) <-> q35 v10.2 (with this fix)
>    pc v11.0 (w/o this fix) <-> pc v11.0 (with this fix)
>    q35 v11.0 (w/o this fix) <-> q35 v11.0 (with this fix)

Tested-by: Peter Maydell <peter.maydell@linaro.org>

I filed a bug yesterday to track this, so we can also say
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3360

-- PMM