Re: [Qemu-devel] [PATCH 1/3] hw/arm_boot, exynos4210, highbank: Fix secondary boot GIC init

[Igor Mitsyanko]

On 11/29/2012 9:02 PM, Peter Maydell wrote:
> Fix the code in the secondary CPU boot stubs so that it correctly
> initialises the GIC rather than relying on bugs or implementation
> dependent aspects of the QEMU GIC implementation:
>   * set the GIC_PMR.Priority field to all-ones, so that all
>     interrupts are passed through. The default of all-zeroes
>     means all interrupts are masked, and QEMU only booted because
>     of a bug in the priority masking in our GIC implementation.
>   * add a barrier after GIC setup and before WFI to ensure that
>     GIC config is complete before we go into a possible low power
>     state. This isn't needed with the software GIC model but could
>     be required when using KVM and executing this code on the
>     real hardware CPU.
>
> Note that of the three secondary stub implementations, only
> the common generic one needs to support both v6 and v7 DSB
> encodings; highbank and exynos4210 will always be v7 CPUs.
>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>   hw/arm_boot.c   |   17 ++++++++++++++---
>   hw/exynos4210.c |   10 +++++++---
>   hw/highbank.c   |    7 +++++--
>   3 files changed, 26 insertions(+), 8 deletions(-)


Reviewed-by: Igor Mitsyanko <address@hidden>


> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
> index 92e2cab..ec3b8d5 100644
> --- a/hw/arm_boot.c
> +++ b/hw/arm_boot.c
> @@ -44,11 +44,17 @@ static uint32_t bootloader[] = {
>    * for an interprocessor interrupt and polling a configurable
>    * location for the kernel secondary CPU entry point.
>    */
> +#define DSB_INSN 0xf57ff04f
> +#define CP15_DSB_INSN 0xee070f9a /* mcr cp15, 0, r0, c7, c10, 4 */
> +
>   static uint32_t smpboot[] = {
> -  0xe59f201c, /* ldr r2, gic_cpu_if */
> -  0xe59f001c, /* ldr r0, startaddr */
> +  0xe59f2028, /* ldr r2, gic_cpu_if */
> +  0xe59f0028, /* ldr r0, startaddr */
>     0xe3a01001, /* mov r1, #1 */
> -  0xe5821000, /* str r1, [r2] */
> +  0xe5821000, /* str r1, [r2] - set GICC_CTLR.Enable */
> +  0xe3a010ff, /* mov r1, #0xff */
> +  0xe5821004, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
> +  DSB_INSN,   /* dsb */
>     0xe320f003, /* wfi */
>     0xe5901000, /* ldr     r1, [r0] */
>     0xe1110001, /* tst     r1, r1 */
> @@ -65,6 +71,11 @@ static void default_write_secondary(ARMCPU *cpu,
>       smpboot[ARRAY_SIZE(smpboot) - 1] = info->smp_bootreg_addr;
>       smpboot[ARRAY_SIZE(smpboot) - 2] = info->gic_cpu_if_addr;
>       for (n = 0; n < ARRAY_SIZE(smpboot); n++) {
> +        /* Replace DSB with the pre-v7 DSB if necessary. */
> +        if (!arm_feature(&cpu->env, ARM_FEATURE_V7) &&
> +            smpboot[n] == DSB_INSN) {
> +            smpboot[n] = CP15_DSB_INSN;
> +        }
>           smpboot[n] = tswap32(smpboot[n]);
>       }
>       rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot),
> diff --git a/hw/exynos4210.c b/hw/exynos4210.c
> index 00d4db8..22148cd 100644
> --- a/hw/exynos4210.c
> +++ b/hw/exynos4210.c
> @@ -80,12 +80,16 @@ void exynos4210_write_secondary(ARMCPU *cpu,
>   {
>       int n;
>       uint32_t smpboot[] = {
> -        0xe59f3024, /* ldr r3, External gic_cpu_if */
> -        0xe59f2024, /* ldr r2, Internal gic_cpu_if */
> -        0xe59f0024, /* ldr r0, startaddr */
> +        0xe59f3034, /* ldr r3, External gic_cpu_if */
> +        0xe59f2034, /* ldr r2, Internal gic_cpu_if */
> +        0xe59f0034, /* ldr r0, startaddr */
>           0xe3a01001, /* mov r1, #1 */
>           0xe5821000, /* str r1, [r2] */
>           0xe5831000, /* str r1, [r3] */
> +        0xe3a010ff, /* mov r1, #0xff */
> +        0xe5821004, /* str r1, [r2, #4] */
> +        0xe5831004, /* str r1, [r3, #4] */
> +        0xf57ff04f, /* dsb */
>           0xe320f003, /* wfi */
>           0xe5901000, /* ldr     r1, [r0] */
>           0xe1110001, /* tst     r1, r1 */
> diff --git a/hw/highbank.c b/hw/highbank.c
> index afbb005..447e57d 100644
> --- a/hw/highbank.c
> +++ b/hw/highbank.c
> @@ -44,9 +44,12 @@ static void hb_write_secondary(ARMCPU *cpu, const struct 
> arm_boot_info *info)
>           0xe210000f, /* ands r0, r0, #0x0f */
>           0xe3a03040, /* mov r3, #0x40 - jump address is 0x40 + 0x10 * core id 
> */
>           0xe0830200, /* add r0, r3, r0, lsl #4 */
> -        0xe59f2018, /* ldr r2, privbase */
> +        0xe59f2024, /* ldr r2, privbase */
>           0xe3a01001, /* mov r1, #1 */
> -        0xe5821100, /* str r1, [r2, #256] */
> +        0xe5821100, /* str r1, [r2, #256] - set GICC_CTLR.Enable */
> +        0xe3a010ff, /* mov r1, #0xff */
> +        0xe5821104, /* str r1, [r2, #260] - set GICC_PMR.Priority to 0xff */
> +        0xf57ff04f, /* dsb */
>           0xe320f003, /* wfi */
>           0xe5901000, /* ldr     r1, [r0] */
>           0xe1110001, /* tst     r1, r1 */