Hi Bibo,

On 1/9/25 10:19, 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
a 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 <[email protected]>
---
v1 ... v2:
   1. Add qemu_unregister_reset() in function loongarch_cpu_unrealizefn(),
      remove reset callback if vCPU is unrealized.
---

Signed-off-by: Bibo Mao <[email protected]>
---
  hw/loongarch/boot.c         |  9 +--------
  hw/loongarch/virt.c         | 14 ++++++++++++++
  include/hw/loongarch/boot.h |  1 +
  target/loongarch/cpu.c      | 11 +++++++++++
  4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
index 14d6c52d4e..4919758a20 100644
--- a/hw/loongarch/boot.c
+++ b/hw/loongarch/boot.c
@@ -324,12 +324,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;
@@ -429,12 +428,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)));

I agree CPU reset shouldn't be part of loading code to memory.

-    }
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..4fc8506c10 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -1199,6 +1199,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, const void *data)
  {
      MachineClass *mc = MACHINE_CLASS(oc);
@@ -1223,6 +1236,7 @@ static void virt_class_init(ObjectClass *oc, const void 
*data)
      mc->has_hotpluggable_cpus = 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 9819f7fbe3..386b4406ad 100644
--- a/include/hw/loongarch/boot.h
+++ b/include/hw/loongarch/boot.h
@@ -114,5 +114,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 3a7621c0ea..61c8acb3c2 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -652,6 +652,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);
@@ -668,6 +675,9 @@ static void loongarch_cpu_realizefn(DeviceState *dev, Error 
**errp)
qemu_init_vcpu(cs);
      cpu_reset(cs);

Devices shouldn't call their DeviceReset handler manually, as it is
always called after DeviceRealize.

+#ifndef CONFIG_USER_ONLY
+    qemu_register_reset(loongarch_cpu_reset_cb, dev);

qemu_register_reset() is a legacy API, replaced by
qemu_register_resettable().

That said, I don't think the CPU object has to register its own
reset handlers. Instead that should the be responsibility of the
object creating the CPU objects.

+#endif
lacc->parent_realize(dev, errp);
  }
@@ -678,6 +688,7 @@ static void loongarch_cpu_unrealizefn(DeviceState *dev)
#ifndef CONFIG_USER_ONLY
      cpu_remove_sync(CPU(dev));
+    qemu_unregister_reset(loongarch_cpu_reset_cb, dev);

Ditto, legacy -> qemu_unregister_resettable().

  #endif
lacc->parent_unrealize(dev);

base-commit: 91589bcd9fee0e66b241d04e5f37cd4f218187a2

Reply via email to