diff mbox series

[MM-PART2,RESEND,v2,12/19] xen/arm32: head: Always zero r3 before update a page-table entry

Message ID 20190514122456.28559-13-julien.grall@arm.com (mailing list archive)
State New, archived
Headers show
Series xen/arm: Clean-up & fixes in boot/mm code | expand

Commit Message

Julien Grall May 14, 2019, 12:24 p.m. UTC
The boot code is using r2 and r3 to hold the page-table entry value.
While r2 is always updated before storing the value, this is not always
the case for r3.

Thankfully today, r3 will always be zero when we care. But this is
difficult to track and error-prone.

So always zero r3 within the few instructions before the write the
page-table entry.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v2:
        - Use 0x0 instead of 0
        - Remove a duplicate mov r3, #0
---
 xen/arch/arm/arm32/head.S | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Andrii Anisov May 21, 2019, 10:03 a.m. UTC | #1
On 14.05.19 15:24, Julien Grall wrote:
> The boot code is using r2 and r3 to hold the page-table entry value.
> While r2 is always updated before storing the value, this is not always
> the case for r3.
> 
> Thankfully today, r3 will always be zero when we care. But this is
> difficult to track and error-prone.
> 
> So always zero r3 within the few instructions before the write the
> page-table entry.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
>      Changes in v2:
>          - Use 0x0 instead of 0
>          - Remove a duplicate mov r3, #0
> ---


Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
Stefano Stabellini June 3, 2019, 11:15 p.m. UTC | #2
On Tue, 14 May 2019, Julien Grall wrote:
> The boot code is using r2 and r3 to hold the page-table entry value.
> While r2 is always updated before storing the value, this is not always
> the case for r3.
> 
> Thankfully today, r3 will always be zero when we care. But this is
> difficult to track and error-prone.
> 
> So always zero r3 within the few instructions before the write the
> page-table entry.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Use 0x0 instead of 0
>         - Remove a duplicate mov r3, #0
> ---
>  xen/arch/arm/arm32/head.S | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 3448817aab..18ded49a04 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -270,6 +270,7 @@ cpu_init_done:
>          orr   r2, r2, #PT_UPPER(MEM) /* r2:r3 := section map */
>          orr   r2, r2, #PT_LOWER(MEM)
>          lsl   r1, r1, #3             /* r1 := Slot offset */
> +        mov   r3, #0x0
>          strd  r2, r3, [r4, r1]       /* Mapping of paddr(start) */
>          mov   r6, #1                 /* r6 := identity map now in place */
>  
> @@ -372,11 +373,11 @@ paging:
>  
>          /* Add UART to the fixmap table */
>          ldr   r1, =xen_fixmap        /* r1 := vaddr (xen_fixmap) */
> -        mov   r3, #0
>          lsr   r2, r11, #THIRD_SHIFT
>          lsl   r2, r2, #THIRD_SHIFT   /* 4K aligned paddr of UART */
>          orr   r2, r2, #PT_UPPER(DEV_L3)
>          orr   r2, r2, #PT_LOWER(DEV_L3) /* r2:r3 := 4K dev map including UART */
> +        mov   r3, #0x0
>          strd  r2, r3, [r1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */
>  1:
>  
> @@ -388,6 +389,7 @@ paging:
>          orr   r2, r2, #PT_LOWER(PT)  /* r2:r3 := table map of xen_fixmap */
>          ldr   r4, =FIXMAP_ADDR(0)
>          mov   r4, r4, lsr #(SECOND_SHIFT - 3)   /* r4 := Slot for FIXMAP(0) */
> +        mov   r3, #0x0
>          strd  r2, r3, [r1, r4]       /* Map it in the fixmap's slot */
>  
>          /* Use a virtual address to access the UART. */
> -- 
> 2.11.0
>
diff mbox series

Patch

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 3448817aab..18ded49a04 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -270,6 +270,7 @@  cpu_init_done:
         orr   r2, r2, #PT_UPPER(MEM) /* r2:r3 := section map */
         orr   r2, r2, #PT_LOWER(MEM)
         lsl   r1, r1, #3             /* r1 := Slot offset */
+        mov   r3, #0x0
         strd  r2, r3, [r4, r1]       /* Mapping of paddr(start) */
         mov   r6, #1                 /* r6 := identity map now in place */
 
@@ -372,11 +373,11 @@  paging:
 
         /* Add UART to the fixmap table */
         ldr   r1, =xen_fixmap        /* r1 := vaddr (xen_fixmap) */
-        mov   r3, #0
         lsr   r2, r11, #THIRD_SHIFT
         lsl   r2, r2, #THIRD_SHIFT   /* 4K aligned paddr of UART */
         orr   r2, r2, #PT_UPPER(DEV_L3)
         orr   r2, r2, #PT_LOWER(DEV_L3) /* r2:r3 := 4K dev map including UART */
+        mov   r3, #0x0
         strd  r2, r3, [r1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */
 1:
 
@@ -388,6 +389,7 @@  paging:
         orr   r2, r2, #PT_LOWER(PT)  /* r2:r3 := table map of xen_fixmap */
         ldr   r4, =FIXMAP_ADDR(0)
         mov   r4, r4, lsr #(SECOND_SHIFT - 3)   /* r4 := Slot for FIXMAP(0) */
+        mov   r3, #0x0
         strd  r2, r3, [r1, r4]       /* Map it in the fixmap's slot */
 
         /* Use a virtual address to access the UART. */