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

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

Posted by Ayan Kumar Halder 6 months, 1 week ago

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@amd.com>
---
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}
-        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 */
-- 
2.25.1

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

Posted by Julien Grall 6 months, 1 week ago

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@amd.com>
> ---
> 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

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

Posted by Michal Orzel 6 months, 1 week ago

Hi Ayan,

On 26/10/2023 13:19, Julien Grall wrote:
> 
> 
> 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@amd.com>
>> ---
>> 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(-)
>>

[...]
>>   /* 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?
Apart from this remark...

> 
>> -        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)
>>    */
there is also a comment here listing r10 as the register in use and explaining
the need to calculate the offset. This should be tweaked as well (r10 is no longer in use
and we don't care if we are running before or after MMU is on, since adr_l will always get us
the address in the current address space which is what we expect (before MMU - physical, after MMU - virtual).

>>   __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

~Michal