[PATCH for-4.10] xen/arm: gic-v3: Make sure ICC_SRE_EL1 is restored before ICH_VMCR_EL2

[PATCH for-4.10] xen/arm: gic-v3: Make sure ICC_SRE_EL1 is restored before ICH_VMCR_EL2

[Julien Grall]

Per 8.4.8 in ARM IHI 0069D, ICH_VMCR_EL2.VFIQEn is RES1 when
ICC_SRE_EL1.SRE is 1. This causes a Group 0 interrupt (as generated in
GICv2 mode) to be delivered as a FIQ to the guest, with potentially
consequence. So we must make sure that ICC_SRE_EL1 has been actually
programmed before at ICH_VMCR_EL2.

This was discovered when booting EFI in a GICv2 guest on a GICv3
hardware.

Signed-off-by: Julien Grall <julien.grall@linaro.org>

---

This patch should be backported up to Xen 4.7.
---
 xen/arch/arm/gic-v3.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 74d00e0c54..b8aff77a6c 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -392,7 +392,16 @@ static void gicv3_restore_state(const struct vcpu *v)
         val |= GICC_SRE_EL2_ENEL1;
     WRITE_SYSREG32(val, ICC_SRE_EL2);
 
+    /*
+     * VFIQEn is RES1 if ICC_SRE_EL1.SRE is 1. This causes a Group0
+     * interrupt (as generated in GICv2 mode) to be delivered as a FIQ
+     * to the guest, with potentially consequence. So we must make sure
+     * that ICC_SRE_EL1 has been actually programmed with the value we
+     * want before starting to mess with the rest of the GIC, and
+     * VMCR_EL1 in particular.
+     */
     WRITE_SYSREG32(v->arch.gic.v3.sre_el1, ICC_SRE_EL1);
+    isb();
     WRITE_SYSREG32(v->arch.gic.v3.vmcr, ICH_VMCR_EL2);
     restore_aprn_regs(&v->arch.gic);
     gicv3_restore_lrs(v);
-- 
2.11.0


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

Re: [PATCH for-4.10] xen/arm: gic-v3: Make sure ICC_SRE_EL1 is restored before ICH_VMCR_EL2

[Stefano Stabellini]

On Thu, 19 Oct 2017, Julien Grall wrote:
> Per 8.4.8 in ARM IHI 0069D, ICH_VMCR_EL2.VFIQEn is RES1 when
> ICC_SRE_EL1.SRE is 1. This causes a Group 0 interrupt (as generated in
> GICv2 mode) to be delivered as a FIQ to the guest, with potentially
> consequence. So we must make sure that ICC_SRE_EL1 has been actually
> programmed before at ICH_VMCR_EL2.
> 
> This was discovered when booting EFI in a GICv2 guest on a GICv3
> hardware.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Nice catch!

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


> ---
> 
> This patch should be backported up to Xen 4.7.
> ---
>  xen/arch/arm/gic-v3.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 74d00e0c54..b8aff77a6c 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -392,7 +392,16 @@ static void gicv3_restore_state(const struct vcpu *v)
>          val |= GICC_SRE_EL2_ENEL1;
>      WRITE_SYSREG32(val, ICC_SRE_EL2);
>  
> +    /*
> +     * VFIQEn is RES1 if ICC_SRE_EL1.SRE is 1. This causes a Group0
> +     * interrupt (as generated in GICv2 mode) to be delivered as a FIQ
> +     * to the guest, with potentially consequence. So we must make sure
> +     * that ICC_SRE_EL1 has been actually programmed with the value we
> +     * want before starting to mess with the rest of the GIC, and
> +     * VMCR_EL1 in particular.
> +     */
>      WRITE_SYSREG32(v->arch.gic.v3.sre_el1, ICC_SRE_EL1);
> +    isb();
>      WRITE_SYSREG32(v->arch.gic.v3.vmcr, ICH_VMCR_EL2);
>      restore_aprn_regs(&v->arch.gic);
>      gicv3_restore_lrs(v);
> -- 
> 2.11.0
> 

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

Re: [PATCH for-4.10] xen/arm: gic-v3: Make sure ICC_SRE_EL1 is restored before ICH_VMCR_EL2

[Stefano Stabellini]

On Thu, 19 Oct 2017, Stefano Stabellini wrote:
> On Thu, 19 Oct 2017, Julien Grall wrote:
> > Per 8.4.8 in ARM IHI 0069D, ICH_VMCR_EL2.VFIQEn is RES1 when
> > ICC_SRE_EL1.SRE is 1. This causes a Group 0 interrupt (as generated in
> > GICv2 mode) to be delivered as a FIQ to the guest, with potentially
> > consequence. So we must make sure that ICC_SRE_EL1 has been actually
> > programmed before at ICH_VMCR_EL2.
> > 
> > This was discovered when booting EFI in a GICv2 guest on a GICv3
> > hardware.
> > 
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> Nice catch!
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

Sorry wrong tag :)


> 
> > ---
> > 
> > This patch should be backported up to Xen 4.7.
> > ---
> >  xen/arch/arm/gic-v3.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> > index 74d00e0c54..b8aff77a6c 100644
> > --- a/xen/arch/arm/gic-v3.c
> > +++ b/xen/arch/arm/gic-v3.c
> > @@ -392,7 +392,16 @@ static void gicv3_restore_state(const struct vcpu *v)
> >          val |= GICC_SRE_EL2_ENEL1;
> >      WRITE_SYSREG32(val, ICC_SRE_EL2);
> >  
> > +    /*
> > +     * VFIQEn is RES1 if ICC_SRE_EL1.SRE is 1. This causes a Group0
> > +     * interrupt (as generated in GICv2 mode) to be delivered as a FIQ
> > +     * to the guest, with potentially consequence. So we must make sure
> > +     * that ICC_SRE_EL1 has been actually programmed with the value we
> > +     * want before starting to mess with the rest of the GIC, and
> > +     * VMCR_EL1 in particular.
> > +     */
> >      WRITE_SYSREG32(v->arch.gic.v3.sre_el1, ICC_SRE_EL1);
> > +    isb();
> >      WRITE_SYSREG32(v->arch.gic.v3.vmcr, ICH_VMCR_EL2);
> >      restore_aprn_regs(&v->arch.gic);
> >      gicv3_restore_lrs(v);
> > -- 
> > 2.11.0
> > 
> 

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