diff mbox

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

Message ID 1506049330-11196-8-git-send-email-tianyu.lan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

lan,Tianyu Sept. 22, 2017, 3:01 a.m. UTC
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(-)

Comments

Roger Pau Monné Oct. 19, 2017, 10 a.m. UTC | #1
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
>
Chao Gao Oct. 20, 2017, 1:44 a.m. UTC | #2
On Thu, Oct 19, 2017 at 11:00:27AM +0100, Roger Pau Monné wrote:
>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>
>> 
>> ---
>> +/*
>> + * 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?

Will give it a shot.

>
>> +    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.

I didn't try to enable vIOMMU for PVH. I will attemp to add vIOMMU
support for PVH and put those patches at the end of this series. 

>
>> +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.

Thanks for your kind reminder.

Chao
diff mbox

Patch

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) {
         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)
+{
+    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;
+
+    /* 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;
+
+    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