Re: [Xen-devel] [PATCH v2 29/35] xen/arm32: head: Move assembly switch to the runtime PT in secondary CPUs path

[Julien Grall <julien.grall@arm.com>]

Hi Stefano,

On 7/30/19 9:25 PM, Stefano Stabellini wrote:
> On Mon, 22 Jul 2019, Julien Grall wrote:
>> The assembly switch to the runtime PT is only necessary for the
>> secondary CPUs. So move the code in the secondary CPUs path.
>>
>> While this is definitely not compliant with the Arm Arm as we are
>> switching between two differents set of page-tables without turning off
>> the MMU. Turning off the MMU is impossible here as the ID map may clash
>> with other mappings in the runtime page-tables. This will require more
>> rework to avoid the problem. So for now add a TODO in the code.
>>
>> Finally, the code is currently assume that r5 will be properly set to 0
>> before hand. This is done by create_page_tables() which is called quite
>> early in the boot process. There are a risk this may be oversight in the
>> future and therefore breaking secondary CPUs boot. Instead, set r5 to 0
>> just before using it.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> ---
>>      Changes in v2:
>>          - Patch added
>> ---
>>   xen/arch/arm/arm32/head.S | 42 ++++++++++++++++++++----------------------
>>   1 file changed, 20 insertions(+), 22 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
>> index 4081a52dfa..6dc6032498 100644
>> --- a/xen/arch/arm/arm32/head.S
>> +++ b/xen/arch/arm/arm32/head.S
>> @@ -201,6 +201,26 @@ GLOBAL(init_secondary)
>>           mov   pc, r0
>>   secondary_switched:
>>           bl    setup_fixmap
>> +
>> +        /*
>> +         * Non-boot CPUs need to move on to the proper pagetables, which were
>> +         * setup in init_secondary_pagetables.
>> +         *
>> +         * XXX: This is not compliant with the Arm Arm.
>> +         */
>> +        ldr   r4, =init_ttbr         /* VA of HTTBR value stashed by CPU 0 */
>> +        mov   r5, #0
> 
> Why do we need to zero r5? Shouldn't ldrd overwrite r5 anyway?

I was on auto-pilot mode and saw the trailing "d" and thought it was a 
"strd". I will remove it in the next version.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel