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

[Julien Grall <julien@xxxxxxx>]

Hi,

Title: This reads as you replace all adr_l with load_paddr. So how about:

xen/arm32: head: Replace load_paddr with adr_l when they are equivalent

On 26/10/2023 12:12, Ayan Kumar Halder wrote:
> Before the MMU is turned on, PC uses physical address. Thus, one can use adr_l
> instead of load_paddr to obtain the physical address of a symbol.
>
> The only exception (for this replacement) is create_table_entry() which is
> called before and after MMU is turned on.
>
> Also, in lookup_processor_type() "r10" is no longer used. The reason being
> __lookup_processor_type uses adr_l (thus r10 is no longer used to obtain the
> physical address offset).
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> ---
> Refer https://lists.archive.carbon60.com/xen/devel/682900 for details.
>
> Changes from :-
>
> v1 :- 1. No need to modify create_table_entry().
> 2. Remove "mov   r10, #0 " in lookup_processor_type().
>
>   xen/arch/arm/arm32/head.S | 13 ++++++-------
>   1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 33b038e7e0..e0ff46e92f 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
> @@ -479,7 +479,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) */
> @@ -578,7 +578,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
> @@ -877,7 +877,6 @@ putn:   mov   pc, lr
>   /* This provides a C-API version of __lookup_processor_type */
>   ENTRY(lookup_processor_type)
>           stmfd sp!, {r4, r10, lr}
Do we still need to save/restore r10?

> -        mov   r10, #0                   /* r10 := offset between virt&phys */
>           bl    __lookup_processor_type
>           mov r0, r1
>           ldmfd sp!, {r4, r10, pc}
> @@ -897,8 +896,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
>   1:      ldr   r3, [r1, #PROCINFO_cpu_mask]
>           and   r4, r0, r3                    /* r4 := our cpu id with mask */
>           ldr   r3, [r1, #PROCINFO_cpu_val]   /* r3 := cpu val in current proc 
> info */
Cheers,

--
Julien Grall