| Message ID | 20171123132955.1261-2-clg@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 23, 2017 at 02:29:31PM +0100, Cédric Le Goater wrote: > The sPAPR and the PowerNV core objects create the interrupt presenter > object of the CPUs in a very similar way. Let's provide a common > routine in which we use the presenter 'type' as a child identifier. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> One tiny nit.., apart from that Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > hw/intc/xics.c | 22 ++++++++++++++++++++++ > hw/ppc/pnv_core.c | 10 +--------- > hw/ppc/spapr_cpu_core.c | 13 ++----------- > include/hw/ppc/xics.h | 3 +++ > 4 files changed, 28 insertions(+), 20 deletions(-) > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index a1cc0e420c98..e4ccdff8f577 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -384,6 +384,28 @@ static const TypeInfo icp_info = { > .class_size = sizeof(ICPStateClass), > }; > > +Object *icp_create(CPUState *cs, const char *type, XICSFabric *xi, Error **errp) > +{ > + Object *child = OBJECT(cs); In the original context 'child' made sense, since it was the child object of the core. Here, it's misleading, since it's the parent of the xics link. It's only used in a couple of places, so I suggest you just opencode OBJECT(cs) in each place. > + Error *local_err = NULL; > + Object *obj; > + > + obj = object_new(type); > + object_property_add_child(child, type, obj, &error_abort); > + object_unref(obj); > + object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(xi), > + &error_abort); > + object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); > + object_property_set_bool(obj, true, "realized", &local_err); > + if (local_err) { > + object_unparent(obj); > + error_propagate(errp, local_err); > + obj = NULL; > + } > + > + return obj; > +} > + > /* > * ICS: Source layer > */ > diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c > index 82ff440b3334..a066736846f8 100644 > --- a/hw/ppc/pnv_core.c > +++ b/hw/ppc/pnv_core.c > @@ -126,7 +126,6 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) > Error *local_err = NULL; > CPUState *cs = CPU(child); > PowerPCCPU *cpu = POWERPC_CPU(cs); > - Object *obj; > > object_property_set_bool(child, true, "realized", &local_err); > if (local_err) { > @@ -134,13 +133,7 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) > return; > } > > - obj = object_new(TYPE_PNV_ICP); > - object_property_add_child(child, "icp", obj, NULL); > - object_unref(obj); > - object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(xi), > - &error_abort); > - object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); > - object_property_set_bool(obj, true, "realized", &local_err); > + icp_create(cs, TYPE_PNV_ICP, xi, &local_err); > if (local_err) { > error_propagate(errp, local_err); > return; > @@ -148,7 +141,6 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) > > powernv_cpu_init(cpu, &local_err); > if (local_err) { > - object_unparent(obj); > error_propagate(errp, local_err); > return; > } > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 4ba8563d49e4..f8a520a2fa2d 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -111,7 +111,6 @@ static void spapr_cpu_core_realize_child(Object *child, > Error *local_err = NULL; > CPUState *cs = CPU(child); > PowerPCCPU *cpu = POWERPC_CPU(cs); > - Object *obj; > > object_property_set_bool(child, true, "realized", &local_err); > if (local_err) { > @@ -123,21 +122,13 @@ static void spapr_cpu_core_realize_child(Object *child, > goto error; > } > > - obj = object_new(spapr->icp_type); > - object_property_add_child(child, "icp", obj, &error_abort); > - object_unref(obj); > - object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(spapr), > - &error_abort); > - object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); > - object_property_set_bool(obj, true, "realized", &local_err); > + icp_create(cs, spapr->icp_type, XICS_FABRIC(spapr), &local_err); > if (local_err) { > - goto free_icp; > + goto error; > } > > return; > > -free_icp: > - object_unparent(obj); > error: > error_propagate(errp, local_err); > } > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index 2df99be111ce..126b47dec38b 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -212,4 +212,7 @@ typedef struct sPAPRMachineState sPAPRMachineState; > int xics_kvm_init(sPAPRMachineState *spapr, Error **errp); > void xics_spapr_init(sPAPRMachineState *spapr); > > +Object *icp_create(CPUState *cs, const char *type, XICSFabric *xi, > + Error **errp); > + > #endif /* XICS_H */
On 11/24/2017 03:51 AM, David Gibson wrote: > On Thu, Nov 23, 2017 at 02:29:31PM +0100, Cédric Le Goater wrote: >> The sPAPR and the PowerNV core objects create the interrupt presenter >> object of the CPUs in a very similar way. Let's provide a common >> routine in which we use the presenter 'type' as a child identifier. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > > One tiny nit.., apart from that > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > >> --- >> hw/intc/xics.c | 22 ++++++++++++++++++++++ >> hw/ppc/pnv_core.c | 10 +--------- >> hw/ppc/spapr_cpu_core.c | 13 ++----------- >> include/hw/ppc/xics.h | 3 +++ >> 4 files changed, 28 insertions(+), 20 deletions(-) >> >> diff --git a/hw/intc/xics.c b/hw/intc/xics.c >> index a1cc0e420c98..e4ccdff8f577 100644 >> --- a/hw/intc/xics.c >> +++ b/hw/intc/xics.c >> @@ -384,6 +384,28 @@ static const TypeInfo icp_info = { >> .class_size = sizeof(ICPStateClass), >> }; >> >> +Object *icp_create(CPUState *cs, const char *type, XICSFabric *xi, Error **errp) >> +{ >> + Object *child = OBJECT(cs); > > In the original context 'child' made sense, since it was the child > object of the core. Here, it's misleading, since it's the parent of > the xics link. It's only used in a couple of places, so I suggest you > just opencode OBJECT(cs) in each place. yes. That was a left over from the copy-paste. Thanks, C. >> + Error *local_err = NULL; >> + Object *obj; >> + >> + obj = object_new(type); >> + object_property_add_child(child, type, obj, &error_abort); >> + object_unref(obj); >> + object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(xi), >> + &error_abort); >> + object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); >> + object_property_set_bool(obj, true, "realized", &local_err); >> + if (local_err) { >> + object_unparent(obj); >> + error_propagate(errp, local_err); >> + obj = NULL; >> + } >> + >> + return obj; >> +} >> + >> /* >> * ICS: Source layer >> */ >> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c >> index 82ff440b3334..a066736846f8 100644 >> --- a/hw/ppc/pnv_core.c >> +++ b/hw/ppc/pnv_core.c >> @@ -126,7 +126,6 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) >> Error *local_err = NULL; >> CPUState *cs = CPU(child); >> PowerPCCPU *cpu = POWERPC_CPU(cs); >> - Object *obj; >> >> object_property_set_bool(child, true, "realized", &local_err); >> if (local_err) { >> @@ -134,13 +133,7 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) >> return; >> } >> >> - obj = object_new(TYPE_PNV_ICP); >> - object_property_add_child(child, "icp", obj, NULL); >> - object_unref(obj); >> - object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(xi), >> - &error_abort); >> - object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); >> - object_property_set_bool(obj, true, "realized", &local_err); >> + icp_create(cs, TYPE_PNV_ICP, xi, &local_err); >> if (local_err) { >> error_propagate(errp, local_err); >> return; >> @@ -148,7 +141,6 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) >> >> powernv_cpu_init(cpu, &local_err); >> if (local_err) { >> - object_unparent(obj); >> error_propagate(errp, local_err); >> return; >> } >> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c >> index 4ba8563d49e4..f8a520a2fa2d 100644 >> --- a/hw/ppc/spapr_cpu_core.c >> +++ b/hw/ppc/spapr_cpu_core.c >> @@ -111,7 +111,6 @@ static void spapr_cpu_core_realize_child(Object *child, >> Error *local_err = NULL; >> CPUState *cs = CPU(child); >> PowerPCCPU *cpu = POWERPC_CPU(cs); >> - Object *obj; >> >> object_property_set_bool(child, true, "realized", &local_err); >> if (local_err) { >> @@ -123,21 +122,13 @@ static void spapr_cpu_core_realize_child(Object *child, >> goto error; >> } >> >> - obj = object_new(spapr->icp_type); >> - object_property_add_child(child, "icp", obj, &error_abort); >> - object_unref(obj); >> - object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(spapr), >> - &error_abort); >> - object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); >> - object_property_set_bool(obj, true, "realized", &local_err); >> + icp_create(cs, spapr->icp_type, XICS_FABRIC(spapr), &local_err); >> if (local_err) { >> - goto free_icp; >> + goto error; >> } >> >> return; >> >> -free_icp: >> - object_unparent(obj); >> error: >> error_propagate(errp, local_err); >> } >> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h >> index 2df99be111ce..126b47dec38b 100644 >> --- a/include/hw/ppc/xics.h >> +++ b/include/hw/ppc/xics.h >> @@ -212,4 +212,7 @@ typedef struct sPAPRMachineState sPAPRMachineState; >> int xics_kvm_init(sPAPRMachineState *spapr, Error **errp); >> void xics_spapr_init(sPAPRMachineState *spapr); >> >> +Object *icp_create(CPUState *cs, const char *type, XICSFabric *xi, >> + Error **errp); >> + >> #endif /* XICS_H */ >
On Thu, 23 Nov 2017 14:29:31 +0100 Cédric Le Goater <clg@kaod.org> wrote: > The sPAPR and the PowerNV core objects create the interrupt presenter > object of the CPUs in a very similar way. Let's provide a common > routine in which we use the presenter 'type' as a child identifier. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- Reviewed-by: Greg Kurz <groug@kaod.org> > hw/intc/xics.c | 22 ++++++++++++++++++++++ > hw/ppc/pnv_core.c | 10 +--------- > hw/ppc/spapr_cpu_core.c | 13 ++----------- > include/hw/ppc/xics.h | 3 +++ > 4 files changed, 28 insertions(+), 20 deletions(-) > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index a1cc0e420c98..e4ccdff8f577 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -384,6 +384,28 @@ static const TypeInfo icp_info = { > .class_size = sizeof(ICPStateClass), > }; > > +Object *icp_create(CPUState *cs, const char *type, XICSFabric *xi, Error **errp) > +{ > + Object *child = OBJECT(cs); > + Error *local_err = NULL; > + Object *obj; > + > + obj = object_new(type); > + object_property_add_child(child, type, obj, &error_abort); > + object_unref(obj); > + object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(xi), > + &error_abort); > + object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); > + object_property_set_bool(obj, true, "realized", &local_err); > + if (local_err) { > + object_unparent(obj); > + error_propagate(errp, local_err); > + obj = NULL; > + } > + > + return obj; > +} > + > /* > * ICS: Source layer > */ > diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c > index 82ff440b3334..a066736846f8 100644 > --- a/hw/ppc/pnv_core.c > +++ b/hw/ppc/pnv_core.c > @@ -126,7 +126,6 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) > Error *local_err = NULL; > CPUState *cs = CPU(child); > PowerPCCPU *cpu = POWERPC_CPU(cs); > - Object *obj; > > object_property_set_bool(child, true, "realized", &local_err); > if (local_err) { > @@ -134,13 +133,7 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) > return; > } > > - obj = object_new(TYPE_PNV_ICP); > - object_property_add_child(child, "icp", obj, NULL); > - object_unref(obj); > - object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(xi), > - &error_abort); > - object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); > - object_property_set_bool(obj, true, "realized", &local_err); > + icp_create(cs, TYPE_PNV_ICP, xi, &local_err); > if (local_err) { > error_propagate(errp, local_err); > return; > @@ -148,7 +141,6 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) > > powernv_cpu_init(cpu, &local_err); > if (local_err) { > - object_unparent(obj); > error_propagate(errp, local_err); > return; > } > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 4ba8563d49e4..f8a520a2fa2d 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -111,7 +111,6 @@ static void spapr_cpu_core_realize_child(Object *child, > Error *local_err = NULL; > CPUState *cs = CPU(child); > PowerPCCPU *cpu = POWERPC_CPU(cs); > - Object *obj; > > object_property_set_bool(child, true, "realized", &local_err); > if (local_err) { > @@ -123,21 +122,13 @@ static void spapr_cpu_core_realize_child(Object *child, > goto error; > } > > - obj = object_new(spapr->icp_type); > - object_property_add_child(child, "icp", obj, &error_abort); > - object_unref(obj); > - object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(spapr), > - &error_abort); > - object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); > - object_property_set_bool(obj, true, "realized", &local_err); > + icp_create(cs, spapr->icp_type, XICS_FABRIC(spapr), &local_err); > if (local_err) { > - goto free_icp; > + goto error; > } > > return; > > -free_icp: > - object_unparent(obj); > error: > error_propagate(errp, local_err); > } > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index 2df99be111ce..126b47dec38b 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -212,4 +212,7 @@ typedef struct sPAPRMachineState sPAPRMachineState; > int xics_kvm_init(sPAPRMachineState *spapr, Error **errp); > void xics_spapr_init(sPAPRMachineState *spapr); > > +Object *icp_create(CPUState *cs, const char *type, XICSFabric *xi, > + Error **errp); > + > #endif /* XICS_H */
On Fri, 24 Nov 2017 13:51:00 +1100 David Gibson <david@gibson.dropbear.id.au> wrote: > On Thu, Nov 23, 2017 at 02:29:31PM +0100, Cédric Le Goater wrote: > > The sPAPR and the PowerNV core objects create the interrupt presenter > > object of the CPUs in a very similar way. Let's provide a common > > routine in which we use the presenter 'type' as a child identifier. > > > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > > One tiny nit.., apart from that > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > > --- > > hw/intc/xics.c | 22 ++++++++++++++++++++++ > > hw/ppc/pnv_core.c | 10 +--------- > > hw/ppc/spapr_cpu_core.c | 13 ++----------- > > include/hw/ppc/xics.h | 3 +++ > > 4 files changed, 28 insertions(+), 20 deletions(-) > > > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > > index a1cc0e420c98..e4ccdff8f577 100644 > > --- a/hw/intc/xics.c > > +++ b/hw/intc/xics.c > > @@ -384,6 +384,28 @@ static const TypeInfo icp_info = { > > .class_size = sizeof(ICPStateClass), > > }; > > > > +Object *icp_create(CPUState *cs, const char *type, XICSFabric *xi, Error **errp) > > +{ > > + Object *child = OBJECT(cs); > > In the original context 'child' made sense, since it was the child > object of the core. Here, it's misleading, since it's the parent of > the xics link. It's only used in a couple of places, so I suggest you Oops yes :) > just opencode OBJECT(cs) in each place. > or rename child to owner, as it is done with DRCs and TCE tables. > > + Error *local_err = NULL; > > + Object *obj; > > + > > + obj = object_new(type); > > + object_property_add_child(child, type, obj, &error_abort); > > + object_unref(obj); > > + object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(xi), > > + &error_abort); > > + object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); > > + object_property_set_bool(obj, true, "realized", &local_err); > > + if (local_err) { > > + object_unparent(obj); > > + error_propagate(errp, local_err); > > + obj = NULL; > > + } > > + > > + return obj; > > +} > > + > > /* > > * ICS: Source layer > > */ > > diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c > > index 82ff440b3334..a066736846f8 100644 > > --- a/hw/ppc/pnv_core.c > > +++ b/hw/ppc/pnv_core.c > > @@ -126,7 +126,6 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) > > Error *local_err = NULL; > > CPUState *cs = CPU(child); > > PowerPCCPU *cpu = POWERPC_CPU(cs); > > - Object *obj; > > > > object_property_set_bool(child, true, "realized", &local_err); > > if (local_err) { > > @@ -134,13 +133,7 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) > > return; > > } > > > > - obj = object_new(TYPE_PNV_ICP); > > - object_property_add_child(child, "icp", obj, NULL); > > - object_unref(obj); > > - object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(xi), > > - &error_abort); > > - object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); > > - object_property_set_bool(obj, true, "realized", &local_err); > > + icp_create(cs, TYPE_PNV_ICP, xi, &local_err); > > if (local_err) { > > error_propagate(errp, local_err); > > return; > > @@ -148,7 +141,6 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) > > > > powernv_cpu_init(cpu, &local_err); > > if (local_err) { > > - object_unparent(obj); > > error_propagate(errp, local_err); > > return; > > } > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > index 4ba8563d49e4..f8a520a2fa2d 100644 > > --- a/hw/ppc/spapr_cpu_core.c > > +++ b/hw/ppc/spapr_cpu_core.c > > @@ -111,7 +111,6 @@ static void spapr_cpu_core_realize_child(Object *child, > > Error *local_err = NULL; > > CPUState *cs = CPU(child); > > PowerPCCPU *cpu = POWERPC_CPU(cs); > > - Object *obj; > > > > object_property_set_bool(child, true, "realized", &local_err); > > if (local_err) { > > @@ -123,21 +122,13 @@ static void spapr_cpu_core_realize_child(Object *child, > > goto error; > > } > > > > - obj = object_new(spapr->icp_type); > > - object_property_add_child(child, "icp", obj, &error_abort); > > - object_unref(obj); > > - object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(spapr), > > - &error_abort); > > - object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); > > - object_property_set_bool(obj, true, "realized", &local_err); > > + icp_create(cs, spapr->icp_type, XICS_FABRIC(spapr), &local_err); > > if (local_err) { > > - goto free_icp; > > + goto error; > > } > > > > return; > > > > -free_icp: > > - object_unparent(obj); > > error: > > error_propagate(errp, local_err); > > } > > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > > index 2df99be111ce..126b47dec38b 100644 > > --- a/include/hw/ppc/xics.h > > +++ b/include/hw/ppc/xics.h > > @@ -212,4 +212,7 @@ typedef struct sPAPRMachineState sPAPRMachineState; > > int xics_kvm_init(sPAPRMachineState *spapr, Error **errp); > > void xics_spapr_init(sPAPRMachineState *spapr); > > > > +Object *icp_create(CPUState *cs, const char *type, XICSFabric *xi, > > + Error **errp); > > + > > #endif /* XICS_H */ >
On Fri, Nov 24, 2017 at 10:55:47AM +0100, Greg Kurz wrote: > On Fri, 24 Nov 2017 13:51:00 +1100 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > On Thu, Nov 23, 2017 at 02:29:31PM +0100, Cédric Le Goater wrote: > > > The sPAPR and the PowerNV core objects create the interrupt presenter > > > object of the CPUs in a very similar way. Let's provide a common > > > routine in which we use the presenter 'type' as a child identifier. > > > > > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > > > > One tiny nit.., apart from that > > > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > > > > --- > > > hw/intc/xics.c | 22 ++++++++++++++++++++++ > > > hw/ppc/pnv_core.c | 10 +--------- > > > hw/ppc/spapr_cpu_core.c | 13 ++----------- > > > include/hw/ppc/xics.h | 3 +++ > > > 4 files changed, 28 insertions(+), 20 deletions(-) > > > > > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > > > index a1cc0e420c98..e4ccdff8f577 100644 > > > --- a/hw/intc/xics.c > > > +++ b/hw/intc/xics.c > > > @@ -384,6 +384,28 @@ static const TypeInfo icp_info = { > > > .class_size = sizeof(ICPStateClass), > > > }; > > > > > > +Object *icp_create(CPUState *cs, const char *type, XICSFabric *xi, Error **errp) > > > +{ > > > + Object *child = OBJECT(cs); > > > > In the original context 'child' made sense, since it was the child > > object of the core. Here, it's misleading, since it's the parent of > > the xics link. It's only used in a couple of places, so I suggest you > > Oops yes :) > > > just opencode OBJECT(cs) in each place. > > > > or rename child to owner, as it is done with DRCs and TCE tables. Sure. Either's fine by me.
diff --git a/hw/intc/xics.c b/hw/intc/xics.c index a1cc0e420c98..e4ccdff8f577 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -384,6 +384,28 @@ static const TypeInfo icp_info = { .class_size = sizeof(ICPStateClass), }; +Object *icp_create(CPUState *cs, const char *type, XICSFabric *xi, Error **errp) +{ + Object *child = OBJECT(cs); + Error *local_err = NULL; + Object *obj; + + obj = object_new(type); + object_property_add_child(child, type, obj, &error_abort); + object_unref(obj); + object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(xi), + &error_abort); + object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); + object_property_set_bool(obj, true, "realized", &local_err); + if (local_err) { + object_unparent(obj); + error_propagate(errp, local_err); + obj = NULL; + } + + return obj; +} + /* * ICS: Source layer */ diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c index 82ff440b3334..a066736846f8 100644 --- a/hw/ppc/pnv_core.c +++ b/hw/ppc/pnv_core.c @@ -126,7 +126,6 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) Error *local_err = NULL; CPUState *cs = CPU(child); PowerPCCPU *cpu = POWERPC_CPU(cs); - Object *obj; object_property_set_bool(child, true, "realized", &local_err); if (local_err) { @@ -134,13 +133,7 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) return; } - obj = object_new(TYPE_PNV_ICP); - object_property_add_child(child, "icp", obj, NULL); - object_unref(obj); - object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(xi), - &error_abort); - object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); - object_property_set_bool(obj, true, "realized", &local_err); + icp_create(cs, TYPE_PNV_ICP, xi, &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -148,7 +141,6 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp) powernv_cpu_init(cpu, &local_err); if (local_err) { - object_unparent(obj); error_propagate(errp, local_err); return; } diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 4ba8563d49e4..f8a520a2fa2d 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -111,7 +111,6 @@ static void spapr_cpu_core_realize_child(Object *child, Error *local_err = NULL; CPUState *cs = CPU(child); PowerPCCPU *cpu = POWERPC_CPU(cs); - Object *obj; object_property_set_bool(child, true, "realized", &local_err); if (local_err) { @@ -123,21 +122,13 @@ static void spapr_cpu_core_realize_child(Object *child, goto error; } - obj = object_new(spapr->icp_type); - object_property_add_child(child, "icp", obj, &error_abort); - object_unref(obj); - object_property_add_const_link(obj, ICP_PROP_XICS, OBJECT(spapr), - &error_abort); - object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort); - object_property_set_bool(obj, true, "realized", &local_err); + icp_create(cs, spapr->icp_type, XICS_FABRIC(spapr), &local_err); if (local_err) { - goto free_icp; + goto error; } return; -free_icp: - object_unparent(obj); error: error_propagate(errp, local_err); } diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h index 2df99be111ce..126b47dec38b 100644 --- a/include/hw/ppc/xics.h +++ b/include/hw/ppc/xics.h @@ -212,4 +212,7 @@ typedef struct sPAPRMachineState sPAPRMachineState; int xics_kvm_init(sPAPRMachineState *spapr, Error **errp); void xics_spapr_init(sPAPRMachineState *spapr); +Object *icp_create(CPUState *cs, const char *type, XICSFabric *xi, + Error **errp); + #endif /* XICS_H */
The sPAPR and the PowerNV core objects create the interrupt presenter object of the CPUs in a very similar way. Let's provide a common routine in which we use the presenter 'type' as a child identifier. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/intc/xics.c | 22 ++++++++++++++++++++++ hw/ppc/pnv_core.c | 10 +--------- hw/ppc/spapr_cpu_core.c | 13 ++----------- include/hw/ppc/xics.h | 3 +++ 4 files changed, 28 insertions(+), 20 deletions(-)