Re: [PATCH v4 31/31] libxl: allow the creation of HVM domains without a device model.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Aug 07, 2015 at 12:18:08PM +0200, Roger Pau Monne wrote:
> Replace the firmware loaded into HVM guests with an OS kernel. Since the HVM
> builder now uses the PV xc_dom_* set of functions this kernel will be parsed
> and loaded inside the guest like on PV, but the container is a pure HVM
> guest.

What is your plan in regards to the 'pvh' parameteR? Should it be
repurposed so you can use both?
> 
> Also, if device_model_version is set to none or a device model for the
> specified domain is not present unconditinally set the nic type to
> LIBXL_NIC_TYPE_VIF.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
> Changes since v3:
>  - Add explicit /* fall through */ comments.
>  - Expand libxl__device_nic_setdefault so that it sets the right nic type
>    for HVMlite guests.
>  - Remove stray space in hvm_build_set_params.
>  - Fix the error paths of libxl__domain_firmware.
> ---
>  docs/man/xl.cfg.pod.5        |  5 ++++
>  tools/libxc/xc_dom_x86.c     |  7 +++++
>  tools/libxl/libxl.c          | 39 ++++++++++++++-----------
>  tools/libxl/libxl_create.c   | 16 ++++++++++-
>  tools/libxl/libxl_dm.c       | 13 ++++++++-
>  tools/libxl/libxl_dom.c      | 68 ++++++++++++++++++++++++++++++--------------
>  tools/libxl/libxl_internal.h |  5 +++-
>  tools/libxl/libxl_types.idl  |  1 +
>  tools/libxl/libxl_x86.c      |  4 ++-
>  tools/libxl/xl_cmdimpl.c     |  2 ++
>  10 files changed, 118 insertions(+), 42 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 80e51bb..8cd7726 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1741,6 +1741,11 @@ This device-model is the default for Linux dom0.
>  Use the device-model based upon the historical Xen fork of Qemu.
>  This device-model is still the default for NetBSD dom0.
>  
> +=item B<none>
> +
> +Don't use any device model. This requires a kernel capable of booting
> +in this mode.
> +
>  =back
>  
>  It is recommended to accept the default value for new guests.  If
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index 1599de4..d67feb0 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -1269,6 +1269,13 @@ static int meminit_hvm(struct xc_dom_image *dom)
>      if ( nr_pages > target_pages )
>          memflags |= XENMEMF_populate_on_demand;
>  
> +    /* Make sure there's a MMIO hole for the special pages. */
> +    if ( dom->mmio_size == 0 )
> +    {
> +        dom->mmio_size = NR_SPECIAL_PAGES << PAGE_SHIFT;
> +        dom->mmio_start = special_pfn(0);
> +    }
> +
>      if ( dom->nr_vmemranges == 0 )
>      {
>          /* Build dummy vnode information
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 083f099..a01868a 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1033,11 +1033,13 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid)
>      }
>  
>      if (type == LIBXL_DOMAIN_TYPE_HVM) {
> -        rc = libxl__domain_resume_device_model(gc, domid);
> -        if (rc < 0) {
> -            LOG(ERROR, "failed to unpause device model for domain %u:%d",
> -                domid, rc);
> -            goto out;
> +        if (libxl__domain_has_device_model(gc, domid)) {
> +            rc = libxl__domain_resume_device_model(gc, domid);
> +            if (rc < 0) {
> +                LOG(ERROR, "failed to unpause device model for domain %u:%d",
> +                    domid, rc);
> +                goto out;
> +            }
>          }
>      }
>      ret = xc_domain_unpause(ctx->xch, domid);
> @@ -1567,7 +1569,6 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
>      libxl_ctx *ctx = CTX;
>      uint32_t domid = dis->domid;
>      char *dom_path;
> -    char *pid;
>      int rc, dm_present;
>  
>      libxl__ev_child_init(&dis->destroyer);
> @@ -1584,14 +1585,13 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
>  
>      switch (libxl__domain_type(gc, domid)) {
>      case LIBXL_DOMAIN_TYPE_HVM:
> -        if (!libxl_get_stubdom_id(CTX, domid))
> -            dm_present = 1;
> -        else
> +        if (libxl_get_stubdom_id(CTX, domid)) {
>              dm_present = 0;
> -        break;
> +            break;
> +        }
> +        /* fall through */
>      case LIBXL_DOMAIN_TYPE_PV:
> -        pid = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "/local/domain/%d/image/device-model-pid", domid));
> -        dm_present = (pid != NULL);
> +        dm_present = libxl__domain_has_device_model(gc, domid);
>          break;
>      case LIBXL_DOMAIN_TYPE_INVALID:
>          rc = ERROR_FAIL;
> @@ -3203,7 +3203,7 @@ out:
>  /******************************************************************************/
>  
>  int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
> -                                 uint32_t domid)
> +                                 uint32_t domid, libxl_domain_build_info *info)
>  {
>      int rc;
>  
> @@ -3240,8 +3240,15 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
>  
>      switch (libxl__domain_type(gc, domid)) {
>      case LIBXL_DOMAIN_TYPE_HVM:
> -        if (!nic->nictype)
> -            nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
> +        if (!nic->nictype) {
> +            if (info != NULL &&
> +                info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE)
> +                nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
> +            else if (libxl__domain_has_device_model(gc, domid))
> +                nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
> +            else
> +                nic->nictype = LIBXL_NIC_TYPE_VIF;
> +        }
>          break;
>      case LIBXL_DOMAIN_TYPE_PV:
>          if (nic->nictype == LIBXL_NIC_TYPE_VIF_IOEMU) {
> @@ -3290,7 +3297,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
>      libxl_device_nic_init(&nic_saved);
>      libxl_device_nic_copy(CTX, &nic_saved, nic);
>  
> -    rc = libxl__device_nic_setdefault(gc, nic, domid);
> +    rc = libxl__device_nic_setdefault(gc, nic, domid, NULL);
>      if (rc) goto out;
>  
>      front = flexarray_make(gc, 16, 1);
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 2348ffc..6738e34 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -119,6 +119,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>                  b_info->u.hvm.bios = LIBXL_BIOS_TYPE_ROMBIOS; break;
>              case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
>                  b_info->u.hvm.bios = LIBXL_BIOS_TYPE_SEABIOS; break;
> +            case LIBXL_DEVICE_MODEL_VERSION_NONE:
> +                break;
>              default:return ERROR_INVAL;
>              }
>  
> @@ -132,6 +134,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>              if (b_info->u.hvm.bios == LIBXL_BIOS_TYPE_ROMBIOS)
>                  return ERROR_INVAL;
>              break;
> +        case LIBXL_DEVICE_MODEL_VERSION_NONE:
> +            break;
>          default:abort();
>          }
>  
> @@ -236,6 +240,9 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>                  break;
>              }
>              break;
> +        case LIBXL_DEVICE_MODEL_VERSION_NONE:
> +            b_info->video_memkb = 0;
> +            break;
>          case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
>          default:
>              switch (b_info->u.hvm.vga.kind) {
> @@ -923,7 +930,8 @@ static void initiate_domain_create(libxl__egc *egc,
>           * called libxl_device_nic_add when domcreate_launch_dm gets called,
>           * but qemu needs the nic information to be complete.
>           */
> -        ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid);
> +        ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid,
> +                                           &d_config->b_info);
>          if (ret) {
>              LOG(ERROR, "Unable to set nic defaults for nic %d", i);
>              goto error_out;
> @@ -1260,6 +1268,12 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
>          libxl__device_console_add(gc, domid, &console, state, &device);
>          libxl__device_console_dispose(&console);
>  
> +        if (d_config->b_info.device_model_version ==
> +            LIBXL_DEVICE_MODEL_VERSION_NONE) {
> +            domcreate_devmodel_started(egc, &dcs->dmss.dm, 0);
> +            return;
> +        }
> +
>          libxl_device_vkb_init(&vkb);
>          libxl__device_vkb_add(gc, domid, &vkb);
>          libxl_device_vkb_dispose(&vkb);
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index f6b4c89..7e71ce9 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1501,7 +1501,8 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
>           * called libxl_device_nic_add at this point, but qemu needs
>           * the nic information to be complete.
>           */
> -        ret = libxl__device_nic_setdefault(gc, &dm_config->nics[i], dm_domid);
> +        ret = libxl__device_nic_setdefault(gc, &dm_config->nics[i], dm_domid,
> +                                           &dm_config->b_info);
>          if (ret)
>              goto out;
>      }
> @@ -1993,6 +1994,16 @@ out:
>      return rc;
>  }
>  
> +bool libxl__domain_has_device_model(libxl__gc *gc, uint32_t domid)
> +{
> +    char *pid;
> +
> +    pid = libxl__xs_read(gc, XBT_NULL,
> +          GCSPRINTF("/local/domain/%d/image/device-model-pid", domid));
> +
> +    return (pid != NULL);
> +}
> +
>  int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
>  {
>      char *path = libxl__device_model_xs_path(gc, LIBXL_TOOLSTACK_DOMID,
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 8b8e079..a8a02ac 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -781,21 +781,23 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
>      uint64_t str_mfn, cons_mfn;
>      int i;
>  
> -    va_map = xc_map_foreign_range(handle, domid,
> -                                  XC_PAGE_SIZE, PROT_READ | PROT_WRITE,
> -                                  HVM_INFO_PFN);
> -    if (va_map == NULL)
> -        return ERROR_FAIL;
> +    if (info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE) {
> +        va_map = xc_map_foreign_range(handle, domid,
> +                                      XC_PAGE_SIZE, PROT_READ | PROT_WRITE,
> +                                      HVM_INFO_PFN);
> +        if (va_map == NULL)
> +            return ERROR_FAIL;
>  
> -    va_hvm = (struct hvm_info_table *)(va_map + HVM_INFO_OFFSET);
> -    va_hvm->apic_mode = libxl_defbool_val(info->u.hvm.apic);
> -    va_hvm->nr_vcpus = info->max_vcpus;
> -    memset(va_hvm->vcpu_online, 0, sizeof(va_hvm->vcpu_online));
> -    memcpy(va_hvm->vcpu_online, info->avail_vcpus.map, info->avail_vcpus.size);
> -    for (i = 0, sum = 0; i < va_hvm->length; i++)
> -        sum += ((uint8_t *) va_hvm)[i];
> -    va_hvm->checksum -= sum;
> -    munmap(va_map, XC_PAGE_SIZE);
> +        va_hvm = (struct hvm_info_table *)(va_map + HVM_INFO_OFFSET);
> +        va_hvm->apic_mode = libxl_defbool_val(info->u.hvm.apic);
> +        va_hvm->nr_vcpus = info->max_vcpus;
> +        memset(va_hvm->vcpu_online, 0, sizeof(va_hvm->vcpu_online));
> +        memcpy(va_hvm->vcpu_online, info->avail_vcpus.map, info->avail_vcpus.size);
> +        for (i = 0, sum = 0; i < va_hvm->length; i++)
> +            sum += ((uint8_t *) va_hvm)[i];
> +        va_hvm->checksum -= sum;
> +        munmap(va_map, XC_PAGE_SIZE);
> +    }
>  
>      xc_hvm_param_get(handle, domid, HVM_PARAM_STORE_PFN, &str_mfn);
>      xc_hvm_param_get(handle, domid, HVM_PARAM_CONSOLE_PFN, &cons_mfn);
> @@ -861,7 +863,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
>      const char *firmware;
> -    int e, rc = ERROR_FAIL;
> +    int e, rc;
>      int datalen = 0;
>      void *data;
>  
> @@ -876,18 +878,34 @@ static int libxl__domain_firmware(libxl__gc *gc,
>          case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
>              firmware = "hvmloader";
>              break;
> +        case LIBXL_DEVICE_MODEL_VERSION_NONE:
> +            if (info->kernel == NULL) {
> +                LOG(ERROR, "no device model requested without a kernel");
> +                rc = ERROR_FAIL;
> +                goto out;
> +            }
> +            break;
>          default:
>              LOG(ERROR, "invalid device model version %d",
>                  info->device_model_version);
> -            return ERROR_FAIL;
> -            break;
> +            rc = ERROR_FAIL;
> +            goto out;
>          }
>      }
>  
> -    rc = xc_dom_kernel_file(dom, libxl__abs_path(gc, firmware,
> +    if (info->kernel != NULL &&
> +        info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE) {
> +        /* Try to load a kernel instead of the firmware. */
> +        rc = xc_dom_kernel_file(dom, info->kernel);
> +        if (rc == 0 && info->ramdisk != NULL)
> +            rc = xc_dom_ramdisk_file(dom, info->ramdisk);
> +    } else {
> +        rc = xc_dom_kernel_file(dom, libxl__abs_path(gc, firmware,
>                                                   libxl__xenfirmwaredir_path()));
> +    }
> +
>      if (rc != 0) {
> -        LOGE(ERROR, "xc_dom_kernel_file failed");
> +        LOGE(ERROR, "xc_dom_{kernel_file/ramdisk_file} failed");
>          goto out;
>      }
>  
> @@ -898,6 +916,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
>          if (e) {
>              LOGEV(ERROR, e, "failed to read SMBIOS firmware file %s",
>                  info->u.hvm.smbios_firmware);
> +            rc = ERROR_FAIL;
>              goto out;
>          }
>          libxl__ptr_add(gc, data);
> @@ -915,6 +934,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
>          if (e) {
>              LOGEV(ERROR, e, "failed to read ACPI firmware file %s",
>                  info->u.hvm.acpi_firmware);
> +            rc = ERROR_FAIL;
>              goto out;
>          }
>          libxl__ptr_add(gc, data);
> @@ -927,6 +947,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
>  
>      return 0;
>  out:
> +    assert(rc != 0);
>      return rc;
>  }
>  
> @@ -939,10 +960,13 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
>      uint64_t mmio_start, lowmem_end, highmem_end, mem_size;
>      libxl_domain_build_info *const info = &d_config->b_info;
>      struct xc_dom_image *dom = NULL;
> +    bool emulated_devices =
> +        info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE ?
> +        true : false;
>  
>      xc_dom_loginit(ctx->xch);
>  
> -    dom = xc_dom_allocate(ctx->xch, NULL, NULL);
> +    dom = xc_dom_allocate(ctx->xch, info->cmdline, NULL);
>      if (!dom) {
>          LOGE(ERROR, "xc_dom_allocate failed");
>          goto out;
> @@ -974,7 +998,7 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
>  
>      if (dom->target_pages == 0)
>          dom->target_pages = mem_size >> XC_PAGE_SHIFT;
> -    if (dom->mmio_size == 0)
> +    if (dom->mmio_size == 0 && emulated_devices)
>          dom->mmio_size = HVM_BELOW_4G_MMIO_LENGTH;
>      lowmem_end = mem_size;
>      highmem_end = 0;
> @@ -987,7 +1011,7 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
>      dom->lowmem_end = lowmem_end;
>      dom->highmem_end = highmem_end;
>      dom->mmio_start = mmio_start;
> -    dom->emulation = true;
> +    dom->emulation = emulated_devices;
>  
>      rc = libxl__domain_device_construct_rdm(gc, d_config,
>                                              info->u.hvm.rdm_mem_boundary_memkb*1024,
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index ea89f1f..ee8d03d 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1172,7 +1172,8 @@ _hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
>  _hidden int libxl__device_disk_setdefault(libxl__gc *gc,
>                                            libxl_device_disk *disk);
>  _hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
> -                                         uint32_t domid);
> +                                         uint32_t domid,
> +                                         libxl_domain_build_info *info);
>  _hidden int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm);
>  _hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
>  _hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
> @@ -1617,6 +1618,8 @@ _hidden int libxl__wait_for_device_model_deprecated(libxl__gc *gc,
>  
>  _hidden int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid);
>  
> +_hidden bool libxl__domain_has_device_model(libxl__gc *gc, uint32_t domid);
> +
>  _hidden const libxl_vnc_info *libxl__dm_vnc(const libxl_domain_config *g_cfg);
>  
>  _hidden char *libxl__abs_path(libxl__gc *gc, const char *s, const char *path);
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index ef346e7..b6e99c4 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -98,6 +98,7 @@ libxl_device_model_version = Enumeration("device_model_version", [
>      (0, "UNKNOWN"),
>      (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm)
>      (2, "QEMU_XEN"),             # Upstream based qemu-xen device model
> +    (3, "NONE"),                 # No device model
>      ])
>  
>  libxl_console_type = Enumeration("console_type", [
> diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> index 270f3cf..21424a5 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -7,7 +7,9 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>                                        libxl_domain_config *d_config,
>                                        xc_domain_configuration_t *xc_config)
>  {
> -    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM)
> +    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
> +        d_config->b_info.device_model_version !=
> +        LIBXL_DEVICE_MODEL_VERSION_NONE)
>          xc_config->emulation_flags = (XEN_X86_EMU_LAPIC | XEN_X86_EMU_HPET |
>                                        XEN_X86_EMU_PMTIMER | XEN_X86_EMU_RTC |
>                                        XEN_X86_EMU_IOAPIC | XEN_X86_EMU_PIC |
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 499a05c..2776318 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2148,6 +2148,8 @@ skip_vfb:
>          } else if (!strcmp(buf, "qemu-xen")) {
>              b_info->device_model_version
>                  = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
> +        } else if (!strcmp(buf, "none")) {
> +            b_info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_NONE;
>          } else {
>              fprintf(stderr,
>                      "Unknown device_model_version \"%s\" specified\n", buf);
> -- 
> 1.9.5 (Apple Git-50.3)
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel