[PATCH v2] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI

[PATCH v2] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI

[Stefano Stabellini]

GICH_LR_HW doesn't work as expected on X-Gene: request maintenance
interrupts and perform EOIs in the hypervisor as a workaround.
Trigger this behaviour with a per platform option.

This patch assumes that GICC_DIR can be written on any pcpu for a given
IRQ, not matter where GICC_IAR has been read before.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: psawargaonkar@apm.com
CC: apatel@apm.com
---

Changes in v2:
- s/PLATFORM_QUIRK_NEED_EOI/PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI/g;
- improved commit message.

---

 xen/arch/arm/gic.c                   |   13 ++++++++++++-
 xen/arch/arm/platforms/xgene-storm.c |    2 +-
 xen/include/asm-arm/platform.h       |    5 +++++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 1965f86..3e58c9c 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -577,7 +577,9 @@ static inline void gic_set_lr(int lr, struct pending_irq *p,
 
     lr_val = state | (GIC_PRI_TO_GUEST(p->priority) << GICH_LR_PRIORITY_SHIFT) |
         ((p->irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT);
-    if ( p->desc != NULL )
+    if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) )
+        lr_val |= GICH_LR_MAINTENANCE_IRQ;
+    else if ( p->desc != NULL )
         lr_val |= GICH_LR_HW | (p->desc->irq << GICH_LR_PHYSICAL_SHIFT);
 
     GICH[GICH_LR + lr] = lr_val;
@@ -656,6 +658,11 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int virtual_irq,
     gic_add_to_lr_pending(v, irq_to_pending(v, virtual_irq));
 }
 
+static void gic_irq_eoi(int irq)
+{
+    GICC[GICC_DIR] = irq;
+}
+
 static void gic_update_one_lr(struct vcpu *v, int i)
 {
     struct pending_irq *p;
@@ -692,7 +699,11 @@ static void gic_update_one_lr(struct vcpu *v, int i)
         clear_bit(i, &this_cpu(lr_mask));
 
         if ( p->desc != NULL )
+        {
             p->desc->status &= ~IRQ_INPROGRESS;
+            if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) )
+                gic_irq_eoi(p->irq);
+        }
         clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
         clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
         p->lr = GIC_INVALID_LR;
diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
index c9dd63c..837d8e6 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -37,7 +37,7 @@ static bool reset_vals_valid = false;
 
 static uint32_t xgene_storm_quirks(void)
 {
-    return PLATFORM_QUIRK_GIC_64K_STRIDE;
+    return PLATFORM_QUIRK_GIC_64K_STRIDE|PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI;
 }
 
 static int map_one_mmio(struct domain *d, const char *what,
diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
index bcd2097..eefaca6 100644
--- a/xen/include/asm-arm/platform.h
+++ b/xen/include/asm-arm/platform.h
@@ -55,6 +55,11 @@ struct platform_desc {
  */
 #define PLATFORM_QUIRK_GIC_64K_STRIDE (1 << 0)
 
+/*
+ * Quirk for platforms where GICH_LR_HW does not work as expected.
+ */
+#define PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI       (1 << 1)
+
 void __init platform_init(void);
 int __init platform_init_time(void);
 int __init platform_specific_mapping(struct domain *d);
-- 
1.7.10.4

Re: [PATCH v2] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI

[Julien Grall]

Hi Stefano,

Sorry I didn't think about it before. Ian has pushed Vijay GICv2 rework
today. So you patch won't apply correctly.

On 07/02/2014 04:33 PM, Stefano Stabellini wrote:
> GICH_LR_HW doesn't work as expected on X-Gene: request maintenance
> interrupts and perform EOIs in the hypervisor as a workaround.
> Trigger this behaviour with a per platform option.
> 
> This patch assumes that GICC_DIR can be written on any pcpu for a given
> IRQ, not matter where GICC_IAR has been read before.

I would replace IRQ by SGIs as if it's an SPI/PPI we are screwed. In
practice it doesn't happen :).

Regards,


-- 
Julien Grall

Re: [PATCH v2] xen/arm: introduce PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI

[Stefano Stabellini]

On Wed, 2 Jul 2014, Stefano Stabellini wrote:
> GICH_LR_HW doesn't work as expected on X-Gene: request maintenance
> interrupts and perform EOIs in the hypervisor as a workaround.
> Trigger this behaviour with a per platform option.
> 
> This patch assumes that GICC_DIR can be written on any pcpu for a given
> IRQ, not matter where GICC_IAR has been read before.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> CC: psawargaonkar@apm.com
> CC: apatel@apm.com
> ---
> 
> Changes in v2:
> - s/PLATFORM_QUIRK_NEED_EOI/PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI/g;
> - improved commit message.
> 
> ---
> 
>  xen/arch/arm/gic.c                   |   13 ++++++++++++-
>  xen/arch/arm/platforms/xgene-storm.c |    2 +-
>  xen/include/asm-arm/platform.h       |    5 +++++
>  3 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 1965f86..3e58c9c 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -577,7 +577,9 @@ static inline void gic_set_lr(int lr, struct pending_irq *p,
>  
>      lr_val = state | (GIC_PRI_TO_GUEST(p->priority) << GICH_LR_PRIORITY_SHIFT) |
>          ((p->irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT);
> -    if ( p->desc != NULL )
> +    if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) )
> +        lr_val |= GICH_LR_MAINTENANCE_IRQ;
> +    else if ( p->desc != NULL )
>          lr_val |= GICH_LR_HW | (p->desc->irq << GICH_LR_PHYSICAL_SHIFT);

Actually I had a better idea: we could still avoid maintenance
interrupts for virtual interrupts (vtimer and evtchn_irq).
I'll resubmit.



>      GICH[GICH_LR + lr] = lr_val;
> @@ -656,6 +658,11 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int virtual_irq,
>      gic_add_to_lr_pending(v, irq_to_pending(v, virtual_irq));
>  }
>  
> +static void gic_irq_eoi(int irq)
> +{
> +    GICC[GICC_DIR] = irq;
> +}
> +
>  static void gic_update_one_lr(struct vcpu *v, int i)
>  {
>      struct pending_irq *p;
> @@ -692,7 +699,11 @@ static void gic_update_one_lr(struct vcpu *v, int i)
>          clear_bit(i, &this_cpu(lr_mask));
>  
>          if ( p->desc != NULL )
> +        {
>              p->desc->status &= ~IRQ_INPROGRESS;
> +            if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) )
> +                gic_irq_eoi(p->irq);
> +        }
>          clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
>          clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
>          p->lr = GIC_INVALID_LR;
> diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
> index c9dd63c..837d8e6 100644
> --- a/xen/arch/arm/platforms/xgene-storm.c
> +++ b/xen/arch/arm/platforms/xgene-storm.c
> @@ -37,7 +37,7 @@ static bool reset_vals_valid = false;
>  
>  static uint32_t xgene_storm_quirks(void)
>  {
> -    return PLATFORM_QUIRK_GIC_64K_STRIDE;
> +    return PLATFORM_QUIRK_GIC_64K_STRIDE|PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI;
>  }
>  
>  static int map_one_mmio(struct domain *d, const char *what,
> diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
> index bcd2097..eefaca6 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -55,6 +55,11 @@ struct platform_desc {
>   */
>  #define PLATFORM_QUIRK_GIC_64K_STRIDE (1 << 0)
>  
> +/*
> + * Quirk for platforms where GICH_LR_HW does not work as expected.
> + */
> +#define PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI       (1 << 1)
> +
>  void __init platform_init(void);
>  int __init platform_init_time(void);
>  int __init platform_specific_mapping(struct domain *d);
> -- 
> 1.7.10.4
>