[Xen-devel,v2,32/35] xen/arm32: head: Rework and document setup_fixmap()

At the moment, the fixmap table is only hooked when earlyprintk is used.
This is fine today because in C land, the fixmap is not used by anyone
until the the boot CPU is switching to the runtime page-tables.

In the future, the boot CPU will not switch between page-tables to
avoid TLB incoherency. Thus, the fixmap table will need to be always
hooked beofre any use. Let's start doing it now in setup_fixmap().

Lastly, document the behavior and the main registers usage within the
function.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v2:
        - Patch added
---
 xen/arch/arm/arm32/head.S | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

On Mon, 22 Jul 2019, Julien Grall wrote:
> At the moment, the fixmap table is only hooked when earlyprintk is used.
> This is fine today because in C land, the fixmap is not used by anyone
> until the the boot CPU is switching to the runtime page-tables.
> 
> In the future, the boot CPU will not switch between page-tables to
> avoid TLB incoherency. Thus, the fixmap table will need to be always
> hooked beofre any use. Let's start doing it now in setup_fixmap().
> 
> Lastly, document the behavior and the main registers usage within the
> function.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/arm32/head.S | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 56e2d09ed4..e0f8c2e0cb 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -536,8 +536,21 @@ identity_mapping_removed:
>          mov   pc, lr
>  ENDPROC(remove_identity_mapping)
>  
> +/*
> + * Map the UART in the fixmap (when earlyprintk is used) and hook the
> + * fixmap table in the page tables.
> + *
> + * The fixmap cannot be mapped in create_page_tables because it may
> + * clash with the 1:1 mapping.
> + *
> + * Inputs:
> + *   r10: Physical offset
> + *   r11: Early UART base physical address
> + *
> + * Clobbers r1 - r4
> + */
>  setup_fixmap:
> -#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
> +#if defined(CONFIG_EARLY_PRINTK)
>          /* Add UART to the fixmap table */
>          ldr   r1, =xen_fixmap        /* r1 := vaddr (xen_fixmap) */
>          lsr   r2, r11, #THIRD_SHIFT
> @@ -546,7 +559,7 @@ setup_fixmap:
>          orr   r2, r2, #PT_LOWER(DEV_L3) /* r2:r3 := 4K dev map including UART */
>          mov   r3, #0x0
>          strd  r2, r3, [r1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */
> -1:
> +#endif

Patch is OK. However, the 1: should be removed in the previous patch
"xen/arm32: head: Don't setup the fixmap on secondary CPUs", where we
took away the beq.

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


>          /* Map fixmap into boot_second */
>          ldr   r1, =boot_second       /* r1 := vaddr (boot_second) */
> @@ -561,7 +574,7 @@ setup_fixmap:
>  
>          /* Ensure any page table updates made above have occurred. */
>          dsb   nshst
> -#endif
> +
>          mov   pc, lr
>  ENDPROC(setup_fixmap)
>  
> -- 
> 2.11.0
>

Hi Stefano,

On 7/30/19 10:14 PM, Stefano Stabellini wrote:
> On Mon, 22 Jul 2019, Julien Grall wrote:
>> At the moment, the fixmap table is only hooked when earlyprintk is used.
>> This is fine today because in C land, the fixmap is not used by anyone
>> until the the boot CPU is switching to the runtime page-tables.
>>
>> In the future, the boot CPU will not switch between page-tables to
>> avoid TLB incoherency. Thus, the fixmap table will need to be always
>> hooked beofre any use. Let's start doing it now in setup_fixmap().
>>
>> Lastly, document the behavior and the main registers usage within the
>> function.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> ---
>>      Changes in v2:
>>          - Patch added
>> ---
>>   xen/arch/arm/arm32/head.S | 19 ++++++++++++++++---
>>   1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
>> index 56e2d09ed4..e0f8c2e0cb 100644
>> --- a/xen/arch/arm/arm32/head.S
>> +++ b/xen/arch/arm/arm32/head.S
>> @@ -536,8 +536,21 @@ identity_mapping_removed:
>>           mov   pc, lr
>>   ENDPROC(remove_identity_mapping)
>>   
>> +/*
>> + * Map the UART in the fixmap (when earlyprintk is used) and hook the
>> + * fixmap table in the page tables.
>> + *
>> + * The fixmap cannot be mapped in create_page_tables because it may
>> + * clash with the 1:1 mapping.
>> + *
>> + * Inputs:
>> + *   r10: Physical offset
>> + *   r11: Early UART base physical address
>> + *
>> + * Clobbers r1 - r4
>> + */
>>   setup_fixmap:
>> -#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
>> +#if defined(CONFIG_EARLY_PRINTK)
>>           /* Add UART to the fixmap table */
>>           ldr   r1, =xen_fixmap        /* r1 := vaddr (xen_fixmap) */
>>           lsr   r2, r11, #THIRD_SHIFT
>> @@ -546,7 +559,7 @@ setup_fixmap:
>>           orr   r2, r2, #PT_LOWER(DEV_L3) /* r2:r3 := 4K dev map including UART */
>>           mov   r3, #0x0
>>           strd  r2, r3, [r1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */
>> -1:
>> +#endif
> 
> Patch is OK. However, the 1: should be removed in the previous patch
> "xen/arm32: head: Don't setup the fixmap on secondary CPUs", where we
> took away the beq.

Good point. I have now moved this to the previous patch.

> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Thank you!

Cheers,