All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] ppc/xics: register reset handlers for the ICP and ICS objects
@ 2017-03-03 12:51 Cédric Le Goater
  2017-03-03 13:05 ` Thomas Huth
  2017-03-06  1:51 ` David Gibson
  0 siblings, 2 replies; 3+ messages in thread
From: Cédric Le Goater @ 2017-03-03 12:51 UTC (permalink / raw)
  To: David Gibson; +Cc: Thomas Huth, qemu-ppc, qemu-devel, Cédric Le Goater

The recent changes on the XICS layer removed the XICSState object to
let the sPAPR machine handle the ICP and ICS directly. The reset of
these objects was previously handled by XICSState, which was a SysBus
device, and to keep the same behavior, the ICP and ICS were assigned
to SysbBus.

But that broke the 'info qtree' command in the monitor. 'qtree'
performs a loop on the children of a bus to print their properties and
SysBus devices are expected to be found under SysBus, which is not the
case anymore.

The fix for this problem is to register reset handlers for the ICP and
ICS objects and stop using SysBus for such devices.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xics.c     | 10 ++++++----
 hw/intc/xics_kvm.c | 15 ++++++++++-----
 hw/ppc/spapr.c     |  2 --
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index ffc0747c7fa2..e740989a1162 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -333,7 +333,7 @@ static const VMStateDescription vmstate_icp_server = {
     },
 };
 
-static void icp_reset(DeviceState *dev)
+static void icp_reset(void *dev)
 {
     ICPState *icp = ICP(dev);
 
@@ -359,6 +359,8 @@ static void icp_realize(DeviceState *dev, Error **errp)
     }
 
     icp->xics = XICS_FABRIC(obj);
+
+    qemu_register_reset(icp_reset, dev);
 }
 
 
@@ -366,7 +368,6 @@ static void icp_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->reset = icp_reset;
     dc->vmsd = &vmstate_icp_server;
     dc->realize = icp_realize;
 }
@@ -522,7 +523,7 @@ static void ics_simple_eoi(ICSState *ics, uint32_t nr)
     }
 }
 
-static void ics_simple_reset(DeviceState *dev)
+static void ics_simple_reset(void *dev)
 {
     ICSState *ics = ICS_SIMPLE(dev);
     int i;
@@ -611,6 +612,8 @@ static void ics_simple_realize(DeviceState *dev, Error **errp)
     }
     ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
     ics->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs);
+
+    qemu_register_reset(ics_simple_reset, dev);
 }
 
 static Property ics_simple_properties[] = {
@@ -626,7 +629,6 @@ static void ics_simple_class_init(ObjectClass *klass, void *data)
     isc->realize = ics_simple_realize;
     dc->props = ics_simple_properties;
     dc->vmsd = &vmstate_ics_simple;
-    dc->reset = ics_simple_reset;
     isc->reject = ics_simple_reject;
     isc->resend = ics_simple_resend;
     isc->eoi = ics_simple_eoi;
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index 0a3daca3bb5a..42e0e0ef8484 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -102,7 +102,7 @@ static int icp_set_kvm_state(ICPState *icp, int version_id)
     return 0;
 }
 
-static void icp_kvm_reset(DeviceState *dev)
+static void icp_kvm_reset(void *dev)
 {
     ICPState *icp = ICP(dev);
 
@@ -146,12 +146,17 @@ static void icp_kvm_cpu_setup(ICPState *icp, PowerPCCPU *cpu)
     icp->cap_irq_xics_enabled = true;
 }
 
+static void icp_kvm_realize(DeviceState *dev, Error **errp)
+{
+    qemu_register_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->reset = icp_kvm_reset;
+    dc->realize = icp_kvm_realize;
     icpc->pre_save = icp_get_kvm_state;
     icpc->post_load = icp_set_kvm_state;
     icpc->cpu_setup = icp_kvm_cpu_setup;
@@ -293,7 +298,7 @@ static void ics_kvm_set_irq(void *opaque, int srcno, int val)
     }
 }
 
-static void ics_kvm_reset(DeviceState *dev)
+static void ics_kvm_reset(void *dev)
 {
     ICSState *ics = ICS_SIMPLE(dev);
     int i;
@@ -324,15 +329,15 @@ static void ics_kvm_realize(DeviceState *dev, Error **errp)
     }
     ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
     ics->qirqs = qemu_allocate_irqs(ics_kvm_set_irq, ics, ics->nr_irqs);
+
+    qemu_register_reset(ics_kvm_reset, dev);
 }
 
 static void ics_kvm_class_init(ObjectClass *klass, void *data)
 {
-    DeviceClass *dc = DEVICE_CLASS(klass);
     ICSStateClass *icsc = ICS_BASE_CLASS(klass);
 
     icsc->realize = ics_kvm_realize;
-    dc->reset = ics_kvm_reset;
     icsc->pre_save = ics_get_kvm_state;
     icsc->post_load = ics_set_kvm_state;
 }
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 14192accf486..c3bb99160545 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -106,7 +106,6 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
     int i;
 
     ics = ICS_SIMPLE(object_new(type_ics));
-    qdev_set_parent_bus(DEVICE(ics), sysbus_get_default());
     object_property_add_child(OBJECT(spapr), "ics", OBJECT(ics), NULL);
     object_property_set_int(OBJECT(ics), nr_irqs, "nr-irqs", &err);
     object_property_add_const_link(OBJECT(ics), "xics", OBJECT(xi), NULL);
@@ -123,7 +122,6 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
         ICPState *icp = &spapr->icps[i];
 
         object_initialize(icp, sizeof(*icp), type_icp);
-        qdev_set_parent_bus(DEVICE(icp), sysbus_get_default());
         object_property_add_child(OBJECT(spapr), "icp[*]", OBJECT(icp), NULL);
         object_property_add_const_link(OBJECT(icp), "xics", OBJECT(xi), NULL);
         object_property_set_bool(OBJECT(icp), true, "realized", &err);
-- 
2.7.4

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

* Re: [Qemu-devel] [PATCH] ppc/xics: register reset handlers for the ICP and ICS objects
  2017-03-03 12:51 [Qemu-devel] [PATCH] ppc/xics: register reset handlers for the ICP and ICS objects Cédric Le Goater
@ 2017-03-03 13:05 ` Thomas Huth
  2017-03-06  1:51 ` David Gibson
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Huth @ 2017-03-03 13:05 UTC (permalink / raw)
  To: Cédric Le Goater, David Gibson; +Cc: qemu-ppc, qemu-devel

On 03.03.2017 13:51, Cédric Le Goater wrote:
> The recent changes on the XICS layer removed the XICSState object to
> let the sPAPR machine handle the ICP and ICS directly. The reset of
> these objects was previously handled by XICSState, which was a SysBus
> device, and to keep the same behavior, the ICP and ICS were assigned
> to SysbBus.
> 
> But that broke the 'info qtree' command in the monitor. 'qtree'
> performs a loop on the children of a bus to print their properties and
> SysBus devices are expected to be found under SysBus, which is not the
> case anymore.
> 
> The fix for this problem is to register reset handlers for the ICP and
> ICS objects and stop using SysBus for such devices.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/intc/xics.c     | 10 ++++++----
>  hw/intc/xics_kvm.c | 15 ++++++++++-----
>  hw/ppc/spapr.c     |  2 --
>  3 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index ffc0747c7fa2..e740989a1162 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -333,7 +333,7 @@ static const VMStateDescription vmstate_icp_server = {
>      },
>  };
>  
> -static void icp_reset(DeviceState *dev)
> +static void icp_reset(void *dev)
>  {
>      ICPState *icp = ICP(dev);
>  
> @@ -359,6 +359,8 @@ static void icp_realize(DeviceState *dev, Error **errp)
>      }
>  
>      icp->xics = XICS_FABRIC(obj);
> +
> +    qemu_register_reset(icp_reset, dev);
>  }
>  
>  
> @@ -366,7 +368,6 @@ static void icp_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    dc->reset = icp_reset;
>      dc->vmsd = &vmstate_icp_server;
>      dc->realize = icp_realize;
>  }
> @@ -522,7 +523,7 @@ static void ics_simple_eoi(ICSState *ics, uint32_t nr)
>      }
>  }
>  
> -static void ics_simple_reset(DeviceState *dev)
> +static void ics_simple_reset(void *dev)
>  {
>      ICSState *ics = ICS_SIMPLE(dev);
>      int i;
> @@ -611,6 +612,8 @@ static void ics_simple_realize(DeviceState *dev, Error **errp)
>      }
>      ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
>      ics->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs);
> +
> +    qemu_register_reset(ics_simple_reset, dev);
>  }
>  
>  static Property ics_simple_properties[] = {
> @@ -626,7 +629,6 @@ static void ics_simple_class_init(ObjectClass *klass, void *data)
>      isc->realize = ics_simple_realize;
>      dc->props = ics_simple_properties;
>      dc->vmsd = &vmstate_ics_simple;
> -    dc->reset = ics_simple_reset;
>      isc->reject = ics_simple_reject;
>      isc->resend = ics_simple_resend;
>      isc->eoi = ics_simple_eoi;
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index 0a3daca3bb5a..42e0e0ef8484 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -102,7 +102,7 @@ static int icp_set_kvm_state(ICPState *icp, int version_id)
>      return 0;
>  }
>  
> -static void icp_kvm_reset(DeviceState *dev)
> +static void icp_kvm_reset(void *dev)
>  {
>      ICPState *icp = ICP(dev);
>  
> @@ -146,12 +146,17 @@ static void icp_kvm_cpu_setup(ICPState *icp, PowerPCCPU *cpu)
>      icp->cap_irq_xics_enabled = true;
>  }
>  
> +static void icp_kvm_realize(DeviceState *dev, Error **errp)
> +{
> +    qemu_register_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->reset = icp_kvm_reset;
> +    dc->realize = icp_kvm_realize;
>      icpc->pre_save = icp_get_kvm_state;
>      icpc->post_load = icp_set_kvm_state;
>      icpc->cpu_setup = icp_kvm_cpu_setup;
> @@ -293,7 +298,7 @@ static void ics_kvm_set_irq(void *opaque, int srcno, int val)
>      }
>  }
>  
> -static void ics_kvm_reset(DeviceState *dev)
> +static void ics_kvm_reset(void *dev)
>  {
>      ICSState *ics = ICS_SIMPLE(dev);
>      int i;
> @@ -324,15 +329,15 @@ static void ics_kvm_realize(DeviceState *dev, Error **errp)
>      }
>      ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
>      ics->qirqs = qemu_allocate_irqs(ics_kvm_set_irq, ics, ics->nr_irqs);
> +
> +    qemu_register_reset(ics_kvm_reset, dev);
>  }
>  
>  static void ics_kvm_class_init(ObjectClass *klass, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
>      ICSStateClass *icsc = ICS_BASE_CLASS(klass);
>  
>      icsc->realize = ics_kvm_realize;
> -    dc->reset = ics_kvm_reset;
>      icsc->pre_save = ics_get_kvm_state;
>      icsc->post_load = ics_set_kvm_state;
>  }
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 14192accf486..c3bb99160545 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -106,7 +106,6 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
>      int i;
>  
>      ics = ICS_SIMPLE(object_new(type_ics));
> -    qdev_set_parent_bus(DEVICE(ics), sysbus_get_default());
>      object_property_add_child(OBJECT(spapr), "ics", OBJECT(ics), NULL);
>      object_property_set_int(OBJECT(ics), nr_irqs, "nr-irqs", &err);
>      object_property_add_const_link(OBJECT(ics), "xics", OBJECT(xi), NULL);
> @@ -123,7 +122,6 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
>          ICPState *icp = &spapr->icps[i];
>  
>          object_initialize(icp, sizeof(*icp), type_icp);
> -        qdev_set_parent_bus(DEVICE(icp), sysbus_get_default());
>          object_property_add_child(OBJECT(spapr), "icp[*]", OBJECT(icp), NULL);
>          object_property_add_const_link(OBJECT(icp), "xics", OBJECT(xi), NULL);
>          object_property_set_bool(OBJECT(icp), true, "realized", &err);

Reviewed-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>

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

* Re: [Qemu-devel] [PATCH] ppc/xics: register reset handlers for the ICP and ICS objects
  2017-03-03 12:51 [Qemu-devel] [PATCH] ppc/xics: register reset handlers for the ICP and ICS objects Cédric Le Goater
  2017-03-03 13:05 ` Thomas Huth
@ 2017-03-06  1:51 ` David Gibson
  1 sibling, 0 replies; 3+ messages in thread
From: David Gibson @ 2017-03-06  1:51 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: Thomas Huth, qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 6091 bytes --]

On Fri, Mar 03, 2017 at 01:51:03PM +0100, Cédric Le Goater wrote:
1;4601;0c> The recent changes on the XICS layer removed the XICSState object to
> let the sPAPR machine handle the ICP and ICS directly. The reset of
> these objects was previously handled by XICSState, which was a SysBus
> device, and to keep the same behavior, the ICP and ICS were assigned
> to SysbBus.
> 
> But that broke the 'info qtree' command in the monitor. 'qtree'
> performs a loop on the children of a bus to print their properties and
> SysBus devices are expected to be found under SysBus, which is not the
> case anymore.
> 
> The fix for this problem is to register reset handlers for the ICP and
> ICS objects and stop using SysBus for such devices.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-2.9, thanks.

> ---
>  hw/intc/xics.c     | 10 ++++++----
>  hw/intc/xics_kvm.c | 15 ++++++++++-----
>  hw/ppc/spapr.c     |  2 --
>  3 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index ffc0747c7fa2..e740989a1162 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -333,7 +333,7 @@ static const VMStateDescription vmstate_icp_server = {
>      },
>  };
>  
> -static void icp_reset(DeviceState *dev)
> +static void icp_reset(void *dev)
>  {
>      ICPState *icp = ICP(dev);
>  
> @@ -359,6 +359,8 @@ static void icp_realize(DeviceState *dev, Error **errp)
>      }
>  
>      icp->xics = XICS_FABRIC(obj);
> +
> +    qemu_register_reset(icp_reset, dev);
>  }
>  
>  
> @@ -366,7 +368,6 @@ static void icp_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    dc->reset = icp_reset;
>      dc->vmsd = &vmstate_icp_server;
>      dc->realize = icp_realize;
>  }
> @@ -522,7 +523,7 @@ static void ics_simple_eoi(ICSState *ics, uint32_t nr)
>      }
>  }
>  
> -static void ics_simple_reset(DeviceState *dev)
> +static void ics_simple_reset(void *dev)
>  {
>      ICSState *ics = ICS_SIMPLE(dev);
>      int i;
> @@ -611,6 +612,8 @@ static void ics_simple_realize(DeviceState *dev, Error **errp)
>      }
>      ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
>      ics->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs);
> +
> +    qemu_register_reset(ics_simple_reset, dev);
>  }
>  
>  static Property ics_simple_properties[] = {
> @@ -626,7 +629,6 @@ static void ics_simple_class_init(ObjectClass *klass, void *data)
>      isc->realize = ics_simple_realize;
>      dc->props = ics_simple_properties;
>      dc->vmsd = &vmstate_ics_simple;
> -    dc->reset = ics_simple_reset;
>      isc->reject = ics_simple_reject;
>      isc->resend = ics_simple_resend;
>      isc->eoi = ics_simple_eoi;
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index 0a3daca3bb5a..42e0e0ef8484 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -102,7 +102,7 @@ static int icp_set_kvm_state(ICPState *icp, int version_id)
>      return 0;
>  }
>  
> -static void icp_kvm_reset(DeviceState *dev)
> +static void icp_kvm_reset(void *dev)
>  {
>      ICPState *icp = ICP(dev);
>  
> @@ -146,12 +146,17 @@ static void icp_kvm_cpu_setup(ICPState *icp, PowerPCCPU *cpu)
>      icp->cap_irq_xics_enabled = true;
>  }
>  
> +static void icp_kvm_realize(DeviceState *dev, Error **errp)
> +{
> +    qemu_register_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->reset = icp_kvm_reset;
> +    dc->realize = icp_kvm_realize;
>      icpc->pre_save = icp_get_kvm_state;
>      icpc->post_load = icp_set_kvm_state;
>      icpc->cpu_setup = icp_kvm_cpu_setup;
> @@ -293,7 +298,7 @@ static void ics_kvm_set_irq(void *opaque, int srcno, int val)
>      }
>  }
>  
> -static void ics_kvm_reset(DeviceState *dev)
> +static void ics_kvm_reset(void *dev)
>  {
>      ICSState *ics = ICS_SIMPLE(dev);
>      int i;
> @@ -324,15 +329,15 @@ static void ics_kvm_realize(DeviceState *dev, Error **errp)
>      }
>      ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
>      ics->qirqs = qemu_allocate_irqs(ics_kvm_set_irq, ics, ics->nr_irqs);
> +
> +    qemu_register_reset(ics_kvm_reset, dev);
>  }
>  
>  static void ics_kvm_class_init(ObjectClass *klass, void *data)
>  {
> -    DeviceClass *dc = DEVICE_CLASS(klass);
>      ICSStateClass *icsc = ICS_BASE_CLASS(klass);
>  
>      icsc->realize = ics_kvm_realize;
> -    dc->reset = ics_kvm_reset;
>      icsc->pre_save = ics_get_kvm_state;
>      icsc->post_load = ics_set_kvm_state;
>  }
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 14192accf486..c3bb99160545 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -106,7 +106,6 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
>      int i;
>  
>      ics = ICS_SIMPLE(object_new(type_ics));
> -    qdev_set_parent_bus(DEVICE(ics), sysbus_get_default());
>      object_property_add_child(OBJECT(spapr), "ics", OBJECT(ics), NULL);
>      object_property_set_int(OBJECT(ics), nr_irqs, "nr-irqs", &err);
>      object_property_add_const_link(OBJECT(ics), "xics", OBJECT(xi), NULL);
> @@ -123,7 +122,6 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
>          ICPState *icp = &spapr->icps[i];
>  
>          object_initialize(icp, sizeof(*icp), type_icp);
> -        qdev_set_parent_bus(DEVICE(icp), sysbus_get_default());
>          object_property_add_child(OBJECT(spapr), "icp[*]", OBJECT(icp), NULL);
>          object_property_add_const_link(OBJECT(icp), "xics", OBJECT(xi), NULL);
>          object_property_set_bool(OBJECT(icp), true, "realized", &err);

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

end of thread, other threads:[~2017-03-06  2:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-03 12:51 [Qemu-devel] [PATCH] ppc/xics: register reset handlers for the ICP and ICS objects Cédric Le Goater
2017-03-03 13:05 ` Thomas Huth
2017-03-06  1:51 ` David Gibson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.