| Message ID | 20190925064534.19155-7-david@gibson.dropbear.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spapr: IRQ subsystem cleanups | expand |
On 25/09/2019 08:45, David Gibson wrote: > We create a subtype of TYPE_ICS specifically for sPAPR. For now all this > does is move the setup of the PAPR specific hcalls and RTAS calls to > the realize() function for this, rather than requiring the PAPR code to > explicitly call xics_spapr_init(). In future it will have some more > function. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Cédric Le Goater <clg@kaod.org> > --- > hw/intc/xics_spapr.c | 34 +++++++++++++++++++++++++++++++++- > hw/ppc/spapr_irq.c | 6 ++---- > include/hw/ppc/xics_spapr.h | 4 +++- > 3 files changed, 38 insertions(+), 6 deletions(-) > > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > index 3e9444813a..e6dd004587 100644 > --- a/hw/intc/xics_spapr.c > +++ b/hw/intc/xics_spapr.c > @@ -283,8 +283,18 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, > rtas_st(rets, 0, RTAS_OUT_SUCCESS); > } > > -void xics_spapr_init(SpaprMachineState *spapr) > +static void ics_spapr_realize(DeviceState *dev, Error **errp) > { > + ICSState *ics = ICS_SPAPR(dev); > + ICSStateClass *icsc = ICS_GET_CLASS(ics); > + Error *local_err = NULL; > + > + icsc->parent_realize(dev, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + > spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive); > spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive); > spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off); > @@ -319,3 +329,25 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, > _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); > _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); > } > + > +static void ics_spapr_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + ICSStateClass *isc = ICS_CLASS(klass); > + > + device_class_set_parent_realize(dc, ics_spapr_realize, > + &isc->parent_realize); > +} > + > +static const TypeInfo ics_spapr_info = { > + .name = TYPE_ICS_SPAPR, > + .parent = TYPE_ICS, > + .class_init = ics_spapr_class_init, > +}; > + > +static void xics_spapr_register_types(void) > +{ > + type_register_static(&ics_spapr_info); > +} > + > +type_init(xics_spapr_register_types) > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index 6c45d2a3c0..8c26fa2d1e 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -98,7 +98,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, > Object *obj; > Error *local_err = NULL; > > - obj = object_new(TYPE_ICS); > + obj = object_new(TYPE_ICS_SPAPR); > object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); > object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), > &error_fatal); > @@ -109,9 +109,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, > return; > } > > - spapr->ics = ICS(obj); > - > - xics_spapr_init(spapr); > + spapr->ics = ICS_SPAPR(obj); > } > > static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h > index 5dabc9a138..691a6d00f7 100644 > --- a/include/hw/ppc/xics_spapr.h > +++ b/include/hw/ppc/xics_spapr.h > @@ -31,11 +31,13 @@ > > #define XICS_NODENAME "interrupt-controller" > > +#define TYPE_ICS_SPAPR "ics-spapr" > +#define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR) > + > void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, > uint32_t phandle); > int xics_kvm_connect(SpaprMachineState *spapr, Error **errp); > void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp); > bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr); > -void xics_spapr_init(SpaprMachineState *spapr); > > #endif /* XICS_SPAPR_H */ >
On Wed, 25 Sep 2019 16:45:20 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > We create a subtype of TYPE_ICS specifically for sPAPR. For now all this > does is move the setup of the PAPR specific hcalls and RTAS calls to > the realize() function for this, rather than requiring the PAPR code to > explicitly call xics_spapr_init(). In future it will have some more > function. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- LGTM, but for extra safety I would also introduce a SpaprIcsState typedef and use it everywhere where we only expect this subtype. Especially in the definition of SpaprMachineState. > hw/intc/xics_spapr.c | 34 +++++++++++++++++++++++++++++++++- > hw/ppc/spapr_irq.c | 6 ++---- > include/hw/ppc/xics_spapr.h | 4 +++- > 3 files changed, 38 insertions(+), 6 deletions(-) > > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > index 3e9444813a..e6dd004587 100644 > --- a/hw/intc/xics_spapr.c > +++ b/hw/intc/xics_spapr.c > @@ -283,8 +283,18 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, > rtas_st(rets, 0, RTAS_OUT_SUCCESS); > } > > -void xics_spapr_init(SpaprMachineState *spapr) > +static void ics_spapr_realize(DeviceState *dev, Error **errp) > { > + ICSState *ics = ICS_SPAPR(dev); > + ICSStateClass *icsc = ICS_GET_CLASS(ics); > + Error *local_err = NULL; > + > + icsc->parent_realize(dev, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + > spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive); > spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive); > spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off); > @@ -319,3 +329,25 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, > _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); > _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); > } > + > +static void ics_spapr_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + ICSStateClass *isc = ICS_CLASS(klass); > + > + device_class_set_parent_realize(dc, ics_spapr_realize, > + &isc->parent_realize); > +} > + > +static const TypeInfo ics_spapr_info = { > + .name = TYPE_ICS_SPAPR, > + .parent = TYPE_ICS, > + .class_init = ics_spapr_class_init, > +}; > + > +static void xics_spapr_register_types(void) > +{ > + type_register_static(&ics_spapr_info); > +} > + > +type_init(xics_spapr_register_types) > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index 6c45d2a3c0..8c26fa2d1e 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -98,7 +98,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, > Object *obj; > Error *local_err = NULL; > > - obj = object_new(TYPE_ICS); > + obj = object_new(TYPE_ICS_SPAPR); > object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); > object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), > &error_fatal); > @@ -109,9 +109,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, > return; > } > > - spapr->ics = ICS(obj); > - > - xics_spapr_init(spapr); > + spapr->ics = ICS_SPAPR(obj); > } > > static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h > index 5dabc9a138..691a6d00f7 100644 > --- a/include/hw/ppc/xics_spapr.h > +++ b/include/hw/ppc/xics_spapr.h > @@ -31,11 +31,13 @@ > > #define XICS_NODENAME "interrupt-controller" > > +#define TYPE_ICS_SPAPR "ics-spapr" > +#define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR) > + > void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, > uint32_t phandle); > int xics_kvm_connect(SpaprMachineState *spapr, Error **errp); > void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp); > bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr); > -void xics_spapr_init(SpaprMachineState *spapr); > > #endif /* XICS_SPAPR_H */
On 25/09/2019 10:40, Greg Kurz wrote: > On Wed, 25 Sep 2019 16:45:20 +1000 > David Gibson <david@gibson.dropbear.id.au> wrote: > >> We create a subtype of TYPE_ICS specifically for sPAPR. For now all this >> does is move the setup of the PAPR specific hcalls and RTAS calls to >> the realize() function for this, rather than requiring the PAPR code to >> explicitly call xics_spapr_init(). In future it will have some more >> function. >> >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >> --- > > LGTM, but for extra safety I would also introduce a SpaprIcsState typedef why ? we have ICS_SPAPR() for the checks already. > and use it everywhere where we only expect this subtype. Especially in > the definition of SpaprMachineState. > >> hw/intc/xics_spapr.c | 34 +++++++++++++++++++++++++++++++++- >> hw/ppc/spapr_irq.c | 6 ++---- >> include/hw/ppc/xics_spapr.h | 4 +++- >> 3 files changed, 38 insertions(+), 6 deletions(-) >> >> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c >> index 3e9444813a..e6dd004587 100644 >> --- a/hw/intc/xics_spapr.c >> +++ b/hw/intc/xics_spapr.c >> @@ -283,8 +283,18 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, >> rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> } >> >> -void xics_spapr_init(SpaprMachineState *spapr) >> +static void ics_spapr_realize(DeviceState *dev, Error **errp) >> { >> + ICSState *ics = ICS_SPAPR(dev); >> + ICSStateClass *icsc = ICS_GET_CLASS(ics); >> + Error *local_err = NULL; >> + >> + icsc->parent_realize(dev, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return; >> + } >> + >> spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive); >> spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive); >> spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off); >> @@ -319,3 +329,25 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, >> _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); >> _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); >> } >> + >> +static void ics_spapr_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + ICSStateClass *isc = ICS_CLASS(klass); >> + >> + device_class_set_parent_realize(dc, ics_spapr_realize, >> + &isc->parent_realize); >> +} >> + >> +static const TypeInfo ics_spapr_info = { >> + .name = TYPE_ICS_SPAPR, >> + .parent = TYPE_ICS, >> + .class_init = ics_spapr_class_init, >> +}; >> + >> +static void xics_spapr_register_types(void) >> +{ >> + type_register_static(&ics_spapr_info); >> +} >> + >> +type_init(xics_spapr_register_types) >> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c >> index 6c45d2a3c0..8c26fa2d1e 100644 >> --- a/hw/ppc/spapr_irq.c >> +++ b/hw/ppc/spapr_irq.c >> @@ -98,7 +98,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, >> Object *obj; >> Error *local_err = NULL; >> >> - obj = object_new(TYPE_ICS); >> + obj = object_new(TYPE_ICS_SPAPR); >> object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); >> object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), >> &error_fatal); >> @@ -109,9 +109,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, >> return; >> } >> >> - spapr->ics = ICS(obj); >> - >> - xics_spapr_init(spapr); >> + spapr->ics = ICS_SPAPR(obj); >> } >> >> static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, >> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h >> index 5dabc9a138..691a6d00f7 100644 >> --- a/include/hw/ppc/xics_spapr.h >> +++ b/include/hw/ppc/xics_spapr.h >> @@ -31,11 +31,13 @@ >> >> #define XICS_NODENAME "interrupt-controller" >> >> +#define TYPE_ICS_SPAPR "ics-spapr" >> +#define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR) >> + >> void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, >> uint32_t phandle); >> int xics_kvm_connect(SpaprMachineState *spapr, Error **errp); >> void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp); >> bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr); >> -void xics_spapr_init(SpaprMachineState *spapr); >> >> #endif /* XICS_SPAPR_H */ >
On Wed, 25 Sep 2019 10:55:35 +0200 Cédric Le Goater <clg@kaod.org> wrote: > On 25/09/2019 10:40, Greg Kurz wrote: > > On Wed, 25 Sep 2019 16:45:20 +1000 > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > >> We create a subtype of TYPE_ICS specifically for sPAPR. For now all this > >> does is move the setup of the PAPR specific hcalls and RTAS calls to > >> the realize() function for this, rather than requiring the PAPR code to > >> explicitly call xics_spapr_init(). In future it will have some more > >> function. > >> > >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > >> --- > > > > LGTM, but for extra safety I would also introduce a SpaprIcsState typedef > > why ? we have ICS_SPAPR() for the checks already. > This is a runtime check. Having a typedef would allow to catch a buggy assignment of a non-sPAPR ICS pointer at compile time. > > and use it everywhere where we only expect this subtype. Especially in > > the definition of SpaprMachineState. > > > >> hw/intc/xics_spapr.c | 34 +++++++++++++++++++++++++++++++++- > >> hw/ppc/spapr_irq.c | 6 ++---- > >> include/hw/ppc/xics_spapr.h | 4 +++- > >> 3 files changed, 38 insertions(+), 6 deletions(-) > >> > >> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > >> index 3e9444813a..e6dd004587 100644 > >> --- a/hw/intc/xics_spapr.c > >> +++ b/hw/intc/xics_spapr.c > >> @@ -283,8 +283,18 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, > >> rtas_st(rets, 0, RTAS_OUT_SUCCESS); > >> } > >> > >> -void xics_spapr_init(SpaprMachineState *spapr) > >> +static void ics_spapr_realize(DeviceState *dev, Error **errp) > >> { > >> + ICSState *ics = ICS_SPAPR(dev); > >> + ICSStateClass *icsc = ICS_GET_CLASS(ics); > >> + Error *local_err = NULL; > >> + > >> + icsc->parent_realize(dev, &local_err); > >> + if (local_err) { > >> + error_propagate(errp, local_err); > >> + return; > >> + } > >> + > >> spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive); > >> spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive); > >> spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off); > >> @@ -319,3 +329,25 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, > >> _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); > >> _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); > >> } > >> + > >> +static void ics_spapr_class_init(ObjectClass *klass, void *data) > >> +{ > >> + DeviceClass *dc = DEVICE_CLASS(klass); > >> + ICSStateClass *isc = ICS_CLASS(klass); > >> + > >> + device_class_set_parent_realize(dc, ics_spapr_realize, > >> + &isc->parent_realize); > >> +} > >> + > >> +static const TypeInfo ics_spapr_info = { > >> + .name = TYPE_ICS_SPAPR, > >> + .parent = TYPE_ICS, > >> + .class_init = ics_spapr_class_init, > >> +}; > >> + > >> +static void xics_spapr_register_types(void) > >> +{ > >> + type_register_static(&ics_spapr_info); > >> +} > >> + > >> +type_init(xics_spapr_register_types) > >> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > >> index 6c45d2a3c0..8c26fa2d1e 100644 > >> --- a/hw/ppc/spapr_irq.c > >> +++ b/hw/ppc/spapr_irq.c > >> @@ -98,7 +98,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, > >> Object *obj; > >> Error *local_err = NULL; > >> > >> - obj = object_new(TYPE_ICS); > >> + obj = object_new(TYPE_ICS_SPAPR); > >> object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); > >> object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), > >> &error_fatal); > >> @@ -109,9 +109,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, > >> return; > >> } > >> > >> - spapr->ics = ICS(obj); > >> - > >> - xics_spapr_init(spapr); > >> + spapr->ics = ICS_SPAPR(obj); > >> } > >> > >> static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > >> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h > >> index 5dabc9a138..691a6d00f7 100644 > >> --- a/include/hw/ppc/xics_spapr.h > >> +++ b/include/hw/ppc/xics_spapr.h > >> @@ -31,11 +31,13 @@ > >> > >> #define XICS_NODENAME "interrupt-controller" > >> > >> +#define TYPE_ICS_SPAPR "ics-spapr" > >> +#define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR) > >> + > >> void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, > >> uint32_t phandle); > >> int xics_kvm_connect(SpaprMachineState *spapr, Error **errp); > >> void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp); > >> bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr); > >> -void xics_spapr_init(SpaprMachineState *spapr); > >> > >> #endif /* XICS_SPAPR_H */ > > >
On Wed, Sep 25, 2019 at 10:55:35AM +0200, Cédric Le Goater wrote: > On 25/09/2019 10:40, Greg Kurz wrote: > > On Wed, 25 Sep 2019 16:45:20 +1000 > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > >> We create a subtype of TYPE_ICS specifically for sPAPR. For now all this > >> does is move the setup of the PAPR specific hcalls and RTAS calls to > >> the realize() function for this, rather than requiring the PAPR code to > >> explicitly call xics_spapr_init(). In future it will have some more > >> function. > >> > >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > >> --- > > > > LGTM, but for extra safety I would also introduce a SpaprIcsState typedef > > why ? we have ICS_SPAPR() for the checks already. Eh.. using typedefs when we haven't actually extended a base type isn't common QOM practice. Yes, it's not as typesafe as it could be, but I'm not really inclined to go to the extra effort here. > > > and use it everywhere where we only expect this subtype. Especially in > > the definition of SpaprMachineState. > > > >> hw/intc/xics_spapr.c | 34 +++++++++++++++++++++++++++++++++- > >> hw/ppc/spapr_irq.c | 6 ++---- > >> include/hw/ppc/xics_spapr.h | 4 +++- > >> 3 files changed, 38 insertions(+), 6 deletions(-) > >> > >> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > >> index 3e9444813a..e6dd004587 100644 > >> --- a/hw/intc/xics_spapr.c > >> +++ b/hw/intc/xics_spapr.c > >> @@ -283,8 +283,18 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, > >> rtas_st(rets, 0, RTAS_OUT_SUCCESS); > >> } > >> > >> -void xics_spapr_init(SpaprMachineState *spapr) > >> +static void ics_spapr_realize(DeviceState *dev, Error **errp) > >> { > >> + ICSState *ics = ICS_SPAPR(dev); > >> + ICSStateClass *icsc = ICS_GET_CLASS(ics); > >> + Error *local_err = NULL; > >> + > >> + icsc->parent_realize(dev, &local_err); > >> + if (local_err) { > >> + error_propagate(errp, local_err); > >> + return; > >> + } > >> + > >> spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive); > >> spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive); > >> spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off); > >> @@ -319,3 +329,25 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, > >> _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); > >> _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); > >> } > >> + > >> +static void ics_spapr_class_init(ObjectClass *klass, void *data) > >> +{ > >> + DeviceClass *dc = DEVICE_CLASS(klass); > >> + ICSStateClass *isc = ICS_CLASS(klass); > >> + > >> + device_class_set_parent_realize(dc, ics_spapr_realize, > >> + &isc->parent_realize); > >> +} > >> + > >> +static const TypeInfo ics_spapr_info = { > >> + .name = TYPE_ICS_SPAPR, > >> + .parent = TYPE_ICS, > >> + .class_init = ics_spapr_class_init, > >> +}; > >> + > >> +static void xics_spapr_register_types(void) > >> +{ > >> + type_register_static(&ics_spapr_info); > >> +} > >> + > >> +type_init(xics_spapr_register_types) > >> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > >> index 6c45d2a3c0..8c26fa2d1e 100644 > >> --- a/hw/ppc/spapr_irq.c > >> +++ b/hw/ppc/spapr_irq.c > >> @@ -98,7 +98,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, > >> Object *obj; > >> Error *local_err = NULL; > >> > >> - obj = object_new(TYPE_ICS); > >> + obj = object_new(TYPE_ICS_SPAPR); > >> object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); > >> object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), > >> &error_fatal); > >> @@ -109,9 +109,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, > >> return; > >> } > >> > >> - spapr->ics = ICS(obj); > >> - > >> - xics_spapr_init(spapr); > >> + spapr->ics = ICS_SPAPR(obj); > >> } > >> > >> static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > >> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h > >> index 5dabc9a138..691a6d00f7 100644 > >> --- a/include/hw/ppc/xics_spapr.h > >> +++ b/include/hw/ppc/xics_spapr.h > >> @@ -31,11 +31,13 @@ > >> > >> #define XICS_NODENAME "interrupt-controller" > >> > >> +#define TYPE_ICS_SPAPR "ics-spapr" > >> +#define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR) > >> + > >> void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, > >> uint32_t phandle); > >> int xics_kvm_connect(SpaprMachineState *spapr, Error **errp); > >> void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp); > >> bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr); > >> -void xics_spapr_init(SpaprMachineState *spapr); > >> > >> #endif /* XICS_SPAPR_H */ > > >
On 26/09/2019 02:56, David Gibson wrote: > On Wed, Sep 25, 2019 at 10:55:35AM +0200, Cédric Le Goater wrote: >> On 25/09/2019 10:40, Greg Kurz wrote: >>> On Wed, 25 Sep 2019 16:45:20 +1000 >>> David Gibson <david@gibson.dropbear.id.au> wrote: >>> >>>> We create a subtype of TYPE_ICS specifically for sPAPR. For now all this >>>> does is move the setup of the PAPR specific hcalls and RTAS calls to >>>> the realize() function for this, rather than requiring the PAPR code to >>>> explicitly call xics_spapr_init(). In future it will have some more >>>> function. >>>> >>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >>>> --- >>> >>> LGTM, but for extra safety I would also introduce a SpaprIcsState typedef >> >> why ? we have ICS_SPAPR() for the checks already. > > Eh.. using typedefs when we haven't actually extended a base type > isn't common QOM practice. Yes, it's not as typesafe as it could be, > but I'm not really inclined to go to the extra effort here. I agree. It is really a pain. C. > >> >>> and use it everywhere where we only expect this subtype. Especially in >>> the definition of SpaprMachineState. >>> >>>> hw/intc/xics_spapr.c | 34 +++++++++++++++++++++++++++++++++- >>>> hw/ppc/spapr_irq.c | 6 ++---- >>>> include/hw/ppc/xics_spapr.h | 4 +++- >>>> 3 files changed, 38 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c >>>> index 3e9444813a..e6dd004587 100644 >>>> --- a/hw/intc/xics_spapr.c >>>> +++ b/hw/intc/xics_spapr.c >>>> @@ -283,8 +283,18 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, >>>> rtas_st(rets, 0, RTAS_OUT_SUCCESS); >>>> } >>>> >>>> -void xics_spapr_init(SpaprMachineState *spapr) >>>> +static void ics_spapr_realize(DeviceState *dev, Error **errp) >>>> { >>>> + ICSState *ics = ICS_SPAPR(dev); >>>> + ICSStateClass *icsc = ICS_GET_CLASS(ics); >>>> + Error *local_err = NULL; >>>> + >>>> + icsc->parent_realize(dev, &local_err); >>>> + if (local_err) { >>>> + error_propagate(errp, local_err); >>>> + return; >>>> + } >>>> + >>>> spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive); >>>> spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive); >>>> spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off); >>>> @@ -319,3 +329,25 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, >>>> _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); >>>> _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); >>>> } >>>> + >>>> +static void ics_spapr_class_init(ObjectClass *klass, void *data) >>>> +{ >>>> + DeviceClass *dc = DEVICE_CLASS(klass); >>>> + ICSStateClass *isc = ICS_CLASS(klass); >>>> + >>>> + device_class_set_parent_realize(dc, ics_spapr_realize, >>>> + &isc->parent_realize); >>>> +} >>>> + >>>> +static const TypeInfo ics_spapr_info = { >>>> + .name = TYPE_ICS_SPAPR, >>>> + .parent = TYPE_ICS, >>>> + .class_init = ics_spapr_class_init, >>>> +}; >>>> + >>>> +static void xics_spapr_register_types(void) >>>> +{ >>>> + type_register_static(&ics_spapr_info); >>>> +} >>>> + >>>> +type_init(xics_spapr_register_types) >>>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c >>>> index 6c45d2a3c0..8c26fa2d1e 100644 >>>> --- a/hw/ppc/spapr_irq.c >>>> +++ b/hw/ppc/spapr_irq.c >>>> @@ -98,7 +98,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, >>>> Object *obj; >>>> Error *local_err = NULL; >>>> >>>> - obj = object_new(TYPE_ICS); >>>> + obj = object_new(TYPE_ICS_SPAPR); >>>> object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); >>>> object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), >>>> &error_fatal); >>>> @@ -109,9 +109,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, >>>> return; >>>> } >>>> >>>> - spapr->ics = ICS(obj); >>>> - >>>> - xics_spapr_init(spapr); >>>> + spapr->ics = ICS_SPAPR(obj); >>>> } >>>> >>>> static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, >>>> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h >>>> index 5dabc9a138..691a6d00f7 100644 >>>> --- a/include/hw/ppc/xics_spapr.h >>>> +++ b/include/hw/ppc/xics_spapr.h >>>> @@ -31,11 +31,13 @@ >>>> >>>> #define XICS_NODENAME "interrupt-controller" >>>> >>>> +#define TYPE_ICS_SPAPR "ics-spapr" >>>> +#define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR) >>>> + >>>> void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, >>>> uint32_t phandle); >>>> int xics_kvm_connect(SpaprMachineState *spapr, Error **errp); >>>> void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp); >>>> bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr); >>>> -void xics_spapr_init(SpaprMachineState *spapr); >>>> >>>> #endif /* XICS_SPAPR_H */ >>> >> >
On Thu, 26 Sep 2019 10:56:46 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > On Wed, Sep 25, 2019 at 10:55:35AM +0200, Cédric Le Goater wrote: > > On 25/09/2019 10:40, Greg Kurz wrote: > > > On Wed, 25 Sep 2019 16:45:20 +1000 > > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > > > >> We create a subtype of TYPE_ICS specifically for sPAPR. For now all this > > >> does is move the setup of the PAPR specific hcalls and RTAS calls to > > >> the realize() function for this, rather than requiring the PAPR code to > > >> explicitly call xics_spapr_init(). In future it will have some more > > >> function. > > >> > > >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > >> --- > > > > > > LGTM, but for extra safety I would also introduce a SpaprIcsState typedef > > > > why ? we have ICS_SPAPR() for the checks already. > > Eh.. using typedefs when we haven't actually extended a base type > isn't common QOM practice. Yes, it's not as typesafe as it could be, > but I'm not really inclined to go to the extra effort here. > I'll soon need to extend the base type with a nr_servers field, and while here with an fd field as well in order to get rid of the ugly global in xics.c. I'll go to the extra effort :) > > > > > and use it everywhere where we only expect this subtype. Especially in > > > the definition of SpaprMachineState. > > > > > >> hw/intc/xics_spapr.c | 34 +++++++++++++++++++++++++++++++++- > > >> hw/ppc/spapr_irq.c | 6 ++---- > > >> include/hw/ppc/xics_spapr.h | 4 +++- > > >> 3 files changed, 38 insertions(+), 6 deletions(-) > > >> > > >> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > > >> index 3e9444813a..e6dd004587 100644 > > >> --- a/hw/intc/xics_spapr.c > > >> +++ b/hw/intc/xics_spapr.c > > >> @@ -283,8 +283,18 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, > > >> rtas_st(rets, 0, RTAS_OUT_SUCCESS); > > >> } > > >> > > >> -void xics_spapr_init(SpaprMachineState *spapr) > > >> +static void ics_spapr_realize(DeviceState *dev, Error **errp) > > >> { > > >> + ICSState *ics = ICS_SPAPR(dev); > > >> + ICSStateClass *icsc = ICS_GET_CLASS(ics); > > >> + Error *local_err = NULL; > > >> + > > >> + icsc->parent_realize(dev, &local_err); > > >> + if (local_err) { > > >> + error_propagate(errp, local_err); > > >> + return; > > >> + } > > >> + > > >> spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive); > > >> spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive); > > >> spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off); > > >> @@ -319,3 +329,25 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, > > >> _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); > > >> _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); > > >> } > > >> + > > >> +static void ics_spapr_class_init(ObjectClass *klass, void *data) > > >> +{ > > >> + DeviceClass *dc = DEVICE_CLASS(klass); > > >> + ICSStateClass *isc = ICS_CLASS(klass); > > >> + > > >> + device_class_set_parent_realize(dc, ics_spapr_realize, > > >> + &isc->parent_realize); > > >> +} > > >> + > > >> +static const TypeInfo ics_spapr_info = { > > >> + .name = TYPE_ICS_SPAPR, > > >> + .parent = TYPE_ICS, > > >> + .class_init = ics_spapr_class_init, > > >> +}; > > >> + > > >> +static void xics_spapr_register_types(void) > > >> +{ > > >> + type_register_static(&ics_spapr_info); > > >> +} > > >> + > > >> +type_init(xics_spapr_register_types) > > >> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > > >> index 6c45d2a3c0..8c26fa2d1e 100644 > > >> --- a/hw/ppc/spapr_irq.c > > >> +++ b/hw/ppc/spapr_irq.c > > >> @@ -98,7 +98,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, > > >> Object *obj; > > >> Error *local_err = NULL; > > >> > > >> - obj = object_new(TYPE_ICS); > > >> + obj = object_new(TYPE_ICS_SPAPR); > > >> object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); > > >> object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), > > >> &error_fatal); > > >> @@ -109,9 +109,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, > > >> return; > > >> } > > >> > > >> - spapr->ics = ICS(obj); > > >> - > > >> - xics_spapr_init(spapr); > > >> + spapr->ics = ICS_SPAPR(obj); > > >> } > > >> > > >> static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, > > >> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h > > >> index 5dabc9a138..691a6d00f7 100644 > > >> --- a/include/hw/ppc/xics_spapr.h > > >> +++ b/include/hw/ppc/xics_spapr.h > > >> @@ -31,11 +31,13 @@ > > >> > > >> #define XICS_NODENAME "interrupt-controller" > > >> > > >> +#define TYPE_ICS_SPAPR "ics-spapr" > > >> +#define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR) > > >> + > > >> void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, > > >> uint32_t phandle); > > >> int xics_kvm_connect(SpaprMachineState *spapr, Error **errp); > > >> void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp); > > >> bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr); > > >> -void xics_spapr_init(SpaprMachineState *spapr); > > >> > > >> #endif /* XICS_SPAPR_H */ > > > > > >
On Fri, Sep 27, 2019 at 06:05:56PM +0200, Greg Kurz wrote: > On Thu, 26 Sep 2019 10:56:46 +1000 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > On Wed, Sep 25, 2019 at 10:55:35AM +0200, Cédric Le Goater wrote: > > > On 25/09/2019 10:40, Greg Kurz wrote: > > > > On Wed, 25 Sep 2019 16:45:20 +1000 > > > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > > > > > >> We create a subtype of TYPE_ICS specifically for sPAPR. For now all this > > > >> does is move the setup of the PAPR specific hcalls and RTAS calls to > > > >> the realize() function for this, rather than requiring the PAPR code to > > > >> explicitly call xics_spapr_init(). In future it will have some more > > > >> function. > > > >> > > > >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > > >> --- > > > > > > > > LGTM, but for extra safety I would also introduce a SpaprIcsState typedef > > > > > > why ? we have ICS_SPAPR() for the checks already. > > > > Eh.. using typedefs when we haven't actually extended a base type > > isn't common QOM practice. Yes, it's not as typesafe as it could be, > > but I'm not really inclined to go to the extra effort here. > > I'll soon need to extend the base type with a nr_servers field, Uh.. nr_servers doesn't seem like it belongs in the base ICS type. That really would conflict with the pnv usage where the ICS is supposed to just represent the ICS, not the xics as a whole. If you need nr_servers information here, it seems like pulling it via a method in XICSFabric would make more sense. > and while here with an fd field as well in order to get rid of > the ugly global in xics.c. I'll go to the extra effort :) That could go in the derived type. We already kind of conflate ICS and XICS-as-a-whole for the PAPR subtype, and the KVM xics is PAPR only anyway.
On Mon, 30 Sep 2019 18:45:30 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > On Fri, Sep 27, 2019 at 06:05:56PM +0200, Greg Kurz wrote: > > On Thu, 26 Sep 2019 10:56:46 +1000 > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > > > On Wed, Sep 25, 2019 at 10:55:35AM +0200, Cédric Le Goater wrote: > > > > On 25/09/2019 10:40, Greg Kurz wrote: > > > > > On Wed, 25 Sep 2019 16:45:20 +1000 > > > > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > > > > > > > >> We create a subtype of TYPE_ICS specifically for sPAPR. For now all this > > > > >> does is move the setup of the PAPR specific hcalls and RTAS calls to > > > > >> the realize() function for this, rather than requiring the PAPR code to > > > > >> explicitly call xics_spapr_init(). In future it will have some more > > > > >> function. > > > > >> > > > > >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > > > >> --- > > > > > > > > > > LGTM, but for extra safety I would also introduce a SpaprIcsState typedef > > > > > > > > why ? we have ICS_SPAPR() for the checks already. > > > > > > Eh.. using typedefs when we haven't actually extended a base type > > > isn't common QOM practice. Yes, it's not as typesafe as it could be, > > > but I'm not really inclined to go to the extra effort here. > > > > I'll soon need to extend the base type with a nr_servers field, > > Uh.. nr_servers doesn't seem like it belongs in the base ICS type. Of course ! I re-used the wording "extended a base type" of your sentence, that I understand as "a subtype extends a base type with some more data". I'm talking about the sPAPR ICS subtype here, not the base ICS type. > That really would conflict with the pnv usage where the ICS is > supposed to just represent the ICS, not the xics as a whole. If you > need nr_servers information here, it seems like pulling it via a > method in XICSFabric would make more sense. > > > and while here with an fd field as well in order to get rid of > > the ugly global in xics.c. I'll go to the extra effort :) > > That could go in the derived type. We already kind of conflate ICS > and XICS-as-a-whole for the PAPR subtype, and the KVM xics is PAPR > only anyway. > Exactly, so that's why I was thinking about adding nr_servers there, but it could go to XICSFabric as well I guess.
On Mon, Sep 30, 2019 at 07:00:43PM +0200, Greg Kurz wrote: > On Mon, 30 Sep 2019 18:45:30 +1000 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > On Fri, Sep 27, 2019 at 06:05:56PM +0200, Greg Kurz wrote: > > > On Thu, 26 Sep 2019 10:56:46 +1000 > > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > > > > > On Wed, Sep 25, 2019 at 10:55:35AM +0200, Cédric Le Goater wrote: > > > > > On 25/09/2019 10:40, Greg Kurz wrote: > > > > > > On Wed, 25 Sep 2019 16:45:20 +1000 > > > > > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > > > > > > > > > >> We create a subtype of TYPE_ICS specifically for sPAPR. For now all this > > > > > >> does is move the setup of the PAPR specific hcalls and RTAS calls to > > > > > >> the realize() function for this, rather than requiring the PAPR code to > > > > > >> explicitly call xics_spapr_init(). In future it will have some more > > > > > >> function. > > > > > >> > > > > > >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > > > > >> --- > > > > > > > > > > > > LGTM, but for extra safety I would also introduce a SpaprIcsState typedef > > > > > > > > > > why ? we have ICS_SPAPR() for the checks already. > > > > > > > > Eh.. using typedefs when we haven't actually extended a base type > > > > isn't common QOM practice. Yes, it's not as typesafe as it could be, > > > > but I'm not really inclined to go to the extra effort here. > > > > > > I'll soon need to extend the base type with a nr_servers field, > > > > Uh.. nr_servers doesn't seem like it belongs in the base ICS type. > > Of course ! I re-used the wording "extended a base type" of your sentence, > that I understand as "a subtype extends a base type with some more data". > I'm talking about the sPAPR ICS subtype here, not the base ICS type. Ah, ok. > > That really would conflict with the pnv usage where the ICS is > > supposed to just represent the ICS, not the xics as a whole. If you > > need nr_servers information here, it seems like pulling it via a > > method in XICSFabric would make more sense. > > > > > and while here with an fd field as well in order to get rid of > > > the ugly global in xics.c. I'll go to the extra effort :) > > > > That could go in the derived type. We already kind of conflate ICS > > and XICS-as-a-whole for the PAPR subtype, and the KVM xics is PAPR > > only anyway. > > > > Exactly, so that's why I was thinking about adding nr_servers there, > but it could go to XICSFabric as well I guess.
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 3e9444813a..e6dd004587 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -283,8 +283,18 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, rtas_st(rets, 0, RTAS_OUT_SUCCESS); } -void xics_spapr_init(SpaprMachineState *spapr) +static void ics_spapr_realize(DeviceState *dev, Error **errp) { + ICSState *ics = ICS_SPAPR(dev); + ICSStateClass *icsc = ICS_GET_CLASS(ics); + Error *local_err = NULL; + + icsc->parent_realize(dev, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive); spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive); spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off); @@ -319,3 +329,25 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); } + +static void ics_spapr_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + ICSStateClass *isc = ICS_CLASS(klass); + + device_class_set_parent_realize(dc, ics_spapr_realize, + &isc->parent_realize); +} + +static const TypeInfo ics_spapr_info = { + .name = TYPE_ICS_SPAPR, + .parent = TYPE_ICS, + .class_init = ics_spapr_class_init, +}; + +static void xics_spapr_register_types(void) +{ + type_register_static(&ics_spapr_info); +} + +type_init(xics_spapr_register_types) diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 6c45d2a3c0..8c26fa2d1e 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -98,7 +98,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, Object *obj; Error *local_err = NULL; - obj = object_new(TYPE_ICS); + obj = object_new(TYPE_ICS_SPAPR); object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), &error_fatal); @@ -109,9 +109,7 @@ static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, return; } - spapr->ics = ICS(obj); - - xics_spapr_init(spapr); + spapr->ics = ICS_SPAPR(obj); } static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h index 5dabc9a138..691a6d00f7 100644 --- a/include/hw/ppc/xics_spapr.h +++ b/include/hw/ppc/xics_spapr.h @@ -31,11 +31,13 @@ #define XICS_NODENAME "interrupt-controller" +#define TYPE_ICS_SPAPR "ics-spapr" +#define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR) + void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, uint32_t phandle); int xics_kvm_connect(SpaprMachineState *spapr, Error **errp); void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp); bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr); -void xics_spapr_init(SpaprMachineState *spapr); #endif /* XICS_SPAPR_H */
We create a subtype of TYPE_ICS specifically for sPAPR. For now all this does is move the setup of the PAPR specific hcalls and RTAS calls to the realize() function for this, rather than requiring the PAPR code to explicitly call xics_spapr_init(). In future it will have some more function. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/intc/xics_spapr.c | 34 +++++++++++++++++++++++++++++++++- hw/ppc/spapr_irq.c | 6 ++---- include/hw/ppc/xics_spapr.h | 4 +++- 3 files changed, 38 insertions(+), 6 deletions(-)