On Thu, Feb 16, 2017 at 02:47:27PM +0100, Cédric Le Goater wrote: > This is, again, to reduce the use of the list of ICS objects. Let's > make each individual ICS and ICP object an InterruptStatsProvider and > remove this same interface from XICSState. > > Signed-off-by: Cédric Le Goater I'm a little hesitant about this, because it means that getting the interrupt stats information is now spread out over the qom tree, whereas previously there was a single location to get a good summary of the systems overall interrupt status. The previous behaviour seems like it would be more convenient for debugging. That said, I see the structural advantages of this split. Hmm.. still thinking.. > --- > hw/intc/xics.c | 76 +++++++++++++++++++++++++++++++--------------------------- > 1 file changed, 41 insertions(+), 35 deletions(-) > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index 9f22814815c9..b1294417a0ae 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -92,44 +92,44 @@ void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu) > } > } > > -static void xics_common_pic_print_info(InterruptStatsProvider *obj, > - Monitor *mon) > +static void icp_pic_print_info(InterruptStatsProvider *obj, > + Monitor *mon) > { > - XICSState *xics = XICS_COMMON(obj); > - ICSState *ics; > + ICPState *icp = ICP(obj); > + int cpu_index = icp->cs ? icp->cs->cpu_index : -1; > + > + if (!icp->output) { > + return; > + } > + monitor_printf(mon, "CPU %d XIRR=%08x (%p) PP=%02x MFRR=%02x\n", > + cpu_index, icp->xirr, icp->xirr_owner, > + icp->pending_priority, icp->mfrr); > +} > + > +static void ics_simple_pic_print_info(InterruptStatsProvider *obj, > + Monitor *mon) > +{ > + ICSState *ics = ICS_SIMPLE(obj); > uint32_t i; > > - for (i = 0; i < xics->nr_servers; i++) { > - ICPState *icp = &xics->ss[i]; > + monitor_printf(mon, "ICS %4x..%4x %p\n", > + ics->offset, ics->offset + ics->nr_irqs - 1, ics); > > - if (!icp->output) { > - continue; > - } > - monitor_printf(mon, "CPU %d XIRR=%08x (%p) PP=%02x MFRR=%02x\n", > - i, icp->xirr, icp->xirr_owner, > - icp->pending_priority, icp->mfrr); > + if (!ics->irqs) { > + return; > } > > - QLIST_FOREACH(ics, &xics->ics, list) { > - monitor_printf(mon, "ICS %4x..%4x %p\n", > - ics->offset, ics->offset + ics->nr_irqs - 1, ics); > + for (i = 0; i < ics->nr_irqs; i++) { > + ICSIRQState *irq = ics->irqs + i; > > - if (!ics->irqs) { > + if (!(irq->flags & XICS_FLAGS_IRQ_MASK)) { > continue; > } > - > - for (i = 0; i < ics->nr_irqs; i++) { > - ICSIRQState *irq = ics->irqs + i; > - > - if (!(irq->flags & XICS_FLAGS_IRQ_MASK)) { > - continue; > - } > - monitor_printf(mon, " %4x %s %02x %02x\n", > - ics->offset + i, > - (irq->flags & XICS_FLAGS_IRQ_LSI) ? > - "LSI" : "MSI", > - irq->priority, irq->status); > - } > + monitor_printf(mon, " %4x %s %02x %02x\n", > + ics->offset + i, > + (irq->flags & XICS_FLAGS_IRQ_LSI) ? > + "LSI" : "MSI", > + irq->priority, irq->status); > } > } > > @@ -161,10 +161,8 @@ static void xics_common_initfn(Object *obj) > static void xics_common_class_init(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > - InterruptStatsProviderClass *ic = INTERRUPT_STATS_PROVIDER_CLASS(oc); > > dc->reset = xics_common_reset; > - ic->print_info = xics_common_pic_print_info; > } > > static const TypeInfo xics_common_info = { > @@ -174,10 +172,6 @@ static const TypeInfo xics_common_info = { > .class_size = sizeof(XICSStateClass), > .instance_init = xics_common_initfn, > .class_init = xics_common_class_init, > - .interfaces = (InterfaceInfo[]) { > - { TYPE_INTERRUPT_STATS_PROVIDER }, > - { } > - }, > }; > > /* > @@ -414,10 +408,12 @@ static void icp_realize(DeviceState *dev, Error **errp) > static void icp_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > + InterruptStatsProviderClass *ic = INTERRUPT_STATS_PROVIDER_CLASS(klass); > > dc->reset = icp_reset; > dc->vmsd = &vmstate_icp_server; > dc->realize = icp_realize; > + ic->print_info = icp_pic_print_info; > } > > static const TypeInfo icp_info = { > @@ -426,6 +422,10 @@ static const TypeInfo icp_info = { > .instance_size = sizeof(ICPState), > .class_init = icp_class_init, > .class_size = sizeof(ICPStateClass), > + .interfaces = (InterfaceInfo[]) { > + { TYPE_INTERRUPT_STATS_PROVIDER }, > + { } > + }, > }; > > /* > @@ -692,6 +692,7 @@ static void ics_simple_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > ICSStateClass *isc = ICS_BASE_CLASS(klass); > + InterruptStatsProviderClass *ic = INTERRUPT_STATS_PROVIDER_CLASS(klass); > > dc->realize = ics_simple_realize; > dc->props = ics_simple_properties; > @@ -701,6 +702,7 @@ static void ics_simple_class_init(ObjectClass *klass, void *data) > isc->reject = ics_simple_reject; > isc->resend = ics_simple_resend; > isc->eoi = ics_simple_eoi; > + ic->print_info = ics_simple_pic_print_info; > } > > static const TypeInfo ics_simple_info = { > @@ -710,6 +712,10 @@ static const TypeInfo ics_simple_info = { > .class_init = ics_simple_class_init, > .class_size = sizeof(ICSStateClass), > .instance_init = ics_simple_initfn, > + .interfaces = (InterfaceInfo[]) { > + { TYPE_INTERRUPT_STATS_PROVIDER }, > + { } > + }, > }; > > static const TypeInfo ics_base_info = { -- 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