|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 2/3] arm: Allow the user to specify the GIC version
On Tue, 2015-07-07 at 15:08 +0100, Julien Grall wrote:
> A platform may have a GIC compatible with previous version of the
> device.
"...a GIC which is compatible with a previous version..."
>
> This is allow to virtualize an unmodified OS on new hardware if the GIC
> is compatible with older version.
"This is to allow virtualization of an unmodified..."
"...with the 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 only GICv2 and GICv3 are supported).
I think "Unless the user has specified in the configuration file..."
>
> 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 left aside a user error reporting for a follow-up as I'm not
> sure how to notify the user which GIC versions are available. May be
> by a new mechanism similar to xen_get_caps?
>
> It may be possible to rework the libxl code to restrict the scope of
> xc_config in libxl_domain_make. This can be done in a follow-up if
> we figure what to do the frequency field.
>
> Changes in v4:
> - Update the documentation to specify the default behavior
> - Update the domain configuration with the GIC version returned
> by the hypervisor.
>
> Changes in v3:
> - Rename GIC_VERSION define in LIBXL_GIC_VERSION_Vn.
> - Change the value of each define
> - Use libxl_gic_version_from_string rather than custom if/else
> - Rename LIBXL_HAVE_BUILDINFO_GIC_VERSION into
> LIBXL_HAVE_BUILDINFO_ARM_GIC_VERSION and update the comment
> - Update doc with Ian's suggestion
> - Typoes
>
> Changes in v2:
> - Introduce arch_arm in libxl_domain_build_info to store ARM
> specific field
> - Add docs
> - Remove code that is not necessary with the new version
> ---
> docs/man/xl.cfg.pod.5 | 34 +++++++++++++++++++++++++++++++++
> tools/libxl/libxl.h | 5 +++++
> tools/libxl/libxl_arch.h | 6 ++++++
> tools/libxl/libxl_arm.c | 35 +++++++++++++++++++++++++++++++++-
> tools/libxl/libxl_create.c | 7 +++++++
> tools/libxl/libxl_types.idl | 11 +++++++++++
> tools/libxl/libxl_x86.c | 7 +++++++
> tools/libxl/xl_cmdimpl.c | 12 ++++++++++++
> xen/arch/arm/domain.c | 45
> ++++++++++++++++++++++++++------------------
> xen/arch/arm/vgic.c | 4 ++--
> xen/include/asm-arm/domain.h | 2 ++
> 11 files changed, 147 insertions(+), 21 deletions(-)
>
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index a3e0e2e..12765d5 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1688,6 +1688,40 @@ The default is B<en-us>.
>
> See L<qemu(1)> for more information.
>
> +=head2 Architecture Specific options
> +
> +=head3 ARM
> +
> +=over 4
> +
> +=item B<gic_version="vN">
> +
> +Version of the GIC emulated for the guest. Currently, the following
> +versions are supported:
> +
> +=over 4
> +
> +=item B<v2>
> +
> +Emulate a GICv2 hardware
"Emulate the GICv2 hardware" or just "Emulate a GICv2" (without the
h/w).
> +
> +=item B<v3>
> +
> +Emulate a GICv3 hardware.
As above.
> Note that the emulated GIC does not support the
> +GICv2 compatibility mode.
> +
> +=item B<default>
> +
> +Emulate the same version as the native GIC hardware used by host where the
> +the domain has been created.
"... where the domain was created".
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 03a9205..8ac90c4 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -61,7 +61,40 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> xc_config->nr_spis = nr_spis;
> LOG(DEBUG, " - Allocate %u SPIs", nr_spis);
>
> - xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
> + switch (d_config->b_info.arch_arm.gic_version) {
> + case LIBXL_GIC_VERSION_DEFAULT:
> + xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
> + break;
> + case LIBXL_GIC_VERSION_V2:
> + xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
> + break;
> + case LIBXL_GIC_VERSION_V3:
> + xc_config->gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
> + break;
> + default:
> + LOG(ERROR, "Unknown GIC version %s\n",
> +
> libxl_gic_version_to_string(d_config->b_info.arch_arm.gic_version));
libxl_gic_version_to_string will return NULL for a truly unknown value.
Perhaps print %d version too/instead?
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index f799081..2b8a506 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -579,6 +579,13 @@ int libxl__domain_make(libxl__gc *gc,
> libxl_domain_config *d_config,
> goto out;
> }
>
> + ret = libxl__arch_domain_save_config(gc, d_config, xc_config);
> + if (ret < 0) {
> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "fail to save domain
> config");
Please use the shorter LOG* macros, and you don't need/want to log errno
here since the function doesn't touch it.
Actually, the function logs on failure itself, no need to repeat it I
think, just document "logs on failure" next to the prototype.
> + rc = ERROR_FAIL;
libxl__arch_domain_save_config already returns a libxl ERROR_*, so it
should be stored into rc directly and propagated.
> + goto out;
> + }
> +
> ret = xc_cpupool_movedomain(ctx->xch, info->poolid, *domid);
> if (ret < 0) {
> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "domain move fail");
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 1be3f8b..e825315 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2250,6 +2250,18 @@ skip_vfb:
> }
> }
>
> + if (!xlu_cfg_get_string (config, "gic_version", &buf, 1)) {
> + libxl_gic_version v;
> +
> + e = libxl_gic_version_from_string(buf, &v);
> + if (e) {
> + fprintf(stderr,
> + "Unknown gic_version \"%s\" specified\n", buf);
> + exit(-ERROR_FAIL);
> + }
> + b_info->arch_arm.gic_version = v;
You can just pass &b_info....gic_version straight to the from_string
function.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |