All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/6] xen/arm: Add support for non-PCI passthrough
@ 2015-05-12 14:33 Julien Grall
  2015-05-12 14:33 ` [PATCH v8 1/6] tools/libxl: Check if fdt_{first, next}_subnode are present in libfdt Julien Grall
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Julien Grall @ 2015-05-12 14:33 UTC (permalink / raw)
  To: xen-devel
  Cc: Julien Grall, stefano.stabellini, tim, ian.campbell, Andrii Anisov

Hello all,

This a resend of the last part of non-PCI passthrough after Andrii spotted
that I forgot to include the new header libxl_libfdt_compat.h.

I hope this will be the last build error found.

Regards,

Cc: Andrii Anisov <andrii.anisov@globallogic.com>

Julien Grall (6):
  tools/libxl: Check if fdt_{first,next}_subnode are present in libfdt
  tools/(lib)xl: Add partial device tree support for ARM
  tools/libxl: arm: Use an higher value for the GIC phandle
  libxl: Add support for Device Tree passthrough
  xl: Add new option dtdev
  docs/misc: arm: Add documentation about Device Tree passthrough

 docs/man/xl.cfg.pod.5             |  18 +++++
 docs/misc/arm/passthrough.txt     |  63 +++++++++++++++
 tools/configure.ac                |   6 ++
 tools/libxl/Makefile              |   2 +-
 tools/libxl/libxl.h               |   7 ++
 tools/libxl/libxl_arm.c           | 160 +++++++++++++++++++++++++++++++++++++-
 tools/libxl/libxl_create.c        |  32 ++++++++
 tools/libxl/libxl_internal.h      |   6 ++
 tools/libxl/libxl_libfdt_compat.c |  94 ++++++++++++++++++++++
 tools/libxl/libxl_libfdt_compat.h |  80 +++++++++++++++++++
 tools/libxl/libxl_types.idl       |  11 +++
 tools/libxl/xl_cmdimpl.c          |  23 +++++-
 12 files changed, 496 insertions(+), 6 deletions(-)
 create mode 100644 docs/misc/arm/passthrough.txt
 create mode 100644 tools/libxl/libxl_libfdt_compat.c
 create mode 100644 tools/libxl/libxl_libfdt_compat.h

-- 
2.1.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v8 1/6] tools/libxl: Check if fdt_{first, next}_subnode are present in libfdt
  2015-05-12 14:33 [PATCH v8 0/6] xen/arm: Add support for non-PCI passthrough Julien Grall
@ 2015-05-12 14:33 ` Julien Grall
  2015-05-13 14:07   ` Ian Campbell
  2015-05-12 14:33 ` [PATCH v8 2/6] tools/(lib)xl: Add partial device tree support for ARM Julien Grall
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Julien Grall @ 2015-05-12 14:33 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, ian.campbell, tim, Julien Grall, Ian Jackson,
	stefano.stabellini

From: Julien Grall <julien.grall@linaro.org>

The functions fdt_{fisrt,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...

The later has been fixed recently in the dtc repo [1]

When the functions are not available, implement our own in order to use
them in a following patch.

[1] git://git.kernel.org/pub/scm/utils/dtc/dtc.git
    commit a4b093f7366fdb429ca1781144d3985fa50d0fbb

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>

---
    tools/configure needs to be regenerate as this patch is modifying
    tools/configure.ac

    Changes in v8:
        - Forgot to add the new header file libxl_libfdt_compat.h on the
        previous version

    Changes in v7:
        - Drop Ian's ack as he request some changes due to build issue
        - Check if the declaration are present if not add declaration
        - Remove changes config.h.in as it will be regenerate before
        committed

    Changes in v6:
        - Add Ian's Ack

    Changes in v5:
        - Add Ian's Signed-off-by for the license part and update the
        license
        - Rename libxl_fdt to libxl_libfdt_compat.c

    Changes in v4:
        - Patch added
---
 tools/configure.ac                |  6 +++
 tools/libxl/Makefile              |  2 +-
 tools/libxl/libxl_internal.h      |  1 +
 tools/libxl/libxl_libfdt_compat.c | 94 +++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_libfdt_compat.h | 80 +++++++++++++++++++++++++++++++++
 5 files changed, 182 insertions(+), 1 deletion(-)
 create mode 100644 tools/libxl/libxl_libfdt_compat.c
 create mode 100644 tools/libxl/libxl_libfdt_compat.h

diff --git a/tools/configure.ac b/tools/configure.ac
index d31c2f3..5b48ab2 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -355,6 +355,12 @@ AC_SUBST(libiconv)
 case "$host_cpu" in
 arm*|aarch64)
 AC_CHECK_LIB([fdt], [fdt_create], [], [AC_MSG_ERROR([Could not find libfdt])])
+
+# 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...
+AC_CHECK_FUNCS([fdt_first_subnode fdt_next_subnode])
+AC_CHECK_DECLS([fdt_first_subnode, fdt_next_subnode],,,[#include <libfdt.h>])
 esac
 
 # Checks for header files.
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 1b16598..2afb146 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -59,7 +59,7 @@ endif
 LIBXL_OBJS-y += libxl_remus_device.o libxl_remus_disk_drbd.o
 
 LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o
-LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o
+LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o
 
 ifeq ($(CONFIG_NetBSD),y)
 LIBXL_OBJS-y += libxl_netbsd.o
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8eb38aa..857abd4 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3692,6 +3692,7 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc,
  */
 void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
                                     const libxl_bitmap *sptr);
+
 #endif
 
 /*
diff --git a/tools/libxl/libxl_libfdt_compat.c b/tools/libxl/libxl_libfdt_compat.c
new file mode 100644
index 0000000..a1823b2
--- /dev/null
+++ b/tools/libxl/libxl_libfdt_compat.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (C) 2006 David Gibson, IBM Corporation.
+ *
+ * This file is part of libxl, and was originally taken from libfdt.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * Additionally, this particular file is dual licensed.  That is,
+ * alternatively, at your option:
+ *
+ *      Redistribution and use in source and binary forms, with or
+ *      without modification, are permitted provided that the following
+ *      conditions are met:
+ *
+ *      1. Redistributions of source code must retain the above
+ *         copyright notice, this list of conditions and the following
+ *         disclaimer.
+ *      2. Redistributions in binary form must reproduce the above
+ *         copyright notice, this list of conditions and the following
+ *         disclaimer in the documentation and/or other materials
+ *         provided with the distribution.
+ *
+ *      THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ *      CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ *      INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ *      MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ *      DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ *      CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *      SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ *      NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ *      LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ *      HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ *      CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ *      OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
+ *      EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * Note that this applies only to this file, and other files with a
+ * similar notice.  Also, note that when the same code is distributed
+ * along with the rest of libxl, you must comply with the terms of the
+ * LGPLv2.1 for the whole of libxl including this file.
+ *
+ * The intent is to permit, in particular, upstream libfdt to
+ * incorporate improvements to this file within upstream libfdt.  At
+ * the time of writing, upstream libfdt is dual licensed: 2-clause BSD
+ * (as above) and GPLv2-or-later.  The 2-clause BSD licence is
+ * compatible with both GPLv2-or-later and LGPLv2.1-only; this permits
+ * copying in both directions, and the optional licence upgrade to a
+ * copyleft licence by libdft upstream or the Xen Project,
+ * respectively.
+ */
+
+#include <libfdt.h>
+
+#include "libxl_libfdt_compat.h"
+
+#ifndef HAVE_FDT_FIRST_SUBNODE
+int fdt_first_subnode(const void *fdt, int offset)
+{
+	int depth = 0;
+
+	offset = fdt_next_node(fdt, offset, &depth);
+	if (offset < 0 || depth != 1)
+		return -FDT_ERR_NOTFOUND;
+
+	return offset;
+}
+#endif
+
+#ifndef HAVE_FDT_NEXT_SUBNODE
+int fdt_next_subnode(const void *fdt, int offset)
+{
+	int depth = 1;
+
+	/*
+	 * With respect to the parent, the depth of the next subnode will be
+	 * the same as the last.
+	 */
+	do {
+		offset = fdt_next_node(fdt, offset, &depth);
+		if (offset < 0 || depth < 1)
+			return -FDT_ERR_NOTFOUND;
+	} while (depth > 1);
+
+	return offset;
+}
+#endif
diff --git a/tools/libxl/libxl_libfdt_compat.h b/tools/libxl/libxl_libfdt_compat.h
new file mode 100644
index 0000000..cf414f5
--- /dev/null
+++ b/tools/libxl/libxl_libfdt_compat.h
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2006 David Gibson, IBM Corporation.
+ *
+ * This file is part of libxl, and was originally taken from libfdt.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * Additionally, this particular file is dual licensed.  That is,
+ * alternatively, at your option:
+ *
+ *      Redistribution and use in source and binary forms, with or
+ *      without modification, are permitted provided that the following
+ *      conditions are met:
+ *
+ *      1. Redistributions of source code must retain the above
+ *         copyright notice, this list of conditions and the following
+ *         disclaimer.
+ *      2. Redistributions in binary form must reproduce the above
+ *         copyright notice, this list of conditions and the following
+ *         disclaimer in the documentation and/or other materials
+ *         provided with the distribution.
+ *
+ *      THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ *      CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ *      INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ *      MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ *      DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ *      CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *      SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ *      NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ *      LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ *      HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ *      CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ *      OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
+ *      EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * Note that this applies only to this file, and other files with a
+ * similar notice.  Also, note that when the same code is distributed
+ * along with the rest of libxl, you must comply with the terms of the
+ * LGPLv2.1 for the whole of libxl including this file.
+ *
+ * The intent is to permit, in particular, upstream libfdt to
+ * incorporate improvements to this file within upstream libfdt.  At
+ * the time of writing, upstream libfdt is dual licensed: 2-clause BSD
+ * (as above) and GPLv2-or-later.  The 2-clause BSD licence is
+ * compatible with both GPLv2-or-later and LGPLv2.1-only; this permits
+ * copying in both directions, and the optional licence upgrade to a
+ * copyleft licence by libdft upstream or the Xen Project,
+ * respectively.
+ */
+
+#ifndef LIBXL_LIBFDT_COMPAT_H
+#define LIBXL_LIBFDT_COMPAT_H
+
+#if !HAVE_DECL_FDT_FIRST_SUBNODE
+int fdt_first_subnode(const void *fdt, int offset);
+#endif
+
+#if !HAVE_DECL_FDT_NEXT_SUBNODE
+int fdt_next_subnode(const void *fdt, int offset);
+#endif
+
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
+
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 2/6] tools/(lib)xl: Add partial device tree support for ARM
  2015-05-12 14:33 [PATCH v8 0/6] xen/arm: Add support for non-PCI passthrough Julien Grall
  2015-05-12 14:33 ` [PATCH v8 1/6] tools/libxl: Check if fdt_{first, next}_subnode are present in libfdt Julien Grall
@ 2015-05-12 14:33 ` Julien Grall
  2015-05-12 14:33 ` [PATCH v8 3/6] tools/libxl: arm: Use an higher value for the GIC phandle Julien Grall
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-05-12 14:33 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, ian.campbell, tim, Julien Grall, Ian Jackson,
	stefano.stabellini

From: Julien Grall <julien.grall@linaro.org>

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.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>

---
    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 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/libxl/libxl_arm.c     | 151 ++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_types.idl |   4 ++
 tools/libxl/xl_cmdimpl.c    |   1 +
 4 files changed, 166 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/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index feded58..6ad8b21 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,135 @@ out:
     }
 }
 
+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;
+}
+
 #define FDT_MAX_SIZE (1<<20)
 
 int libxl__arch_domain_init_hw_description(libxl__gc *gc,
@@ -550,8 +680,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 +703,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 +785,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 023b21e..2b57785 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 526a1f6..19bf103 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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 3/6] tools/libxl: arm: Use an higher value for the GIC phandle
  2015-05-12 14:33 [PATCH v8 0/6] xen/arm: Add support for non-PCI passthrough Julien Grall
  2015-05-12 14:33 ` [PATCH v8 1/6] tools/libxl: Check if fdt_{first, next}_subnode are present in libfdt Julien Grall
  2015-05-12 14:33 ` [PATCH v8 2/6] tools/(lib)xl: Add partial device tree support for ARM Julien Grall
@ 2015-05-12 14:33 ` Julien Grall
  2015-05-12 14:33 ` [PATCH v8 4/6] libxl: Add support for Device Tree passthrough Julien Grall
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-05-12 14:33 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, ian.campbell, tim, Julien Grall, Ian Jackson,
	stefano.stabellini

From: Julien Grall <julien.grall@linaro.org>

The partial device tree may contains phandle. The Device Tree Compiler
tends to allocate the phandle from 1.

Reserve the ID 65000 for the GIC phandle. I think we can safely assume
that the partial device tree will never contain a such ID.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>

---
    To allocate dynamically the phandle, we would need to fill in
    post-hoc (like we do with e.g the initramfs location) the
    #interrupt-parent in "/". That would also require some refactoring
    in the code to pass the phandle every time.

    Defer this solution to a follow-up in order as having 65000 would be
    very unlikely.

    Changes in v6:
        - Add a note in the doc about the reserved phandle

    Changes in v5:
        - Add Ian C.'s Ack.

    Changes in v3:
        - Patch added
---
 docs/man/xl.cfg.pod.5       | 3 +++
 tools/libxl/libxl_arm.c     | 9 +++++----
 tools/libxl/libxl_types.idl | 2 ++
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index ead8a5c..fb8721f 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -470,6 +470,9 @@ 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.
 
+Note that the partial device tree should avoid to use the phandle 65000
+which is reserved by the toolstack.
+
 =back
 
 =head2 Devices
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 6ad8b21..e144535 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -81,10 +81,11 @@ static struct arch_info {
     {"xen-3.0-aarch64", "arm,armv8-timer", "arm,armv8" },
 };
 
-enum {
-    PHANDLE_NONE = 0,
-    PHANDLE_GIC,
-};
+/*
+ * The device tree compiler (DTC) is allocating the phandle from 1 to
+ * onwards. Reserve a high value for the GIC phandle.
+ */
+#define PHANDLE_GIC (65000)
 
 typedef uint32_t be32;
 typedef be32 gic_interrupt[3];
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 2b57785..3254617 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -416,6 +416,8 @@ libxl_domain_build_info = Struct("domain_build_info",[
     # 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.
+    # Note that the partial device tree should avoid to use the phandle
+    # 65000 which is reserved by the toolstack.
     ("device_tree",      string),
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 4/6] libxl: Add support for Device Tree passthrough
  2015-05-12 14:33 [PATCH v8 0/6] xen/arm: Add support for non-PCI passthrough Julien Grall
                   ` (2 preceding siblings ...)
  2015-05-12 14:33 ` [PATCH v8 3/6] tools/libxl: arm: Use an higher value for the GIC phandle Julien Grall
@ 2015-05-12 14:33 ` Julien Grall
  2015-05-12 14:33 ` [PATCH v8 5/6] xl: Add new option dtdev Julien Grall
  2015-05-12 14:33 ` [PATCH v8 6/6] docs/misc: arm: Add documentation about Device Tree passthrough Julien Grall
  5 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-05-12 14:33 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, ian.campbell, tim, Julien Grall, Ian Jackson,
	stefano.stabellini

From: Julien Grall <julien.grall@linaro.org>

On ARM, every non-PCI device are described in the device tree. Each of
them can be found via a path.

This patch introduces a very basic support, only the IOMMU will be set
up correctly. The user will have to:
    - Describe the device in the partial device tree
    - Map manually MMIO/IRQ

This is a first approach, that will allow to have a basic Device Tree
passthrough support in Xen. This could be improved later.

Furthermore add LIBXL_HAVE_DEVICETREE_PASSTHROUGH to indicate we
support Device Tree passthrough and partial device tree (introduced by a
previous patch).

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>

---
    Changes in v6:
        - Add Ian C. and Ian J.'s ack

    Changes in v5:
        - Replace some "non-PCI" to "Device Tree"

    Changes in v4:
        - Add LIBXL_HAVE_DEVICTREE_PASSTHROUGH to indicate we support
        non-PCI passthrough. This is also used in order to indicate
        partial device tree is supported
        - Remove libxl_dtdev.c as it contains only a 2 lines functions
        and call directly xc_* from libxl_create.c
        - Introduce domcreate_attach_dtdev

    Changes in v3:
        - Dynamic allocation has been dropped
        - Rework the commit message in accordance with the previous
        item

    Changes in v2:
        - Get DT infos earlier
        - Allocate/map IRQ in libxl__arch_domain_create rather than in
        libxl__device_dt_add
        - Use VIRQ rather than the PIRQ to construct the interrupts
        properties of the device tree
        - Correct cpumask in make_dtdev_node. We allow the interrupt to
        be used on the 8 CPUs
        - Fix LOGE when we map the MMIO region in the guest in
        libxl__device_dt_add. The domain and the IRQ were inverted
        - Calculate the number of SPIs to configure the VGIC
        - xc_physdev_dtdev_* helpers has been renamed to xc_dtdev_*
        - Rename libxl_device_dt to libxl_device_dtdev
---
 tools/libxl/libxl.h          |  7 +++++++
 tools/libxl/libxl_create.c   | 32 ++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h |  5 +++++
 tools/libxl/libxl_types.idl  |  5 +++++
 4 files changed, 49 insertions(+)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index efc0617..4aab751 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -192,6 +192,13 @@
  * is not present, instead of ERROR_INVAL.
  */
 #define LIBXL_HAVE_ERROR_DOMAIN_NOTFOUND 1
+
+/*
+ * libxl_domain_build_info has device_tree and libxl_device_dtdev
+ * exists. This mean Device Tree passthrough is supported for ARM
+ */
+#define LIBXL_HAVE_DEVICETREE_PASSTHROUGH 1
+
 /*
  * libxl ABI compatibility
  *
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f0da7dc..0a2359e 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -764,6 +764,8 @@ static void domcreate_attach_vtpms(libxl__egc *egc, libxl__multidev *multidev,
                                    int ret);
 static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *aodevs,
                                  int ret);
+static void domcreate_attach_dtdev(libxl__egc *egc,
+                                   libxl__domain_create_state *dcs);
 
 static void domcreate_console_available(libxl__egc *egc,
                                         libxl__domain_create_state *dcs);
@@ -1457,6 +1459,36 @@ static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev,
         }
     }
 
+    domcreate_attach_dtdev(egc, dcs);
+    return;
+
+error_out:
+    assert(ret);
+    domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_attach_dtdev(libxl__egc *egc,
+                                   libxl__domain_create_state *dcs)
+{
+    STATE_AO_GC(dcs->ao);
+    int i;
+    int ret;
+    int domid = dcs->guest_domid;
+
+    /* convenience aliases */
+    libxl_domain_config *const d_config = dcs->guest_config;
+
+    for (i = 0; i < d_config->num_dtdevs; i++) {
+        const libxl_device_dtdev *dtdev = &d_config->dtdevs[i];
+
+        LOG(DEBUG, "Assign device \"%s\" to dom%u", dtdev->path, domid);
+        ret = xc_assign_dt_device(CTX->xch, domid, dtdev->path);
+        if (ret < 0) {
+            LOG(ERROR, "xc_assign_dtdevice failed: %d\n", ret);
+            goto error_out;
+        }
+    }
+
     domcreate_console_available(egc, dcs);
 
     domcreate_complete(egc, dcs, 0);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 857abd4..dcb3f5c 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1203,6 +1203,11 @@ _hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
                                       libxl_device_pci *pcidev, int num);
 _hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
 
+/* from libxl_dtdev */
+
+_hidden int libxl__device_dt_add(libxl__gc *gc, uint32_t domid,
+                                 const libxl_device_dtdev *dtdev);
+
 /*----- xswait: wait for a xenstore node to be suitable -----*/
 
 typedef struct libxl__xswait_state libxl__xswait_state;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 3254617..8158691 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -541,6 +541,10 @@ libxl_device_pci = Struct("device_pci", [
     ("seize", bool),
     ])
 
+libxl_device_dtdev = Struct("device_dtdev", [
+    ("path", string),
+    ])
+
 libxl_device_vtpm = Struct("device_vtpm", [
     ("backend_domid",    libxl_domid),
     ("backend_domname",  string),
@@ -567,6 +571,7 @@ libxl_domain_config = Struct("domain_config", [
     ("disks", Array(libxl_device_disk, "num_disks")),
     ("nics", Array(libxl_device_nic, "num_nics")),
     ("pcidevs", Array(libxl_device_pci, "num_pcidevs")),
+    ("dtdevs", Array(libxl_device_dtdev, "num_dtdevs")),
     ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
     ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
     ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 5/6] xl: Add new option dtdev
  2015-05-12 14:33 [PATCH v8 0/6] xen/arm: Add support for non-PCI passthrough Julien Grall
                   ` (3 preceding siblings ...)
  2015-05-12 14:33 ` [PATCH v8 4/6] libxl: Add support for Device Tree passthrough Julien Grall
@ 2015-05-12 14:33 ` Julien Grall
  2015-05-12 14:33 ` [PATCH v8 6/6] docs/misc: arm: Add documentation about Device Tree passthrough Julien Grall
  5 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-05-12 14:33 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, ian.campbell, tim, Julien Grall, Ian Jackson,
	stefano.stabellini

From: Julien Grall <julien.grall@linaro.org>

The option "dtdev" will be used to passthrough a device described
in the device tree to a guest.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>

---
    Changes in v5:
        - Drop "non-PCI" in the commit message
        - Add Ian's ack

    Changes in v4:
        - Typoes in the documentation
        - Wrap the line in xl_cmdimpl.c

    Changes in v2:
        - libxl_device_dt has been rename to libxl_device_dtdev
        - use xrealloc instead of realloc
---
 docs/man/xl.cfg.pod.5    |  5 +++++
 tools/libxl/xl_cmdimpl.c | 22 +++++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index fb8721f..a189091 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -789,6 +789,11 @@ More information about Xen gfx_passthru feature is available
 on the XenVGAPassthrough L<http://wiki.xen.org/wiki/XenVGAPassthrough>
 wiki page.
 
+=item B<dtdev=[ "DTDEV_PATH", "DTDEV_PATH", ... ]>
+
+Specifies the host device tree nodes to passthrough to this guest. Each
+DTDEV_PATH is the absolute path in the device tree.
+
 =item B<ioports=[ "IOPORT_RANGE", "IOPORT_RANGE", ... ]>
 
 Allow guest to access specific legacy I/O ports. Each B<IOPORT_RANGE>
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 19bf103..51ab85a 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1169,7 +1169,7 @@ static void parse_config_data(const char *config_source,
     long l, vcpus = 0;
     XLU_Config *config;
     XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms;
-    XLU_ConfigList *channels, *ioports, *irqs, *iomem, *viridian;
+    XLU_ConfigList *channels, *ioports, *irqs, *iomem, *viridian, *dtdevs;
     int num_ioports, num_irqs, num_iomem, num_cpus, num_viridian;
     int pci_power_mgmt = 0;
     int pci_msitranslate = 0;
@@ -1941,6 +1941,26 @@ skip_vfb:
             libxl_defbool_set(&b_info->u.pv.e820_host, true);
     }
 
+    if (!xlu_cfg_get_list (config, "dtdev", &dtdevs, 0, 0)) {
+        d_config->num_dtdevs = 0;
+        d_config->dtdevs = NULL;
+        for (i = 0; (buf = xlu_cfg_get_listitem(dtdevs, i)) != NULL; i++) {
+            libxl_device_dtdev *dtdev;
+
+            d_config->dtdevs = xrealloc(d_config->dtdevs,
+                                        sizeof (libxl_device_dtdev) * (i + 1));
+            dtdev = d_config->dtdevs + d_config->num_dtdevs;
+            libxl_device_dtdev_init(dtdev);
+
+            dtdev->path = strdup(buf);
+            if (dtdev->path == NULL) {
+                fprintf(stderr, "unable to duplicate string for dtdevs\n");
+                exit(-1);
+            }
+            d_config->num_dtdevs++;
+        }
+    }
+
     switch (xlu_cfg_get_list(config, "cpuid", &cpuids, 0, 1)) {
     case 0:
         {
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 6/6] docs/misc: arm: Add documentation about Device Tree passthrough
  2015-05-12 14:33 [PATCH v8 0/6] xen/arm: Add support for non-PCI passthrough Julien Grall
                   ` (4 preceding siblings ...)
  2015-05-12 14:33 ` [PATCH v8 5/6] xl: Add new option dtdev Julien Grall
@ 2015-05-12 14:33 ` Julien Grall
  5 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-05-12 14:33 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

From: Julien Grall <julien.grall@linaro.org>

Note that the example is done on Midway whose SMMU driver is not
supported on Xen upstream.

Currently, I don't have other platform where I can test Device Tree
passthrough.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v6:
        - Typo in the doc
        - Add a link to http://www.devicetree.org/Device_Tree_Usage
        - Add Ian's ack

    Changes in v5:
        - Drop references to "non-PCI" in favor of "Device Tree"
        - Typoes and update the docs

    Changes in v4:
        - Patch added
---
 docs/misc/arm/passthrough.txt | 63 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 docs/misc/arm/passthrough.txt

diff --git a/docs/misc/arm/passthrough.txt b/docs/misc/arm/passthrough.txt
new file mode 100644
index 0000000..4a39649
--- /dev/null
+++ b/docs/misc/arm/passthrough.txt
@@ -0,0 +1,63 @@
+Passthrough a device described in the Device Tree to a guest
+============================================================
+
+The example will use the secondary network card for the midway server.
+
+1) Mark the device to let Xen know the device will be used for passthrough.
+This is done in the device tree node describing the device by adding the
+property "xen,passthrough". The command to do it in U-Boot is:
+
+    fdt set /soc/ethernet@fff51000 xen,passthrough
+
+2) Create a partial device tree describing the device. The IRQ are mapped
+1:1 to the guest (i.e VIRQ == IRQ). For MMIO, you will have to find a hole
+in the guest memory layout (see xen/include/public/arch-arm.h, note that
+the layout is not stable and can change between versions of Xen).
+
+/dts-v1/;
+
+/ {
+    /* #*cells are here to keep DTC happy */
+    #address-cells = <2>;
+    #size-cells = <2>;
+
+    aliases {
+        net = &mac0;
+    };
+
+    passthrough {
+        compatible = "simple-bus";
+        ranges;
+        #address-cells = <2>;
+        #size-cells = <2>;
+        mac0: ethernet@10000000 {
+            compatible = "calxeda,hb-xgmac";
+            reg = <0 0x10000000 0 0x1000>;
+            interrupts = <0 80 4  0 81 4  0 82 4>;
+        };
+    };
+};
+
+Note:
+    * The interrupt-parent property will be added by the toolstack in the
+    root node;
+    * The following properties are mandatory with the /passthrough node:
+        - compatible: It should always contain "simple-bus"
+        - ranges
+        - #address-cells
+        - #size-cells
+    * See http://www.devicetree.org/Device_Tree_Usage for more
+    information about device tree.
+
+3) Compile the partial guest device with dtc (Device Tree Compiler).
+For our purpose, the compiled file will be called guest-midway.dtb and
+placed in /root in DOM0.
+
+3) Add the following options in the guest configuration file:
+
+device_tree = "/root/guest-midway.dtb"
+dtdev = [ "/soc/ethernet@fff51000" ]
+irqs = [ 112, 113, 114 ]
+iomem = [ "0xfff51,1@0x10000" ]
+
+
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 1/6] tools/libxl: Check if fdt_{first, next}_subnode are present in libfdt
  2015-05-12 14:33 ` [PATCH v8 1/6] tools/libxl: Check if fdt_{first, next}_subnode are present in libfdt Julien Grall
@ 2015-05-13 14:07   ` Ian Campbell
  2015-05-13 15:22     ` Julien Grall
  0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2015-05-13 14:07 UTC (permalink / raw)
  To: Julien Grall
  Cc: Wei Liu, Ian Jackson, Julien Grall, tim, stefano.stabellini, xen-devel

On Tue, 2015-05-12 at 15:33 +0100, Julien Grall wrote:
> From: Julien Grall <julien.grall@linaro.org>
> 
> The functions fdt_{fisrt,next}_subnode may not be available because:

"first"

>     * It has been introduced in 2013 => Doesn't work on Wheezy
>     * The prototype exists but the functions are not exposed. Don't ask
>     why...
> 
> The later has been fixed recently in the dtc repo [1]
> 
> When the functions are not available, implement our own in order to use
> them in a following patch.
> 
> [1] git://git.kernel.org/pub/scm/utils/dtc/dtc.git
>     commit a4b093f7366fdb429ca1781144d3985fa50d0fbb
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>

I'm afraid this failed to build again this time with:

tools/libxl/libxenlight.so: undefined reference to `fdt_first_property_offset'
tools/libxl/libxenlight.so: undefined reference to `fdt_get_property_by_offset'
tools/libxl/libxenlight.so: undefined reference to `fdt_next_property_offset'

That's with arm32 == Debian Wheezy and arm64 == Ubuntu Saucy.

The arm32 machine is marilith-n13 here in Cambridge which you should
have access to.

arm64 is a chroot on my workstation, I could give you access if that
would be helpful, but I expect solving this for arm32 would solve it for
arm64 too.

>  tools/libxl/libxl_internal.h      |  1 +

Spurious change, please drop.

>  tools/libxl/libxl_libfdt_compat.c | 94 +++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxl_libfdt_compat.h | 80 +++++++++++++++++++++++++++++++++
>  5 files changed, 182 insertions(+), 1 deletion(-)
>  create mode 100644 tools/libxl/libxl_libfdt_compat.c
>  create mode 100644 tools/libxl/libxl_libfdt_compat.h
> 
> diff --git a/tools/configure.ac b/tools/configure.ac
> index d31c2f3..5b48ab2 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -355,6 +355,12 @@ AC_SUBST(libiconv)
>  case "$host_cpu" in
>  arm*|aarch64)
>  AC_CHECK_LIB([fdt], [fdt_create], [], [AC_MSG_ERROR([Could not find libfdt])])
> +
> +# 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...
> +AC_CHECK_FUNCS([fdt_first_subnode fdt_next_subnode])
> +AC_CHECK_DECLS([fdt_first_subnode, fdt_next_subnode],,,[#include <libfdt.h>])
>  esac
>  
>  # Checks for header files.
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 1b16598..2afb146 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -59,7 +59,7 @@ endif
>  LIBXL_OBJS-y += libxl_remus_device.o libxl_remus_disk_drbd.o
>  
>  LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o
> -LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o
> +LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o
>  
>  ifeq ($(CONFIG_NetBSD),y)
>  LIBXL_OBJS-y += libxl_netbsd.o
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 8eb38aa..857abd4 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -3692,6 +3692,7 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc,
>   */
>  void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
>                                      const libxl_bitmap *sptr);
> +
>  #endif
>  
>  /*
> diff --git a/tools/libxl/libxl_libfdt_compat.c b/tools/libxl/libxl_libfdt_compat.c
> new file mode 100644
> index 0000000..a1823b2
> --- /dev/null
> +++ b/tools/libxl/libxl_libfdt_compat.c
> @@ -0,0 +1,94 @@
> +/*
> + * Copyright (C) 2006 David Gibson, IBM Corporation.
> + *
> + * This file is part of libxl, and was originally taken from libfdt.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * Additionally, this particular file is dual licensed.  That is,
> + * alternatively, at your option:
> + *
> + *      Redistribution and use in source and binary forms, with or
> + *      without modification, are permitted provided that the following
> + *      conditions are met:
> + *
> + *      1. Redistributions of source code must retain the above
> + *         copyright notice, this list of conditions and the following
> + *         disclaimer.
> + *      2. Redistributions in binary form must reproduce the above
> + *         copyright notice, this list of conditions and the following
> + *         disclaimer in the documentation and/or other materials
> + *         provided with the distribution.
> + *
> + *      THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> + *      CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
> + *      INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + *      MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + *      DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> + *      CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *      SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + *      NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> + *      LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + *      HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + *      CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> + *      OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
> + *      EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * Note that this applies only to this file, and other files with a
> + * similar notice.  Also, note that when the same code is distributed
> + * along with the rest of libxl, you must comply with the terms of the
> + * LGPLv2.1 for the whole of libxl including this file.
> + *
> + * The intent is to permit, in particular, upstream libfdt to
> + * incorporate improvements to this file within upstream libfdt.  At
> + * the time of writing, upstream libfdt is dual licensed: 2-clause BSD
> + * (as above) and GPLv2-or-later.  The 2-clause BSD licence is
> + * compatible with both GPLv2-or-later and LGPLv2.1-only; this permits
> + * copying in both directions, and the optional licence upgrade to a
> + * copyleft licence by libdft upstream or the Xen Project,
> + * respectively.
> + */
> +
> +#include <libfdt.h>
> +
> +#include "libxl_libfdt_compat.h"
> +
> +#ifndef HAVE_FDT_FIRST_SUBNODE
> +int fdt_first_subnode(const void *fdt, int offset)
> +{
> +	int depth = 0;
> +
> +	offset = fdt_next_node(fdt, offset, &depth);
> +	if (offset < 0 || depth != 1)
> +		return -FDT_ERR_NOTFOUND;
> +
> +	return offset;
> +}
> +#endif
> +
> +#ifndef HAVE_FDT_NEXT_SUBNODE
> +int fdt_next_subnode(const void *fdt, int offset)
> +{
> +	int depth = 1;
> +
> +	/*
> +	 * With respect to the parent, the depth of the next subnode will be
> +	 * the same as the last.
> +	 */
> +	do {
> +		offset = fdt_next_node(fdt, offset, &depth);
> +		if (offset < 0 || depth < 1)
> +			return -FDT_ERR_NOTFOUND;
> +	} while (depth > 1);
> +
> +	return offset;
> +}
> +#endif
> diff --git a/tools/libxl/libxl_libfdt_compat.h b/tools/libxl/libxl_libfdt_compat.h
> new file mode 100644
> index 0000000..cf414f5
> --- /dev/null
> +++ b/tools/libxl/libxl_libfdt_compat.h
> @@ -0,0 +1,80 @@
> +/*
> + * Copyright (C) 2006 David Gibson, IBM Corporation.
> + *
> + * This file is part of libxl, and was originally taken from libfdt.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * Additionally, this particular file is dual licensed.  That is,
> + * alternatively, at your option:
> + *
> + *      Redistribution and use in source and binary forms, with or
> + *      without modification, are permitted provided that the following
> + *      conditions are met:
> + *
> + *      1. Redistributions of source code must retain the above
> + *         copyright notice, this list of conditions and the following
> + *         disclaimer.
> + *      2. Redistributions in binary form must reproduce the above
> + *         copyright notice, this list of conditions and the following
> + *         disclaimer in the documentation and/or other materials
> + *         provided with the distribution.
> + *
> + *      THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> + *      CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
> + *      INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + *      MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + *      DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> + *      CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *      SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + *      NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> + *      LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + *      HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + *      CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> + *      OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
> + *      EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * Note that this applies only to this file, and other files with a
> + * similar notice.  Also, note that when the same code is distributed
> + * along with the rest of libxl, you must comply with the terms of the
> + * LGPLv2.1 for the whole of libxl including this file.
> + *
> + * The intent is to permit, in particular, upstream libfdt to
> + * incorporate improvements to this file within upstream libfdt.  At
> + * the time of writing, upstream libfdt is dual licensed: 2-clause BSD
> + * (as above) and GPLv2-or-later.  The 2-clause BSD licence is
> + * compatible with both GPLv2-or-later and LGPLv2.1-only; this permits
> + * copying in both directions, and the optional licence upgrade to a
> + * copyleft licence by libdft upstream or the Xen Project,
> + * respectively.
> + */
> +
> +#ifndef LIBXL_LIBFDT_COMPAT_H
> +#define LIBXL_LIBFDT_COMPAT_H
> +
> +#if !HAVE_DECL_FDT_FIRST_SUBNODE
> +int fdt_first_subnode(const void *fdt, int offset);
> +#endif
> +
> +#if !HAVE_DECL_FDT_NEXT_SUBNODE
> +int fdt_next_subnode(const void *fdt, int offset);
> +#endif
> +
> +#endif
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> +

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 1/6] tools/libxl: Check if fdt_{first, next}_subnode are present in libfdt
  2015-05-13 14:07   ` Ian Campbell
@ 2015-05-13 15:22     ` Julien Grall
  2015-05-13 15:39       ` Ian Campbell
  0 siblings, 1 reply; 11+ messages in thread
From: Julien Grall @ 2015-05-13 15:22 UTC (permalink / raw)
  To: Ian Campbell, Julien Grall
  Cc: Wei Liu, tim, Julien Grall, Ian Jackson, stefano.stabellini, xen-devel

Hi Ian,

On 13/05/15 15:07, Ian Campbell wrote:
> On Tue, 2015-05-12 at 15:33 +0100, Julien Grall wrote:
>> From: Julien Grall <julien.grall@linaro.org>
>>
>> The functions fdt_{fisrt,next}_subnode may not be available because:
> 
> "first"
> 
>>     * It has been introduced in 2013 => Doesn't work on Wheezy
>>     * The prototype exists but the functions are not exposed. Don't ask
>>     why...
>>
>> The later has been fixed recently in the dtc repo [1]
>>
>> When the functions are not available, implement our own in order to use
>> them in a following patch.
>>
>> [1] git://git.kernel.org/pub/scm/utils/dtc/dtc.git
>>     commit a4b093f7366fdb429ca1781144d3985fa50d0fbb
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Wei Liu <wei.liu2@citrix.com>
> 
> I'm afraid this failed to build again this time with:
> 
> tools/libxl/libxenlight.so: undefined reference to `fdt_first_property_offset'
> tools/libxl/libxenlight.so: undefined reference to `fdt_get_property_by_offset'
> tools/libxl/libxenlight.so: undefined reference to `fdt_next_property_offset'
> 
> That's with arm32 == Debian Wheezy and arm64 == Ubuntu Saucy.

:(. Both the distribution are using an old version of libfdt where the 3
prototypes are defined but the implementation is not exposed in the
library (this is because they use a whitelist for building it)

I gave look to see if we can import them from libfdt. It will require to
import few others in order to make them work:
	- _fdt_check_node_offset
	- _fdt_offset_ptr
	- _nextprop

I think we can skip the first one because it's only a validity check.
FWIW, we declared the the partial device tree should be trusted so valid.

Nonetheless we would add 5 more functions (+ the actual 2) in libxl
which represents ~50 lines of codes.

The 3 offending function have been correctly exposed since the version
v1.4.0 released in June 2013.

I gave a look to major distribution to see which version is using an old
version of libfdt (i.e < 1.4.0:
	- Centos: < Centos 6
	- Debian: < Jessie => Wheezy using an old version
	- Fedora: < Fedora 20
	- openSuse: < opensuse 13.1
	- ubuntu: < Ubuntu 14.04 => The LTS (14.04) is using a new version
	- RedHat: < Redhat 6

AFAICT, all major distributions except debian (for wheezy) are using a
libfdt > v1.4. So I would suggest to disable the partial device tree
support on distribution using older version.

If the user want to use platform device passthrough it would have either
to build a newer version of libfdt or append a device tree to the guest
kernel.

Note: IIRC osstest is using wheezy. If so, we won't be able to test
platform device passthrough until the distribution version is upgraded.

Although, we don't currently have a platform supporting non-PCI
passthrough in osstest.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 1/6] tools/libxl: Check if fdt_{first, next}_subnode are present in libfdt
  2015-05-13 15:22     ` Julien Grall
@ 2015-05-13 15:39       ` Ian Campbell
  2015-05-13 16:04         ` Julien Grall
  0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2015-05-13 15:39 UTC (permalink / raw)
  To: Julien Grall
  Cc: Wei Liu, tim, Julien Grall, Ian Jackson, stefano.stabellini, xen-devel

On Wed, 2015-05-13 at 16:22 +0100, Julien Grall wrote:

> :(. Both the distribution are using an old version of libfdt where the 3
> prototypes are defined but the implementation is not exposed in the
> library (this is because they use a whitelist for building it)
> 
> I gave look to see if we can import them from libfdt. It will require to
> import few others in order to make them work:
> 	- _fdt_check_node_offset
> 	- _fdt_offset_ptr
> 	- _nextprop
> 
> I think we can skip the first one because it's only a validity check.
> FWIW, we declared the the partial device tree should be trusted so valid.
> 
> Nonetheless we would add 5 more functions (+ the actual 2) in libxl
> which represents ~50 lines of codes.
> 
> The 3 offending function have been correctly exposed since the version
> v1.4.0 released in June 2013.
> 
> I gave a look to major distribution to see which version is using an old
> version of libfdt (i.e < 1.4.0:
> 	- Centos: < Centos 6
> 	- Debian: < Jessie => Wheezy using an old version
> 	- Fedora: < Fedora 20
> 	- openSuse: < opensuse 13.1
> 	- ubuntu: < Ubuntu 14.04 => The LTS (14.04) is using a new version
> 	- RedHat: < Redhat 6
> 
> AFAICT, all major distributions except debian (for wheezy) are using a
> libfdt > v1.4. So I would suggest to disable the partial device tree
> support on distribution using older version.

OK, I think we can live with this.

> If the user want to use platform device passthrough it would have either
> to build a newer version of libfdt or append a device tree to the guest
> kernel.
> 
> Note: IIRC osstest is using wheezy. If so, we won't be able to test
> platform device passthrough until the distribution version is upgraded.
> 
> Although, we don't currently have a platform supporting non-PCI
> passthrough in osstest.

Right, I'd expect us to have upgraded to Jessie before we got hold of
such production hardware anyway.

Ian.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 1/6] tools/libxl: Check if fdt_{first, next}_subnode are present in libfdt
  2015-05-13 15:39       ` Ian Campbell
@ 2015-05-13 16:04         ` Julien Grall
  0 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2015-05-13 16:04 UTC (permalink / raw)
  To: Ian Campbell, Julien Grall
  Cc: Wei Liu, Ian Jackson, Julien Grall, tim, stefano.stabellini, xen-devel

Hi Ian,

On 13/05/15 16:39, Ian Campbell wrote:
> On Wed, 2015-05-13 at 16:22 +0100, Julien Grall wrote:
> 
>> :(. Both the distribution are using an old version of libfdt where the 3
>> prototypes are defined but the implementation is not exposed in the
>> library (this is because they use a whitelist for building it)
>>
>> I gave look to see if we can import them from libfdt. It will require to
>> import few others in order to make them work:
>> 	- _fdt_check_node_offset
>> 	- _fdt_offset_ptr
>> 	- _nextprop
>>
>> I think we can skip the first one because it's only a validity check.
>> FWIW, we declared the the partial device tree should be trusted so valid.
>>
>> Nonetheless we would add 5 more functions (+ the actual 2) in libxl
>> which represents ~50 lines of codes.
>>
>> The 3 offending function have been correctly exposed since the version
>> v1.4.0 released in June 2013.
>>
>> I gave a look to major distribution to see which version is using an old
>> version of libfdt (i.e < 1.4.0:
>> 	- Centos: < Centos 6
>> 	- Debian: < Jessie => Wheezy using an old version
>> 	- Fedora: < Fedora 20
>> 	- openSuse: < opensuse 13.1
>> 	- ubuntu: < Ubuntu 14.04 => The LTS (14.04) is using a new version
>> 	- RedHat: < Redhat 6
>>
>> AFAICT, all major distributions except debian (for wheezy) are using a
>> libfdt > v1.4. So I would suggest to disable the partial device tree
>> support on distribution using older version.
> 
> OK, I think we can live with this.

I will resend the series later today or tomorrow.

>> If the user want to use platform device passthrough it would have either
>> to build a newer version of libfdt or append a device tree to the guest
>> kernel.
>>
>> Note: IIRC osstest is using wheezy. If so, we won't be able to test
>> platform device passthrough until the distribution version is upgraded.
>>
>> Although, we don't currently have a platform supporting non-PCI
>> passthrough in osstest.
> 
> Right, I'd expect us to have upgraded to Jessie before we got hold of
> such production hardware anyway.

Good!

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-05-13 16:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-12 14:33 [PATCH v8 0/6] xen/arm: Add support for non-PCI passthrough Julien Grall
2015-05-12 14:33 ` [PATCH v8 1/6] tools/libxl: Check if fdt_{first, next}_subnode are present in libfdt Julien Grall
2015-05-13 14:07   ` Ian Campbell
2015-05-13 15:22     ` Julien Grall
2015-05-13 15:39       ` Ian Campbell
2015-05-13 16:04         ` Julien Grall
2015-05-12 14:33 ` [PATCH v8 2/6] tools/(lib)xl: Add partial device tree support for ARM Julien Grall
2015-05-12 14:33 ` [PATCH v8 3/6] tools/libxl: arm: Use an higher value for the GIC phandle Julien Grall
2015-05-12 14:33 ` [PATCH v8 4/6] libxl: Add support for Device Tree passthrough Julien Grall
2015-05-12 14:33 ` [PATCH v8 5/6] xl: Add new option dtdev Julien Grall
2015-05-12 14:33 ` [PATCH v8 6/6] docs/misc: arm: Add documentation about Device Tree passthrough Julien Grall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.