On Fri, 21 Mar 2025 11:12:55 +0800
Bibo Mao <maob...@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 <maob...@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;
> }
>
