Re: [Xen-devel] [PATCH v2] arm: configure interrupts to be in non-secure group1

[Andre Przywara]

Hi,

On 18/10/17 22:29, Stefano Stabellini wrote:
> Xen uses non-secure group1 interrupts, however it doesn't configure the
> GICv3 accordingly. Xen needs to set GICD_IGROUPR for SPIs and
> GICR_IGROUPR0 for local interrupt to "1" to specify that interrupts
> belong to group1. This is particularly important if the system has
> GICD_CTLR.DS set, also see commit
> 7c9b973061b03af62734f613f6abec46c0dd4a88 in Linux.

Indeed, good catch!
The spec says that those registers initialize to 0, and on normal
hardware this will be adjusted by the secure firmware side.
That's why we didn't see the issue before.
Now with QEMU there might be no secure firmware, also the emulated GIC
only provides a single security state, so we have to set this up ourselves.

> Signed-off-by: Stefano Stabellini 

Reviewed-by: Andre Przywara 


> This is a candidate for stable backports.

It should definitely go into 4.10.

Cheers,
Andre.

> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 74d00e0..77da892 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -569,6 +569,13 @@ static void __init gicv3_dist_init(void)
>  for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 32 )
>  writel_relaxed(0x, GICD + GICD_ICENABLER + (i / 32) * 4);
>  
> +/*
> + * Configure SPIs as non-secure Group-1. This will only matter
> + * if the GIC only has a single security state.
> + */
> +for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 32 )
> +writel_relaxed(GENMASK(31, 0), GICD + GICD_IGROUPR + (i / 32) * 4);
> +
>  gicv3_dist_wait_for_rwp();
>  
>  /* Turn on the distributor */
> @@ -775,6 +782,8 @@ static int gicv3_cpu_init(void)
>   */
>  writel_relaxed(0x, GICD_RDIST_SGI_BASE + GICR_ICENABLER0);
>  writel_relaxed(0x, GICD_RDIST_SGI_BASE + GICR_ISENABLER0);
> +/* Configure SGIs/PPIs as non-secure Group-1 */
> +writel_relaxed(GENMASK(31, 0), GICD_RDIST_SGI_BASE + GICR_IGROUPR0);
>  
>  gicv3_redist_wait_for_rwp();
>  
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
> 

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

Re: [Xen-devel] [PATCH v2] arm: configure interrupts to be in non-secure group1

[Julien Grall]

Hi Stefano,

On 18/10/17 22:29, Stefano Stabellini wrote:

Xen uses non-secure group1 interrupts, however it doesn't configure the
GICv3 accordingly. Xen needs to set GICD_IGROUPR for SPIs and
GICR_IGROUPR0 for local interrupt to "1" to specify that interrupts
belong to group1. This is particularly important if the system has
GICD_CTLR.DS set, also see commit
7c9b973061b03af62734f613f6abec46c0dd4a88 in Linux.

Signed-off-by: Stefano Stabellini 


Reviewed-by: Julien Grall 
Released-acked-by: Julien Grall 

Cheers,



---

This is a candidate for stable backports.

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 74d00e0..77da892 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -569,6 +569,13 @@ static void __init gicv3_dist_init(void)
  for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 32 )
  writel_relaxed(0x, GICD + GICD_ICENABLER + (i / 32) * 4);
  
+/*

+ * Configure SPIs as non-secure Group-1. This will only matter
+ * if the GIC only has a single security state.
+ */
+for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 32 )
+writel_relaxed(GENMASK(31, 0), GICD + GICD_IGROUPR + (i / 32) * 4);
+
  gicv3_dist_wait_for_rwp();
  
  /* Turn on the distributor */

@@ -775,6 +782,8 @@ static int gicv3_cpu_init(void)
   */
  writel_relaxed(0x, GICD_RDIST_SGI_BASE + GICR_ICENABLER0);
  writel_relaxed(0x, GICD_RDIST_SGI_BASE + GICR_ISENABLER0);
+/* Configure SGIs/PPIs as non-secure Group-1 */
+writel_relaxed(GENMASK(31, 0), GICD_RDIST_SGI_BASE + GICR_IGROUPR0);
  
  gicv3_redist_wait_for_rwp();
  


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



--
Julien Grall

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

[Xen-devel] [PATCH v2] arm: configure interrupts to be in non-secure group1

[Stefano Stabellini]

Xen uses non-secure group1 interrupts, however it doesn't configure the
GICv3 accordingly. Xen needs to set GICD_IGROUPR for SPIs and
GICR_IGROUPR0 for local interrupt to "1" to specify that interrupts
belong to group1. This is particularly important if the system has
GICD_CTLR.DS set, also see commit
7c9b973061b03af62734f613f6abec46c0dd4a88 in Linux.

Signed-off-by: Stefano Stabellini 

---

This is a candidate for stable backports.

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 74d00e0..77da892 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -569,6 +569,13 @@ static void __init gicv3_dist_init(void)
 for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 32 )
 writel_relaxed(0x, GICD + GICD_ICENABLER + (i / 32) * 4);
 
+/*
+ * Configure SPIs as non-secure Group-1. This will only matter
+ * if the GIC only has a single security state.
+ */
+for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 32 )
+writel_relaxed(GENMASK(31, 0), GICD + GICD_IGROUPR + (i / 32) * 4);
+
 gicv3_dist_wait_for_rwp();
 
 /* Turn on the distributor */
@@ -775,6 +782,8 @@ static int gicv3_cpu_init(void)
  */
 writel_relaxed(0x, GICD_RDIST_SGI_BASE + GICR_ICENABLER0);
 writel_relaxed(0x, GICD_RDIST_SGI_BASE + GICR_ISENABLER0);
+/* Configure SGIs/PPIs as non-secure Group-1 */
+writel_relaxed(GENMASK(31, 0), GICD_RDIST_SGI_BASE + GICR_IGROUPR0);
 
 gicv3_redist_wait_for_rwp();
 

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