qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hw/loongarch/virt: Add reset interface for virt-machine
@ 2024-10-31  6:54 Bibo Mao
  2024-10-31 16:16 ` Philippe Mathieu-Daudé
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Bibo Mao @ 2024-10-31  6:54 UTC (permalink / raw)
  To: Song Gao; +Cc: Jiaxun Yang, qemu-devel

With generic cpu reset interface, pc register is entry of FLASH for
UEFI BIOS. However with direct kernel booting requirement, there is
little different, pc register of primary cpu is entry address of ELF
file.

At the same time with requirement of cpu hotplug, hot-added CPU should
register reset interface for this cpu object. Now reset callback is
not registered for hot-added CPU.

With this patch reset callback for CPU is register when CPU instance
is created, and reset interface is added for virt-machine board. In
reset interface of virt-machine, reset for direct kernel booting
requirement is called.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 hw/loongarch/boot.c         |  9 +--------
 hw/loongarch/virt.c         | 14 ++++++++++++++
 include/hw/loongarch/boot.h |  1 +
 target/loongarch/cpu.c      | 10 ++++++++++
 4 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
index cb668703bd..cbb4e3737d 100644
--- a/hw/loongarch/boot.c
+++ b/hw/loongarch/boot.c
@@ -216,12 +216,11 @@ 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;
 
-    cpu_reset(CPU(cpu));
     if (env->load_elf) {
 	if (cpu == LOONGARCH_CPU(first_cpu)) {
             env->gpr[4] = env->boot_info->a0;
@@ -320,12 +319,6 @@ static void loongarch_direct_kernel_boot(struct loongarch_boot_info *info)
 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 9a635d1d3d..80680d178c 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -1434,6 +1434,19 @@ static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx)
     }
 }
 
+static void virt_reset(MachineState *machine, ResetType type)
+{
+    CPUState *cs;
+
+    /* Reset all devices including CPU devices */
+    qemu_devices_reset(type);
+
+    /* Reset PC and register context for kernel direct booting method */
+    CPU_FOREACH(cs) {
+        reset_load_elf(LOONGARCH_CPU(cs));
+    }
+}
+
 static void virt_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -1457,6 +1470,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
     mc->auto_enable_numa_with_memdev = true;
     mc->get_hotplug_handler = virt_get_hotplug_handler;
     mc->default_nic = "virtio-net-pci";
+    mc->reset = virt_reset;
     hc->plug = virt_device_plug_cb;
     hc->pre_plug = virt_device_pre_plug;
     hc->unplug_request = virt_device_unplug_request;
diff --git a/include/hw/loongarch/boot.h b/include/hw/loongarch/boot.h
index b3b870df1f..c7020ec9bb 100644
--- a/include/hw/loongarch/boot.h
+++ b/include/hw/loongarch/boot.h
@@ -115,5 +115,6 @@ struct memmap_entry {
 };
 
 void loongarch_load_kernel(MachineState *ms, struct loongarch_boot_info *info);
+void reset_load_elf(void *opaque);
 
 #endif /* HW_LOONGARCH_BOOT_H */
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 7212fb5f8f..f7f8fcc024 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -592,6 +592,13 @@ static void loongarch_cpu_disas_set_info(CPUState *s, disassemble_info *info)
     info->print_insn = print_insn_loongarch;
 }
 
+#ifndef CONFIG_USER_ONLY
+static void loongarch_cpu_reset_cb(void *opaque)
+{
+    cpu_reset((CPUState *) opaque);
+}
+#endif
+
 static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
@@ -607,6 +614,9 @@ static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
     loongarch_cpu_register_gdb_regs_for_features(cs);
 
     cpu_reset(cs);
+#ifndef CONFIG_USER_ONLY
+    qemu_register_reset(loongarch_cpu_reset_cb, dev);
+#endif
     qemu_init_vcpu(cs);
 
     lacc->parent_realize(dev, errp);

base-commit: 58d49b5895f2e0b5cfe4b2901bf24f3320b74f29
-- 
2.39.3



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] hw/loongarch/virt: Add reset interface for virt-machine
  2024-10-31  6:54 [PATCH] hw/loongarch/virt: Add reset interface for virt-machine Bibo Mao
@ 2024-10-31 16:16 ` Philippe Mathieu-Daudé
  2024-11-07 11:26   ` gaosong
  2024-11-07 11:28 ` Peter Maydell
  2025-08-29  7:45 ` lixianglai
  2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-10-31 16:16 UTC (permalink / raw)
  To: Bibo Mao, Song Gao, Peter Maydell; +Cc: Jiaxun Yang, qemu-devel

Cc'ing Peter who is working on the Reset API.

On 31/10/24 03:54, Bibo Mao wrote:
> With generic cpu reset interface, pc register is entry of FLASH for
> UEFI BIOS. However with direct kernel booting requirement, there is
> little different, pc register of primary cpu is entry address of ELF
> file.
> 
> At the same time with requirement of cpu hotplug, hot-added CPU should
> register reset interface for this cpu object. Now reset callback is
> not registered for hot-added CPU.
> 
> With this patch reset callback for CPU is register when CPU instance
> is created, and reset interface is added for virt-machine board. In
> reset interface of virt-machine, reset for direct kernel booting
> requirement is called.
> 
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>   hw/loongarch/boot.c         |  9 +--------
>   hw/loongarch/virt.c         | 14 ++++++++++++++
>   include/hw/loongarch/boot.h |  1 +
>   target/loongarch/cpu.c      | 10 ++++++++++
>   4 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
> index cb668703bd..cbb4e3737d 100644
> --- a/hw/loongarch/boot.c
> +++ b/hw/loongarch/boot.c
> @@ -216,12 +216,11 @@ 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;
>   
> -    cpu_reset(CPU(cpu));
>       if (env->load_elf) {
>   	if (cpu == LOONGARCH_CPU(first_cpu)) {
>               env->gpr[4] = env->boot_info->a0;
> @@ -320,12 +319,6 @@ static void loongarch_direct_kernel_boot(struct loongarch_boot_info *info)
>   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 9a635d1d3d..80680d178c 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -1434,6 +1434,19 @@ static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx)
>       }
>   }
>   
> +static void virt_reset(MachineState *machine, ResetType type)
> +{
> +    CPUState *cs;
> +
> +    /* Reset all devices including CPU devices */
> +    qemu_devices_reset(type);
> +
> +    /* Reset PC and register context for kernel direct booting method */
> +    CPU_FOREACH(cs) {
> +        reset_load_elf(LOONGARCH_CPU(cs));
> +    }
> +}
> +
>   static void virt_class_init(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1457,6 +1470,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
>       mc->auto_enable_numa_with_memdev = true;
>       mc->get_hotplug_handler = virt_get_hotplug_handler;
>       mc->default_nic = "virtio-net-pci";
> +    mc->reset = virt_reset;
>       hc->plug = virt_device_plug_cb;
>       hc->pre_plug = virt_device_pre_plug;
>       hc->unplug_request = virt_device_unplug_request;
> diff --git a/include/hw/loongarch/boot.h b/include/hw/loongarch/boot.h
> index b3b870df1f..c7020ec9bb 100644
> --- a/include/hw/loongarch/boot.h
> +++ b/include/hw/loongarch/boot.h
> @@ -115,5 +115,6 @@ struct memmap_entry {
>   };
>   
>   void loongarch_load_kernel(MachineState *ms, struct loongarch_boot_info *info);
> +void reset_load_elf(void *opaque);
>   
>   #endif /* HW_LOONGARCH_BOOT_H */
> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> index 7212fb5f8f..f7f8fcc024 100644
> --- a/target/loongarch/cpu.c
> +++ b/target/loongarch/cpu.c
> @@ -592,6 +592,13 @@ static void loongarch_cpu_disas_set_info(CPUState *s, disassemble_info *info)
>       info->print_insn = print_insn_loongarch;
>   }
>   
> +#ifndef CONFIG_USER_ONLY
> +static void loongarch_cpu_reset_cb(void *opaque)
> +{
> +    cpu_reset((CPUState *) opaque);
> +}
> +#endif
> +
>   static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>   {
>       CPUState *cs = CPU(dev);
> @@ -607,6 +614,9 @@ static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>       loongarch_cpu_register_gdb_regs_for_features(cs);
>   
>       cpu_reset(cs);
> +#ifndef CONFIG_USER_ONLY
> +    qemu_register_reset(loongarch_cpu_reset_cb, dev);
> +#endif
>       qemu_init_vcpu(cs);
>   
>       lacc->parent_realize(dev, errp);
> 
> base-commit: 58d49b5895f2e0b5cfe4b2901bf24f3320b74f29



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] hw/loongarch/virt: Add reset interface for virt-machine
  2024-10-31 16:16 ` Philippe Mathieu-Daudé
@ 2024-11-07 11:26   ` gaosong
  0 siblings, 0 replies; 7+ messages in thread
From: gaosong @ 2024-11-07 11:26 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Bibo Mao, Peter Maydell
  Cc: Jiaxun Yang, qemu-devel

在 2024/11/1 上午12:16, Philippe Mathieu-Daudé 写道:
> Cc'ing Peter who is working on the Reset API.
>
> On 31/10/24 03:54, Bibo Mao wrote:
>> With generic cpu reset interface, pc register is entry of FLASH for
>> UEFI BIOS. However with direct kernel booting requirement, there is
>> little different, pc register of primary cpu is entry address of ELF
>> file.
>>
>> At the same time with requirement of cpu hotplug, hot-added CPU should
>> register reset interface for this cpu object. Now reset callback is
>> not registered for hot-added CPU.
>>
>> With this patch reset callback for CPU is register when CPU instance
>> is created, and reset interface is added for virt-machine board. In
>> reset interface of virt-machine, reset for direct kernel booting
>> requirement is called.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>   hw/loongarch/boot.c         |  9 +--------
>>   hw/loongarch/virt.c         | 14 ++++++++++++++
>>   include/hw/loongarch/boot.h |  1 +
>>   target/loongarch/cpu.c      | 10 ++++++++++
>>   4 files changed, 26 insertions(+), 8 deletions(-)
>>
Acked-by: Song Gao <gaosong@loongson.cn>

Thanks.
Song Gao
>> diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
>> index cb668703bd..cbb4e3737d 100644
>> --- a/hw/loongarch/boot.c
>> +++ b/hw/loongarch/boot.c
>> @@ -216,12 +216,11 @@ 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;
>>   -    cpu_reset(CPU(cpu));
>>       if (env->load_elf) {
>>       if (cpu == LOONGARCH_CPU(first_cpu)) {
>>               env->gpr[4] = env->boot_info->a0;
>> @@ -320,12 +319,6 @@ static void loongarch_direct_kernel_boot(struct 
>> loongarch_boot_info *info)
>>   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 9a635d1d3d..80680d178c 100644
>> --- a/hw/loongarch/virt.c
>> +++ b/hw/loongarch/virt.c
>> @@ -1434,6 +1434,19 @@ static int64_t 
>> virt_get_default_cpu_node_id(const MachineState *ms, int idx)
>>       }
>>   }
>>   +static void virt_reset(MachineState *machine, ResetType type)
>> +{
>> +    CPUState *cs;
>> +
>> +    /* Reset all devices including CPU devices */
>> +    qemu_devices_reset(type);
>> +
>> +    /* Reset PC and register context for kernel direct booting 
>> method */
>> +    CPU_FOREACH(cs) {
>> +        reset_load_elf(LOONGARCH_CPU(cs));
>> +    }
>> +}
>> +
>>   static void virt_class_init(ObjectClass *oc, void *data)
>>   {
>>       MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -1457,6 +1470,7 @@ static void virt_class_init(ObjectClass *oc, 
>> void *data)
>>       mc->auto_enable_numa_with_memdev = true;
>>       mc->get_hotplug_handler = virt_get_hotplug_handler;
>>       mc->default_nic = "virtio-net-pci";
>> +    mc->reset = virt_reset;
>>       hc->plug = virt_device_plug_cb;
>>       hc->pre_plug = virt_device_pre_plug;
>>       hc->unplug_request = virt_device_unplug_request;
>> diff --git a/include/hw/loongarch/boot.h b/include/hw/loongarch/boot.h
>> index b3b870df1f..c7020ec9bb 100644
>> --- a/include/hw/loongarch/boot.h
>> +++ b/include/hw/loongarch/boot.h
>> @@ -115,5 +115,6 @@ struct memmap_entry {
>>   };
>>     void loongarch_load_kernel(MachineState *ms, struct 
>> loongarch_boot_info *info);
>> +void reset_load_elf(void *opaque);
>>     #endif /* HW_LOONGARCH_BOOT_H */
>> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
>> index 7212fb5f8f..f7f8fcc024 100644
>> --- a/target/loongarch/cpu.c
>> +++ b/target/loongarch/cpu.c
>> @@ -592,6 +592,13 @@ static void 
>> loongarch_cpu_disas_set_info(CPUState *s, disassemble_info *info)
>>       info->print_insn = print_insn_loongarch;
>>   }
>>   +#ifndef CONFIG_USER_ONLY
>> +static void loongarch_cpu_reset_cb(void *opaque)
>> +{
>> +    cpu_reset((CPUState *) opaque);
>> +}
>> +#endif
>> +
>>   static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>>   {
>>       CPUState *cs = CPU(dev);
>> @@ -607,6 +614,9 @@ static void loongarch_cpu_realizefn(DeviceState 
>> *dev, Error **errp)
>>       loongarch_cpu_register_gdb_regs_for_features(cs);
>>         cpu_reset(cs);
>> +#ifndef CONFIG_USER_ONLY
>> +    qemu_register_reset(loongarch_cpu_reset_cb, dev);
>> +#endif
>>       qemu_init_vcpu(cs);
>>         lacc->parent_realize(dev, errp);
>>
>> base-commit: 58d49b5895f2e0b5cfe4b2901bf24f3320b74f29



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] hw/loongarch/virt: Add reset interface for virt-machine
  2024-10-31  6:54 [PATCH] hw/loongarch/virt: Add reset interface for virt-machine Bibo Mao
  2024-10-31 16:16 ` Philippe Mathieu-Daudé
@ 2024-11-07 11:28 ` Peter Maydell
  2024-11-07 11:54   ` maobibo
  2025-08-29  7:45 ` lixianglai
  2 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2024-11-07 11:28 UTC (permalink / raw)
  To: Bibo Mao; +Cc: Song Gao, Jiaxun Yang, qemu-devel

On Thu, 31 Oct 2024 at 06:55, Bibo Mao <maobibo@loongson.cn> wrote:
>
> With generic cpu reset interface, pc register is entry of FLASH for
> UEFI BIOS. However with direct kernel booting requirement, there is
> little different, pc register of primary cpu is entry address of ELF
> file.
>
> At the same time with requirement of cpu hotplug, hot-added CPU should
> register reset interface for this cpu object. Now reset callback is
> not registered for hot-added CPU.
>
> With this patch reset callback for CPU is register when CPU instance
> is created, and reset interface is added for virt-machine board. In
> reset interface of virt-machine, reset for direct kernel booting
> requirement is called.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  hw/loongarch/boot.c         |  9 +--------
>  hw/loongarch/virt.c         | 14 ++++++++++++++
>  include/hw/loongarch/boot.h |  1 +
>  target/loongarch/cpu.c      | 10 ++++++++++
>  4 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
> index cb668703bd..cbb4e3737d 100644
> --- a/hw/loongarch/boot.c
> +++ b/hw/loongarch/boot.c
> @@ -216,12 +216,11 @@ 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;
>
> -    cpu_reset(CPU(cpu));
>      if (env->load_elf) {
>         if (cpu == LOONGARCH_CPU(first_cpu)) {
>              env->gpr[4] = env->boot_info->a0;
> @@ -320,12 +319,6 @@ static void loongarch_direct_kernel_boot(struct loongarch_boot_info *info)
>  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 9a635d1d3d..80680d178c 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -1434,6 +1434,19 @@ static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx)
>      }
>  }
>
> +static void virt_reset(MachineState *machine, ResetType type)
> +{
> +    CPUState *cs;
> +
> +    /* Reset all devices including CPU devices */
> +    qemu_devices_reset(type);
> +
> +    /* Reset PC and register context for kernel direct booting method */
> +    CPU_FOREACH(cs) {
> +        reset_load_elf(LOONGARCH_CPU(cs));
> +    }
> +}
> +
>  static void virt_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1457,6 +1470,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
>      mc->auto_enable_numa_with_memdev = true;
>      mc->get_hotplug_handler = virt_get_hotplug_handler;
>      mc->default_nic = "virtio-net-pci";
> +    mc->reset = virt_reset;
>      hc->plug = virt_device_plug_cb;
>      hc->pre_plug = virt_device_pre_plug;
>      hc->unplug_request = virt_device_unplug_request;
> diff --git a/include/hw/loongarch/boot.h b/include/hw/loongarch/boot.h
> index b3b870df1f..c7020ec9bb 100644
> --- a/include/hw/loongarch/boot.h
> +++ b/include/hw/loongarch/boot.h
> @@ -115,5 +115,6 @@ struct memmap_entry {
>  };
>
>  void loongarch_load_kernel(MachineState *ms, struct loongarch_boot_info *info);
> +void reset_load_elf(void *opaque);
>
>  #endif /* HW_LOONGARCH_BOOT_H */
> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> index 7212fb5f8f..f7f8fcc024 100644
> --- a/target/loongarch/cpu.c
> +++ b/target/loongarch/cpu.c
> @@ -592,6 +592,13 @@ static void loongarch_cpu_disas_set_info(CPUState *s, disassemble_info *info)
>      info->print_insn = print_insn_loongarch;
>  }
>
> +#ifndef CONFIG_USER_ONLY
> +static void loongarch_cpu_reset_cb(void *opaque)
> +{
> +    cpu_reset((CPUState *) opaque);
> +}
> +#endif
> +
>  static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>  {
>      CPUState *cs = CPU(dev);
> @@ -607,6 +614,9 @@ static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>      loongarch_cpu_register_gdb_regs_for_features(cs);
>
>      cpu_reset(cs);
> +#ifndef CONFIG_USER_ONLY
> +    qemu_register_reset(loongarch_cpu_reset_cb, dev);
> +#endif

Please don't add new uses of qemu_register_reset().
I know that CPU reset is currently rather awkward (because
we don't automatically-reset CPU objects the way we do most
device objects), but generally what should happen is that
the machine model should arrange to reset the CPU objects
it creates. (Which is what it looks like the code you're
removing in this patch was doing already.)

thanks
-- PMM


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] hw/loongarch/virt: Add reset interface for virt-machine
  2024-11-07 11:28 ` Peter Maydell
@ 2024-11-07 11:54   ` maobibo
  2025-08-29  7:53     ` lixianglai
  0 siblings, 1 reply; 7+ messages in thread
From: maobibo @ 2024-11-07 11:54 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Song Gao, Jiaxun Yang, qemu-devel



On 2024/11/7 下午7:28, Peter Maydell wrote:
> On Thu, 31 Oct 2024 at 06:55, Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> With generic cpu reset interface, pc register is entry of FLASH for
>> UEFI BIOS. However with direct kernel booting requirement, there is
>> little different, pc register of primary cpu is entry address of ELF
>> file.
>>
>> At the same time with requirement of cpu hotplug, hot-added CPU should
>> register reset interface for this cpu object. Now reset callback is
>> not registered for hot-added CPU.
>>
>> With this patch reset callback for CPU is register when CPU instance
>> is created, and reset interface is added for virt-machine board. In
>> reset interface of virt-machine, reset for direct kernel booting
>> requirement is called.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>   hw/loongarch/boot.c         |  9 +--------
>>   hw/loongarch/virt.c         | 14 ++++++++++++++
>>   include/hw/loongarch/boot.h |  1 +
>>   target/loongarch/cpu.c      | 10 ++++++++++
>>   4 files changed, 26 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
>> index cb668703bd..cbb4e3737d 100644
>> --- a/hw/loongarch/boot.c
>> +++ b/hw/loongarch/boot.c
>> @@ -216,12 +216,11 @@ 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;
>>
>> -    cpu_reset(CPU(cpu));
>>       if (env->load_elf) {
>>          if (cpu == LOONGARCH_CPU(first_cpu)) {
>>               env->gpr[4] = env->boot_info->a0;
>> @@ -320,12 +319,6 @@ static void loongarch_direct_kernel_boot(struct loongarch_boot_info *info)
>>   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 9a635d1d3d..80680d178c 100644
>> --- a/hw/loongarch/virt.c
>> +++ b/hw/loongarch/virt.c
>> @@ -1434,6 +1434,19 @@ static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx)
>>       }
>>   }
>>
>> +static void virt_reset(MachineState *machine, ResetType type)
>> +{
>> +    CPUState *cs;
>> +
>> +    /* Reset all devices including CPU devices */
>> +    qemu_devices_reset(type);
>> +
>> +    /* Reset PC and register context for kernel direct booting method */
>> +    CPU_FOREACH(cs) {
>> +        reset_load_elf(LOONGARCH_CPU(cs));
>> +    }
>> +}
>> +
>>   static void virt_class_init(ObjectClass *oc, void *data)
>>   {
>>       MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -1457,6 +1470,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
>>       mc->auto_enable_numa_with_memdev = true;
>>       mc->get_hotplug_handler = virt_get_hotplug_handler;
>>       mc->default_nic = "virtio-net-pci";
>> +    mc->reset = virt_reset;
>>       hc->plug = virt_device_plug_cb;
>>       hc->pre_plug = virt_device_pre_plug;
>>       hc->unplug_request = virt_device_unplug_request;
>> diff --git a/include/hw/loongarch/boot.h b/include/hw/loongarch/boot.h
>> index b3b870df1f..c7020ec9bb 100644
>> --- a/include/hw/loongarch/boot.h
>> +++ b/include/hw/loongarch/boot.h
>> @@ -115,5 +115,6 @@ struct memmap_entry {
>>   };
>>
>>   void loongarch_load_kernel(MachineState *ms, struct loongarch_boot_info *info);
>> +void reset_load_elf(void *opaque);
>>
>>   #endif /* HW_LOONGARCH_BOOT_H */
>> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
>> index 7212fb5f8f..f7f8fcc024 100644
>> --- a/target/loongarch/cpu.c
>> +++ b/target/loongarch/cpu.c
>> @@ -592,6 +592,13 @@ static void loongarch_cpu_disas_set_info(CPUState *s, disassemble_info *info)
>>       info->print_insn = print_insn_loongarch;
>>   }
>>
>> +#ifndef CONFIG_USER_ONLY
>> +static void loongarch_cpu_reset_cb(void *opaque)
>> +{
>> +    cpu_reset((CPUState *) opaque);
>> +}
>> +#endif
>> +
>>   static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>>   {
>>       CPUState *cs = CPU(dev);
>> @@ -607,6 +614,9 @@ static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>>       loongarch_cpu_register_gdb_regs_for_features(cs);
>>
>>       cpu_reset(cs);
>> +#ifndef CONFIG_USER_ONLY
>> +    qemu_register_reset(loongarch_cpu_reset_cb, dev);
>> +#endif
> 
> Please don't add new uses of qemu_register_reset().
> I know that CPU reset is currently rather awkward (because
> we don't automatically-reset CPU objects the way we do most
> device objects), but generally what should happen is that
> the machine model should arrange to reset the CPU objects
> it creates. (Which is what it looks like the code you're
> removing in this patch was doing already.)
Currently the machine model does not register reset callback for 
hot-added CPUs, so we put reset callback registration on CPU object 
created stage, that works for hot-added CPUs.

Regards
Bibo Mao
> 
> thanks
> -- PMM
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] hw/loongarch/virt: Add reset interface for virt-machine
  2024-10-31  6:54 [PATCH] hw/loongarch/virt: Add reset interface for virt-machine Bibo Mao
  2024-10-31 16:16 ` Philippe Mathieu-Daudé
  2024-11-07 11:28 ` Peter Maydell
@ 2025-08-29  7:45 ` lixianglai
  2 siblings, 0 replies; 7+ messages in thread
From: lixianglai @ 2025-08-29  7:45 UTC (permalink / raw)
  To: Bibo Mao, Song Gao; +Cc: Jiaxun Yang, qemu-devel, Peter Maydell

Hi Bibo Mao:
> With generic cpu reset interface, pc register is entry of FLASH for
> UEFI BIOS. However with direct kernel booting requirement, there is
> little different, pc register of primary cpu is entry address of ELF
> file.
>
> At the same time with requirement of cpu hotplug, hot-added CPU should
> register reset interface for this cpu object. Now reset callback is
> not registered for hot-added CPU.
>
> With this patch reset callback for CPU is register when CPU instance
> is created, and reset interface is added for virt-machine board. In
> reset interface of virt-machine, reset for direct kernel booting
> requirement is called.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>   hw/loongarch/boot.c         |  9 +--------
>   hw/loongarch/virt.c         | 14 ++++++++++++++
>   include/hw/loongarch/boot.h |  1 +
>   target/loongarch/cpu.c      | 10 ++++++++++
>   4 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
> index cb668703bd..cbb4e3737d 100644
> --- a/hw/loongarch/boot.c
> +++ b/hw/loongarch/boot.c
> @@ -216,12 +216,11 @@ 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;
>   
> -    cpu_reset(CPU(cpu));
>       if (env->load_elf) {
>   	if (cpu == LOONGARCH_CPU(first_cpu)) {
>               env->gpr[4] = env->boot_info->a0;
> @@ -320,12 +319,6 @@ static void loongarch_direct_kernel_boot(struct loongarch_boot_info *info)
>   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 9a635d1d3d..80680d178c 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -1434,6 +1434,19 @@ static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx)
>       }
>   }
>   
> +static void virt_reset(MachineState *machine, ResetType type)
> +{
> +    CPUState *cs;
> +
> +    /* Reset all devices including CPU devices */
> +    qemu_devices_reset(type);
> +
> +    /* Reset PC and register context for kernel direct booting method */
> +    CPU_FOREACH(cs) {
> +        reset_load_elf(LOONGARCH_CPU(cs));
> +    }
> +}
> +
>   static void virt_class_init(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1457,6 +1470,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
>       mc->auto_enable_numa_with_memdev = true;
>       mc->get_hotplug_handler = virt_get_hotplug_handler;
>       mc->default_nic = "virtio-net-pci";
> +    mc->reset = virt_reset;
>       hc->plug = virt_device_plug_cb;
>       hc->pre_plug = virt_device_pre_plug;
>       hc->unplug_request = virt_device_unplug_request;
> diff --git a/include/hw/loongarch/boot.h b/include/hw/loongarch/boot.h
> index b3b870df1f..c7020ec9bb 100644
> --- a/include/hw/loongarch/boot.h
> +++ b/include/hw/loongarch/boot.h
> @@ -115,5 +115,6 @@ struct memmap_entry {
>   };
>   
>   void loongarch_load_kernel(MachineState *ms, struct loongarch_boot_info *info);
> +void reset_load_elf(void *opaque);
>   
>   #endif /* HW_LOONGARCH_BOOT_H */
> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> index 7212fb5f8f..f7f8fcc024 100644
> --- a/target/loongarch/cpu.c
> +++ b/target/loongarch/cpu.c
> @@ -592,6 +592,13 @@ static void loongarch_cpu_disas_set_info(CPUState *s, disassemble_info *info)
>       info->print_insn = print_insn_loongarch;
>   }
>   
> +#ifndef CONFIG_USER_ONLY
> +static void loongarch_cpu_reset_cb(void *opaque)
> +{
> +    cpu_reset((CPUState *) opaque);
> +}
> +#endif
> +
>   static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>   {
>       CPUState *cs = CPU(dev);
> @@ -607,6 +614,9 @@ static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>       loongarch_cpu_register_gdb_regs_for_features(cs);
>   
>       cpu_reset(cs);
> +#ifndef CONFIG_USER_ONLY
> +    qemu_register_reset(loongarch_cpu_reset_cb, dev);

I think we should implement the corresponding unregistration process in 
loongarch_cpu_unrealizefn

Thanks!
Xianglai

> +#endif
>       qemu_init_vcpu(cs);
>   
>       lacc->parent_realize(dev, errp);
>
> base-commit: 58d49b5895f2e0b5cfe4b2901bf24f3320b74f29



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Re: [PATCH] hw/loongarch/virt: Add reset interface for virt-machine
  2024-11-07 11:54   ` maobibo
@ 2025-08-29  7:53     ` lixianglai
  0 siblings, 0 replies; 7+ messages in thread
From: lixianglai @ 2025-08-29  7:53 UTC (permalink / raw)
  To: maobibo, Peter Maydell; +Cc: Song Gao, Jiaxun Yang, qemu-devel

Hi Peter:
>> On Thu, 31 Oct 2024 at 06:55, Bibo Mao <maobibo@loongson.cn> wrote:
>>>
>>> With generic cpu reset interface, pc register is entry of FLASH for
>>> UEFI BIOS. However with direct kernel booting requirement, there is
>>> little different, pc register of primary cpu is entry address of ELF
>>> file.
>>>
>>> At the same time with requirement of cpu hotplug, hot-added CPU should
>>> register reset interface for this cpu object. Now reset callback is
>>> not registered for hot-added CPU.
>>>
>>> With this patch reset callback for CPU is register when CPU instance
>>> is created, and reset interface is added for virt-machine board. In
>>> reset interface of virt-machine, reset for direct kernel booting
>>> requirement is called.
>>>
>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>> ---
>>>   hw/loongarch/boot.c         |  9 +--------
>>>   hw/loongarch/virt.c         | 14 ++++++++++++++
>>>   include/hw/loongarch/boot.h |  1 +
>>>   target/loongarch/cpu.c      | 10 ++++++++++
>>>   4 files changed, 26 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
>>> index cb668703bd..cbb4e3737d 100644
>>> --- a/hw/loongarch/boot.c
>>> +++ b/hw/loongarch/boot.c
>>> @@ -216,12 +216,11 @@ 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;
>>>
>>> -    cpu_reset(CPU(cpu));
>>>       if (env->load_elf) {
>>>          if (cpu == LOONGARCH_CPU(first_cpu)) {
>>>               env->gpr[4] = env->boot_info->a0;
>>> @@ -320,12 +319,6 @@ static void loongarch_direct_kernel_boot(struct 
>>> loongarch_boot_info *info)
>>>   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 9a635d1d3d..80680d178c 100644
>>> --- a/hw/loongarch/virt.c
>>> +++ b/hw/loongarch/virt.c
>>> @@ -1434,6 +1434,19 @@ static int64_t 
>>> virt_get_default_cpu_node_id(const MachineState *ms, int idx)
>>>       }
>>>   }
>>>
>>> +static void virt_reset(MachineState *machine, ResetType type)
>>> +{
>>> +    CPUState *cs;
>>> +
>>> +    /* Reset all devices including CPU devices */
>>> +    qemu_devices_reset(type);
>>> +
>>> +    /* Reset PC and register context for kernel direct booting 
>>> method */
>>> +    CPU_FOREACH(cs) {
>>> +        reset_load_elf(LOONGARCH_CPU(cs));
>>> +    }
>>> +}
>>> +
>>>   static void virt_class_init(ObjectClass *oc, void *data)
>>>   {
>>>       MachineClass *mc = MACHINE_CLASS(oc);
>>> @@ -1457,6 +1470,7 @@ static void virt_class_init(ObjectClass *oc, 
>>> void *data)
>>>       mc->auto_enable_numa_with_memdev = true;
>>>       mc->get_hotplug_handler = virt_get_hotplug_handler;
>>>       mc->default_nic = "virtio-net-pci";
>>> +    mc->reset = virt_reset;
>>>       hc->plug = virt_device_plug_cb;
>>>       hc->pre_plug = virt_device_pre_plug;
>>>       hc->unplug_request = virt_device_unplug_request;
>>> diff --git a/include/hw/loongarch/boot.h b/include/hw/loongarch/boot.h
>>> index b3b870df1f..c7020ec9bb 100644
>>> --- a/include/hw/loongarch/boot.h
>>> +++ b/include/hw/loongarch/boot.h
>>> @@ -115,5 +115,6 @@ struct memmap_entry {
>>>   };
>>>
>>>   void loongarch_load_kernel(MachineState *ms, struct 
>>> loongarch_boot_info *info);
>>> +void reset_load_elf(void *opaque);
>>>
>>>   #endif /* HW_LOONGARCH_BOOT_H */
>>> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
>>> index 7212fb5f8f..f7f8fcc024 100644
>>> --- a/target/loongarch/cpu.c
>>> +++ b/target/loongarch/cpu.c
>>> @@ -592,6 +592,13 @@ static void 
>>> loongarch_cpu_disas_set_info(CPUState *s, disassemble_info *info)
>>>       info->print_insn = print_insn_loongarch;
>>>   }
>>>
>>> +#ifndef CONFIG_USER_ONLY
>>> +static void loongarch_cpu_reset_cb(void *opaque)
>>> +{
>>> +    cpu_reset((CPUState *) opaque);
>>> +}
>>> +#endif
>>> +
>>>   static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>>>   {
>>>       CPUState *cs = CPU(dev);
>>> @@ -607,6 +614,9 @@ static void loongarch_cpu_realizefn(DeviceState 
>>> *dev, Error **errp)
>>>       loongarch_cpu_register_gdb_regs_for_features(cs);
>>>
>>>       cpu_reset(cs);
>>> +#ifndef CONFIG_USER_ONLY
>>> +    qemu_register_reset(loongarch_cpu_reset_cb, dev);
>>> +#endif
>>
>> Please don't add new uses of qemu_register_reset().
>> I know that CPU reset is currently rather awkward (because
>> we don't automatically-reset CPU objects the way we do most
>> device objects), but generally what should happen is that
>> the machine model should arrange to reset the CPU objects
>> it creates. (Which is what it looks like the code you're
>> removing in this patch was doing already.)
> Currently the machine model does not register reset callback for 
> hot-added CPUs, so we put reset callback registration on CPU object 
> created stage, that works for hot-added CPUs.
>

I see this comment has been around for a long time. Can this patch be 
merged? Recently,
I also encountered the same problem, so I submitted a patch.
However, I found that this patch was a bit better than the one I submitted.
So, I wonder what the final conclusion of this patch is.

Thanks!
Xianglai.

> Regards
> Bibo Mao
>>
>> thanks
>> -- PMM
>>
>
>
>



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-08-30 15:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-31  6:54 [PATCH] hw/loongarch/virt: Add reset interface for virt-machine Bibo Mao
2024-10-31 16:16 ` Philippe Mathieu-Daudé
2024-11-07 11:26   ` gaosong
2024-11-07 11:28 ` Peter Maydell
2024-11-07 11:54   ` maobibo
2025-08-29  7:53     ` lixianglai
2025-08-29  7:45 ` lixianglai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).