|
[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 01/15/2014 01:32 PM, Ian Campbell wrote:
> 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?
This sounds a good fix for Xen 4.4.
Acked-by: Julien Grall <julien.grall@xxxxxxxxxx>
>> ---
>> 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
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |