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. With this patch applied, it is mostly obvious, even for the newbie: $ 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. >
pgpiJ4vL6Czio.pgp
Description: OpenPGP digital signature
