[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link properties |
Date: |
Fri, 9 Jun 2017 07:46:26 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 06/09/2017 04:10 AM, David Gibson wrote:
> On Thu, Jun 08, 2017 at 07:26:35PM +0200, Cédric Le Goater wrote:
>> On 06/08/2017 07:00 PM, Greg Kurz wrote:
>>> On Thu, 8 Jun 2017 18:08:44 +0200
>>> Cédric Le Goater <address@hidden> 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.
>
> There doesn't seem to be a strong convention about whether to use raw
> property names or defines across qemu. I'm not all that fussed either
> way.
>
> I do see one small advantage to use defines: if you make a typo, it
> will probably result in a compile time error, whereas with a bare
> string it won't show up until a runtime error.
ok. I can agree with that.
> In this case, I intend to take the macro patch, mostly just on the
> basis of avoiding further delays to rework the remaining patches.
But I don't think having two different defines is a good idea :
+#define ICP_PROP_XICS "xics"
+#define ICS_PROP_XICS "xics"
C.
- [Qemu-devel] [PATCH v4 0/6] spapr/xics: fix migration of older machine types, Greg Kurz, 2017/06/08
- [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link properties, Greg Kurz, 2017/06/08
- Re: [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link properties, Cédric Le Goater, 2017/06/08
- Re: [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link properties, Greg Kurz, 2017/06/08
- Re: [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link properties, Cédric Le Goater, 2017/06/08
- Re: [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link properties, Greg Kurz, 2017/06/08
- Re: [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link properties, Cédric Le Goater, 2017/06/08
- Re: [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link properties, Greg Kurz, 2017/06/08
- Re: [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link properties, Cédric Le Goater, 2017/06/08
- Re: [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link properties, David Gibson, 2017/06/08
- Re: [Qemu-devel] [PATCH v4 1/6] xics: introduce macros for ICP/ICS link properties,
Cédric Le Goater <=
[Qemu-devel] [PATCH v4 2/6] xics: pass appropriate types to realize() handlers., Greg Kurz, 2017/06/08
[Qemu-devel] [PATCH v4 3/6] xics: setup cpu at realize time, Greg Kurz, 2017/06/08
[Qemu-devel] [PATCH v4 4/6] xics: drop ICPStateClass::cpu_setup() handler, Greg Kurz, 2017/06/08
[Qemu-devel] [PATCH v4 5/6] xics: directly register ICPState objects to vmstate, Greg Kurz, 2017/06/08
[Qemu-devel] [PATCH v4 6/6] spapr: fix migration of ICPState objects from/to older QEMU, Greg Kurz, 2017/06/08