Re: [Xen-devel] [PATCH v3 8/8] xen/arm: Handle translated addresses for hardware domains

[Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>]

> Hi Frediano,
> 
> On 11/05/2014 09:41 AM, Frediano Ziglio wrote:
> > Translated address could have an offset applied to them.
> > Replicate same value for device node to avoid improper address
> > computation in the OS.
> >
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
> > ---
> >  xen/arch/arm/gic-v2.c | 23 +++++++++++++++++++++--
> >  1 file changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c index
> > 2f6bbd5..271074d 100644
> > --- a/xen/arch/arm/gic-v2.c
> > +++ b/xen/arch/arm/gic-v2.c
> > @@ -67,8 +67,10 @@
> >  /* Global state */
> >  static struct {
> >      paddr_t dbase;            /* Address of distributor registers */
> > +    paddr_t dbase_raw;        /* Untranslated address of distributor
> registers */
> >      void __iomem * map_dbase; /* IO mapped Address of distributor
> registers */
> >      paddr_t cbase;            /* Address of CPU interface registers
> */
> > +    paddr_t cbase_raw;        /* Untranslated address of CPU
> interface registers */
> >      void __iomem * map_cbase[2]; /* IO mapped Address of CPU
> interface registers */
> >      paddr_t hbase;            /* Address of virtual interface
> registers */
> >      void __iomem * map_hbase; /* IO Address of virtual interface
> > registers */ @@ -671,8 +673,17 @@ static int gicv2_make_dt_node(const
> struct domain *d,
> >          return -FDT_ERR_XEN(ENOMEM);
> >
> >      tmp = new_cells;
> > -    dt_set_range(&tmp, node, d->arch.vgic.dbase, PAGE_SIZE);
> > -    dt_set_range(&tmp, node, d->arch.vgic.cbase, PAGE_SIZE * 2);
> > +
> > +    if ( is_hardware_domain(d) )
> > +    {
> 
> This check is pointless, as said on an earlier version of this series,
> gicv2_make_dt_node is only used to create DOM0 (hardware domain) device
> tree.
> 
> Regards,
> 
> --
> Julien Grall

How does sound something like this (already tested, it's working). Perhaps just 
to be paranoid a test on len after reading reg property would be perfect.

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 2f6bbd5..2c4d097 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -632,7 +632,7 @@ static int gicv2_make_dt_node(const struct domain *d,
     const void *compatible = NULL;
     u32 len;
     __be32 *new_cells, *tmp;
-    int res = 0;
+    int res = 0, na, ns;
 
     compatible = dt_get_property(gic, "compatible", &len);
     if ( !compatible )
@@ -664,15 +664,27 @@ static int gicv2_make_dt_node(const struct domain *d,
     if ( res )
         return res;
 
-    len = dt_cells_to_size(dt_n_addr_cells(node) + dt_n_size_cells(node));
+    /* copy GICC and GICD regions */
+    na = dt_n_addr_cells(node);
+    ns = dt_n_size_cells(node);
+
+    if ( na != dt_n_addr_cells(gic) || ns != dt_n_size_cells(gic) )
+        return -FDT_ERR_XEN(EINVAL);
+
+    tmp = (__be32 *) dt_get_property(gic, "reg", &len);
+    if ( !tmp )
+    {
+        dprintk(XENLOG_ERR, "Can't find reg property for the gic node\n");
+        return -FDT_ERR_XEN(ENOENT);
+    }
+
+    len = dt_cells_to_size(na + ns);
     len *= 2; /* GIC has two memory regions: Distributor + CPU interface */
     new_cells = xzalloc_bytes(len);
     if ( new_cells == NULL )
         return -FDT_ERR_XEN(ENOMEM);
 
-    tmp = new_cells;
-    dt_set_range(&tmp, node, d->arch.vgic.dbase, PAGE_SIZE);
-    dt_set_range(&tmp, node, d->arch.vgic.cbase, PAGE_SIZE * 2);
+    memcpy(new_cells, tmp, len);
 
     res = fdt_property(fdt, "reg", new_cells, len);
     xfree(new_cells);
-- 
1.9.1

Frediano


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