diff mbox

[Xen-devel] xen: arm: Write to the correct PT when mapping the DTB on boot on arm64

Message ID ae6d0334bc37442af171d8b7196995894d01f5e3.1406280726.git.ian.campbell@citrix.com
State Accepted
Commit 159e504e77bdf683779b898b881584f34aedb46a
Headers show

Commit Message

Ian Campbell July 25, 2014, 9:32 a.m. UTC
We currently get away with this because when debug=y and earlyprintk is enabled
the previous block of (conditinal) code would have set this up. Historically we
mostly got away with it even without those options because the pre paging code
would normally (at least on h/w we test) leave x4 set to the paddr of
boot_second.

This latent bug has always been present but was exposed by ca59618967fe "xen:
arm: Handle 4K aligned hypervisor load address" (or one of the related patches)
since now x4 is quite likely to point to boot_third not boot_second.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/arm64/head.S |    1 +
 1 file changed, 1 insertion(+)

Comments

Julien Grall July 25, 2014, 12:12 p.m. UTC | #1
On 07/25/2014 10:32 AM, Ian Campbell wrote:
> We currently get away with this because when debug=y and earlyprintk is enabled
> the previous block of (conditinal) code would have set this up. Historically we

NIT: conditional

> mostly got away with it even without those options because the pre paging code
> would normally (at least on h/w we test) leave x4 set to the paddr of
> boot_second.
> 
> This latent bug has always been present but was exposed by ca59618967fe "xen:
> arm: Handle 4K aligned hypervisor load address" (or one of the related patches)
> since now x4 is quite likely to point to boot_third not boot_second.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Acked-by: Julien Grall <julien.grall@linaro.org>

Regards,

> ---
>  xen/arch/arm/arm64/head.S |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index dcb7071..43b5e72 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -428,6 +428,7 @@ paging:
>          /* Map the DTB in the boot misc slot */
>          cbnz  x22, 1f                /* Only on boot CPU */
>  
> +        ldr   x4, =boot_second       /* x4 := vaddr (boot_second) */
>          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 */
>
Ian Campbell Aug. 4, 2014, 2:01 p.m. UTC | #2
On Fri, 2014-07-25 at 13:12 +0100, Julien Grall wrote:
> On 07/25/2014 10:32 AM, Ian Campbell wrote:
> > We currently get away with this because when debug=y and earlyprintk is enabled
> > the previous block of (conditinal) code would have set this up. Historically we
> 
> NIT: conditional

Fixed.

> > mostly got away with it even without those options because the pre paging code
> > would normally (at least on h/w we test) leave x4 set to the paddr of
> > boot_second.
> > 
> > This latent bug has always been present but was exposed by ca59618967fe "xen:
> > arm: Handle 4K aligned hypervisor load address" (or one of the related patches)
> > since now x4 is quite likely to point to boot_third not boot_second.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Acked-by: Julien Grall <julien.grall@linaro.org>

Applied, thanks.
diff mbox

Patch

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index dcb7071..43b5e72 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -428,6 +428,7 @@  paging:
         /* Map the DTB in the boot misc slot */
         cbnz  x22, 1f                /* Only on boot CPU */
 
+        ldr   x4, =boot_second       /* x4 := vaddr (boot_second) */
         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 */