[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2] xen/arm{32, 64}: fix section shift when mapping 2MB block in boot page table



On Tue, 2014-01-14 at 06:19 +0800, Chen Baozi wrote:
> Section shift for level-2 page table should be #21 rather than #20. Besides,
> since there are {FIRST,SECOND,THIRD}_SHIFT macros defined in asm/page.h, use
> these macros instead of hard-coded shift value.
> 
> Signed-off-by: Chen Baozi <baozich@xxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

WRT a 4.4 freeze exception the main bit is the use of #21 instead of #20
as the shift for the L2 entry, which can result in an UNK/SBZP bit being
set. ARM ARM says:

        Hardware must implement the bit as Read-As-Zero, and must ignore
        writes to the field.
        
        Software must not rely on the field reading as all 0s, and
        except for writing back to the register must treat the value
        as if it is UNKNOWN. Software must use an SBZP policy to write
        to the field.

The danger is that some future version of the architecture assigns
meaning to that bit. All in all this seems like a pretty benign issue,
but on the flip side the fix is reasonable low risk, the only real
danger is that one of the replacements is wrong and most of them are
pretty trivial, although s/#18/#(SECOND_SHIFT - 3)/ is a bit less so.

I was initially leaning towards putting this into the queue for 4.5, but
on reflection I'm now starting to lean the other way.

Does anyone feel strongly that this shouldn't go into 4.4?

> ---
>  xen/arch/arm/arm32/head.S | 20 ++++++++++----------
>  xen/arch/arm/arm64/head.S | 26 +++++++++++++-------------
>  2 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 96230ac..f3eab89 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -291,14 +291,14 @@ cpu_init_done:
>          ldr   r4, =boot_second
>          add   r4, r4, r10            /* r1 := paddr (boot_second) */
>  
> -        lsr   r2, r9, #20            /* Base address for 2MB mapping */
> -        lsl   r2, r2, #20
> +        lsr   r2, r9, #SECOND_SHIFT  /* Base address for 2MB mapping */
> +        lsl   r2, r2, #SECOND_SHIFT
>          orr   r2, r2, #PT_UPPER(MEM) /* r2:r3 := section map */
>          orr   r2, r2, #PT_LOWER(MEM)
>  
>          /* ... map of vaddr(start) in boot_second */
>          ldr   r1, =start
> -        lsr   r1, #18                /* Slot for vaddr(start) */
> +        lsr   r1, #(SECOND_SHIFT - 3)   /* Slot for vaddr(start) */
>          strd  r2, r3, [r4, r1]       /* Map vaddr(start) */
>  
>          /* ... map of paddr(start) in boot_second */
> @@ -307,7 +307,7 @@ cpu_init_done:
>                                        * then the mapping was done in
>                                        * boot_pgtable above */
>  
> -        mov   r1, r9, lsr #18        /* Slot for paddr(start) */
> +        mov   r1, r9, lsr #(SECOND_SHIFT - 3)   /* Slot for paddr(start) */
>          strd  r2, r3, [r4, r1]       /* Map Xen there */
>  1:
>  
> @@ -339,8 +339,8 @@ paging:
>          /* Add UART to the fixmap table */
>          ldr   r1, =xen_fixmap        /* r1 := vaddr (xen_fixmap) */
>          mov   r3, #0
> -        lsr   r2, r11, #12
> -        lsl   r2, r2, #12            /* 4K aligned paddr of UART */
> +        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 */
>          strd  r2, r3, [r1, #(FIXMAP_CONSOLE*8)] /* Map it in the first 
> fixmap's slot */
> @@ -353,7 +353,7 @@ paging:
>          orr   r2, r2, #PT_UPPER(PT)
>          orr   r2, r2, #PT_LOWER(PT)  /* r2:r3 := table map of xen_fixmap */
>          ldr   r4, =FIXMAP_ADDR(0)
> -        mov   r4, r4, lsr #18        /* r4 := Slot for FIXMAP(0) */
> +        mov   r4, r4, lsr #(SECOND_SHIFT - 3)   /* r4 := Slot for FIXMAP(0) 
> */
>          strd  r2, r3, [r1, r4]       /* Map it in the fixmap's slot */
>  
>          /* Use a virtual address to access the UART. */
> @@ -365,12 +365,12 @@ paging:
>  
>          ldr   r1, =boot_second
>          mov   r3, #0x0
> -        lsr   r2, r8, #21
> -        lsl   r2, r2, #21            /* r2: 2MB-aligned paddr of DTB */
> +        lsr   r2, r8, #SECOND_SHIFT
> +        lsl   r2, r2, #SECOND_SHIFT  /* r2: 2MB-aligned paddr of DTB */
>          orr   r2, r2, #PT_UPPER(MEM)
>          orr   r2, r2, #PT_LOWER(MEM) /* r2:r3 := 2MB RAM incl. DTB */
>          ldr   r4, =BOOT_FDT_VIRT_START
> -        mov   r4, r4, lsr #18        /* Slot for BOOT_FDT_VIRT_START */
> +        mov   r4, r4, lsr #(SECOND_SHIFT)   /* Slot for BOOT_FDT_VIRT_START 
> */
>          strd  r2, r3, [r1, r4]       /* Map it in the early fdt slot */
>          dsb
>  1:
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index bebddf0..5b164e9 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -278,11 +278,11 @@ skip_bss:
>          str   x2, [x4, #0]           /* Map it in slot 0 */
>  
>          /* ... map of paddr(start) in boot_first */
> -        lsr   x2, x19, #30           /* x2 := Offset of base paddr in 
> boot_first */
> +        lsr   x2, x19, #FIRST_SHIFT  /* x2 := Offset of base paddr in 
> boot_first */
>          and   x1, x2, 0x1ff          /* x1 := Slot to use */
>          cbz   x1, 1f                 /* It's in slot 0, map in boot_second */
>  
> -        lsl   x2, x2, #30            /* Base address for 1GB mapping */
> +        lsl   x2, x2, #FIRST_SHIFT   /* Base address for 1GB mapping */
>          mov   x3, #PT_MEM            /* x2 := Section map */
>          orr   x2, x2, x3
>          lsl   x1, x1, #3             /* x1 := Slot offset */
> @@ -292,23 +292,23 @@ skip_bss:
>          ldr   x4, =boot_second
>          add   x4, x4, x20            /* x4 := paddr (boot_second) */
>  
> -        lsr   x2, x19, #20           /* Base address for 2MB mapping */
> -        lsl   x2, x2, #20
> +        lsr   x2, x19, #SECOND_SHIFT /* Base address for 2MB mapping */
> +        lsl   x2, x2, #SECOND_SHIFT
>          mov   x3, #PT_MEM            /* x2 := Section map */
>          orr   x2, x2, x3
>  
>          /* ... map of vaddr(start) in boot_second */
>          ldr   x1, =start
> -        lsr   x1, x1, #18            /* Slot for vaddr(start) */
> +        lsr   x1, x1, #(SECOND_SHIFT - 3)   /* Slot for vaddr(start) */
>          str   x2, [x4, x1]           /* Map vaddr(start) */
>  
>          /* ... map of paddr(start) in boot_second */
> -        lsr   x1, x19, #30           /* Base paddr */
> +        lsr   x1, x19, #FIRST_SHIFT  /* Base paddr */
>          cbnz  x1, 1f                 /* If paddr(start) is not in slot 0
>                                        * then the mapping was done in
>                                        * boot_pgtable or boot_first above */
>  
> -        lsr   x1, x19, #18           /* Slot for paddr(start) */
> +        lsr   x1, x19, #(SECOND_SHIFT - 3)  /* Slot for paddr(start) */
>          str   x2, [x4, x1]           /* Map Xen there */
>  1:
>  
> @@ -340,8 +340,8 @@ paging:
>          /* Add UART to the fixmap table */
>          ldr   x1, =xen_fixmap
>          add   x1, x1, x20            /* x1 := paddr (xen_fixmap) */
> -        lsr   x2, x23, #12
> -        lsl   x2, x2, #12            /* 4K aligned paddr of UART */
> +        lsr   x2, x23, #THIRD_SHIFT
> +        lsl   x2, x2, #THIRD_SHIFT   /* 4K aligned paddr of UART */
>          mov   x3, #PT_DEV_L3
>          orr   x2, x2, x3             /* x2 := 4K dev map including UART */
>          str   x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's 
> slot */
> @@ -354,7 +354,7 @@ paging:
>          mov   x3, #PT_PT
>          orr   x2, x2, x3             /* x2 := table map of xen_fixmap */
>          ldr   x1, =FIXMAP_ADDR(0)
> -        lsr   x1, x1, #18            /* x1 := Slot for FIXMAP(0) */
> +        lsr   x1, x1, #(SECOND_SHIFT - 3)   /* x1 := Slot for FIXMAP(0) */
>          str   x2, [x4, x1]           /* Map it in the fixmap's slot */
>  
>          /* Use a virtual address to access the UART. */
> @@ -364,12 +364,12 @@ paging:
>          /* Map the DTB in the boot misc slot */
>          cbnz  x22, 1f                /* Only on boot CPU */
>  
> -        lsr   x2, x21, #21
> -        lsl   x2, x2, #21            /* x2 := 2MB-aligned paddr of DTB */
> +        lsr   x2, x21, #SECOND_SHIFT
> +        lsl   x2, x2, #SECOND_SHIFT  /* x2 := 2MB-aligned paddr of DTB */
>          mov   x3, #PT_MEM            /* x2 := 2MB RAM incl. DTB */
>          orr   x2, x2, x3
>          ldr   x1, =BOOT_FDT_VIRT_START
> -        lsr   x1, x1, #18            /* x4 := Slot for BOOT_FDT_VIRT_START */
> +        lsr   x1, x1, #(SECOND_SHIFT - 3)   /* x4 := Slot for 
> BOOT_FDT_VIRT_START */
>          str   x2, [x4, x1]           /* Map it in the early fdt slot */
>          dsb   sy
>  1:



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.