Re: [XEN] xen/arm: arm32: Use adr_l instead of load_paddr for getting address of symbols

[Julien Grall <julien@xxxxxxx>]

Hi,

On 25/10/2023 18:03, Ayan Kumar Halder wrote:
> Before the MMU is turned on, the address returned for any symbol will always be
> physical address. Thus, one can use adr_l instead of load_paddr.
>
> create_table_entry() now takes an extra argument to denote if it is called after
> the mmu is turned on or not.  If it is called with mmu on, then it uses
> load_paddr to get the physical address of the page table symbol.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> ---
> Refer https://lists.archive.carbon60.com/xen/devel/682900 for details.
>
>   xen/arch/arm/arm32/head.S | 35 ++++++++++++++++++++---------------
>   1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 33b038e7e0..bf442b0434 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -171,7 +171,7 @@ past_zImage:
>   
>           /* Using the DTB in the .dtb section? */
>   .ifnes CONFIG_DTB_FILE,""
> -        load_paddr r8, _sdtb
> +        adr_l r8, _sdtb
>   .endif
>   
>           /* Initialize the UART if earlyprintk has been enabled. */
> @@ -213,7 +213,7 @@ GLOBAL(init_secondary)
>           mrc   CP32(r1, MPIDR)
>           bic   r7, r1, #(~MPIDR_HWID_MASK) /* Mask out flags to get CPU ID */
>   
> -        load_paddr r0, smp_up_cpu
> +        adr_l r0, smp_up_cpu
>           dsb
>   2:      ldr   r1, [r0]
>           cmp   r1, r7
> @@ -406,6 +406,7 @@ ENDPROC(cpu_init)
>    * tbl:     table symbol to point to
>    * virt:    virtual address
>    * lvl:     page-table level
> + * mmu_on:  is mmu on
>    *
>    * Preserves \virt
>    * Clobbers r1 - r4
> @@ -414,10 +415,14 @@ ENDPROC(cpu_init)
>    *
>    * Note that \virt should be in a register other than r1 - r4
>    */
> -.macro create_table_entry, ptbl, tbl, virt, lvl
> +.macro create_table_entry, ptbl, tbl, virt, lvl, mmu_on
> +    .if \mmu_on == 1
>           load_paddr r4, \tbl
> -        create_table_entry_from_paddr \ptbl, r4, \virt, \lvl
> - .endm
> +    .else
> +        adr_l r4, \tbl
> +    .endif
> +    create_table_entry_from_paddr \ptbl, r4, \virt, \lvl
> +.endm
>   
>   /*
>    * Macro to create a mapping entry in \tbl to \paddr. Only mapping in 3rd
> @@ -479,7 +484,7 @@ create_page_tables:
>            * create_table_entry_paddr() will clobber the register storing
>            * the physical address of the table to point to.
>            */
> -        load_paddr r5, boot_third
> +        adr_l r5, boot_third
>           mov_w r4, XEN_VIRT_START
>   .rept XEN_NR_ENTRIES(2)
>           mov   r0, r5                        /* r0 := paddr(l3 table) */
> @@ -522,8 +527,8 @@ create_page_tables:
>            * Setup the 1:1 mapping so we can turn the MMU on. Note that
>            * only the first page of Xen will be part of the 1:1 mapping.
>            */
> -        create_table_entry boot_pgtable, boot_second_id, r9, 1
> -        create_table_entry boot_second_id, boot_third_id, r9, 2
> +        create_table_entry boot_pgtable, boot_second_id, r9, 1, 0
> +        create_table_entry boot_second_id, boot_third_id, r9, 2, 0
>           create_mapping_entry boot_third_id, r9, r9
>   
>           /*
> @@ -537,7 +542,7 @@ create_page_tables:
>           bne   use_temporary_mapping
>   
>           mov_w r0, XEN_VIRT_START
> -        create_table_entry boot_pgtable, boot_second, r0, 1
> +        create_table_entry boot_pgtable, boot_second, r0, 1, 0
>           mov   r12, #0                /* r12 := temporary mapping not created 
> */
>           mov   pc, lr
>   
> @@ -551,7 +556,7 @@ use_temporary_mapping:
>   
>           /* Map boot_second (cover Xen mappings) to the temporary 1st slot */
>           mov_w r0, TEMPORARY_XEN_VIRT_START
> -        create_table_entry boot_pgtable, boot_second, r0, 1
> +        create_table_entry boot_pgtable, boot_second, r0, 1, 0
>   
>           mov   r12, #1                /* r12 := temporary mapping created */
>           mov   pc, lr
> @@ -578,7 +583,7 @@ enable_mmu:
>           flush_xen_tlb_local r0
>   
>           /* Write Xen's PT's paddr into the HTTBR */
> -        load_paddr r0, boot_pgtable
> +        adr_l r0, boot_pgtable
>           mov   r1, #0                 /* r0:r1 is paddr (boot_pagetable) */
>           mcrr  CP64(r0, r1, HTTBR)
>           isb
> @@ -658,7 +663,7 @@ switch_to_runtime_mapping:
>   
>           /* Map boot_second into boot_pgtable */
>           mov_w r0, XEN_VIRT_START
> -        create_table_entry boot_pgtable, boot_second, r0, 1
> +        create_table_entry boot_pgtable, boot_second, r0, 1, 1
>   
>           /* Ensure any page table updates are visible before continuing */
>           dsb   nsh
> @@ -739,7 +744,7 @@ setup_fixmap:
>   #endif
>           /* Map fixmap into boot_second */
>           mov_w r0, FIXMAP_ADDR(0)
> -        create_table_entry boot_second, xen_fixmap, r0, 2
> +        create_table_entry boot_second, xen_fixmap, r0, 2, 0
>           /* Ensure any page table updates made above have occurred. */
>           dsb   nshst
>           /*
> @@ -897,8 +902,8 @@ ENTRY(lookup_processor_type)
>    */
>   __lookup_processor_type:
>           mrc   CP32(r0, MIDR)                /* r0 := our cpu id */
> -        load_paddr r1, __proc_info_start
> -        load_paddr r2, __proc_info_end
> +        adr_l r1, __proc_info_start
> +        adr_l r2, __proc_info_end
On top of what's Michal already mentioned, you can remove the line in 
lookup_process_type() which set r10. It was only present so load_paddr() 
do nothing when called from the C world.
It is a good hack to remove :).

Cheers,

--
Julien Grall