diff mbox series

x86/efi: error handling on efi memory map

Message ID 20250608090356.1115-1-khaliidcaliy@gmail.com
State New
Headers show
Series x86/efi: error handling on efi memory map | expand

Commit Message

Khalid Ali June 8, 2025, 8:59 a.m. UTC 
From: Khalid Ali <khaliidcaliy@gmail.com>

Memory mapping could fail and we need account it and handle it
gracefully or at least do some actions about it.

Currently this patch from the surface look incomplete, and the reason for
that is i didn't know what to do with mapping failures, and how to react
with.

The point is this patch addresses it assuming the worse case scenario all
or some mappings failed. Also i found FIXME that is
saying missing error handling and i think this is the best way we can
fix it. The functions i modified there prototype are important however there usage propably was
once or twice so they have low risk.

Signed-off-by: Khalid Ali <khaliidcaliy@gmail.com>
---
 arch/x86/include/asm/efi.h     |  4 ++--
 arch/x86/platform/efi/efi.c    | 10 ++++++----
 arch/x86/platform/efi/efi_32.c |  7 +++++--
 arch/x86/platform/efi/efi_64.c | 34 ++++++++++++++++++++++------------
 4 files changed, 35 insertions(+), 20 deletions(-)

Comments

kernel test robot June 8, 2025, 1:22 p.m. UTC | #1 
Hi Khalid,

kernel test robot noticed the following build errors:

[auto build test ERROR on efi/next]
[also build test ERROR on tip/master tip/auto-latest linus/master v6.15 next-20250606]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Khalid-Ali/x86-efi-error-handling-on-efi-memory-map/20250608-170711
base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
patch link:    https://lore.kernel.org/r/20250608090356.1115-1-khaliidcaliy%40gmail.com
patch subject: [PATCH] x86/efi: error handling on efi memory map
config: i386-buildonly-randconfig-002-20250608 (https://download.01.org/0day-ci/archive/20250608/202506082104.4eSjYWTe-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250608/202506082104.4eSjYWTe-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506082104.4eSjYWTe-lkp@intel.com/

All errors (new ones prefixed by >>):

>> arch/x86/platform/efi/efi_32.c:90:13: error: conflicting types for 'efi_map_region_fixed'
      90 | void __init efi_map_region_fixed(efi_memory_desc_t *md) {}
         |             ^
   arch/x86/include/asm/efi.h:131:19: note: previous declaration is here
     131 | extern int __init efi_map_region_fixed(efi_memory_desc_t *md);
         |                   ^
   1 error generated.


vim +/efi_map_region_fixed +90 arch/x86/platform/efi/efi_32.c

d2f7cbe7b26a74 Borislav Petkov 2013-10-31  89  
3b2664964bc886 Dave Young      2013-12-20 @90  void __init efi_map_region_fixed(efi_memory_desc_t *md) {}
1fec0533693cd7 Dave Young      2013-12-20  91  void __init parse_efi_setup(u64 phys_addr, u32 data_len) {}
3b2664964bc886 Dave Young      2013-12-20  92
diff mbox series

Patch

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index f227a70ac91f..bc73c07e5d7c 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -127,8 +127,8 @@  extern bool efi_disable_ibt_for_runtime;
 
 extern int __init efi_memblock_x86_reserve_range(void);
 extern void __init efi_print_memmap(void);
-extern void __init efi_map_region(efi_memory_desc_t *md);
-extern void __init efi_map_region_fixed(efi_memory_desc_t *md);
+extern int __init efi_map_region(efi_memory_desc_t *md);
+extern int __init efi_map_region_fixed(efi_memory_desc_t *md);
 extern void efi_sync_low_kernel_mappings(void);
 extern int __init efi_alloc_page_tables(void);
 extern int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages);
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 463b784499a8..0a614039e4bb 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -690,7 +690,8 @@  static void * __init efi_map_regions(int *count, int *pg_shift)
 		if (!should_map_region(md))
 			continue;
 
-		efi_map_region(md);
+		if (efi_map_region(md))
+			return NULL;
 
 		if (left < desc_size) {
 			new_memmap = realloc_pages(new_memmap, *pg_shift);
@@ -736,9 +737,10 @@  static void __init kexec_enter_virtual_mode(void)
 	* Map efi regions which were passed via setup_data. The virt_addr is a
 	* fixed addr which was used in first kernel of a kexec boot.
 	*/
-	for_each_efi_memory_desc(md)
-		efi_map_region_fixed(md); /* FIXME: add error handling */
-
+	for_each_efi_memory_desc(md) {
+		if (efi_map_region_fixed(md))
+			return;
+	}
 	/*
 	 * Unregister the early EFI memmap from efi_init() and install
 	 * the new EFI memory map.
diff --git a/arch/x86/platform/efi/efi_32.c b/arch/x86/platform/efi/efi_32.c
index b2cc7b4552a1..5ca63a72b1f1 100644
--- a/arch/x86/platform/efi/efi_32.c
+++ b/arch/x86/platform/efi/efi_32.c
@@ -33,7 +33,7 @@ 
 #include <asm/tlbflush.h>
 #include <asm/efi.h>
 
-void __init efi_map_region(efi_memory_desc_t *md)
+int __init efi_map_region(efi_memory_desc_t *md)
 {
 	u64 start_pfn, end_pfn, end;
 	unsigned long size;
@@ -54,8 +54,11 @@  void __init efi_map_region(efi_memory_desc_t *md)
 	}
 
 	md->virt_addr = (unsigned long)va;
-	if (!va)
+	if (!va) {
 		pr_err("ioremap of 0x%llX failed!\n", md->phys_addr);
+		return -EINVAL;
+	}
+	return 0;
 }
 
 /*
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index e7e8f77f77f8..b75a9557685c 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -268,12 +268,12 @@  int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 	return 0;
 }
 
-static void __init __map_region(efi_memory_desc_t *md, u64 va)
+static int __init __map_region(efi_memory_desc_t *md, u64 va)
 {
 	unsigned long flags = _PAGE_RW;
 	unsigned long pfn;
 	pgd_t *pgd = efi_mm.pgd;
-
+	int retval;
 	/*
 	 * EFI_RUNTIME_SERVICES_CODE regions typically cover PE/COFF
 	 * executable images in memory that consist of both R-X and
@@ -298,22 +298,25 @@  static void __init __map_region(efi_memory_desc_t *md, u64 va)
 		flags |= _PAGE_ENC;
 
 	pfn = md->phys_addr >> PAGE_SHIFT;
-	if (kernel_map_pages_in_pgd(pgd, pfn, va, md->num_pages, flags))
+	retval = kernel_map_pages_in_pgd(pgd, pfn, va, md->num_pages, flags);	
+	if (retval)
 		pr_warn("Error mapping PA 0x%llx -> VA 0x%llx!\n",
 			   md->phys_addr, va);
+	return retval;
 }
 
-void __init efi_map_region(efi_memory_desc_t *md)
+int __init efi_map_region(efi_memory_desc_t *md)
 {
 	unsigned long size = md->num_pages << PAGE_SHIFT;
 	u64 pa = md->phys_addr;
+	int retval = 0;
 
 	/*
 	 * Make sure the 1:1 mappings are present as a catch-all for b0rked
 	 * firmware which doesn't update all internal pointers after switching
 	 * to virtual mode and would otherwise crap on us.
 	 */
-	__map_region(md, md->phys_addr);
+	retval = __map_region(md, md->phys_addr);
 
 	/*
 	 * Enforce the 1:1 mapping as the default virtual address when
@@ -322,7 +325,7 @@  void __init efi_map_region(efi_memory_desc_t *md)
 	 */
 	if (efi_is_mixed()) {
 		md->virt_addr = md->phys_addr;
-		return;
+		return retval;
 	}
 
 	efi_va -= size;
@@ -343,12 +346,13 @@  void __init efi_map_region(efi_memory_desc_t *md)
 
 	if (efi_va < EFI_VA_END) {
 		pr_warn(FW_WARN "VA address range overflow!\n");
-		return;
+		return retval;
 	}
 
 	/* Do the VA map */
-	__map_region(md, efi_va);
+	retval = __map_region(md, efi_va);
 	md->virt_addr = efi_va;
+	return retval;
 }
 
 /*
@@ -356,10 +360,16 @@  void __init efi_map_region(efi_memory_desc_t *md)
  * md->virt_addr is the original virtual address which had been mapped in kexec
  * 1st kernel.
  */
-void __init efi_map_region_fixed(efi_memory_desc_t *md)
-{
-	__map_region(md, md->phys_addr);
-	__map_region(md, md->virt_addr);
+int __init efi_map_region_fixed(efi_memory_desc_t *md)
+{
+	int retval;
+	retval = __map_region(md, md->phys_addr);
+	if (retval)
+		return retval;
+	retval = __map_region(md, md->virt_addr);
+	if (retval)
+		return retval;
+	return 0;
 }
 
 void __init parse_efi_setup(u64 phys_addr, u32 data_len)