[Xen-devel] [PATCH v9 2/6] tools/(lib)xl: Add partial device tree support for ARM

[Julien Grall <julien.grall@xxxxxxxxxx>]

From: Julien Grall <julien.grall@xxxxxxxxxx>

Allow the user to pass additional nodes to the guest device tree. For
this purpose, everything in the node /passthrough from the partial
device tree will be copied into the guest device tree.

The node /aliases will be also copied to allow the user to define
aliases which can be used by the guest kernel.

A simple partial device tree will look like:

/dts-v1/;

/ {
        #address-cells = <2>;
        #size-cells = <2>;

        passthrough {
            compatible = "simple-bus";
            ranges;
            #address-cells = <2>;
            #size-cells = <2>;

            /* List of your nodes */
        }
};

Note that:
    * The interrupt-parent property will be added by the toolstack in
    the root node
    * The properties compatible, ranges, #address-cells and #size-cells
    in /passthrough are mandatory.

The helpers provided by the libfdt don't perform all the necessary
security check on a given device tree. Therefore, only trusted device
tree should be used.

Note: The partial device tree code requires the presence of libfdt
functions which have been only correctly exported in libfdt 1.4.0 and
higher. All the major distributions but Debian Wheezy are using v1.4.0
or higher. It has been decided to disable partial device tree support on
OSes where libfdt doesn't meet the requirement.

Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Wei Liu <wei.liu2@xxxxxxxxxx>

---
    An example of the partial device tree, as long as how to passthrough
    a non-pci device will be added to the tree in a follow-up patch.

    A new LIBXL_HAVE_* will be added in the patch which add support for
    non-PCI passthrough as both are tight.

    Changes in v9:
        - Drop ack from the 2 Ian's due to build changes
        - Only enable partial device tree when libfdt > 1.4.0 is present

    Changes in v7:
        - Drop declaration of fdt_{first,next}_subnode. They are
        correctly define in libxl_libfdt_compat.h if necessary

    Changes in v6:
        - Fix grammar in the commit message
        - Spelling mistake the IDL
        - Add Ian J. and Ian C.'s ack

    Changes in v5:
        - Add a warning in the IDL
        - Remove the requirement to use only the version 17 of the FDT
        format.

    Changes in v4:
        - Mark the option as unsafe
        - The _fdt_* helpers has been moved in a separate patch/file.
        Only the prototype is declared
        - The partial DT is considered valid. Remove some security check
        which make the code cleaner
        - Typoes

    Changes in v3:
        - Patch added
---
 docs/man/xl.cfg.pod.5       |  10 +++
 tools/configure.ac          |  12 +++
 tools/libxl/libxl_arm.c     | 173 ++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_types.idl |   4 +
 tools/libxl/xl_cmdimpl.c    |   1 +
 5 files changed, 200 insertions(+)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 8e4154f..ead8a5c 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -460,6 +460,16 @@ not emulated.
 Specify that this domain is a driver domain. This enables certain
 features needed in order to run a driver domain.
 
+=item B<device_tree=PATH>
+
+Specify a partial device tree (compiled via the Device Tree Compiler).
+Everything under the node "/passthrough" will be copied into the guest
+device tree. For convenience, the node "/aliases" is also copied to allow
+the user to defined aliases which can be used by the guest kernel.
+
+Given the complexity of verifying the validity of a device tree, this
+option should only be used with trusted device tree.
+
 =back
 
 =head2 Devices
diff --git a/tools/configure.ac b/tools/configure.ac
index 5b48ab2..58b6caa 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -356,6 +356,18 @@ case "$host_cpu" in
 arm*|aarch64)
 AC_CHECK_LIB([fdt], [fdt_create], [], [AC_MSG_ERROR([Could not find libfdt])])
 
+# Check for libfdt >= 1.4.0. If present enable passthrough
+# Note that libfdt doesn't provide versionning. So we need to rely on
+# function present in new version.
+# Use fdt_first_property_offset which has been correctly exported since v1.4.0
+AC_CHECK_FUNC(fdt_first_property_offset, [partial_dt="y"], [partial_dt="n"])
+
+AS_IF([test "x$partial_dt" = "xy" ],
+      [AC_DEFINE([ENABLE_PARTIAL_DEVICE_TREE], [1],
+                 [Enabling support partial device tree in libxl])],
+      [AC_MSG_WARN([Disabling support for partial device tree in libxl.
+       Please install libfdt library - version 1.4.0 or higher])])
+
 # The functions fdt_{first,next}_subnode may not be available because:
 #   * It has been introduced in 2013 => Doesn't work on Wheezy
 #   * The prototype exists but the functions are not exposed. Don't ask why...
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index feded58..71dbb9e 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -1,5 +1,6 @@
 #include "libxl_internal.h"
 #include "libxl_arch.h"
+#include "libxl_libfdt_compat.h"
 
 #include <xc_dom.h>
 #include <stdbool.h>
@@ -542,6 +543,157 @@ out:
     }
 }
 
+#ifdef ENABLE_PARTIAL_DEVICE_TREE
+
+static int check_partial_fdt(libxl__gc *gc, void *fdt, size_t size)
+{
+    int r;
+
+    if (fdt_magic(fdt) != FDT_MAGIC) {
+        LOG(ERROR, "Partial FDT is not a valid Flat Device Tree");
+        return ERROR_FAIL;
+    }
+
+    r = fdt_check_header(fdt);
+    if (r) {
+        LOG(ERROR, "Failed to check the partial FDT (%d)", r);
+        return ERROR_FAIL;
+    }
+
+    if (fdt_totalsize(fdt) > size) {
+        LOG(ERROR, "Partial FDT totalsize is too big");
+        return ERROR_FAIL;
+    }
+
+    return 0;
+}
+
+static int copy_properties(libxl__gc *gc, void *fdt, void *pfdt,
+                           int nodeoff)
+{
+    int propoff, nameoff, r;
+    const struct fdt_property *prop;
+
+    for (propoff = fdt_first_property_offset(pfdt, nodeoff);
+         propoff >= 0;
+         propoff = fdt_next_property_offset(pfdt, propoff)) {
+
+        if (!(prop = fdt_get_property_by_offset(pfdt, propoff, NULL))) {
+            return -FDT_ERR_INTERNAL;
+        }
+
+        nameoff = fdt32_to_cpu(prop->nameoff);
+        r = fdt_property(fdt, fdt_string(pfdt, nameoff),
+                         prop->data, fdt32_to_cpu(prop->len));
+        if (r) return r;
+    }
+
+    /* FDT_ERR_NOTFOUND => There is no more properties for this node */
+    return (propoff != -FDT_ERR_NOTFOUND)? propoff : 0;
+}
+
+/* Copy a node from the partial device tree to the guest device tree */
+static int copy_node(libxl__gc *gc, void *fdt, void *pfdt,
+                     int nodeoff, int depth)
+{
+    int r;
+
+    r = fdt_begin_node(fdt, fdt_get_name(pfdt, nodeoff, NULL));
+    if (r) return r;
+
+    r = copy_properties(gc, fdt, pfdt, nodeoff);
+    if (r) return r;
+
+    for (nodeoff = fdt_first_subnode(pfdt, nodeoff);
+         nodeoff >= 0;
+         nodeoff = fdt_next_subnode(pfdt, nodeoff)) {
+        r = copy_node(gc, fdt, pfdt, nodeoff, depth + 1);
+        if (r) return r;
+    }
+
+    if (nodeoff != -FDT_ERR_NOTFOUND)
+        return nodeoff;
+
+    r = fdt_end_node(fdt);
+    if (r) return r;
+
+    return 0;
+}
+
+static int copy_node_by_path(libxl__gc *gc, const char *path,
+                             void *fdt, void *pfdt)
+{
+    int nodeoff, r;
+    const char *name = strrchr(path, '/');
+
+    if (!name)
+        return -FDT_ERR_INTERNAL;
+
+    name++;
+
+    /*
+     * The FDT function to look at a node doesn't take into account the
+     * unit (i.e anything after @) when search by name. Check if the
+     * name exactly matches.
+     */
+    nodeoff = fdt_path_offset(pfdt, path);
+    if (nodeoff < 0)
+        return nodeoff;
+
+    if (strcmp(fdt_get_name(pfdt, nodeoff, NULL), name))
+        return -FDT_ERR_NOTFOUND;
+
+    r = copy_node(gc, fdt, pfdt, nodeoff, 0);
+    if (r) return r;
+
+    return 0;
+}
+
+/*
+ * The partial device tree is not copied entirely. Only the relevant bits are
+ * copied to the guest device tree:
+ *  - /passthrough node
+ *  - /aliases node
+ */
+static int copy_partial_fdt(libxl__gc *gc, void *fdt, void *pfdt)
+{
+    int r;
+
+    r = copy_node_by_path(gc, "/passthrough", fdt, pfdt);
+    if (r < 0) {
+        LOG(ERROR, "Can't copy the node \"/passthrough\" from the partial 
FDT");
+        return r;
+    }
+
+    r = copy_node_by_path(gc, "/aliases", fdt, pfdt);
+    if (r < 0 && r != -FDT_ERR_NOTFOUND) {
+        LOG(ERROR, "Can't copy the node \"/aliases\" from the partial FDT");
+        return r;
+    }
+
+    return 0;
+}
+
+#else
+
+static int check_partial_fdt(libxl__gc *gc, void *fdt, size_t size)
+{
+    LOG(ERROR, "partial device tree not supported");
+
+    return ERROR_FAIL;
+}
+
+static int copy_partial_fdt(libxl__gc *gc, void *fdt, void *pfdt)
+{
+    /*
+     * We should never be here when the partial device tree is not
+     * supported.
+     * */
+    return -FDT_ERR_INTERNAL;
+}
+
+#endif /* ENABLE_PARTIAL_DEVICE_TREE */
+
 #define FDT_MAX_SIZE (1<<20)
 
 int libxl__arch_domain_init_hw_description(libxl__gc *gc,
@@ -550,8 +702,10 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
                                            struct xc_dom_image *dom)
 {
     void *fdt = NULL;
+    void *pfdt = NULL;
     int rc, res;
     size_t fdt_size = 0;
+    int pfdt_size = 0;
 
     const libxl_version_info *vers;
     const struct arch_info *ainfo;
@@ -571,6 +725,22 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
         vers->xen_version_major, vers->xen_version_minor);
     LOG(DEBUG, " - vGIC version: %s", gicv_to_string(xc_config->gic_version));
 
+    if (info->device_tree) {
+        LOG(DEBUG, " - Partial device tree provided: %s", info->device_tree);
+
+        rc = libxl_read_file_contents(CTX, info->device_tree,
+                                      &pfdt, &pfdt_size);
+        if (rc) {
+            LOGEV(ERROR, rc, "failed to read the partial device file %s",
+                  info->device_tree);
+            return ERROR_FAIL;
+        }
+        libxl__ptr_add(gc, pfdt);
+
+        if (check_partial_fdt(gc, pfdt, pfdt_size))
+            return ERROR_FAIL;
+    }
+
 /*
  * Call "call" handling FDT_ERR_*. Will either:
  * - loop back to retry_resize
@@ -637,6 +807,9 @@ next_resize:
         FDT( make_timer_node(gc, fdt, ainfo) );
         FDT( make_hypervisor_node(gc, fdt, vers) );
 
+        if (pfdt)
+            FDT( copy_partial_fdt(gc, fdt, pfdt) );
+
         FDT( fdt_end_node(fdt) );
 
         FDT( fdt_finish(fdt) );
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 65d479f..4ea1290 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -413,6 +413,10 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("kernel",           string),
     ("cmdline",          string),
     ("ramdisk",          string),
+    # Given the complexity of verifying the validity of a device tree,
+    # libxl doesn't do any security check on it. It's the responsibility
+    # of the caller to provide only trusted device tree.
+    ("device_tree",      string),
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 373aa37..2125a09 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1379,6 +1379,7 @@ static void parse_config_data(const char *config_source,
 
     xlu_cfg_replace_string (config, "kernel", &b_info->kernel, 0);
     xlu_cfg_replace_string (config, "ramdisk", &b_info->ramdisk, 0);
+    xlu_cfg_replace_string (config, "device_tree", &b_info->device_tree, 0);
     b_info->cmdline = parse_cmdline(config);
 
     xlu_cfg_get_defbool(config, "driver_domain", &c_info->driver_domain, 0);
-- 
2.1.4


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