Re: [v3 12/13] arm: Allow the user to specify the GIC version

[Julien Grall <julien.grall@citrix.com>]

On 01/07/15 14:52, Ian Jackson wrote:
> Julien Grall writes ("[v3 12/13] arm: Allow the user to specify the GIC version"):
>> A platform may have a GIC compatible with previous version of the
>> device.
> ...
>> +=item B<gic_version="vN">
>> +
>> +Version of the GIC emulated for the guest. Currently, the following
>> +versions are supported:
> 
> This documentation should say what happens if this is not specified.
> 
> If the default behaviour is not simply `v3' (which I think it isn't)
> then there should be a way to explicitly request it.  That is, there
> should not be a behaviour which is only obtainable by not setting the
> config option.  Instead, there should be a config option value to
> request whatever that behaviour is.  (At the xl layer.)

The enum for the gic_version contains 3 options: v2, v3 and default.
If the parameter is not specified, the default option is chosen.

I will document the default behavior.

> 
> AFIACT the default is "offer the guest the hardware's native version".

This is true when the domain is firstly created. But this will be
confusing if the user decide to migrate the guest to a platform where
the native GIC is different.

This is a valid use-case, though we don't support migration yet, and the
guest will expect to get the same virtual GIC (the IRQ controller driver
can't be changed).

> I think having called this XEN_DOMCTL_CONFIG_GIC_DEFAULT is rather
> odd.  I think "native" would be a better description.

I do agree that XEN_DOMCTL_CONFIG_GIC_DEFAULT is odd. This option should
be renamed to XEN_DOMCTL_CONFIG_GIC_NATIVE as this is the behavior
adopted by the hypervisor.

Although, I think we should not expose the option "native" to the user
but "default" (see why above). When the domain is created, the
hypervisor will update the field in to specify which version is
effectively used (see xen_arch_domain_create).

> It might be better to deal with this default-filling-in by changing
> the value in the domain config struct along with the other setdefault
> type things in libxl_create.c.  If you do that, then saying
> `libxl_domain_create_new', and then looking at the supplied config,
> will give you a config specifying the GIC version explicitly, which I
> think is more correct.

Good idea. We talked about it when the XEN_DOMCTL_CONFIG_* was firstly
introduced late in the release of Xen 4.5 and forgot to revisit it. I
will see to integrate it in this patch series.

-- 
Julien Grall