| Message ID | 20231027180715.3484439-1-ayan.kumar.halder@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [XEN,v3] xen/arm32: head: Replace load_paddr with adr_l when they are equivalent | expand |
Hi Ayan, On 27/10/2023 20:07, 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). Consequently, there is no need to save/restore r10. > > 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(). > > v2 :- 1. No need to save/restore r10 in lookup_processor_type(). > 2. Update the commit message title. > > xen/arch/arm/arm32/head.S | 19 ++++++++----------- > 1 file changed, 8 insertions(+), 11 deletions(-) > > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S > index 33b038e7e0..1fcc6f745e 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 > @@ -876,11 +876,10 @@ 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 */ > + stmfd sp!, {r4, lr} > bl __lookup_processor_type > mov r0, r1 > - ldmfd sp!, {r4, r10, pc} > + ldmfd sp!, {r4, pc} > > /* > * Read processor ID register (CP#15, CR0), and Look up in the linker-built > @@ -888,8 +887,6 @@ ENTRY(lookup_processor_type) > * the __proc_info lists since we aren't running with the MMU on (and therefore, > * we are not in correct address space). We have to calculate the offset. In v2, I mentioned that this comment needs to be tweaked as well. We no longer use load_paddr thus we don't care about the offset. I would remove the comment starting from "Note that...". to avoid confusion or add a proper explanation if you want to keep it. With that addressed: Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
On 30/10/2023 08:28, Michal Orzel wrote: > Hi Ayan, > > On 27/10/2023 20:07, 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). Consequently, there is no need to save/restore r10. >> >> 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(). >> >> v2 :- 1. No need to save/restore r10 in lookup_processor_type(). >> 2. Update the commit message title. >> >> xen/arch/arm/arm32/head.S | 19 ++++++++----------- >> 1 file changed, 8 insertions(+), 11 deletions(-) >> >> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S >> index 33b038e7e0..1fcc6f745e 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 >> @@ -876,11 +876,10 @@ 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 */ >> + stmfd sp!, {r4, lr} >> bl __lookup_processor_type >> mov r0, r1 >> - ldmfd sp!, {r4, r10, pc} >> + ldmfd sp!, {r4, pc} >> >> /* >> * Read processor ID register (CP#15, CR0), and Look up in the linker-built >> @@ -888,8 +887,6 @@ ENTRY(lookup_processor_type) >> * the __proc_info lists since we aren't running with the MMU on (and therefore, >> * we are not in correct address space). We have to calculate the offset. > In v2, I mentioned that this comment needs to be tweaked as well. We no longer use load_paddr > thus we don't care about the offset. I would remove the comment starting from "Note that...". > to avoid confusion or add a proper explanation if you want to keep it. > With that addressed: > Reviewed-by: Michal Orzel <michal.orzel@amd.com> I have committed with the following diff: diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 1fcc6f745e31..bbbdf7daf89e 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -882,10 +882,8 @@ ENTRY(lookup_processor_type) ldmfd sp!, {r4, pc} /* - * Read processor ID register (CP#15, CR0), and Look up in the linker-built - * supported processor list. Note that we can't use the absolute addresses for - * the __proc_info lists since we aren't running with the MMU on (and therefore, - * we are not in correct address space). We have to calculate the offset. + * Read processor ID register (CP#15, CR0), and Look up in the linker-built + * supported processor list. * * Returns: * r0: CPUID Note that I took the opportunity to remove the extra space on the first line of the comment. > > ~Michal
On 16/11/2023 14:07, Julien Grall wrote: > On 30/10/2023 08:28, Michal Orzel wrote: >> Hi Ayan, >> >> On 27/10/2023 20:07, 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). Consequently, there is no need to >>> save/restore r10. >>> >>> 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(). >>> >>> v2 :- 1. No need to save/restore r10 in lookup_processor_type(). >>> 2. Update the commit message title. >>> >>> xen/arch/arm/arm32/head.S | 19 ++++++++----------- >>> 1 file changed, 8 insertions(+), 11 deletions(-) >>> >>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S >>> index 33b038e7e0..1fcc6f745e 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 >>> @@ -876,11 +876,10 @@ 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 */ >>> + stmfd sp!, {r4, lr} >>> bl __lookup_processor_type >>> mov r0, r1 >>> - ldmfd sp!, {r4, r10, pc} >>> + ldmfd sp!, {r4, pc} >>> /* >>> * Read processor ID register (CP#15, CR0), and Look up in the >>> linker-built >>> @@ -888,8 +887,6 @@ ENTRY(lookup_processor_type) >>> * the __proc_info lists since we aren't running with the MMU on >>> (and therefore, >>> * we are not in correct address space). We have to calculate the >>> offset. >> In v2, I mentioned that this comment needs to be tweaked as well. We >> no longer use load_paddr >> thus we don't care about the offset. I would remove the comment >> starting from "Note that...". >> to avoid confusion or add a proper explanation if you want to keep it. >> With that addressed: >> Reviewed-by: Michal Orzel <michal.orzel@amd.com> > > I have committed with the following diff: > > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S > index 1fcc6f745e31..bbbdf7daf89e 100644 > --- a/xen/arch/arm/arm32/head.S > +++ b/xen/arch/arm/arm32/head.S > @@ -882,10 +882,8 @@ ENTRY(lookup_processor_type) > ldmfd sp!, {r4, pc} > > /* > - * Read processor ID register (CP#15, CR0), and Look up in the > linker-built > - * supported processor list. Note that we can't use the absolute > addresses for > - * the __proc_info lists since we aren't running with the MMU on (and > therefore, > - * we are not in correct address space). We have to calculate the offset. > + * Read processor ID register (CP#15, CR0), and Look up in the > linker-built > + * supported processor list. > * > * Returns: > * r0: CPUID > > Note that I took the opportunity to remove the extra space on the first > line of the comment. Oh I didn't realize there was a v4 sent. Looking at it this was this only change. So I will not revert. Sorry for that. Cheers,
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 33b038e7e0..1fcc6f745e 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 @@ -876,11 +876,10 @@ 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 */ + stmfd sp!, {r4, lr} bl __lookup_processor_type mov r0, r1 - ldmfd sp!, {r4, r10, pc} + ldmfd sp!, {r4, pc} /* * Read processor ID register (CP#15, CR0), and Look up in the linker-built @@ -888,8 +887,6 @@ ENTRY(lookup_processor_type) * the __proc_info lists since we aren't running with the MMU on (and therefore, * we are not in correct address space). We have to calculate the offset. * - * r10: offset between virt&phys - * * Returns: * r0: CPUID * r1: proc_info pointer @@ -897,8 +894,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 */
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). Consequently, there is no need to save/restore r10. 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(). v2 :- 1. No need to save/restore r10 in lookup_processor_type(). 2. Update the commit message title. xen/arch/arm/arm32/head.S | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)