| Message ID | 20231017125219.76626-1-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for-4.19] xen/arm64: head: only use the macro load_paddr() in the MMU code | expand |
Hi Julien, On 17/10/2023 14:52, Julien Grall wrote: > > > The macro load_paddr() requires to know the offset between the > physical location of Xen and the virtual location. > > When using the MPU, x20 will always be 0. Rather than wasting > a register for a compile-time constant value, it would be best if > we can avoid using load_paddr() altogher in the common head.S code. s/altogher/altogether/ > > The current use of load_paddr() are equivalent to adr_l() because > the MMU is off. > > All the use of load_paddr() in arm64/head.S are now replaced with > adr_l(). With that, load_paddr() can now be moved in arm64/mmu/head.S. > > For now, x20 is still unconditionally set. But this could change > in the future if needed. > > Signed-off-by: Julien Grall <julien@xen.org> Reviewed-by: Michal Orzel <michal.orzel@amd.com> Side note: Looking at all the uses of load_paddr(), none of them takes place after enabling MMU which would indicate that we actually don't need this macro at all. Do you agree? ~Michal
Hi, On 17/10/2023 15:07, Michal Orzel wrote: > On 17/10/2023 14:52, Julien Grall wrote: >> >> >> The macro load_paddr() requires to know the offset between the >> physical location of Xen and the virtual location. >> >> When using the MPU, x20 will always be 0. Rather than wasting >> a register for a compile-time constant value, it would be best if >> we can avoid using load_paddr() altogher in the common head.S code. > s/altogher/altogether/ I will fix it. > >> >> The current use of load_paddr() are equivalent to adr_l() because >> the MMU is off. >> >> All the use of load_paddr() in arm64/head.S are now replaced with >> adr_l(). With that, load_paddr() can now be moved in arm64/mmu/head.S. >> >> For now, x20 is still unconditionally set. But this could change >> in the future if needed. >> >> Signed-off-by: Julien Grall <julien@xen.org> > Reviewed-by: Michal Orzel <michal.orzel@amd.com> > > Side note: > Looking at all the uses of load_paddr(), none of them takes place after enabling MMU > which would indicate that we actually don't need this macro at all. Do you agree? I agree they are none today called after enabling the MMU. But, create_table_entry() used to be called after and I can't rule out this will not happen again. So I don't think we should replace the use in create_table_entry() with adr_l. We could consider to open-code code though. Cheers,
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 4ad85dcf58d2..8dbd3300d89f 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -259,7 +259,7 @@ real_start_efi: /* Using the DTB in the .dtb section? */ .ifnes CONFIG_DTB_FILE,"" - load_paddr x21, _sdtb + adr_l x21, _sdtb .endif /* Initialize the UART if earlyprintk has been enabled. */ @@ -301,7 +301,7 @@ GLOBAL(init_secondary) bic x24, x0, x13 /* Mask out flags to get CPU ID */ /* Wait here until __cpu_up is ready to handle the CPU */ - load_paddr x0, smp_up_cpu + adr_l x0, smp_up_cpu dsb sy 2: ldr x1, [x0] cmp x1, x24 diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S index 88075ef08334..412b28e649a4 100644 --- a/xen/arch/arm/arm64/mmu/head.S +++ b/xen/arch/arm/arm64/mmu/head.S @@ -19,6 +19,12 @@ #define XEN_FIRST_SLOT first_table_offset(XEN_VIRT_START) #define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START) +/* Load the physical address of a symbol into xb */ +.macro load_paddr xb, sym + ldr \xb, =\sym + add \xb, \xb, x20 +.endm + /* * Flush local TLBs * diff --git a/xen/arch/arm/include/asm/arm64/macros.h b/xen/arch/arm/include/asm/arm64/macros.h index 99c401fcafa9..fb9a0602494d 100644 --- a/xen/arch/arm/include/asm/arm64/macros.h +++ b/xen/arch/arm/include/asm/arm64/macros.h @@ -62,12 +62,6 @@ add \dst, \dst, :lo12:\sym .endm -/* Load the physical address of a symbol into xb */ -.macro load_paddr xb, sym - ldr \xb, =\sym - add \xb, \xb, x20 -.endm - /* * Register aliases. */
The macro load_paddr() requires to know the offset between the physical location of Xen and the virtual location. When using the MPU, x20 will always be 0. Rather than wasting a register for a compile-time constant value, it would be best if we can avoid using load_paddr() altogher in the common head.S code. The current use of load_paddr() are equivalent to adr_l() because the MMU is off. All the use of load_paddr() in arm64/head.S are now replaced with adr_l(). With that, load_paddr() can now be moved in arm64/mmu/head.S. For now, x20 is still unconditionally set. But this could change in the future if needed. Signed-off-by: Julien Grall <julien@xen.org> --- xen/arch/arm/arm64/head.S | 4 ++-- xen/arch/arm/arm64/mmu/head.S | 6 ++++++ xen/arch/arm/include/asm/arm64/macros.h | 6 ------ 3 files changed, 8 insertions(+), 8 deletions(-)