Re: [PATCH V3 7/29] tools/libxl: build DMAR table for a guest with one virtual VTD

["Roger Pau Monné" <roger.pau@citrix.com>]

On Thu, Sep 21, 2017 at 11:01:48PM -0400, Lan Tianyu wrote:
> From: Chao Gao <chao.gao@intel.com>
> 
> A new logic is added to build ACPI DMAR table in tool stack for a guest
> with one virtual VTD and pass through it to guest via existing mechanism. If
> there already are ACPI tables needed to pass through, we joint the tables.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> 
> ---
> v3:
>  - build dmar and initialize related acpi_modules struct in
>  libxl_x86_acpi.c, keeping in accordance with pvh.
> 
> ---
>  tools/libxl/libxl_x86.c      |  3 +-
>  tools/libxl/libxl_x86_acpi.c | 98 ++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 96 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> index 455f6f0..23c9a55 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -381,8 +381,7 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
>  {
>      int rc = 0;
>  
> -    if ((info->type == LIBXL_DOMAIN_TYPE_HVM) &&
> -        (info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE)) {
> +    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {

You will have to rebase this on top of current staging,
LIBXL_DEVICE_MODEL_VERSION_NONE is now gone.

>          rc = libxl__dom_load_acpi(gc, info, dom);
>          if (rc != 0)
>              LOGE(ERROR, "libxl_dom_load_acpi failed");
> diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
> index 1761756..adf02f4 100644
> --- a/tools/libxl/libxl_x86_acpi.c
> +++ b/tools/libxl/libxl_x86_acpi.c
> @@ -16,6 +16,7 @@
>  #include "libxl_arch.h"
>  #include <xen/hvm/hvm_info_table.h>
>  #include <xen/hvm/e820.h>
> +#include "libacpi/acpi2_0.h"
>  #include "libacpi/libacpi.h"
>  
>  #include <xc_dom.h>
> @@ -161,9 +162,9 @@ out:
>      return rc;
>  }
>  
> -int libxl__dom_load_acpi(libxl__gc *gc,
> -                         const libxl_domain_build_info *b_info,
> -                         struct xc_dom_image *dom)
> +static int libxl__dom_load_acpi_pvh(libxl__gc *gc,
> +                                    const libxl_domain_build_info *b_info,
> +                                    struct xc_dom_image *dom)
>  {
>      struct acpi_config config = {0};
>      struct libxl_acpi_ctxt libxl_ctxt;
> @@ -236,6 +237,97 @@ out:
>      return rc;
>  }
>  
> +static void *acpi_memalign(struct acpi_ctxt *ctxt, uint32_t size,
> +                           uint32_t align)
> +{
> +    int ret;
> +    void *ptr;
> +
> +    ret = posix_memalign(&ptr, align, size);
> +    if (ret != 0 || !ptr)
> +        return NULL;
> +
> +    return ptr;
> +}
> +
> +/*
> + * For hvm, we don't need build acpi in libxl. Instead, it's built in hvmloader.
> + * But if one hvm has virtual VTD(s), we build DMAR table for it and joint this
> + * table with existing content in acpi_modules in order to employ HVM
> + * firmware pass-through mechanism to pass-through DMAR table.
> + */
> +static int libxl__dom_load_acpi_hvm(libxl__gc *gc,
> +                                    const libxl_domain_build_info *b_info,
> +                                    struct xc_dom_image *dom)
> +{

AFAICT there's some code duplication between libxl__dom_load_acpi_hvm
and libxl__dom_load_acpi_pvh, isn't there a chance you could put this
in a common function?

> +    struct acpi_config config = { 0 };
> +    struct acpi_ctxt ctxt;
> +    void *table;
> +    uint32_t len;
> +
> +    if ((b_info->type != LIBXL_DOMAIN_TYPE_HVM) ||
> +        (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE) ||
> +        (b_info->num_viommus != 1) ||
> +        (b_info->viommu[0].type != LIBXL_VIOMMU_TYPE_INTEL_VTD))
> +        return 0;
> +
> +    ctxt.mem_ops.alloc = acpi_memalign;
> +    ctxt.mem_ops.v2p = virt_to_phys;
> +    ctxt.mem_ops.free = acpi_mem_free;
> +
> +    if (libxl_defbool_val(b_info->viommu[0].intremap))
> +        config.iommu_intremap_supported = true;
> +    /* x2apic is always enabled since in no case we must disable it */
> +    config.iommu_x2apic_supported = true;
> +    config.iommu_base_addr = b_info->viommu[0].base_addr;

I don't see libxl__dom_load_acpi_pvh setting any of the vIOMMU fields.

> +
> +    /* IOAPIC id and PSEUDO BDF */
> +    config.ioapic_id = 1;
> +    config.ioapic_bus = 0xff;
> +    config.ioapic_devfn = 0x0;
> +
> +    config.host_addr_width = 39;
> +
> +    table = construct_dmar(&ctxt, &config);
> +    if ( !table )
> +        return ERROR_NOMEM;
> +    len = ((struct acpi_header *)table)->length;
> +
> +    if (len) {
> +        libxl__ptr_add(gc, table);
> +        if (!dom->acpi_modules[0].data) {
> +            dom->acpi_modules[0].data = table;
> +            dom->acpi_modules[0].length = len;
> +        } else {
> +            /* joint tables */
> +            void *newdata;
> +
> +            newdata = libxl__malloc(gc, len + dom->acpi_modules[0].length);
> +            memcpy(newdata, dom->acpi_modules[0].data,
> +                   dom->acpi_modules[0].length);
> +            memcpy(newdata + dom->acpi_modules[0].length, table, len);
> +
> +            free(dom->acpi_modules[0].data);
> +            dom->acpi_modules[0].data = newdata;
> +            dom->acpi_modules[0].length += len;
> +        }
> +    }
> +    return 0;
> +}
> +
> +int libxl__dom_load_acpi(libxl__gc *gc,
> +                         const libxl_domain_build_info *b_info,
> +                         struct xc_dom_image *dom)
> +{
> +
> +    if (b_info->type != LIBXL_DOMAIN_TYPE_HVM)
> +        return 0;

Keep in mind a new PVH domain type has been introduced recently in
libxl, you will have to change this to b_info->type == LIBXL_DOMAIN_TYPE_PV.

> +
> +    if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE)
> +        return libxl__dom_load_acpi_pvh(gc, b_info, dom);
> +    else
> +        return libxl__dom_load_acpi_hvm(gc, b_info, dom);
> +}
>  /*
>   * Local variables:
>   * mode: C
> -- 
> 1.8.3.1
> 

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