In function virt_cpu_plug(), it will send cpu plug message to interrupt
controller extioi and ipi irqchip. If there is problem in this function,
system should continue to run and keep state the same before cpu is
added.

Object cpuslot::cpu is set at last only when there is no any error.
If there is, send cpu unplug message to extioi and ipi irqchip, and then
return immediately.

Fixes: ab9935d2991e (hw/loongarch/virt: Implement cpu plug interface)
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 hw/loongarch/virt.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index a5840ff968..8563967c8b 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -981,8 +981,6 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
     LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
     Error *err = NULL;
 
-    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id);
-    cpu_slot->cpu = CPU(dev);
     if (lvms->ipi) {
         hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), dev, &err);
         if (err) {
@@ -995,6 +993,10 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
         hotplug_handler_plug(HOTPLUG_HANDLER(lvms->extioi), dev, &err);
         if (err) {
             error_propagate(errp, err);
+            if (lvms->ipi) {
+                hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->ipi), dev,
+                                       &error_abort);
+            }
             return;
         }
     }
@@ -1003,9 +1005,21 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
         hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &err);
         if (err) {
             error_propagate(errp, err);
+            if (lvms->ipi) {
+                hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->ipi), dev,
+                                       &error_abort);
+            }
+
+            if (lvms->extioi) {
+                hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->extioi), dev,
+                                       &error_abort);
+            }
+            return;
         }
     }
 
+    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id);
+    cpu_slot->cpu = CPU(dev);
     return;
 }
 
-- 
2.39.3

On Fri, 21 Mar 2025 11:12:55 +0800
Bibo Mao <maobibo@loongson.cn> wrote:

> In function virt_cpu_plug(), it will send cpu plug message to interrupt
> controller extioi and ipi irqchip. If there is problem in this function,
> system should continue to run and keep state the same before cpu is
> added.


_plug is not supposed to fail ever,
hence we are using error_abort.

so I'd drop those chunks 

> Object cpuslot::cpu is set at last only when there is no any error.
> If there is, send cpu unplug message to extioi and ipi irqchip, and then
> return immediately.

it doesn't matter when it's set since you are already in _plug() handler.
both way are correct.

(I'd set it as the last if calls to chained handlers do not depend on it)

> 
> Fixes: ab9935d2991e (hw/loongarch/virt: Implement cpu plug interface)
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  hw/loongarch/virt.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index a5840ff968..8563967c8b 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -981,8 +981,6 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
>      LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
>      Error *err = NULL;
>  
> -    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id);
> -    cpu_slot->cpu = CPU(dev);
>      if (lvms->ipi) {
>          hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), dev, &err);
>          if (err) {
> @@ -995,6 +993,10 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
>          hotplug_handler_plug(HOTPLUG_HANDLER(lvms->extioi), dev, &err);
>          if (err) {
>              error_propagate(errp, err);
> +            if (lvms->ipi) {
> +                hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->ipi), dev,
> +                                       &error_abort);
> +            }
>              return;
>          }
>      }
> @@ -1003,9 +1005,21 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
>          hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &err);
>          if (err) {
>              error_propagate(errp, err);
> +            if (lvms->ipi) {
> +                hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->ipi), dev,
> +                                       &error_abort);
> +            }
> +
> +            if (lvms->extioi) {
> +                hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->extioi), dev,
> +                                       &error_abort);
> +            }
> +            return;
>          }
>      }
>  
> +    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id);
> +    cpu_slot->cpu = CPU(dev);
>      return;
>  }
>