Re: [PATCH v5 3/7] ppc/pnv: Introduce a PnvCore reset handler

[Philippe Mathieu-Daudé]

Hi Cédric,

On 10/22/19 6:38 PM, Cédric Le Goater wrote:

in which individual CPUs are reset. It will ease the introduction of
future change reseting the interrupt presenter from the CPU reset
handler.

Signed-off-by: Cédric Le Goater <c...@kaod.org>
Reviewed-by: Greg Kurz <gr...@kaod.org>
---
  hw/ppc/pnv_core.c | 19 +++++++++++++++----
  1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index b1a7489e7abf..9f981a4940e6 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -40,9 +40,8 @@ static const char *pnv_core_cpu_typename(PnvCore *pc)
      return cpu_type;
  }

-static void pnv_cpu_reset(void *opaque)

+static void pnv_core_cpu_reset(PowerPCCPU *cpu)
  {
-    PowerPCCPU *cpu = opaque;
      CPUState *cs = CPU(cpu);
      CPUPPCState *env = &cpu->env;

@@ -192,8 +191,17 @@ static void pnv_realize_vcpu(PowerPCCPU *cpu, PnvChip *chip, Error **errp) /* Set time-base frequency to 512 MHz */

      cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ);
+}
+
+static void pnv_core_reset(void *dev)

Here the opaque pointer is a 'PnvCore *pc'.
If you don't want to call it 'opaque', maybe 'pc' is better.

+{
+    CPUCore *cc = CPU_CORE(dev);
+    PnvCore *pc = PNV_CORE(dev);

This type conversion is not necessary.

What about:

   static void pnv_core_reset(void *opaque)
   {
       PnvCore *pc = opaque;
       CPUCore *cc = CPU_CORE(pc);

+    int i;

- qemu_register_reset(pnv_cpu_reset, cpu);

+    for (i = 0; i < cc->nr_threads; i++) {
+        pnv_core_cpu_reset(pc->threads[i]);
+    }
  }

static void pnv_core_realize(DeviceState *dev, Error **errp)

@@ -244,6 +252,8 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
      snprintf(name, sizeof(name), "xscom-core.%d", cc->core_id);
      pnv_xscom_region_init(&pc->xscom_regs, OBJECT(dev), pcc->xscom_ops,
                            pc, name, PNV_XSCOM_EX_SIZE);
+
+    qemu_register_reset(pnv_core_reset, pc);
      return;

err:

@@ -259,7 +269,6 @@ static void pnv_unrealize_vcpu(PowerPCCPU *cpu)
  {
      PnvCPUState *pnv_cpu = pnv_cpu_state(cpu);

- qemu_unregister_reset(pnv_cpu_reset, cpu);

      object_unparent(OBJECT(pnv_cpu_state(cpu)->intc));
      cpu_remove_sync(CPU(cpu));
      cpu->machine_data = NULL;
@@ -273,6 +282,8 @@ static void pnv_core_unrealize(DeviceState *dev, Error 
**errp)
      CPUCore *cc = CPU_CORE(dev);
      int i;

+ qemu_unregister_reset(pnv_core_reset, pc);

+
      for (i = 0; i < cc->nr_threads; i++) {
          pnv_unrealize_vcpu(pc->threads[i]);
      }