Taking into account that qemu_set_irq() returns immediatly if its first
argument is NULL, icp_kvm_reset() largely duplicates icp_reset().

This patch introduces a reset() handler, so that the common logic can
be implemented in icp_reset() only.

While there we can also drop icp_kvm_realize() and icp_kvm_unrealize(). This
causes icp-kvm to be realized in icp_realize(), which sets icp->xics, but
it has no impact.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/intc/xics.c        |    5 +++++
 hw/intc/xics_kvm.c    |   27 ++-------------------------
 include/hw/ppc/xics.h |    1 +
 3 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index ea3516794af7..ec73f02144c9 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -325,6 +325,7 @@ static const VMStateDescription vmstate_icp_server = {
 static void icp_reset(void *dev)
 {
     ICPState *icp = ICP(dev);
+    ICPStateClass *icpc = ICP_GET_CLASS(icp);
 
     icp->xirr = 0;
     icp->pending_priority = 0xff;
@@ -332,6 +333,10 @@ static void icp_reset(void *dev)
 
     /* Make all outputs are deasserted */
     qemu_set_irq(icp->output, 0);
+
+    if (icpc->reset) {
+        icpc->reset(icp);
+    }
 }
 
 static void icp_realize(DeviceState *dev, Error **errp)
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index 14b8f6f6e478..45bf110b51e6 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -110,19 +110,8 @@ static int icp_set_kvm_state(ICPState *icp, int version_id)
     return 0;
 }
 
-static void icp_kvm_reset(void *dev)
+static void icp_kvm_reset(ICPState *icp)
 {
-    ICPState *icp = ICP(dev);
-
-    icp->xirr = 0;
-    icp->pending_priority = 0xff;
-    icp->mfrr = 0xff;
-
-    /* Make all outputs as deasserted only if the CPU thread is in use */
-    if (icp->output) {
-        qemu_set_irq(icp->output, 0);
-    }
-
     icp_set_kvm_state(icp, 1);
 }
 
@@ -159,26 +148,14 @@ static void icp_kvm_cpu_setup(ICPState *icp, PowerPCCPU *cpu)
     QLIST_INSERT_HEAD(&kvm_enabled_icps, enabled_icp, node);
 }
 
-static void icp_kvm_realize(DeviceState *dev, Error **errp)
-{
-    qemu_register_reset(icp_kvm_reset, dev);
-}
-
-static void icp_kvm_unrealize(DeviceState *dev, Error **errp)
-{
-    qemu_unregister_reset(icp_kvm_reset, dev);
-}
-
 static void icp_kvm_class_init(ObjectClass *klass, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
     ICPStateClass *icpc = ICP_CLASS(klass);
 
-    dc->realize = icp_kvm_realize;
-    dc->unrealize = icp_kvm_unrealize;
     icpc->pre_save = icp_get_kvm_state;
     icpc->post_load = icp_set_kvm_state;
     icpc->cpu_setup = icp_kvm_cpu_setup;
+    icpc->reset = icp_kvm_reset;
 }
 
 static const TypeInfo icp_kvm_info = {
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index a3073f90533a..40a506eacfb4 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -69,6 +69,7 @@ struct ICPStateClass {
     void (*pre_save)(ICPState *s);
     int (*post_load)(ICPState *s, int version_id);
     void (*cpu_setup)(ICPState *icp, PowerPCCPU *cpu);
+    void (*reset)(ICPState *icp);
 };
 
 struct ICPState {

On 06/07/2017 07:17 PM, Greg Kurz wrote:
> Taking into account that qemu_set_irq() returns immediatly if its first
> argument is NULL, icp_kvm_reset() largely duplicates icp_reset().
> 
> This patch introduces a reset() handler, so that the common logic can
> be implemented in icp_reset() only.
> 
> While there we can also drop icp_kvm_realize() and icp_kvm_unrealize(). This
> causes icp-kvm to be realized in icp_realize(), which sets icp->xics, but
> it has no impact.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

> ---
>  hw/intc/xics.c        |    5 +++++
>  hw/intc/xics_kvm.c    |   27 ++-------------------------
>  include/hw/ppc/xics.h |    1 +
>  3 files changed, 8 insertions(+), 25 deletions(-)

another good cleanup !

Thanks,

C. 

> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index ea3516794af7..ec73f02144c9 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -325,6 +325,7 @@ static const VMStateDescription vmstate_icp_server = {
>  static void icp_reset(void *dev)
>  {
>      ICPState *icp = ICP(dev);
> +    ICPStateClass *icpc = ICP_GET_CLASS(icp);
>  
>      icp->xirr = 0;
>      icp->pending_priority = 0xff;
> @@ -332,6 +333,10 @@ static void icp_reset(void *dev)
>  
>      /* Make all outputs are deasserted */
>      qemu_set_irq(icp->output, 0);
> +
> +    if (icpc->reset) {
> +        icpc->reset(icp);
> +    }
>  }
>  
>  static void icp_realize(DeviceState *dev, Error **errp)
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index 14b8f6f6e478..45bf110b51e6 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -110,19 +110,8 @@ static int icp_set_kvm_state(ICPState *icp, int version_id)
>      return 0;
>  }
>  
> -static void icp_kvm_reset(void *dev)
> +static void icp_kvm_reset(ICPState *icp)
>  {
> -    ICPState *icp = ICP(dev);
> -
> -    icp->xirr = 0;
> -    icp->pending_priority = 0xff;
> -    icp->mfrr = 0xff;
> -
> -    /* Make all outputs as deasserted only if the CPU thread is in use */
> -    if (icp->output) {
> -        qemu_set_irq(icp->output, 0);
> -    }
> -
>      icp_set_kvm_state(icp, 1);
>  }
>  
> @@ -159,26 +148,14 @@ static void icp_kvm_cpu_setup(ICPState *icp, PowerPCCPU *cpu)
>      QLIST_INSERT_HEAD(&kvm_enabled_icps, enabled_icp, node);
>  }
>  
> -static void icp_kvm_realize(DeviceState *dev, Error **errp)
> -{
> -    qemu_register_reset(icp_kvm_reset, dev);
> -}
> -
> -static void icp_kvm_unrealize(DeviceState *dev, Error **errp)
> -{
> -    qemu_unregister_reset(icp_kvm_reset, dev);
> -}
> -
>  static void icp_kvm_class_init(ObjectClass *klass, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
>      ICPStateClass *icpc = ICP_CLASS(klass);
>  
> -    dc->realize = icp_kvm_realize;
> -    dc->unrealize = icp_kvm_unrealize;
>      icpc->pre_save = icp_get_kvm_state;
>      icpc->post_load = icp_set_kvm_state;
>      icpc->cpu_setup = icp_kvm_cpu_setup;
> +    icpc->reset = icp_kvm_reset;
>  }
>  
>  static const TypeInfo icp_kvm_info = {
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index a3073f90533a..40a506eacfb4 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -69,6 +69,7 @@ struct ICPStateClass {
>      void (*pre_save)(ICPState *s);
>      int (*post_load)(ICPState *s, int version_id);
>      void (*cpu_setup)(ICPState *icp, PowerPCCPU *cpu);
> +    void (*reset)(ICPState *icp);
>  };
>  
>  struct ICPState {
> 

On Wed, Jun 07, 2017 at 07:47:04PM +0200, Cédric Le Goater wrote:
1;4602;0c> On 06/07/2017 07:17 PM, Greg Kurz wrote:
> > Taking into account that qemu_set_irq() returns immediatly if its first
> > argument is NULL, icp_kvm_reset() largely duplicates icp_reset().
> > 
> > This patch introduces a reset() handler, so that the common logic can
> > be implemented in icp_reset() only.
> > 
> > While there we can also drop icp_kvm_realize() and icp_kvm_unrealize(). This
> > causes icp-kvm to be realized in icp_realize(), which sets icp->xics, but
> > it has no impact.
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> 
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> > ---
> >  hw/intc/xics.c        |    5 +++++
> >  hw/intc/xics_kvm.c    |   27 ++-------------------------
> >  include/hw/ppc/xics.h |    1 +
> >  3 files changed, 8 insertions(+), 25 deletions(-)
> 
> another good cleanup !

I concur.  Applied to ppc-for-2.10.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson