[U-Boot] [PATCH] arm64 patch: gicv3 support

Thu Jan 16 08:21:27 CET 2014



> -----Original Messages-----
> From: "bhupesh.sharma at freescale.com" <bhupesh.sharma at freescale.com>
> Sent Time: 2014-01-16 11:45:18 (Thursday)
> To: 'FengHua' <fenghua at phytium.com.cn>
> Cc: "u-boot at lists.denx.de" <u-boot at lists.denx.de>, "trini at ti.com" <trini at ti.com>, "arnab.basu at freescale.com" <arnab.basu at freescale.com>
> Subject: RE: RE: [U-Boot] [PATCH] arm64 patch: gicv3 support
> 
> > -----Original Message-----
> > From: FengHua [mailto:fenghua at phytium.com.cn]
> > Sent: Thursday, January 16, 2014 6:47 AM
> > To: Sharma Bhupesh-B45370
> > Cc: u-boot at lists.denx.de; trini at ti.com; Basu Arnab-B45036
> > Subject: Re: RE: [U-Boot] [PATCH] arm64 patch: gicv3 support
> > 
> > 
> > hi bhupesh,
> > 
> > 
> > > -----Original Messages-----
> > > From: "bhupesh.sharma at freescale.com" <bhupesh.sharma at freescale.com>
> > > Sent Time: 2014-01-15 22:19:02 (Wednesday)
> > > To: "'fenghua at phytium.com.cn'" <fenghua at phytium.com.cn>,
> > > "u-boot at lists.denx.de" <u-boot at lists.denx.de>
> > > Cc: "trini at ti.com" <trini at ti.com>, "arnab.basu at freescale.com"
> > > <arnab.basu at freescale.com>
> > > Subject: RE: [U-Boot] [PATCH] arm64 patch: gicv3 support
> > >
> > > Hi David,
> > >
> > > Thanks for the patch.
> > > Please see some comments inline:
> > >
> > > > -----Original Message-----
> > > > From: u-boot-bounces at lists.denx.de
> > > > [mailto:u-boot-bounces at lists.denx.de]
> > > > On Behalf Of fenghua at phytium.com.cn
> > > > Sent: Wednesday, January 15, 2014 1:41 PM
> > > > To: u-boot at lists.denx.de
> > > > Cc: trini at ti.com
> > > > Subject: [U-Boot] [PATCH] arm64 patch: gicv3 support
> > > >
> > > > From: David Feng <fenghua at phytium.com.cn>
> > > >
> > > > This patch add gicv3 support to uboot armv8 platform.
> > > > Modifications cover 4 source files, as follows:
> > > >   gic.S: gicv3 initialization and sgi interrupt raising.
> > > >   goc.h: gicv3 register definitions.
> > >
> > >  	^^^
> > > Typo - gic.h
> > >
> > > >   vexpress_aemv8a.h: add CONFIG_GICV2/CONFIG_GICV3 switch.
> > > >   start.S: set SCR_EL3.NS bit to 1, gicv3 register of ICC_SRE_EL2
> > > >            could be accessed only when SCR_EL3.NS=1.
> > > >            set SCR_EL3.IRQ|FIQ|EA bits, reroute all interrupts to
> > > >            el3 at all cores, slaves could be waken up by interrupt
> > > >            only when the interrupt is routed to it when running
> > > >            at el3.
> > >
> > > Hmmm. This seems a bit suspicious - if we reroute even IRQs and Aborts
> > > to the cores which work in EL3, they will not be visible to Linux or
> > > Hypervisor which work in EL2 or EL1.
> > >
> > These bits will be cleared when jumping to el2.
> 
> I seem to be missing this clear operation in your patch. Can you please point me to the same.
> 
armv8_switch_to_el2 routine in transition.S.

> > > Have you tried to launch linux on the ARMv8 foundation model v2 with
> > these changes?
> > >
> > Yes.
> 
> But I thought we still don't have GICv3 driver in Linux. So did you boot linux with GICv2 or GICv3?
> 
GICv3. MAZ's gicv3 kernel git tree contains GICv3 driver.

> > 
> > > > Note: please use the latest gcc 4.8 compiler from linaro
> > > >       which support gicv3 system register assembling.
> > > >
> > >
> > > Two generic comments :
> > >
> > > - I see in the Foundation model README that the optional GICv3 is
> > > supported with memory-mapped CPU interface and distributor, but I see
> > > your patch accessing them via the sytem register interface (via msr and
> > mrs). Is this a BUG in the README?
> > >
> > The document did not describe it clearly, but actually it support.
> 
> So this seems to be a documentation BUG, did you provide a feedback to the ARM support folks regarding the same
> - they should probably fix the documentation.
I have not. Actually, I did not notice this before receiving your mail.

>  
> > > Did you check this patch on the latest ARMv8 foundation model - v2?
> > >
> > Yes.
> > 
> > > - Also how about sharing the GICv2 coding between ARMv7 and ARMv8 -
> > > some of the code may seems like a duplication from the ARMv7 GICv2
> > content.
> > >
> > Many codes could be shared between armv7 and armv8 such as cache
> > maintenance and gic.
> > These need be rearranged seriously. We'd better wait a period of time
> > before the armv8 codes are more widely acquainted and get more feedback
> > about this.
> 
> I agree about the ARMv7/v8 code sharing, but with GICv3 there is an additional problem - it can be configured as 
> GICv2 as well and for the same it would make sense to move common code b/w the CONFIG_GICV2/CONFIG_GICV3 code legs
> to a common leg.
Yes, this need more considerations.

> 
> > 
> > > > Signed-off-by: David Feng <fenghua at phytium.com.cn>
> > > > ---
> > > >  arch/arm/cpu/armv8/gic.S          |   84
> > > > ++++++++++++++++++++++++++++++++++++-
> > > >  arch/arm/cpu/armv8/start.S        |    5 ++-
> > > >  arch/arm/include/asm/gic.h        |   56 +++++++++++++++++++++++++
> > > >  include/configs/vexpress_aemv8a.h |    7 ++++
> > > >  4 files changed, 150 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/arm/cpu/armv8/gic.S b/arch/arm/cpu/armv8/gic.S
> > > > index 599aa8f..a08939a 100644
> > > > --- a/arch/arm/cpu/armv8/gic.S
> > > > +++ b/arch/arm/cpu/armv8/gic.S
> > > > @@ -23,6 +23,74 @@
> > > >   *
> > > >
> > > > ********************************************************************
> > > > *****
> > > > /
> > > >  WEAK(gic_init)
> > > > +#if defined(CONFIG_GICV3)
> > > > +	branch_if_slave	x0, 3f
> > > > +
> > > > +	/* Initialize Distributor */
> > > > +	ldr	x1, =GICD_BASE
> > > > +	mov	w0, #0x37		/* EnableGrp0 | EnableGrp1NS */
> > > > +					/* EnableGrp1S | ARE_S | ARE_NS */
> > > > +	str	w0, [x1, GICD_CTLR]	/* Secure GICD_CTLR */
> > > > +	ldr	w0, [x1, GICD_TYPER]
> > > > +	and	w2, w0, #0x1f		/* ITLinesNumber */
> > > > +	cbz	w2, 1f			/* No SPIs */
> > > > +	add	x3, x1, (GICD_IGROUPRn + 4)
> > > > +	add	x4, x1, (GICD_IGROUPMODRn + 4)
> > > > +	mov	w5, #~0
> > > > +0:	str	w5, [x3], #0x4
> > > > +	str	wzr, [x4], #0x4		/* Config SPIs as Group1NS */
> > > > +	sub	w2, w2, #0x1
> > > > +	cbnz	w2, 0b
> > > > +
> > > > +	/* Initialize All ReDistributors */
> > > > +1:	ldr	x1, =GICR_BASE
> > > > +2:	mov	w0, #~0x2
> > > > +	ldr	w2, [x1, GICR_WAKER]
> > > > +	and	w2, w2, w0		/* Clear ProcessorSleep */
> > > > +	str	w2, [x1, GICR_WAKER]
> > > > +	dsb	st
> > > > +	isb
> > > > +0:	ldr	w0, [x1, GICR_WAKER]
> > > > +	tbnz	w0, #2, 0b		/* Wait Children be Alive */
> > > > +
> > > > +	add	x2, x1, #(1 << 16)	/* SGI_Base */
> > > > +	mov	w5, #~0
> > > > +	str	w5, [x2, GICR_IGROUPRn]
> > > > +	str	wzr, [x2, GICR_IGROUPMODRn]	/* SGIs|PPIs Group1NS */
> > > > +	mov	w0, #0x1		/* Enable SGI 0 */
> > > > +	str	w0, [x2, GICR_ISENABLERn]
> > > > +
> > > > +	ldr	w0, [x1, GICR_TYPER]
> > > > +	add	x1, x1, #(2 << 16)
> > > > +	tbz	w0, #4, 2b		/* Next ReDistributor if Exist */
> > > > +
> > > > +	/* Initialize Cpu Interface */
> > > > +3:	mrs	x0, ICC_SRE_EL3
> > > > +	orr	x0, x0, #0xf		/* SRE & Disable IRQ/FIQ Bypass & */
> > > > +					/* Allow EL2 access to ICC_SRE_EL2 */
> > > > +	msr	ICC_SRE_EL3, x0
> > > > +	isb
> > > > +
> > > > +	mrs	x0, ICC_SRE_EL2
> > > > +	orr	x0, x0, #0xf		/* SRE & Disable IRQ/FIQ Bypass & */
> > > > +					/* Allow EL1 access to ICC_SRE_EL1 */
> > > > +	msr	ICC_SRE_EL2, x0
> > > > +	isb
> > > > +
> > > > +	mov	x0, #0x3		/* EnableGrp1NS | EnableGrp1S */
> > > > +	msr	ICC_IGRPEN1_EL3, x0
> > > > +	isb
> > > > +
> > > > +	msr	ICC_CTLR_EL3, xzr
> > > > +	isb
> > > > +
> > > > +	msr	ICC_CTLR_EL1, xzr	/* NonSecure ICC_CTLR_EL1 */
> > > > +	isb
> > > > +
> > > > +	mov	x0, #0x1 << 7		/* Non-Secure access to ICC_PMR_EL1
> > > > */
> > > > +	msr	ICC_PMR_EL1, x0
> > > > +	isb
> > > > +#elif defined(CONFIG_GICV2)
> > > >  	branch_if_slave	x0, 2f
> > > >
> > > >  	/* Initialize Distributor and SPIs */ @@ -54,7 +122,7 @@
> > > > WEAK(gic_init)
> > > >
> > > >  	mov	w0, #0x1 << 7		/* Non-Secure access to GICC_PMR */
> > > >  	str	w0, [x1, GICC_PMR]
> > > > -
> > > > +#endif
> > > >  	ret
> > > >  ENDPROC(gic_init)
> > > >
> > > > @@ -65,11 +133,18 @@ ENDPROC(gic_init)
> > > >   *
> > > >
> > > > ********************************************************************
> > > > *****
> > > > /
> > > >  WEAK(gic_send_sgi)
> > > > +#if defined(CONFIG_GICV3)
> > > > +	mov	x1, #(1 << 40)
> > > > +	orr	x1, x1, x0, lsl #24
> > > > +	msr	ICC_ASGI1R_EL1, x1
> > > > +	isb
> > > > +#elif defined(CONFIG_GICV2)
> > > >  	ldr	x1, =GICD_BASE
> > > >  	mov	w2, #0x8000
> > > >  	movk	w2, #0x100, lsl #16
> > > >  	orr	w2, w2, w0
> > > >  	str	w2, [x1, GICD_SGIR]
> > > > +#endif
> > > >  	ret
> > > >  ENDPROC(gic_send_sgi)
> > > >
> > > > @@ -82,11 +157,18 @@ ENDPROC(gic_send_sgi)
> > > >   *
> > > >
> > > > ********************************************************************
> > > > *****
> > > > /
> > > >  WEAK(wait_for_wakeup)
> > > > +#if defined(CONFIG_GICV3)
> > > > +0:	wfi
> > > > +	mrs	x0, ICC_IAR1_EL1
> > > > +	msr	ICC_EOIR1_EL1, x0
> > > > +	cbnz	x0, 0b
> > > > +#elif defined(CONFIG_GICV2)
> > > >  	ldr	x1, =GICC_BASE
> > > >  0:	wfi
> > > >  	ldr	w0, [x1, GICC_AIAR]
> > > >  	str	w0, [x1, GICC_AEOIR]
> > > >  	cbnz	w0, 0b
> > > > +#endif
> > > >  	ret
> > > >  ENDPROC(wait_for_wakeup)
> > > >
> > > > diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
> > > > index bcc2603..9911817 100644
> > > > --- a/arch/arm/cpu/armv8/start.S
> > > > +++ b/arch/arm/cpu/armv8/start.S
> > > > @@ -50,7 +50,10 @@ reset:
> > > >  	 */
> > > >  	adr	x0, vectors
> > > >  	switch_el x1, 3f, 2f, 1f
> > > > -3:	msr	vbar_el3, x0
> > > > +3:	mrs	x0, scr_el3
> > > > +	orr	x0, x0, #0xf			/* SCR_EL3.NS|IRQ|FIQ|EA */
> > > > +	msr	scr_el3, x0
> > > > +	msr	vbar_el3, x0
> > > >  	msr	cptr_el3, xzr			/* Enable FP/SIMD */
> > > >  	ldr	x0, =COUNTER_FREQUENCY
> > > >  	msr	cntfrq_el0, x0			/* Initialize CNTFRQ */
> > > > diff --git a/arch/arm/include/asm/gic.h b/arch/arm/include/asm/gic.h
> > > > index ac2b2bf..bd3a80c 100644
> > > > --- a/arch/arm/include/asm/gic.h
> > > > +++ b/arch/arm/include/asm/gic.h
> > > > @@ -51,4 +51,60 @@
> > > >  #define GICC_IIDR		0x00fc
> > > >  #define GICC_DIR		0x1000
> > > >
> > > > +/* ReDistributor Registers for Control and Physical LPIs */
> > > > +#define GICR_CTLR		0x0000
> > > > +#define GICR_IIDR		0x0004
> > > > +#define GICR_TYPER		0x0008
> > > > +#define GICR_STATUSR		0x0010
> > > > +#define GICR_WAKER		0x0014
> > > > +#define GICR_SETLPIR		0x0040
> > > > +#define GICR_CLRLPIR		0x0048
> > > > +#define GICR_SEIR		0x0068
> > > > +#define GICR_PROPBASER		0x0070
> > > > +#define GICR_PENDBASER		0x0078
> > > > +#define GICR_INVLPIR		0x00a0
> > > > +#define GICR_INVALLR		0x00b0
> > > > +#define GICR_SYNCR		0x00c0
> > > > +#define GICR_MOVLPIR		0x0100
> > > > +#define GICR_MOVALLR		0x0110
> > > > +
> > > > +/* ReDistributor Registers for SGIs and PPIs */
> > > > +#define GICR_IGROUPRn		0x0080
> > > > +#define GICR_ISENABLERn		0x0100
> > > > +#define GICR_ICENABLERn		0x0180
> > > > +#define GICR_ISPENDRn		0x0200
> > > > +#define GICR_ICPENDRn		0x0280
> > > > +#define GICR_ISACTIVERn		0x0300
> > > > +#define GICR_ICACTIVERn		0x0380
> > > > +#define GICR_IPRIORITYRn	0x0400
> > > > +#define GICR_ICFGR0		0x0c00
> > > > +#define GICR_ICFGR1		0x0c04
> > > > +#define GICR_IGROUPMODRn	0x0d00
> > > > +#define GICR_NSACRn		0x0e00
> > > > +
> > > > +/* Cpu Interface System Registers */
> > > > +#define ICC_IAR0_EL1		S3_0_C12_C8_0
> > > > +#define ICC_IAR1_EL1		S3_0_C12_C12_0
> > > > +#define ICC_EOIR0_EL1		S3_0_C12_C8_1
> > > > +#define ICC_EOIR1_EL1		S3_0_C12_C12_1
> > > > +#define ICC_HPPIR0_EL1		S3_0_C12_C8_2
> > > > +#define ICC_HPPIR1_EL1		S3_0_C12_C12_2
> > > > +#define ICC_BPR0_EL1		S3_0_C12_C8_3
> > > > +#define ICC_BPR1_EL1		S3_0_C12_C12_3
> > > > +#define ICC_DIR_EL1		S3_0_C12_C11_1
> > > > +#define ICC_PMR_EL1		S3_0_C4_C6_0
> > > > +#define ICC_RPR_EL1		S3_0_C12_C11_3
> > > > +#define ICC_CTLR_EL1		S3_0_C12_C12_4
> > > > +#define ICC_CTLR_EL3		S3_6_C12_C12_4
> > > > +#define ICC_SRE_EL1		S3_0_C12_C12_5
> > > > +#define ICC_SRE_EL2		S3_4_C12_C9_5
> > > > +#define ICC_SRE_EL3		S3_6_C12_C12_5
> > > > +#define ICC_IGRPEN0_EL1		S3_0_C12_C12_6
> > > > +#define ICC_IGRPEN1_EL1		S3_0_C12_C12_7
> > > > +#define ICC_IGRPEN1_EL3		S3_6_C12_C12_7
> > > > +#define ICC_SEIEN_EL1		S3_0_C12_C13_0
> > > > +#define ICC_SGI0R_EL1		S3_0_C12_C11_7
> > > > +#define ICC_SGI1R_EL1		S3_0_C12_C11_5
> > > > +#define ICC_ASGI1R_EL1		S3_0_C12_C11_6
> > > > +
> > > >  #endif /* __GIC_H__ */
> > > > diff --git a/include/configs/vexpress_aemv8a.h
> > > > b/include/configs/vexpress_aemv8a.h
> > > > index ce5f384..ac67887 100644
> > > > --- a/include/configs/vexpress_aemv8a.h
> > > > +++ b/include/configs/vexpress_aemv8a.h
> > > > @@ -12,6 +12,8 @@
> > > >
> > > >  #define CONFIG_REMAKE_ELF
> > > >
> > > > +#define CONFIG_GICV2
> > > > +
> > > >  /*#define CONFIG_ARMV8_SWITCH_TO_EL1*/
> > > >
> > > >  /*#define CONFIG_SYS_GENERIC_BOARD*/ @@ -93,8 +95,13 @@
> > > >  #define COUNTER_FREQUENCY		(0x1800000)	/* 24MHz */
> > > >
> > > >  /* Generic Interrupt Controller Definitions */
> > > > +#ifdef CONFIG_GICV3
> > > > +#define GICD_BASE			(0x2f000000)
> > > > +#define GICR_BASE			(0x2f100000)
> > > > +#else
> > > >  #define GICD_BASE			(0x2C001000)
> > > >  #define GICC_BASE			(0x2C002000)
> > > > +#endif
> > > >
> > > >  #define CONFIG_SYS_MEMTEST_START	V2M_BASE
> > > >  #define CONFIG_SYS_MEMTEST_END		(V2M_BASE + 0x80000000)
> > > > --
> > > > 1.7.9.5
> > > >
> > >
> > > Regards,
> > > Bhupesh
> > 
> > 
> > 
> > 
> > 
> > 
> > 
>