Re: [Xen-devel] [RFC 21/22] arm: Allow the user to specify the GIC version

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Fri, 2015-05-08 at 14:29 +0100, Julien Grall wrote:
> A platform may have a GIC compatible with previous version of the
> device.
> 
> This is allow to virtualize an unmodified OS on new hardware if the GIC
> is compatible with older version.
> 
> When a guest is created, the vGIC will emulate same version as the
> hardware. Although, the user can specify in the configuration file the
> preferred version (currently on GICv2 and GICv3 are supported).
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> 
> ---
> 
>     The hypervisor will check if the GIC is able to virtualize the
>     version specified by the user (via the DOMCTL createdomain).
>     If it's not compatible an error will be send on the Xen console
>     which will make the error not obvious for user.
> 
>     I'm wondering if we should expose to the toolstack the vGIC versions
>     supported via a mecanism similar to xen_get_caps?

There may even be room in xencaps, although it isn't very scalable.

Another option might be to overwrite bad fields in the config struct
with some sentinel to indicate they weren't usable. Not sure I like that
though.

>     I didn't add the documention because I wasn't sure in which section
>     I should put it in the xl man.

Probably needs a new ARCHITECTURE: ARM section, and I bet a lot of stuff
could move to ARCH: X86 (you don't have to do all that though).

> @@ -387,7 +393,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
>      ("blkdev_start",    string),
>  
>      ("vnuma_nodes", Array(libxl_vnode_info, "num_vnuma_nodes")),
> -    
> +
> +    ("gic_version", libxl_gic_version),

For historical reasons the domain config is rather x86 centric. Fixing
that is hard without breaking the API, but I think we should at least
think about putting new arch specific stuff in a per-arch location.

Just wrapping this in a struct named arm would be an ok start, we can
add x86 when the next thing which should live there comes along.

I think having the fields always present, even for builds on arches
where they don't make sense is ok, the alternative is to extend the IDL
language to allow things to be made arch specific and generate suitable
ifdefs...


> +
>      ("device_model_version", libxl_device_model_version),
>      ("device_model_stubdomain", libxl_defbool),
>      # if you set device_model you must set device_model_version too
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 648ca08..b033c0b 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1298,6 +1298,18 @@ static void parse_config_data(const char 
> *config_source,
>          !xlu_cfg_get_string (config, "cpus_soft", &buf, 0))
>          parse_vcpu_affinity(b_info, cpus, buf, num_cpus, false);
>  
> +    if (!xlu_cfg_get_string (config, "gic_version", &buf, 1)) {

Could we just make this an integer?

> +        if (!strcmp(buf, "v2"))
> +            b_info->gic_version = LIBXL_GIC_VERSION_2;
> +        else if (!strcmp(buf, "v3"))
> +            b_info->gic_version = LIBXL_GIC_VERSION_3;
> +        else {
> +            fprintf(stderr,
> +                    "Uknown gic_version \"%s\" specified\n", buf);

"Unknown"

> +            exit(1);
> +        }
> +    }
> +
>      libxl_defbool_set(&b_info->claim_mode, claim_mode);
>  
>      if (xlu_cfg_get_string (config, "on_poweroff", &buf, 0))



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