[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 04/21] xen/arm: move a few DT related defines to public/device_tree_defs.h

Hi Stefano,

Thank you, this change looks better to me. Few comments below.

On 07/07/18 00:11, Stefano Stabellini wrote:

Move a few constants defined by libxl_arm.c to
xen/include/public/device_tree_defs.h, so that they can be used from Xen
and libxl. Prepend GUEST_ to avoid conflicts.

Move the DT_IRQ_TYPE* definitions from libxl_arm.c to
public/device_tree_defs.h. Use them in Xen where appropriate.

Re-define the existing Xen internal IRQ_TYPEs as DT_IRQ_TYPEs: they
already happen to be the same, let make it clear.

Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
CC: wei.liu2@xxxxxxxxxx
CC: ian.jackson@xxxxxxxxxxxxx
---
Changes in v2:
- introduce DT_IRQ_TYPE_* definitions in device_tree_guest.h
- prepend GUEST_ to other definitions moved to device_tree_guest.h
- redefine IRQ_TYPE_* as  DT_IRQ_TYPE_*
- use DT_IRQ_TYPE_* in Xen when dealing with DT
---
  tools/libxl/libxl_arm.c               | 59 ++++++++++-------------------------
  xen/arch/arm/domain_build.c           |  8 ++---
  xen/include/asm-arm/irq.h             | 16 ++++++++++
  xen/include/public/device_tree_defs.h | 39 +++++++++++++++++++++++
  xen/include/xen/device_tree.h         | 33 +++-----------------
  5 files changed, 80 insertions(+), 75 deletions(-)
  create mode 100644 xen/include/public/device_tree_defs.h

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 8af9f6f..01ce2ea 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -7,23 +7,7 @@
  #include <stdbool.h>
  #include <libfdt.h>
  #include <assert.h>
-
-/**
- * IRQ line type.
- * DT_IRQ_TYPE_NONE            - default, unspecified type
- * DT_IRQ_TYPE_EDGE_RISING     - rising edge triggered
- * DT_IRQ_TYPE_EDGE_FALLING    - falling edge triggered
- * DT_IRQ_TYPE_EDGE_BOTH       - rising and falling edge triggered
- * DT_IRQ_TYPE_LEVEL_HIGH      - high level triggered
- * DT_IRQ_TYPE_LEVEL_LOW       - low level triggered
- */
-#define DT_IRQ_TYPE_NONE           0x00000000
-#define DT_IRQ_TYPE_EDGE_RISING    0x00000001
-#define DT_IRQ_TYPE_EDGE_FALLING   0x00000002
-#define DT_IRQ_TYPE_EDGE_BOTH                           \
-    (DT_IRQ_TYPE_EDGE_FALLING | DT_IRQ_TYPE_EDGE_RISING)
-#define DT_IRQ_TYPE_LEVEL_HIGH     0x00000004
-#define DT_IRQ_TYPE_LEVEL_LOW      0x00000008
+#include <xen/device_tree_defs.h>
static const char *gicv_to_string(libxl_gic_version gic_version) 
  {
@@ -165,18 +149,9 @@ static struct arch_info {
      {"xen-3.0-aarch64", "arm,armv8-timer", "arm,armv8" },
  };
-/* 
- * 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];
-#define ROOT_ADDRESS_CELLS 2 
-#define ROOT_SIZE_CELLS 2
-
  #define PROP_INITRD_START "linux,initrd-start"
  #define PROP_INITRD_END "linux,initrd-end"
@@ -252,7 +227,7 @@ static int fdt_property_interrupts(libxl__gc *gc, void *fdt, 
      res = fdt_property(fdt, "interrupts", intr, sizeof (intr[0]) * num_irq);
      if (res) return res;
- res = fdt_property_cell(fdt, "interrupt-parent", PHANDLE_GIC); 
+    res = fdt_property_cell(fdt, "interrupt-parent", GUEST_PHANDLE_GIC);
      if (res) return res;
return 0; 
@@ -298,13 +273,13 @@ static int make_root_properties(libxl__gc *gc,
                                "xen,xenvm");
      if (res) return res;
- res = fdt_property_cell(fdt, "interrupt-parent", PHANDLE_GIC); 
+    res = fdt_property_cell(fdt, "interrupt-parent", GUEST_PHANDLE_GIC);
      if (res) return res;
- res = fdt_property_cell(fdt, "#address-cells", ROOT_ADDRESS_CELLS); 
+    res = fdt_property_cell(fdt, "#address-cells", GUEST_ROOT_ADDRESS_CELLS);
      if (res) return res;
- res = fdt_property_cell(fdt, "#size-cells", ROOT_SIZE_CELLS); 
+    res = fdt_property_cell(fdt, "#size-cells", GUEST_ROOT_SIZE_CELLS);
      if (res) return res;
return 0; 
@@ -346,7 +321,7 @@ static int make_chosen_node(libxl__gc *gc, void *fdt, bool 
ramdisk,
                                    "multiboot,module");
          if (res) return res;
- res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS, 
+        res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS, 
GUEST_ROOT_SIZE_CELLS,
                                  1, 0, 0);
          if (res) return res;
@@ -450,7 +425,7 @@ static int make_memory_nodes(libxl__gc *gc, void *fdt, 
          res = fdt_property_string(fdt, "device_type", "memory");
          if (res) return res;
- res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS, 
+        res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS, 
GUEST_ROOT_SIZE_CELLS,
                                  1, 0, 0);
          if (res) return res;
@@ -486,16 +461,16 @@ static int make_gicv2_node(libxl__gc *gc, void *fdt, 
      res = fdt_property(fdt, "interrupt-controller", NULL, 0);
      if (res) return res;
- res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS, 
+    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS, 
GUEST_ROOT_SIZE_CELLS,
                              2,
                              gicd_base, gicd_size,
                              gicc_base, gicc_size);
      if (res) return res;
- res = fdt_property_cell(fdt, "linux,phandle", PHANDLE_GIC); 
+    res = fdt_property_cell(fdt, "linux,phandle", GUEST_PHANDLE_GIC);
      if (res) return res;
- res = fdt_property_cell(fdt, "phandle", PHANDLE_GIC); 
+    res = fdt_property_cell(fdt, "phandle", GUEST_PHANDLE_GIC);
      if (res) return res;
res = fdt_end_node(fdt); 
@@ -528,16 +503,16 @@ static int make_gicv3_node(libxl__gc *gc, void *fdt)
      res = fdt_property(fdt, "interrupt-controller", NULL, 0);
      if (res) return res;
- res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS, 
+    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS, 
GUEST_ROOT_SIZE_CELLS,
                              2,
                              gicd_base, gicd_size,
                              gicr0_base, gicr0_size);
      if (res) return res;
- res = fdt_property_cell(fdt, "linux,phandle", PHANDLE_GIC); 
+    res = fdt_property_cell(fdt, "linux,phandle", GUEST_PHANDLE_GIC);
      if (res) return res;
- res = fdt_property_cell(fdt, "phandle", PHANDLE_GIC); 
+    res = fdt_property_cell(fdt, "phandle", GUEST_PHANDLE_GIC);
      if (res) return res;
res = fdt_end_node(fdt); 
@@ -593,7 +568,7 @@ static int make_hypervisor_node(libxl__gc *gc, void *fdt,
      if (res) return res;
/* reg 0 is grant table space */ 
-    res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
+    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS, 
GUEST_ROOT_SIZE_CELLS,
                              1,GUEST_GNTTAB_BASE, GUEST_GNTTAB_SIZE);
      if (res) return res;
@@ -626,7 +601,7 @@ static int make_vpl011_uart_node(libxl__gc *gc, void *fdt, 
      res = fdt_property_compat(gc, fdt, 1, "arm,sbsa-uart");
      if (res) return res;
- res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS, 
+    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS, 
GUEST_ROOT_SIZE_CELLS,
                              1,
                              GUEST_PL011_BASE, GUEST_PL011_SIZE);
      if (res) return res;
@@ -1027,12 +1002,12 @@ static void finalise_one_node(libxl__gc *gc, void *fdt, 
const char *uname,
          LOG(DEBUG, "Nopping out placeholder node %s", name);
          fdt_nop_node(fdt, node);
      } else {
-        uint32_t regs[ROOT_ADDRESS_CELLS+ROOT_SIZE_CELLS];
+        uint32_t regs[GUEST_ROOT_ADDRESS_CELLS+GUEST_ROOT_SIZE_CELLS];
          be32 *cells = &regs[0];
LOG(DEBUG, "Populating placeholder node %s", name); - set_range(&cells, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS, base, size); 
+        set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS, 
base, size);
res = fdt_setprop_inplace(fdt, node, "reg", regs, sizeof(regs)); 
          assert(!res);
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2a6619a..ae3ebc5 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -658,7 +658,7 @@ static int make_hypervisor_node(struct domain *d,
       *  - All CPUs
       *  TODO: Handle properly the cpumask;
       */
-    set_interrupt_ppi(intr, d->arch.evtchn_irq, 0xf, IRQ_TYPE_LEVEL_LOW);
+    set_interrupt_ppi(intr, d->arch.evtchn_irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
      res = fdt_property_interrupts(fdt, &intr, 1);
      if ( res )
          return res;
@@ -934,15 +934,15 @@ static int make_timer_node(const struct domain *d, void 
*fdt,
irq = timer_get_irq(TIMER_PHYS_SECURE_PPI); 
      dt_dprintk("  Secure interrupt %u\n", irq);
-    set_interrupt_ppi(intrs[0], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
+    set_interrupt_ppi(intrs[0], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
irq = timer_get_irq(TIMER_PHYS_NONSECURE_PPI); 
      dt_dprintk("  Non secure interrupt %u\n", irq);
-    set_interrupt_ppi(intrs[1], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
+    set_interrupt_ppi(intrs[1], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
irq = timer_get_irq(TIMER_VIRT_PPI); 
      dt_dprintk("  Virt interrupt %u\n", irq);
-    set_interrupt_ppi(intrs[2], irq, 0xf, IRQ_TYPE_LEVEL_LOW);
+    set_interrupt_ppi(intrs[2], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
res = fdt_property_interrupts(fdt, intrs, 3); 
      if ( res )
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 9d55e9b..e45d574 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -2,6 +2,22 @@
  #define _ASM_HW_IRQ_H
#include <xen/device_tree.h> 
+#include <public/device_tree_defs.h>
+
+/*
+ * These defines correspond to the Xen internal representation of the
+ * IRQ types. We choose to make them the same as the existing device
+ * tree definitions for convenience.
+ */
+#define IRQ_TYPE_NONE           DT_IRQ_TYPE_NONE
+#define IRQ_TYPE_EDGE_RISING    DT_IRQ_TYPE_EDGE_RISING
+#define IRQ_TYPE_EDGE_FALLING   DT_IRQ_TYPE_EDGE_FALLING
+#define IRQ_TYPE_EDGE_BOTH      DT_IRQ_TYPE_EDGE_BOTH
+#define IRQ_TYPE_LEVEL_HIGH     DT_IRQ_TYPE_LEVEL_HIGH
+#define IRQ_TYPE_LEVEL_LOW      DT_IRQ_TYPE_LEVEL_LOW
+#define IRQ_TYPE_LEVEL_MASK     DT_IRQ_TYPE_LEVEL_MASK
+#define IRQ_TYPE_SENSE_MASK     DT_IRQ_TYPE_SENSE_MASK
+#define IRQ_TYPE_INVALID        DT_IRQ_TYPE_INVALID
#define NR_VECTORS 256 /* XXX */ diff --git a/xen/include/public/device_tree_defs.h b/xen/include/public/device_tree_defs.h 
new file mode 100644
index 0000000..476b04d
--- /dev/null
+++ b/xen/include/public/device_tree_defs.h
@@ -0,0 +1,39 @@
+#ifndef __XEN_DEVICE_TREE_DEFS_H__
+#define __XEN_DEVICE_TREE_DEFS_H__


This should only be exposed for Xen and Tools:

#if defined(__XEN__) || defined(__XEN_TOOLS__)


+
+/*
+ * The device tree compiler (DTC) is allocating the phandle from 1 to
+ * onwards. Reserve a high value for the GIC phandle.
+ */
+#define GUEST_PHANDLE_GIC (65000)
+
+#define GUEST_ROOT_ADDRESS_CELLS 2
+#define GUEST_ROOT_SIZE_CELLS 2
+
+/**
+ * IRQ line type.
+ *
+ * DT_IRQ_TYPE_NONE            - default, unspecified type
+ * DT_IRQ_TYPE_EDGE_RISING     - rising edge triggered
+ * DT_IRQ_TYPE_EDGE_FALLING    - falling edge triggered
+ * DT_IRQ_TYPE_EDGE_BOTH       - rising and falling edge triggered
+ * DT_IRQ_TYPE_LEVEL_HIGH      - high level triggered
+ * DT_IRQ_TYPE_LEVEL_LOW       - low level triggered
+ * DT_IRQ_TYPE_LEVEL_MASK      - Mask to filter out the level bits
+ * DT_IRQ_TYPE_SENSE_MASK      - Mask for all the above bits
+ * DT_IRQ_TYPE_INVALID         - Use to initialize the type
+ */
+#define DT_IRQ_TYPE_NONE           0x00000000
+#define DT_IRQ_TYPE_EDGE_RISING    0x00000001
+#define DT_IRQ_TYPE_EDGE_FALLING   0x00000002
+#define DT_IRQ_TYPE_EDGE_BOTH                           \
+    (DT_IRQ_TYPE_EDGE_FALLING | DT_IRQ_TYPE_EDGE_RISING)
+#define DT_IRQ_TYPE_LEVEL_HIGH     0x00000004
+#define DT_IRQ_TYPE_LEVEL_LOW      0x00000008
+#define DT_IRQ_TYPE_LEVEL_MASK                          \
+    (DT_IRQ_TYPE_LEVEL_LOW | DT_IRQ_TYPE_LEVEL_HIGH)
+#define DT_IRQ_TYPE_SENSE_MASK     0x0000000f
+
+#define DT_IRQ_TYPE_INVALID        0x00000010
+
+#endif
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 0aecbe0..638b926 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -13,6 +13,7 @@
  #include <asm/byteorder.h>
  #include <asm/device.h>
  #include <public/xen.h>
+#include <public/device_tree_defs.h>
  #include <xen/kernel.h>
  #include <xen/init.h>
  #include <xen/string.h>
@@ -114,35 +115,9 @@ struct dt_phandle_args {
  };
/** 
- * IRQ line type.
- *
- * IRQ_TYPE_NONE            - default, unspecified type
- * IRQ_TYPE_EDGE_RISING     - rising edge triggered
- * IRQ_TYPE_EDGE_FALLING    - falling edge triggered
- * IRQ_TYPE_EDGE_BOTH       - rising and falling edge triggered
- * IRQ_TYPE_LEVEL_HIGH      - high level triggered
- * IRQ_TYPE_LEVEL_LOW       - low level triggered
- * IRQ_TYPE_LEVEL_MASK      - Mask to filter out the level bits
- * IRQ_TYPE_SENSE_MASK      - Mask for all the above bits
- * IRQ_TYPE_INVALID         - Use to initialize the type
- */
-#define IRQ_TYPE_NONE           0x00000000
-#define IRQ_TYPE_EDGE_RISING    0x00000001
-#define IRQ_TYPE_EDGE_FALLING   0x00000002
-#define IRQ_TYPE_EDGE_BOTH                           \
-    (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING)
-#define IRQ_TYPE_LEVEL_HIGH     0x00000004
-#define IRQ_TYPE_LEVEL_LOW      0x00000008
-#define IRQ_TYPE_LEVEL_MASK                          \
-    (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH)
-#define IRQ_TYPE_SENSE_MASK     0x0000000f
-
-#define IRQ_TYPE_INVALID        0x00000010
-
-/**
   * dt_irq - describe an IRQ in the device tree
   * @irq: IRQ number
- * @type: IRQ type (see IRQ_TYPE_*)
+ * @type: IRQ type (see DT_IRQ_TYPE_*)
   *
   * This structure is returned when an interrupt is mapped.
   */
@@ -151,12 +126,12 @@ struct dt_irq {
      unsigned int type;
  };
-/* If type == IRQ_TYPE_NONE, assume we use level triggered */ 
+/* If type == DT_IRQ_TYPE_NONE, assume we use level triggered */
  static inline bool_t dt_irq_is_level_triggered(const struct dt_irq *irq)
  {
      unsigned int type = irq->type;
- return (type & IRQ_TYPE_LEVEL_MASK) || (type == IRQ_TYPE_NONE); 
+    return (type & DT_IRQ_TYPE_LEVEL_MASK) || (type == DT_IRQ_TYPE_NONE);
  }
/** 



Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.

Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.