Re: [PATCH v3 3/3] hw/loongarch/virt: Register reset interface with CPU object

[Igor Mammedov <imammedo@redhat.com>]

On Wed,  3 Sep 2025 10:35:56 +0800
Bibo Mao <maobibo@loongson.cn> wrote:

> With cpu hotplug is implemented on LoongArch virt machine, reset
> interface with hot-added CPU should be registered. Otherwise there
> will be problem if system reboots after cpu is hot-added.
> 
> Now register reset interface with CPU object is realized and remove
> the reset interface with CPU object unrealizd.
> 
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  hw/loongarch/boot.c    | 13 -------------
>  target/loongarch/cpu.c |  4 ++++
>  2 files changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
> index 5799b4c75c..a516415822 100644
> --- a/hw/loongarch/boot.c
> +++ b/hw/loongarch/boot.c
> @@ -350,13 +350,6 @@ static int64_t load_kernel_info(struct loongarch_boot_info *info)
>      return kernel_entry;
>  }
>  
> -static void reset_load_elf(void *opaque)
> -{
> -    LoongArchCPU *cpu = opaque;
> -
> -    cpu_reset(CPU(cpu));
> -}
> -
>  static void fw_cfg_add_kernel_info(struct loongarch_boot_info *info,
>                                     FWCfgState *fw_cfg)
>  {
> @@ -439,12 +432,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/target/loongarch/cpu.c b/target/loongarch/cpu.c
> index 3a7621c0ea..9edb8ebc4d 100644
> --- a/target/loongarch/cpu.c
> +++ b/target/loongarch/cpu.c
> @@ -668,6 +668,9 @@ static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>  
>      qemu_init_vcpu(cs);
>      cpu_reset(cs);
> + #ifndef CONFIG_USER_ONLY
> +    qemu_register_resettable(OBJECT(dev));
> + #endif

I'd put this in virt_cpu_plug() as last step, which should work both for
cold and hotpluged cpus. And drop CONFIG_USER_ONLY while at it.

with that
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

PS:
the rest of the patches are very arch specific so I won't review them.

>  
>      lacc->parent_realize(dev, errp);
>  }
> @@ -678,6 +681,7 @@ static void loongarch_cpu_unrealizefn(DeviceState *dev)
>  
>  #ifndef CONFIG_USER_ONLY
>      cpu_remove_sync(CPU(dev));
> +    qemu_unregister_resettable(OBJECT(dev));
>  #endif
>  
>      lacc->parent_unrealize(dev);