| Message ID | 1487252865-12064-2-git-send-email-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Feb 16, 2017 at 02:47:24PM +0100, Cédric Le Goater wrote: > Today, the ICS (Interrupt Controller Source) object is created and > realized by the init and realize routines of the XICS object, but some > of the parameters are only known at the machine level. > > These parameters are passed from the sPAPR machine to the ICS object > in a rather convoluted way using property handlers and a class handler > of the XICS object. The number of irqs required to allocate the IRQ > state objects in the ICS realize routine is one of them. > > Let's simplify the process by creating the ICS object along with the > XICS object at the machine level and link the ICS into the XICS list > of ICSs at this level also. In the sPAPR machine, there is only a > single ICS but that will change with the PowerNV machine. > > Also, QOMify the creation of the objects and get rid of the > superfluous code. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > > Changes since v1: > > - added a XICS link to the ICS object > > hw/intc/xics.c | 51 ++++++++++++++++--------------------------------- > hw/intc/xics_kvm.c | 44 ++++++++++-------------------------------- > hw/intc/xics_spapr.c | 34 --------------------------------- > hw/ppc/spapr.c | 53 +++++++++++++++++++++++++++++++++++---------------- > include/hw/ppc/xics.h | 2 -- > 5 files changed, 63 insertions(+), 121 deletions(-) > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index 372b8311fb8b..e70d3b8b1095 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -151,38 +151,6 @@ static void xics_common_reset(DeviceState *d) > } > } > > -static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name, > - void *opaque, Error **errp) > -{ > - XICSState *xics = XICS_COMMON(obj); > - int64_t value = xics->nr_irqs; > - > - visit_type_int(v, name, &value, errp); > -} > - > -static void xics_prop_set_nr_irqs(Object *obj, Visitor *v, const char *name, > - void *opaque, Error **errp) > -{ > - XICSState *xics = XICS_COMMON(obj); > - XICSStateClass *info = XICS_COMMON_GET_CLASS(xics); > - Error *error = NULL; > - int64_t value; > - > - visit_type_int(v, name, &value, &error); > - if (error) { > - error_propagate(errp, error); > - return; > - } > - if (xics->nr_irqs) { > - error_setg(errp, "Number of interrupts is already set to %u", > - xics->nr_irqs); > - return; > - } > - > - assert(info->set_nr_irqs); > - info->set_nr_irqs(xics, value, errp); > -} > - > void xics_set_nr_servers(XICSState *xics, uint32_t nr_servers, > const char *typename, Error **errp) > { > @@ -241,9 +209,6 @@ static void xics_common_initfn(Object *obj) > XICSState *xics = XICS_COMMON(obj); > > QLIST_INIT(&xics->ics); > - object_property_add(obj, "nr_irqs", "int", > - xics_prop_get_nr_irqs, xics_prop_set_nr_irqs, > - NULL, NULL, NULL); > object_property_add(obj, "nr_servers", "int", > xics_prop_get_nr_servers, xics_prop_set_nr_servers, > NULL, NULL, NULL); > @@ -737,6 +702,16 @@ static void ics_simple_initfn(Object *obj) > static void ics_simple_realize(DeviceState *dev, Error **errp) > { > ICSState *ics = ICS_SIMPLE(dev); > + Object *obj; > + Error *err = NULL; > + > + obj = object_property_get_link(OBJECT(dev), "xics", &err); > + if (!obj) { > + error_setg(errp, "%s: required link 'xics' not found: %s", > + __func__, error_get_pretty(err)); > + return; > + } > + ics->xics = XICS_COMMON(obj); > > if (!ics->nr_irqs) { > error_setg(errp, "Number of interrupts needs to be greater 0"); > @@ -746,12 +721,18 @@ static void ics_simple_realize(DeviceState *dev, Error **errp) > ics->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs); > } > > +static Property ics_simple_properties[] = { > + DEFINE_PROP_UINT32("nr-irqs", ICSState, nr_irqs, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void ics_simple_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > ICSStateClass *isc = ICS_BASE_CLASS(klass); > > dc->realize = ics_simple_realize; > + dc->props = ics_simple_properties; > dc->vmsd = &vmstate_ics_simple; > dc->reset = ics_simple_reset; > isc->post_load = ics_simple_post_load; > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > index 17694eaa8709..4a6c0522feb6 100644 > --- a/hw/intc/xics_kvm.c > +++ b/hw/intc/xics_kvm.c > @@ -294,6 +294,16 @@ static void ics_kvm_reset(DeviceState *dev) > static void ics_kvm_realize(DeviceState *dev, Error **errp) > { > ICSState *ics = ICS_SIMPLE(dev); > + Object *obj; > + Error *err = NULL; > + > + obj = object_property_get_link(OBJECT(dev), "xics", &err); > + if (!obj) { > + error_setg(errp, "%s: required link 'xics' not found: %s", > + __func__, error_get_pretty(err)); > + return; > + } > + ics->xics = XICS_COMMON(obj); Couldn't you have this logic in the base class rather than repeating it in ics simple and ics kvm? > if (!ics->nr_irqs) { > error_setg(errp, "Number of interrupts needs to be greater 0"); > @@ -358,18 +368,6 @@ static void xics_kvm_cpu_setup(XICSState *xics, PowerPCCPU *cpu) > ss->cap_irq_xics_enabled = true; > } > > -static void xics_kvm_set_nr_irqs(XICSState *xics, uint32_t nr_irqs, > - Error **errp) > -{ > - ICSState *ics = QLIST_FIRST(&xics->ics); > - > - /* This needs to be deprecated ... */ > - xics->nr_irqs = nr_irqs; > - if (ics) { > - ics->nr_irqs = nr_irqs; > - } > -} > - > static void xics_kvm_set_nr_servers(XICSState *xics, uint32_t nr_servers, > Error **errp) > { > @@ -389,7 +387,6 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp) > { > KVMXICSState *xicskvm = XICS_SPAPR_KVM(dev); > XICSState *xics = XICS_COMMON(dev); > - ICSState *ics; > int i, rc; > Error *error = NULL; > struct kvm_create_device xics_create_device = { > @@ -441,14 +438,6 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp) > > xicskvm->kernel_xics_fd = xics_create_device.fd; > > - QLIST_FOREACH(ics, &xics->ics, list) { > - object_property_set_bool(OBJECT(ics), true, "realized", &error); > - if (error) { > - error_propagate(errp, error); > - goto fail; > - } > - } > - > assert(xics->nr_servers); > for (i = 0; i < xics->nr_servers; i++) { > object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized", > @@ -472,17 +461,6 @@ fail: > kvmppc_define_rtas_kernel_token(0, "ibm,int-off"); > } > > -static void xics_kvm_initfn(Object *obj) > -{ > - XICSState *xics = XICS_COMMON(obj); > - ICSState *ics; > - > - ics = ICS_SIMPLE(object_new(TYPE_ICS_KVM)); > - object_property_add_child(obj, "ics", OBJECT(ics), NULL); > - ics->xics = xics; > - QLIST_INSERT_HEAD(&xics->ics, ics, list); > -} > - > static void xics_kvm_class_init(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > @@ -490,7 +468,6 @@ static void xics_kvm_class_init(ObjectClass *oc, void *data) > > dc->realize = xics_kvm_realize; > xsc->cpu_setup = xics_kvm_cpu_setup; > - xsc->set_nr_irqs = xics_kvm_set_nr_irqs; > xsc->set_nr_servers = xics_kvm_set_nr_servers; > } > > @@ -499,7 +476,6 @@ static const TypeInfo xics_spapr_kvm_info = { > .parent = TYPE_XICS_COMMON, > .instance_size = sizeof(KVMXICSState), > .class_init = xics_kvm_class_init, > - .instance_init = xics_kvm_initfn, > }; > > static void xics_kvm_register_types(void) > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > index 2e3f1c5e95b2..03e42a866603 100644 > --- a/hw/intc/xics_spapr.c > +++ b/hw/intc/xics_spapr.c > @@ -239,18 +239,6 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr, > rtas_st(rets, 0, RTAS_OUT_SUCCESS); > } > > -static void xics_spapr_set_nr_irqs(XICSState *xics, uint32_t nr_irqs, > - Error **errp) > -{ > - ICSState *ics = QLIST_FIRST(&xics->ics); > - > - /* This needs to be deprecated ... */ > - xics->nr_irqs = nr_irqs; > - if (ics) { > - ics->nr_irqs = nr_irqs; > - } > -} > - > static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers, > Error **errp) > { > @@ -260,7 +248,6 @@ static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers, > static void xics_spapr_realize(DeviceState *dev, Error **errp) > { > XICSState *xics = XICS_SPAPR(dev); > - ICSState *ics; > Error *error = NULL; > int i; > > @@ -282,14 +269,6 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp) > spapr_register_hypercall(H_EOI, h_eoi); > spapr_register_hypercall(H_IPOLL, h_ipoll); > > - QLIST_FOREACH(ics, &xics->ics, list) { > - object_property_set_bool(OBJECT(ics), true, "realized", &error); > - if (error) { > - error_propagate(errp, error); > - return; > - } > - } > - > for (i = 0; i < xics->nr_servers; i++) { > object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized", > &error); > @@ -300,24 +279,12 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp) > } > } > > -static void xics_spapr_initfn(Object *obj) > -{ > - XICSState *xics = XICS_SPAPR(obj); > - ICSState *ics; > - > - ics = ICS_SIMPLE(object_new(TYPE_ICS_SIMPLE)); > - object_property_add_child(obj, "ics", OBJECT(ics), NULL); > - ics->xics = xics; > - QLIST_INSERT_HEAD(&xics->ics, ics, list); > -} > - > static void xics_spapr_class_init(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > XICSStateClass *xsc = XICS_SPAPR_CLASS(oc); > > dc->realize = xics_spapr_realize; > - xsc->set_nr_irqs = xics_spapr_set_nr_irqs; > xsc->set_nr_servers = xics_spapr_set_nr_servers; > } > > @@ -327,7 +294,6 @@ static const TypeInfo xics_spapr_info = { > .instance_size = sizeof(XICSState), > .class_size = sizeof(XICSStateClass), > .class_init = xics_spapr_class_init, > - .instance_init = xics_spapr_initfn, > }; > > #define ICS_IRQ_FREE(ics, srcno) \ > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 9b6ad0f2fd77..94b1e8e3227a 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -95,22 +95,42 @@ > > #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift)) > > -static XICSState *try_create_xics(const char *type, int nr_servers, > - int nr_irqs, Error **errp) > -{ > - Error *err = NULL; > - DeviceState *dev; > +static XICSState *try_create_xics(const char *type, const char *type_ics, > + int nr_servers, int nr_irqs, Error **errp) > +{ > + Error *err = NULL, *local_err = NULL; > + XICSState *xics; > + ICSState *ics = NULL; > + > + xics = XICS_COMMON(object_new(type)); > + qdev_set_parent_bus(DEVICE(xics), sysbus_get_default()); > + object_property_set_int(OBJECT(xics), nr_servers, "nr_servers", &err); > + object_property_set_bool(OBJECT(xics), true, "realized", &local_err); > + error_propagate(&err, local_err); > + if (err) { > + goto error; > + } > > - dev = DEVICE(object_new(type)); > - qdev_prop_set_uint32(dev, "nr_servers", nr_servers); > - qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs); > - object_property_set_bool(OBJECT(dev), true, "realized", &err); > + ics = ICS_SIMPLE(object_new(type_ics)); > + object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL); > + object_property_set_int(OBJECT(ics), nr_irqs, "nr-irqs", &err); > + object_property_add_const_link(OBJECT(ics), "xics", OBJECT(xics), NULL); > + object_property_set_bool(OBJECT(ics), true, "realized", &local_err); > + error_propagate(&err, local_err); > if (err) { > - error_propagate(errp, err); > - object_unparent(OBJECT(dev)); > - return NULL; > + goto error; > } > - return XICS_COMMON(dev); > + QLIST_INSERT_HEAD(&xics->ics, ics, list); > + > + return xics; > + > +error: > + error_propagate(errp, err); > + if (ics) { > + object_unparent(OBJECT(ics)); > + } > + object_unparent(OBJECT(xics)); > + return NULL; > } > > static XICSState *xics_system_init(MachineState *machine, > @@ -122,8 +142,8 @@ static XICSState *xics_system_init(MachineState *machine, > Error *err = NULL; > > if (machine_kernel_irqchip_allowed(machine)) { > - xics = try_create_xics(TYPE_XICS_SPAPR_KVM, nr_servers, nr_irqs, > - &err); > + xics = try_create_xics(TYPE_XICS_SPAPR_KVM, TYPE_ICS_KVM, > + nr_servers, nr_irqs, &err); > } > if (machine_kernel_irqchip_required(machine) && !xics) { > error_reportf_err(err, > @@ -134,7 +154,8 @@ static XICSState *xics_system_init(MachineState *machine, > } > > if (!xics) { > - xics = try_create_xics(TYPE_XICS_SPAPR, nr_servers, nr_irqs, errp); > + xics = try_create_xics(TYPE_XICS_SPAPR, TYPE_ICS_SIMPLE, nr_servers, > + nr_irqs, errp); > } > > return xics; > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index 3f0c31610aa4..8fe3c4d2ceab 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -74,7 +74,6 @@ struct XICSStateClass { > DeviceClass parent_class; > > void (*cpu_setup)(XICSState *icp, PowerPCCPU *cpu); > - void (*set_nr_irqs)(XICSState *icp, uint32_t nr_irqs, Error **errp); > void (*set_nr_servers)(XICSState *icp, uint32_t nr_servers, Error **errp); > }; > > @@ -83,7 +82,6 @@ struct XICSState { > SysBusDevice parent_obj; > /*< public >*/ > uint32_t nr_servers; > - uint32_t nr_irqs; > ICPState *ss; > QLIST_HEAD(, ICSState) ics; > };
On 02/22/2017 04:21 AM, David Gibson wrote: > On Thu, Feb 16, 2017 at 02:47:24PM +0100, Cédric Le Goater wrote: >> Today, the ICS (Interrupt Controller Source) object is created and >> realized by the init and realize routines of the XICS object, but some >> of the parameters are only known at the machine level. >> >> These parameters are passed from the sPAPR machine to the ICS object >> in a rather convoluted way using property handlers and a class handler >> of the XICS object. The number of irqs required to allocate the IRQ >> state objects in the ICS realize routine is one of them. >> >> Let's simplify the process by creating the ICS object along with the >> XICS object at the machine level and link the ICS into the XICS list >> of ICSs at this level also. In the sPAPR machine, there is only a >> single ICS but that will change with the PowerNV machine. >> >> Also, QOMify the creation of the objects and get rid of the >> superfluous code. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> >> Changes since v1: >> >> - added a XICS link to the ICS object >> >> hw/intc/xics.c | 51 ++++++++++++++++--------------------------------- >> hw/intc/xics_kvm.c | 44 ++++++++++-------------------------------- >> hw/intc/xics_spapr.c | 34 --------------------------------- >> hw/ppc/spapr.c | 53 +++++++++++++++++++++++++++++++++++---------------- >> include/hw/ppc/xics.h | 2 -- >> 5 files changed, 63 insertions(+), 121 deletions(-) >> >> diff --git a/hw/intc/xics.c b/hw/intc/xics.c >> index 372b8311fb8b..e70d3b8b1095 100644 >> --- a/hw/intc/xics.c >> +++ b/hw/intc/xics.c >> @@ -151,38 +151,6 @@ static void xics_common_reset(DeviceState *d) >> } >> } >> >> -static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name, >> - void *opaque, Error **errp) >> -{ >> - XICSState *xics = XICS_COMMON(obj); >> - int64_t value = xics->nr_irqs; >> - >> - visit_type_int(v, name, &value, errp); >> -} >> - >> -static void xics_prop_set_nr_irqs(Object *obj, Visitor *v, const char *name, >> - void *opaque, Error **errp) >> -{ >> - XICSState *xics = XICS_COMMON(obj); >> - XICSStateClass *info = XICS_COMMON_GET_CLASS(xics); >> - Error *error = NULL; >> - int64_t value; >> - >> - visit_type_int(v, name, &value, &error); >> - if (error) { >> - error_propagate(errp, error); >> - return; >> - } >> - if (xics->nr_irqs) { >> - error_setg(errp, "Number of interrupts is already set to %u", >> - xics->nr_irqs); >> - return; >> - } >> - >> - assert(info->set_nr_irqs); >> - info->set_nr_irqs(xics, value, errp); >> -} >> - >> void xics_set_nr_servers(XICSState *xics, uint32_t nr_servers, >> const char *typename, Error **errp) >> { >> @@ -241,9 +209,6 @@ static void xics_common_initfn(Object *obj) >> XICSState *xics = XICS_COMMON(obj); >> >> QLIST_INIT(&xics->ics); >> - object_property_add(obj, "nr_irqs", "int", >> - xics_prop_get_nr_irqs, xics_prop_set_nr_irqs, >> - NULL, NULL, NULL); >> object_property_add(obj, "nr_servers", "int", >> xics_prop_get_nr_servers, xics_prop_set_nr_servers, >> NULL, NULL, NULL); >> @@ -737,6 +702,16 @@ static void ics_simple_initfn(Object *obj) >> static void ics_simple_realize(DeviceState *dev, Error **errp) >> { >> ICSState *ics = ICS_SIMPLE(dev); >> + Object *obj; >> + Error *err = NULL; >> + >> + obj = object_property_get_link(OBJECT(dev), "xics", &err); >> + if (!obj) { >> + error_setg(errp, "%s: required link 'xics' not found: %s", >> + __func__, error_get_pretty(err)); >> + return; >> + } >> + ics->xics = XICS_COMMON(obj); >> >> if (!ics->nr_irqs) { >> error_setg(errp, "Number of interrupts needs to be greater 0"); >> @@ -746,12 +721,18 @@ static void ics_simple_realize(DeviceState *dev, Error **errp) >> ics->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs); >> } >> >> +static Property ics_simple_properties[] = { >> + DEFINE_PROP_UINT32("nr-irqs", ICSState, nr_irqs, 0), >> + DEFINE_PROP_END_OF_LIST(), >> +}; >> + >> static void ics_simple_class_init(ObjectClass *klass, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(klass); >> ICSStateClass *isc = ICS_BASE_CLASS(klass); >> >> dc->realize = ics_simple_realize; >> + dc->props = ics_simple_properties; >> dc->vmsd = &vmstate_ics_simple; >> dc->reset = ics_simple_reset; >> isc->post_load = ics_simple_post_load; >> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c >> index 17694eaa8709..4a6c0522feb6 100644 >> --- a/hw/intc/xics_kvm.c >> +++ b/hw/intc/xics_kvm.c >> @@ -294,6 +294,16 @@ static void ics_kvm_reset(DeviceState *dev) >> static void ics_kvm_realize(DeviceState *dev, Error **errp) >> { >> ICSState *ics = ICS_SIMPLE(dev); >> + Object *obj; >> + Error *err = NULL; >> + >> + obj = object_property_get_link(OBJECT(dev), "xics", &err); >> + if (!obj) { >> + error_setg(errp, "%s: required link 'xics' not found: %s", >> + __func__, error_get_pretty(err)); >> + return; >> + } >> + ics->xics = XICS_COMMON(obj); > > Couldn't you have this logic in the base class rather than repeating > it in ics simple and ics kvm? Yes we could. But, as I am getting rid of the backlinks at the end of the patchset, the redundancy is not much of a problem. Thanks, C. >> if (!ics->nr_irqs) { >> error_setg(errp, "Number of interrupts needs to be greater 0"); >> @@ -358,18 +368,6 @@ static void xics_kvm_cpu_setup(XICSState *xics, PowerPCCPU *cpu) >> ss->cap_irq_xics_enabled = true; >> } >> >> -static void xics_kvm_set_nr_irqs(XICSState *xics, uint32_t nr_irqs, >> - Error **errp) >> -{ >> - ICSState *ics = QLIST_FIRST(&xics->ics); >> - >> - /* This needs to be deprecated ... */ >> - xics->nr_irqs = nr_irqs; >> - if (ics) { >> - ics->nr_irqs = nr_irqs; >> - } >> -} >> - >> static void xics_kvm_set_nr_servers(XICSState *xics, uint32_t nr_servers, >> Error **errp) >> { >> @@ -389,7 +387,6 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp) >> { >> KVMXICSState *xicskvm = XICS_SPAPR_KVM(dev); >> XICSState *xics = XICS_COMMON(dev); >> - ICSState *ics; >> int i, rc; >> Error *error = NULL; >> struct kvm_create_device xics_create_device = { >> @@ -441,14 +438,6 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp) >> >> xicskvm->kernel_xics_fd = xics_create_device.fd; >> >> - QLIST_FOREACH(ics, &xics->ics, list) { >> - object_property_set_bool(OBJECT(ics), true, "realized", &error); >> - if (error) { >> - error_propagate(errp, error); >> - goto fail; >> - } >> - } >> - >> assert(xics->nr_servers); >> for (i = 0; i < xics->nr_servers; i++) { >> object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized", >> @@ -472,17 +461,6 @@ fail: >> kvmppc_define_rtas_kernel_token(0, "ibm,int-off"); >> } >> >> -static void xics_kvm_initfn(Object *obj) >> -{ >> - XICSState *xics = XICS_COMMON(obj); >> - ICSState *ics; >> - >> - ics = ICS_SIMPLE(object_new(TYPE_ICS_KVM)); >> - object_property_add_child(obj, "ics", OBJECT(ics), NULL); >> - ics->xics = xics; >> - QLIST_INSERT_HEAD(&xics->ics, ics, list); >> -} >> - >> static void xics_kvm_class_init(ObjectClass *oc, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(oc); >> @@ -490,7 +468,6 @@ static void xics_kvm_class_init(ObjectClass *oc, void *data) >> >> dc->realize = xics_kvm_realize; >> xsc->cpu_setup = xics_kvm_cpu_setup; >> - xsc->set_nr_irqs = xics_kvm_set_nr_irqs; >> xsc->set_nr_servers = xics_kvm_set_nr_servers; >> } >> >> @@ -499,7 +476,6 @@ static const TypeInfo xics_spapr_kvm_info = { >> .parent = TYPE_XICS_COMMON, >> .instance_size = sizeof(KVMXICSState), >> .class_init = xics_kvm_class_init, >> - .instance_init = xics_kvm_initfn, >> }; >> >> static void xics_kvm_register_types(void) >> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c >> index 2e3f1c5e95b2..03e42a866603 100644 >> --- a/hw/intc/xics_spapr.c >> +++ b/hw/intc/xics_spapr.c >> @@ -239,18 +239,6 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr, >> rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> } >> >> -static void xics_spapr_set_nr_irqs(XICSState *xics, uint32_t nr_irqs, >> - Error **errp) >> -{ >> - ICSState *ics = QLIST_FIRST(&xics->ics); >> - >> - /* This needs to be deprecated ... */ >> - xics->nr_irqs = nr_irqs; >> - if (ics) { >> - ics->nr_irqs = nr_irqs; >> - } >> -} >> - >> static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers, >> Error **errp) >> { >> @@ -260,7 +248,6 @@ static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers, >> static void xics_spapr_realize(DeviceState *dev, Error **errp) >> { >> XICSState *xics = XICS_SPAPR(dev); >> - ICSState *ics; >> Error *error = NULL; >> int i; >> >> @@ -282,14 +269,6 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp) >> spapr_register_hypercall(H_EOI, h_eoi); >> spapr_register_hypercall(H_IPOLL, h_ipoll); >> >> - QLIST_FOREACH(ics, &xics->ics, list) { >> - object_property_set_bool(OBJECT(ics), true, "realized", &error); >> - if (error) { >> - error_propagate(errp, error); >> - return; >> - } >> - } >> - >> for (i = 0; i < xics->nr_servers; i++) { >> object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized", >> &error); >> @@ -300,24 +279,12 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp) >> } >> } >> >> -static void xics_spapr_initfn(Object *obj) >> -{ >> - XICSState *xics = XICS_SPAPR(obj); >> - ICSState *ics; >> - >> - ics = ICS_SIMPLE(object_new(TYPE_ICS_SIMPLE)); >> - object_property_add_child(obj, "ics", OBJECT(ics), NULL); >> - ics->xics = xics; >> - QLIST_INSERT_HEAD(&xics->ics, ics, list); >> -} >> - >> static void xics_spapr_class_init(ObjectClass *oc, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(oc); >> XICSStateClass *xsc = XICS_SPAPR_CLASS(oc); >> >> dc->realize = xics_spapr_realize; >> - xsc->set_nr_irqs = xics_spapr_set_nr_irqs; >> xsc->set_nr_servers = xics_spapr_set_nr_servers; >> } >> >> @@ -327,7 +294,6 @@ static const TypeInfo xics_spapr_info = { >> .instance_size = sizeof(XICSState), >> .class_size = sizeof(XICSStateClass), >> .class_init = xics_spapr_class_init, >> - .instance_init = xics_spapr_initfn, >> }; >> >> #define ICS_IRQ_FREE(ics, srcno) \ >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 9b6ad0f2fd77..94b1e8e3227a 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -95,22 +95,42 @@ >> >> #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift)) >> >> -static XICSState *try_create_xics(const char *type, int nr_servers, >> - int nr_irqs, Error **errp) >> -{ >> - Error *err = NULL; >> - DeviceState *dev; >> +static XICSState *try_create_xics(const char *type, const char *type_ics, >> + int nr_servers, int nr_irqs, Error **errp) >> +{ >> + Error *err = NULL, *local_err = NULL; >> + XICSState *xics; >> + ICSState *ics = NULL; >> + >> + xics = XICS_COMMON(object_new(type)); >> + qdev_set_parent_bus(DEVICE(xics), sysbus_get_default()); >> + object_property_set_int(OBJECT(xics), nr_servers, "nr_servers", &err); >> + object_property_set_bool(OBJECT(xics), true, "realized", &local_err); >> + error_propagate(&err, local_err); >> + if (err) { >> + goto error; >> + } >> >> - dev = DEVICE(object_new(type)); >> - qdev_prop_set_uint32(dev, "nr_servers", nr_servers); >> - qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs); >> - object_property_set_bool(OBJECT(dev), true, "realized", &err); >> + ics = ICS_SIMPLE(object_new(type_ics)); >> + object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL); >> + object_property_set_int(OBJECT(ics), nr_irqs, "nr-irqs", &err); >> + object_property_add_const_link(OBJECT(ics), "xics", OBJECT(xics), NULL); >> + object_property_set_bool(OBJECT(ics), true, "realized", &local_err); >> + error_propagate(&err, local_err); >> if (err) { >> - error_propagate(errp, err); >> - object_unparent(OBJECT(dev)); >> - return NULL; >> + goto error; >> } >> - return XICS_COMMON(dev); >> + QLIST_INSERT_HEAD(&xics->ics, ics, list); >> + >> + return xics; >> + >> +error: >> + error_propagate(errp, err); >> + if (ics) { >> + object_unparent(OBJECT(ics)); >> + } >> + object_unparent(OBJECT(xics)); >> + return NULL; >> } >> >> static XICSState *xics_system_init(MachineState *machine, >> @@ -122,8 +142,8 @@ static XICSState *xics_system_init(MachineState *machine, >> Error *err = NULL; >> >> if (machine_kernel_irqchip_allowed(machine)) { >> - xics = try_create_xics(TYPE_XICS_SPAPR_KVM, nr_servers, nr_irqs, >> - &err); >> + xics = try_create_xics(TYPE_XICS_SPAPR_KVM, TYPE_ICS_KVM, >> + nr_servers, nr_irqs, &err); >> } >> if (machine_kernel_irqchip_required(machine) && !xics) { >> error_reportf_err(err, >> @@ -134,7 +154,8 @@ static XICSState *xics_system_init(MachineState *machine, >> } >> >> if (!xics) { >> - xics = try_create_xics(TYPE_XICS_SPAPR, nr_servers, nr_irqs, errp); >> + xics = try_create_xics(TYPE_XICS_SPAPR, TYPE_ICS_SIMPLE, nr_servers, >> + nr_irqs, errp); >> } >> >> return xics; >> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h >> index 3f0c31610aa4..8fe3c4d2ceab 100644 >> --- a/include/hw/ppc/xics.h >> +++ b/include/hw/ppc/xics.h >> @@ -74,7 +74,6 @@ struct XICSStateClass { >> DeviceClass parent_class; >> >> void (*cpu_setup)(XICSState *icp, PowerPCCPU *cpu); >> - void (*set_nr_irqs)(XICSState *icp, uint32_t nr_irqs, Error **errp); >> void (*set_nr_servers)(XICSState *icp, uint32_t nr_servers, Error **errp); >> }; >> >> @@ -83,7 +82,6 @@ struct XICSState { >> SysBusDevice parent_obj; >> /*< public >*/ >> uint32_t nr_servers; >> - uint32_t nr_irqs; >> ICPState *ss; >> QLIST_HEAD(, ICSState) ics; >> }; >
diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 372b8311fb8b..e70d3b8b1095 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -151,38 +151,6 @@ static void xics_common_reset(DeviceState *d) } } -static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name, - void *opaque, Error **errp) -{ - XICSState *xics = XICS_COMMON(obj); - int64_t value = xics->nr_irqs; - - visit_type_int(v, name, &value, errp); -} - -static void xics_prop_set_nr_irqs(Object *obj, Visitor *v, const char *name, - void *opaque, Error **errp) -{ - XICSState *xics = XICS_COMMON(obj); - XICSStateClass *info = XICS_COMMON_GET_CLASS(xics); - Error *error = NULL; - int64_t value; - - visit_type_int(v, name, &value, &error); - if (error) { - error_propagate(errp, error); - return; - } - if (xics->nr_irqs) { - error_setg(errp, "Number of interrupts is already set to %u", - xics->nr_irqs); - return; - } - - assert(info->set_nr_irqs); - info->set_nr_irqs(xics, value, errp); -} - void xics_set_nr_servers(XICSState *xics, uint32_t nr_servers, const char *typename, Error **errp) { @@ -241,9 +209,6 @@ static void xics_common_initfn(Object *obj) XICSState *xics = XICS_COMMON(obj); QLIST_INIT(&xics->ics); - object_property_add(obj, "nr_irqs", "int", - xics_prop_get_nr_irqs, xics_prop_set_nr_irqs, - NULL, NULL, NULL); object_property_add(obj, "nr_servers", "int", xics_prop_get_nr_servers, xics_prop_set_nr_servers, NULL, NULL, NULL); @@ -737,6 +702,16 @@ static void ics_simple_initfn(Object *obj) static void ics_simple_realize(DeviceState *dev, Error **errp) { ICSState *ics = ICS_SIMPLE(dev); + Object *obj; + Error *err = NULL; + + obj = object_property_get_link(OBJECT(dev), "xics", &err); + if (!obj) { + error_setg(errp, "%s: required link 'xics' not found: %s", + __func__, error_get_pretty(err)); + return; + } + ics->xics = XICS_COMMON(obj); if (!ics->nr_irqs) { error_setg(errp, "Number of interrupts needs to be greater 0"); @@ -746,12 +721,18 @@ static void ics_simple_realize(DeviceState *dev, Error **errp) ics->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs); } +static Property ics_simple_properties[] = { + DEFINE_PROP_UINT32("nr-irqs", ICSState, nr_irqs, 0), + DEFINE_PROP_END_OF_LIST(), +}; + static void ics_simple_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); ICSStateClass *isc = ICS_BASE_CLASS(klass); dc->realize = ics_simple_realize; + dc->props = ics_simple_properties; dc->vmsd = &vmstate_ics_simple; dc->reset = ics_simple_reset; isc->post_load = ics_simple_post_load; diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c index 17694eaa8709..4a6c0522feb6 100644 --- a/hw/intc/xics_kvm.c +++ b/hw/intc/xics_kvm.c @@ -294,6 +294,16 @@ static void ics_kvm_reset(DeviceState *dev) static void ics_kvm_realize(DeviceState *dev, Error **errp) { ICSState *ics = ICS_SIMPLE(dev); + Object *obj; + Error *err = NULL; + + obj = object_property_get_link(OBJECT(dev), "xics", &err); + if (!obj) { + error_setg(errp, "%s: required link 'xics' not found: %s", + __func__, error_get_pretty(err)); + return; + } + ics->xics = XICS_COMMON(obj); if (!ics->nr_irqs) { error_setg(errp, "Number of interrupts needs to be greater 0"); @@ -358,18 +368,6 @@ static void xics_kvm_cpu_setup(XICSState *xics, PowerPCCPU *cpu) ss->cap_irq_xics_enabled = true; } -static void xics_kvm_set_nr_irqs(XICSState *xics, uint32_t nr_irqs, - Error **errp) -{ - ICSState *ics = QLIST_FIRST(&xics->ics); - - /* This needs to be deprecated ... */ - xics->nr_irqs = nr_irqs; - if (ics) { - ics->nr_irqs = nr_irqs; - } -} - static void xics_kvm_set_nr_servers(XICSState *xics, uint32_t nr_servers, Error **errp) { @@ -389,7 +387,6 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp) { KVMXICSState *xicskvm = XICS_SPAPR_KVM(dev); XICSState *xics = XICS_COMMON(dev); - ICSState *ics; int i, rc; Error *error = NULL; struct kvm_create_device xics_create_device = { @@ -441,14 +438,6 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp) xicskvm->kernel_xics_fd = xics_create_device.fd; - QLIST_FOREACH(ics, &xics->ics, list) { - object_property_set_bool(OBJECT(ics), true, "realized", &error); - if (error) { - error_propagate(errp, error); - goto fail; - } - } - assert(xics->nr_servers); for (i = 0; i < xics->nr_servers; i++) { object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized", @@ -472,17 +461,6 @@ fail: kvmppc_define_rtas_kernel_token(0, "ibm,int-off"); } -static void xics_kvm_initfn(Object *obj) -{ - XICSState *xics = XICS_COMMON(obj); - ICSState *ics; - - ics = ICS_SIMPLE(object_new(TYPE_ICS_KVM)); - object_property_add_child(obj, "ics", OBJECT(ics), NULL); - ics->xics = xics; - QLIST_INSERT_HEAD(&xics->ics, ics, list); -} - static void xics_kvm_class_init(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); @@ -490,7 +468,6 @@ static void xics_kvm_class_init(ObjectClass *oc, void *data) dc->realize = xics_kvm_realize; xsc->cpu_setup = xics_kvm_cpu_setup; - xsc->set_nr_irqs = xics_kvm_set_nr_irqs; xsc->set_nr_servers = xics_kvm_set_nr_servers; } @@ -499,7 +476,6 @@ static const TypeInfo xics_spapr_kvm_info = { .parent = TYPE_XICS_COMMON, .instance_size = sizeof(KVMXICSState), .class_init = xics_kvm_class_init, - .instance_init = xics_kvm_initfn, }; static void xics_kvm_register_types(void) diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 2e3f1c5e95b2..03e42a866603 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -239,18 +239,6 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr, rtas_st(rets, 0, RTAS_OUT_SUCCESS); } -static void xics_spapr_set_nr_irqs(XICSState *xics, uint32_t nr_irqs, - Error **errp) -{ - ICSState *ics = QLIST_FIRST(&xics->ics); - - /* This needs to be deprecated ... */ - xics->nr_irqs = nr_irqs; - if (ics) { - ics->nr_irqs = nr_irqs; - } -} - static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers, Error **errp) { @@ -260,7 +248,6 @@ static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers, static void xics_spapr_realize(DeviceState *dev, Error **errp) { XICSState *xics = XICS_SPAPR(dev); - ICSState *ics; Error *error = NULL; int i; @@ -282,14 +269,6 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp) spapr_register_hypercall(H_EOI, h_eoi); spapr_register_hypercall(H_IPOLL, h_ipoll); - QLIST_FOREACH(ics, &xics->ics, list) { - object_property_set_bool(OBJECT(ics), true, "realized", &error); - if (error) { - error_propagate(errp, error); - return; - } - } - for (i = 0; i < xics->nr_servers; i++) { object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized", &error); @@ -300,24 +279,12 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp) } } -static void xics_spapr_initfn(Object *obj) -{ - XICSState *xics = XICS_SPAPR(obj); - ICSState *ics; - - ics = ICS_SIMPLE(object_new(TYPE_ICS_SIMPLE)); - object_property_add_child(obj, "ics", OBJECT(ics), NULL); - ics->xics = xics; - QLIST_INSERT_HEAD(&xics->ics, ics, list); -} - static void xics_spapr_class_init(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); XICSStateClass *xsc = XICS_SPAPR_CLASS(oc); dc->realize = xics_spapr_realize; - xsc->set_nr_irqs = xics_spapr_set_nr_irqs; xsc->set_nr_servers = xics_spapr_set_nr_servers; } @@ -327,7 +294,6 @@ static const TypeInfo xics_spapr_info = { .instance_size = sizeof(XICSState), .class_size = sizeof(XICSStateClass), .class_init = xics_spapr_class_init, - .instance_init = xics_spapr_initfn, }; #define ICS_IRQ_FREE(ics, srcno) \ diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 9b6ad0f2fd77..94b1e8e3227a 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -95,22 +95,42 @@ #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift)) -static XICSState *try_create_xics(const char *type, int nr_servers, - int nr_irqs, Error **errp) -{ - Error *err = NULL; - DeviceState *dev; +static XICSState *try_create_xics(const char *type, const char *type_ics, + int nr_servers, int nr_irqs, Error **errp) +{ + Error *err = NULL, *local_err = NULL; + XICSState *xics; + ICSState *ics = NULL; + + xics = XICS_COMMON(object_new(type)); + qdev_set_parent_bus(DEVICE(xics), sysbus_get_default()); + object_property_set_int(OBJECT(xics), nr_servers, "nr_servers", &err); + object_property_set_bool(OBJECT(xics), true, "realized", &local_err); + error_propagate(&err, local_err); + if (err) { + goto error; + } - dev = DEVICE(object_new(type)); - qdev_prop_set_uint32(dev, "nr_servers", nr_servers); - qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs); - object_property_set_bool(OBJECT(dev), true, "realized", &err); + ics = ICS_SIMPLE(object_new(type_ics)); + object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL); + object_property_set_int(OBJECT(ics), nr_irqs, "nr-irqs", &err); + object_property_add_const_link(OBJECT(ics), "xics", OBJECT(xics), NULL); + object_property_set_bool(OBJECT(ics), true, "realized", &local_err); + error_propagate(&err, local_err); if (err) { - error_propagate(errp, err); - object_unparent(OBJECT(dev)); - return NULL; + goto error; } - return XICS_COMMON(dev); + QLIST_INSERT_HEAD(&xics->ics, ics, list); + + return xics; + +error: + error_propagate(errp, err); + if (ics) { + object_unparent(OBJECT(ics)); + } + object_unparent(OBJECT(xics)); + return NULL; } static XICSState *xics_system_init(MachineState *machine, @@ -122,8 +142,8 @@ static XICSState *xics_system_init(MachineState *machine, Error *err = NULL; if (machine_kernel_irqchip_allowed(machine)) { - xics = try_create_xics(TYPE_XICS_SPAPR_KVM, nr_servers, nr_irqs, - &err); + xics = try_create_xics(TYPE_XICS_SPAPR_KVM, TYPE_ICS_KVM, + nr_servers, nr_irqs, &err); } if (machine_kernel_irqchip_required(machine) && !xics) { error_reportf_err(err, @@ -134,7 +154,8 @@ static XICSState *xics_system_init(MachineState *machine, } if (!xics) { - xics = try_create_xics(TYPE_XICS_SPAPR, nr_servers, nr_irqs, errp); + xics = try_create_xics(TYPE_XICS_SPAPR, TYPE_ICS_SIMPLE, nr_servers, + nr_irqs, errp); } return xics; diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h index 3f0c31610aa4..8fe3c4d2ceab 100644 --- a/include/hw/ppc/xics.h +++ b/include/hw/ppc/xics.h @@ -74,7 +74,6 @@ struct XICSStateClass { DeviceClass parent_class; void (*cpu_setup)(XICSState *icp, PowerPCCPU *cpu); - void (*set_nr_irqs)(XICSState *icp, uint32_t nr_irqs, Error **errp); void (*set_nr_servers)(XICSState *icp, uint32_t nr_servers, Error **errp); }; @@ -83,7 +82,6 @@ struct XICSState { SysBusDevice parent_obj; /*< public >*/ uint32_t nr_servers; - uint32_t nr_irqs; ICPState *ss; QLIST_HEAD(, ICSState) ics; };
Today, the ICS (Interrupt Controller Source) object is created and realized by the init and realize routines of the XICS object, but some of the parameters are only known at the machine level. These parameters are passed from the sPAPR machine to the ICS object in a rather convoluted way using property handlers and a class handler of the XICS object. The number of irqs required to allocate the IRQ state objects in the ICS realize routine is one of them. Let's simplify the process by creating the ICS object along with the XICS object at the machine level and link the ICS into the XICS list of ICSs at this level also. In the sPAPR machine, there is only a single ICS but that will change with the PowerNV machine. Also, QOMify the creation of the objects and get rid of the superfluous code. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- Changes since v1: - added a XICS link to the ICS object hw/intc/xics.c | 51 ++++++++++++++++--------------------------------- hw/intc/xics_kvm.c | 44 ++++++++++-------------------------------- hw/intc/xics_spapr.c | 34 --------------------------------- hw/ppc/spapr.c | 53 +++++++++++++++++++++++++++++++++++---------------- include/hw/ppc/xics.h | 2 -- 5 files changed, 63 insertions(+), 121 deletions(-)