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

[Julien Grall <julien.grall@linaro.org>]

Hi Jan,

On 31/10/2014 15:12, Jan Beulich wrote:
>>>> On 31.10.14 at 14:53, <julien.grall@linaro.org> wrote:
>> On 10/31/2014 01:37 PM, Jan Beulich wrote:
>>>>>> On 31.10.14 at 12:23, <julien.grall@linaro.org> 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.

IIRC, DOMCTL can be modify whenever we want.

I doubt that x86 will use it, so if we plan to add a new architecture 
which require this DOMCTL then we could rename the structure at that time.

-- 
Julien Grall