Hi Igor,
> From: Igor Mammedov <imamm...@redhat.com>
> Sent: Friday, October 18, 2024 3:31 PM
> To: Salil Mehta <salil.me...@huawei.com>
>
> On Mon, 14 Oct 2024 20:22:05 +0100
> Salil Mehta <salil.me...@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.me...@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,
Point taken. It would helpful if you can elucidate further how that
might happen?
> but before bothering peterx, maybe we won't need this hunk if is_enabled
> is migrated as part of vCPU state.
Again. This entire argument hinges on the fact whether to keep all the
possible vCPUs realized or not. I think we need a thorough discussion
on that first so that pain of all the stakeholders can be addressed.
> > 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)
Sure. AFAICS by checking the usage of this in the other parts of the code
and please correct me if I'm wrong here,
#1. it is used at the source VM
#2. used to decide whether to include certain subsection in the state
being migrated.
Q1: How does the source VM know what version is there at the
destination VM? or is it managed by the administrator?
Q2: Or maybe the is_needed hooks provides a way for the administrator
to configure that at the source?
>
> > +
> > 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
> > }
>
