On 06/08/2017 07:00 PM, Greg Kurz wrote: > On Thu, 8 Jun 2017 18:08:44 +0200 > Cédric Le Goater <c...@kaod.org> wrote: > >>>>> FWIW, other people do that as well (see hw/i386/pc_q35.c for example). >>>> >>>> well, I don't see the benefits of changing a string constant by a >>>> define. >>>> >>> >>> Improved semantics, especially since the "xics" string appears in >>> many places with different meanings. >> >> ah ? If so, we should do a cleanup up. The code seems consistent from >> what I can see. xics is a general name for : >> >> 'PowerPC interrupt controller (type 2)' >> >> and it is mostly used as a prefix. There are no "xics" object, only a > > I'm only talking about "xics" as a property name actually: > > $ git grep '"xics"' > hw/intc/xics.c: obj = object_property_get_link(OBJECT(dev), "xics", &err); > hw/intc/xics.c: obj = object_property_get_link(OBJECT(dev), "xics", &err); > hw/ppc/pnv.c: object_property_add_const_link(OBJECT(&chip->psi), "xics", > hw/ppc/pnv.c: object_property_add_const_link(OBJECT(pnv_core), "xics", > hw/ppc/pnv_core.c: object_property_add_const_link(obj, "xics", OBJECT(xi), > &error_abort); > hw/ppc/pnv_core.c: xi = object_property_get_link(OBJECT(dev), "xics", > &local_err); > hw/ppc/pnv_psi.c: obj = object_property_get_link(OBJECT(dev), "xics", > &err); > hw/ppc/pnv_psi.c: object_property_add_const_link(OBJECT(ics), "xics", obj, > &error_abort); > hw/ppc/spapr.c: object_property_add_const_link(obj, "xics", OBJECT(spapr), > &error_abort); > hw/ppc/spapr_cpu_core.c: object_property_add_const_link(obj, "xics", > OBJECT(spapr), &error_abort); > > You have to read the code to know which ones are related.
The "xics" property link always point to the same object : the XICSFabric object which is the machine, spapr or pnv. > With this patch applied, it is mostly obvious, even for the newbie: ah. the goal is to know where in the code the link was set. It can be even more complex with aliases. Cheers, C. > > $ git grep -E 'IC._PROP_XICS|"xics"' > hw/intc/xics.c: obj = object_property_get_link(OBJECT(dev), ICP_PROP_XICS, > &err); > hw/intc/xics.c: error_setg(errp, "%s: required link '" ICP_PROP_XICS > "' not found: %s", > hw/intc/xics.c: obj = object_property_get_link(OBJECT(dev), ICS_PROP_XICS, > &err); > hw/intc/xics.c: error_setg(errp, "%s: required link '" ICS_PROP_XICS > "' not found: %s", > > hw/ppc/pnv.c: object_property_add_const_link(OBJECT(&chip->psi), "xics", > hw/ppc/pnv.c: object_property_add_const_link(OBJECT(pnv_core), "xics", > > these two ^^ are related to... > > hw/ppc/pnv_core.c: object_property_add_const_link(obj, ICP_PROP_XICS, > OBJECT(xi), > > hw/ppc/pnv_core.c: xi = object_property_get_link(OBJECT(dev), "xics", > &local_err); > hw/ppc/pnv_psi.c: obj = object_property_get_link(OBJECT(dev), "xics", > &err); > > these two ^^ > > hw/ppc/pnv_psi.c: object_property_add_const_link(OBJECT(ics), > ICS_PROP_XICS, obj, > hw/ppc/spapr.c: object_property_add_const_link(obj, ICS_PROP_XICS, > OBJECT(spapr), > hw/ppc/spapr_cpu_core.c: object_property_add_const_link(obj, > ICP_PROP_XICS, OBJECT(spapr), > include/hw/ppc/xics.h:#define ICP_PROP_XICS "xics" > include/hw/ppc/xics.h:#define ICS_PROP_XICS "xics" > >> XICSFabric which is a QOM interface of the machine with a bunch of >> handlers. It worked out pretty well for sPAPR and PowerNV. >> >> It won't be the case for XIVE, the interrupt controller (type 3) for >> the P9. It is more complex and there are quite a lot of internal >> states which will need to be gathered under an object. We can keep >> the ICPState fortunately but the sources are quite different. >> >> C. >> >