Update the `AcpiCpuStatus` for `is_enabled` and `is_present` accordingly when
vCPUs are hot-plugged or hot-unplugged, taking into account the *persistence*
of the vCPUs.

Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 hw/acpi/cpu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 083c4010c2..700aa855e9 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -291,6 +291,8 @@ void acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
     }
 
     cdev->cpu = CPU(dev);
+    cdev->is_present = true;
+    cdev->is_enabled = true;
     if (dev->hotplugged) {
         cdev->is_inserting = true;
         acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS);
@@ -322,6 +324,11 @@ void acpi_cpu_unplug_cb(CPUHotplugState *cpu_st,
         return;
     }
 
+    cdev->is_enabled = false;
+    if (!acpi_persistent_cpu(CPU(dev))) {
+        cdev->is_present = false;
+    }
+
     cdev->cpu = NULL;
 }
 
-- 
2.34.1

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

> Update the `AcpiCpuStatus` for `is_enabled` and `is_present` accordingly when
> vCPUs are hot-plugged or hot-unplugged, taking into account the *persistence*
> of the vCPUs.
> 
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  hw/acpi/cpu.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 083c4010c2..700aa855e9 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -291,6 +291,8 @@ void acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
>      }
>  
>      cdev->cpu = CPU(dev);
> +    cdev->is_present = true;
> +    cdev->is_enabled = true;

hmm, if cpu is always present, then these fields are redundant
since
  (!cdev->cpu) == present
and
  then is_enabled could be fetched from cdev->cpu directly

>      if (dev->hotplugged) {
>          cdev->is_inserting = true;
>          acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS);
> @@ -322,6 +324,11 @@ void acpi_cpu_unplug_cb(CPUHotplugState *cpu_st,
>          return;
>      }
>  
> +    cdev->is_enabled = false;
> +    if (!acpi_persistent_cpu(CPU(dev))) {
> +        cdev->is_present = false;
> +    }

and other way around works as well.

then we don't have to carry around extra state and making sure that it's in sync/migrated.

> +
>      cdev->cpu = NULL;
>  }
>

Hi Igor,

>  From: Igor Mammedov <imammedo@redhat.com>
>  Sent: Friday, October 18, 2024 3:18 PM
>  To: Salil Mehta <salil.mehta@huawei.com>
>  
>  On Mon, 14 Oct 2024 20:22:03 +0100
>  Salil Mehta <salil.mehta@huawei.com> wrote:
>  
>  > Update the `AcpiCpuStatus` for `is_enabled` and `is_present`
>  > accordingly when vCPUs are hot-plugged or hot-unplugged, taking into
>  > account the *persistence* of the vCPUs.
>  >
>  > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
>  > ---
>  >  hw/acpi/cpu.c | 7 +++++++
>  >  1 file changed, 7 insertions(+)
>  >
>  > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index
>  > 083c4010c2..700aa855e9 100644
>  > --- a/hw/acpi/cpu.c
>  > +++ b/hw/acpi/cpu.c
>  > @@ -291,6 +291,8 @@ void acpi_cpu_plug_cb(HotplugHandler
>  *hotplug_dev,
>  >      }
>  >
>  >      cdev->cpu = CPU(dev);
>  > +    cdev->is_present = true;
>  > +    cdev->is_enabled = true;
>  
>  hmm, if cpu is always present, then these fields are redundant since
>    (!cdev->cpu) == present
>  and
>    then is_enabled could be fetched from cdev->cpu directly


I believe you are assuming, that all the QOM possible vCPUs objects will
exists (and hence realized) for the entire life time of the VM? If yes, then
we need to first debate on that as we cannot let presence of all the possible
vCPUs affect  the boot time. This is one of the key requirement from this
feature.


>  
>  >      if (dev->hotplugged) {
>  >          cdev->is_inserting = true;
>  >          acpi_send_event(DEVICE(hotplug_dev),
>  > ACPI_CPU_HOTPLUG_STATUS); @@ -322,6 +324,11 @@ void
>  acpi_cpu_unplug_cb(CPUHotplugState *cpu_st,
>  >          return;
>  >      }
>  >
>  > +    cdev->is_enabled = false;
>  > +    if (!acpi_persistent_cpu(CPU(dev))) {
>  > +        cdev->is_present = false;
>  > +    }
>  
>  and other way around works as well.
>  
>  then we don't have to carry around extra state and making sure that it's in
>  sync/migrated.
>  
>  > +
>  >      cdev->cpu = NULL;
>  >  }
>  >
>