[Xen-devel] [PATCH v2 04/45] xen/arm: vgic: Override the group in lr everytime

2018-03-15 Thread Andre Przywara
From: Julien Grall 

At the moment, write_lr is assuming the caller will set correctly the
group. However the group should always be 0 when the guest is using
vGICv2 and 1 for vGICv3. As the caller should not care about the group,
override it directly.

With that change, write_lr is now behaving like update_lr for the group.

Signed-off-by: Julien Grall 
Reviewed-by: Andre Przywara 
Signed-off-by: Andre Przywara 
---
Changes:
- Add Andre's reviewed-by

 xen/arch/arm/gic-v2.c |  4 +---
 xen/arch/arm/gic-v3.c | 11 ---
 xen/include/asm-arm/gic.h |  1 -
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index f16e17c1a3..fc105c08b8 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -469,7 +469,6 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
 lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & 
GICH_V2_LR_PRIORITY_MASK;
 lr_reg->state = (lrv >> GICH_V2_LR_STATE_SHIFT) & 
GICH_V2_LR_STATE_MASK;
 lr_reg->hw_status = (lrv >> GICH_V2_LR_HW_SHIFT) & GICH_V2_LR_HW_MASK;
-lr_reg->grp   = (lrv >> GICH_V2_LR_GRP_SHIFT) & GICH_V2_LR_GRP_MASK;
 }
 
 static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg)
@@ -483,8 +482,7 @@ static void gicv2_write_lr(int lr, const struct gic_lr 
*lr_reg)
   ((uint32_t)(lr_reg->state & GICH_V2_LR_STATE_MASK)
<< GICH_V2_LR_STATE_SHIFT) |
   ((uint32_t)(lr_reg->hw_status & GICH_V2_LR_HW_MASK)
-   << GICH_V2_LR_HW_SHIFT)  |
-  ((uint32_t)(lr_reg->grp & GICH_V2_LR_GRP_MASK) << 
GICH_V2_LR_GRP_SHIFT) );
+   << GICH_V2_LR_HW_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 09b49a07d5..0dfa1a1e08 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1012,7 +1012,6 @@ static void gicv3_read_lr(int lr, struct gic_lr *lr_reg)
 lr_reg->priority  = (lrv >> ICH_LR_PRIORITY_SHIFT) & ICH_LR_PRIORITY_MASK;
 lr_reg->state = (lrv >> ICH_LR_STATE_SHIFT) & ICH_LR_STATE_MASK;
 lr_reg->hw_status = (lrv >> ICH_LR_HW_SHIFT) & ICH_LR_HW_MASK;
-lr_reg->grp   = (lrv >> ICH_LR_GRP_SHIFT) & ICH_LR_GRP_MASK;
 }
 
 static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr)
@@ -1023,8 +1022,14 @@ static void gicv3_write_lr(int lr_reg, const struct 
gic_lr *lr)
 ((u64)(lr->virq & ICH_LR_VIRTUAL_MASK)  << ICH_LR_VIRTUAL_SHIFT) |
 ((u64)(lr->priority & ICH_LR_PRIORITY_MASK) << ICH_LR_PRIORITY_SHIFT)|
 ((u64)(lr->state & ICH_LR_STATE_MASK) << ICH_LR_STATE_SHIFT) |
-((u64)(lr->hw_status & ICH_LR_HW_MASK) << ICH_LR_HW_SHIFT)  |
-((u64)(lr->grp & ICH_LR_GRP_MASK) << ICH_LR_GRP_SHIFT) );
+((u64)(lr->hw_status & ICH_LR_HW_MASK) << ICH_LR_HW_SHIFT) );
+
+/*
+ * When the guest is using vGICv3, all the IRQs are Group 1. Group 0
+ * would result in a FIQ, which will not be expected by the guest OS.
+ */
+if ( current->domain->arch.vgic.version == GIC_V3 )
+lrv |= ICH_LR_GRP1;
 
 gicv3_ich_write_lr(lr_reg, lrv);
 }
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 49cb94f792..1eb08b856e 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -211,7 +211,6 @@ struct gic_lr {
uint8_t priority;
uint8_t state;
uint8_t hw_status;
-   uint8_t grp;
 };
 
 enum gic_version {
-- 
2.14.1


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 04/45] xen/arm: vgic: Override the group in lr everytime

2018-03-16 Thread Stefano Stabellini
On Thu, 15 Mar 2018, Andre Przywara wrote:
> From: Julien Grall 
> 
> At the moment, write_lr is assuming the caller will set correctly the
> group. However the group should always be 0 when the guest is using
> vGICv2 and 1 for vGICv3. As the caller should not care about the group,
> override it directly.
> 
> With that change, write_lr is now behaving like update_lr for the group.
> 
> Signed-off-by: Julien Grall 
> Reviewed-by: Andre Przywara 
> Signed-off-by: Andre Przywara 

Acked-by: Stefano Stabellini 


> ---
> Changes:
> - Add Andre's reviewed-by
> 
>  xen/arch/arm/gic-v2.c |  4 +---
>  xen/arch/arm/gic-v3.c | 11 ---
>  xen/include/asm-arm/gic.h |  1 -
>  3 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index f16e17c1a3..fc105c08b8 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -469,7 +469,6 @@ static void gicv2_read_lr(int lr, struct gic_lr *lr_reg)
>  lr_reg->priority = (lrv >> GICH_V2_LR_PRIORITY_SHIFT) & 
> GICH_V2_LR_PRIORITY_MASK;
>  lr_reg->state = (lrv >> GICH_V2_LR_STATE_SHIFT) & 
> GICH_V2_LR_STATE_MASK;
>  lr_reg->hw_status = (lrv >> GICH_V2_LR_HW_SHIFT) & GICH_V2_LR_HW_MASK;
> -lr_reg->grp   = (lrv >> GICH_V2_LR_GRP_SHIFT) & GICH_V2_LR_GRP_MASK;
>  }
>  
>  static void gicv2_write_lr(int lr, const struct gic_lr *lr_reg)
> @@ -483,8 +482,7 @@ static void gicv2_write_lr(int lr, const struct gic_lr 
> *lr_reg)
>((uint32_t)(lr_reg->state & GICH_V2_LR_STATE_MASK)
> << GICH_V2_LR_STATE_SHIFT) |
>((uint32_t)(lr_reg->hw_status & GICH_V2_LR_HW_MASK)
> -   << GICH_V2_LR_HW_SHIFT)  |
> -  ((uint32_t)(lr_reg->grp & GICH_V2_LR_GRP_MASK) << 
> GICH_V2_LR_GRP_SHIFT) );
> +   << GICH_V2_LR_HW_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 09b49a07d5..0dfa1a1e08 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1012,7 +1012,6 @@ static void gicv3_read_lr(int lr, struct gic_lr *lr_reg)
>  lr_reg->priority  = (lrv >> ICH_LR_PRIORITY_SHIFT) & 
> ICH_LR_PRIORITY_MASK;
>  lr_reg->state = (lrv >> ICH_LR_STATE_SHIFT) & ICH_LR_STATE_MASK;
>  lr_reg->hw_status = (lrv >> ICH_LR_HW_SHIFT) & ICH_LR_HW_MASK;
> -lr_reg->grp   = (lrv >> ICH_LR_GRP_SHIFT) & ICH_LR_GRP_MASK;
>  }
>  
>  static void gicv3_write_lr(int lr_reg, const struct gic_lr *lr)
> @@ -1023,8 +1022,14 @@ static void gicv3_write_lr(int lr_reg, const struct 
> gic_lr *lr)
>  ((u64)(lr->virq & ICH_LR_VIRTUAL_MASK)  << ICH_LR_VIRTUAL_SHIFT) |
>  ((u64)(lr->priority & ICH_LR_PRIORITY_MASK) << 
> ICH_LR_PRIORITY_SHIFT)|
>  ((u64)(lr->state & ICH_LR_STATE_MASK) << ICH_LR_STATE_SHIFT) |
> -((u64)(lr->hw_status & ICH_LR_HW_MASK) << ICH_LR_HW_SHIFT)  |
> -((u64)(lr->grp & ICH_LR_GRP_MASK) << ICH_LR_GRP_SHIFT) );
> +((u64)(lr->hw_status & ICH_LR_HW_MASK) << ICH_LR_HW_SHIFT) );
> +
> +/*
> + * When the guest is using vGICv3, all the IRQs are Group 1. Group 0
> + * would result in a FIQ, which will not be expected by the guest OS.
> + */
> +if ( current->domain->arch.vgic.version == GIC_V3 )
> +lrv |= ICH_LR_GRP1;
>  
>  gicv3_ich_write_lr(lr_reg, lrv);
>  }
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 49cb94f792..1eb08b856e 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -211,7 +211,6 @@ struct gic_lr {
> uint8_t priority;
> uint8_t state;
> uint8_t hw_status;
> -   uint8_t grp;
>  };
>  
>  enum gic_version {
> -- 
> 2.14.1
> 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel