[PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Frediano Ziglio]

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@huawei.com>
---
 xen/arch/arm/gic-v2.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Submit again after release. This patch was developed as a specific
platform patch but it applies to all platforms.

Previous discussions at
- http://lists.xen.org/archives/html/xen-devel/2014-11/msg00406.html
- http://lists.xen.org/archives/html/xen-devel/2014-11/msg00520.html

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 31fb81a..4d1924e 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -590,7 +590,7 @@ static int gicv2_make_dt_node(const struct domain *d,
     const struct dt_device_node *gic = dt_interrupt_controller;
     const void *compatible = NULL;
     u32 len;
-    __be32 *new_cells, *tmp;
+    const __be32 *regs;
     int res = 0;
 
     compatible = dt_get_property(gic, "compatible", &len);
@@ -617,18 +617,18 @@ static int gicv2_make_dt_node(const struct domain *d,
     if ( res )
         return res;
 
+    /* copy GICC and GICD regions */
+    regs = dt_get_property(gic, "reg", &len);
+    if ( !regs )
+    {
+        dprintk(XENLOG_ERR, "Can't find reg property for the gic node\n");
+        return -FDT_ERR_XEN(ENOENT);
+    }
+
     len = dt_cells_to_size(dt_n_addr_cells(node) + dt_n_size_cells(node));
     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);
 
-    res = fdt_property(fdt, "reg", new_cells, len);
-    xfree(new_cells);
+    res = fdt_property(fdt, "reg", regs, len);
 
     return res;
 }
-- 
1.9.1

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Julien Grall]

Hi Frediano,

On 20/02/15 09:38, 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@huawei.com>

Reviewed-by: Julien Grall <julien.grall@linaro.org>

I'm wondering if it's worth to backport this patch to Xen 4.5?

I will wrote a patch to do the same for GICv3.

Regards,

-- 
Julien Grall

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Ian Campbell]

On Mon, 2015-02-23 at 16:30 +0000, Julien Grall wrote:
> I'm wondering if it's worth to backport this patch to Xen 4.5?

Maybe, I doubt 4.5 is being used on platforms which suffer from this
issue, but it's pretty un-intrusive I suppose.

> I will wrote a patch to do the same for GICv3.

Thanks.

I certainly wouldn't want to backport a fix to gic v2 only.

Ian.

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Julien Grall]

On 24/02/15 16:01, Ian Campbell wrote:
> On Mon, 2015-02-23 at 16:30 +0000, Julien Grall wrote:
>> I'm wondering if it's worth to backport this patch to Xen 4.5?
> 
> Maybe, I doubt 4.5 is being used on platforms which suffer from this
> issue, but it's pretty un-intrusive I suppose.

There is no reason to consider only the set of platforms we support
upstream. We should take into account people who want to ship a product
based on Xen 4.5. This release aims to work anywhere by only adding few
lines of code in the platform code.

Furthermore, the current code rely on how the device has been written.
For instance the calxeda device tree is using ranges and, therefore, the
regs contains offset.
Fortunately, the offset and the translated address is the same.

> I certainly wouldn't want to backport a fix to gic v2 only.

As we didn't backport other patches related to GICv3, it would not make
much sense to backport a similar fix for GICv3..

Regards,

-- 
Julien Grall

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Ian Campbell]

On Tue, 2015-02-24 at 16:21 +0000, Julien Grall wrote:
> On 24/02/15 16:01, Ian Campbell wrote:
> > On Mon, 2015-02-23 at 16:30 +0000, Julien Grall wrote:
> >> I'm wondering if it's worth to backport this patch to Xen 4.5?
> > 
> > Maybe, I doubt 4.5 is being used on platforms which suffer from this
> > issue, but it's pretty un-intrusive I suppose.
> 
> There is no reason to consider only the set of platforms we support
> upstream.

No, but it is a factor: we shouldn't be risking destabilising things for
those platforms which were supported out of the box on 4.5 for the
benefit of platforms which were and are not.

Of course risk is a judgement call, and as I said this change is pretty
unobtrusive (read: probably not that risky), hence "maybe" rather than
"no".

>  We should take into account people who want to ship a product
> based on Xen 4.5.

Again, this is a factor but not the most important one.

>  This release aims to work anywhere by only adding few
> lines of code in the platform code.

And so is this.

> Furthermore, the current code rely on how the device has been written.
> For instance the calxeda device tree is using ranges and, therefore, the
> regs contains offset.
> Fortunately, the offset and the translated address is the same.
> 
> > I certainly wouldn't want to backport a fix to gic v2 only.
> 
> As we didn't backport other patches related to GICv3, it would not make
> much sense to backport a similar fix for GICv3..

We should consider each patch/series on its own merits. The other GICv3
fixes were far more invasive and complicated than what is being done
here.

Assuming the v3 fix is similar to the v2 one here either both are
justified or neither.

Ian.

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Julien Grall]

On 24/02/15 17:00, Ian Campbell wrote:
>> Furthermore, the current code rely on how the device has been written.
>> For instance the calxeda device tree is using ranges and, therefore, the
>> regs contains offset.
>> Fortunately, the offset and the translated address is the same.
>>
>>> I certainly wouldn't want to backport a fix to gic v2 only.
>>
>> As we didn't backport other patches related to GICv3, it would not make
>> much sense to backport a similar fix for GICv3..
> 
> We should consider each patch/series on its own merits. The other GICv3
> fixes were far more invasive and complicated than what is being done
> here.

Agreed. Just it doesn't make sense to backport new GICv3 patches as we
know that without the previous series any Linux >= 3.19 won't boot at all.

Regards,

-- 
Julien Grall

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Ian Campbell]

On Fri, 2015-02-20 at 09:38 +0000, 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.

I don't think this adequately explains what is going on here.

AIUI the issue is that the values in d->arch.vgic.{c,d}base have already
been translated to host level addresses from whatever bus address
applied to them in the DT.

By copying the original address from the DT directly we then get the
original untranslated reg property, which the guest will then apply the
translations to and, presumably, retrieve the same answer we have in
{c,d}base.

This relies on us putting our GIC node at the same place as the
platforms DTB, which is indeed the case.

Please can you rewrite the commit message to explain this more clearly.

One minor comment on the code:

> +    /* copy GICC and GICD regions */
> +    regs = dt_get_property(gic, "reg", &len);
> +    if ( !regs )
> +    {
> +        dprintk(XENLOG_ERR, "Can't find reg property for the gic node\n");
> +        return -FDT_ERR_XEN(ENOENT);
> +    }
> +
>      len = dt_cells_to_size(dt_n_addr_cells(node) + dt_n_size_cells(node));
>      len *= 2; /* GIC has two memory regions: Distributor + CPU interface */

It takes a second to figure out why we aren't just using the length
retrieved from the dt. Please add a comment.

Ian.

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Frediano Ziglio]

Translated address (in d->arch.vgic.{c,d}base) are now bus addresses
which could not always be applied to the DT.
Copy the original address from DT directly to get the original
untraslated reg property which will give same d->arch.vgic.{c,d}base
values once translated again.

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
---
 xen/arch/arm/gic-v2.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

Updated comment and commit message as requested.

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 31fb81a..6899ab1 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -590,7 +590,7 @@ static int gicv2_make_dt_node(const struct domain *d,
     const struct dt_device_node *gic = dt_interrupt_controller;
     const void *compatible = NULL;
     u32 len;
-    __be32 *new_cells, *tmp;
+    const __be32 *regs;
     int res = 0;
 
     compatible = dt_get_property(gic, "compatible", &len);
@@ -617,18 +617,22 @@ 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));
-    len *= 2; /* GIC has two memory regions: Distributor + CPU interface */
-    new_cells = xzalloc_bytes(len);
-    if ( new_cells == NULL )
-        return -FDT_ERR_XEN(ENOMEM);
+    /*
+     * Copy only GICC and GICD regions, not entire regions.
+     * DTB provides up to 4 regions to handle virtualization
+     * however dom0 just needs GICC and GICD provided by Xen.
+     */
+    regs = dt_get_property(gic, "reg", &len);
+    if ( !regs )
+    {
+        dprintk(XENLOG_ERR, "Can't find reg property for the gic node\n");
+        return -FDT_ERR_XEN(ENOENT);
+    }
 
-    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);
+    len = dt_cells_to_size(dt_n_addr_cells(node) + dt_n_size_cells(node));
+    len *= 2;
 
-    res = fdt_property(fdt, "reg", new_cells, len);
-    xfree(new_cells);
+    res = fdt_property(fdt, "reg", regs, len);
 
     return res;
 }
-- 
1.9.1

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Julien Grall]

On 25/02/15 11:14, Frediano Ziglio wrote:
> Translated address (in d->arch.vgic.{c,d}base) are now bus addresses

addresses

> which could not always be applied to the DT.
> Copy the original address from DT directly to get the original

addresses

> untraslated reg property which will give same d->arch.vgic.{c,d}base

untranslated

> values once translated again.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> ---
>  xen/arch/arm/gic-v2.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> Updated comment and commit message as requested.
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 31fb81a..6899ab1 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -590,7 +590,7 @@ static int gicv2_make_dt_node(const struct domain *d,
>      const struct dt_device_node *gic = dt_interrupt_controller;
>      const void *compatible = NULL;
>      u32 len;
> -    __be32 *new_cells, *tmp;
> +    const __be32 *regs;
>      int res = 0;
>  
>      compatible = dt_get_property(gic, "compatible", &len);
> @@ -617,18 +617,22 @@ 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));
> -    len *= 2; /* GIC has two memory regions: Distributor + CPU interface */
> -    new_cells = xzalloc_bytes(len);
> -    if ( new_cells == NULL )
> -        return -FDT_ERR_XEN(ENOMEM);
> +    /*
> +     * Copy only GICC and GICD regions, not entire regions.
> +     * DTB provides up to 4 regions to handle virtualization
> +     * however dom0 just needs GICC and GICD provided by Xen.

The 2 sentences say pretty much the same things. Although, the second
one is more complete. I would only keep the second one.

> +     */
> +    regs = dt_get_property(gic, "reg", &len);
> +    if ( !regs )
> +    {
> +        dprintk(XENLOG_ERR, "Can't find reg property for the gic node\n");
> +        return -FDT_ERR_XEN(ENOENT);
> +    }
>  
> -    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);
> +    len = dt_cells_to_size(dt_n_addr_cells(node) + dt_n_size_cells(node));
> +    len *= 2;
>  
> -    res = fdt_property(fdt, "reg", new_cells, len);
> -    xfree(new_cells);
> +    res = fdt_property(fdt, "reg", regs, len);
>  
>      return res;
>  }
> 


Regards,

-- 
Julien Grall

[PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Frediano Ziglio]

Translated addresses (in d->arch.vgic.{c,d}base) are now bus addresses
which could not always be applied to the DT.
Copy the original addresses from DT directly to get the original
untranslated reg property which will give same d->arch.vgic.{c,d}base
values once translated again.

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
---
 xen/arch/arm/gic-v2.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

Fixed typos in comments.

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 31fb81a..a401e3f 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -590,7 +590,7 @@ static int gicv2_make_dt_node(const struct domain *d,
     const struct dt_device_node *gic = dt_interrupt_controller;
     const void *compatible = NULL;
     u32 len;
-    __be32 *new_cells, *tmp;
+    const __be32 *regs;
     int res = 0;
 
     compatible = dt_get_property(gic, "compatible", &len);
@@ -617,18 +617,21 @@ 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));
-    len *= 2; /* GIC has two memory regions: Distributor + CPU interface */
-    new_cells = xzalloc_bytes(len);
-    if ( new_cells == NULL )
-        return -FDT_ERR_XEN(ENOMEM);
+    /*
+     * DTB provides up to 4 regions to handle virtualization
+     * however dom0 just needs GICC and GICD provided by Xen.
+     */
+    regs = dt_get_property(gic, "reg", &len);
+    if ( !regs )
+    {
+        dprintk(XENLOG_ERR, "Can't find reg property for the gic node\n");
+        return -FDT_ERR_XEN(ENOENT);
+    }
 
-    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);
+    len = dt_cells_to_size(dt_n_addr_cells(node) + dt_n_size_cells(node));
+    len *= 2;
 
-    res = fdt_property(fdt, "reg", new_cells, len);
-    xfree(new_cells);
+    res = fdt_property(fdt, "reg", regs, len);
 
     return res;
 }
-- 
1.9.1

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Julien Grall]

Hi Frediano,

On 25/02/15 13:21, Frediano Ziglio wrote:
> Translated addresses (in d->arch.vgic.{c,d}base) are now bus addresses
> which could not always be applied to the DT.
> Copy the original addresses from DT directly to get the original
> untranslated reg property which will give same d->arch.vgic.{c,d}base
> values once translated again.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> ---
>  xen/arch/arm/gic-v2.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> Fixed typos in comments.
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 31fb81a..a401e3f 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -590,7 +590,7 @@ static int gicv2_make_dt_node(const struct domain *d,
>      const struct dt_device_node *gic = dt_interrupt_controller;
>      const void *compatible = NULL;
>      u32 len;
> -    __be32 *new_cells, *tmp;
> +    const __be32 *regs;
>      int res = 0;
>  
>      compatible = dt_get_property(gic, "compatible", &len);
> @@ -617,18 +617,21 @@ 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));
> -    len *= 2; /* GIC has two memory regions: Distributor + CPU interface */
> -    new_cells = xzalloc_bytes(len);
> -    if ( new_cells == NULL )
> -        return -FDT_ERR_XEN(ENOMEM);
> +    /*
> +     * DTB provides up to 4 regions to handle virtualization

Sorry to ask more change.

I'm not sure why you speak about virtualization here.

Also, can you write somewhere that the GICC and GICD are the first 2
regions of the "reg"?

Other than that this patch looks good to me:

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Ian Campbell]

On Fri, 2015-02-27 at 13:53 +0000, Julien Grall wrote:
> Hi Frediano,
> 
> On 25/02/15 13:21, Frediano Ziglio wrote:
> > Translated addresses (in d->arch.vgic.{c,d}base) are now bus addresses
> > which could not always be applied to the DT.
> > Copy the original addresses from DT directly to get the original
> > untranslated reg property which will give same d->arch.vgic.{c,d}base
> > values once translated again.
> > 
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> > ---
> >  xen/arch/arm/gic-v2.c | 25 ++++++++++++++-----------
> >  1 file changed, 14 insertions(+), 11 deletions(-)
> > 
> > Fixed typos in comments.
> > 
> > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> > index 31fb81a..a401e3f 100644
> > --- a/xen/arch/arm/gic-v2.c
> > +++ b/xen/arch/arm/gic-v2.c
> > @@ -590,7 +590,7 @@ static int gicv2_make_dt_node(const struct domain *d,
> >      const struct dt_device_node *gic = dt_interrupt_controller;
> >      const void *compatible = NULL;
> >      u32 len;
> > -    __be32 *new_cells, *tmp;
> > +    const __be32 *regs;
> >      int res = 0;
> >  
> >      compatible = dt_get_property(gic, "compatible", &len);
> > @@ -617,18 +617,21 @@ 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));
> > -    len *= 2; /* GIC has two memory regions: Distributor + CPU interface */
> > -    new_cells = xzalloc_bytes(len);
> > -    if ( new_cells == NULL )
> > -        return -FDT_ERR_XEN(ENOMEM);
> > +    /*
> > +     * DTB provides up to 4 regions to handle virtualization
> 
> Sorry to ask more change.
> 
> I'm not sure why you speak about virtualization here.

Because two of the regions are GICH and GICV, and those are the ones we
are truncating out here.

> 
> Also, can you write somewhere that the GICC and GICD are the first 2
> regions of the "reg"?
> 
> Other than that this patch looks good to me:
> 
> Reviewed-by: Julien Grall <julien.grall@linaro.org>
> 
> Regards,
> 

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Julien Grall]

On 27/02/15 13:57, Ian Campbell wrote:
> On Fri, 2015-02-27 at 13:53 +0000, Julien Grall wrote:
>> Hi Frediano,
>>
>> On 25/02/15 13:21, Frediano Ziglio wrote:
>>> Translated addresses (in d->arch.vgic.{c,d}base) are now bus addresses
>>> which could not always be applied to the DT.
>>> Copy the original addresses from DT directly to get the original
>>> untranslated reg property which will give same d->arch.vgic.{c,d}base
>>> values once translated again.
>>>
>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
>>> ---
>>>  xen/arch/arm/gic-v2.c | 25 ++++++++++++++-----------
>>>  1 file changed, 14 insertions(+), 11 deletions(-)
>>>
>>> Fixed typos in comments.
>>>
>>> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
>>> index 31fb81a..a401e3f 100644
>>> --- a/xen/arch/arm/gic-v2.c
>>> +++ b/xen/arch/arm/gic-v2.c
>>> @@ -590,7 +590,7 @@ static int gicv2_make_dt_node(const struct domain *d,
>>>      const struct dt_device_node *gic = dt_interrupt_controller;
>>>      const void *compatible = NULL;
>>>      u32 len;
>>> -    __be32 *new_cells, *tmp;
>>> +    const __be32 *regs;
>>>      int res = 0;
>>>  
>>>      compatible = dt_get_property(gic, "compatible", &len);
>>> @@ -617,18 +617,21 @@ 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));
>>> -    len *= 2; /* GIC has two memory regions: Distributor + CPU interface */
>>> -    new_cells = xzalloc_bytes(len);
>>> -    if ( new_cells == NULL )
>>> -        return -FDT_ERR_XEN(ENOMEM);
>>> +    /*
>>> +     * DTB provides up to 4 regions to handle virtualization
>>
>> Sorry to ask more change.
>>
>> I'm not sure why you speak about virtualization here.
> 
> Because two of the regions are GICH and GICV, and those are the ones we
> are truncating out here.

I though about it but I wasn't sure if he meant that.

Regards,

-- 
Julien Grall

[PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Frediano Ziglio]

Translated addresses (in d->arch.vgic.{c,d}base) are now bus addresses
which could not always be applied to the DT.
Copy the original addresses from DT directly to get the original
untranslated reg property which will give same d->arch.vgic.{c,d}base
values once translated again.

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
---
 xen/arch/arm/gic-v2.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)


Changed some comments.


diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index c05b64a..fa695d1 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -604,7 +604,7 @@ static int gicv2_make_dt_node(const struct domain *d,
     const struct dt_device_node *gic = dt_interrupt_controller;
     const void *compatible = NULL;
     u32 len;
-    __be32 *new_cells, *tmp;
+    const __be32 *regs;
     int res = 0;
 
     compatible = dt_get_property(gic, "compatible", &len);
@@ -631,18 +631,22 @@ 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));
-    len *= 2; /* GIC has two memory regions: Distributor + CPU interface */
-    new_cells = xzalloc_bytes(len);
-    if ( new_cells == NULL )
-        return -FDT_ERR_XEN(ENOMEM);
+    /*
+     * DTB provides up to 4 regions to handle virtualization
+     * (in order GICD, GICC, GICH and GICV interfaces)
+     * however dom0 just needs GICD and GICC provided by Xen.
+     */
+    regs = dt_get_property(gic, "reg", &len);
+    if ( !regs )
+    {
+        dprintk(XENLOG_ERR, "Can't find reg property for the gic node\n");
+        return -FDT_ERR_XEN(ENOENT);
+    }
 
-    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);
+    len = dt_cells_to_size(dt_n_addr_cells(node) + dt_n_size_cells(node));
+    len *= 2;
 
-    res = fdt_property(fdt, "reg", new_cells, len);
-    xfree(new_cells);
+    res = fdt_property(fdt, "reg", regs, len);
 
     return res;
 }
-- 
1.9.1

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Ian Campbell]

On Fri, 2015-02-27 at 14:08 +0000, Frediano Ziglio wrote:
> Translated addresses (in d->arch.vgic.{c,d}base) are now bus addresses
> which could not always be applied to the DT.
> Copy the original addresses from DT directly to get the original
> untranslated reg property which will give same d->arch.vgic.{c,d}base
> values once translated again.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>

Acked + applied. I reworded the commit message a bit in an attempt to
make it clearer:

   xen/arm: Handle translated addresses for hardware domains in GICv2
    
    Translated addresses (in d->arch.vgic.{c,d}base) are bus addresses
    which are not always correct in the context of a subnode in the DTB
    exposed to domain 0 since they would then be subject to retranslation.
    
    Copy the original addresses from DT directly to get the original
    untranslated reg property which will give same d->arch.vgic.{c,d}base
    values once translated again by the guest.
    
    Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
    Acked-by: Ian Campbell <ian.campbell@citrix.com>
    [ ijc -- attempt to clarify the commit message ]

I hope that is ok.

Re: [PATCH] xen/arm: Handle translated addresses for hardware domains in GICv2

[Frediano Ziglio]

2015-03-02 14:50 GMT+00:00 Ian Campbell <ian.campbell@citrix.com>:
> On Fri, 2015-02-27 at 14:08 +0000, Frediano Ziglio wrote:
>> Translated addresses (in d->arch.vgic.{c,d}base) are now bus addresses
>> which could not always be applied to the DT.
>> Copy the original addresses from DT directly to get the original
>> untranslated reg property which will give same d->arch.vgic.{c,d}base
>> values once translated again.
>>
>> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
>
> Acked + applied. I reworded the commit message a bit in an attempt to
> make it clearer:
>
>    xen/arm: Handle translated addresses for hardware domains in GICv2
>
>     Translated addresses (in d->arch.vgic.{c,d}base) are bus addresses
>     which are not always correct in the context of a subnode in the DTB
>     exposed to domain 0 since they would then be subject to retranslation.
>
>     Copy the original addresses from DT directly to get the original
>     untranslated reg property which will give same d->arch.vgic.{c,d}base
>     values once translated again by the guest.
>
>     Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
>     Acked-by: Ian Campbell <ian.campbell@citrix.com>
>     [ ijc -- attempt to clarify the commit message ]
>
> I hope that is ok.
>

Perfect!

Thanks,
  Frediano