|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2] tools/libxl: Switch Arm guest type to PVH
On Wed, Aug 22, 2018 at 04:00:45PM +0100, Julien Grall wrote:
> Currently, the toolstack is considering Arm guest always PV. However,
> they are very similar to PVH because HW virtualization extension are used
> and QEMU is not started. So switch Arm guest type to PVH.
>
> To keep compatibility with toolstack creating Arm guest with PV type
> (e.g libvirt), libxl will now convert those guests to PVH.
>
> Furthermore, the default type for Arm in xl will now be PVH to allow
> smooth transition for user.
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
>
> ---
>
> This was discussed at Xen Summit and also in various thread on
> xen-devel. The latest one was when Andrew sent a patch to deny guest creation
> on Arm with XEN_DOMCTL_CDF_hap unset.
>
> I suspect we first implemented Arm guest as PV in libxl because PVH was
> non-existent and the type was easier to avoid spawning QEMU. Note that
> Linux and Xen are already considering Arm guest as PVH.
>
> Changes in v2:
> - Rather than denying PV guest, convert them to PVH.
> ---
> docs/man/xl.cfg.pod.5.in | 3 ++-
> tools/libxl/libxl_arch.h | 3 ++-
> tools/libxl/libxl_arm.c | 39 +++++++++++++++++++++++++++++++++++++--
> tools/libxl/libxl_create.c | 2 +-
> tools/libxl/libxl_x86.c | 3 ++-
> tools/xl/xl_parse.c | 4 ++++
> 6 files changed, 48 insertions(+), 6 deletions(-)
>
> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> index b72718151b..a0568e397d 100644
> --- a/docs/man/xl.cfg.pod.5.in
> +++ b/docs/man/xl.cfg.pod.5.in
> @@ -86,7 +86,8 @@ guest operating systems. This is the default.
>
> Specifies that this is to be an PVH domain. That is a lightweight HVM-like
> guest without a device model and without many of the emulated devices
> -available to HVM guests. Note that this mode requires a PVH aware kernel.
> +available to HVM guests. Note that this mode requires a PVH aware kernel on
> +x86.
Should you add "This is the default on ARM". And likewise modify
'type="pv"' so it's last sentence is "This is the default on x86"?
>
> =item B<type="hvm">
>
> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
> index 5ab0c95974..930570ef1e 100644
> --- a/tools/libxl/libxl_arch.h
> +++ b/tools/libxl/libxl_arch.h
> @@ -65,7 +65,8 @@ _hidden
> int libxl__arch_domain_map_irq(libxl__gc *gc, uint32_t domid, int irq);
>
> _hidden
> -void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info
> *b_info);
> +void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
> + libxl_domain_build_info
> *b_info);
>
> _hidden
> int libxl__arch_extra_memory(libxl__gc *gc,
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 18c41f4ee9..0cf92d7af4 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -978,7 +978,11 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> int rc;
> uint64_t val;
>
> - assert(info->type == LIBXL_DOMAIN_TYPE_PV);
> + if (info->type != LIBXL_DOMAIN_TYPE_PVH) {
> + LOG(ERROR, "Unsupported Arm guest type %s",
> + libxl_domain_type_to_string(info->type));
> + return ERROR_FAIL;
Nit: ERROR_INVAL might be better here.
> + }
>
> /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ. */
> val = MASK_INSR(HVM_PARAM_CALLBACK_TYPE_PPI,
> @@ -1135,10 +1139,41 @@ int libxl__arch_domain_map_irq(libxl__gc *gc,
> uint32_t domid, int irq)
> return xc_domain_bind_pt_spi_irq(CTX->xch, domid, irq, irq);
> }
>
> -void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info
> *b_info)
> +void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
> + libxl_domain_build_info
> *b_info)
> {
> /* ACPI is disabled by default */
> libxl_defbool_setdefault(&b_info->acpi, false);
> +
> + /*
> + * Arm guest are now considered as PVH by the toolstack. To allow
> + * compatibility with previous toolstack, PV guest are automatically
> + * converted to PVH.
> + */
> + if (b_info->type != LIBXL_DOMAIN_TYPE_PV)
> + return;
> +
> + LOG(WARN, "Converting PV guest to PVH");
> + LOG(WARN, "ARM guest are now PVH. Please update your toolstack.");
"Please update your configuration file.". Updating the toolstack won't
make this message go away AFAICT :).
> +
> + b_info->type = LIBXL_DOMAIN_TYPE_PVH;
> +
> + /*
> + * They only field in u.pv that matters on Arm are: kernel, cmdline,
> + * ramdisk.
> + */
> +
> + if (!b_info->kernel && b_info->u.pv.kernel)
> + b_info->kernel = b_info->u.pv.kernel;
> +
> + if (!b_info->ramdisk && b_info->u.pv.ramdisk)
> + b_info->ramdisk = b_info->u.pv.ramdisk;
> +
> + if (!b_info->cmdline && b_info->u.pv.cmdline)
> + b_info->cmdline = b_info->u.pv.cmdline;
> +
> + /* Reset b_info->u.pvh to default values */
> + memset(&b_info->u.pvh, 0, sizeof(b_info->u.pvh));
I'm afraid that's not correct. The default values for u.pvh are set
by libxl__domain_build_info_setdefault.
> }
>
> /*
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index d4fa06daea..a6431c5d3f 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -215,7 +215,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> if (!b_info->event_channels)
> b_info->event_channels = 1023;
>
> - libxl__arch_domain_build_info_setdefault(b_info);
> + libxl__arch_domain_build_info_setdefault(gc, b_info);
> libxl_defbool_setdefault(&b_info->dm_restrict, false);
>
> switch (b_info->type) {
> diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> index 81523a568f..8b6759c089 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -613,7 +613,8 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc
> *gc,
> return rc;
> }
>
> -void libxl__arch_domain_build_info_setdefault(libxl_domain_build_info
> *b_info)
> +void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
> + libxl_domain_build_info
> *b_info)
> {
> libxl_defbool_setdefault(&b_info->acpi, true);
> }
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 971ec1bc56..0bda28152b 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -1286,7 +1286,11 @@ void parse_config_data(const char *config_source,
> }
>
> if (c_info->type == LIBXL_DOMAIN_TYPE_INVALID)
> +#if defined(__arm__) || defined(__aarch64__)
I think #ifdef CONFIG_ARM should DTRT and it's cleaner IMO.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |