Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1559402 > unrolled thread

[PATCH V2 0/4] efi/x86: move efi bgrt init code to early init

Back to article view | Back to linux.kernel

Contents

  [PATCH V2 0/4] efi/x86: move efi bgrt init code to early init Dave Young <dyoung@redhat.com> - 2017-01-16 04:00 +0100
    [PATCH V2 2/4] efi/x86: move efi_print_memmap to drivers/firmware/efi/memmap.c Dave Young <dyoung@redhat.com> - 2017-01-16 04:00 +0100
    [PATCH V2 3/4] efi/x86: add debug code to print cooked memmap Dave Young <dyoung@redhat.com> - 2017-01-16 04:00 +0100
    [PATCH V2 4/4] efi/x86: make efi_memmap_reserve only insert into boot mem areas Dave Young <dyoung@redhat.com> - 2017-01-16 04:00 +0100
    [PATCH V2 1/4] efi/x86: move efi bgrt init code to early init code Dave Young <dyoung@redhat.com> - 2017-01-16 04:00 +0100

#1559402 — [PATCH V2 0/4] efi/x86: move efi bgrt init code to early init

Hi,

Here the the update of the series for moving bgrt init code to early init.

Main changes is:
- Move the 1st patch to the last because it does not block the 2nd patch
any more with Peter's patch to prune invlid memmap entries:
https://git.kernel.org/cgit/linux/kernel/git/efi/efi.git/commit/?h=next&id=b2a91
a35445229
But it is still tood to have since efi_mem_reserve only cares about boot related
mem ranges.

- Other comments about code itself, details please the the patches themselves.

 arch/x86/include/asm/efi.h       |    1 
 arch/x86/kernel/acpi/boot.c      |   12 +++++++
 arch/x86/platform/efi/efi-bgrt.c |   59 ++++++++++++++++-----------------------
 arch/x86/platform/efi/efi.c      |   26 +++--------------
 arch/x86/platform/efi/quirks.c   |    2 -
 drivers/acpi/bgrt.c              |   28 +++++++++++++-----
 drivers/firmware/efi/fake_mem.c  |    3 +
 drivers/firmware/efi/memmap.c    |   22 +++++++++++++-
 include/linux/efi-bgrt.h         |    7 +---
 include/linux/efi.h              |    5 +--
 init/main.c                      |    1 
 11 files changed, 92 insertions(+), 74 deletions(-)

Thanks
Dave

[toc] | [next] | [standalone]

#1559404 — [PATCH V2 2/4] efi/x86: move efi_print_memmap to drivers/firmware/efi/memmap.c

Signed-off-by: Dave Young <dyoung@redhat.com>
---
v1->v2: move efi_print_memmap declaration to general header file 
 arch/x86/include/asm/efi.h    |    1 -
 arch/x86/platform/efi/efi.c   |   16 ----------------
 drivers/firmware/efi/memmap.c |   16 ++++++++++++++++
 include/linux/efi.h           |    1 +
 4 files changed, 17 insertions(+), 17 deletions(-)

--- linux-x86.orig/arch/x86/platform/efi/efi.c
+++ linux-x86/arch/x86/platform/efi/efi.c
@@ -278,22 +278,6 @@ static void __init efi_clean_memmap(void
 	}
 }
 
-void __init efi_print_memmap(void)
-{
-	efi_memory_desc_t *md;
-	int i = 0;
-
-	for_each_efi_memory_desc(md) {
-		char buf[64];
-
-		pr_info("mem%02u: %s range=[0x%016llx-0x%016llx] (%lluMB)\n",
-			i++, efi_md_typeattr_format(buf, sizeof(buf), md),
-			md->phys_addr,
-			md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - 1,
-			(md->num_pages >> (20 - EFI_PAGE_SHIFT)));
-	}
-}
-
 static int __init efi_systab_init(void *phys)
 {
 	if (efi_enabled(EFI_64BIT)) {
--- linux-x86.orig/drivers/firmware/efi/memmap.c
+++ linux-x86/drivers/firmware/efi/memmap.c
@@ -10,6 +10,22 @@
 #include <linux/io.h>
 #include <asm/early_ioremap.h>
 
+void __init efi_print_memmap(void)
+{
+	efi_memory_desc_t *md;
+	int i = 0;
+
+	for_each_efi_memory_desc(md) {
+		char buf[64];
+
+		pr_info("mem%02u: %s range=[0x%016llx-0x%016llx] (%lluMB)\n",
+			i++, efi_md_typeattr_format(buf, sizeof(buf), md),
+			md->phys_addr,
+			md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - 1,
+			(md->num_pages >> (20 - EFI_PAGE_SHIFT)));
+	}
+}
+
 /**
  * __efi_memmap_init - Common code for mapping the EFI memory map
  * @data: EFI memory map data
--- linux-x86.orig/arch/x86/include/asm/efi.h
+++ linux-x86/arch/x86/include/asm/efi.h
@@ -116,7 +116,6 @@ extern void __init efi_set_executable(ef
 extern int __init efi_memblock_x86_reserve_range(void);
 extern pgd_t * __init efi_call_phys_prolog(void);
 extern void __init efi_call_phys_epilog(pgd_t *save_pgd);
-extern void __init efi_print_memmap(void);
 extern void __init efi_memory_uc(u64 addr, unsigned long size);
 extern void __init efi_map_region(efi_memory_desc_t *md);
 extern void __init efi_map_region_fixed(efi_memory_desc_t *md);
--- linux-x86.orig/include/linux/efi.h
+++ linux-x86/include/linux/efi.h
@@ -949,6 +949,7 @@ static inline efi_status_t efi_query_var
 	return EFI_SUCCESS;
 }
 #endif
+extern void __init efi_print_memmap(void);
 extern void __iomem *efi_lookup_mapped_addr(u64 phys_addr);
 
 extern int __init efi_memmap_init_early(struct efi_memory_map_data *data);

[toc] | [prev] | [next] | [standalone]

#1559407 — [PATCH V2 3/4] efi/x86: add debug code to print cooked memmap

It is not obvious if the reserved boot area are added correctly, add a
efi_print_memmap to print the new memmap.

Signed-off-by: Dave Young <dyoung@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/x86/platform/efi/efi.c |    5 +++++
 1 file changed, 5 insertions(+)

--- linux-x86.orig/arch/x86/platform/efi/efi.c
+++ linux-x86/arch/x86/platform/efi/efi.c
@@ -943,6 +943,11 @@ static void __init __efi_enter_virtual_m
 		return;
 	}
 
+	if (efi_enabled(EFI_DBG)) {
+		pr_info("EFI runtime memory map:\n");
+		efi_print_memmap();
+	}
+
 	BUG_ON(!efi.systab);
 
 	if (efi_setup_page_tables(pa, 1 << pg_shift)) {

[toc] | [prev] | [next] | [standalone]

#1559408 — [PATCH V2 4/4] efi/x86: make efi_memmap_reserve only insert into boot mem areas

efi_mem_reserve cares only about boot services regions, for making sure
later efi_free_boot_services does not free areas which are still useful,
such as bgrt image buffer. 

So add a new argument to efi_memmap_insert for this purpose.
 
Signed-off-by: Dave Young <dyoung@redhat.com>
---
v1->v2: only check EFI_BOOT_SERVICES_CODE/_DATA
 arch/x86/platform/efi/quirks.c  |    2 +-
 drivers/firmware/efi/fake_mem.c |    3 ++-
 drivers/firmware/efi/memmap.c   |    6 +++++-
 include/linux/efi.h             |    4 ++--
 4 files changed, 10 insertions(+), 5 deletions(-)

--- linux-x86.orig/drivers/firmware/efi/memmap.c
+++ linux-x86/drivers/firmware/efi/memmap.c
@@ -229,7 +229,7 @@ int __init efi_memmap_split_count(efi_me
  * to see how large @buf needs to be.
  */
 void __init efi_memmap_insert(struct efi_memory_map *old_memmap, void *buf,
-			      struct efi_mem_range *mem)
+			      struct efi_mem_range *mem, bool boot_only)
 {
 	u64 m_start, m_end, m_attr;
 	efi_memory_desc_t *md;
@@ -262,6 +262,10 @@ void __init efi_memmap_insert(struct efi
 		start = md->phys_addr;
 		end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - 1;
 
+		if (boot_only && !(md->type == EFI_BOOT_SERVICES_CODE ||
+				   md->type == EFI_BOOT_SERVICES_DATA))
+			continue;
+
 		if (m_start <= start && end <= m_end)
 			md->attribute |= m_attr;
 
--- linux-x86.orig/arch/x86/platform/efi/quirks.c
+++ linux-x86/arch/x86/platform/efi/quirks.c
@@ -226,7 +226,7 @@ void __init efi_arch_mem_reserve(phys_ad
 		return;
 	}
 
-	efi_memmap_insert(&efi.memmap, new, &mr);
+	efi_memmap_insert(&efi.memmap, new, &mr, true);
 	early_memunmap(new, new_size);
 
 	efi_memmap_install(new_phys, num_entries);
--- linux-x86.orig/drivers/firmware/efi/fake_mem.c
+++ linux-x86/drivers/firmware/efi/fake_mem.c
@@ -85,7 +85,8 @@ void __init efi_fake_memmap(void)
 	}
 
 	for (i = 0; i < nr_fake_mem; i++)
-		efi_memmap_insert(&efi.memmap, new_memmap, &fake_mems[i]);
+		efi_memmap_insert(&efi.memmap, new_memmap, &fake_mems[i],
+				  false);
 
 	/* swap into new EFI memmap */
 	early_memunmap(new_memmap, efi.memmap.desc_size * new_nr_map);
--- linux-x86.orig/include/linux/efi.h
+++ linux-x86/include/linux/efi.h
@@ -959,8 +959,8 @@ extern int __init efi_memmap_install(phy
 extern int __init efi_memmap_split_count(efi_memory_desc_t *md,
 					 struct range *range);
 extern void __init efi_memmap_insert(struct efi_memory_map *old_memmap,
-				     void *buf, struct efi_mem_range *mem);
-
+				     void *buf, struct efi_mem_range *mem,
+				     bool boot_only);
 extern int efi_config_init(efi_config_table_type_t *arch_tables);
 #ifdef CONFIG_EFI_ESRT
 extern void __init efi_esrt_init(void);

[toc] | [prev] | [next] | [standalone]

#1559410 — [PATCH V2 1/4] efi/x86: move efi bgrt init code to early init code

Before invoking the arch specific handler, efi_mem_reserve() reserves
the given memory region through memblock.

efi_bgrt_init will call efi_mem_reserve after mm_init(), at that time
memblock is dead and it should not be used any more.

efi bgrt code depend on acpi intialization to get the bgrt acpi table,
moving bgrt parsing to acpi early boot code can make sure efi_mem_reserve
in efi bgrt code still use memblock safely. 

Signed-off-by: Dave Young <dyoung@redhat.com>
---
v1->v2: efi_bgrt_init: check table length first before copying bgrt table
        error checking in drivers/acpi/bgrt.c
 arch/x86/kernel/acpi/boot.c      |   12 +++++++
 arch/x86/platform/efi/efi-bgrt.c |   59 ++++++++++++++++-----------------------
 arch/x86/platform/efi/efi.c      |    5 ---
 drivers/acpi/bgrt.c              |   28 +++++++++++++-----
 include/linux/efi-bgrt.h         |    7 +---
 init/main.c                      |    1 
 6 files changed, 60 insertions(+), 52 deletions(-)

--- linux-x86.orig/arch/x86/kernel/acpi/boot.c
+++ linux-x86/arch/x86/kernel/acpi/boot.c
@@ -35,6 +35,7 @@
 #include <linux/bootmem.h>
 #include <linux/ioport.h>
 #include <linux/pci.h>
+#include <linux/efi-bgrt.h>
 
 #include <asm/irqdomain.h>
 #include <asm/pci_x86.h>
@@ -1557,6 +1558,14 @@ int __init early_acpi_boot_init(void)
 	return 0;
 }
 
+#ifdef CONFIG_ACPI_BGRT
+static int __init acpi_parse_bgrt(struct acpi_table_header *table)
+{
+	efi_bgrt_init(table);
+	return 0;
+}
+#endif
+
 int __init acpi_boot_init(void)
 {
 	/* those are executed after early-quirks are executed */
@@ -1581,6 +1590,9 @@ int __init acpi_boot_init(void)
 	acpi_process_madt();
 
 	acpi_table_parse(ACPI_SIG_HPET, acpi_parse_hpet);
+#ifdef CONFIG_ACPI_BGRT
+	acpi_table_parse("BGRT", acpi_parse_bgrt);
+#endif
 
 	if (!acpi_noirq)
 		x86_init.pci.init = pci_acpi_init;
--- linux-x86.orig/arch/x86/platform/efi/efi-bgrt.c
+++ linux-x86/arch/x86/platform/efi/efi-bgrt.c
@@ -19,8 +19,7 @@
 #include <linux/efi.h>
 #include <linux/efi-bgrt.h>
 
-struct acpi_table_bgrt *bgrt_tab;
-void *__initdata bgrt_image;
+struct acpi_table_bgrt bgrt_tab;
 size_t __initdata bgrt_image_size;
 
 struct bmp_header {
@@ -28,66 +27,58 @@ struct bmp_header {
 	u32 size;
 } __packed;
 
-void __init efi_bgrt_init(void)
+void __init efi_bgrt_init(struct acpi_table_header *table)
 {
-	acpi_status status;
 	void *image;
 	struct bmp_header bmp_header;
+	struct acpi_table_bgrt *bgrt = &bgrt_tab;
 
 	if (acpi_disabled)
 		return;
 
-	status = acpi_get_table("BGRT", 0,
-	                        (struct acpi_table_header **)&bgrt_tab);
-	if (ACPI_FAILURE(status))
-		return;
-
-	if (bgrt_tab->header.length < sizeof(*bgrt_tab)) {
+	if (table->length < sizeof(bgrt_tab)) {
 		pr_notice("Ignoring BGRT: invalid length %u (expected %zu)\n",
-		       bgrt_tab->header.length, sizeof(*bgrt_tab));
+		       table->length, sizeof(bgrt_tab));
 		return;
 	}
-	if (bgrt_tab->version != 1) {
+	*bgrt = *(struct acpi_table_bgrt *)table;
+	if (bgrt->version != 1) {
 		pr_notice("Ignoring BGRT: invalid version %u (expected 1)\n",
-		       bgrt_tab->version);
-		return;
+		       bgrt->version);
+		goto out;
 	}
-	if (bgrt_tab->status & 0xfe) {
+	if (bgrt->status & 0xfe) {
 		pr_notice("Ignoring BGRT: reserved status bits are non-zero %u\n",
-		       bgrt_tab->status);
-		return;
+		       bgrt->status);
+		goto out;
 	}
-	if (bgrt_tab->image_type != 0) {
+	if (bgrt->image_type != 0) {
 		pr_notice("Ignoring BGRT: invalid image type %u (expected 0)\n",
-		       bgrt_tab->image_type);
-		return;
+		       bgrt->image_type);
+		goto out;
 	}
-	if (!bgrt_tab->image_address) {
+	if (!bgrt->image_address) {
 		pr_notice("Ignoring BGRT: null image address\n");
-		return;
+		goto out;
 	}
 
-	image = memremap(bgrt_tab->image_address, sizeof(bmp_header), MEMREMAP_WB);
+	image = early_memremap(bgrt->image_address, sizeof(bmp_header));
 	if (!image) {
 		pr_notice("Ignoring BGRT: failed to map image header memory\n");
-		return;
+		goto out;
 	}
 
 	memcpy(&bmp_header, image, sizeof(bmp_header));
-	memunmap(image);
+	early_memunmap(image, sizeof(bmp_header));
 	if (bmp_header.id != 0x4d42) {
 		pr_notice("Ignoring BGRT: Incorrect BMP magic number 0x%x (expected 0x4d42)\n",
 			bmp_header.id);
-		return;
+		goto out;
 	}
 	bgrt_image_size = bmp_header.size;
+	efi_mem_reserve(bgrt->image_address, bgrt_image_size);
 
-	bgrt_image = memremap(bgrt_tab->image_address, bmp_header.size, MEMREMAP_WB);
-	if (!bgrt_image) {
-		pr_notice("Ignoring BGRT: failed to map image memory\n");
-		bgrt_image = NULL;
-		return;
-	}
-
-	efi_mem_reserve(bgrt_tab->image_address, bgrt_image_size);
+	return;
+out:
+	memset(bgrt, 0, sizeof(bgrt_tab));
 }
--- linux-x86.orig/drivers/acpi/bgrt.c
+++ linux-x86/drivers/acpi/bgrt.c
@@ -15,40 +15,41 @@
 #include <linux/sysfs.h>
 #include <linux/efi-bgrt.h>
 
+static void *bgrt_image;
 static struct kobject *bgrt_kobj;
 
 static ssize_t show_version(struct device *dev,
 			    struct device_attribute *attr, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%d\n", bgrt_tab->version);
+	return snprintf(buf, PAGE_SIZE, "%d\n", bgrt_tab.version);
 }
 static DEVICE_ATTR(version, S_IRUGO, show_version, NULL);
 
 static ssize_t show_status(struct device *dev,
 			   struct device_attribute *attr, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%d\n", bgrt_tab->status);
+	return snprintf(buf, PAGE_SIZE, "%d\n", bgrt_tab.status);
 }
 static DEVICE_ATTR(status, S_IRUGO, show_status, NULL);
 
 static ssize_t show_type(struct device *dev,
 			 struct device_attribute *attr, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%d\n", bgrt_tab->image_type);
+	return snprintf(buf, PAGE_SIZE, "%d\n", bgrt_tab.image_type);
 }
 static DEVICE_ATTR(type, S_IRUGO, show_type, NULL);
 
 static ssize_t show_xoffset(struct device *dev,
 			    struct device_attribute *attr, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%d\n", bgrt_tab->image_offset_x);
+	return snprintf(buf, PAGE_SIZE, "%d\n", bgrt_tab.image_offset_x);
 }
 static DEVICE_ATTR(xoffset, S_IRUGO, show_xoffset, NULL);
 
 static ssize_t show_yoffset(struct device *dev,
 			    struct device_attribute *attr, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%d\n", bgrt_tab->image_offset_y);
+	return snprintf(buf, PAGE_SIZE, "%d\n", bgrt_tab.image_offset_y);
 }
 static DEVICE_ATTR(yoffset, S_IRUGO, show_yoffset, NULL);
 
@@ -84,15 +85,24 @@ static int __init bgrt_init(void)
 {
 	int ret;
 
-	if (!bgrt_image)
+	if (!bgrt_tab.image_address)
 		return -ENODEV;
 
+	bgrt_image = memremap(bgrt_tab.image_address, bgrt_image_size,
+			      MEMREMAP_WB);
+	if (!bgrt_image) {
+		pr_notice("Ignoring BGRT: failed to map image memory\n");
+		return -ENOMEM;
+	}
+
 	bin_attr_image.private = bgrt_image;
 	bin_attr_image.size = bgrt_image_size;
 
 	bgrt_kobj = kobject_create_and_add("bgrt", acpi_kobj);
-	if (!bgrt_kobj)
-		return -EINVAL;
+	if (!bgrt_kobj) {
+		ret = -EINVAL;
+		goto out_memmap;
+	}
 
 	ret = sysfs_create_group(bgrt_kobj, &bgrt_attribute_group);
 	if (ret)
@@ -102,6 +112,8 @@ static int __init bgrt_init(void)
 
 out_kobject:
 	kobject_put(bgrt_kobj);
+out_memmap:
+	memunmap(bgrt_image);
 	return ret;
 }
 device_initcall(bgrt_init);
--- linux-x86.orig/include/linux/efi-bgrt.h
+++ linux-x86/include/linux/efi-bgrt.h
@@ -5,16 +5,15 @@
 
 #include <linux/acpi.h>
 
-void efi_bgrt_init(void);
+void efi_bgrt_init(struct acpi_table_header *table);
 
 /* The BGRT data itself; only valid if bgrt_image != NULL. */
-extern void *bgrt_image;
 extern size_t bgrt_image_size;
-extern struct acpi_table_bgrt *bgrt_tab;
+extern struct acpi_table_bgrt bgrt_tab;
 
 #else /* !CONFIG_ACPI_BGRT */
 
-static inline void efi_bgrt_init(void) {}
+static inline void efi_bgrt_init(struct acpi_table_header *table) {}
 
 #endif /* !CONFIG_ACPI_BGRT */
 
--- linux-x86.orig/arch/x86/platform/efi/efi.c
+++ linux-x86/arch/x86/platform/efi/efi.c
@@ -546,11 +546,6 @@ void __init efi_init(void)
 		efi_print_memmap();
 }
 
-void __init efi_late_init(void)
-{
-	efi_bgrt_init();
-}
-
 void __init efi_set_executable(efi_memory_desc_t *md, bool executable)
 {
 	u64 addr, npages;
--- linux-x86.orig/init/main.c
+++ linux-x86/init/main.c
@@ -663,7 +663,6 @@ asmlinkage __visible void __init start_k
 	sfi_init_late();
 
 	if (efi_enabled(EFI_RUNTIME_SERVICES)) {
-		efi_late_init();
 		efi_free_boot_services();
 	}
 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web