Re: [Xen-devel] [PATCH v2] xen: arm: correctly configure NSACR.

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 07/15/2013 12:22 PM, Stefano Stabellini wrote:
> On Mon, 15 Jul 2013, Ian Campbell wrote:
>> From: Ian Campbell <ian.campbell@xxxxxxxxxx>
>>
>> Previously we were setting it up twice, the second time neglecting to set the
>> NS_SMP bit.
>>
>> NSACR.NS_SMP is a processor specific bit which on Cortex-A7 and -A15 
>> regulates
>> access to the (also processor specific) ACTLR.SMP bit. Not setting 
>> NSACR.NS_SMP
>> meant that Xen's attempts to set ACTLR.SMP was silently ignored. Setting this
>> bit is required in order to cause the processor to take part in cache and TLB
>> coherency protocols. Failure to set this bit leads to random memory 
>> corruption
>> in guests (although nothing like as catestrophic as you might expect!).
                                          ^
                                       catastrophic
>>
>> An alternative fix would have been to set ACTLR.SMP when in Secure World,
>> however Linux expects to set ACTLR.SMP itself in NS mode, so it's a good bet
>> that bootloaders will set NSACR.NS_SMP instead.
>>
>> While here switch to a read-modify-write of NSACR to preserve any existing 
>> bits
>> -- seems safer.
>>
>> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>> ---
>> v2: Modify rather than overwrite NSACR.
> 
> Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

Except the minor typo:
Acked-by: Julien Grall <julien.grall@xxxxxxxxxx>

> 
>>  xen/arch/arm/arm32/mode_switch.S | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm32/mode_switch.S 
>> b/xen/arch/arm/arm32/mode_switch.S
>> index c92a1cf..3500eb0 100644
>> --- a/xen/arch/arm/arm32/mode_switch.S
>> +++ b/xen/arch/arm/arm32/mode_switch.S
>> @@ -104,7 +104,11 @@ enter_hyp_mode:
>>           * memory-mapped control registers live, we can't find out the
>>           * right frequency. */
>>          mcr   CP32(r0, CNTFRQ)
>> -        ldr   r0, =0x40c00           /* SMP, c11, c10 in non-secure mode */
>> +
>> +        mrc   CP32(r0,NSACR)
>> +        ldr   r4, =0x3fff            /* Allow access to all co-processors 
>> in NS mode */
>> +        orr   r0, r0, r4
>> +        orr   r0, r0, #(1<<18)       /* CA7/CA15: Allow access to ACTLR.SMP 
>> in NS mode */
>>          mcr   CP32(r0, NSACR)
>>  
>>          add   r0, r1, #GIC_DR_OFFSET
>> @@ -143,9 +147,6 @@ skip_spis:
>>          mov   r0, #0
>>          mcr   CP32(r0, FCSEIDR)
>>          mcr   CP32(r0, CONTEXTIDR)
>> -        /* Allow non-secure access to coprocessors, FIQs, VFP and NEON */
>> -        ldr   r1, =0x3fff            /* 14 CP bits set, all others clear */
>> -        mcr   CP32(r1, NSACR)
>>  
>>          mrs   r0, cpsr               /* Copy the CPSR */
>>          add   r0, r0, #0x4           /* 0x16 (Monitor) -> 0x1a (Hyp) */
>> -- 
>> 1.8.3.2
>>
> 


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