-----Original Message----- From: FengHua [mailto:fenghua@phytium.com.cn] Sent: Thursday, January 16, 2014 6:47 AM To: Sharma Bhupesh-B45370 Cc: u-boot@lists.denx.de; trini@ti.com; Basu Arnab-B45036 Subject: Re: RE: [U-Boot] [PATCH] arm64 patch: gicv3 support
hi bhupesh,
-----Original Messages----- From: "bhupesh.sharma@freescale.com" bhupesh.sharma@freescale.com Sent Time: 2014-01-15 22:19:02 (Wednesday) To: "'fenghua@phytium.com.cn'" fenghua@phytium.com.cn, "u-boot@lists.denx.de" u-boot@lists.denx.de Cc: "trini@ti.com" trini@ti.com, "arnab.basu@freescale.com" arnab.basu@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@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of fenghua@phytium.com.cn Sent: Wednesday, January 15, 2014 1:41 PM To: u-boot@lists.denx.de Cc: trini@ti.com Subject: [U-Boot] [PATCH] arm64 patch: gicv3 support
From: David Feng fenghua@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.
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?
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.
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.
Signed-off-by: David Feng fenghua@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