Skip to contentSkip to conversation

ARM, arm64: kvm: get rid of the bounce page

PATCH New Ard Biesheuvel 3 participants 5 messages Mar 27, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM, arm64: kvm: get rid of the bounce page

The HYP init bounce page is a runtime construct that ensures that the HYP init code does not cross a page boundary. However, this is something we can do perfectly well at build time, by aligning the code appropriately.

For both ARM and arm64, we just align to 4 KB, and enforce that the code size is less than 4 KB, regardless of the chosen page size.

Note that this also fixes a benign off-by-one error in the original bounce page code, where a bounce page would be allocated unnecessarily if the code was exactly 1 page in size.

---
 arch/arm/kernel/vmlinux.lds.S   | 11 +++++++----
 arch/arm/kvm/mmu.c              | 42 +++++------------------------------------
 arch/arm64/kernel/vmlinux.lds.S | 17 +++++++++++------
 3 files changed, 23 insertions(+), 47 deletions(-)
arch/arm/kernel/vmlinux.lds.S +6 -3
index b31aa73e8076..f2db429ea75d 100644
@@ -23,7 +23,7 @@
 	VMLINUX_SYMBOL(__idmap_text_start) = .;				\
 	*(.idmap.text)							\
 	VMLINUX_SYMBOL(__idmap_text_end) = .;				\
-	. = ALIGN(32);							\
+	. = ALIGN(PAGE_SIZE);						\
 	VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;			\
 	*(.hyp.idmap.text)						\
 	VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
@@ -346,8 +346,11 @@ SECTIONS
  */
 ASSERT((__proc_info_end - __proc_info_begin), "missing CPU support")
 ASSERT((__arch_info_end - __arch_info_begin), "no machine record defined")
+
 /*
- * The HYP init code can't be more than a page long.
+ * The HYP init code can't be more than a page long,
+ * and should not cross a page boundary.
  * The above comment applies as well.
  */
-ASSERT(((__hyp_idmap_text_end - __hyp_idmap_text_start) <= PAGE_SIZE), "HYP init code too big")
+ASSERT(__hyp_idmap_text_end - (__hyp_idmap_text_start & PAGE_MASK) <= PAGE_SIZE,
+	"HYP init code too big or misaligned")
arch/arm/kvm/mmu.c +5 -37

60 lines not on this page · raw message

arch/arm64/kernel/vmlinux.lds.S +11 -6
index 5d9d2dca530d..a2c29865c3fe 100644
@@ -23,10 +23,14 @@ jiffies = jiffies_64;
 
 #define HYPERVISOR_TEXT					\
 	/*						\
-	 * Force the alignment to be compatible with	\
-	 * the vectors requirements			\
+	 * Align to 4 KB so that			\
+	 * a) the HYP vector table is@its minimum	\
+	 *    alignment of 2048 bytes			\
+	 * b) the HYP init code will not cross a page	\
+	 *    boundary if its size does not exceed	\
+	 *    4 KB (see related ASSERT() below)		\
 	 */						\
-	. = ALIGN(2048);				\
+	. = ALIGN(SZ_4K);				\
 	VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;	\
 	*(.hyp.idmap.text)				\
 	VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;	\
@@ -163,10 +167,11 @@ SECTIONS
 }
 
 /*
- * The HYP init code can't be more than a page long.
+ * The HYP init code can't be more than a page long,
+ * and should not cross a page boundary.
  */
-ASSERT(((__hyp_idmap_text_start + PAGE_SIZE) > __hyp_idmap_text_end),
-       "HYP init code too big")
+ASSERT(__hyp_idmap_text_end - (__hyp_idmap_text_start & ~(SZ_4K - 1)) <= SZ_4K,
+	"HYP init code too big or misaligned")
 
 /*
  * If padding is applied before .head.text, virt<->phys conversions will fail.
Signature · 1 line
1.8.3.2
#1 lore raw

PATCH 1/1 ARM: kvm: round HYP section to page size instead of log2 upper bound

Older binutils do not support expressions involving the values of external symbols so just round up the HYP region to the page size.

---
 arch/arm/kernel/vmlinux.lds.S | 31 +------------------------------
 arch/arm/kvm/init.S           |  3 ---
 2 files changed, 1 insertion(+), 33 deletions(-)
arch/arm/kernel/vmlinux.lds.S +1 -30
index 808398ec024e..f2db429ea75d 100644
@@ -12,26 +12,6 @@
 #include <asm/pgtable.h>
 #endif
 
-/*
- * Poor man's version of LOG2CEIL(), which is
- * not available in binutils before v2.24.
- */
-#define LOG2_ROUNDUP(size) (		\
-	__LOG2_ROUNDUP(size,  2)	\
-	__LOG2_ROUNDUP(size,  3)	\
-	__LOG2_ROUNDUP(size,  4)	\
-	__LOG2_ROUNDUP(size,  5)	\
-	__LOG2_ROUNDUP(size,  6)	\
-	__LOG2_ROUNDUP(size,  7)	\
-	__LOG2_ROUNDUP(size,  8)	\
-	__LOG2_ROUNDUP(size,  9)	\
-	__LOG2_ROUNDUP(size, 10)	\
-	__LOG2_ROUNDUP(size, 11)	\
-	12)
-
-#define __LOG2_ROUNDUP(size, order)	\
-	(size) <= (1 << order) ? order :
-
 #define PROC_INFO							\
 	. = ALIGN(4);							\
 	VMLINUX_SYMBOL(__proc_info_begin) = .;				\
@@ -43,20 +23,11 @@
 	VMLINUX_SYMBOL(__idmap_text_start) = .;				\
 	*(.idmap.text)							\
 	VMLINUX_SYMBOL(__idmap_text_end) = .;				\
-	. = ALIGN(1 << LOG2_ROUNDUP(__hyp_idmap_size));			\
+	. = ALIGN(PAGE_SIZE);						\
 	VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;			\
 	*(.hyp.idmap.text)						\
 	VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
 
-/*
- * If the HYP idmap .text section is populated, it needs to be positioned
- * such that it will not cross a page boundary in the final output image.
- * So align it to the section size rounded up to the next power of 2.
- * If __hyp_idmap_size is undefined, the section will be empty so define
- * it as 0 in that case.
- */
-PROVIDE(__hyp_idmap_size = 0);
-
 #ifdef CONFIG_HOTPLUG_CPU
 #define ARM_CPU_DISCARD(x)
 #define ARM_CPU_KEEP(x)		x
arch/arm/kvm/init.S +0 -3
index 11fb1d56f449..3988e72d16ff 100644
@@ -157,6 +157,3 @@ target:	@ We're now in the trampoline code, switch page tables
 __kvm_hyp_init_end:
 
 	.popsection
-
-	.global	__hyp_idmap_size
-	.set	__hyp_idmap_size, __kvm_hyp_init_end - __kvm_hyp_init
Signature · 1 line
1.8.3.2
#2 lore raw

PATCH 1/1 ARM: kvm: round HYP section to page size instead of log2 upper bound

Quoted 4 lines · Ard Biesheuvel

Older binutils do not support expressions involving the values of external symbols so just round up the HYP region to the page size.

Thanks Ard. I managed to reproduce the failures in -next, and this solves the problem for me (the only toolchain that chokes seems to be the one on kernel.org, unfortunately).

I'll push an updated branch after a bit more testing. Let's not touch this file ever again :)

Will

Quoted 72 lines
---
 arch/arm/kernel/vmlinux.lds.S | 31 +------------------------------
 arch/arm/kvm/init.S           |  3 ---
 2 files changed, 1 insertion(+), 33 deletions(-)
arch/arm/kernel/vmlinux.lds.S +1 -30
index 808398ec024e..f2db429ea75d 100644
@@ -12,26 +12,6 @@
 #include <asm/pgtable.h>
 #endif
 
-/*
- * Poor man's version of LOG2CEIL(), which is
- * not available in binutils before v2.24.
- */
-#define LOG2_ROUNDUP(size) (		\
-	__LOG2_ROUNDUP(size,  2)	\
-	__LOG2_ROUNDUP(size,  3)	\
-	__LOG2_ROUNDUP(size,  4)	\
-	__LOG2_ROUNDUP(size,  5)	\
-	__LOG2_ROUNDUP(size,  6)	\
-	__LOG2_ROUNDUP(size,  7)	\
-	__LOG2_ROUNDUP(size,  8)	\
-	__LOG2_ROUNDUP(size,  9)	\
-	__LOG2_ROUNDUP(size, 10)	\
-	__LOG2_ROUNDUP(size, 11)	\
-	12)
-
-#define __LOG2_ROUNDUP(size, order)	\
-	(size) <= (1 << order) ? order :
-
 #define PROC_INFO							\
 	. = ALIGN(4);							\
 	VMLINUX_SYMBOL(__proc_info_begin) = .;				\
@@ -43,20 +23,11 @@
 	VMLINUX_SYMBOL(__idmap_text_start) = .;				\
 	*(.idmap.text)							\
 	VMLINUX_SYMBOL(__idmap_text_end) = .;				\
-	. = ALIGN(1 << LOG2_ROUNDUP(__hyp_idmap_size));			\
+	. = ALIGN(PAGE_SIZE);						\
 	VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;			\
 	*(.hyp.idmap.text)						\
 	VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
 
-/*
- * If the HYP idmap .text section is populated, it needs to be positioned
- * such that it will not cross a page boundary in the final output image.
- * So align it to the section size rounded up to the next power of 2.
- * If __hyp_idmap_size is undefined, the section will be empty so define
- * it as 0 in that case.
- */
-PROVIDE(__hyp_idmap_size = 0);
-
 #ifdef CONFIG_HOTPLUG_CPU
 #define ARM_CPU_DISCARD(x)
 #define ARM_CPU_KEEP(x)		x
arch/arm/kvm/init.S +0 -3
index 11fb1d56f449..3988e72d16ff 100644
@@ -157,6 +157,3 @@ target:	@ We're now in the trampoline code, switch page tables
 __kvm_hyp_init_end:
 
 	.popsection
-
-	.global	__hyp_idmap_size
-	.set	__hyp_idmap_size, __kvm_hyp_init_end - __kvm_hyp_init
Signature · 1 line
1.8.3.2
#3 lore raw

PATCH 1/1 ARM: kvm: round HYP section to page size instead of log2 upper bound

Quoted 4 lines · Will Deacon
Quoted 3 lines · Ard Biesheuvel

Older binutils do not support expressions involving the values of external symbols so just round up the HYP region to the page size.

Quoted 9 lines
Quoted 1 line

Thanks Ard. I managed to reproduce the failures in -next, and this solves the problem for me (the only toolchain that chokes seems to be the one on kernel.org, unfortunately).

I'll push an updated branch after a bit more testing. Let's not touch this file ever again :)

Fingers crossed ...

Quoted 72 lines
Quoted 72 lines
---
 arch/arm/kernel/vmlinux.lds.S | 31 +------------------------------
 arch/arm/kvm/init.S           |  3 ---
 2 files changed, 1 insertion(+), 33 deletions(-)
arch/arm/kernel/vmlinux.lds.S +1 -30
index 808398ec024e..f2db429ea75d 100644
@@ -12,26 +12,6 @@
 #include <asm/pgtable.h>
 #endif
 
-/*
- * Poor man's version of LOG2CEIL(), which is
- * not available in binutils before v2.24.
- */
-#define LOG2_ROUNDUP(size) (         \
-     __LOG2_ROUNDUP(size,  2)        \
-     __LOG2_ROUNDUP(size,  3)        \
-     __LOG2_ROUNDUP(size,  4)        \
-     __LOG2_ROUNDUP(size,  5)        \
-     __LOG2_ROUNDUP(size,  6)        \
-     __LOG2_ROUNDUP(size,  7)        \
-     __LOG2_ROUNDUP(size,  8)        \
-     __LOG2_ROUNDUP(size,  9)        \
-     __LOG2_ROUNDUP(size, 10)        \
-     __LOG2_ROUNDUP(size, 11)        \
-     12)
-
-#define __LOG2_ROUNDUP(size, order)  \
-     (size) <= (1 << order) ? order :
-
 #define PROC_INFO                                                    \
      . = ALIGN(4);                                                   \
      VMLINUX_SYMBOL(__proc_info_begin) = .;                          \
@@ -43,20 +23,11 @@
      VMLINUX_SYMBOL(__idmap_text_start) = .;                         \
      *(.idmap.text)                                                  \
      VMLINUX_SYMBOL(__idmap_text_end) = .;                           \
-     . = ALIGN(1 << LOG2_ROUNDUP(__hyp_idmap_size));                 \
+     . = ALIGN(PAGE_SIZE);                                           \
      VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;                     \
      *(.hyp.idmap.text)                                              \
      VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
 
-/*
- * If the HYP idmap .text section is populated, it needs to be positioned
- * such that it will not cross a page boundary in the final output image.
- * So align it to the section size rounded up to the next power of 2.
- * If __hyp_idmap_size is undefined, the section will be empty so define
- * it as 0 in that case.
- */
-PROVIDE(__hyp_idmap_size = 0);
-
 #ifdef CONFIG_HOTPLUG_CPU
 #define ARM_CPU_DISCARD(x)
 #define ARM_CPU_KEEP(x)              x
arch/arm/kvm/init.S +0 -4
index 11fb1d56f449..3988e72d16ff 100644
@@ -157,6 +157,3 @@ target:   @ We're now in the trampoline code, switch page tables
 __kvm_hyp_init_end:
 
      .popsection
-
-     .global __hyp_idmap_size
-     .set    __hyp_idmap_size, __kvm_hyp_init_end - __kvm_hyp_init
--

1.8.3.2

#4 lore raw

PATCH 1/1 ARM: kvm: round HYP section to page size instead of log2 upper bound

Hi Ard, Hi Will,

Quoted 10 lines · Ard Biesheuvel
Quoted 9 lines · Will Deacon
Quoted 4 lines · Ard Biesheuvel

Older binutils do not support expressions involving the values of external symbols so just round up the HYP region to the page size.

Thanks Ard. I managed to reproduce the failures in -next, and this solves the problem for me (the only toolchain that chokes seems to be the one on kernel.org, unfortunately).

For the record that is the one I am using.

Quoted 2 lines
Quoted 2 lines

I'll push an updated branch after a bit more testing. Let's not touch this file ever again :)

:)

Quoted 1 line

Fingers crossed ...

I have tested this patch applied on top of next-20150326 and the build problem I observed is gone. I have also tested that the resulting kernel boots on a range of Renesas boards.