Hi David,

Thanks for the patch.
Please see some comments inline:

> -----Original Message-----
> From: u-boot-boun...@lists.denx.de [mailto:u-boot-boun...@lists.denx.de]
> On Behalf Of feng...@phytium.com.cn
> Sent: Wednesday, January 15, 2014 1:41 PM
> To: u-boot@lists.denx.de
> Cc: tr...@ti.com
> Subject: [U-Boot] [PATCH] arm64 patch: gicv3 support
> 
> From: David Feng <feng...@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.

Have you tried to launch linux on the ARMv8 foundation model v2 with these 
changes?

> 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?

Did you check this patch on the latest ARMv8 foundation model - v2?

- 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.
 
> Signed-off-by: David Feng <feng...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to