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

Groups > linux.kernel > #1429709 > unrolled thread

[PATCH 00/11] efi: Permanent runtime EFI memmap support

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/11] efi: Permanent runtime EFI memmap support Matt Fleming <matt@codeblueprint.co.uk> - 2016-06-23 13:40 +0200
    [PATCH 03/11] efi: Refactor efi_memmap_init_early() into arch-neutral code Matt Fleming <matt@codeblueprint.co.uk> - 2016-06-23 13:40 +0200
      Re: [PATCH 03/11] efi: Refactor efi_memmap_init_early() into  arch-neutral code Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-06-24 13:50 +0200
        Re: [PATCH 03/11] efi: Refactor efi_memmap_init_early() into  arch-neutral code Matt Fleming <matt@codeblueprint.co.uk> - 2016-07-04 14:30 +0200
          Re: [PATCH 03/11] efi: Refactor efi_memmap_init_early() into arch-neutral code Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-07-05 00:30 +0200
    Re: [PATCH 00/11] efi: Permanent runtime EFI memmap support Dave Young <dyoung@redhat.com> - 2016-06-30 16:40 +0200

#1429709 — [PATCH 00/11] efi: Permanent runtime EFI memmap support

This series adds support for keeping the EFI memory map around at
runtime for x86 (ARM and arm64 already have this support).

Additionally, drivers can now reserve EFI boot services regions such
that they are not released back to the kernel during
efi_free_boot_services().

With these two features it becomes possible to pass things like the
EFI System Resource Table data structures across kexec reboot.

This series also includes a whole bunch of refactoring and cleanups to
move code out of arch or driver-specific files into generic places.

I've given these patches some light testing, but it would be good if
others could provide more given that this series touches so many
different code paths.

ARM/arm64 folks, you're going to want to pay particular attention to
PATCH 3 where I drop the read-only mapping of the EFI memory map since
it causes problems for x86.

I am aware that we leak memory maps when new ones get installed, e.g.
during __efi_enter_virtual_mode(), but that's an existing bug that
requires everyone to allocate memory maps the same way before it can
be fixed.

 arch/x86/include/asm/efi.h         |   1 -
 arch/x86/kernel/setup.c            |  13 +-
 arch/x86/platform/efi/efi-bgrt.c   |  13 +-
 arch/x86/platform/efi/efi.c        | 198 ++++++++++++-------------
 arch/x86/platform/efi/efi_64.c     |  20 ---
 arch/x86/platform/efi/quirks.c     | 123 ++++++++++++++--
 drivers/firmware/efi/Makefile      |   2 +-
 drivers/firmware/efi/arm-init.c    |  17 +--
 drivers/firmware/efi/arm-runtime.c |   4 +-
 drivers/firmware/efi/efi.c         |  64 ++++----
 drivers/firmware/efi/esrt.c        |  20 +--
 drivers/firmware/efi/fake_mem.c    | 125 ++--------------
 drivers/firmware/efi/memmap.c      | 292 +++++++++++++++++++++++++++++++++++++
 drivers/firmware/efi/runtime-map.c |  35 ++---
 include/linux/efi.h                |  34 ++++-
 15 files changed, 605 insertions(+), 356 deletions(-)
 create mode 100644 drivers/firmware/efi/memmap.c

[toc] | [next] | [standalone]

#1429713 — [PATCH 03/11] efi: Refactor efi_memmap_init_early() into arch-neutral code

Every EFI architecture apart from ia64 needs to setup the EFI memory
map at efi.memmap, and the code for doing that is essentially the same
across all implementations. Therefore, it makes sense to factor this
out into the common code under drivers/firmware/efi/.

The only slight variation is the data structure out of which we pull
the initial memory map information, such as physical address, memory
descriptor size and version, etc. We can address this by passing a
generic data structure (struct efi_memory_map_data) as the argument to
efi_memmap_init_early() which contains the minimum info required for
initialising the memory map.

In the process, this patch also fixes a few undesirable implementation
differences:

 - ARM and arm64 were failing to clear the EFI_MEMMAP bit when
   unmapping the early EFI memory map. EFI_MEMMAP indicates whether
   the EFI memory map is mapped (not the regions contained within) and
   can be traversed.  It's more correct to set the bit as soon as we
   memremap() the passed in EFI memmap.

 - Rename efi_unmmap_memmap() to efi_memmap_unmap() to adhere to the
   regular naming scheme.

This patch also uses a read-write mapping for the memory map instead
of the read-only mapping currently used on ARM and arm64. x86 needs
the ability to update the memory map in-place when assigning virtual
addresses to regions (efi_map_region()) and tagging regions when
reserving boot services (efi_reserve_boot_services()).

There's no way for the generic fake_mem code to know which mapping to
use without introducing some arch-specific constant/hook, so just use
read-write since read-only is of dubious value for the EFI memory map.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Peter Jones <pjones@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Dave Young <dyoung@redhat.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 arch/x86/include/asm/efi.h      |  1 -
 arch/x86/platform/efi/efi.c     | 66 +++++++++++------------------------------
 arch/x86/platform/efi/quirks.c  |  4 +--
 drivers/firmware/efi/arm-init.c | 17 +++++------
 drivers/firmware/efi/efi.c      | 46 ++++++++++++++++++++++++++++
 drivers/firmware/efi/fake_mem.c | 15 ++++++----
 include/linux/efi.h             | 16 ++++++++++
 7 files changed, 98 insertions(+), 67 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 78d1e7467eae..67983035bd7b 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -118,7 +118,6 @@ 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_unmap_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);
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 7c3d9092c668..b434c887229c 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -188,7 +188,9 @@ static void __init do_add_efi_memmap(void)
 int __init efi_memblock_x86_reserve_range(void)
 {
 	struct efi_info *e = &boot_params.efi_info;
+	struct efi_memory_map_data data;
 	phys_addr_t pmap;
+	int rv;
 
 	if (efi_enabled(EFI_PARAVIRT))
 		return 0;
@@ -203,11 +205,17 @@ int __init efi_memblock_x86_reserve_range(void)
 #else
 	pmap = (e->efi_memmap |	((__u64)e->efi_memmap_hi << 32));
 #endif
-	efi.memmap.phys_map	= pmap;
-	efi.memmap.nr_map	= e->efi_memmap_size /
-				  e->efi_memdesc_size;
-	efi.memmap.desc_size	= e->efi_memdesc_size;
-	efi.memmap.desc_version	= e->efi_memdesc_version;
+	data.phys_map		= pmap;
+	data.size 		= e->efi_memmap_size;
+	data.desc_size		= e->efi_memdesc_size;
+	data.desc_version	= e->efi_memdesc_version;
+
+	rv = efi_memmap_init_early(&data);
+	if (rv)
+		return rv;
+
+	if (add_efi_memmap)
+		do_add_efi_memmap();
 
 	WARN(efi.memmap.desc_version != 1,
 	     "Unexpected EFI_MEMORY_DESCRIPTOR version %ld",
@@ -234,19 +242,6 @@ void __init efi_print_memmap(void)
 	}
 }
 
-void __init efi_unmap_memmap(void)
-{
-	unsigned long size;
-
-	clear_bit(EFI_MEMMAP, &efi.flags);
-
-	size = efi.memmap.nr_map * efi.memmap.desc_size;
-	if (efi.memmap.map) {
-		early_memunmap(efi.memmap.map, size);
-		efi.memmap.map = NULL;
-	}
-}
-
 static int __init efi_systab_init(void *phys)
 {
 	if (efi_enabled(EFI_64BIT)) {
@@ -430,33 +425,6 @@ static int __init efi_runtime_init(void)
 	return 0;
 }
 
-static int __init efi_memmap_init(void)
-{
-	unsigned long addr, size;
-
-	if (efi_enabled(EFI_PARAVIRT))
-		return 0;
-
-	/* Map the EFI memory map */
-	size = efi.memmap.nr_map * efi.memmap.desc_size;
-	addr = (unsigned long)efi.memmap.phys_map;
-
-	efi.memmap.map = early_memremap(addr, size);
-	if (efi.memmap.map == NULL) {
-		pr_err("Could not map the memory map!\n");
-		return -ENOMEM;
-	}
-
-	efi.memmap.map_end = efi.memmap.map + size;
-
-	if (add_efi_memmap)
-		do_add_efi_memmap();
-
-	set_bit(EFI_MEMMAP, &efi.flags);
-
-	return 0;
-}
-
 void __init efi_init(void)
 {
 	efi_char16_t *c16;
@@ -514,11 +482,11 @@ void __init efi_init(void)
 	if (!efi_runtime_supported())
 		pr_info("No EFI runtime due to 32/64-bit mismatch with kernel\n");
 	else {
-		if (efi_runtime_disabled() || efi_runtime_init())
+		if (efi_runtime_disabled() || efi_runtime_init()) {
+			efi_memmap_unmap();
 			return;
+		}
 	}
-	if (efi_memmap_init())
-		return;
 
 	if (efi_enabled(EFI_DBG))
 		efi_print_memmap();
@@ -855,7 +823,7 @@ static void __init kexec_enter_virtual_mode(void)
 	 * non-native EFI
 	 */
 	if (!efi_is_native()) {
-		efi_unmap_memmap();
+		efi_memmap_unmap();
 		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 		return;
 	}
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 4480c06cade7..0af180004e74 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -266,7 +266,7 @@ void __init efi_free_boot_services(void)
 		free_bootmem_late(start, size);
 	}
 
-	efi_unmap_memmap();
+	efi_memmap_unmap();
 }
 
 /*
@@ -344,7 +344,7 @@ void __init efi_apply_memmap_quirks(void)
 	 */
 	if (!efi_runtime_supported()) {
 		pr_info("Setup done, disabling due to 32/64-bit mismatch\n");
-		efi_unmap_memmap();
+		efi_memmap_unmap();
 	}
 
 	/* UV2+ BIOS has a fix for this issue.  UV1 still needs the quirk. */
diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index c49d50e68aee..5a2df3fefccc 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -211,12 +211,11 @@ static __init void reserve_regions(void)
 			memblock_mark_nomap(paddr, size);
 
 	}
-
-	set_bit(EFI_MEMMAP, &efi.flags);
 }
 
 void __init efi_init(void)
 {
+	struct efi_memory_map_data data;
 	struct efi_fdt_params params;
 
 	/* Grab UEFI information placed in FDT by stub */
@@ -225,9 +224,12 @@ void __init efi_init(void)
 
 	efi_system_table = params.system_table;
 
-	efi.memmap.phys_map = params.mmap;
-	efi.memmap.map = early_memremap_ro(params.mmap, params.mmap_size);
-	if (efi.memmap.map == NULL) {
+	data.desc_version = params.desc_ver;
+	data.desc_size = params.desc_size;
+	data.size = params.mmap_size;
+	data.phys_map = params.mmap;
+
+	if (efi_memmap_init_early(&data) < 0) {
 		/*
 		* If we are booting via UEFI, the UEFI memory map is the only
 		* description of memory we have, so there is little point in
@@ -235,9 +237,6 @@ void __init efi_init(void)
 		*/
 		panic("Unable to map EFI memory map.\n");
 	}
-	efi.memmap.map_end = efi.memmap.map + params.mmap_size;
-	efi.memmap.desc_size = params.desc_size;
-	efi.memmap.desc_version = params.desc_ver;
 
 	WARN(efi.memmap.desc_version != 1,
 	     "Unexpected EFI_MEMORY_DESCRIPTOR version %ld",
@@ -248,7 +247,7 @@ void __init efi_init(void)
 
 	reserve_regions();
 	efi_memattr_init();
-	early_memunmap(efi.memmap.map, params.mmap_size);
+	efi_memmap_unmap();
 
 	memblock_reserve(params.mmap & PAGE_MASK,
 			 PAGE_ALIGN(params.mmap_size +
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 05509f3aaee8..3e6dce71f54d 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -448,6 +448,52 @@ int __init efi_config_init(efi_config_table_type_t *arch_tables)
 	return ret;
 }
 
+/**
+ * efi_memmap_init_early - Map the EFI memory map data structure
+ * @data: EFI memory map data
+ *
+ * Use early_memremap() to map the passed in EFI memory map and assign
+ * it to efi.memmap.
+ */
+int __init efi_memmap_init_early(struct efi_memory_map_data *data)
+{
+	struct efi_memory_map map;
+
+	if (efi_enabled(EFI_PARAVIRT))
+		return 0;
+
+	map.phys_map = data->phys_map;
+
+	map.map = early_memremap(data->phys_map, data->size);
+	if (!map.map) {
+		pr_err("Could not map the memory map!\n");
+		return -ENOMEM;
+	}
+
+	map.nr_map = data->size / data->desc_size;
+	map.map_end = map.map + data->size;
+
+	map.desc_version = data->desc_version;
+	map.desc_size = data->desc_size;
+
+	set_bit(EFI_MEMMAP, &efi.flags);
+
+	efi.memmap = map;
+
+	return 0;
+}
+
+void __init efi_memmap_unmap(void)
+{
+	unsigned long size;
+
+	size = efi.memmap.desc_size * efi.memmap.nr_map;
+
+	early_memunmap(efi.memmap.map, size);
+	efi.memmap.map = NULL;
+	clear_bit(EFI_MEMMAP, &efi.flags);
+}
+
 #ifdef CONFIG_EFI_VARS_MODULE
 static int __init efi_load_efivars(void)
 {
diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
index c437388a7b85..939eec47139f 100644
--- a/drivers/firmware/efi/fake_mem.c
+++ b/drivers/firmware/efi/fake_mem.c
@@ -57,6 +57,7 @@ static int __init cmp_fake_mem(const void *x1, const void *x2)
 void __init efi_fake_memmap(void)
 {
 	u64 start, end, m_start, m_end, m_attr;
+	struct efi_memory_map_data data;
 	int new_nr_map = efi.memmap.nr_map;
 	efi_memory_desc_t *md;
 	phys_addr_t new_memmap_phy;
@@ -180,12 +181,14 @@ void __init efi_fake_memmap(void)
 	}
 
 	/* swap into new EFI memmap */
-	efi_unmap_memmap();
-	efi.memmap.map = new_memmap;
-	efi.memmap.phys_map = new_memmap_phy;
-	efi.memmap.nr_map = new_nr_map;
-	efi.memmap.map_end = efi.memmap.map + efi.memmap.nr_map * efi.memmap.desc_size;
-	set_bit(EFI_MEMMAP, &efi.flags);
+	early_memunmap(new_memmap, efi.memmap.desc_size * new_nr_map);
+	efi_memmap_unmap();
+
+	data.phys_map = new_memmap_phy;
+	data.size = efi.memmap.desc_size * new_nr_map;
+	data.desc_version = efi.memmap.desc_version;
+	data.desc_size = efi.memmap.desc_size;
+	efi_memmap_init_early(&data);
 
 	/* print new EFI memmap */
 	efi_print_memmap();
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 03009695760d..055fbecd3156 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -742,6 +742,18 @@ typedef struct {
 	unsigned long tables;
 } efi_system_table_t;
 
+/*
+ * Architecture independent structure for describing a memory map for the
+ * benefit of efi_memmap_init_early(), saving us the need to pass four
+ * parameters.
+ */
+struct efi_memory_map_data {
+	phys_addr_t phys_map;
+	unsigned long size;
+	unsigned long desc_version;
+	unsigned long desc_size;
+};
+
 struct efi_memory_map {
 	phys_addr_t phys_map;
 	void *map;
@@ -973,6 +985,10 @@ static inline efi_status_t efi_query_variable_store(u32 attributes,
 }
 #endif
 extern void __iomem *efi_lookup_mapped_addr(u64 phys_addr);
+
+extern int __init efi_memmap_init_early(struct efi_memory_map_data *data);
+extern void __init efi_memmap_unmap(void);
+
 extern int efi_config_init(efi_config_table_type_t *arch_tables);
 #ifdef CONFIG_EFI_ESRT
 extern void __init efi_esrt_init(void);
-- 
2.7.3

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

#1430601 — Re: [PATCH 03/11] efi: Refactor efi_memmap_init_early() into arch-neutral code

Hi Matt,

On 23 June 2016 at 13:34, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> Every EFI architecture apart from ia64 needs to setup the EFI memory
> map at efi.memmap, and the code for doing that is essentially the same
> across all implementations. Therefore, it makes sense to factor this
> out into the common code under drivers/firmware/efi/.
>
> The only slight variation is the data structure out of which we pull
> the initial memory map information, such as physical address, memory
> descriptor size and version, etc. We can address this by passing a
> generic data structure (struct efi_memory_map_data) as the argument to
> efi_memmap_init_early() which contains the minimum info required for
> initialising the memory map.
>
> In the process, this patch also fixes a few undesirable implementation
> differences:
>
>  - ARM and arm64 were failing to clear the EFI_MEMMAP bit when
>    unmapping the early EFI memory map. EFI_MEMMAP indicates whether
>    the EFI memory map is mapped (not the regions contained within) and
>    can be traversed.  It's more correct to set the bit as soon as we
>    memremap() the passed in EFI memmap.
>

This patch (and the series) looks mostly fine to me, but this patch
does break ARM currently, please see below.

>  - Rename efi_unmmap_memmap() to efi_memmap_unmap() to adhere to the
>    regular naming scheme.
>
> This patch also uses a read-write mapping for the memory map instead
> of the read-only mapping currently used on ARM and arm64. x86 needs
> the ability to update the memory map in-place when assigning virtual
> addresses to regions (efi_map_region()) and tagging regions when
> reserving boot services (efi_reserve_boot_services()).
>
> There's no way for the generic fake_mem code to know which mapping to
> use without introducing some arch-specific constant/hook, so just use
> read-write since read-only is of dubious value for the EFI memory map.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Dave Young <dyoung@redhat.com>
> Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
> ---
>  arch/x86/include/asm/efi.h      |  1 -
>  arch/x86/platform/efi/efi.c     | 66 +++++++++++------------------------------
>  arch/x86/platform/efi/quirks.c  |  4 +--
>  drivers/firmware/efi/arm-init.c | 17 +++++------
>  drivers/firmware/efi/efi.c      | 46 ++++++++++++++++++++++++++++
>  drivers/firmware/efi/fake_mem.c | 15 ++++++----
>  include/linux/efi.h             | 16 ++++++++++
>  7 files changed, 98 insertions(+), 67 deletions(-)
>
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index 78d1e7467eae..67983035bd7b 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -118,7 +118,6 @@ 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_unmap_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);
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 7c3d9092c668..b434c887229c 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -188,7 +188,9 @@ static void __init do_add_efi_memmap(void)
>  int __init efi_memblock_x86_reserve_range(void)
>  {
>         struct efi_info *e = &boot_params.efi_info;
> +       struct efi_memory_map_data data;
>         phys_addr_t pmap;
> +       int rv;
>
>         if (efi_enabled(EFI_PARAVIRT))
>                 return 0;
> @@ -203,11 +205,17 @@ int __init efi_memblock_x86_reserve_range(void)
>  #else
>         pmap = (e->efi_memmap | ((__u64)e->efi_memmap_hi << 32));
>  #endif
> -       efi.memmap.phys_map     = pmap;
> -       efi.memmap.nr_map       = e->efi_memmap_size /
> -                                 e->efi_memdesc_size;
> -       efi.memmap.desc_size    = e->efi_memdesc_size;
> -       efi.memmap.desc_version = e->efi_memdesc_version;
> +       data.phys_map           = pmap;
> +       data.size               = e->efi_memmap_size;
> +       data.desc_size          = e->efi_memdesc_size;
> +       data.desc_version       = e->efi_memdesc_version;
> +
> +       rv = efi_memmap_init_early(&data);
> +       if (rv)
> +               return rv;
> +
> +       if (add_efi_memmap)
> +               do_add_efi_memmap();
>
>         WARN(efi.memmap.desc_version != 1,
>              "Unexpected EFI_MEMORY_DESCRIPTOR version %ld",
> @@ -234,19 +242,6 @@ void __init efi_print_memmap(void)
>         }
>  }
>
> -void __init efi_unmap_memmap(void)
> -{
> -       unsigned long size;
> -
> -       clear_bit(EFI_MEMMAP, &efi.flags);
> -
> -       size = efi.memmap.nr_map * efi.memmap.desc_size;
> -       if (efi.memmap.map) {
> -               early_memunmap(efi.memmap.map, size);
> -               efi.memmap.map = NULL;
> -       }
> -}
> -
>  static int __init efi_systab_init(void *phys)
>  {
>         if (efi_enabled(EFI_64BIT)) {
> @@ -430,33 +425,6 @@ static int __init efi_runtime_init(void)
>         return 0;
>  }
>
> -static int __init efi_memmap_init(void)
> -{
> -       unsigned long addr, size;
> -
> -       if (efi_enabled(EFI_PARAVIRT))
> -               return 0;
> -
> -       /* Map the EFI memory map */
> -       size = efi.memmap.nr_map * efi.memmap.desc_size;
> -       addr = (unsigned long)efi.memmap.phys_map;
> -
> -       efi.memmap.map = early_memremap(addr, size);
> -       if (efi.memmap.map == NULL) {
> -               pr_err("Could not map the memory map!\n");
> -               return -ENOMEM;
> -       }
> -
> -       efi.memmap.map_end = efi.memmap.map + size;
> -
> -       if (add_efi_memmap)
> -               do_add_efi_memmap();
> -
> -       set_bit(EFI_MEMMAP, &efi.flags);
> -
> -       return 0;
> -}
> -
>  void __init efi_init(void)
>  {
>         efi_char16_t *c16;
> @@ -514,11 +482,11 @@ void __init efi_init(void)
>         if (!efi_runtime_supported())
>                 pr_info("No EFI runtime due to 32/64-bit mismatch with kernel\n");
>         else {
> -               if (efi_runtime_disabled() || efi_runtime_init())
> +               if (efi_runtime_disabled() || efi_runtime_init()) {
> +                       efi_memmap_unmap();
>                         return;
> +               }
>         }
> -       if (efi_memmap_init())
> -               return;
>
>         if (efi_enabled(EFI_DBG))
>                 efi_print_memmap();
> @@ -855,7 +823,7 @@ static void __init kexec_enter_virtual_mode(void)
>          * non-native EFI
>          */
>         if (!efi_is_native()) {
> -               efi_unmap_memmap();
> +               efi_memmap_unmap();
>                 clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>                 return;
>         }
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index 4480c06cade7..0af180004e74 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -266,7 +266,7 @@ void __init efi_free_boot_services(void)
>                 free_bootmem_late(start, size);
>         }
>
> -       efi_unmap_memmap();
> +       efi_memmap_unmap();
>  }
>
>  /*
> @@ -344,7 +344,7 @@ void __init efi_apply_memmap_quirks(void)
>          */
>         if (!efi_runtime_supported()) {
>                 pr_info("Setup done, disabling due to 32/64-bit mismatch\n");
> -               efi_unmap_memmap();
> +               efi_memmap_unmap();
>         }
>
>         /* UV2+ BIOS has a fix for this issue.  UV1 still needs the quirk. */
> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index c49d50e68aee..5a2df3fefccc 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -211,12 +211,11 @@ static __init void reserve_regions(void)
>                         memblock_mark_nomap(paddr, size);
>
>         }
> -
> -       set_bit(EFI_MEMMAP, &efi.flags);
>  }
>
>  void __init efi_init(void)
>  {
> +       struct efi_memory_map_data data;
>         struct efi_fdt_params params;
>
>         /* Grab UEFI information placed in FDT by stub */
> @@ -225,9 +224,12 @@ void __init efi_init(void)
>
>         efi_system_table = params.system_table;
>
> -       efi.memmap.phys_map = params.mmap;
> -       efi.memmap.map = early_memremap_ro(params.mmap, params.mmap_size);
> -       if (efi.memmap.map == NULL) {
> +       data.desc_version = params.desc_ver;
> +       data.desc_size = params.desc_size;
> +       data.size = params.mmap_size;
> +       data.phys_map = params.mmap;
> +
> +       if (efi_memmap_init_early(&data) < 0) {
>                 /*
>                 * If we are booting via UEFI, the UEFI memory map is the only
>                 * description of memory we have, so there is little point in
> @@ -235,9 +237,6 @@ void __init efi_init(void)
>                 */
>                 panic("Unable to map EFI memory map.\n");
>         }
> -       efi.memmap.map_end = efi.memmap.map + params.mmap_size;
> -       efi.memmap.desc_size = params.desc_size;
> -       efi.memmap.desc_version = params.desc_ver;
>
>         WARN(efi.memmap.desc_version != 1,
>              "Unexpected EFI_MEMORY_DESCRIPTOR version %ld",
> @@ -248,7 +247,7 @@ void __init efi_init(void)
>
>         reserve_regions();
>         efi_memattr_init();
> -       early_memunmap(efi.memmap.map, params.mmap_size);
> +       efi_memmap_unmap();
>
>         memblock_reserve(params.mmap & PAGE_MASK,
>                          PAGE_ALIGN(params.mmap_size +
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 05509f3aaee8..3e6dce71f54d 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -448,6 +448,52 @@ int __init efi_config_init(efi_config_table_type_t *arch_tables)
>         return ret;
>  }
>
> +/**
> + * efi_memmap_init_early - Map the EFI memory map data structure
> + * @data: EFI memory map data
> + *
> + * Use early_memremap() to map the passed in EFI memory map and assign
> + * it to efi.memmap.
> + */
> +int __init efi_memmap_init_early(struct efi_memory_map_data *data)
> +{
> +       struct efi_memory_map map;
> +
> +       if (efi_enabled(EFI_PARAVIRT))
> +               return 0;
> +
> +       map.phys_map = data->phys_map;
> +
> +       map.map = early_memremap(data->phys_map, data->size);
> +       if (!map.map) {
> +               pr_err("Could not map the memory map!\n");
> +               return -ENOMEM;
> +       }
> +
> +       map.nr_map = data->size / data->desc_size;
> +       map.map_end = map.map + data->size;
> +
> +       map.desc_version = data->desc_version;
> +       map.desc_size = data->desc_size;
> +
> +       set_bit(EFI_MEMMAP, &efi.flags);
> +
> +       efi.memmap = map;
> +
> +       return 0;
> +}
> +
> +void __init efi_memmap_unmap(void)
> +{
> +       unsigned long size;
> +
> +       size = efi.memmap.desc_size * efi.memmap.nr_map;
> +
> +       early_memunmap(efi.memmap.map, size);
> +       efi.memmap.map = NULL;

This assignment breaks the calculation of mapsize in
arm_enable_runtime_services(), so you should probably fold the
following hunk into this patch.

diff --git a/drivers/firmware/efi/arm-runtime.c
b/drivers/firmware/efi/arm-runtime.c
index ce1424672d89..1884347a3ef6 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -109,7 +109,7 @@ static int __init arm_enable_runtime_services(void)

        pr_info("Remapping and enabling EFI services.\n");

-       mapsize = efi.memmap.map_end - efi.memmap.map;
+       mapsize = efi.memmap.desc_size * efi.memmap.nr_map;

        if (efi_memmap_init_late(efi.memmap.phys_map, mapsize)) {
                pr_err("Failed to remap EFI memory map\n");


With that change (or an equivalent one):

Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

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

#1436450 — Re: [PATCH 03/11] efi: Refactor efi_memmap_init_early() into arch-neutral code

On Fri, 24 Jun, at 01:44:48PM, Ard Biesheuvel wrote:
> 
> This assignment breaks the calculation of mapsize in
> arm_enable_runtime_services(), so you should probably fold the
> following hunk into this patch.
> 
> diff --git a/drivers/firmware/efi/arm-runtime.c
> b/drivers/firmware/efi/arm-runtime.c
> index ce1424672d89..1884347a3ef6 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -109,7 +109,7 @@ static int __init arm_enable_runtime_services(void)
> 
>         pr_info("Remapping and enabling EFI services.\n");
> 
> -       mapsize = efi.memmap.map_end - efi.memmap.map;
> +       mapsize = efi.memmap.desc_size * efi.memmap.nr_map;
> 
>         if (efi_memmap_init_late(efi.memmap.phys_map, mapsize)) {
>                 pr_err("Failed to remap EFI memory map\n");
 
Thanks Ard, I folded this in.

> With that change (or an equivalent one):
> 
> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Are those tags for just this patch or the entire series?

FYI, my plan right now is to queue this for v4.9 because it's fairly
invasive and I expect some fallout. If anyone has a problem with that
and knows of a reason it should be queued sooner, please let me know.

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

#1436622 — Re: [PATCH 03/11] efi: Refactor efi_memmap_init_early() into arch-neutral code

> On 4 jul. 2016, at 20:19, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> 
>> On Fri, 24 Jun, at 01:44:48PM, Ard Biesheuvel wrote:
>> 
>> This assignment breaks the calculation of mapsize in
>> arm_enable_runtime_services(), so you should probably fold the
>> following hunk into this patch.
>> 
>> diff --git a/drivers/firmware/efi/arm-runtime.c
>> b/drivers/firmware/efi/arm-runtime.c
>> index ce1424672d89..1884347a3ef6 100644
>> --- a/drivers/firmware/efi/arm-runtime.c
>> +++ b/drivers/firmware/efi/arm-runtime.c
>> @@ -109,7 +109,7 @@ static int __init arm_enable_runtime_services(void)
>> 
>>        pr_info("Remapping and enabling EFI services.\n");
>> 
>> -       mapsize = efi.memmap.map_end - efi.memmap.map;
>> +       mapsize = efi.memmap.desc_size * efi.memmap.nr_map;
>> 
>>        if (efi_memmap_init_late(efi.memmap.phys_map, mapsize)) {
>>                pr_err("Failed to remap EFI memory map\n");
> 
> Thanks Ard, I folded this in.
> 
>> With that change (or an equivalent one):
>> 
>> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Are those tags for just this patch or the entire series?
> 

That is for the series (but I only tested arm, so ymmv)

> FYI, my plan right now is to queue this for v4.9 because it's fairly
> invasive and I expect some fallout. If anyone has a problem with that
> and knows of a reason it should be queued sooner, please let me know.

No problem, but once you've queued it i'd like the arm esrt patches to go on top, ideally in the same release. I will dust those off once this series hits your tree

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

#1434631

Hi, Matt

On 06/23/16 at 12:34pm, Matt Fleming wrote:
> This series adds support for keeping the EFI memory map around at
> runtime for x86 (ARM and arm64 already have this support).
> 
> Additionally, drivers can now reserve EFI boot services regions such
> that they are not released back to the kernel during
> efi_free_boot_services().
> 
> With these two features it becomes possible to pass things like the
> EFI System Resource Table data structures across kexec reboot.
> 
> This series also includes a whole bunch of refactoring and cleanups to
> move code out of arch or driver-specific files into generic places.
> 
> I've given these patches some light testing, but it would be good if
> others could provide more given that this series touches so many
> different code paths.

Tested kexec/kdump and the bgrt reservation with kexec and kexec_file
system call. But due to a crypto issue I can not test signature
verification of kexec_file, but I think that is a separate problem.

For these parts:
Tested-by: Dave Young <dyoung@redhat.com>

Thanks
Dave

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web