diff mbox series

[v3,2/4] spapr_cpu_core: Implement DeviceClass::reset

Message ID 20191022072246.9200-3-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series ppc: reset the interrupt presenter from the CPU reset handler | expand

Commit Message

Cédric Le Goater Oct. 22, 2019, 7:22 a.m. UTC 
From: Greg Kurz <groug@kaod.org>

Since vCPUs aren't plugged into a bus, we manually register a reset
handler for each vCPU. We also call this handler at realize time
to ensure hot plugged vCPUs are reset before being exposed to the
guest. This results in vCPUs being reset twice at machine reset.
It doesn't break anything but it is slightly suboptimal and above
all confusing.

The hotplug path in device_set_realized() already knows how to reset
a hotplugged device if the device reset handler is present. Implement
one for sPAPR CPU cores that resets all vCPUs under a core.

While here rename spapr_cpu_reset() to spapr_reset_vcpu() for
consistency with spapr_realize_vcpu() and spapr_unrealize_vcpu().

Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr_cpu_core.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

Comments

Philippe Mathieu-Daudé Oct. 22, 2019, 8:58 a.m. UTC | #1 
On 10/22/19 9:22 AM, Cédric Le Goater wrote:
> From: Greg Kurz <groug@kaod.org>
> 
> Since vCPUs aren't plugged into a bus, we manually register a reset
> handler for each vCPU. We also call this handler at realize time
> to ensure hot plugged vCPUs are reset before being exposed to the
> guest. This results in vCPUs being reset twice at machine reset.
> It doesn't break anything but it is slightly suboptimal and above
> all confusing.
> 
> The hotplug path in device_set_realized() already knows how to reset
> a hotplugged device if the device reset handler is present. Implement
> one for sPAPR CPU cores that resets all vCPUs under a core.
> 
> While here rename spapr_cpu_reset() to spapr_reset_vcpu() for
> consistency with spapr_realize_vcpu() and spapr_unrealize_vcpu().
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/ppc/spapr_cpu_core.c | 31 ++++++++++++++++++++++---------
>   1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 992f00da6540..5947e39b36ad 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -25,9 +25,8 @@
>   #include "sysemu/hw_accel.h"
>   #include "qemu/error-report.h"
>   
> -static void spapr_cpu_reset(void *opaque)
> +static void spapr_reset_vcpu(PowerPCCPU *cpu)
>   {
> -    PowerPCCPU *cpu = opaque;
>       CPUState *cs = CPU(cpu);
>       CPUPPCState *env = &cpu->env;
>       PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> @@ -193,7 +192,6 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc)
>       if (!sc->pre_3_0_migration) {
>           vmstate_unregister(NULL, &vmstate_spapr_cpu_state, cpu->machine_data);
>       }
> -    qemu_unregister_reset(spapr_cpu_reset, cpu);
>       if (spapr_cpu_state(cpu)->icp) {
>           object_unparent(OBJECT(spapr_cpu_state(cpu)->icp));
>       }
> @@ -204,12 +202,30 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc)
>       object_unparent(OBJECT(cpu));
>   }
>   
> +static void spapr_cpu_core_reset(DeviceState *dev)
> +{
> +    CPUCore *cc = CPU_CORE(dev);
> +    SpaprCpuCore *sc = SPAPR_CPU_CORE(dev);
> +    int i;
> +
> +    for (i = 0; i < cc->nr_threads; i++) {
> +        spapr_reset_vcpu(sc->threads[i]);
> +    }
> +}
> +
> +static void spapr_cpu_core_reset_handler(void *opaque)
> +{
> +    spapr_cpu_core_reset(opaque);
> +}
> +
>   static void spapr_cpu_core_unrealize(DeviceState *dev, Error **errp)
>   {
>       SpaprCpuCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
>       CPUCore *cc = CPU_CORE(dev);
>       int i;
>   
> +    qemu_unregister_reset(spapr_cpu_core_reset_handler, sc);
> +
>       for (i = 0; i < cc->nr_threads; i++) {
>           spapr_unrealize_vcpu(sc->threads[i], sc);
>       }
> @@ -238,12 +254,6 @@ static void spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr,
>           goto error_intc_create;
>       }
>   
> -    /*
> -     * FIXME: Hot-plugged CPUs are not reseted. Do it at realize.
> -     */
> -    qemu_register_reset(spapr_cpu_reset, cpu);
> -    spapr_cpu_reset(cpu);
> -
>       if (!sc->pre_3_0_migration) {
>           vmstate_register(NULL, cs->cpu_index, &vmstate_spapr_cpu_state,
>                            cpu->machine_data);
> @@ -338,6 +348,8 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
>               goto err_unrealize;
>           }
>       }
> +
> +    qemu_register_reset(spapr_cpu_core_reset_handler, sc);
>       return;
>   
>   err_unrealize:
> @@ -366,6 +378,7 @@ static void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
>   
>       dc->realize = spapr_cpu_core_realize;
>       dc->unrealize = spapr_cpu_core_unrealize;
> +    dc->reset = spapr_cpu_core_reset;
>       dc->props = spapr_cpu_core_properties;
>       scc->cpu_type = data;
>   }
> 

Good cleanup!

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff mbox series

Patch

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 992f00da6540..5947e39b36ad 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -25,9 +25,8 @@ 
 #include "sysemu/hw_accel.h"
 #include "qemu/error-report.h"
 
-static void spapr_cpu_reset(void *opaque)
+static void spapr_reset_vcpu(PowerPCCPU *cpu)
 {
-    PowerPCCPU *cpu = opaque;
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
@@ -193,7 +192,6 @@  static void spapr_unrealize_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc)
     if (!sc->pre_3_0_migration) {
         vmstate_unregister(NULL, &vmstate_spapr_cpu_state, cpu->machine_data);
     }
-    qemu_unregister_reset(spapr_cpu_reset, cpu);
     if (spapr_cpu_state(cpu)->icp) {
         object_unparent(OBJECT(spapr_cpu_state(cpu)->icp));
     }
@@ -204,12 +202,30 @@  static void spapr_unrealize_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc)
     object_unparent(OBJECT(cpu));
 }
 
+static void spapr_cpu_core_reset(DeviceState *dev)
+{
+    CPUCore *cc = CPU_CORE(dev);
+    SpaprCpuCore *sc = SPAPR_CPU_CORE(dev);
+    int i;
+
+    for (i = 0; i < cc->nr_threads; i++) {
+        spapr_reset_vcpu(sc->threads[i]);
+    }
+}
+
+static void spapr_cpu_core_reset_handler(void *opaque)
+{
+    spapr_cpu_core_reset(opaque);
+}
+
 static void spapr_cpu_core_unrealize(DeviceState *dev, Error **errp)
 {
     SpaprCpuCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
     CPUCore *cc = CPU_CORE(dev);
     int i;
 
+    qemu_unregister_reset(spapr_cpu_core_reset_handler, sc);
+
     for (i = 0; i < cc->nr_threads; i++) {
         spapr_unrealize_vcpu(sc->threads[i], sc);
     }
@@ -238,12 +254,6 @@  static void spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr,
         goto error_intc_create;
     }
 
-    /*
-     * FIXME: Hot-plugged CPUs are not reseted. Do it at realize.
-     */
-    qemu_register_reset(spapr_cpu_reset, cpu);
-    spapr_cpu_reset(cpu);
-
     if (!sc->pre_3_0_migration) {
         vmstate_register(NULL, cs->cpu_index, &vmstate_spapr_cpu_state,
                          cpu->machine_data);
@@ -338,6 +348,8 @@  static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
             goto err_unrealize;
         }
     }
+
+    qemu_register_reset(spapr_cpu_core_reset_handler, sc);
     return;
 
 err_unrealize:
@@ -366,6 +378,7 @@  static void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
 
     dc->realize = spapr_cpu_core_realize;
     dc->unrealize = spapr_cpu_core_unrealize;
+    dc->reset = spapr_cpu_core_reset;
     dc->props = spapr_cpu_core_properties;
     scc->cpu_type = data;
 }