Re: [Xen-devel] [PATCH v3 3/3] tools/libxl: Switch Arm guest type to PVH

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Oct 01, 2018 at 07:57:21PM +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>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> 
> ---
> 
> 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 v3:
>         - Properly reset u.pvh
>         - Update documentation and print
>         - Return ERROR_INVAL rather than ERROR_FAIL
> 
>     Changes in v2:
>         - Rather than denying PV guest, convert them to PVH
> ---
>  docs/man/xl.cfg.pod.5.in   |  5 +++--
>  tools/libxl/libxl_arch.h   |  3 ++-
>  tools/libxl/libxl_arm.c    | 26 ++++++++++++++++++++++++--
>  tools/libxl/libxl_create.c |  2 +-
>  tools/libxl/libxl_x86.c    |  3 ++-
>  tools/xl/xl_parse.c        |  4 ++++
>  6 files changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> index b72718151b..b1c0be14cd 100644
> --- a/docs/man/xl.cfg.pod.5.in
> +++ b/docs/man/xl.cfg.pod.5.in
> @@ -80,13 +80,14 @@ single host must be unique.
>  =item B<type="pv">
>  
>  Specifies that this is to be a PV domain, suitable for hosting Xen-aware
> -guest operating systems. This is the default.
> +guest operating systems. This is the default on x86.
>  
>  =item B<type="pvh">
>  
>  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. This is the default on Arm.
>  
>  =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 699fd9ddc6..25dc3defc6 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -953,7 +953,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_INVAL;
> +    }
>  
>      /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ. */
>      val = MASK_INSR(HVM_PARAM_CALLBACK_TYPE_PPI,
> @@ -1110,10 +1114,28 @@ 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.");
> +    LOG(WARN, "Please fix your configuration file/toolstack.");
> +
> +    /* Re-initialize type to PVH and all associated fields to defaults. */
> +    memset(&b_info->u, '\0', sizeof(b_info->u));

I don't really like the zeroing done here, but I think it's fine as
long as it's only done when converting from PV -> PVH and after
copying the deprecated fields.

Thansk, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel