[Qemu-devel] [PATCH 0/3] arm_gic: Improve handling of GICD_ICFGR

[Qemu-devel] [PATCH 0/3] arm_gic: Improve handling of GICD_ICFGR

[Adam Lackorzynski]

The following patches address the behavior of the GICD_ICFGR register
in the ARM GIC.                                                                                   

Changes to previous version:
 - Setting of model mode only for old GIC revisions
 - Less invasive change for PPI settings


Adam Lackorzynski (3):
  arm_gic: Fix read of GICD_ICFGR
  arm_gic: GICD_ICFGR: Write model only for pre v1 GICs
  arm_gic: Do not force PPIs to edge-triggered mode

 hw/intc/arm_gic.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

-- 
2.1.0.rc1

[Qemu-devel] [PATCH 1/3] arm_gic: Fix read of GICD_ICFGR

[Adam Lackorzynski]

The GICD_ICFGR register covers 4 interrupts per byte.

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
---
 hw/intc/arm_gic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 1532ef9..d2b1aaf 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -372,7 +372,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset)
         }
     } else if (offset < 0xf00) {
         /* Interrupt Configuration.  */
-        irq = (offset - 0xc00) * 2 + GIC_BASE_IRQ;
+        irq = (offset - 0xc00) * 4 + GIC_BASE_IRQ;
         if (irq >= s->num_irq)
             goto bad_reg;
         res = 0;
-- 
2.1.0.rc1

[Qemu-devel] [PATCH 2/3] arm_gic: GICD_ICFGR: Write model only for pre v1 GICs

[Adam Lackorzynski]

Setting the model is only available in pre-v1 GIC models.
---
 hw/intc/arm_gic.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index d2b1aaf..e546647 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -561,10 +561,12 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
         if (irq < GIC_INTERNAL)
             value |= 0xaa;
         for (i = 0; i < 4; i++) {
-            if (value & (1 << (i * 2))) {
-                GIC_SET_MODEL(irq + i);
-            } else {
-                GIC_CLEAR_MODEL(irq + i);
+            if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
+                if (value & (1 << (i * 2))) {
+                    GIC_SET_MODEL(irq + i);
+                } else {
+                    GIC_CLEAR_MODEL(irq + i);
+                }
             }
             if (value & (2 << (i * 2))) {
                 GIC_SET_EDGE_TRIGGER(irq + i);
-- 
2.1.0.rc1

[Qemu-devel] [PATCH 3/3] arm_gic: Do not force PPIs to edge-triggered mode

[Adam Lackorzynski]

Only SGIs must be WI, done by forcing them to their default
(edge-triggered).

Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
---
 hw/intc/arm_gic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index e546647..55019c9 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -558,7 +558,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
         irq = (offset - 0xc00) * 4 + GIC_BASE_IRQ;
         if (irq >= s->num_irq)
             goto bad_reg;
-        if (irq < GIC_INTERNAL)
+        if (irq < GIC_NR_SGIS)
             value |= 0xaa;
         for (i = 0; i < 4; i++) {
             if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
-- 
2.1.0.rc1

Re: [Qemu-devel] [PATCH 2/3] arm_gic: GICD_ICFGR: Write model only for pre v1 GICs

[Christoffer Dall]

On Sat, Aug 16, 2014 at 09:48:20PM +0200, Adam Lackorzynski wrote:
> Setting the model is only available in pre-v1 GIC models.
> ---
>  hw/intc/arm_gic.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index d2b1aaf..e546647 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -561,10 +561,12 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>          if (irq < GIC_INTERNAL)
>              value |= 0xaa;
>          for (i = 0; i < 4; i++) {
> -            if (value & (1 << (i * 2))) {
> -                GIC_SET_MODEL(irq + i);
> -            } else {
> -                GIC_CLEAR_MODEL(irq + i);
> +            if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
> +                if (value & (1 << (i * 2))) {
> +                    GIC_SET_MODEL(irq + i);
> +                } else {
> +                    GIC_CLEAR_MODEL(irq + i);
> +                }
>              }
>              if (value & (2 << (i * 2))) {
>                  GIC_SET_EDGE_TRIGGER(irq + i);
> -- 
> 2.1.0.rc1
> 
looks good,

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

Re: [Qemu-devel] [PATCH 3/3] arm_gic: Do not force PPIs to edge-triggered mode

[Christoffer Dall]

On Sat, Aug 16, 2014 at 09:48:21PM +0200, Adam Lackorzynski wrote:
> Only SGIs must be WI, done by forcing them to their default
> (edge-triggered).
> 
> Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
> ---
>  hw/intc/arm_gic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index e546647..55019c9 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -558,7 +558,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>          irq = (offset - 0xc00) * 4 + GIC_BASE_IRQ;
>          if (irq >= s->num_irq)
>              goto bad_reg;
> -        if (irq < GIC_INTERNAL)
> +        if (irq < GIC_NR_SGIS)
>              value |= 0xaa;
>          for (i = 0; i < 4; i++) {
>              if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
> -- 
> 2.1.0.rc1
> 

where do we ensure that the SGIs are actually configured as
edge-triggered when creating the gic?

-Christoffer

Re: [Qemu-devel] [PATCH 3/3] arm_gic: Do not force PPIs to edge-triggered mode

[Adam Lackorzynski]

On Mon Aug 18, 2014 at 14:48:15 +0200, Christoffer Dall wrote:
> On Sat, Aug 16, 2014 at 09:48:21PM +0200, Adam Lackorzynski wrote:
> > Only SGIs must be WI, done by forcing them to their default
> > (edge-triggered).
> > 
> > Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
> > ---
> >  hw/intc/arm_gic.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> > index e546647..55019c9 100644
> > --- a/hw/intc/arm_gic.c
> > +++ b/hw/intc/arm_gic.c
> > @@ -558,7 +558,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
> >          irq = (offset - 0xc00) * 4 + GIC_BASE_IRQ;
> >          if (irq >= s->num_irq)
> >              goto bad_reg;
> > -        if (irq < GIC_INTERNAL)
> > +        if (irq < GIC_NR_SGIS)
> >              value |= 0xaa;
> >          for (i = 0; i < 4; i++) {
> >              if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
> > -- 
> > 2.1.0.rc1
> > 
> 
> where do we ensure that the SGIs are actually configured as
> edge-triggered when creating the gic?

It's setup in arm_gic_common_reset() in arm_gic_common.c.
(Looks like I should add another change to use GIC_NR_SGIS there too.)



Adam
-- 
Adam                 adam@os.inf.tu-dresden.de
  Lackorzynski         http://os.inf.tu-dresden.de/~adam/

Re: [Qemu-devel] [PATCH 3/3] arm_gic: Do not force PPIs to edge-triggered mode

[Christoffer Dall]

On Mon, Aug 18, 2014 at 02:52:47PM +0200, Adam Lackorzynski wrote:
> On Mon Aug 18, 2014 at 14:48:15 +0200, Christoffer Dall wrote:
> > On Sat, Aug 16, 2014 at 09:48:21PM +0200, Adam Lackorzynski wrote:
> > > Only SGIs must be WI, done by forcing them to their default
> > > (edge-triggered).
> > > 
> > > Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
> > > ---
> > >  hw/intc/arm_gic.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> > > index e546647..55019c9 100644
> > > --- a/hw/intc/arm_gic.c
> > > +++ b/hw/intc/arm_gic.c
> > > @@ -558,7 +558,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
> > >          irq = (offset - 0xc00) * 4 + GIC_BASE_IRQ;
> > >          if (irq >= s->num_irq)
> > >              goto bad_reg;
> > > -        if (irq < GIC_INTERNAL)
> > > +        if (irq < GIC_NR_SGIS)
> > >              value |= 0xaa;
> > >          for (i = 0; i < 4; i++) {
> > >              if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
> > > -- 
> > > 2.1.0.rc1
> > > 
> > 
> > where do we ensure that the SGIs are actually configured as
> > edge-triggered when creating the gic?
> 
> It's setup in arm_gic_common_reset() in arm_gic_common.c.
> (Looks like I should add another change to use GIC_NR_SGIS there too.)
> 
That would be good, for this patch:

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>


Thanks!