arm64: implement support for KASLR

This introduces the preprocessor symbol KIMAGE_VADDR which will serve as the symbolic virtual base of the kernel region, i.e., the kernel's virtual offset will be KIMAGE_VADDR + TEXT_OFFSET. For now, we define it as being equal to PAGE_OFFSET, but in the future, it will be moved below it once we move the kernel virtual mapping out of the linear mapping.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/memory.h | 10 ++++++++--
 arch/arm64/kernel/head.S        |  2 +-
 arch/arm64/kernel/vmlinux.lds.S |  4 ++--
 3 files changed, 11 insertions(+), 5 deletions(-)

arch/arm64/include/asm/memory.h +2 -1

index 853953cd1f08..bea9631b34a8 100644
@@ -51,7 +51,8 @@
 #define VA_BITS			(CONFIG_ARM64_VA_BITS)
 #define VA_START		(UL(0xffffffffffffffff) << VA_BITS)
 #define PAGE_OFFSET		(UL(0xffffffffffffffff) << (VA_BITS - 1))
-#define MODULES_END		(PAGE_OFFSET)
+#define KIMAGE_VADDR		(PAGE_OFFSET)
+#define MODULES_END		(KIMAGE_VADDR)
 #define MODULES_VADDR		(MODULES_END - SZ_64M)
 #define PCI_IO_END		(MODULES_VADDR - SZ_2M)
 #define PCI_IO_START		(PCI_IO_END - PCI_IO_SIZE)

arch/arm64/include/asm/memory.h +2 -0

+
 #define __phys_to_virt(x)	((unsigned long)((x) - PHYS_OFFSET + PAGE_OFFSET))
+#define __phys_to_kimg(x)	((unsigned long)((x) - PHYS_OFFSET + KIMAGE_VADDR))
 
 /*
  * Convert a page to/from a physical address

arch/arm64/kernel/head.S +1 -1

index 23cfc08fc8ba..6434c844a0e4 100644
@@ -389,7 +389,7 @@ __create_page_tables:
 	 * Map the kernel image (starting with PHYS_OFFSET).
 	 */
 	mov	x0, x26				// swapper_pg_dir
-	mov	x5, #PAGE_OFFSET
+	ldr	x5, =KIMAGE_VADDR
 	create_pgd_entry x0, x5, x3, x6
 	ldr	x6, =KERNEL_END			// __va(KERNEL_END)
 	mov	x3, x24				// phys offset

arch/arm64/kernel/vmlinux.lds.S +2 -2

index 7de6c39858a5..ced0dedcabcc 100644
@@ -88,7 +88,7 @@ SECTIONS
 		*(.discard.*)
 	}
 
-	. = PAGE_OFFSET + TEXT_OFFSET;
+	. = KIMAGE_VADDR + TEXT_OFFSET;
 
 	.head.text : {
 		_text = .;

@@ -185,4 +185,4 @@ ASSERT(__idmap_text_end - (__idmap_text_start & ~(SZ_4K - 1)) <= SZ_4K,
 /*
  * If padding is applied before .head.text, virt<->phys conversions will fail.
  */
-ASSERT(_text == (PAGE_OFFSET + TEXT_OFFSET), "HEAD is misaligned")
+ASSERT(_text == (KIMAGE_VADDR + TEXT_OFFSET), "HEAD is misaligned")

Signature · 1 line

2.5.0

Quoted 31 lines · Ard Biesheuvel

This introduces the preprocessor symbol KIMAGE_VADDR which will serve as the symbolic virtual base of the kernel region, i.e., the kernel's virtual offset will be KIMAGE_VADDR + TEXT_OFFSET. For now, we define it as being equal to PAGE_OFFSET, but in the future, it will be moved below it once we move the kernel virtual mapping out of the linear mapping.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/memory.h | 10 ++++++++--
 arch/arm64/kernel/head.S        |  2 +-
 arch/arm64/kernel/vmlinux.lds.S |  4 ++--
 3 files changed, 11 insertions(+), 5 deletions(-)

arch/arm64/include/asm/memory.h +2 -1

index 853953cd1f08..bea9631b34a8 100644
@@ -51,7 +51,8 @@
 #define VA_BITS			(CONFIG_ARM64_VA_BITS)
 #define VA_START		(UL(0xffffffffffffffff) << VA_BITS)
 #define PAGE_OFFSET		(UL(0xffffffffffffffff) << (VA_BITS - 1))
-#define MODULES_END		(PAGE_OFFSET)
+#define KIMAGE_VADDR		(PAGE_OFFSET)
+#define MODULES_END		(KIMAGE_VADDR)
 #define MODULES_VADDR		(MODULES_END - SZ_64M)
 #define PCI_IO_END		(MODULES_VADDR - SZ_2M)
 #define PCI_IO_START		(PCI_IO_END - PCI_IO_SIZE)

@@ -75,8 +76,13 @@
  * private definitions which should NOT be used outside memory.h
  * files.  Use virt_to_phys/phys_to_virt/__pa/__va instead.
  */

arch/arm64/include/asm/memory.h line 82

Quoted 5 lines

arch/arm64/include/asm/memory.h +4 -1

-#define __virt_to_phys(x)	(((phys_addr_t)(x) - PAGE_OFFSET + PHYS_OFFSET))
+#define __virt_to_phys(x) ({						\
+	phys_addr_t __x = (phys_addr_t)(x);				\
+	__x >= PAGE_OFFSET ? (__x - PAGE_OFFSET + PHYS_OFFSET) :	\
+			     (__x - KIMAGE_VADDR + PHYS_OFFSET); })

so __virt_to_phys will now work with a subset of the non-linear namely all except vmalloced and ioremapped ones?

Quoted 32 lines · Christoffer Dall

Quoted 31 lines · Ard Biesheuvel

This introduces the preprocessor symbol KIMAGE_VADDR which will serve as the symbolic virtual base of the kernel region, i.e., the kernel's virtual offset will be KIMAGE_VADDR + TEXT_OFFSET. For now, we define it as being equal to PAGE_OFFSET, but in the future, it will be moved below it once we move the kernel virtual mapping out of the linear mapping.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/memory.h | 10 ++++++++--
 arch/arm64/kernel/head.S        |  2 +-
 arch/arm64/kernel/vmlinux.lds.S |  4 ++--
 3 files changed, 11 insertions(+), 5 deletions(-)

arch/arm64/include/asm/memory.h +2 -1

index 853953cd1f08..bea9631b34a8 100644
@@ -51,7 +51,8 @@
 #define VA_BITS			(CONFIG_ARM64_VA_BITS)
 #define VA_START		(UL(0xffffffffffffffff) << VA_BITS)
 #define PAGE_OFFSET		(UL(0xffffffffffffffff) << (VA_BITS - 1))
-#define MODULES_END		(PAGE_OFFSET)
+#define KIMAGE_VADDR		(PAGE_OFFSET)
+#define MODULES_END		(KIMAGE_VADDR)
 #define MODULES_VADDR		(MODULES_END - SZ_64M)
 #define PCI_IO_END		(MODULES_VADDR - SZ_2M)
 #define PCI_IO_START		(PCI_IO_END - PCI_IO_SIZE)

@@ -75,8 +76,13 @@
  * private definitions which should NOT be used outside memory.h
  * files.  Use virt_to_phys/phys_to_virt/__pa/__va instead.
  */

Quoted 8 lines

arch/arm64/include/asm/memory.h line 82

Quoted 5 lines

arch/arm64/include/asm/memory.h +4 -1

-#define __virt_to_phys(x)	(((phys_addr_t)(x) - PAGE_OFFSET + PHYS_OFFSET))
+#define __virt_to_phys(x) ({						\
+	phys_addr_t __x = (phys_addr_t)(x);				\
+	__x >= PAGE_OFFSET ? (__x - PAGE_OFFSET + PHYS_OFFSET) :	\
+			     (__x - KIMAGE_VADDR + PHYS_OFFSET); })

so __virt_to_phys will now work with a subset of the non-linear namely all except vmalloced and ioremapped ones?

It will work for linear mapped memory and for the kernel image, which is what it used to do. It's just that the relationship between the image and the linear map is broken.

The same rules apply to x86, where their virt_to_phys eventually boils down to:

static inline unsigned long __phys_addr_nodebug(unsigned long x)

{
        unsigned long y = x - __START_KERNEL_map;

        /* use the carry flag to determine if x was < __START_KERNEL_map */
        x = y + ((x > y) ? phys_base : (__START_KERNEL_map - PAGE_OFFSET));

        return x;
}

Before introducing new statically allocated page tables and increasing their alignment in subsequent patches, update the reservation logic so that only pages that are in actual use end up as reserved with memblock.

From the looks of the next patch we won't have redundant levels of

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/mm/init.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/arm64/mm/init.c +4 -1

index 6bacba847923..8e678d05ad84 100644
@@ -36,6 +36,7 @@
 #include <linux/swiotlb.h>
 
 #include <asm/fixmap.h>
+#include <asm/kernel-pgtable.h>
 #include <asm/memory.h>
 #include <asm/sections.h>
 #include <asm/setup.h>

@@ -165,11 +166,13 @@ void __init arm64_memblock_init(void)
 	 * Register the kernel text, kernel data, initrd, and initial
 	 * pagetables with memblock.
 	 */
-	memblock_reserve(__pa(_text), _end - _text);
+	memblock_reserve(__pa(_text), __bss_stop - _text);
 #ifdef CONFIG_BLK_DEV_INITRD
 	if (initrd_start)
 		memblock_reserve(__virt_to_phys(initrd_start), initrd_end - initrd_start);
 #endif
+	memblock_reserve(__pa(idmap_pg_dir), IDMAP_DIR_SIZE);
+	memblock_reserve(__pa(swapper_pg_dir), SWAPPER_DIR_SIZE);
 
 	early_init_fdt_scan_reserved_mem();
 

Signature · 1 line

2.5.0

Quoted 4 lines · Ard Biesheuvel

Before introducing new statically allocated page tables and increasing their alignment in subsequent patches, update the reservation logic so that only pages that are in actual use end up as reserved with memblock.

Could you add something to the commit message about what this will gain us (i.e. which pages we don't have to reserve)? It's not immediately obvious why we'd have page tables we wouldn't want to reserve.

From the looks of the next patch we won't have redundant levels of fixmap table for a given configuration, so I guess we're catering for the case the fixmap shares a pgd/pud/pmd entry with the image mapping?

Does that happen? If so that would invalidate the assumption I make when copying the fixmap over in [1] (see map_kernel).

To handle that either we need some special logic to copy over the relevant bits for the fixmap (as with kasan_copy_shadow), or we need to avoid sharing a pgd entry.

Thoughts?

Quoted 10 lines · Mark Rutland

Quoted 4 lines · Ard Biesheuvel

Before introducing new statically allocated page tables and increasing their alignment in subsequent patches, update the reservation logic so that only pages that are in actual use end up as reserved with memblock.

Could you add something to the commit message about what this will gain us (i.e. which pages we don't have to reserve)? It's not immediately obvious why we'd have page tables we wouldn't want to reserve.

OK. In the original series, I also aligned the pgdir section to a log2 upper bound of its size, but that is not necessary anymore with your changes. So the original goal was to avoid reserving the alignment padding as well as the pgdirs that end up unused

Quoted 4 lines

From the looks of the next patch we won't have redundant levels of fixmap table for a given configuration, so I guess we're catering for the case the fixmap shares a pgd/pud/pmd entry with the image mapping?

Quoted 3 lines

Does that happen? If so that would invalidate the assumption I make when copying the fixmap over in [1] (see map_kernel).

It is a lot less likely to happen now that I moved the kernel to the start of the vmalloc area rather than right below PAGE_OFFSET. But in general, it seems sensible to only populate entries after confirming that they are in fact vacant.

Quoted 6 lines

To handle that either we need some special logic to copy over the relevant bits for the fixmap (as with kasan_copy_shadow), or we need to avoid sharing a pgd entry.

Thoughts?

Yes, I have added that to my v3 version of the vmalloc base move patch here

https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/commitdiff/0beef2c1a6bfc90cc116a6ba1b24f2ba35e7e5f6

but I think 16k/4 levels is the only config affected when the kernel is always in the lower half of the vmalloc area. That also implies that the fixmap pgd is either always shared, or never, depending on the build time config, so I could probably simplify that part somewhat.

Since the early fixmap page tables are populated using pages that are part of the static footprint of the kernel, they are covered by the initial kernel mapping, and we can refer to them without using __va/__pa translations, which are tied to the linear mapping.

Instead, let's introduce __phys_to_kimg, which will be tied to the kernel virtual mapping, regardless of whether or not it intersects with the linear mapping. This will allow us to move the kernel out of the linear mapping in a subsequent patch.

Hi Ard!

Quoted 10 lines · Ard Biesheuvel

Since the early fixmap page tables are populated using pages that are part of the static footprint of the kernel, they are covered by the initial kernel mapping, and we can refer to them without using __va/__pa translations, which are tied to the linear mapping.

Instead, let's introduce __phys_to_kimg, which will be tied to the kernel virtual mapping, regardless of whether or not it intersects with the linear mapping. This will allow us to move the kernel out of the linear mapping in a subsequent patch.

I gave your arm64-kaslr-v2 branch a go on juno r1, currently with ARM64_RELOCATABLE_KERNEL=n, to find it didn't boot.

git bisect pointed to this patch. From the debugger it looks like rubbish is ending up the page tables after early_fixmap_init(), printing bits of bm_pmd and friends shows these aren't zeroed.

I think this is because the section(".pgdir") is dragging these outside the __bss_start/__bss_stop range that is zeroed in head.S:__mmap_switched().

The following inelegant patch fixes this problem for me:
----------------------------%<----------------------------

arch/arm64/mm/mmu.c +2 -0

index a78fc5a882da..15fc9712ddc1 100644
@@ -559,6 +559,7 @@ void __init early_fixmap_init(void)
        if (pgd_none(*pgd)) {
                static pud_t bm_pud[PTRS_PER_PUD] __pgdir;
 
+               memset(bm_pud, 0, sizeof(bm_pud));
                pgd_populate(&init_mm, pgd, bm_pud);
                memblock_reserve(__pa(bm_pud), sizeof(bm_pud));
        }

@@ -570,6 +571,7 @@ void __init early_fixmap_init(void)
        if (pud_none(*pud)) {
                static pmd_t bm_pmd[PTRS_PER_PMD] __pgdir;
 
+               memset(bm_pmd, 0, sizeof(bm_pmd));
                pud_populate(&init_mm, pud, bm_pmd);
                memblock_reserve(__pa(bm_pmd), sizeof(bm_pmd));
        }

arch/arm64/mm/mmu.c +19 -5

@@ -583,33 +555,42 @@ void __init early_fixmap_init(void)
 	unsigned long addr = FIXADDR_START;
 
 	pgd = pgd_offset_k(addr);
-	pgd_populate(&init_mm, pgd, bm_pud);
-	pud = pud_offset(pgd, addr);
-	pud_populate(&init_mm, pud, bm_pmd);
-	pmd = pmd_offset(pud, addr);
-	pmd_populate_kernel(&init_mm, pmd, bm_pte);
+#if CONFIG_PGTABLE_LEVELS > 3
+	if (pgd_none(*pgd)) {
+		static pud_t bm_pud[PTRS_PER_PUD] __pgdir;
+
+		pgd_populate(&init_mm, pgd, bm_pud);
+		memblock_reserve(__pa(bm_pud), sizeof(bm_pud));
+	}
+	pud = (pud_t *)__phys_to_kimg(pud_offset_phys(pgd, addr));
+#else
+	pud = (pud_t *)pgd;
+#endif
+#if CONFIG_PGTABLE_LEVELS > 2
+	if (pud_none(*pud)) {
+		static pmd_t bm_pmd[PTRS_PER_PMD] __pgdir;
+
+		pud_populate(&init_mm, pud, bm_pmd);
+		memblock_reserve(__pa(bm_pmd), sizeof(bm_pmd));
+	}
+	pmd = (pmd_t *)__phys_to_kimg(pmd_offset_phys(pud, addr));

Quoted 34 lines · Ard Biesheuvel

arch/arm64/mm/mmu.c +25 -5

@@ -583,33 +555,42 @@ void __init early_fixmap_init(void)
      unsigned long addr = FIXADDR_START;
 
      pgd = pgd_offset_k(addr);
-     pgd_populate(&init_mm, pgd, bm_pud);
-     pud = pud_offset(pgd, addr);
-     pud_populate(&init_mm, pud, bm_pmd);
-     pmd = pmd_offset(pud, addr);
-     pmd_populate_kernel(&init_mm, pmd, bm_pte);
+#if CONFIG_PGTABLE_LEVELS > 3
+     if (pgd_none(*pgd)) {
+             static pud_t bm_pud[PTRS_PER_PUD] __pgdir;
+
+             pgd_populate(&init_mm, pgd, bm_pud);
+             memblock_reserve(__pa(bm_pud), sizeof(bm_pud));
+     }
+     pud = (pud_t *)__phys_to_kimg(pud_offset_phys(pgd, addr));
+#else
+     pud = (pud_t *)pgd;
+#endif
+#if CONFIG_PGTABLE_LEVELS > 2
+     if (pud_none(*pud)) {
+             static pmd_t bm_pmd[PTRS_PER_PMD] __pgdir;
+
+             pud_populate(&init_mm, pud, bm_pmd);
+             memblock_reserve(__pa(bm_pmd), sizeof(bm_pmd));
+     }
+     pmd = (pmd_t *)__phys_to_kimg(pmd_offset_phys(pud, addr));
+#else
+     pmd = (pmd_t *)pud;
+#endif
+     if (pmd_none(*pmd)) {
+             static pte_t bm_pte[PTRS_PER_PTE] __pgdir;
+

arch/arm64/mm/mmu.c void __init early_fixmap_init(void) · line 586

Quoted 4 lines

arch/arm64/mm/mmu.c +4 -0

+             pmd_populate_kernel(&init_mm, pmd, bm_pte);
+             memblock_reserve(__pa(bm_pte), sizeof(bm_pte));
+     }
+     __fixmap_pte = (pte_t *)__phys_to_kimg(pmd_page_paddr(*pmd));

I haven't tried but could you not avoid the #if and just rely on the pud_none() etc. definitions to be 0 and the compiler+linker optimising the irrelevant code out?

KVM on arm64 uses a fixed offset between the linear mapping at EL1 and the HYP mapping at EL2. Before we can move the kernel virtual mapping out of the linear mapping, we have to make sure that references to kernel symbols that are accessed via the HYP mapping are translated to their linear equivalent.

To prevent inadvertent direct references from sneaking in later, change the type of all extern declarations to HYP kernel symbols to the opaque 'struct kvm_ksym', which does not decay to a pointer type like char arrays and function references. This is not bullet proof, but at least forces the user to take the address explicitly rather than referencing it directly.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm/include/asm/kvm_asm.h   |  2 ++
 arch/arm/include/asm/kvm_mmu.h   |  2 ++
 arch/arm/kvm/arm.c               |  9 +++++----
 arch/arm/kvm/mmu.c               | 12 +++++------
 arch/arm64/include/asm/kvm_asm.h | 21 +++++++++++---------
 arch/arm64/include/asm/kvm_mmu.h |  2 ++
 arch/arm64/include/asm/virt.h    |  4 ----
 arch/arm64/kernel/vmlinux.lds.S  |  4 ++--
 arch/arm64/kvm/debug.c           |  4 +++-
 virt/kvm/arm/vgic-v3.c           |  2 +-
 10 files changed, 34 insertions(+), 28 deletions(-)

arch/arm/include/asm/kvm_asm.h +2 -0

index 194c91b610ff..484ffdf7c70b 100644
@@ -99,6 +99,8 @@ extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
 
 extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+
+extern char __hyp_idmap_text_start[], __hyp_idmap_text_end[];
 #endif
 
 #endif /* __ARM_KVM_ASM_H__ */

arch/arm/include/asm/kvm_mmu.h +2 -0

index 405aa1883307..412b363f79e9 100644
@@ -30,6 +30,8 @@
 #define HYP_PAGE_OFFSET		PAGE_OFFSET
 #define KERN_TO_HYP(kva)	(kva)
 
+#define kvm_ksym_ref(kva)	(kva)
+
 /*
  * Our virtual mapping for the boot-time MMU-enable code. Must be
  * shared across all the page-tables. Conveniently, we use the vectors

arch/arm/kvm/arm.c +5 -4

index e06fd299de08..014b542ea658 100644
@@ -427,7 +427,7 @@ static void update_vttbr(struct kvm *kvm)
 		 * shareable domain to make sure all data structures are
 		 * clean.
 		 */
-		kvm_call_hyp(__kvm_flush_vm_context);
+		kvm_call_hyp(kvm_ksym_ref(__kvm_flush_vm_context));
 	}
 
 	kvm->arch.vmid_gen = atomic64_read(&kvm_vmid_gen);

@@ -600,7 +600,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 		__kvm_guest_enter();
 		vcpu->mode = IN_GUEST_MODE;
 
-		ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
+		ret = kvm_call_hyp(kvm_ksym_ref(__kvm_vcpu_run), vcpu);
 
 		vcpu->mode = OUTSIDE_GUEST_MODE;
 		/*

@@ -969,7 +969,7 @@ static void cpu_init_hyp_mode(void *dummy)
 	pgd_ptr = kvm_mmu_get_httbr();
 	stack_page = __this_cpu_read(kvm_arm_hyp_stack_page);
 	hyp_stack_ptr = stack_page + PAGE_SIZE;
-	vector_ptr = (unsigned long)__kvm_hyp_vector;
+	vector_ptr = (unsigned long)kvm_ksym_ref(__kvm_hyp_vector);
 
 	__cpu_init_hyp_mode(boot_pgd_ptr, pgd_ptr, hyp_stack_ptr, vector_ptr);
 

@@ -1061,7 +1061,8 @@ static int init_hyp_mode(void)
 	/*
 	 * Map the Hyp-code called directly from the host
 	 */
-	err = create_hyp_mappings(__kvm_hyp_code_start, __kvm_hyp_code_end);
+	err = create_hyp_mappings(kvm_ksym_ref(__kvm_hyp_code_start),
+				  kvm_ksym_ref(__kvm_hyp_code_end));
 	if (err) {
 		kvm_err("Cannot map world-switch code\n");
 		goto out_free_mappings;

arch/arm/kvm/mmu.c +0 -2

index 7dace909d5cf..7c448b943e3a 100644
@@ -31,8 +31,6 @@
 
 #include "trace.h"
 
-extern char  __hyp_idmap_text_start[], __hyp_idmap_text_end[];
-
 static pgd_t *boot_hyp_pgd;
 static pgd_t *hyp_pgd;
 static pgd_t *merged_hyp_pgd;

arch/arm/kvm/mmu.c +1 -1

 }
 
 static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)

@@ -75,7 +73,7 @@ static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 	 * anything there.
 	 */
 	if (kvm)
-		kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa);
+		kvm_call_hyp(kvm_ksym_ref(__kvm_tlb_flush_vmid_ipa), kvm, ipa);
 }
 
 /*

arch/arm64/include/asm/kvm_asm.h +11 -9

 extern char __kvm_hyp_init[];
 extern char __kvm_hyp_init_end[];
 
-extern char __kvm_hyp_vector[];
+extern struct kvm_ksym __kvm_hyp_vector;
 
-#define	__kvm_hyp_code_start	__hyp_text_start
-#define	__kvm_hyp_code_end	__hyp_text_end
+extern struct kvm_ksym __kvm_hyp_code_start;
+extern struct kvm_ksym __kvm_hyp_code_end;
 
-extern void __kvm_flush_vm_context(void);
-extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
-extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
+extern struct kvm_ksym __kvm_flush_vm_context;
+extern struct kvm_ksym __kvm_tlb_flush_vmid_ipa;
+extern struct kvm_ksym __kvm_tlb_flush_vmid;
 
-extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+extern struct kvm_ksym __kvm_vcpu_run;
 
-extern u64 __vgic_v3_get_ich_vtr_el2(void);
+extern struct kvm_ksym __hyp_idmap_text_start, __hyp_idmap_text_end;
 
-extern u32 __kvm_get_mdcr_el2(void);
+extern struct kvm_ksym __vgic_v3_get_ich_vtr_el2;
+
+extern struct kvm_ksym __kvm_get_mdcr_el2;
 
 #endif
 

arch/arm64/include/asm/kvm_mmu.h +2 -0

index 61505676d085..0899026a2821 100644
@@ -73,6 +73,8 @@
 
 #define KERN_TO_HYP(kva)	((unsigned long)kva - PAGE_OFFSET + HYP_PAGE_OFFSET)
 
+#define kvm_ksym_ref(sym)	((void *)&sym - KIMAGE_VADDR + PAGE_OFFSET)
+
 /*
  * We currently only support a 40bit IPA.
  */

arch/arm64/include/asm/virt.h +0 -4

index 7a5df5252dd7..215ad4649dd7 100644
@@ -50,10 +50,6 @@ static inline bool is_hyp_mode_mismatched(void)
 	return __boot_cpu_mode[0] != __boot_cpu_mode[1];
 }
 
-/* The section containing the hypervisor text */
-extern char __hyp_text_start[];
-extern char __hyp_text_end[];
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* ! __ASM__VIRT_H */

arch/arm64/kernel/vmlinux.lds.S +0 -0

index 363c2f529951..f935f082188d 100644

arch/arm64/kernel/vmlinux.lds.S +0 -0

 #define IDMAP_TEXT					\
 	. = ALIGN(SZ_4K);				\

arch/arm64/kvm/debug.c +3 -1

index 47e5f0feaee8..99e5a403af4e 100644
@@ -24,6 +24,7 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_arm.h>
 #include <asm/kvm_emulate.h>
+#include <asm/kvm_mmu.h>
 
 #include "trace.h"
 

@@ -72,7 +73,8 @@ static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
 
 void kvm_arm_init_debug(void)
 {
-	__this_cpu_write(mdcr_el2, kvm_call_hyp(__kvm_get_mdcr_el2));
+	__this_cpu_write(mdcr_el2,
+			 kvm_call_hyp(kvm_ksym_ref(__kvm_get_mdcr_el2)));
 }
 
 /**

virt/kvm/arm/vgic-v3.c +0 -0

index 487d6357b7e7..58f5a6521307 100644

Hi Ard,

Quoted 6 lines · Ard Biesheuvel

KVM on arm64 uses a fixed offset between the linear mapping at EL1 and the HYP mapping at EL2. Before we can move the kernel virtual mapping out of the linear mapping, we have to make sure that references to kernel symbols that are accessed via the HYP mapping are translated to their linear equivalent.

Quoted 7 lines

To prevent inadvertent direct references from sneaking in later, change the type of all extern declarations to HYP kernel symbols to the opaque 'struct kvm_ksym', which does not decay to a pointer type like char arrays and function references. This is not bullet proof, but at least forces the user to take the address explicitly rather than referencing it directly.

Signed-off-by Ard Biesheuvel <[email protected]>

This looks good to me, a few comments below.

Quoted 88 lines

---
 arch/arm/include/asm/kvm_asm.h   |  2 ++
 arch/arm/include/asm/kvm_mmu.h   |  2 ++
 arch/arm/kvm/arm.c               |  9 +++++----
 arch/arm/kvm/mmu.c               | 12 +++++------
 arch/arm64/include/asm/kvm_asm.h | 21 +++++++++++---------
 arch/arm64/include/asm/kvm_mmu.h |  2 ++
 arch/arm64/include/asm/virt.h    |  4 ----
 arch/arm64/kernel/vmlinux.lds.S  |  4 ++--
 arch/arm64/kvm/debug.c           |  4 +++-
 virt/kvm/arm/vgic-v3.c           |  2 +-
 10 files changed, 34 insertions(+), 28 deletions(-)

arch/arm/include/asm/kvm_asm.h +2 -0

index 194c91b610ff..484ffdf7c70b 100644
@@ -99,6 +99,8 @@ extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
 
 extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+
+extern char __hyp_idmap_text_start[], __hyp_idmap_text_end[];
 #endif
 
 #endif /* __ARM_KVM_ASM_H__ */

arch/arm/include/asm/kvm_mmu.h +2 -0

index 405aa1883307..412b363f79e9 100644
@@ -30,6 +30,8 @@
 #define HYP_PAGE_OFFSET              PAGE_OFFSET
 #define KERN_TO_HYP(kva)     (kva)
 
+#define kvm_ksym_ref(kva)    (kva)
+
 /*
  * Our virtual mapping for the boot-time MMU-enable code. Must be
  * shared across all the page-tables. Conveniently, we use the vectors

arch/arm/kvm/arm.c +0 -0

index e06fd299de08..014b542ea658 100644

@@ -427,7 +42You can use it, but you don't have to, since yo7,7 @@ static void update_vttbr(struct kvm *kvm)
* shareable domain to make sure all data structures are
* clean.
*/

arch/arm/kvm/arm.c +5 -4

-             kvm_call_hyp(__kvm_flush_vm_context);
+             kvm_call_hyp(kvm_ksym_ref(__kvm_flush_vm_context));
      }
 
      kvm->arch.vmid_gen = atomic64_read(&kvm_vmid_gen);

@@ -600,7 +600,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
              __kvm_guest_enter();
              vcpu->mode = IN_GUEST_MODE;
 
-             ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
+             ret = kvm_call_hyp(kvm_ksym_ref(__kvm_vcpu_run), vcpu);
 
              vcpu->mode = OUTSIDE_GUEST_MODE;
              /*

@@ -969,7 +969,7 @@ static void cpu_init_hyp_mode(void *dummy)
      pgd_ptr = kvm_mmu_get_httbr();
      stack_page = __this_cpu_read(kvm_arm_hyp_stack_page);
      hyp_stack_ptr = stack_page + PAGE_SIZE;
-     vector_ptr = (unsigned long)__kvm_hyp_vector;
+     vector_ptr = (unsigned long)kvm_ksym_ref(__kvm_hyp_vector);
 
      __cpu_init_hyp_mode(boot_pgd_ptr, pgd_ptr, hyp_stack_ptr, vector_ptr);
 

@@ -1061,7 +1061,8 @@ static int init_hyp_mode(void)
      /*
       * Map the Hyp-code called directly from the host
       */
-     err = create_hyp_mappings(__kvm_hyp_code_start, __kvm_hyp_code_end);
+     err = create_hyp_mappings(kvm_ksym_ref(__kvm_hyp_code_start),
+                               kvm_ksym_ref(__kvm_hyp_code_end));
      if (err) {
              kvm_err("Cannot map world-switch code\n");
              goto out_free_mappings;

arch/arm/kvm/mmu.c +0 -0

index 7dace909d5cf..7c448b943e3a 100644
@@ -31,8 +31,6 @@
 
 #include "trace.h"
 

arch/arm/kvm/mmu.c static bool memslot_is_logging(struct kvm_memory_slot *memslot) · line 64

Quoted 11 lines

arch/arm/kvm/mmu.c +1 -3

-extern char  __hyp_idmap_text_start[], __hyp_idmap_text_end[];
-
 static pgd_t *boot_hyp_pgd;
 static pgd_t *hyp_pgd;
 static pgd_t *merged_hyp_pgd;

@@ -63,7 +61,7 @@ static bool memslot_is_logging(struct kvm_memory_slot *memslot)
  */
 void kvm_flush_remote_tlbs(struct kvm *kvm)
 {
-     kvm_call_hyp(__kvm_tlb_flush_vmid, kvm);
+     kvm_call_hyp(kvm_ksym_ref(__kvm_tlb_flush_vmid), kvm);

Any chance we could bury kvm_ksym_ref in kvm_call_hyp? It may make the change more readable, but I have the feeling it would require an intermediate #define...

Quoted 16 lines

arch/arm/kvm/mmu.c +1 -1

 }
 
 static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)

@@ -75,7 +73,7 @@ static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
       * anything there.
       */
      if (kvm)
-             kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa);
+             kvm_call_hyp(kvm_ksym_ref(__kvm_tlb_flush_vmid_ipa), kvm, ipa);
 }
 
 /*

@@ -1647,9 +1645,9 @@ int kvm_mmu_init(void)
 {
      int err;
 

arch/arm/kvm/mmu.c int kvm_mmu_init(void) · line 1650

Quoted 6 lines

arch/arm/kvm/mmu.c +3 -3

-     hyp_idmap_start = kvm_virt_to_phys(__hyp_idmap_text_start);
-     hyp_idmap_end = kvm_virt_to_phys(__hyp_idmap_text_end);
-     hyp_idmap_vector = kvm_virt_to_phys(__kvm_hyp_init);
+     hyp_idmap_start = kvm_virt_to_phys(&__hyp_idmap_text_start);
+     hyp_idmap_end = kvm_virt_to_phys(&__hyp_idmap_text_end);
+     hyp_idmap_vector = kvm_virt_to_phys(&__kvm_hyp_init);

Why don't you need to use kvm_ksym_ref here? Is the idmap treated differently?

arch/arm64/include/asm/kvm_asm.h line 108

Quoted 12 lines

arch/arm/kvm/mmu.c +0 -0

      /*
       * We rely on the linker script to ensure at build time that the HYP

arch/arm64/include/asm/kvm_asm.h +1 -0

index 5e377101f919..830402f847e0 100644
@@ -105,24 +105,27 @@
 #ifndef __ASSEMBLY__
 struct kvm;
 struct kvm_vcpu;
+struct kvm_ksym;

And that's it? Never actually defined? That's cunning! ;-)

Quoted 65 lines

arch/arm64/include/asm/kvm_asm.h +11 -9

 extern char __kvm_hyp_init[];
 extern char __kvm_hyp_init_end[];
 
-extern char __kvm_hyp_vector[];
+extern struct kvm_ksym __kvm_hyp_vector;
 
-#define      __kvm_hyp_code_start    __hyp_text_start
-#define      __kvm_hyp_code_end      __hyp_text_end
+extern struct kvm_ksym __kvm_hyp_code_start;
+extern struct kvm_ksym __kvm_hyp_code_end;
 
-extern void __kvm_flush_vm_context(void);
-extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
-extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
+extern struct kvm_ksym __kvm_flush_vm_context;
+extern struct kvm_ksym __kvm_tlb_flush_vmid_ipa;
+extern struct kvm_ksym __kvm_tlb_flush_vmid;
 
-extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+extern struct kvm_ksym __kvm_vcpu_run;
 
-extern u64 __vgic_v3_get_ich_vtr_el2(void);
+extern struct kvm_ksym __hyp_idmap_text_start, __hyp_idmap_text_end;
 
-extern u32 __kvm_get_mdcr_el2(void);
+extern struct kvm_ksym __vgic_v3_get_ich_vtr_el2;
+
+extern struct kvm_ksym __kvm_get_mdcr_el2;
 
 #endif
 

arch/arm64/include/asm/kvm_mmu.h +2 -0

index 61505676d085..0899026a2821 100644
@@ -73,6 +73,8 @@
 
 #define KERN_TO_HYP(kva)     ((unsigned long)kva - PAGE_OFFSET + HYP_PAGE_OFFSET)
 
+#define kvm_ksym_ref(sym)    ((void *)&sym - KIMAGE_VADDR + PAGE_OFFSET)
+
 /*
  * We currently only support a 40bit IPA.
  */

arch/arm64/include/asm/virt.h +0 -4

index 7a5df5252dd7..215ad4649dd7 100644
@@ -50,10 +50,6 @@ static inline bool is_hyp_mode_mismatched(void)
      return __boot_cpu_mode[0] != __boot_cpu_mode[1];
 }
 
-/* The section containing the hypervisor text */
-extern char __hyp_text_start[];
-extern char __hyp_text_end[];
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* ! __ASM__VIRT_H */

arch/arm64/kernel/vmlinux.lds.S +0 -0

index 363c2f529951..f935f082188d 100644

arch/arm64/kernel/vmlinux.lds.S jiffies = jiffies_64; · line 40

Quoted 8 lines

arch/arm64/kernel/vmlinux.lds.S +2 -2

@@ -35,9 +35,9 @@ jiffies = jiffies_64;
      VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;     \
      *(.hyp.idmap.text)                              \
      VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;       \
-     VMLINUX_SYMBOL(__hyp_text_start) = .;           \
+     VMLINUX_SYMBOL(__kvm_hyp_code_start) = .;       \
      *(.hyp.text)                                    \
-     VMLINUX_SYMBOL(__hyp_text_end) = .;
+     VMLINUX_SYMBOL(__kvm_hyp_code_end) = .;

I have a couple of patches going in the exact opposite direction (making arm more similar to arm64):

http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm/wsinc&id=94a3d4d4ff1d8ad59f9150dfa9fdd1685ab03950
http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm/wsinc&id=44aec57b62dca67cf91f425e3707f257b9bbeb18

As at least the first patch is required to convert the 32bit HYP code to C, I'd rather not change this in the 64bit code.

Quoted 31 lines

arch/arm64/kernel/vmlinux.lds.S +0 -0

 #define IDMAP_TEXT                                   \
      . = ALIGN(SZ_4K);                               \

arch/arm64/kvm/debug.c +3 -1

index 47e5f0feaee8..99e5a403af4e 100644
@@ -24,6 +24,7 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_arm.h>
 #include <asm/kvm_emulate.h>
+#include <asm/kvm_mmu.h>
 
 #include "trace.h"
 

@@ -72,7 +73,8 @@ static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
 
 void kvm_arm_init_debug(void)
 {
-     __this_cpu_write(mdcr_el2, kvm_call_hyp(__kvm_get_mdcr_el2));
+     __this_cpu_write(mdcr_el2,
+                      kvm_call_hyp(kvm_ksym_ref(__kvm_get_mdcr_el2)));
 }
 
 /**

virt/kvm/arm/vgic-v3.c +0 -0

index 487d6357b7e7..58f5a6521307 100644

virt/kvm/arm/vgic-v3.c int vgic_v3_probe(struct device_node *vgic_node, · line 253

Quoted 8 lines

virt/kvm/arm/vgic-v3.c +1 -1

@@ -247,7 +247,7 @@ int vgic_v3_probe(struct device_node *vgic_node,
              goto out;
      }
 
-     ich_vtr_el2 = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
+     ich_vtr_el2 = kvm_call_hyp(kvm_ksym_ref(__vgic_v3_get_ich_vtr_el2));
 
      /*
       * The ListRegs field is 5 bits, but there is a architectural

Thanks,

        M.

--
Jazz is not dead. It just smells funny...

Quoted 107 lines · Marc Zyngier

Hi Ard,

Quoted 6 lines · Ard Biesheuvel

KVM on arm64 uses a fixed offset between the linear mapping at EL1 and the HYP mapping at EL2. Before we can move the kernel virtual mapping out of the linear mapping, we have to make sure that references to kernel symbols that are accessed via the HYP mapping are translated to their linear equivalent.

Quoted 7 lines

To prevent inadvertent direct references from sneaking in later, change the type of all extern declarations to HYP kernel symbols to the opaque 'struct kvm_ksym', which does not decay to a pointer type like char arrays and function references. This is not bullet proof, but at least forces the user to take the address explicitly rather than referencing it directly.

Signed-off-by Ard Biesheuvel <[email protected]>

This looks good to me, a few comments below.

Quoted 88 lines

---
 arch/arm/include/asm/kvm_asm.h   |  2 ++
 arch/arm/include/asm/kvm_mmu.h   |  2 ++
 arch/arm/kvm/arm.c               |  9 +++++----
 arch/arm/kvm/mmu.c               | 12 +++++------
 arch/arm64/include/asm/kvm_asm.h | 21 +++++++++++---------
 arch/arm64/include/asm/kvm_mmu.h |  2 ++
 arch/arm64/include/asm/virt.h    |  4 ----
 arch/arm64/kernel/vmlinux.lds.S  |  4 ++--
 arch/arm64/kvm/debug.c           |  4 +++-
 virt/kvm/arm/vgic-v3.c           |  2 +-
 10 files changed, 34 insertions(+), 28 deletions(-)

arch/arm/include/asm/kvm_asm.h +2 -0

index 194c91b610ff..484ffdf7c70b 100644
@@ -99,6 +99,8 @@ extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
 
 extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+
+extern char __hyp_idmap_text_start[], __hyp_idmap_text_end[];
 #endif
 
 #endif /* __ARM_KVM_ASM_H__ */

arch/arm/include/asm/kvm_mmu.h +2 -0

index 405aa1883307..412b363f79e9 100644
@@ -30,6 +30,8 @@
 #define HYP_PAGE_OFFSET              PAGE_OFFSET
 #define KERN_TO_HYP(kva)     (kva)
 
+#define kvm_ksym_ref(kva)    (kva)
+
 /*
  * Our virtual mapping for the boot-time MMU-enable code. Must be
  * shared across all the page-tables. Conveniently, we use the vectors

arch/arm/kvm/arm.c +0 -0

index e06fd299de08..014b542ea658 100644

@@ -427,7 +42You can use it, but you don't have to, since yo7,7 @@ static void update_vttbr(struct kvm *kvm)
* shareable domain to make sure all data structures are
* clean.
*/

arch/arm/kvm/arm.c +5 -4

-             kvm_call_hyp(__kvm_flush_vm_context);
+             kvm_call_hyp(kvm_ksym_ref(__kvm_flush_vm_context));
      }
 
      kvm->arch.vmid_gen = atomic64_read(&kvm_vmid_gen);

@@ -600,7 +600,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
              __kvm_guest_enter();
              vcpu->mode = IN_GUEST_MODE;
 
-             ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
+             ret = kvm_call_hyp(kvm_ksym_ref(__kvm_vcpu_run), vcpu);
 
              vcpu->mode = OUTSIDE_GUEST_MODE;
              /*

@@ -969,7 +969,7 @@ static void cpu_init_hyp_mode(void *dummy)
      pgd_ptr = kvm_mmu_get_httbr();
      stack_page = __this_cpu_read(kvm_arm_hyp_stack_page);
      hyp_stack_ptr = stack_page + PAGE_SIZE;
-     vector_ptr = (unsigned long)__kvm_hyp_vector;
+     vector_ptr = (unsigned long)kvm_ksym_ref(__kvm_hyp_vector);
 
      __cpu_init_hyp_mode(boot_pgd_ptr, pgd_ptr, hyp_stack_ptr, vector_ptr);
 

@@ -1061,7 +1061,8 @@ static int init_hyp_mode(void)
      /*
       * Map the Hyp-code called directly from the host
       */
-     err = create_hyp_mappings(__kvm_hyp_code_start, __kvm_hyp_code_end);
+     err = create_hyp_mappings(kvm_ksym_ref(__kvm_hyp_code_start),
+                               kvm_ksym_ref(__kvm_hyp_code_end));
      if (err) {
              kvm_err("Cannot map world-switch code\n");
              goto out_free_mappings;

arch/arm/kvm/mmu.c +0 -0

index 7dace909d5cf..7c448b943e3a 100644
@@ -31,8 +31,6 @@
 
 #include "trace.h"
 

Quoted 16 lines

arch/arm/kvm/mmu.c static bool memslot_is_logging(struct kvm_memory_slot *memslot) · line 64

Quoted 11 lines

arch/arm/kvm/mmu.c +1 -3

-extern char  __hyp_idmap_text_start[], __hyp_idmap_text_end[];
-
 static pgd_t *boot_hyp_pgd;
 static pgd_t *hyp_pgd;
 static pgd_t *merged_hyp_pgd;

@@ -63,7 +61,7 @@ static bool memslot_is_logging(struct kvm_memory_slot *memslot)
  */
 void kvm_flush_remote_tlbs(struct kvm *kvm)
 {
-     kvm_call_hyp(__kvm_tlb_flush_vmid, kvm);
+     kvm_call_hyp(kvm_ksym_ref(__kvm_tlb_flush_vmid), kvm);

Any chance we could bury kvm_ksym_ref in kvm_call_hyp? It may make the change more readable, but I have the feeling it would require an intermediate #define...

Yes, we'd have to rename the actual kvm_call_hyp definition so we can wrap it in a macro

And the call in __cpu_init_hyp_mode() would need to omit the macro, since it passes a pointer into the linear mapping, not a kernel symbol.
So if you think that's ok, I'm happy to change that.

KVM on arm64 uses a fixed offset between the linear mapping at EL1 and the HYP mapping at EL2. Before we can move the kernel virtual mapping out of the linear mapping, we have to make sure that references to kernel symbols that are accessed via the HYP mapping are translated to their linear equivalent.

To prevent inadvertent direct references from sneaking in later, change the type of all extern declarations to HYP kernel symbols to the opaque 'struct kvm_ksym', which does not decay to a pointer type like char arrays and function references. This is not bullet proof, but at least forces the user to take the address explicitly rather than referencing it directly.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm/include/asm/kvm_asm.h   |  2 ++
 arch/arm/include/asm/kvm_mmu.h   |  2 ++
 arch/arm/kvm/arm.c               |  9 +++++----
 arch/arm/kvm/mmu.c               | 12 +++++------
 arch/arm64/include/asm/kvm_asm.h | 21 +++++++++++---------
 arch/arm64/include/asm/kvm_mmu.h |  2 ++
 arch/arm64/include/asm/virt.h    |  4 ----
 arch/arm64/kernel/vmlinux.lds.S  |  4 ++--
 arch/arm64/kvm/debug.c           |  4 +++-
 virt/kvm/arm/vgic-v3.c           |  2 +-
 10 files changed, 34 insertions(+), 28 deletions(-)

arch/arm/include/asm/kvm_asm.h +2 -0

index 194c91b610ff..484ffdf7c70b 100644
@@ -99,6 +99,8 @@ extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
 
 extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+
+extern char __hyp_idmap_text_start[], __hyp_idmap_text_end[];
 #endif
 
 #endif /* __ARM_KVM_ASM_H__ */

arch/arm/include/asm/kvm_mmu.h +2 -0

index 405aa1883307..412b363f79e9 100644
@@ -30,6 +30,8 @@
 #define HYP_PAGE_OFFSET		PAGE_OFFSET
 #define KERN_TO_HYP(kva)	(kva)
 
+#define kvm_ksym_ref(kva)	(kva)
+
 /*
  * Our virtual mapping for the boot-time MMU-enable code. Must be
  * shared across all the page-tables. Conveniently, we use the vectors

arch/arm/kvm/arm.c +5 -4

index e06fd299de08..014b542ea658 100644
@@ -427,7 +427,7 @@ static void update_vttbr(struct kvm *kvm)
 		 * shareable domain to make sure all data structures are
 		 * clean.
 		 */
-		kvm_call_hyp(__kvm_flush_vm_context);
+		kvm_call_hyp(kvm_ksym_ref(__kvm_flush_vm_context));
 	}
 
 	kvm->arch.vmid_gen = atomic64_read(&kvm_vmid_gen);

@@ -600,7 +600,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 		__kvm_guest_enter();
 		vcpu->mode = IN_GUEST_MODE;
 
-		ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
+		ret = kvm_call_hyp(kvm_ksym_ref(__kvm_vcpu_run), vcpu);
 
 		vcpu->mode = OUTSIDE_GUEST_MODE;
 		/*

@@ -969,7 +969,7 @@ static void cpu_init_hyp_mode(void *dummy)
 	pgd_ptr = kvm_mmu_get_httbr();
 	stack_page = __this_cpu_read(kvm_arm_hyp_stack_page);
 	hyp_stack_ptr = stack_page + PAGE_SIZE;
-	vector_ptr = (unsigned long)__kvm_hyp_vector;
+	vector_ptr = (unsigned long)kvm_ksym_ref(__kvm_hyp_vector);
 
 	__cpu_init_hyp_mode(boot_pgd_ptr, pgd_ptr, hyp_stack_ptr, vector_ptr);
 

@@ -1061,7 +1061,8 @@ static int init_hyp_mode(void)
 	/*
 	 * Map the Hyp-code called directly from the host
 	 */
-	err = create_hyp_mappings(__kvm_hyp_code_start, __kvm_hyp_code_end);
+	err = create_hyp_mappings(kvm_ksym_ref(__kvm_hyp_code_start),
+				  kvm_ksym_ref(__kvm_hyp_code_end));
 	if (err) {
 		kvm_err("Cannot map world-switch code\n");
 		goto out_free_mappings;

arch/arm/kvm/mmu.c +5 -7

index 7dace909d5cf..7c448b943e3a 100644
@@ -31,8 +31,6 @@
 
 #include "trace.h"
 
-extern char  __hyp_idmap_text_start[], __hyp_idmap_text_end[];
-
 static pgd_t *boot_hyp_pgd;
 static pgd_t *hyp_pgd;
 static pgd_t *merged_hyp_pgd;

@@ -63,7 +61,7 @@ static bool memslot_is_logging(struct kvm_memory_slot *memslot)
  */
 void kvm_flush_remote_tlbs(struct kvm *kvm)
 {
-	kvm_call_hyp(__kvm_tlb_flush_vmid, kvm);
+	kvm_call_hyp(kvm_ksym_ref(__kvm_tlb_flush_vmid), kvm);
 }
 
 static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)

@@ -75,7 +73,7 @@ static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 	 * anything there.
 	 */
 	if (kvm)
-		kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa);
+		kvm_call_hyp(kvm_ksym_ref(__kvm_tlb_flush_vmid_ipa), kvm, ipa);
 }
 
 /*

@@ -1647,9 +1645,9 @@ int kvm_mmu_init(void)
 {
 	int err;
 
-	hyp_idmap_start = kvm_virt_to_phys(__hyp_idmap_text_start);
-	hyp_idmap_end = kvm_virt_to_phys(__hyp_idmap_text_end);
-	hyp_idmap_vector = kvm_virt_to_phys(__kvm_hyp_init);
+	hyp_idmap_start = kvm_virt_to_phys(&__hyp_idmap_text_start);
+	hyp_idmap_end = kvm_virt_to_phys(&__hyp_idmap_text_end);
+	hyp_idmap_vector = kvm_virt_to_phys(&__kvm_hyp_init);
 
 	/*
 	 * We rely on the linker script to ensure at build time that the HYP

arch/arm64/include/asm/kvm_asm.h +12 -9

index 5e377101f919..830402f847e0 100644
@@ -105,24 +105,27 @@
 #ifndef __ASSEMBLY__
 struct kvm;
 struct kvm_vcpu;
+struct kvm_ksym;
 
 extern char __kvm_hyp_init[];
 extern char __kvm_hyp_init_end[];
 
-extern char __kvm_hyp_vector[];
+extern struct kvm_ksym __kvm_hyp_vector;
 
-#define	__kvm_hyp_code_start	__hyp_text_start
-#define	__kvm_hyp_code_end	__hyp_text_end
+extern struct kvm_ksym __kvm_hyp_code_start;
+extern struct kvm_ksym __kvm_hyp_code_end;
 
-extern void __kvm_flush_vm_context(void);
-extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
-extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
+extern struct kvm_ksym __kvm_flush_vm_context;
+extern struct kvm_ksym __kvm_tlb_flush_vmid_ipa;
+extern struct kvm_ksym __kvm_tlb_flush_vmid;
 
-extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+extern struct kvm_ksym __kvm_vcpu_run;
 
-extern u64 __vgic_v3_get_ich_vtr_el2(void);
+extern struct kvm_ksym __hyp_idmap_text_start, __hyp_idmap_text_end;
 
-extern u32 __kvm_get_mdcr_el2(void);
+extern struct kvm_ksym __vgic_v3_get_ich_vtr_el2;
+
+extern struct kvm_ksym __kvm_get_mdcr_el2;
 
 #endif
 

arch/arm64/include/asm/kvm_mmu.h +2 -0

index 61505676d085..0899026a2821 100644
@@ -73,6 +73,8 @@
 
 #define KERN_TO_HYP(kva)	((unsigned long)kva - PAGE_OFFSET + HYP_PAGE_OFFSET)
 
+#define kvm_ksym_ref(sym)	((void *)&sym - KIMAGE_VADDR + PAGE_OFFSET)
+
 /*
  * We currently only support a 40bit IPA.
  */

arch/arm64/include/asm/virt.h +0 -4

index 7a5df5252dd7..215ad4649dd7 100644
@@ -50,10 +50,6 @@ static inline bool is_hyp_mode_mismatched(void)
 	return __boot_cpu_mode[0] != __boot_cpu_mode[1];
 }
 
-/* The section containing the hypervisor text */
-extern char __hyp_text_start[];
-extern char __hyp_text_end[];
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* ! __ASM__VIRT_H */

arch/arm64/kernel/vmlinux.lds.S +0 -0

index 363c2f529951..f935f082188d 100644

Quoted 205 lines · Ard Biesheuvel

KVM on arm64 uses a fixed offset between the linear mapping at EL1 and the HYP mapping at EL2. Before we can move the kernel virtual mapping out of the linear mapping, we have to make sure that references to kernel symbols that are accessed via the HYP mapping are translated to their linear equivalent.

To prevent inadvertent direct references from sneaking in later, change the type of all extern declarations to HYP kernel symbols to the opaque 'struct kvm_ksym', which does not decay to a pointer type like char arrays and function references. This is not bullet proof, but at least forces the user to take the address explicitly rather than referencing it directly.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm/include/asm/kvm_asm.h   |  2 ++
 arch/arm/include/asm/kvm_mmu.h   |  2 ++
 arch/arm/kvm/arm.c               |  9 +++++----
 arch/arm/kvm/mmu.c               | 12 +++++------
 arch/arm64/include/asm/kvm_asm.h | 21 +++++++++++---------
 arch/arm64/include/asm/kvm_mmu.h |  2 ++
 arch/arm64/include/asm/virt.h    |  4 ----
 arch/arm64/kernel/vmlinux.lds.S  |  4 ++--
 arch/arm64/kvm/debug.c           |  4 +++-
 virt/kvm/arm/vgic-v3.c           |  2 +-
 10 files changed, 34 insertions(+), 28 deletions(-)

arch/arm/include/asm/kvm_asm.h +2 -0

index 194c91b610ff..484ffdf7c70b 100644
@@ -99,6 +99,8 @@ extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
 
 extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+
+extern char __hyp_idmap_text_start[], __hyp_idmap_text_end[];
 #endif
 
 #endif /* __ARM_KVM_ASM_H__ */

arch/arm/include/asm/kvm_mmu.h +2 -0

index 405aa1883307..412b363f79e9 100644
@@ -30,6 +30,8 @@
 #define HYP_PAGE_OFFSET              PAGE_OFFSET
 #define KERN_TO_HYP(kva)     (kva)
 
+#define kvm_ksym_ref(kva)    (kva)
+
 /*
  * Our virtual mapping for the boot-time MMU-enable code. Must be
  * shared across all the page-tables. Conveniently, we use the vectors

arch/arm/kvm/arm.c +5 -4

index e06fd299de08..014b542ea658 100644
@@ -427,7 +427,7 @@ static void update_vttbr(struct kvm *kvm)
               * shareable domain to make sure all data structures are
               * clean.
               */
-             kvm_call_hyp(__kvm_flush_vm_context);
+             kvm_call_hyp(kvm_ksym_ref(__kvm_flush_vm_context));
      }
 
      kvm->arch.vmid_gen = atomic64_read(&kvm_vmid_gen);

@@ -600,7 +600,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
              __kvm_guest_enter();
              vcpu->mode = IN_GUEST_MODE;
 
-             ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
+             ret = kvm_call_hyp(kvm_ksym_ref(__kvm_vcpu_run), vcpu);
 
              vcpu->mode = OUTSIDE_GUEST_MODE;
              /*

@@ -969,7 +969,7 @@ static void cpu_init_hyp_mode(void *dummy)
      pgd_ptr = kvm_mmu_get_httbr();
      stack_page = __this_cpu_read(kvm_arm_hyp_stack_page);
      hyp_stack_ptr = stack_page + PAGE_SIZE;
-     vector_ptr = (unsigned long)__kvm_hyp_vector;
+     vector_ptr = (unsigned long)kvm_ksym_ref(__kvm_hyp_vector);
 
      __cpu_init_hyp_mode(boot_pgd_ptr, pgd_ptr, hyp_stack_ptr, vector_ptr);
 

@@ -1061,7 +1061,8 @@ static int init_hyp_mode(void)
      /*
       * Map the Hyp-code called directly from the host
       */
-     err = create_hyp_mappings(__kvm_hyp_code_start, __kvm_hyp_code_end);
+     err = create_hyp_mappings(kvm_ksym_ref(__kvm_hyp_code_start),
+                               kvm_ksym_ref(__kvm_hyp_code_end));
      if (err) {
              kvm_err("Cannot map world-switch code\n");
              goto out_free_mappings;

arch/arm/kvm/mmu.c +5 -7

index 7dace909d5cf..7c448b943e3a 100644
@@ -31,8 +31,6 @@
 
 #include "trace.h"
 
-extern char  __hyp_idmap_text_start[], __hyp_idmap_text_end[];
-
 static pgd_t *boot_hyp_pgd;
 static pgd_t *hyp_pgd;
 static pgd_t *merged_hyp_pgd;

@@ -63,7 +61,7 @@ static bool memslot_is_logging(struct kvm_memory_slot *memslot)
  */
 void kvm_flush_remote_tlbs(struct kvm *kvm)
 {
-     kvm_call_hyp(__kvm_tlb_flush_vmid, kvm);
+     kvm_call_hyp(kvm_ksym_ref(__kvm_tlb_flush_vmid), kvm);
 }
 
 static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)

@@ -75,7 +73,7 @@ static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
       * anything there.
       */
      if (kvm)
-             kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa);
+             kvm_call_hyp(kvm_ksym_ref(__kvm_tlb_flush_vmid_ipa), kvm, ipa);
 }
 
 /*

@@ -1647,9 +1645,9 @@ int kvm_mmu_init(void)
 {
      int err;
 
-     hyp_idmap_start = kvm_virt_to_phys(__hyp_idmap_text_start);
-     hyp_idmap_end = kvm_virt_to_phys(__hyp_idmap_text_end);
-     hyp_idmap_vector = kvm_virt_to_phys(__kvm_hyp_init);
+     hyp_idmap_start = kvm_virt_to_phys(&__hyp_idmap_text_start);
+     hyp_idmap_end = kvm_virt_to_phys(&__hyp_idmap_text_end);
+     hyp_idmap_vector = kvm_virt_to_phys(&__kvm_hyp_init);
 
      /*
       * We rely on the linker script to ensure at build time that the HYP

arch/arm64/include/asm/kvm_asm.h +12 -9

index 5e377101f919..830402f847e0 100644
@@ -105,24 +105,27 @@
 #ifndef __ASSEMBLY__
 struct kvm;
 struct kvm_vcpu;
+struct kvm_ksym;
 
 extern char __kvm_hyp_init[];
 extern char __kvm_hyp_init_end[];
 
-extern char __kvm_hyp_vector[];
+extern struct kvm_ksym __kvm_hyp_vector;
 
-#define      __kvm_hyp_code_start    __hyp_text_start
-#define      __kvm_hyp_code_end      __hyp_text_end
+extern struct kvm_ksym __kvm_hyp_code_start;
+extern struct kvm_ksym __kvm_hyp_code_end;
 
-extern void __kvm_flush_vm_context(void);
-extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
-extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
+extern struct kvm_ksym __kvm_flush_vm_context;
+extern struct kvm_ksym __kvm_tlb_flush_vmid_ipa;
+extern struct kvm_ksym __kvm_tlb_flush_vmid;
 
-extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+extern struct kvm_ksym __kvm_vcpu_run;
 
-extern u64 __vgic_v3_get_ich_vtr_el2(void);
+extern struct kvm_ksym __hyp_idmap_text_start, __hyp_idmap_text_end;
 
-extern u32 __kvm_get_mdcr_el2(void);
+extern struct kvm_ksym __vgic_v3_get_ich_vtr_el2;
+
+extern struct kvm_ksym __kvm_get_mdcr_el2;
 
 #endif
 

arch/arm64/include/asm/kvm_mmu.h +2 -0

index 61505676d085..0899026a2821 100644
@@ -73,6 +73,8 @@
 
 #define KERN_TO_HYP(kva)     ((unsigned long)kva - PAGE_OFFSET + HYP_PAGE_OFFSET)
 
+#define kvm_ksym_ref(sym)    ((void *)&sym - KIMAGE_VADDR + PAGE_OFFSET)
+
 /*
  * We currently only support a 40bit IPA.
  */

arch/arm64/include/asm/virt.h +0 -4

index 7a5df5252dd7..215ad4649dd7 100644
@@ -50,10 +50,6 @@ static inline bool is_hyp_mode_mismatched(void)
      return __boot_cpu_mode[0] != __boot_cpu_mode[1];
 }
 
-/* The section containing the hypervisor text */
-extern char __hyp_text_start[];
-extern char __hyp_text_end[];
-
 #endif /* __ASSEMBLY__ */
 

Quoted 6 lines

arch/arm64/include/asm/virt.h +0 -0

 #endif /* ! __ASM__VIRT_H */

arch/arm64/kernel/vmlinux.lds.S +0 -0

index 363c2f529951..f935f082188d 100644

arch/arm64/kernel/vmlinux.lds.S jiffies = jiffies_64; · line 40

Quoted 8 lines

arch/arm64/kernel/vmlinux.lds.S +2 -2

@@ -35,9 +35,9 @@ jiffies = jiffies_64;
      VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;     \
      *(.hyp.idmap.text)                              \
      VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;       \
-     VMLINUX_SYMBOL(__hyp_text_start) = .;           \
+     VMLINUX_SYMBOL(__kvm_hyp_code_start) = .;       \
      *(.hyp.text)                                    \
-     VMLINUX_SYMBOL(__hyp_text_end) = .;
+     VMLINUX_SYMBOL(__kvm_hyp_code_end) = .;

why this rename?

Quoted 206 lines · Christoffer Dall

Quoted 205 lines · Ard Biesheuvel

KVM on arm64 uses a fixed offset between the linear mapping at EL1 and the HYP mapping at EL2. Before we can move the kernel virtual mapping out of the linear mapping, we have to make sure that references to kernel symbols that are accessed via the HYP mapping are translated to their linear equivalent.

To prevent inadvertent direct references from sneaking in later, change the type of all extern declarations to HYP kernel symbols to the opaque 'struct kvm_ksym', which does not decay to a pointer type like char arrays and function references. This is not bullet proof, but at least forces the user to take the address explicitly rather than referencing it directly.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm/include/asm/kvm_asm.h   |  2 ++
 arch/arm/include/asm/kvm_mmu.h   |  2 ++
 arch/arm/kvm/arm.c               |  9 +++++----
 arch/arm/kvm/mmu.c               | 12 +++++------
 arch/arm64/include/asm/kvm_asm.h | 21 +++++++++++---------
 arch/arm64/include/asm/kvm_mmu.h |  2 ++
 arch/arm64/include/asm/virt.h    |  4 ----
 arch/arm64/kernel/vmlinux.lds.S  |  4 ++--
 arch/arm64/kvm/debug.c           |  4 +++-
 virt/kvm/arm/vgic-v3.c           |  2 +-
 10 files changed, 34 insertions(+), 28 deletions(-)

arch/arm/include/asm/kvm_asm.h +2 -0

index 194c91b610ff..484ffdf7c70b 100644
@@ -99,6 +99,8 @@ extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
 
 extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+
+extern char __hyp_idmap_text_start[], __hyp_idmap_text_end[];
 #endif
 
 #endif /* __ARM_KVM_ASM_H__ */

arch/arm/include/asm/kvm_mmu.h +2 -0

index 405aa1883307..412b363f79e9 100644
@@ -30,6 +30,8 @@
 #define HYP_PAGE_OFFSET              PAGE_OFFSET
 #define KERN_TO_HYP(kva)     (kva)
 
+#define kvm_ksym_ref(kva)    (kva)
+
 /*
  * Our virtual mapping for the boot-time MMU-enable code. Must be
  * shared across all the page-tables. Conveniently, we use the vectors

arch/arm/kvm/arm.c +5 -4

index e06fd299de08..014b542ea658 100644
@@ -427,7 +427,7 @@ static void update_vttbr(struct kvm *kvm)
               * shareable domain to make sure all data structures are
               * clean.
               */
-             kvm_call_hyp(__kvm_flush_vm_context);
+             kvm_call_hyp(kvm_ksym_ref(__kvm_flush_vm_context));
      }
 
      kvm->arch.vmid_gen = atomic64_read(&kvm_vmid_gen);

@@ -600,7 +600,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
              __kvm_guest_enter();
              vcpu->mode = IN_GUEST_MODE;
 
-             ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
+             ret = kvm_call_hyp(kvm_ksym_ref(__kvm_vcpu_run), vcpu);
 
              vcpu->mode = OUTSIDE_GUEST_MODE;
              /*

@@ -969,7 +969,7 @@ static void cpu_init_hyp_mode(void *dummy)
      pgd_ptr = kvm_mmu_get_httbr();
      stack_page = __this_cpu_read(kvm_arm_hyp_stack_page);
      hyp_stack_ptr = stack_page + PAGE_SIZE;
-     vector_ptr = (unsigned long)__kvm_hyp_vector;
+     vector_ptr = (unsigned long)kvm_ksym_ref(__kvm_hyp_vector);
 
      __cpu_init_hyp_mode(boot_pgd_ptr, pgd_ptr, hyp_stack_ptr, vector_ptr);
 

@@ -1061,7 +1061,8 @@ static int init_hyp_mode(void)
      /*
       * Map the Hyp-code called directly from the host
       */
-     err = create_hyp_mappings(__kvm_hyp_code_start, __kvm_hyp_code_end);
+     err = create_hyp_mappings(kvm_ksym_ref(__kvm_hyp_code_start),
+                               kvm_ksym_ref(__kvm_hyp_code_end));
      if (err) {
              kvm_err("Cannot map world-switch code\n");
              goto out_free_mappings;

arch/arm/kvm/mmu.c +5 -7

index 7dace909d5cf..7c448b943e3a 100644
@@ -31,8 +31,6 @@
 
 #include "trace.h"
 
-extern char  __hyp_idmap_text_start[], __hyp_idmap_text_end[];
-
 static pgd_t *boot_hyp_pgd;
 static pgd_t *hyp_pgd;
 static pgd_t *merged_hyp_pgd;

@@ -63,7 +61,7 @@ static bool memslot_is_logging(struct kvm_memory_slot *memslot)
  */
 void kvm_flush_remote_tlbs(struct kvm *kvm)
 {
-     kvm_call_hyp(__kvm_tlb_flush_vmid, kvm);
+     kvm_call_hyp(kvm_ksym_ref(__kvm_tlb_flush_vmid), kvm);
 }
 
 static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)

@@ -75,7 +73,7 @@ static void kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
       * anything there.
       */
      if (kvm)
-             kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, kvm, ipa);
+             kvm_call_hyp(kvm_ksym_ref(__kvm_tlb_flush_vmid_ipa), kvm, ipa);
 }
 
 /*

@@ -1647,9 +1645,9 @@ int kvm_mmu_init(void)
 {
      int err;
 
-     hyp_idmap_start = kvm_virt_to_phys(__hyp_idmap_text_start);
-     hyp_idmap_end = kvm_virt_to_phys(__hyp_idmap_text_end);
-     hyp_idmap_vector = kvm_virt_to_phys(__kvm_hyp_init);
+     hyp_idmap_start = kvm_virt_to_phys(&__hyp_idmap_text_start);
+     hyp_idmap_end = kvm_virt_to_phys(&__hyp_idmap_text_end);
+     hyp_idmap_vector = kvm_virt_to_phys(&__kvm_hyp_init);
 
      /*
       * We rely on the linker script to ensure at build time that the HYP

arch/arm64/include/asm/kvm_asm.h +12 -9

index 5e377101f919..830402f847e0 100644
@@ -105,24 +105,27 @@
 #ifndef __ASSEMBLY__
 struct kvm;
 struct kvm_vcpu;
+struct kvm_ksym;
 
 extern char __kvm_hyp_init[];
 extern char __kvm_hyp_init_end[];
 
-extern char __kvm_hyp_vector[];
+extern struct kvm_ksym __kvm_hyp_vector;
 
-#define      __kvm_hyp_code_start    __hyp_text_start
-#define      __kvm_hyp_code_end      __hyp_text_end
+extern struct kvm_ksym __kvm_hyp_code_start;
+extern struct kvm_ksym __kvm_hyp_code_end;
 
-extern void __kvm_flush_vm_context(void);
-extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
-extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
+extern struct kvm_ksym __kvm_flush_vm_context;
+extern struct kvm_ksym __kvm_tlb_flush_vmid_ipa;
+extern struct kvm_ksym __kvm_tlb_flush_vmid;
 
-extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
+extern struct kvm_ksym __kvm_vcpu_run;
 
-extern u64 __vgic_v3_get_ich_vtr_el2(void);
+extern struct kvm_ksym __hyp_idmap_text_start, __hyp_idmap_text_end;
 
-extern u32 __kvm_get_mdcr_el2(void);
+extern struct kvm_ksym __vgic_v3_get_ich_vtr_el2;
+
+extern struct kvm_ksym __kvm_get_mdcr_el2;
 
 #endif
 

arch/arm64/include/asm/kvm_mmu.h +2 -0

index 61505676d085..0899026a2821 100644
@@ -73,6 +73,8 @@
 
 #define KERN_TO_HYP(kva)     ((unsigned long)kva - PAGE_OFFSET + HYP_PAGE_OFFSET)
 
+#define kvm_ksym_ref(sym)    ((void *)&sym - KIMAGE_VADDR + PAGE_OFFSET)
+
 /*
  * We currently only support a 40bit IPA.
  */

arch/arm64/include/asm/virt.h +0 -4

index 7a5df5252dd7..215ad4649dd7 100644
@@ -50,10 +50,6 @@ static inline bool is_hyp_mode_mismatched(void)
      return __boot_cpu_mode[0] != __boot_cpu_mode[1];
 }
 
-/* The section containing the hypervisor text */
-extern char __hyp_text_start[];
-extern char __hyp_text_end[];
-
 #endif /* __ASSEMBLY__ */
 

Quoted 17 lines

Quoted 6 lines

arch/arm64/include/asm/virt.h +0 -0

 #endif /* ! __ASM__VIRT_H */

arch/arm64/kernel/vmlinux.lds.S +0 -0

index 363c2f529951..f935f082188d 100644

arch/arm64/kernel/vmlinux.lds.S jiffies = jiffies_64; · line 40

Quoted 8 lines

arch/arm64/kernel/vmlinux.lds.S +2 -2

@@ -35,9 +35,9 @@ jiffies = jiffies_64;
      VMLINUX_SYMBOL(__hyp_idmap_text_start) = .;     \
      *(.hyp.idmap.text)                              \
      VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;       \
-     VMLINUX_SYMBOL(__hyp_text_start) = .;           \
+     VMLINUX_SYMBOL(__kvm_hyp_code_start) = .;       \
      *(.hyp.text)                                    \
-     VMLINUX_SYMBOL(__hyp_text_end) = .;
+     VMLINUX_SYMBOL(__kvm_hyp_code_end) = .;

why this rename?

I already got rid of it based on Marc's feedback. The only reason was to align between ARM and arm64, but he is already doing the same in the opposite direction

This relaxes the kernel Image placement requirements, so that it may be placed at any 2 MB aligned offset in physical memory.

This is accomplished by ignoring PHYS_OFFSET when installing memblocks, and accounting for the apparent virtual offset of the kernel Image. As a result, virtual address references below PAGE_OFFSET are correctly mapped onto physical references into the kernel Image regardless of where it sits in memory.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 Documentation/arm64/booting.txt  | 12 ++---
 arch/arm64/include/asm/boot.h    |  5 ++
 arch/arm64/include/asm/kvm_mmu.h |  2 +-
 arch/arm64/include/asm/memory.h  | 15 +++---
 arch/arm64/kernel/head.S         |  6 ++-
 arch/arm64/mm/init.c             | 50 +++++++++++++++++++-
 arch/arm64/mm/mmu.c              | 12 +++++
 7 files changed, 86 insertions(+), 16 deletions(-)

Documentation/arm64/booting.txt +0 -4

index 701d39d3171a..03e02ebc1b0c 100644
@@ -117,14 +117,14 @@ Header notes:
   depending on selected features, and is effectively unbound.
 
 The Image must be placed text_offset bytes from a 2MB aligned base
-address near the start of usable system RAM and called there. Memory
-below that base address is currently unusable by Linux, and therefore it
-is strongly recommended that this location is the start of system RAM.
-The region between the 2 MB aligned base address and the start of the

arch/arm64/include/asm/boot.h +5 -0

+static void __init enforce_memory_limit(void)
+{
+	const phys_addr_t kbase = round_down(__pa(_text), MIN_KIMG_ALIGN);
+	u64 to_remove = memblock_phys_mem_size() - memory_limit;
+	phys_addr_t max_addr = 0;

Hi,

Quoted 32 lines · Ard Biesheuvel

This relaxes the kernel Image placement requirements, so that it may be placed at any 2 MB aligned offset in physical memory.

This is accomplished by ignoring PHYS_OFFSET when installing memblocks, and accounting for the apparent virtual offset of the kernel Image. As a result, virtual address references below PAGE_OFFSET are correctly mapped onto physical references into the kernel Image regardless of where it sits in memory.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 Documentation/arm64/booting.txt  | 12 ++---
 arch/arm64/include/asm/boot.h    |  5 ++
 arch/arm64/include/asm/kvm_mmu.h |  2 +-
 arch/arm64/include/asm/memory.h  | 15 +++---
 arch/arm64/kernel/head.S         |  6 ++-
 arch/arm64/mm/init.c             | 50 +++++++++++++++++++-
 arch/arm64/mm/mmu.c              | 12 +++++
 7 files changed, 86 insertions(+), 16 deletions(-)

Documentation/arm64/booting.txt +0 -4

index 701d39d3171a..03e02ebc1b0c 100644
@@ -117,14 +117,14 @@ Header notes:
   depending on selected features, and is effectively unbound.
 
 The Image must be placed text_offset bytes from a 2MB aligned base
-address near the start of usable system RAM and called there. Memory
-below that base address is currently unusable by Linux, and therefore it
-is strongly recommended that this location is the start of system RAM.
-The region between the 2 MB aligned base address and the start of the

Documentation/arm64/booting.txt Header notes: · line 127

Quoted 10 lines

Documentation/arm64/booting.txt +6 -2

-image has no special significance to the kernel, and may be used for
-other purposes.
+address anywhere in usable system RAM and called there. The region
+between the 2 MB aligned base address and the start of the image has no
+special significance to the kernel, and may be used for other purposes.
 At least image_size bytes from the start of the image must be free for
 use by the kernel.
+NOTE: versions prior to v4.6 cannot make use of memory below the
+physical offset of the Image so it is recommended that the Image be
+placed as close as possible to the start of system RAM.

We need a head flag for this so that a bootloader can determine whether it can load the kernel anywhere or should try for the lowest possible address. Then the note would describe the recommended behaviour in the absence of the flag.

The flag for KASLR isn't sufficient as you can build without it (and it only tells the bootloader that the kernel accepts entropy in x1).

We might also want to consider if we need to determine whether or not the bootloader actually provided entropy, (and if we need a more general handshake between the bootlaoder and kernel to determine that kind of thing).

Quoted 10 lines

Documentation/arm64/booting.txt +0 -0

 Any memory described to the kernel (even that below the start of the
 image) which is not marked as reserved from the kernel (e.g., with a

arch/arm64/include/asm/boot.h +0 -0

index 81151b67b26b..984cb0fa61ce 100644
@@ -11,4 +11,9 @@
 #define MIN_FDT_ALIGN                8
 #define MAX_FDT_SIZE         SZ_2M
 

arch/arm64/include/asm/boot.h line 15

Quoted 2 lines

arch/arm64/include/asm/boot.h +2 -0

+/*
+ * arm64 requires the kernel image to be 2 MB aligned

Nit: The image is TEXT_OFFSET from that 2M-aligned base. s/image/mapping/?

[...]

Quoted 5 lines

arch/arm64/include/asm/boot.h +5 -0

+static void __init enforce_memory_limit(void)
+{
+     const phys_addr_t kbase = round_down(__pa(_text), MIN_KIMG_ALIGN);
+     u64 to_remove = memblock_phys_mem_size() - memory_limit;
+     phys_addr_t max_addr = 0;

arch/arm64/include/asm/boot.h

Quoted 10 lines

arch/arm64/include/asm/boot.h +10 -0

+     struct memblock_region *r;
+
+     if (memory_limit == (phys_addr_t)ULLONG_MAX)
+             return;
+
+     /*
+      * The kernel may be high up in physical memory, so try to apply the
+      * limit below the kernel first, and only let the generic handling
+      * take over if it turns out we haven't clipped enough memory yet.
+      */

We might want ot preserve the low 4GB if possible, for those IOMMU-less devices which can only do 32-bit addressing.

Otherwise this looks good to me!

Quoted 4 lines · Mark Rutland

We might also want to consider if we need to determine whether or not the bootloader actually provided entropy, (and if we need a more general handshake between the bootlaoder and kernel to determine that kind of thing).

Yes, that is interesting. We should also think about how to handle 'nokaslr' if it appears on the command line, since in the !EFI case, we will be way too late to parse this, and a capable kernel will already be running from a randomized offset. That means it is the bootloader's responsibility to ensure that the presence of 'nokaslr' and the entropy in x1 are consistent with each other.

Quoted 16 lines

arch/arm64/include/asm/boot.h line 15

Quoted 12 lines

Any memory described to the kernel (even that below the start of the image) which is not marked as reserved from the kernel (e.g., with a

arch/arm64/include/asm/boot.h +2 -0

index 81151b67b26b..984cb0fa61ce 100644
@@ -11,4 +11,9 @@
 #define MIN_FDT_ALIGN                8
 #define MAX_FDT_SIZE         SZ_2M
 
+/*
+ * arm64 requires the kernel image to be 2 MB aligned

Nit: The image is TEXT_OFFSET from that 2M-aligned base. s/image/mapping/?

Quoted 2 lines

[...]

Yep. I hate TEXT_OFFSET, did I mention that?

arch/arm64/mm/init.c +26 -0

+static void __init enforce_memory_limit(void)
+{
+	const phys_addr_t kbase = round_down(__pa(_text), MIN_KIMG_ALIGN);
+	u64 to_remove = memblock_phys_mem_size() - memory_limit;
+	phys_addr_t max_addr = 0;
+	struct memblock_region *r;
+
+	if (memory_limit == (phys_addr_t)ULLONG_MAX)
+		return;
+
+	/*
+	 * The kernel may be high up in physical memory, so try to apply the
+	 * limit below the kernel first, and only let the generic handling
+	 * take over if it turns out we haven't clipped enough memory yet.
+	 */
+	for_each_memblock(memory, r) {
+		if (r->base + r->size > kbase) {
+			u64 rem = min(to_remove, kbase - r->base);
+
+			max_addr = r->base + rem;
+			to_remove -= rem;
+			break;
+		}
+		if (to_remove <= r->size) {
+			max_addr = r->base + to_remove;
+			to_remove = 0;

Quoted 26 lines · Ard Biesheuvel

arch/arm64/mm/init.c +26 -0

+static void __init enforce_memory_limit(void)
+{
+     const phys_addr_t kbase = round_down(__pa(_text), MIN_KIMG_ALIGN);
+     u64 to_remove = memblock_phys_mem_size() - memory_limit;
+     phys_addr_t max_addr = 0;
+     struct memblock_region *r;
+
+     if (memory_limit == (phys_addr_t)ULLONG_MAX)
+             return;
+
+     /*
+      * The kernel may be high up in physical memory, so try to apply the
+      * limit below the kernel first, and only let the generic handling
+      * take over if it turns out we haven't clipped enough memory yet.
+      */
+     for_each_memblock(memory, r) {
+             if (r->base + r->size > kbase) {
+                     u64 rem = min(to_remove, kbase - r->base);
+
+                     max_addr = r->base + rem;
+                     to_remove -= rem;
+                     break;
+             }
+             if (to_remove <= r->size) {
+                     max_addr = r->base + to_remove;
+                     to_remove = 0;

arch/arm64/mm/init.c static int __init early_mem(char *p) · line 197

Quoted 10 lines

arch/arm64/mm/init.c +10 -0

+                     break;
+             }
+             to_remove -= r->size;
+     }
+
+     memblock_remove(0, max_addr);
+
+     if (to_remove)
+             memblock_enforce_memory_limit(memory_limit);
+}

IIUC, this is changing the user expectations a bit. There are people using the mem= limit to hijack some top of the RAM for other needs (though they could do it in a saner way like changing the DT memory nodes). Your patch first tries to remove the memory below the kernel image and only remove the top if additional limitation is necessary.

Can you not remove memory from the top and block the limit if it goes below the end of the kernel image, with some warning that memory limit was not entirely fulfilled?

Quoted 33 lines · Ard Biesheuvel

arch/arm64/mm/init.c +33 -0

+static void __init enforce_memory_limit(void)
+{
+	const phys_addr_t kbase = round_down(__pa(_text), MIN_KIMG_ALIGN);
+	u64 to_remove = memblock_phys_mem_size() - memory_limit;
+	phys_addr_t max_addr = 0;
+	struct memblock_region *r;
+
+	if (memory_limit == (phys_addr_t)ULLONG_MAX)
+		return;
+
+	/*
+	 * The kernel may be high up in physical memory, so try to apply the
+	 * limit below the kernel first, and only let the generic handling
+	 * take over if it turns out we haven't clipped enough memory yet.
+	 */
+	for_each_memblock(memory, r) {
+		if (r->base + r->size > kbase) {
+			u64 rem = min(to_remove, kbase - r->base);
+
+			max_addr = r->base + rem;
+			to_remove -= rem;
+			break;
+		}
+		if (to_remove <= r->size) {
+			max_addr = r->base + to_remove;
+			to_remove = 0;
+			break;
+		}
+		to_remove -= r->size;
+	}
+
+	memblock_remove(0, max_addr);
+

arch/arm64/mm/init.c static int __init early_mem(char *p) · line 197

Quoted 3 lines

arch/arm64/mm/init.c +3 -0

+	if (to_remove)
+		memblock_enforce_memory_limit(memory_limit);
+}

IIUC, this is changing the user expectations a bit. There are people using the mem= limit to hijack some top of the RAM for other needs (though they could do it in a saner way like changing the DT memory nodes).

Quoted 34 lines · Catalin Marinas

Quoted 33 lines · Ard Biesheuvel

arch/arm64/mm/init.c +33 -0

+static void __init enforce_memory_limit(void)
+{
+	const phys_addr_t kbase = round_down(__pa(_text), MIN_KIMG_ALIGN);
+	u64 to_remove = memblock_phys_mem_size() - memory_limit;
+	phys_addr_t max_addr = 0;
+	struct memblock_region *r;
+
+	if (memory_limit == (phys_addr_t)ULLONG_MAX)
+		return;
+
+	/*
+	 * The kernel may be high up in physical memory, so try to apply the
+	 * limit below the kernel first, and only let the generic handling
+	 * take over if it turns out we haven't clipped enough memory yet.
+	 */
+	for_each_memblock(memory, r) {
+		if (r->base + r->size > kbase) {
+			u64 rem = min(to_remove, kbase - r->base);
+
+			max_addr = r->base + rem;
+			to_remove -= rem;
+			break;
+		}
+		if (to_remove <= r->size) {
+			max_addr = r->base + to_remove;
+			to_remove = 0;
+			break;
+		}
+		to_remove -= r->size;
+	}
+
+	memblock_remove(0, max_addr);
+

Quoted 8 lines

arch/arm64/mm/init.c static int __init early_mem(char *p) · line 197

Quoted 3 lines

arch/arm64/mm/init.c +3 -0

+	if (to_remove)
+		memblock_enforce_memory_limit(memory_limit);
+}

IIUC, this is changing the user expectations a bit. There are people using the mem= limit to hijack some top of the RAM for other needs (though they could do it in a saner way like changing the DT memory nodes).