[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 15/16] xen/arm: Update Dom0 GIC dt node with GICv3 information



Hello Vijaya,

On 15/04/14 12:17, vijay.kilari@xxxxxxxxx wrote:
From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>

Update GIC device tree node for DOM0 with GICv3
information. GIC hw specfic device tree information
is moved to respective GIC driver.

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
---
  xen/arch/arm/domain_build.c |    9 +++------
  xen/arch/arm/gic-v2.c       |    9 +++++++++
  xen/arch/arm/gic-v3.c       |   33 +++++++++++++++++++++++++++++++++
  xen/arch/arm/gic.c          |    5 +++++
  xen/include/asm-arm/gic.h   |    4 ++++
  5 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 5ca2f15..5ef08da 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -576,13 +576,10 @@ static int make_gic_node(const struct domain *d, void 
*fdt,
          return -FDT_ERR_XEN(ENOMEM);
  Hello
      tmp = new_cells;
-    DPRINT("  Set Distributor Base 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
-           d->arch.vgic.dbase, d->arch.vgic.dbase + PAGE_SIZE - 1);
-    dt_set_range(&tmp, node, d->arch.vgic.dbase, PAGE_SIZE);

-    DPRINT("  Set Cpu Base 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
-           d->arch.vgic.cbase, d->arch.vgic.cbase + (PAGE_SIZE * 2) - 1);
-    dt_set_range(&tmp, node, d->arch.vgic.cbase, PAGE_SIZE * 2);
+    res = gic_make_node(d, node, fdt, tmp);


This solution is buggy because tmp is allocated for only 2 ranges. GICv3 can have more than 2 ranges.

Why do you give tmp rather than moving the creation of the property "reg" in each GIC driver?

I would also move all this function in gic_make_node for more clarity.

  static const char * const gicv2_dt_compat[] __initconst =
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 8625e0c..e27b094 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -30,6 +30,7 @@
  #include <xen/softirq.h>
  #include <xen/list.h>
  #include <xen/device_tree.h>
+#include <xen/libfdt/libfdt.h>
  #include <xen/delay.h>
  #include <asm/p2m.h>
  #include <asm/domain.h>
@@ -825,6 +826,37 @@ static unsigned int gicv3_read_vmcr_priority(void)
              GICH_VMCR_PRIORITY_MASK);
  }

+int static gicv3_make_dt_node(const struct domain *d,

static should be before int.

+              const struct dt_device_node *node, void *fdt, __be32 *cells)

With my previous remark, this callback should be:

gicv3_make_dt_node(const struct domain *d,
                   const struct dt_device_node *node, void *fdt)

And you allocate the number of cells in this function.

Regards,

--
Julien Grall

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


 


Rackspace

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