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.
*
- * 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 */
--
2.25.1
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 -- Julien Grall
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, -- Julien Grall
© 2016 - 2024 Red Hat, Inc.