diff mbox series

[4/7] xen/arm32: heap: Rework adr_l so it doesn't rely on where Xen is loaded

Message ID 20220812192448.43016-5-julien@xen.org (mailing list archive)
State New, archived
Headers show
Series xen/arm: More clean-ups and improvement | expand

Commit Message

Julien Grall Aug. 12, 2022, 7:24 p.m. UTC 
From: Julien Grall <jgrall@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].

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 <jgrall@amazon.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..
---
 xen/arch/arm/arm32/head.S | 38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

Comments

Wei Chen Aug. 15, 2022, 1:56 a.m. UTC | #1 
Hi Julien,

On 2022/8/13 3:24, Julien Grall wrote:
> From: Julien Grall <jgrall@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].
> 
> 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 <jgrall@amazon.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..
> ---
>   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
>   

LGTM.

Reviewed-by: Wei Chen <Wei.Chen@arm.com>
Bertrand Marquis Aug. 15, 2022, 3:28 p.m. UTC | #2 
Hi Julien,

> On 12 Aug 2022, at 20:24, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@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 <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@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
>
diff mbox series

Patch

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