[Xen-devel,v2,10/10] ARM: VGIC: rework gicv[23]_update_lr to not use pending_irq

Message ID 20171207161415.20380-11-andre.przywara@linaro.org
State New
Headers show
Series
  • ARM: VGIC/GIC separation cleanups
Related show

Commit Message

Andre Przywara Dec. 7, 2017, 4:14 p.m.
The functions to actually populate a list register were accessing
the VGIC internal pending_irq struct, although they should be abstracting
from that.
Break the needed information down to remove the reference to pending_irq
from gic-v[23].c.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 xen/arch/arm/gic-v2.c     | 14 +++++++-------
 xen/arch/arm/gic-v3.c     | 12 ++++++------
 xen/arch/arm/gic-vgic.c   |  3 ++-
 xen/include/asm-arm/gic.h |  4 ++--
 xen/include/asm-arm/irq.h |  3 +++
 5 files changed, 20 insertions(+), 16 deletions(-)

Comments

Stefano Stabellini Dec. 8, 2017, 10:05 p.m. | #1
On Thu, 7 Dec 2017, Andre Przywara wrote:
> The functions to actually populate a list register were accessing
> the VGIC internal pending_irq struct, although they should be abstracting
> from that.
> Break the needed information down to remove the reference to pending_irq
> from gic-v[23].c.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/gic-v2.c     | 14 +++++++-------
>  xen/arch/arm/gic-v3.c     | 12 ++++++------
>  xen/arch/arm/gic-vgic.c   |  3 ++-
>  xen/include/asm-arm/gic.h |  4 ++--
>  xen/include/asm-arm/irq.h |  3 +++
>  5 files changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 511c8d7294..2b271ba322 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -428,8 +428,8 @@ static void gicv2_disable_interface(void)
>      spin_unlock(&gicv2.lock);
>  }
>  
> -static void gicv2_update_lr(int lr, const struct pending_irq *p,
> -                            unsigned int state)
> +static void gicv2_update_lr(int lr, unsigned int virq, uint8_t priority,
> +                            unsigned int hw_irq, unsigned int state)
>  {
>      uint32_t lr_reg;
>  
> @@ -437,12 +437,12 @@ static void gicv2_update_lr(int lr, const struct pending_irq *p,
>      BUG_ON(lr < 0);
>  
>      lr_reg = (((state & GICH_V2_LR_STATE_MASK) << GICH_V2_LR_STATE_SHIFT)  |
> -              ((GIC_PRI_TO_GUEST(p->priority) & GICH_V2_LR_PRIORITY_MASK)
> -                                             << GICH_V2_LR_PRIORITY_SHIFT) |
> -              ((p->irq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT));
> +              ((GIC_PRI_TO_GUEST(priority) & GICH_V2_LR_PRIORITY_MASK)
> +                                          << GICH_V2_LR_PRIORITY_SHIFT) |
> +              ((virq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT));
>  
> -    if ( p->desc != NULL )
> -        lr_reg |= GICH_V2_LR_HW | ((p->desc->irq & GICH_V2_LR_PHYSICAL_MASK )
> +    if ( hw_irq != INVALID_IRQ )
> +        lr_reg |= GICH_V2_LR_HW | ((hw_irq & GICH_V2_LR_PHYSICAL_MASK )
>                                     << GICH_V2_LR_PHYSICAL_SHIFT);
>  
>      writel_gich(lr_reg, GICH_LR + lr * 4);
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 473e26111f..ce1e5cad25 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -962,8 +962,8 @@ static void gicv3_disable_interface(void)
>      spin_unlock(&gicv3.lock);
>  }
>  
> -static void gicv3_update_lr(int lr, const struct pending_irq *p,
> -                            unsigned int state)
> +static void gicv3_update_lr(int lr, unsigned int virq, uint8_t priority,
> +                            unsigned int hw_irq, unsigned int state)
>  {
>      uint64_t val = 0;
>  
> @@ -979,11 +979,11 @@ static void gicv3_update_lr(int lr, const struct pending_irq *p,
>      if ( current->domain->arch.vgic.version == GIC_V3 )
>          val |= GICH_LR_GRP1;
>  
> -    val |= ((uint64_t)p->priority & 0xff) << GICH_LR_PRIORITY_SHIFT;
> -    val |= ((uint64_t)p->irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT;
> +    val |= (uint64_t)priority << GICH_LR_PRIORITY_SHIFT;
> +    val |= ((uint64_t)virq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT;
>  
> -   if ( p->desc != NULL )
> -       val |= GICH_LR_HW | (((uint64_t)p->desc->irq & GICH_LR_PHYSICAL_MASK)
> +   if ( hw_irq != INVALID_IRQ )
> +       val |= GICH_LR_HW | (((uint64_t)hw_irq & GICH_LR_PHYSICAL_MASK)
>                             << GICH_LR_PHYSICAL_SHIFT);
>  
>      gicv3_ich_write_lr(lr, val);
> diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
> index 8d43a6ba76..60f6498092 100644
> --- a/xen/arch/arm/gic-vgic.c
> +++ b/xen/arch/arm/gic-vgic.c
> @@ -52,7 +52,8 @@ static inline void gic_set_lr(int lr, struct pending_irq *p,
>  
>      clear_bit(GIC_IRQ_GUEST_PRISTINE_LPI, &p->status);
>  
> -    gic_hw_ops->update_lr(lr, p, state);
> +    gic_hw_ops->update_lr(lr, p->irq, p->priority,
> +                          p->desc ? p->desc->irq : INVALID_IRQ, state);
>  
>      set_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
>      clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status);
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 4f4fd555c1..ce9d1d058a 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -342,8 +342,8 @@ struct gic_hw_operations {
>      /* Disable CPU physical and virtual interfaces */
>      void (*disable_interface)(void);
>      /* Update LR register with state and priority */
> -    void (*update_lr)(int lr, const struct pending_irq *pending_irq,
> -                      unsigned int state);
> +    void (*update_lr)(int lr, unsigned int virq, uint8_t priority,
> +                      unsigned int hw_irq, unsigned int state);
>      /* Update HCR status register */
>      void (*update_hcr_status)(uint32_t flag, bool set);
>      /* Clear LR register */
> diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
> index abc8f06a13..0d110ecb08 100644
> --- a/xen/include/asm-arm/irq.h
> +++ b/xen/include/asm-arm/irq.h
> @@ -31,6 +31,9 @@ struct arch_irq_desc {
>  /* LPIs are always numbered starting at 8192, so 0 is a good invalid case. */
>  #define INVALID_LPI     0
>  
> +/* This is a spurious interrupt ID which never makes it into the GIC code. */
> +#define INVALID_IRQ     1023
> +
>  extern unsigned int nr_irqs;
>  #define nr_static_irqs NR_IRQS
>  #define arch_hwdom_irqs(domid) NR_IRQS
> -- 
> 2.14.1
>

Patch

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 511c8d7294..2b271ba322 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -428,8 +428,8 @@  static void gicv2_disable_interface(void)
     spin_unlock(&gicv2.lock);
 }
 
-static void gicv2_update_lr(int lr, const struct pending_irq *p,
-                            unsigned int state)
+static void gicv2_update_lr(int lr, unsigned int virq, uint8_t priority,
+                            unsigned int hw_irq, unsigned int state)
 {
     uint32_t lr_reg;
 
@@ -437,12 +437,12 @@  static void gicv2_update_lr(int lr, const struct pending_irq *p,
     BUG_ON(lr < 0);
 
     lr_reg = (((state & GICH_V2_LR_STATE_MASK) << GICH_V2_LR_STATE_SHIFT)  |
-              ((GIC_PRI_TO_GUEST(p->priority) & GICH_V2_LR_PRIORITY_MASK)
-                                             << GICH_V2_LR_PRIORITY_SHIFT) |
-              ((p->irq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT));
+              ((GIC_PRI_TO_GUEST(priority) & GICH_V2_LR_PRIORITY_MASK)
+                                          << GICH_V2_LR_PRIORITY_SHIFT) |
+              ((virq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT));
 
-    if ( p->desc != NULL )
-        lr_reg |= GICH_V2_LR_HW | ((p->desc->irq & GICH_V2_LR_PHYSICAL_MASK )
+    if ( hw_irq != INVALID_IRQ )
+        lr_reg |= GICH_V2_LR_HW | ((hw_irq & GICH_V2_LR_PHYSICAL_MASK )
                                    << GICH_V2_LR_PHYSICAL_SHIFT);
 
     writel_gich(lr_reg, GICH_LR + lr * 4);
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 473e26111f..ce1e5cad25 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -962,8 +962,8 @@  static void gicv3_disable_interface(void)
     spin_unlock(&gicv3.lock);
 }
 
-static void gicv3_update_lr(int lr, const struct pending_irq *p,
-                            unsigned int state)
+static void gicv3_update_lr(int lr, unsigned int virq, uint8_t priority,
+                            unsigned int hw_irq, unsigned int state)
 {
     uint64_t val = 0;
 
@@ -979,11 +979,11 @@  static void gicv3_update_lr(int lr, const struct pending_irq *p,
     if ( current->domain->arch.vgic.version == GIC_V3 )
         val |= GICH_LR_GRP1;
 
-    val |= ((uint64_t)p->priority & 0xff) << GICH_LR_PRIORITY_SHIFT;
-    val |= ((uint64_t)p->irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT;
+    val |= (uint64_t)priority << GICH_LR_PRIORITY_SHIFT;
+    val |= ((uint64_t)virq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT;
 
-   if ( p->desc != NULL )
-       val |= GICH_LR_HW | (((uint64_t)p->desc->irq & GICH_LR_PHYSICAL_MASK)
+   if ( hw_irq != INVALID_IRQ )
+       val |= GICH_LR_HW | (((uint64_t)hw_irq & GICH_LR_PHYSICAL_MASK)
                            << GICH_LR_PHYSICAL_SHIFT);
 
     gicv3_ich_write_lr(lr, val);
diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
index 8d43a6ba76..60f6498092 100644
--- a/xen/arch/arm/gic-vgic.c
+++ b/xen/arch/arm/gic-vgic.c
@@ -52,7 +52,8 @@  static inline void gic_set_lr(int lr, struct pending_irq *p,
 
     clear_bit(GIC_IRQ_GUEST_PRISTINE_LPI, &p->status);
 
-    gic_hw_ops->update_lr(lr, p, state);
+    gic_hw_ops->update_lr(lr, p->irq, p->priority,
+                          p->desc ? p->desc->irq : INVALID_IRQ, state);
 
     set_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
     clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status);
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 4f4fd555c1..ce9d1d058a 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -342,8 +342,8 @@  struct gic_hw_operations {
     /* Disable CPU physical and virtual interfaces */
     void (*disable_interface)(void);
     /* Update LR register with state and priority */
-    void (*update_lr)(int lr, const struct pending_irq *pending_irq,
-                      unsigned int state);
+    void (*update_lr)(int lr, unsigned int virq, uint8_t priority,
+                      unsigned int hw_irq, unsigned int state);
     /* Update HCR status register */
     void (*update_hcr_status)(uint32_t flag, bool set);
     /* Clear LR register */
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index abc8f06a13..0d110ecb08 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -31,6 +31,9 @@  struct arch_irq_desc {
 /* LPIs are always numbered starting at 8192, so 0 is a good invalid case. */
 #define INVALID_LPI     0
 
+/* This is a spurious interrupt ID which never makes it into the GIC code. */
+#define INVALID_IRQ     1023
+
 extern unsigned int nr_irqs;
 #define nr_static_irqs NR_IRQS
 #define arch_hwdom_irqs(domid) NR_IRQS