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

Re: [Xen-devel] [PATCH v2 2/4] xen: arm: Handle 4K aligned hypervisor load address.



Hi Ian,

On 18/07/14 17:12, Ian Campbell wrote:
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 51501dc..0a76c2e 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -26,6 +26,7 @@

  #define PT_PT     0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */
  #define PT_MEM    0xf7d /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=0 P=1 */
+#define PT_MEM_L3 0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */
  #define PT_DEV    0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */
  #define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */

@@ -258,11 +259,11 @@ cpu_init_done:
          /* Setup boot_pgtable: */
          ldr   r1, =boot_second
          add   r1, r1, r10            /* r1 := paddr (boot_second) */
-        mov   r3, #0x0

          /* ... map boot_second in boot_pgtable[0] */
          orr   r2, r1, #PT_UPPER(PT)  /* r2:r3 := table map of boot_second */
          orr   r2, r2, #PT_LOWER(PT)  /* (+ rights for linear PT) */
+        mov   r3, #0x0
          strd  r2, r3, [r4, #0]       /* Map it in slot 0 */

          /* ... map of paddr(start) in boot_pgtable */
@@ -279,31 +280,61 @@ cpu_init_done:
          ldr   r4, =boot_second
          add   r4, r4, r10            /* r4 := paddr (boot_second) */

-        lsr   r2, r9, #SECOND_SHIFT  /* Base address for 2MB mapping */
-        lsl   r2, r2, #SECOND_SHIFT
+        ldr   r1, =boot_third
+        add   r1, r1, r10            /* r1 := paddr (boot_third) */
+
+        /* ... map boot_third in boot_second[1] */
+        orr   r2, r1, #PT_UPPER(PT)  /* r2:r3 := table map of boot_third */
+        orr   r2, r2, #PT_LOWER(PT)  /* (+ rights for linear PT) */
+        mov   r3, #0x0
+        strd  r2, r3, [r4, #8]       /* Map it in slot 1 */
+
+        /* ... map of paddr(start) in boot_second */
+        lsr   r2, r9, #SECOND_SHIFT  /* Offset of base paddr in boot_second */
+        mov   r3, #0x0ff             /* r2 := LPAE entries mask */

The register in the comment looks wrong to me.


+        orr   r3, r3, #0x100

I took me several minutes to understand why the orr... I think the 2 previous assembly lines could be replaced by a single one:
           ldr r3, =LPAE_ENTRY_MASK

[..]

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index d46481b..5d7b2b5 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -27,6 +27,7 @@

  #define PT_PT     0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */
  #define PT_MEM    0xf7d /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=0 P=1 */
+#define PT_MEM_L3 0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */
  #define PT_DEV    0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */
  #define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */

@@ -274,8 +275,9 @@ skip_bss:
          lsl   x2, x1, #ZEROETH_SHIFT /* Base address for 512GB mapping */
          mov   x3, #PT_MEM            /* x2 := Section mapping */
          orr   x2, x2, x3
-        lsl   x1, x1, #3             /* x1 := Slot offset */
-        str   x2, [x4, x1]           /* Mapping of paddr(start)*/
+        and   x1, x1, #0x1ff         /* x1 := Slot offset */

The #0x1ff could be replaced by #LPAE_ENTRY_MASK

+        lsl   x1, x1, #3
+        str   x2, [x4, x1]           /* Mapping of paddr(start) */

  1:      /* Setup boot_first: */
          ldr   x4, =boot_first        /* Next level into boot_first */
@@ -290,7 +292,7 @@ skip_bss:

          /* ... map of paddr(start) in boot_first */
          lsr   x2, x19, #FIRST_SHIFT  /* x2 := Offset of base paddr in 
boot_first */
-        and   x1, x2, 0x1ff          /* x1 := Slot to use */
+        and   x1, x2, #0x1ff         /* x1 := Slot to use */

Same here.

OOI, you only added the #. Was there any issue before?

          cbz   x1, 1f                 /* It's in slot 0, map in boot_second */

          lsl   x2, x2, #FIRST_SHIFT   /* Base address for 1GB mapping */
@@ -303,31 +305,55 @@ skip_bss:
          ldr   x4, =boot_second       /* Next level into boot_second */
          add   x4, x4, x20            /* x4 := paddr(boot_second) */

-        lsr   x2, x19, #SECOND_SHIFT /* Base address for 2MB mapping */
-        lsl   x2, x2, #SECOND_SHIFT
+        /* ... map boot_third in boot_second[1] */
+        ldr   x1, =boot_third
+        add   x1, x1, x20            /* x1 := paddr(boot_third) */
+        mov   x3, #PT_PT             /* x2 := table map of boot_third */
+        orr   x2, x1, x3             /*       + rights for linear PT */
+        str   x2, [x4, #8]           /* Map it in slot 1 */
+
+        /* ... map of paddr(start) in boot_second */
+        lsr   x2, x19, #SECOND_SHIFT /* x2 := Offset of base paddr in 
boot_second */
+        and   x1, x2, 0x1ff          /* x1 := Slot to use */

A bit strange that few lines above you changed a similar lines to add #, and here you forgot it.

I would also replace the 0x1ff by LPAE_ENTRY_MASK.

Regards,

--
Julien Grall

_______________________________________________
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®.