Re: [Xen-devel] [PATCH for Xen 4.5] xen/arm: Add support for GICv3 for domU

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.10.14 at 14:53, <julien.grall@xxxxxxxxxx> wrote:
> On 10/31/2014 01:37 PM, Jan Beulich wrote:
>>>>> On 31.10.14 at 12:23, <julien.grall@xxxxxxxxxx> wrote:
>>> On 31/10/2014 09:02, Jan Beulich wrote:
>>>>> --- a/xen/include/public/domctl.h
>>>>> +++ b/xen/include/public/domctl.h
>>>>> @@ -68,6 +68,19 @@ struct xen_domctl_createdomain {
>>>>>   typedef struct xen_domctl_createdomain xen_domctl_createdomain_t;
>>>>>   DEFINE_XEN_GUEST_HANDLE(xen_domctl_createdomain_t);
>>>>>
>>>>> +#if defined(__arm__) || defined(__aarch64__)
>>>>> +#define XEN_DOMCTL_CONFIG_GIC_DEFAULT   0
>>>>> +#define XEN_DOMCTL_CONFIG_GIC_V2        1
>>>>> +#define XEN_DOMCTL_CONFIG_GIC_V3        2
>>>>> +/* XEN_DOMCTL_configure_domain */
>>>>> +struct xen_domctl_configuredomain {
>>>>
>>>> The naming suggests that the #if really should be around just the
>>>> gic_version field (with a dummy field in the #else case to be C89
>>>> compatible, e.g. a zero width unnamed bitfield) and the
>>>> corresponding #define-s above, ...
>>>
>>> It's a bit like xen_domctl_setvcpuextstate which is defined only for x86 
>>> while the name seem pretty common.
>> 
>> That's a particularly bad example to compare with, as this is about
>> CPU registers having got added after the ABI was defined. This
>> (hopefully) shouldn't have many similar cases on other architectures.
>> Plus, doing things in an odd way just because there's an odd
>> precedent is always suspicious to me.
>> 
>>> I think we have to stay consistent in this header and not defining 
>>> DOMCTL which is not used for a specific architecture.
>> 
>> Yes, I always advocate for consistency - provided what is there is
>> a reasonable reference and was done properly.
> 
> Would renaming the structure name with "xen_arm_domctl_configuredomain"
> would be sufficient for you?

Maybe (better xen_domctl_arm_configure_domain then), if you are
reasonably certain this can't become useful for another arch.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel