Hi Julien,
> On 12 Aug 2022, at 20:24, Julien Grall <jul...@xen.org> wrote:
>
> From: Julien Grall <jgr...@amazon.com>
>
> At the moment, the macro addr_l needs to know whether the caller
> is running with the MMU on. This is fine today because there are
> only two possible cases:
> 1) MMU off
> 2) MMU on and linked to the virtual address
>
> This is still cumbersome to use for the developer as they need
> to know if the MMU is on.
>
> Thankfully, Linux developpers came up with a great way to allow
> adr_l to work within the range +/- 4GB of PC by emitting a PC-relative
> reference [1].
This is indeed a great solution :-)
>
> Re-use the same approach on Arm and drop the parameter 'mmu'.
>
> [1] 0b1674638a5c ("ARM: assembler: introduce adr_l, ldr_l and str_l macros")
>
> Signed-off-by: Julien Grall <jgr...@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marq...@arm.com>
>
> ----
> I haven't added an Origin tag because this is quite different
> from the Linux commit. I am happy to add one if this is desired..
I think the reference in the commit message is enough as you reuse the
idea but not the code per say.
Cheers
Bertrand
> ---
> xen/arch/arm/arm32/head.S | 38 +++++++++++++++-----------------------
> 1 file changed, 15 insertions(+), 23 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 50f6fa4eb38d..27d02ac8d68f 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -49,20 +49,16 @@
> .endm
>
> /*
> - * There are no easy way to have a PC relative address within the range
> - * +/- 4GB of the PC.
> + * Pseudo-op for PC relative adr <reg>, <symbol> where <symbol> is
> + * within the range +/- 4GB of the PC.
> *
> - * This macro workaround it by asking the user to tell whether the MMU
> - * has been turned on or not.
> - *
> - * When the MMU is turned off, we need to apply the physical offset
> - * (r10) in order to find the associated physical address.
> + * @dst: destination register
> + * @sym: name of the symbol
> */
> -.macro adr_l, dst, sym, mmu
> - ldr \dst, =\sym
> - .if \mmu == 0
> - add \dst, \dst, r10
> - .endif
> +.macro adr_l, dst, sym
> + mov_w \dst, \sym - .Lpc\@
> + .set .Lpc\@, .+ 8 /* PC bias */
> + add \dst, \dst, pc
> .endm
>
> .macro load_paddr rb, sym
> @@ -383,7 +379,6 @@ ENDPROC(cpu_init)
> * tbl: table symbol to point to
> * virt: virtual address
> * lvl: page-table level
> - * mmu: Is the MMU turned on/off. If not specified it will be off
> *
> * Preserves \virt
> * Clobbers r1 - r4
> @@ -392,7 +387,7 @@ ENDPROC(cpu_init)
> *
> * Note that \virt should be in a register other than r1 - r4
> */
> -.macro create_table_entry, ptbl, tbl, virt, lvl, mmu=0
> +.macro create_table_entry, ptbl, tbl, virt, lvl
> get_table_slot r1, \virt, \lvl /* r1 := slot in \tlb */
> lsl r1, r1, #3 /* r1 := slot offset in \tlb */
>
> @@ -402,7 +397,7 @@ ENDPROC(cpu_init)
> orr r2, r2, r4 /* + \tlb paddr */
> mov r3, #0
>
> - adr_l r4, \ptbl, \mmu
> + adr_l r4, \ptbl
>
> strd r2, r3, [r4, r1]
> .endm
> @@ -415,17 +410,14 @@ ENDPROC(cpu_init)
> * virt: virtual address
> * phys: physical address
> * type: mapping type. If not specified it will be normal memory
> (PT_MEM_L3)
> - * mmu: Is the MMU turned on/off. If not specified it will be off
> *
> * Preserves \virt, \phys
> * Clobbers r1 - r4
> *
> - * * Also use r10 for the phys offset.
> - *
> * Note that \virt and \paddr should be in other registers than r1 - r4
> * and be distinct.
> */
> -.macro create_mapping_entry, ptbl, virt, phys, type=PT_MEM_L3, mmu=0
> +.macro create_mapping_entry, ptbl, virt, phys, type=PT_MEM_L3
> mov_w r2, XEN_PT_LPAE_ENTRY_MASK
> lsr r1, \virt, #THIRD_SHIFT
> and r1, r1, r2 /* r1 := slot in \tlb */
> @@ -438,7 +430,7 @@ ENDPROC(cpu_init)
> orr r2, r2, r4 /* + PAGE_ALIGNED(phys) */
> mov r3, #0
>
> - adr_l r4, \ptbl, \mmu
> + adr_l r4, \ptbl
>
> strd r2, r3, [r4, r1]
> .endm
> @@ -468,7 +460,7 @@ create_page_tables:
> create_table_entry boot_second, boot_third, r0, 2
>
> /* Setup boot_third: */
> - adr_l r4, boot_third, mmu=0
> + adr_l r4, boot_third
>
> lsr r2, r9, #THIRD_SHIFT /* Base address for 4K mapping */
> lsl r2, r2, #THIRD_SHIFT
> @@ -632,11 +624,11 @@ setup_fixmap:
> #if defined(CONFIG_EARLY_PRINTK)
> /* Add UART to the fixmap table */
> ldr r0, =EARLY_UART_VIRTUAL_ADDRESS
> - create_mapping_entry xen_fixmap, r0, r11, type=PT_DEV_L3, mmu=1
> + create_mapping_entry xen_fixmap, r0, r11, type=PT_DEV_L3
> #endif
> /* Map fixmap into boot_second */
> mov_w r0, FIXMAP_ADDR(0)
> - create_table_entry boot_second, xen_fixmap, r0, 2, mmu=1
> + create_table_entry boot_second, xen_fixmap, r0, 2
> /* Ensure any page table updates made above have occurred. */
> dsb nshst
>
> --
> 2.37.1
>
