Re: [PATCH] hw/loongarch/virt: Fix the cpu hotplug issue

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 29 Aug 2025 10:31:57 +0800
Bibo Mao <maobibo@loongson.cn> wrote:

> On 2025/8/29 上午9:32, Xianglai Li wrote:
> > The hot-plugged cpu does not register the cpu reset function, so the cpu
> > plugged in later cannot reset properly, and there will be problems when
> > restarting.
> > 
> > Now register the cpu reset function in the cpu hotplug callback function.  
> Oh, it is actually one problem and it is missing :(. There is similiar 
> patch posted at:
>  
> https://lore.kernel.org/qemu-devel/20241031065418.3111892-1-maobibo@loongson.cn/
> 
> I prefer to adding cpu reset in CPU object realize function rather 
> hotplug handler, and executing reset_load_elf() in board specific
> reset callback. However peter has different thoughts.

this patch and above mentioned one expose direct boot specific
reset_load_elf() on a generic hardware bring up path. That's
probably isn't right.

Ideally default (cpu_reset()) reset would happen 1st during
cpu creation/board init and then direct boot would patch
the CPU that actually would execute payload.

PS:
what's the reason to call reset_load_elf() on all present
vCPUs and why hotplugged ones matter here?

Shouldn't direct booted QEMU patch BSP only?

> 
> Regards
> Bibo Mao
> > 
> > Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
> > ---
> >   hw/loongarch/boot.c         | 8 +-------
> >   hw/loongarch/virt.c         | 4 ++++
> >   include/hw/loongarch/virt.h | 1 +
> >   3 files changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
> > index 14d6c52d4e..6bc1f3d50c 100644
> > --- a/hw/loongarch/boot.c
> > +++ b/hw/loongarch/boot.c
> > @@ -324,7 +324,7 @@ static int64_t load_kernel_info(struct loongarch_boot_info *info)
> >       return kernel_entry;
> >   }
> >   
> > -static void reset_load_elf(void *opaque)
> > +void reset_load_elf(void *opaque)
> >   {
> >       LoongArchCPU *cpu = opaque;
> >       CPULoongArchState *env = &cpu->env;
> > @@ -429,12 +429,6 @@ static void loongarch_direct_kernel_boot(MachineState *ms,
> >   void loongarch_load_kernel(MachineState *ms, struct loongarch_boot_info *info)
> >   {
> >       LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(ms);
> > -    int i;
> > -
> > -    /* register reset function */
> > -    for (i = 0; i < ms->smp.cpus; i++) {
> > -        qemu_register_reset(reset_load_elf, LOONGARCH_CPU(qemu_get_cpu(i)));
> > -    }
> >   
> >       info->kernel_filename = ms->kernel_filename;
> >       info->kernel_cmdline = ms->kernel_cmdline;
> > diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> > index b15ada2078..71f8ddc980 100644
> > --- a/hw/loongarch/virt.c
> > +++ b/hw/loongarch/virt.c
> > @@ -1013,6 +1013,8 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev,
> >       /* Notify acpi ged CPU removed */
> >       hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &error_abort);
> >   
> > +    /* unregister reset function */
> > +    qemu_unregister_reset(reset_load_elf, cpu);
> >       cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id);
> >       cpu_slot->cpu = NULL;
> >   }
> > @@ -1037,6 +1039,8 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
> >                                &error_abort);
> >       }
> >   
> > +    /* register reset function */
> > +    qemu_register_reset(reset_load_elf, cpu);
> >       cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id);
> >       cpu_slot->cpu = CPU(dev);
> >   }
> > diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> > index 602feab0f0..15ea393386 100644
> > --- a/include/hw/loongarch/virt.h
> > +++ b/include/hw/loongarch/virt.h
> > @@ -71,6 +71,7 @@ struct LoongArchVirtMachineState {
> >   OBJECT_DECLARE_SIMPLE_TYPE(LoongArchVirtMachineState, LOONGARCH_VIRT_MACHINE)
> >   void virt_acpi_setup(LoongArchVirtMachineState *lvms);
> >   void virt_fdt_setup(LoongArchVirtMachineState *lvms);
> > +void reset_load_elf(void *opaque);
> >   
> >   static inline bool virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms)
> >   {
> >   
> 
>