| Message ID | 20241014192205.253479-5-salil.mehta@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Arch agnostic ACPI changes to support vCPU Hotplug (on Archs like ARM) | expand |
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 > }
Hi Igor, > From: Igor Mammedov <imammedo@redhat.com> > Sent: Friday, October 18, 2024 3:31 PM > To: Salil Mehta <salil.mehta@huawei.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, 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 > > } >
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), 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() + } +}; + 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 }
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(+)