Re: [Xen-devel] [PATCH 1/4] arm: disable distributor delivery on boot CPU only

[Tim Deegan <tim@xxxxxxx>]

At 11:34 +0000 on 06 Aug (1344252897), Ian Campbell wrote:
> The secondary processors do not call enter_hyp_mode until the boot CPU
> has brought most of the system up, including enabling delivery via the
> distributor. This means that bringing up secondary CPUs unexpectedly
> disables the GICD again, meaning we get no further interrupts on any
> CPU.
> 
> It's not clear that the distributor actually needs to be disabled to
> modify the group registers but it seems reasonable that the bringup
> code should make sure the GICD is disabled even if not doing the
> transition to hyp mode, so move this to the main flow of head.S and
> only do it on the boot processor.
> 
> For completeness also disable the GICC (CPU interface) on all CPUs
> too.

I think that having interrupts disabled is something we can rely on the
bootloader/firmware handling for us, so this should all stay in
mode_switch.S for now, and avoid leaking GIC_* magic constants into
head.S.  (Unless you fancy writing a DT parser in assembler :))

Tim.

> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
>  xen/arch/arm/head.S        |   14 ++++++++++++++
>  xen/arch/arm/mode_switch.S |   14 +++++++++-----
>  2 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/head.S b/xen/arch/arm/head.S
> index cdbe011..a69bf72 100644
> --- a/xen/arch/arm/head.S
> +++ b/xen/arch/arm/head.S
> @@ -67,6 +67,12 @@ start:
>       add   r8, r10                /* r8 := paddr(DTB) */
>  #endif
>  
> +     /* Disable interrupt delivery at the GIC's CPU interface */
> +     mov   r0, #GIC_BASE_ADDRESS
> +     add   r0, r0, #GIC_CR_OFFSET
> +     mov   r1, #0
> +     str   r1, [r0]
> +
>       /* Are we the boot CPU? */
>       mov   r12, #0                /* r12 := CPU ID */
>       mrc   CP32(r0, MPIDR)
> @@ -85,8 +91,16 @@ start:
>       ldr   r1, [r0]               /* Which CPU is being booted? */
>       teq   r1, r12                /* Is it us? */
>       bne   1b
> +     b     secondary_cpu
>  
>  boot_cpu:
> +     /* Setup which only needs to be done once, on the boot CPU */
> +     mov   r0, #GIC_BASE_ADDRESS
> +     add   r0, r0, #GIC_DR_OFFSET
> +     mov   r1, #0
> +     str   r1, [r0]               /* Disable delivery in the distributor */
> +
> +secondary_cpu:
>  #ifdef EARLY_UART_ADDRESS
>       ldr   r11, =EARLY_UART_ADDRESS  /* r11 := UART base address */
>       teq   r12, #0                   /* CPU 0 sets up the UART too */
> diff --git a/xen/arch/arm/mode_switch.S b/xen/arch/arm/mode_switch.S
> index f5549d7..9211d26 100644
> --- a/xen/arch/arm/mode_switch.S
> +++ b/xen/arch/arm/mode_switch.S
> @@ -49,13 +49,17 @@ enter_hyp_mode:
>       /* Continuing ugliness: Set up the GIC so NS state owns interrupts */
>       mov   r0, #GIC_BASE_ADDRESS
>       add   r0, r0, #GIC_DR_OFFSET
> -     mov   r1, #0
> -     str   r1, [r0]               /* Disable delivery in the distributor */
>       add   r0, r0, #0x80          /* GICD_IGROUP0 */
> -     mov   r2, #0xffffffff        /* All interrupts to group 1 */
> +     mov   r2, #0xffffffff        /* Interrupts 0-31 (SGI&PPI) to group 1 */
> +     /* The remaining interrupts are Shared Periphal Interrupts and so
> +      * need reconfiguring only once, on the boot CPU */
>       str   r2, [r0]
> -     str   r2, [r0, #4]
> -     str   r2, [r0, #8]
> +     teq   r12, #0
> +     bne   skip_spi
> +     str   r2, [r0, #4]           /* Interrupts 32-63 (SPI) to group 1 */
> +     str   r2, [r0, #8]           /* Interrupts 64-95 (SPI) to group 1 */
> +skip_spi:
> +     
>       /* Must drop priority mask below 0x80 before entering NS state */
>       mov   r0, #GIC_BASE_ADDRESS
>       add   r0, r0, #GIC_CR_OFFSET
> -- 
> 1.7.9.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel