Re: [PATCH V1 4/4] hw/acpi: Populate vCPU Hotplug VMSD to migrate `is_{present,enabled}` states

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 14 Oct 2024 20:22:05 +0100
Salil Mehta <salil.mehta@huawei.com> wrote:

> The ACPI CPU hotplug states `is_{present, enabled}` must be migrated alongside
> other vCPU hotplug states to the destination VM. Therefore, they should be
> integrated into the existing CPU Hotplug VM State Description (VMSD) table.
> Depending on the architecture and its implementation of CPU hotplug events
> (such as ACPI GED, etc.), the CPU hotplug states should be populated
> appropriately within their corresponding subsections of the VMSD table.
> 
>     "acpi-ged (16)": {
>         "ged_state": {
>             "sel": "0x00000000"
>         },
>         [...]
>         "acpi-ged/cpuhp": {
>             "cpuhp_state": {
>                 "selector": "0x00000005",
>                 "command": "0x00",
>                 "devs": [
>                     {
>                         "is_inserting": false,
>                         "is_removing": false,
>                         "is_present": true,
>                         "is_enabled": true,
>                         "ost_event": "0x00000000",
>                         "ost_status": "0x00000000"
>                     },
>                     {
>                         "is_inserting": false,
>                         "is_removing": false,
>                         "is_present": true,
>                         "is_enabled": true,
>                         "ost_event": "0x00000000",
>                         "ost_status": "0x00000000"
>                     },
>                     {
>                         "is_inserting": false,
>                         "is_removing": false,
>                         "is_present": true,
>                         "is_enabled": true,
>                         "ost_event": "0x00000000",
>                         "ost_status": "0x00000000"
>                     },
>                     {
>                         "is_inserting": false,
>                         "is_removing": false,
>                         "is_present": true,
>                         "is_enabled": true,
>                         "ost_event": "0x00000000",
>                         "ost_status": "0x00000000"
>                     },
>                     {
>                         "is_inserting": false,
>                         "is_removing": false,
>                         "is_present": true,
>                         "is_enabled": false,
>                         "ost_event": "0x00000000",
>                         "ost_status": "0x00000000"
>                     },
>                     {
>                         "is_inserting": false,
>                         "is_removing": false,
>                         "is_present": true,
>                         "is_enabled": false,
>                         "ost_event": "0x00000000",
>                         "ost_status": "0x00000000"
>                     }
>                 ]
>             }
>         }
>     },
> 
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  hw/acpi/cpu.c                  |  2 ++
>  hw/acpi/generic_event_device.c | 11 +++++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 23ea2b9c70..d34c1e601e 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -340,6 +340,8 @@ static const VMStateDescription vmstate_cpuhp_sts = {
>      .fields = (const VMStateField[]) {
>          VMSTATE_BOOL(is_inserting, AcpiCpuStatus),
>          VMSTATE_BOOL(is_removing, AcpiCpuStatus),
> +        VMSTATE_BOOL(is_present, AcpiCpuStatus),
> +        VMSTATE_BOOL(is_enabled, AcpiCpuStatus),

that's likely will break x86 migration,
but before bothering peterx, maybe we won't need this hunk if
is_enabled is migrated as part of vCPU state.

>          VMSTATE_UINT32(ost_event, AcpiCpuStatus),
>          VMSTATE_UINT32(ost_status, AcpiCpuStatus),
>          VMSTATE_END_OF_LIST()
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index 15b4c3ebbf..a4d78a534c 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c
> @@ -331,6 +331,16 @@ static const VMStateDescription vmstate_memhp_state = {
>      }
>  };
>  
> +static const VMStateDescription vmstate_cpuhp_state = {
> +    .name = "acpi-ged/cpuhp",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};

this subsection likely needs is_needed hook to avoid breaking
case where target doesn't have cpuhp support (older QEMU)

> +
>  static const VMStateDescription vmstate_ged_state = {
>      .name = "acpi-ged-state",
>      .version_id = 1,
> @@ -379,6 +389,7 @@ static const VMStateDescription vmstate_acpi_ged = {
>      },
>      .subsections = (const VMStateDescription * const []) {
>          &vmstate_memhp_state,
> +        &vmstate_cpuhp_state,
>          &vmstate_ghes_state,
>          NULL
>      }