diff mbox series

[Xen-devel,v2,07/45] xen/arm: GIC: Only set pirq in the LR when hw_status is set

Message ID 20180315203050.19791-8-andre.przywara@linaro.org
State New
Headers show
Series New VGIC(-v2) implementation | expand

Commit Message

Andre Przywara March 15, 2018, 8:30 p.m. UTC 
From: Julien Grall <julien.grall@arm.com>

The field pirq should only be valid when the virtual interrupt
is associated to a physical interrupt.

This change will help to extend gic_lr for supporting specific virtual
interrupt field (e.g eoi, source) that clashes with the PIRQ field.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
Changes:
- Add Andre's reviewed-by

 xen/arch/arm/gic-v2.c     | 13 ++++++++++---
 xen/arch/arm/gic-v3.c     | 10 +++++++---
 xen/include/asm-arm/gic.h |  2 +-
 3 files changed, 18 insertions(+), 7 deletions(-)

Comments

Stefano Stabellini March 16, 2018, 9:38 p.m. UTC | #1 
On Thu, 15 Mar 2018, Andre Przywara wrote:
> From: Julien Grall <julien.grall@arm.com>
> 
> The field pirq should only be valid when the virtual interrupt
> is associated to a physical interrupt.
> 
> This change will help to extend gic_lr for supporting specific virtual
> interrupt field (e.g eoi, source) that clashes with the PIRQ field.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

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

> ---
> Changes:
> - Add Andre's reviewed-by
> 
>  xen/arch/arm/gic-v2.c     | 13 ++++++++++---
>  xen/arch/arm/gic-v3.c     | 10 +++++++---
>  xen/include/asm-arm/gic.h |  2 +-
>  3 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 6dae5c1e55..2f012692e0 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -466,20 +466,24 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
>      uint32_t lrv;
>  
>      lrv          = readl_gich(GICH_LR + lr * 4);
> -    lr_reg->pirq = (lrv >> GICH_V2_LR_PHYSICAL_SHIFT) & GICH_V2_LR_PHYSICAL_MASK;
>      lr_reg->virq = (lrv >> GICH_V2_LR_VIRTUAL_SHIFT) & GICH_V2_LR_VIRTUAL_MASK;
>      lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & GICH_V2_LR_PRIORITY_MASK;
>      lr_reg->pending = lrv & GICH_V2_LR_PENDING;
>      lr_reg->active = lrv & GICH_V2_LR_ACTIVE;
>      lr_reg->hw_status = lrv & GICH_V2_LR_HW;
> +
> +    if ( lr_reg->hw_status )
> +    {
> +        lr_reg->pirq = lrv >> GICH_V2_LR_PHYSICAL_SHIFT;
> +        lr_reg->pirq &= GICH_V2_LR_PHYSICAL_MASK;
> +    }
>  }
>  
>  static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg)
>  {
>      uint32_t lrv = 0;
>  
> -    lrv = ( ((lr_reg->pirq & GICH_V2_LR_PHYSICAL_MASK) << GICH_V2_LR_PHYSICAL_SHIFT) |
> -          ((lr_reg->virq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT)   |
> +    lrv = (((lr_reg->virq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT)   |
>            ((uint32_t)(lr_reg->priority & GICH_V2_LR_PRIORITY_MASK)
>                                        << GICH_V2_LR_PRIORITY_SHIFT) );
>  
> @@ -490,7 +494,10 @@ static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg)
>          lrv |= GICH_V2_LR_PENDING;
>  
>      if ( lr_reg->hw_status )
> +    {
>          lrv |= GICH_V2_LR_HW;
> +        lrv |= lr_reg->pirq << GICH_V2_LR_PHYSICAL_SHIFT;
> +    }
>  
>      writel_gich(lrv, GICH_LR + lr * 4);
>  }
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 6547b5eb0d..e901210b78 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1006,21 +1006,22 @@ static void gicv3_read_lr(int lr, struct gic_lr *lr_reg)
>  
>      lrv = gicv3_ich_read_lr(lr);
>  
> -    lr_reg->pirq = (lrv >> ICH_LR_PHYSICAL_SHIFT) & ICH_LR_PHYSICAL_MASK;
>      lr_reg->virq = (lrv >> ICH_LR_VIRTUAL_SHIFT) & ICH_LR_VIRTUAL_MASK;
>  
>      lr_reg->priority  = (lrv >> ICH_LR_PRIORITY_SHIFT) & ICH_LR_PRIORITY_MASK;
>      lr_reg->pending   = lrv & ICH_LR_STATE_PENDING;
>      lr_reg->active    = lrv & ICH_LR_STATE_ACTIVE;
>      lr_reg->hw_status = lrv & ICH_LR_HW;
> +
> +    if ( lr_reg->hw_status )
> +        lr_reg->pirq = (lrv >> ICH_LR_PHYSICAL_SHIFT) & ICH_LR_PHYSICAL_MASK;
>  }
>  
>  static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr)
>  {
>      uint64_t lrv = 0;
>  
> -    lrv = ( ((u64)(lr->pirq & ICH_LR_PHYSICAL_MASK) << ICH_LR_PHYSICAL_SHIFT)|
> -        ((u64)(lr->virq & ICH_LR_VIRTUAL_MASK)  << ICH_LR_VIRTUAL_SHIFT) |
> +    lrv = ( ((u64)(lr->virq & ICH_LR_VIRTUAL_MASK)  << ICH_LR_VIRTUAL_SHIFT) |
>          ((u64)(lr->priority & ICH_LR_PRIORITY_MASK) << ICH_LR_PRIORITY_SHIFT) );
>  
>      if ( lr->active )
> @@ -1030,7 +1031,10 @@ static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr)
>          lrv |= ICH_LR_STATE_PENDING;
>  
>      if ( lr->hw_status )
> +    {
>          lrv |= ICH_LR_HW;
> +        lrv |= (uint64_t)lr->pirq << ICH_LR_PHYSICAL_SHIFT;
> +    }
>  
>      /*
>       * When the guest is using vGICv3, all the IRQs are Group 1. Group 0
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index c32861d4fa..545901b120 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -204,7 +204,7 @@ union gic_state_data {
>   * The LR register format is different for GIC HW version
>   */
>  struct gic_lr {
> -   /* Physical IRQ */
> +   /* Physical IRQ -> Only set when hw_status is set. */
>     uint32_t pirq;
>     /* Virtual IRQ */
>     uint32_t virq;
> -- 
> 2.14.1
>
diff mbox series

Patch

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 6dae5c1e55..2f012692e0 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -466,20 +466,24 @@  static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
     uint32_t lrv;
 
     lrv          = readl_gich(GICH_LR + lr * 4);
-    lr_reg->pirq = (lrv >> GICH_V2_LR_PHYSICAL_SHIFT) & GICH_V2_LR_PHYSICAL_MASK;
     lr_reg->virq = (lrv >> GICH_V2_LR_VIRTUAL_SHIFT) & GICH_V2_LR_VIRTUAL_MASK;
     lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & GICH_V2_LR_PRIORITY_MASK;
     lr_reg->pending = lrv & GICH_V2_LR_PENDING;
     lr_reg->active = lrv & GICH_V2_LR_ACTIVE;
     lr_reg->hw_status = lrv & GICH_V2_LR_HW;
+
+    if ( lr_reg->hw_status )
+    {
+        lr_reg->pirq = lrv >> GICH_V2_LR_PHYSICAL_SHIFT;
+        lr_reg->pirq &= GICH_V2_LR_PHYSICAL_MASK;
+    }
 }
 
 static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg)
 {
     uint32_t lrv = 0;
 
-    lrv = ( ((lr_reg->pirq & GICH_V2_LR_PHYSICAL_MASK) << GICH_V2_LR_PHYSICAL_SHIFT) |
-          ((lr_reg->virq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT)   |
+    lrv = (((lr_reg->virq & GICH_V2_LR_VIRTUAL_MASK) << GICH_V2_LR_VIRTUAL_SHIFT)   |
           ((uint32_t)(lr_reg->priority & GICH_V2_LR_PRIORITY_MASK)
                                       << GICH_V2_LR_PRIORITY_SHIFT) );
 
@@ -490,7 +494,10 @@  static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg)
         lrv |= GICH_V2_LR_PENDING;
 
     if ( lr_reg->hw_status )
+    {
         lrv |= GICH_V2_LR_HW;
+        lrv |= lr_reg->pirq << GICH_V2_LR_PHYSICAL_SHIFT;
+    }
 
     writel_gich(lrv, GICH_LR + lr * 4);
 }
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 6547b5eb0d..e901210b78 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1006,21 +1006,22 @@  static void gicv3_read_lr(int lr, struct gic_lr *lr_reg)
 
     lrv = gicv3_ich_read_lr(lr);
 
-    lr_reg->pirq = (lrv >> ICH_LR_PHYSICAL_SHIFT) & ICH_LR_PHYSICAL_MASK;
     lr_reg->virq = (lrv >> ICH_LR_VIRTUAL_SHIFT) & ICH_LR_VIRTUAL_MASK;
 
     lr_reg->priority  = (lrv >> ICH_LR_PRIORITY_SHIFT) & ICH_LR_PRIORITY_MASK;
     lr_reg->pending   = lrv & ICH_LR_STATE_PENDING;
     lr_reg->active    = lrv & ICH_LR_STATE_ACTIVE;
     lr_reg->hw_status = lrv & ICH_LR_HW;
+
+    if ( lr_reg->hw_status )
+        lr_reg->pirq = (lrv >> ICH_LR_PHYSICAL_SHIFT) & ICH_LR_PHYSICAL_MASK;
 }
 
 static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr)
 {
     uint64_t lrv = 0;
 
-    lrv = ( ((u64)(lr->pirq & ICH_LR_PHYSICAL_MASK) << ICH_LR_PHYSICAL_SHIFT)|
-        ((u64)(lr->virq & ICH_LR_VIRTUAL_MASK)  << ICH_LR_VIRTUAL_SHIFT) |
+    lrv = ( ((u64)(lr->virq & ICH_LR_VIRTUAL_MASK)  << ICH_LR_VIRTUAL_SHIFT) |
         ((u64)(lr->priority & ICH_LR_PRIORITY_MASK) << ICH_LR_PRIORITY_SHIFT) );
 
     if ( lr->active )
@@ -1030,7 +1031,10 @@  static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr)
         lrv |= ICH_LR_STATE_PENDING;
 
     if ( lr->hw_status )
+    {
         lrv |= ICH_LR_HW;
+        lrv |= (uint64_t)lr->pirq << ICH_LR_PHYSICAL_SHIFT;
+    }
 
     /*
      * When the guest is using vGICv3, all the IRQs are Group 1. Group 0
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index c32861d4fa..545901b120 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -204,7 +204,7 @@  union gic_state_data {
  * The LR register format is different for GIC HW version
  */
 struct gic_lr {
-   /* Physical IRQ */
+   /* Physical IRQ -> Only set when hw_status is set. */
    uint32_t pirq;
    /* Virtual IRQ */
    uint32_t virq;