diff mbox

[9/25] Tools/libacpi: Add a user configurable parameter to control vIOMMU attributes

Message ID 1498715457-16565-10-git-send-email-tianyu.lan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

lan,Tianyu June 29, 2017, 5:50 a.m. UTC
From: Chao Gao <chao.gao@intel.com>

a field, viommu_info, is added to struct libxl_domain_build_info. Several
attributes can be specified by guest configuration file for the DMAR table
building and vIOMMU creation.

In domain creation process, a new logic is added to build ACPI DMAR table in
tool stack according VM configuration and to pass though it to hvmloader via
xenstore ACPI PT channel. If there 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>
---
 docs/man/xl.cfg.pod.5.in    | 34 +++++++++++++++++-
 tools/libacpi/build.c       |  5 +++
 tools/libacpi/libacpi.h     |  1 +
 tools/libxl/libxl_dom.c     | 87 +++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_types.idl | 10 ++++++
 tools/xl/xl_parse.c         | 64 +++++++++++++++++++++++++++++++++
 6 files changed, 200 insertions(+), 1 deletion(-)

Comments

Wei Liu June 30, 2017, 1:44 p.m. UTC | #1
Title is wrong. Should be tagged "tools".

On Thu, Jun 29, 2017 at 01:50:41AM -0400, Lan Tianyu wrote:
> From: Chao Gao <chao.gao@intel.com>
> 
> a field, viommu_info, is added to struct libxl_domain_build_info. Several

Capitalise "a" please.

> attributes can be specified by guest configuration file for the DMAR table
> building and vIOMMU creation.
> 
> In domain creation process, a new logic is added to build ACPI DMAR table in
> tool stack according VM configuration and to pass though it to hvmloader via
> xenstore ACPI PT channel. If there 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>
> ---
>  docs/man/xl.cfg.pod.5.in    | 34 +++++++++++++++++-
>  tools/libacpi/build.c       |  5 +++
>  tools/libacpi/libacpi.h     |  1 +

Can the changes to libacpi be split out to a separate patch?

>  tools/libxl/libxl_dom.c     | 87 +++++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxl_types.idl | 10 ++++++
>  tools/xl/xl_parse.c         | 64 +++++++++++++++++++++++++++++++++
>  6 files changed, 200 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> index 38084c7..874f3f2 100644
> --- a/docs/man/xl.cfg.pod.5.in
> +++ b/docs/man/xl.cfg.pod.5.in
[...]
> diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
> index 6a4e1cf..0a58d6f 100644
> --- a/tools/libacpi/libacpi.h
> +++ b/tools/libacpi/libacpi.h
> @@ -109,6 +109,7 @@ struct acpi_config {
>  #define DMAR_X2APIC_OPT_OUT 0x2
>  struct acpi_dmar *construct_dmar(struct acpi_ctxt *ctxt,
>                                   const struct acpi_config *config);
> +uint32_t acpi_get_table_size(struct acpi_header * header);
>  int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config);
>  
>  #endif /* __LIBACPI_H__ */
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 5d914a5..f8d61c2 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -19,11 +19,13 @@
>  
>  #include "libxl_internal.h"
>  #include "libxl_arch.h"
> +#include "libacpi/libacpi.h"
>  
>  #include <xc_dom.h>
>  #include <xen/hvm/hvm_info_table.h>
>  #include <xen/hvm/hvm_xs_strings.h>
>  #include <xen/hvm/e820.h>
> +#include <xen/viommu.h>
>  
>  #include "_paths.h"
>  
> @@ -925,6 +927,43 @@ out:
>      return rc;
>  }
>  
> +static unsigned long acpi_v2p(struct acpi_ctxt *ctxt, void *v)
> +{
> +    return (unsigned long)v;
> +}
> +
> +static void *acpi_mem_alloc(struct acpi_ctxt *ctxt,
> +                            uint32_t size, uint32_t align)
> +{
> +    return aligned_alloc(align, size);

posix_memalign please.

aligned_alloc is in C11. We support C99.

> +}
> +
> +static void acpi_mem_free(struct acpi_ctxt *ctxt,
> +                          void *v, uint32_t size)
> +{
> +    /* ACPI builder currently doesn't free memory so this is just a stub */
> +}
> +
> +static int libxl__acpi_build_dmar(libxl__gc *gc,
> +                                  struct acpi_config *config,
> +                                  void **data_r, int *datalen_r)

This should probably go to libxl_x86_acpi.

It seems that you should be able to use a lot of code there instead of
writing your own.

In any case, I don't think having two places to construct acpi tables
for guest is a good idea.

> +{
> +    struct acpi_ctxt ctxt;
> +    void *table;
> +
> +    ctxt.mem_ops.alloc = acpi_mem_alloc;
> +    ctxt.mem_ops.free = acpi_mem_free;
> +    ctxt.mem_ops.v2p = acpi_v2p;
> +
> +    table = construct_dmar(&ctxt, config);
> +    if ( !table )
> +        return ERROR_FAIL;
> +
> +    *data_r = table;
> +    *datalen_r = acpi_get_table_size((struct acpi_header *)table);
> +    return 0;
> +}
> +
>  static int libxl__domain_firmware(libxl__gc *gc,
>                                    libxl_domain_build_info *info,
>                                    struct xc_dom_image *dom)
> @@ -1045,6 +1084,54 @@ static int libxl__domain_firmware(libxl__gc *gc,
>          }
>      }
>  
> +    /* build DMAR table according guest configuration and joint it with other
> +     * apci tables specified by acpi_modules */
> +    if ((info->u.hvm.viommu.type == VIOMMU_TYPE_INTEL_VTD) &&
> +        !libxl_defbool_is_default(info->u.hvm.viommu.intremap) &&
> +        info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> +        struct acpi_config config;
> +
> +        memset(&config, 0, sizeof(config));
> +        if (libxl_defbool_val(info->u.hvm.viommu.intremap)) {
> +            config.table_flags |= ACPI_HAS_DMAR;
> +            config.dmar_flag = DMAR_INTR_REMAP;
> +            if (!libxl_defbool_is_default(info->u.hvm.viommu.x2apic)
> +                && !libxl_defbool_val(info->u.hvm.viommu.x2apic))
> +                config.dmar_flag |= DMAR_X2APIC_OPT_OUT;
> +
> +            config.viommu_base_addr = info->u.hvm.viommu.base_addr;
> +            data = NULL;
> +            e = libxl__acpi_build_dmar(gc, &config, &data, &datalen);
> +            if (e) {
> +                LOGE(ERROR, "failed to build DMAR table");
> +                rc = ERROR_FAIL;
> +                goto out;
> +            }
> +
> +            libxl__ptr_add(gc, data);
> +            if (datalen) {
> +                if (!dom->acpi_modules[0].data) {
> +                    dom->acpi_modules[0].data = data;
> +                    dom->acpi_modules[0].length = (uint32_t)datalen;
> +                } else {
> +                    /* joint tables */
> +                    void *newdata;
> +                    newdata = malloc(datalen + dom->acpi_modules[0].length);
> +                    if (!newdata) {
> +                        LOGE(ERROR, "failed to joint DMAR table to acpi modules");
> +                        rc = ERROR_FAIL;
> +                        goto out;
> +                    }
> +                    memcpy(newdata, dom->acpi_modules[0].data,
> +                           dom->acpi_modules[0].length);
> +                    memcpy(newdata + dom->acpi_modules[0].length, data, datalen);
> +                    dom->acpi_modules[0].data = newdata;
> +                    dom->acpi_modules[0].length += (uint32_t)datalen;
> +                }
> +            }
> +        }
> +    }
> +

This seems to clash badly with the code in libxl__dom_load_acpi which
also writes to modules[0].
diff mbox

Patch

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 38084c7..874f3f2 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1540,7 +1540,39 @@  Do not provide a VM generation ID.
 See also "Virtual Machine Generation ID" by Microsoft:
 L<http://www.microsoft.com/en-us/download/details.aspx?id=30707>
 
-=back 
+=back
+
+=item B<viommu="VIOMMU_STRING">
+
+Specifies the viommu which are to be provided to the guest.
+
+B<VIOMMU_STRING> has the form C<KEY=VALUE,KEY=VALUE,...> where:
+
+=over 4
+
+=item B<KEY=VALUE>
+
+Possible B<KEY>s are:
+
+=over 4
+
+=item B<type="STRING">
+
+Currently there is only one valid type:
+
+(X86 only) "vtd" means providing a emulated intel VT-d to the guest.
+
+=item B<intremap=BOOLEAN>
+
+Specifies whether the vvtd should support intrrupt remapping
+and default 'true'.
+
+=item B<x2apic=BOOLEAN>
+
+Specifies whether the vvtd should support x2apic mode
+and default 'true'.
+
+=back
 
 =head3 Guest Virtual Time Controls
 
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index d5bedfd..0c3d3db 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -561,6 +561,11 @@  static int new_vm_gid(struct acpi_ctxt *ctxt,
     return 1;
 }
 
+uint32_t acpi_get_table_size(struct acpi_header * header)
+{
+    return header ? header->length : 0;
+}
+
 int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
 {
     struct acpi_info *acpi_info;
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index 6a4e1cf..0a58d6f 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -109,6 +109,7 @@  struct acpi_config {
 #define DMAR_X2APIC_OPT_OUT 0x2
 struct acpi_dmar *construct_dmar(struct acpi_ctxt *ctxt,
                                  const struct acpi_config *config);
+uint32_t acpi_get_table_size(struct acpi_header * header);
 int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config);
 
 #endif /* __LIBACPI_H__ */
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 5d914a5..f8d61c2 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -19,11 +19,13 @@ 
 
 #include "libxl_internal.h"
 #include "libxl_arch.h"
+#include "libacpi/libacpi.h"
 
 #include <xc_dom.h>
 #include <xen/hvm/hvm_info_table.h>
 #include <xen/hvm/hvm_xs_strings.h>
 #include <xen/hvm/e820.h>
+#include <xen/viommu.h>
 
 #include "_paths.h"
 
@@ -925,6 +927,43 @@  out:
     return rc;
 }
 
+static unsigned long acpi_v2p(struct acpi_ctxt *ctxt, void *v)
+{
+    return (unsigned long)v;
+}
+
+static void *acpi_mem_alloc(struct acpi_ctxt *ctxt,
+                            uint32_t size, uint32_t align)
+{
+    return aligned_alloc(align, size);
+}
+
+static void acpi_mem_free(struct acpi_ctxt *ctxt,
+                          void *v, uint32_t size)
+{
+    /* ACPI builder currently doesn't free memory so this is just a stub */
+}
+
+static int libxl__acpi_build_dmar(libxl__gc *gc,
+                                  struct acpi_config *config,
+                                  void **data_r, int *datalen_r)
+{
+    struct acpi_ctxt ctxt;
+    void *table;
+
+    ctxt.mem_ops.alloc = acpi_mem_alloc;
+    ctxt.mem_ops.free = acpi_mem_free;
+    ctxt.mem_ops.v2p = acpi_v2p;
+
+    table = construct_dmar(&ctxt, config);
+    if ( !table )
+        return ERROR_FAIL;
+
+    *data_r = table;
+    *datalen_r = acpi_get_table_size((struct acpi_header *)table);
+    return 0;
+}
+
 static int libxl__domain_firmware(libxl__gc *gc,
                                   libxl_domain_build_info *info,
                                   struct xc_dom_image *dom)
@@ -1045,6 +1084,54 @@  static int libxl__domain_firmware(libxl__gc *gc,
         }
     }
 
+    /* build DMAR table according guest configuration and joint it with other
+     * apci tables specified by acpi_modules */
+    if ((info->u.hvm.viommu.type == VIOMMU_TYPE_INTEL_VTD) &&
+        !libxl_defbool_is_default(info->u.hvm.viommu.intremap) &&
+        info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
+        struct acpi_config config;
+
+        memset(&config, 0, sizeof(config));
+        if (libxl_defbool_val(info->u.hvm.viommu.intremap)) {
+            config.table_flags |= ACPI_HAS_DMAR;
+            config.dmar_flag = DMAR_INTR_REMAP;
+            if (!libxl_defbool_is_default(info->u.hvm.viommu.x2apic)
+                && !libxl_defbool_val(info->u.hvm.viommu.x2apic))
+                config.dmar_flag |= DMAR_X2APIC_OPT_OUT;
+
+            config.viommu_base_addr = info->u.hvm.viommu.base_addr;
+            data = NULL;
+            e = libxl__acpi_build_dmar(gc, &config, &data, &datalen);
+            if (e) {
+                LOGE(ERROR, "failed to build DMAR table");
+                rc = ERROR_FAIL;
+                goto out;
+            }
+
+            libxl__ptr_add(gc, data);
+            if (datalen) {
+                if (!dom->acpi_modules[0].data) {
+                    dom->acpi_modules[0].data = data;
+                    dom->acpi_modules[0].length = (uint32_t)datalen;
+                } else {
+                    /* joint tables */
+                    void *newdata;
+                    newdata = malloc(datalen + dom->acpi_modules[0].length);
+                    if (!newdata) {
+                        LOGE(ERROR, "failed to joint DMAR table to acpi modules");
+                        rc = ERROR_FAIL;
+                        goto out;
+                    }
+                    memcpy(newdata, dom->acpi_modules[0].data,
+                           dom->acpi_modules[0].length);
+                    memcpy(newdata + dom->acpi_modules[0].length, data, datalen);
+                    dom->acpi_modules[0].data = newdata;
+                    dom->acpi_modules[0].length += (uint32_t)datalen;
+                }
+            }
+        }
+    }
+
     return 0;
 out:
     assert(rc != 0);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 2204425..93e9e2c 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -450,6 +450,15 @@  libxl_altp2m_mode = Enumeration("altp2m_mode", [
     (3, "limited"),
     ], init_val = "LIBXL_ALTP2M_MODE_DISABLED")
 
+libxl_viommu_info = Struct("viommu_info", [
+    ("type",            uint64),
+    ("intremap",        libxl_defbool),
+    ("x2apic",          libxl_defbool),
+    ("cap",             uint64),
+    ("base_addr",       uint64),
+    ("length",          uint64),
+    ])
+
 libxl_domain_build_info = Struct("domain_build_info",[
     ("max_vcpus",       integer),
     ("avail_vcpus",     libxl_bitmap),
@@ -564,6 +573,7 @@  libxl_domain_build_info = Struct("domain_build_info",[
                                        ("serial_list",      libxl_string_list),
                                        ("rdm", libxl_rdm_reserve),
                                        ("rdm_mem_boundary_memkb", MemKB),
+                                       ("viommu",           libxl_viommu_info),
                                        ])),
                  ("pv", Struct(None, [("kernel", string),
                                       ("slack_memkb", MemKB),
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 856a304..584d805 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -18,6 +18,7 @@ 
 #include <stdio.h>
 #include <stdlib.h>
 #include <xen/hvm/e820.h>
+#include <xen/viommu.h>
 
 #include <libxl.h>
 #include <libxl_utils.h>
@@ -29,6 +30,9 @@ 
 
 extern void set_default_nic_values(libxl_device_nic *nic);
 
+#define VIOMMU_BASE_ADDR 0xfed90000UL
+#define VIOMMU_REGISTER_LEN 0x1000UL
+
 #define ARRAY_EXTEND_INIT__CORE(array,count,initfn,more)                \
     ({                                                                  \
         typeof((count)) array_extend_old_count = (count);               \
@@ -803,6 +807,32 @@  int parse_usbdev_config(libxl_device_usbdev *usbdev, char *token)
     return 0;
 }
 
+/* Parses viommu data and adds info into viommu
+ * Returns 1 if the input token does not match one of the keys
+ * or parsed values are not correct. Successful parse returns 0 */
+static int parse_viommu_config(libxl_viommu_info *viommu, char *token)
+{
+    char *oparg;
+
+    if (MATCH_OPTION("type", token, oparg)) {
+        if (!strcmp(oparg, "vtd")) {
+            viommu->type = VIOMMU_TYPE_INTEL_VTD;
+        } else {
+            fprintf(stderr, "Invalid viommu type: %s\n", oparg);
+            return 1;
+        }
+    } else if (MATCH_OPTION("intremap", token, oparg)) {
+        libxl_defbool_set(&viommu->intremap, !!strtoul(oparg, NULL, 0));
+    } else if (MATCH_OPTION("x2apic", token, oparg)) {
+        libxl_defbool_set(&viommu->x2apic, !!strtoul(oparg, NULL, 0));
+    } else {
+        fprintf(stderr, "Unknown string `%s' in viommu spec\n", token);
+        return 1;
+    }
+
+    return 0;
+}
+
 void parse_config_data(const char *config_source,
                        const char *config_data,
                        int config_len,
@@ -1182,6 +1212,40 @@  void parse_config_data(const char *config_source,
 
         if (!xlu_cfg_get_long (config, "rdm_mem_boundary", &l, 0))
             b_info->u.hvm.rdm_mem_boundary_memkb = l * 1024;
+
+        if (!xlu_cfg_get_string(config, "viommu", &buf, 0)) {
+            libxl_viommu_info viommu;
+            char *p, *str2;
+
+            str2 = strdup(buf);
+            if (!str2) {
+                fprintf(stderr, "ERROR: strdup failed\n");
+                exit (1);
+            }
+            p = strtok(str2, ",");
+            if (!p) {
+                fprintf(stderr, "ERROR: invalid viommu_info format\n");
+                exit (1);
+            }
+            do {
+                if (*p == ' ')
+                    p++;
+                if (parse_viommu_config(&viommu, p)) {
+                    fprintf(stderr, "ERROR: invalid viommu setting\n");
+                    exit (1);
+                }
+            } while ((p=strtok(NULL, ",")) != NULL);
+            free(str2);
+            b_info->u.hvm.viommu.type = viommu.type;
+            b_info->u.hvm.viommu.intremap = viommu.intremap;
+            b_info->u.hvm.viommu.x2apic = viommu.x2apic;
+            if ( libxl_defbool_val(b_info->u.hvm.viommu.intremap) )
+            {
+                b_info->u.hvm.viommu.cap = VIOMMU_CAP_IRQ_REMAPPING;
+                b_info->u.hvm.viommu.base_addr = VIOMMU_BASE_ADDR;
+                b_info->u.hvm.viommu.length = VIOMMU_REGISTER_LEN;
+            }
+        }
         break;
     case LIBXL_DOMAIN_TYPE_PV:
     {