arm64: stable UEFI mappings for kexec

For UEFI, we need to install the memory mappings used for Runtime Services in a dedicated set of page tables. Add create_pgd_mapping(), which allows us to allocate and install those page table entries early.

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

---
 arch/arm64/include/asm/mmu.h     |  3 +++
 arch/arm64/include/asm/pgtable.h |  5 +++++
 arch/arm64/mm/mmu.c              | 43 ++++++++++++++++++++--------------------
 3 files changed, 30 insertions(+), 21 deletions(-)

arch/arm64/include/asm/mmu.h +3 -0

index c2f006c48bdb..5fd40c43be80 100644
@@ -33,5 +33,8 @@ extern void __iomem *early_io_map(phys_addr_t phys, unsigned long virt);
 extern void init_mem_pgprot(void);
 /* create an identity mapping for memory (or io if map_io is true) */
 extern void create_id_mapping(phys_addr_t addr, phys_addr_t size, int map_io);
+extern void create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
+			       unsigned long virt, phys_addr_t size,
+			       pgprot_t prot);
 
 #endif

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

index 41a43bf26492..1abe4d08725b 100644
@@ -264,6 +264,11 @@ static inline pmd_t pte_pmd(pte_t pte)
 	return __pmd(pte_val(pte));
 }
 
+static inline pgprot_t mk_sect_prot(pgprot_t prot)
+{
+	return __pgprot(pgprot_val(prot) & ~PTE_TABLE_BIT);
+}
+
 /*
  * THP definitions.
  */

arch/arm64/mm/mmu.c +22 -21

index 83e6713143a3..4d583aa9ff4e 100644
@@ -157,20 +157,10 @@ static void __init alloc_init_pte(pmd_t *pmd, unsigned long addr,
 
 static void __init alloc_init_pmd(struct mm_struct *mm, pud_t *pud,
 				  unsigned long addr, unsigned long end,
-				  phys_addr_t phys, int map_io)
+				  phys_addr_t phys, pgprot_t prot)
 {
 	pmd_t *pmd;
 	unsigned long next;
-	pmdval_t prot_sect;
-	pgprot_t prot_pte;
-
-	if (map_io) {
-		prot_sect = PROT_SECT_DEVICE_nGnRE;
-		prot_pte = __pgprot(PROT_DEVICE_nGnRE);
-	} else {
-		prot_sect = PROT_SECT_NORMAL_EXEC;
-		prot_pte = PAGE_KERNEL_EXEC;
-	}
 
 	/*
 	 * Check for initial section mappings in the pgd/pud and remove them.

@@ -186,7 +176,8 @@ static void __init alloc_init_pmd(struct mm_struct *mm, pud_t *pud,
 		/* try section mapping first */
 		if (((addr | next | phys) & ~SECTION_MASK) == 0) {
 			pmd_t old_pmd =*pmd;
-			set_pmd(pmd, __pmd(phys | prot_sect));
+			set_pmd(pmd, __pmd(phys |
+					   pgprot_val(mk_sect_prot(prot))));
 			/*
 			 * Check for previous table entries created during
 			 * boot (__create_page_tables) and flush them.

@@ -195,7 +186,7 @@ static void __init alloc_init_pmd(struct mm_struct *mm, pud_t *pud,
 				flush_tlb_all();
 		} else {
 			alloc_init_pte(pmd, addr, next, __phys_to_pfn(phys),
-				       prot_pte);
+				       prot);
 		}
 		phys += next - addr;
 	} while (pmd++, addr = next, addr != end);

@@ -203,7 +194,7 @@ static void __init alloc_init_pmd(struct mm_struct *mm, pud_t *pud,
 
 static void __init alloc_init_pud(struct mm_struct *mm, pgd_t *pgd,
 				  unsigned long addr, unsigned long end,
-				  unsigned long phys, int map_io)
+				  unsigned long phys, pgprot_t prot)
 {
 	pud_t *pud;
 	unsigned long next;

@@ -221,10 +212,11 @@ static void __init alloc_init_pud(struct mm_struct *mm, pgd_t *pgd,
 		/*
 		 * For 4K granule only, attempt to put down a 1GB block
 		 */
-		if (!map_io && (PAGE_SHIFT == 12) &&
+		if ((PAGE_SHIFT == 12) &&
 		    ((addr | next | phys) & ~PUD_MASK) == 0) {
 			pud_t old_pud = *pud;
-			set_pud(pud, __pud(phys | PROT_SECT_NORMAL_EXEC));
+			set_pud(pud, __pud(phys |
+					   pgprot_val(mk_sect_prot(prot))));
 
 			/*
 			 * If we have an old value for a pud, it will

@@ -239,7 +231,7 @@ static void __init alloc_init_pud(struct mm_struct *mm, pgd_t *pgd,
 				flush_tlb_all();
 			}
 		} else {
-			alloc_init_pmd(mm, pud, addr, next, phys, map_io);
+			alloc_init_pmd(mm, pud, addr, next, phys, prot);
 		}
 		phys += next - addr;
 	} while (pud++, addr = next, addr != end);

@@ -251,7 +243,7 @@ static void __init alloc_init_pud(struct mm_struct *mm, pgd_t *pgd,
  */
 static void __init __create_mapping(struct mm_struct *mm, pgd_t *pgd,
 				    phys_addr_t phys, unsigned long virt,
-				    phys_addr_t size, int map_io)
+				    phys_addr_t size, pgprot_t prot)
 {
 	unsigned long addr, length, end, next;
 

@@ -261,7 +253,7 @@ static void __init __create_mapping(struct mm_struct *mm, pgd_t *pgd,
 	end = addr + length;
 	do {
 		next = pgd_addr_end(addr, end);
-		alloc_init_pud(mm, pgd, addr, next, phys, map_io);
+		alloc_init_pud(mm, pgd, addr, next, phys, prot);
 		phys += next - addr;
 	} while (pgd++, addr = next, addr != end);
 }

@@ -275,7 +267,7 @@ static void __init create_mapping(phys_addr_t phys, unsigned long virt,
 		return;
 	}
 	__create_mapping(&init_mm, pgd_offset_k(virt & PAGE_MASK), phys, virt,
-			 size, 0);
+			 size, PAGE_KERNEL_EXEC);
 }
 
 void __init create_id_mapping(phys_addr_t addr, phys_addr_t size, int map_io)

@@ -285,7 +277,16 @@ void __init create_id_mapping(phys_addr_t addr, phys_addr_t size, int map_io)
 		return;
 	}
 	__create_mapping(&init_mm, &idmap_pg_dir[pgd_index(addr)],
-			 addr, addr, size, map_io);
+			 addr, addr, size,
+			 map_io ? __pgprot(PROT_DEVICE_nGnRE)
+				: PAGE_KERNEL_EXEC);
+}
+
+void __init create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
+			       unsigned long virt, phys_addr_t size,
+			       pgprot_t prot)
+{
+	__create_mapping(mm, pgd_offset(mm, virt), phys, virt, size, prot);
 }
 
 static void __init map_mem(void)

Signature · 1 line

1.8.3.2

Improve the handling of /dev/mem mappings under CONFIG_STRICT_DEVMEM by:
- allowing read-only access to parts of System RAM that are not considered memory by the kernel, this is mainly intended for exposing UEFI Configuration tables to userland;
- avoid using non-cached mappings for those parts of System RAM, as it may result in mismatched attributes.

Quoted 6 lines · Ard Biesheuvel

Improve the handling of /dev/mem mappings under CONFIG_STRICT_DEVMEM by:
- allowing read-only access to parts of System RAM that are not considered memory by the kernel, this is mainly intended for exposing UEFI Configuration tables to userland;
- avoid using non-cached mappings for those parts of System RAM, as it may result in mismatched attributes.

Is this really the best way to expose EFI config tables?

We already have parts in /sys/firmware/efi/ and in particular we expose the runtime mappings there for kexec on x86.

Hooking this into the /dev/mem infrastructure just seems wrong to me.

Improve the handling of /dev/mem mappings under CONFIG_STRICT_DEVMEM by:
- allowing read-only access to parts of System RAM that are not considered memory by the kernel, this is mainly intended for exposing UEFI Configuration tables to userland;

- avoid using non-cached mappings for those parts of System RAM, as it may result in mismatched attributes.

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

---
 arch/arm64/mm/mmap.c |  5 +++--
 arch/arm64/mm/mmu.c  | 15 ++++++++++++++-
 2 files changed, 17 insertions(+), 3 deletions(-)

arch/arm64/mm/mmap.c +3 -2

index 1d73662f00ff..802cdf0df921 100644
@@ -127,14 +127,15 @@ int valid_mmap_phys_addr_range(unsigned long pfn, size_t size)
 /*
  * devmem_is_allowed() checks to see if /dev/mem access to a certain address
  * is valid. The argument is a physical page number.  We mimic x86 here by
- * disallowing access to system RAM as well as device-exclusive MMIO regions.
+ * disallowing access to system RAM that is in active use by the kernel, as
+ * well as device-exclusive MMIO regions.
  * This effectively disable read()/write() on /dev/mem.
  */
 int devmem_is_allowed(unsigned long pfn)
 {
 	if (iomem_is_exclusive(pfn << PAGE_SHIFT))
 		return 0;
-	if (!page_is_ram(pfn))
+	if (!pfn_valid(pfn))
 		return 1;
 	return 0;
 }

arch/arm64/mm/mmu.c +14 -1

index 4d583aa9ff4e..93ba10838359 100644
@@ -121,7 +121,7 @@ early_param("cachepolicy", early_cachepolicy);
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 			      unsigned long size, pgprot_t vma_prot)
 {
-	if (!pfn_valid(pfn))
+	if (!page_is_ram(pfn))
 		return pgprot_noncached(vma_prot);
 	else if (file->f_flags & O_SYNC)
 		return pgprot_writecombine(vma_prot);

@@ -129,6 +129,19 @@ pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 }
 EXPORT_SYMBOL(phys_mem_access_prot);
 
+/*
+ * This definition of phys_mem_access_prot_allowed() overrides
+ * the __weak definition in drivers/char/mem.c
+ */
+int phys_mem_access_prot_allowed(struct file *file, unsigned long pfn,
+				 unsigned long size, pgprot_t *prot)
+{
+	/* Disallow read-write access to reserved system RAM */
+	if ((pgprot_val(*prot) & PTE_WRITE) && page_is_ram(pfn))
+		return 0;
+	return 1;
+}
+
 static void __init *early_alloc(unsigned long sz)
 {
 	void *ptr = __va(memblock_alloc(sz, sz));

Signature · 1 line

1.8.3.2

Quoted 4 lines · Ard Biesheuvel

Improve the handling of /dev/mem mappings under CONFIG_STRICT_DEVMEM by:
- allowing read-only access to parts of System RAM that are not considered memory by the kernel, this is mainly intended for exposing UEFI Configuration tables to userland;

Ard, can you elabrate a bit? Are this for the acpi case because no dtb in procfs?

Anyway I also think using /dev/mem looks not a good way to expose infomation to userspace.

Quoted 67 lines

- avoid using non-cached mappings for those parts of System RAM, as it may result in mismatched attributes.

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

---
 arch/arm64/mm/mmap.c |  5 +++--
 arch/arm64/mm/mmu.c  | 15 ++++++++++++++-
 2 files changed, 17 insertions(+), 3 deletions(-)

arch/arm64/mm/mmap.c +3 -2

index 1d73662f00ff..802cdf0df921 100644
@@ -127,14 +127,15 @@ int valid_mmap_phys_addr_range(unsigned long pfn, size_t size)
 /*
  * devmem_is_allowed() checks to see if /dev/mem access to a certain address
  * is valid. The argument is a physical page number.  We mimic x86 here by
- * disallowing access to system RAM as well as device-exclusive MMIO regions.
+ * disallowing access to system RAM that is in active use by the kernel, as
+ * well as device-exclusive MMIO regions.
  * This effectively disable read()/write() on /dev/mem.
  */
 int devmem_is_allowed(unsigned long pfn)
 {
      if (iomem_is_exclusive(pfn << PAGE_SHIFT))
              return 0;
-     if (!page_is_ram(pfn))
+     if (!pfn_valid(pfn))
              return 1;
      return 0;
 }

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

index 4d583aa9ff4e..93ba10838359 100644
@@ -121,7 +121,7 @@ early_param("cachepolicy", early_cachepolicy);
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
                            unsigned long size, pgprot_t vma_prot)
 {
-     if (!pfn_valid(pfn))
+     if (!page_is_ram(pfn))
              return pgprot_noncached(vma_prot);
      else if (file->f_flags & O_SYNC)
              return pgprot_writecombine(vma_prot);

@@ -129,6 +129,19 @@ pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 }
 EXPORT_SYMBOL(phys_mem_access_prot);
 
+/*
+ * This definition of phys_mem_access_prot_allowed() overrides
+ * the __weak definition in drivers/char/mem.c
+ */
+int phys_mem_access_prot_allowed(struct file *file, unsigned long pfn,
+                              unsigned long size, pgprot_t *prot)
+{
+     /* Disallow read-write access to reserved system RAM */
+     if ((pgprot_val(*prot) & PTE_WRITE) && page_is_ram(pfn))
+             return 0;
+     return 1;
+}
+
 static void __init *early_alloc(unsigned long sz)
 {
      void *ptr = __va(memblock_alloc(sz, sz));
--

1.8.3.2

Quoted 9 lines · Dave Young

Quoted 4 lines · Ard Biesheuvel

Improve the handling of /dev/mem mappings under CONFIG_STRICT_DEVMEM by:
- allowing read-only access to parts of System RAM that are not considered memory by the kernel, this is mainly intended for exposing UEFI Configuration tables to userland;

Ard, can you elabrate a bit? Are this for the acpi case because no dtb in procfs?

No, that is a different issue.

Quoted 3 lines

Anyway I also think using /dev/mem looks not a good way to expose infomation to userspace.

This is for compatibility with existing tools like dmidecode and lshw that already use /dev/mem.
However, on x86, the tables they access are in iomem resource ranges that are accessible under CONFIG_STRICT_DEVMEM, whereas on arm64, those regions are inaccessible.

So while our position is that using /dev/mem for *anything* is an awful idea, this particular patch just tries to bring arm64 in line with what currently existing tools expect.

Split of the remapping code from efi_config_init() so that the caller can perform its own remapping. This is necessary to correctly handle virtually remapped UEFI memory regions under kexec, as efi.systab will have been updated to a virtual address.

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

---
 drivers/firmware/efi/efi.c | 49 +++++++++++++++++++++++++++++-----------------
 include/linux/efi.h        |  2 ++
 2 files changed, 33 insertions(+), 18 deletions(-)

Quoted 10 lines · Ard Biesheuvel

Split of the remapping code from efi_config_init() so that the caller can perform its own remapping. This is necessary to correctly handle virtually remapped UEFI memory regions under kexec, as efi.systab will have been updated to a virtual address.

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

---
 drivers/firmware/efi/efi.c | 49 +++++++++++++++++++++++++++++-----------------
 include/linux/efi.h        |  2 ++
 2 files changed, 33 insertions(+), 18 deletions(-)

Looks straight forward.

What tree are these going through?

drivers/firmware/efi/virtmap.c +1 -0

index 98735fb43581..4b6a5c31629f 100644
@@ -8,6 +8,7 @@
  */
 
 #include <linux/efi.h>
+#include <linux/memblock.h>
 #include <linux/mm_types.h>
 #include <linux/rbtree.h>
 #include <linux/rwsem.h>

@@ -97,8 +98,15 @@ void __init efi_virtmap_init(void)
                u64 paddr, npages, size;
                pgprot_t prot;
 

Quoted 17 lines · Ard Biesheuvel

drivers/firmware/efi/virtmap.c +1 -0

index 98735fb43581..4b6a5c31629f 100644
@@ -8,6 +8,7 @@
  */
 
 #include <linux/efi.h>
+#include <linux/memblock.h>
 #include <linux/mm_types.h>
 #include <linux/rbtree.h>
 #include <linux/rwsem.h>

@@ -97,8 +98,15 @@ void __init efi_virtmap_init(void)
                u64 paddr, npages, size;
                pgprot_t prot;
 

drivers/firmware/efi/virtmap.c void __init efi_virtmap_init(void) · line 109

Quoted 10 lines

drivers/firmware/efi/virtmap.c +8 -1

-               if (!efi_mem_is_usable_region(md))
+               paddr = md->phys_addr;
+               npages = md->num_pages;
+               memrange_efi_to_native(&paddr, &npages);
+               size = npages << PAGE_SHIFT;
+
+               if (!efi_mem_is_usable_region(md)) {
                        efi_register_mem_resource(md);
+                       memblock_remove(paddr, size);
+               }

What exactly is the memblock_remove trying to accomplish? With it, I get:

 Unable to handle kernel NULL pointer dereference at virtual address 00000008
 pgd = fffffe03d5890000
 [00000008] *pgd=00000043d5ae0003, *pud=00000043d5ae0003, *pmd=00000043d5ae0003, *pte=0000000000000000
 Internal error: Oops: 96000007 [#1] SMP
 Modules linked in:
 CPU: 0 PID: 107 Comm: modprobe Not tainted 3.18.0-rc5+ #8
 Hardware name: APM X-Gene Mustang board (DT)
 task: fffffe03d5770000 ti: fffffe03dd300000 task.ti: fffffe03dd300000
 PC is at mem_cgroup_begin_page_stat+0x40/0x110
 LR is at mem_cgroup_begin_page_stat+0x40/0x110
 pc : [<fffffe00001ec34c>] lr : [<fffffe00001ec34c>] pstate: 60000145
 sp : fffffe03dd303a80
 x29: fffffe03dd303a80 x28: 000000000000ffe0
 x27: fffffdfef0f56bc0 x26: 0000000000000000
 x25: 000003ff89b80000 x24: 0000000000000000
 x23: fffffe03dd303ae8 x22: fffffe03dd303ae7
 x21: 0000000000000000 x20: fffffe03d58a0420
 x19: fffffdfef0004500 x18: 0000000000000000
 x17: 000003ff89ba40dc x16: 000003ff89bbff90
 x15: 000003ff89ba3b44 x14: 31313030343d6e66
 x13: 7020303035343030 x12: 0000000000000006
 x11: 0000000000000000 x10: 0000000000000186
 x9 : 0000000000000187 x8 : 3030306665666466
 x7 : fffffe0001041c18 x6 : 00000000000000ff
 x5 : fffffe00004aaf20 x4 : fffffe00004b209c
 x3 : 0000000000000000 x2 : 0000000000000000
 x1 : fffffe0000f6a5d0 x0 : 0000000000000000

Process modprobe (pid: 107, stack limit = 0xfffffe03dd300058)

 Stack: (0xfffffe03dd303a80 to 0xfffffe03dd304000)
 3a80: dd303ac0 fffffe03 001c3e20 fffffe00 f0004500 fffffdfe d58a0420 fffffe03
 3aa0: f0004500 fffffdfe d58f4dc0 fffffe03 00000000 00000000 001b62d4 fffffe00
 3ac0: dd303af0 fffffe03 001b9984 fffffe00 01140f53 00200040 d58a0420 fffffe03
 3ae0: d5880000 fffffe03 d58a0420 fffffe03 dd303b40 fffffe03 001b9c4c fffffe00
 3b00: d58a0420 fffffe03 89b80000 000003ff f0f56bf0 fffffdfe 00000000 00000000
 ...
 3fa0: 89b907b7 000003ff 89bbf000 000003ff 89bc16e8 000003ff eb753490 000003ff
 3fc0: 89b91f9c 000003ff eb753490 000003ff 89b91fac 000003ff 20000000 00000000
 3fe0: 00000000 00000000 ffffffff ffffffff aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa
 Call trace:
 [<fffffe00001ec34c>] mem_cgroup_begin_page_stat+0x40/0x110
 [<fffffe00001c3e1c>] page_add_file_rmap+0x24/0x80
 [<fffffe00001b9980>] do_set_pte+0x108/0x19c
 [<fffffe00001b9c48>] do_read_fault.isra.93+0x234/0x28c
 [<fffffe00001ba57c>] handle_mm_fault+0x488/0xd64
 [<fffffe00000a0ea0>] do_page_fault+0x224/0x358
 [<fffffe0000090128>] do_mem_abort+0x4c/0xb0
 Exception stack(0xfffffe03dd303e30 to 0xfffffe03dd303f50)
 3e20:                                     00000000 00000000 89bbf000 000003ff
 3e40: ffffffff ffffffff 89b91fac 000003ff 01043280 fffffe00 00000002 00000000
 3e60: dd303e70 fffffe03 008ce3a0 fffffe00 dd303e90 fffffe03 001c0400 fffffe00
 3e80: d5880000 fffffe03 000cbc60 fffffe00 eb753610 000003ff 00092fdc fffffe00
 3ea0: 00000000 00000000 89bbf000 000003ff ffffffff ffffffff 89ba4654 000003ff
 3ec0: 80000000 00000000 d5521480 fffffe03 89bc16e8 000003ff 89bc1a40 000003ff
 3ee0: 00000004 00000000 89b80000 000003ff fefefeff fefefefe 00000000 00000000
 3f00: 0000000f 00000000 89bc1028 000003ff 89ba7000 000003ff 0000079c 00000000
 3f20: 00400000 00000000 ffff0000 ffffffff 004038a0 00000000 00010000 00000000
 3f40: 89ba3c1c 000003ff 89ba3b44 000003ff
 Code: b9405835 35000275 aa1303e0 9400065c (f9400413)

Quoted 18 lines · Mark Salter

Quoted 17 lines · Ard Biesheuvel

drivers/firmware/efi/virtmap.c +1 -0

index 98735fb43581..4b6a5c31629f 100644
@@ -8,6 +8,7 @@
  */
 
 #include <linux/efi.h>
+#include <linux/memblock.h>
 #include <linux/mm_types.h>
 #include <linux/rbtree.h>
 #include <linux/rwsem.h>

@@ -97,8 +98,15 @@ void __init efi_virtmap_init(void)
                u64 paddr, npages, size;
                pgprot_t prot;
 

Quoted 14 lines

drivers/firmware/efi/virtmap.c void __init efi_virtmap_init(void) · line 109

Quoted 10 lines

drivers/firmware/efi/virtmap.c +8 -1

-               if (!efi_mem_is_usable_region(md))
+               paddr = md->phys_addr;
+               npages = md->num_pages;
+               memrange_efi_to_native(&paddr, &npages);
+               size = npages << PAGE_SHIFT;
+
+               if (!efi_mem_is_usable_region(md)) {
                        efi_register_mem_resource(md);
+                       memblock_remove(paddr, size);
+               }

What exactly is the memblock_remove trying to accomplish? With it, I get:

The idea was pfn_valid() will then return false for those pfns, allowing us to distinguish them from memory that the kernel may be using, primarily for /dev/mem filtering. But apparently it is causing problems for you, so I will have to figure out if there is a better way of addressing this.

Quoted 62 lines

 Unable to handle kernel NULL pointer dereference at virtual address 00000008
 pgd = fffffe03d5890000
 [00000008] *pgd=00000043d5ae0003, *pud=00000043d5ae0003, *pmd=00000043d5ae0003, *pte=0000000000000000
 Internal error: Oops: 96000007 [#1] SMP
 Modules linked in:
 CPU: 0 PID: 107 Comm: modprobe Not tainted 3.18.0-rc5+ #8
 Hardware name: APM X-Gene Mustang board (DT)
 task: fffffe03d5770000 ti: fffffe03dd300000 task.ti: fffffe03dd300000
 PC is at mem_cgroup_begin_page_stat+0x40/0x110
 LR is at mem_cgroup_begin_page_stat+0x40/0x110
 pc : [<fffffe00001ec34c>] lr : [<fffffe00001ec34c>] pstate: 60000145
 sp : fffffe03dd303a80
 x29: fffffe03dd303a80 x28: 000000000000ffe0
 x27: fffffdfef0f56bc0 x26: 0000000000000000
 x25: 000003ff89b80000 x24: 0000000000000000
 x23: fffffe03dd303ae8 x22: fffffe03dd303ae7
 x21: 0000000000000000 x20: fffffe03d58a0420
 x19: fffffdfef0004500 x18: 0000000000000000
 x17: 000003ff89ba40dc x16: 000003ff89bbff90
 x15: 000003ff89ba3b44 x14: 31313030343d6e66
 x13: 7020303035343030 x12: 0000000000000006
 x11: 0000000000000000 x10: 0000000000000186
 x9 : 0000000000000187 x8 : 3030306665666466
 x7 : fffffe0001041c18 x6 : 00000000000000ff
 x5 : fffffe00004aaf20 x4 : fffffe00004b209c
 x3 : 0000000000000000 x2 : 0000000000000000
 x1 : fffffe0000f6a5d0 x0 : 0000000000000000

Process modprobe (pid: 107, stack limit = 0xfffffe03dd300058)

 Stack: (0xfffffe03dd303a80 to 0xfffffe03dd304000)
 3a80: dd303ac0 fffffe03 001c3e20 fffffe00 f0004500 fffffdfe d58a0420 fffffe03
 3aa0: f0004500 fffffdfe d58f4dc0 fffffe03 00000000 00000000 001b62d4 fffffe00
 3ac0: dd303af0 fffffe03 001b9984 fffffe00 01140f53 00200040 d58a0420 fffffe03
 3ae0: d5880000 fffffe03 d58a0420 fffffe03 dd303b40 fffffe03 001b9c4c fffffe00
 3b00: d58a0420 fffffe03 89b80000 000003ff f0f56bf0 fffffdfe 00000000 00000000
 ...
 3fa0: 89b907b7 000003ff 89bbf000 000003ff 89bc16e8 000003ff eb753490 000003ff
 3fc0: 89b91f9c 000003ff eb753490 000003ff 89b91fac 000003ff 20000000 00000000
 3fe0: 00000000 00000000 ffffffff ffffffff aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa
 Call trace:
 [<fffffe00001ec34c>] mem_cgroup_begin_page_stat+0x40/0x110
 [<fffffe00001c3e1c>] page_add_file_rmap+0x24/0x80
 [<fffffe00001b9980>] do_set_pte+0x108/0x19c
 [<fffffe00001b9c48>] do_read_fault.isra.93+0x234/0x28c
 [<fffffe00001ba57c>] handle_mm_fault+0x488/0xd64
 [<fffffe00000a0ea0>] do_page_fault+0x224/0x358
 [<fffffe0000090128>] do_mem_abort+0x4c/0xb0
 Exception stack(0xfffffe03dd303e30 to 0xfffffe03dd303f50)
 3e20:                                     00000000 00000000 89bbf000 000003ff
 3e40: ffffffff ffffffff 89b91fac 000003ff 01043280 fffffe00 00000002 00000000
 3e60: dd303e70 fffffe03 008ce3a0 fffffe00 dd303e90 fffffe03 001c0400 fffffe00
 3e80: d5880000 fffffe03 000cbc60 fffffe00 eb753610 000003ff 00092fdc fffffe00
 3ea0: 00000000 00000000 89bbf000 000003ff ffffffff ffffffff 89ba4654 000003ff
 3ec0: 80000000 00000000 d5521480 fffffe03 89bc16e8 000003ff 89bc1a40 000003ff
 3ee0: 00000004 00000000 89b80000 000003ff fefefeff fefefefe 00000000 00000000
 3f00: 0000000f 00000000 89bc1028 000003ff 89ba7000 000003ff 0000079c 00000000
 3f20: 00400000 00000000 ffff0000 ffffffff 004038a0 00000000 00010000 00000000
 3f40: 89ba3c1c 000003ff 89ba3b44 000003ff
 Code: b9405835 35000275 aa1303e0 9400065c (f9400413)

Quoted 34 lines · Ard Biesheuvel

Quoted 18 lines · Mark Salter

Quoted 17 lines · Ard Biesheuvel

drivers/firmware/efi/virtmap.c +1 -0

index 98735fb43581..4b6a5c31629f 100644
@@ -8,6 +8,7 @@
  */
 
 #include <linux/efi.h>
+#include <linux/memblock.h>
 #include <linux/mm_types.h>
 #include <linux/rbtree.h>
 #include <linux/rwsem.h>

@@ -97,8 +98,15 @@ void __init efi_virtmap_init(void)
                u64 paddr, npages, size;
                pgprot_t prot;
 

Quoted 14 lines

drivers/firmware/efi/virtmap.c void __init efi_virtmap_init(void) · line 109

Quoted 10 lines

drivers/firmware/efi/virtmap.c +8 -1

-               if (!efi_mem_is_usable_region(md))
+               paddr = md->phys_addr;
+               npages = md->num_pages;
+               memrange_efi_to_native(&paddr, &npages);
+               size = npages << PAGE_SHIFT;
+
+               if (!efi_mem_is_usable_region(md)) {
                        efi_register_mem_resource(md);
+                       memblock_remove(paddr, size);
+               }

What exactly is the memblock_remove trying to accomplish? With it, I get:

Quoted 6 lines

The idea was pfn_valid() will then return false for those pfns, allowing us to distinguish them from memory that the kernel may be using, primarily for /dev/mem filtering. But apparently it is causing problems for you, so I will have to figure out if there is a better way of addressing this.

Okay. Well I think that works for pfn_valid, but it is confusing the mem_cgroup code. I think because you end up with multiple memory regions after creating the holes. Earlier memory management code saw one memory region. And because this comes after paging_init(), all of the memory ends up in the kernel linear mapping which is something we were trying to avoid.

Quoted 62 lines

Quoted 62 lines

 Unable to handle kernel NULL pointer dereference at virtual address 00000008
 pgd = fffffe03d5890000
 [00000008] *pgd=00000043d5ae0003, *pud=00000043d5ae0003, *pmd=00000043d5ae0003, *pte=0000000000000000
 Internal error: Oops: 96000007 [#1] SMP
 Modules linked in:
 CPU: 0 PID: 107 Comm: modprobe Not tainted 3.18.0-rc5+ #8
 Hardware name: APM X-Gene Mustang board (DT)
 task: fffffe03d5770000 ti: fffffe03dd300000 task.ti: fffffe03dd300000
 PC is at mem_cgroup_begin_page_stat+0x40/0x110
 LR is at mem_cgroup_begin_page_stat+0x40/0x110
 pc : [<fffffe00001ec34c>] lr : [<fffffe00001ec34c>] pstate: 60000145
 sp : fffffe03dd303a80
 x29: fffffe03dd303a80 x28: 000000000000ffe0
 x27: fffffdfef0f56bc0 x26: 0000000000000000
 x25: 000003ff89b80000 x24: 0000000000000000
 x23: fffffe03dd303ae8 x22: fffffe03dd303ae7
 x21: 0000000000000000 x20: fffffe03d58a0420
 x19: fffffdfef0004500 x18: 0000000000000000
 x17: 000003ff89ba40dc x16: 000003ff89bbff90
 x15: 000003ff89ba3b44 x14: 31313030343d6e66
 x13: 7020303035343030 x12: 0000000000000006
 x11: 0000000000000000 x10: 0000000000000186
 x9 : 0000000000000187 x8 : 3030306665666466
 x7 : fffffe0001041c18 x6 : 00000000000000ff
 x5 : fffffe00004aaf20 x4 : fffffe00004b209c
 x3 : 0000000000000000 x2 : 0000000000000000
 x1 : fffffe0000f6a5d0 x0 : 0000000000000000

Process modprobe (pid: 107, stack limit = 0xfffffe03dd300058)

 Stack: (0xfffffe03dd303a80 to 0xfffffe03dd304000)
 3a80: dd303ac0 fffffe03 001c3e20 fffffe00 f0004500 fffffdfe d58a0420 fffffe03
 3aa0: f0004500 fffffdfe d58f4dc0 fffffe03 00000000 00000000 001b62d4 fffffe00
 3ac0: dd303af0 fffffe03 001b9984 fffffe00 01140f53 00200040 d58a0420 fffffe03
 3ae0: d5880000 fffffe03 d58a0420 fffffe03 dd303b40 fffffe03 001b9c4c fffffe00
 3b00: d58a0420 fffffe03 89b80000 000003ff f0f56bf0 fffffdfe 00000000 00000000
 ...
 3fa0: 89b907b7 000003ff 89bbf000 000003ff 89bc16e8 000003ff eb753490 000003ff
 3fc0: 89b91f9c 000003ff eb753490 000003ff 89b91fac 000003ff 20000000 00000000
 3fe0: 00000000 00000000 ffffffff ffffffff aaaaaaaa aaaaaaaa aaaaaaaa aaaaaaaa
 Call trace:
 [<fffffe00001ec34c>] mem_cgroup_begin_page_stat+0x40/0x110
 [<fffffe00001c3e1c>] page_add_file_rmap+0x24/0x80
 [<fffffe00001b9980>] do_set_pte+0x108/0x19c
 [<fffffe00001b9c48>] do_read_fault.isra.93+0x234/0x28c
 [<fffffe00001ba57c>] handle_mm_fault+0x488/0xd64
 [<fffffe00000a0ea0>] do_page_fault+0x224/0x358
 [<fffffe0000090128>] do_mem_abort+0x4c/0xb0
 Exception stack(0xfffffe03dd303e30 to 0xfffffe03dd303f50)
 3e20:                                     00000000 00000000 89bbf000 000003ff
 3e40: ffffffff ffffffff 89b91fac 000003ff 01043280 fffffe00 00000002 00000000
 3e60: dd303e70 fffffe03 008ce3a0 fffffe00 dd303e90 fffffe03 001c0400 fffffe00
 3e80: d5880000 fffffe03 000cbc60 fffffe00 eb753610 000003ff 00092fdc fffffe00
 3ea0: 00000000 00000000 89bbf000 000003ff ffffffff ffffffff 89ba4654 000003ff
 3ec0: 80000000 00000000 d5521480 fffffe03 89bc16e8 000003ff 89bc1a40 000003ff
 3ee0: 00000004 00000000 89b80000 000003ff fefefeff fefefefe 00000000 00000000
 3f00: 0000000f 00000000 89bc1028 000003ff 89ba7000 000003ff 0000079c 00000000
 3f20: 00400000 00000000 ffff0000 ffffffff 004038a0 00000000 00010000 00000000
 3f40: 89ba3c1c 000003ff 89ba3b44 000003ff
 Code: b9405835 35000275 aa1303e0 9400065c (f9400413)

Quoted 41 lines · Mark Salter

Quoted 34 lines · Ard Biesheuvel

Quoted 18 lines · Mark Salter

Quoted 17 lines · Ard Biesheuvel

drivers/firmware/efi/virtmap.c +1 -0

index 98735fb43581..4b6a5c31629f 100644
@@ -8,6 +8,7 @@
  */
 
 #include <linux/efi.h>
+#include <linux/memblock.h>
 #include <linux/mm_types.h>
 #include <linux/rbtree.h>
 #include <linux/rwsem.h>

@@ -97,8 +98,15 @@ void __init efi_virtmap_init(void)
                u64 paddr, npages, size;
                pgprot_t prot;
 

Quoted 14 lines

drivers/firmware/efi/virtmap.c void __init efi_virtmap_init(void) · line 109

Quoted 10 lines

drivers/firmware/efi/virtmap.c +8 -1

-               if (!efi_mem_is_usable_region(md))
+               paddr = md->phys_addr;
+               npages = md->num_pages;
+               memrange_efi_to_native(&paddr, &npages);
+               size = npages << PAGE_SHIFT;
+
+               if (!efi_mem_is_usable_region(md)) {
                        efi_register_mem_resource(md);
+                       memblock_remove(paddr, size);
+               }

What exactly is the memblock_remove trying to accomplish? With it, I get:

Quoted 6 lines

The idea was pfn_valid() will then return false for those pfns, allowing us to distinguish them from memory that the kernel may be using, primarily for /dev/mem filtering. But apparently it is causing problems for you, so I will have to figure out if there is a better way of addressing this.

Quoted 7 lines

Okay. Well I think that works for pfn_valid, but it is confusing the mem_cgroup code. I think because you end up with multiple memory regions after creating the holes. Earlier memory management code saw one memory region. And because this comes after paging_init(), all of the memory ends up in the kernel linear mapping which is something we were trying to avoid.

I will drop the memblock_remove() then. I guess I could make the test in devmem_is_allowed() do

if (!memblock_is_memory() || memblock_is_reserved())

instead of 'if (!pfn_valid())' so that reserved regions become accessible with having to remove them.

Are you happy with the other change in this patch, i.e., using memblock_add() directly so that we don't have to deal with the rounding?

Quoted 53 lines · Ard Biesheuvel

Quoted 41 lines · Mark Salter

Quoted 34 lines · Ard Biesheuvel

Quoted 18 lines · Mark Salter

Quoted 17 lines · Ard Biesheuvel

drivers/firmware/efi/virtmap.c +1 -0

index 98735fb43581..4b6a5c31629f 100644
@@ -8,6 +8,7 @@
  */
 
 #include <linux/efi.h>
+#include <linux/memblock.h>
 #include <linux/mm_types.h>
 #include <linux/rbtree.h>
 #include <linux/rwsem.h>

@@ -97,8 +98,15 @@ void __init efi_virtmap_init(void)
                u64 paddr, npages, size;
                pgprot_t prot;
 

Quoted 14 lines

drivers/firmware/efi/virtmap.c void __init efi_virtmap_init(void) · line 109

Quoted 10 lines

drivers/firmware/efi/virtmap.c +8 -1

-               if (!efi_mem_is_usable_region(md))
+               paddr = md->phys_addr;
+               npages = md->num_pages;
+               memrange_efi_to_native(&paddr, &npages);
+               size = npages << PAGE_SHIFT;
+
+               if (!efi_mem_is_usable_region(md)) {
                        efi_register_mem_resource(md);
+                       memblock_remove(paddr, size);
+               }

What exactly is the memblock_remove trying to accomplish? With it, I get:

Quoted 6 lines

The idea was pfn_valid() will then return false for those pfns, allowing us to distinguish them from memory that the kernel may be using, primarily for /dev/mem filtering. But apparently it is causing problems for you, so I will have to figure out if there is a better way of addressing this.

Quoted 7 lines

Okay. Well I think that works for pfn_valid, but it is confusing the mem_cgroup code. I think because you end up with multiple memory regions after creating the holes. Earlier memory management code saw one memory region. And because this comes after paging_init(), all of the memory ends up in the kernel linear mapping which is something we were trying to avoid.

I will drop the memblock_remove() then. I guess I could make the test in devmem_is_allowed() do

Quoted 4 lines

if (!memblock_is_memory() || memblock_is_reserved())

instead of 'if (!pfn_valid())' so that reserved regions become accessible with having to remove them.

Maybe we should add a new memblock to keep track of uefi tables. The memblock_is_reserved() seems overly permissive to me.

Quoted 5 lines

Are you happy with the other change in this patch, i.e., using memblock_add() directly so that we don't have to deal with the rounding?

Yeah, I think that's okay. Everything else seems to be working fine in my testing.

On systems with 64 KB pages, it is preferable for UEFI memory map entries to be 64 KB aligned multiples of 64 KB, because it relieves us of having to deal with the residues.
So, if EFI_ALLOC_ALIGN is #define'd by the platform, use it to round up all memory allocations made.

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

---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

This is v3 of the series to update the UEFI memory map handling for the arm64 architecture so that:

This is v3 of the series to update the UEFI memory map handling for the arm64 architecture so that:

Hi Ard,

Quoted 2 lines · Ard Biesheuvel

This is v3 of the series to update the UEFI memory map handling for the arm64 architecture so that:

I merged in your EFI patches into my master branch [1] and was able to do a successful chain of (four) kexec re-boots of the same kernel image on the ARM base model running LEG 14.10 EFI binaries [2]. EFI showed:

UEFI v2.40 (ARM Fixed Virtual Platform EFI Oct 23 2014 15:53:09, 0x00000000)

I only verified that all CPU's came up and that /proc/meminfo and the /proc/iomem 'System RAM' entries looked ok.

[1] https://git.linaro.org/people/geoff.levand/linux-kexec.git
[2] https://releases.linaro.org/14.10/components/kernel/uefi-linaro/release/fvp_minimal