Buggy branch in __init_el2_gicv3?

[Marc Zyngier]

On Mon, 14 Feb 2022 15:14:41 +0000,
Joakim Tjernlund <Joakim.Tjernlund at infinera.com> wrote:
> 
> On Mon, 2022-02-14 at 14:56 +0000, Mark Rutland wrote:
> > On Mon, Feb 14, 2022 at 12:34:12PM +0000, Joakim Tjernlund wrote:
> > > In init_el2_state calls __init_el2_gicv3 which looks:
> > > 
> > > /* GICv3 system register access */
> > > .macro __init_el2_gicv3
> > > 	mrs	x0, id_aa64pfr0_el1
> > > 	ubfx	x0, x0, #ID_AA64PFR0_GIC_SHIFT, #4
> > > 	cbz	x0, .Lskip_gicv3_\@
> > > 
> > > 	mrs_s	x0, SYS_ICC_SRE_EL2
> > > 	orr	x0, x0, #ICC_SRE_EL2_SRE	// Set ICC_SRE_EL2.SRE==1
> > > 	orr	x0, x0, #ICC_SRE_EL2_ENABLE	// Set ICC_SRE_EL2.Enable==1
> > > 	msr_s	SYS_ICC_SRE_EL2, x0
> > > 	isb					// Make sure SRE is now set
> > > 	mrs_s	x0, SYS_ICC_SRE_EL2		// Read SRE back,
> > > 	tbz     x0, #0, 1f
> > > //	tbz	x0, #0, .Lskip_gicv3_\@		// and check that it sticks
> > > 	msr_s	SYS_ICH_HCR_EL2, xzr		// Reset ICC_HCR_EL2 to defaults
> > > .Lskip_gicv3_\@:
> > > .endm
> > > 
> > > Note the tbz     x0, #0, 1f, this instruction causes my A53 CPU to jump far out of the macro and make the
> > > CPU go into EL0, failing with an exception a bit later. I believe the 1f is a bug and if I replace it with
> > >  tbz	x0, #0, .Lskip_gicv3_\@
> > > the CPU continue too boot.
> > 
> > You're right, that should be targetting `.Lskip_gicv3_\@`. It looks like that
> > was left behind in commit:
> > 
> >   114945d84a30a5fe ("arm64: Fix labels in el2_setup macros")
> > 
> > Marc?
> > 
> > > Why this is trigged is still a mystery to me, any pointers welcome :)
> > 
> > I believe this happens if EL3 has ICC_SRE_EL3.SRE == 0b0; in that
> > case ICC_SRE_EL2.SRE is RAZ/WI. i.e. your EL3 firmware isn't using
> > GICv3 and hasn't enabled it for lower ELs.
> 
> Correct, I am now trying to enable GIC but I thing I note is that we
> have a GIC500v4 and u-boot(my EL3 FW) only has GICV3 as far as I can
> see. Will that be a problem or is GICv3 and v4 compatible as in this
> respect ?

Hmmm. There is no such thing as a GIC500v4. GIC500 is a GICv3
implementation, and the most recent revision seems to be r1p1.

Note that it doesn't matter much, as GICv3 and GICv4 have programming
models that are largely identical, and the differences are only
meaningful to a hypervisor. Linux has support for both anyway.

Are you actually using u-boot as your EL3 firmware? That's... an
interesting choice. Last time I checked, the GICv3 support was pretty
bad (as in totally broken). Something like TF-A would seem like a
better choice when it comes to architectural support).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.