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 09:02, Jan Beulich wrote:
>> +        domctl->u.configuredomain.gic_version = gic_version;
>> +
>> +        /* TODO: Make the copy generic for all ARCH domctl */
>> +        if ( __copy_to_guest(u_domctl, domctl, 1) )
>
> With just a single field needing copying, __copy_field_to_guest()
> would be quite a bit more efficient.

The configuredomain structure contains only a field and I plan to rework 
this code for Xen 4.6 to make this copy generic within the function (see 
the TODO).

Anyway, for this use case using __copy_field_to_guest is not more 
efficient for ARM.

>> --- 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.

I think we have to stay consistent in this header and not defining 
DOMCTL which is not used for a specific architecture.

Regards,

-- 
Julien Grall