[PATCH 00/10] efi: some cleanups/refactoring for efi/next

[PATCH 00/10] efi: some cleanups/refactoring for efi/next

[Arvind Sankar]

This series is on top of efi/next.

Patch 1 fixes the size allocated for x86 boot_params.
Patch 2 refactors the setting of various hi/lo 32-bit fields, mainly on x86.
Patches 3-5 convert the remaining uses of efi_printk to print error
messages to use pr_efi_err instead.
Patches 6-8 refactor initrd loading, moving it into efi-stub-helper.
Patch 9 adds support for x86 builtin command line.
Patch 10 adds error checking for efi_parse_options.

Arvind Sankar (10):
  efi/x86: Use correct size for boot_params
  efi/libstub: Add a helper function to split 64-bit values
  efi/x86: Use pr_efi_err for error messages
  efi/gop: Use pr_efi_err for error messages
  efi/tpm: Use pr_efi_err for error messages
  efi/x86: Move command-line initrd loading to efi_main
  efi/libstub: Unify initrd loading across architectures
  efi/x86: Drop soft_limit for x86 initrd loading
  efi/x86: Support builtin command line
  efi/libstub: Check return value of efi_parse_options

 .../firmware/efi/libstub/efi-stub-helper.c    |  42 +++++-
 drivers/firmware/efi/libstub/efi-stub.c       |  29 +++--
 drivers/firmware/efi/libstub/efistub.h        |  32 ++---
 drivers/firmware/efi/libstub/file.c           |  13 +-
 drivers/firmware/efi/libstub/gop.c            |  16 +--
 drivers/firmware/efi/libstub/tpm.c            |   2 +-
 drivers/firmware/efi/libstub/x86-stub.c       | 121 ++++++++----------
 7 files changed, 130 insertions(+), 125 deletions(-)

-- 
2.26.2

[PATCH 01/10] efi/x86: Use correct size for boot_params

[Arvind Sankar]

struct boot_params is only 4096 bytes, not 16384. Fix this by using
sizeof(struct boot_params) instead of hardcoding the incorrect value.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/x86-stub.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 597793fe8d22..d4bafd7f6f9f 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -379,13 +379,14 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	image_base = efi_table_attr(image, image_base);
 	image_offset = (void *)startup_32 - image_base;
 
-	status = efi_allocate_pages(0x4000, (unsigned long *)&boot_params, ULONG_MAX);
+	status = efi_allocate_pages(sizeof(struct boot_params),
+				    (unsigned long *)&boot_params, ULONG_MAX);
 	if (status != EFI_SUCCESS) {
 		efi_printk("Failed to allocate lowmem for boot params\n");
 		efi_exit(handle, status);
 	}
 
-	memset(boot_params, 0x0, 0x4000);
+	memset(boot_params, 0x0, sizeof(struct boot_params));
 
 	hdr = &boot_params->hdr;
 
@@ -439,7 +440,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 fail2:
 	efi_free(options_size, (unsigned long)cmdline_ptr);
 fail:
-	efi_free(0x4000, (unsigned long)boot_params);
+	efi_free(sizeof(struct boot_params), (unsigned long)boot_params);
 
 	efi_exit(handle, status);
 }
-- 
2.26.2

[PATCH 02/10] efi: Add a helper function to split 64-bit values

[Arvind Sankar]

In several places 64-bit values need to be split up into two 32-bit
fields, in order to be backward-compatible with the old 32-bit ABIs.

Instead of open-coding this, add a helper function to set a 64-bit value
as two 32-bit fields.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/efistub.h  |  7 ++++++
 drivers/firmware/efi/libstub/gop.c      |  6 ++---
 drivers/firmware/efi/libstub/x86-stub.c | 32 +++++++++++--------------
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 5ff63230a1f1..e8aa70ba08d5 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -87,6 +87,13 @@ extern const efi_system_table_t *efi_system_table;
 		((handle = efi_get_handle_at((array), i)) || true);	\
 	     i++)
 
+static inline
+void efi_set_u64_split(u64 data, u32 *lo, u32 *hi)
+{
+	*lo = lower_32_bits(data);
+	*hi = upper_32_bits(data);
+}
+
 /*
  * Allocation types for calls to boottime->allocate_pages.
  */
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 216327d0b034..64cee0febae0 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -422,7 +422,6 @@ static efi_status_t setup_gop(struct screen_info *si, efi_guid_t *proto,
 	efi_graphics_output_protocol_t *gop;
 	efi_graphics_output_protocol_mode_t *mode;
 	efi_graphics_output_mode_info_t *info;
-	efi_physical_addr_t fb_base;
 
 	gop = find_gop(proto, size, handles);
 
@@ -442,9 +441,8 @@ static efi_status_t setup_gop(struct screen_info *si, efi_guid_t *proto,
 	si->lfb_width  = info->horizontal_resolution;
 	si->lfb_height = info->vertical_resolution;
 
-	fb_base		 = efi_table_attr(mode, frame_buffer_base);
-	si->lfb_base	 = lower_32_bits(fb_base);
-	si->ext_lfb_base = upper_32_bits(fb_base);
+	efi_set_u64_split(efi_table_attr(mode, frame_buffer_base),
+			  &si->lfb_base, &si->ext_lfb_base);
 	if (si->ext_lfb_base)
 		si->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
 
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index d4bafd7f6f9f..677b5a1e0543 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -408,9 +408,8 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	if (!cmdline_ptr)
 		goto fail;
 
-	hdr->cmd_line_ptr = (unsigned long)cmdline_ptr;
-	/* Fill in upper bits of command line address, NOP on 32 bit  */
-	boot_params->ext_cmd_line_ptr = (u64)(unsigned long)cmdline_ptr >> 32;
+	efi_set_u64_split((u64)cmdline_ptr,
+			  &hdr->cmd_line_ptr, &boot_params->ext_cmd_line_ptr);
 
 	hdr->ramdisk_image = 0;
 	hdr->ramdisk_size = 0;
@@ -427,10 +426,10 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 						 ULONG_MAX);
 			if (status != EFI_SUCCESS)
 				goto fail2;
-			hdr->ramdisk_image = ramdisk_addr & 0xffffffff;
-			hdr->ramdisk_size  = ramdisk_size & 0xffffffff;
-			boot_params->ext_ramdisk_image = (u64)ramdisk_addr >> 32;
-			boot_params->ext_ramdisk_size  = (u64)ramdisk_size >> 32;
+			efi_set_u64_split(ramdisk_addr, &hdr->ramdisk_image,
+					  &boot_params->ext_ramdisk_image);
+			efi_set_u64_split(ramdisk_size, &hdr->ramdisk_size,
+					  &boot_params->ext_ramdisk_size);
 		}
 	}
 
@@ -639,17 +638,14 @@ static efi_status_t exit_boot_func(struct efi_boot_memmap *map,
 				   : EFI32_LOADER_SIGNATURE;
 	memcpy(&p->efi->efi_loader_signature, signature, sizeof(__u32));
 
-	p->efi->efi_systab		= (unsigned long)efi_system_table;
+	efi_set_u64_split((u64)efi_system_table,
+			  &p->efi->efi_systab, &p->efi->efi_systab_hi);
 	p->efi->efi_memdesc_size	= *map->desc_size;
 	p->efi->efi_memdesc_version	= *map->desc_ver;
-	p->efi->efi_memmap		= (unsigned long)*map->map;
+	efi_set_u64_split((u64)*map->map,
+			  &p->efi->efi_memmap, &p->efi->efi_memmap_hi);
 	p->efi->efi_memmap_size		= *map->map_size;
 
-#ifdef CONFIG_X86_64
-	p->efi->efi_systab_hi		= (unsigned long)efi_system_table >> 32;
-	p->efi->efi_memmap_hi		= (unsigned long)*map->map >> 32;
-#endif
-
 	return EFI_SUCCESS;
 }
 
@@ -785,10 +781,10 @@ unsigned long efi_main(efi_handle_t handle,
 
 		status = efi_load_initrd_dev_path(&addr, &size, ULONG_MAX);
 		if (status == EFI_SUCCESS) {
-			hdr->ramdisk_image		= (u32)addr;
-			hdr->ramdisk_size 		= (u32)size;
-			boot_params->ext_ramdisk_image	= (u64)addr >> 32;
-			boot_params->ext_ramdisk_size 	= (u64)size >> 32;
+			efi_set_u64_split(addr, &hdr->ramdisk_image,
+					  &boot_params->ext_ramdisk_image);
+			efi_set_u64_split(size, &hdr->ramdisk_size,
+					  &boot_params->ext_ramdisk_size);
 		} else if (status != EFI_NOT_FOUND) {
 			efi_printk("efi_load_initrd_dev_path() failed!\n");
 			goto fail;
-- 
2.26.2

Re: [PATCH 02/10] efi: Add a helper function to split 64-bit values

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5417 bytes --]

Hi Arvind,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on efi/next]
[also build test WARNING on next-20200429]
[cannot apply to v5.7-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Arvind-Sankar/efi-some-cleanups-refactoring-for-efi-next/20200430-051025
base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
config: i386-defconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/firmware/efi/libstub/x86-stub.c: In function 'efi_pe_entry':
>> drivers/firmware/efi/libstub/x86-stub.c:411:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     efi_set_u64_split((u64)cmdline_ptr,
                       ^
   drivers/firmware/efi/libstub/x86-stub.c: In function 'exit_boot_func':
   drivers/firmware/efi/libstub/x86-stub.c:641:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     efi_set_u64_split((u64)efi_system_table,
                       ^
   drivers/firmware/efi/libstub/x86-stub.c:645:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     efi_set_u64_split((u64)*map->map,
                       ^

vim +411 drivers/firmware/efi/libstub/x86-stub.c

   343	
   344	void __noreturn efi_stub_entry(efi_handle_t handle,
   345				       efi_system_table_t *sys_table_arg,
   346				       struct boot_params *boot_params);
   347	
   348	/*
   349	 * Because the x86 boot code expects to be passed a boot_params we
   350	 * need to create one ourselves (usually the bootloader would create
   351	 * one for us).
   352	 */
   353	efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
   354					   efi_system_table_t *sys_table_arg)
   355	{
   356		struct boot_params *boot_params;
   357		struct setup_header *hdr;
   358		efi_loaded_image_t *image;
   359		void *image_base;
   360		efi_guid_t proto = LOADED_IMAGE_PROTOCOL_GUID;
   361		int options_size = 0;
   362		efi_status_t status;
   363		char *cmdline_ptr;
   364		unsigned long ramdisk_addr;
   365		unsigned long ramdisk_size;
   366	
   367		efi_system_table = sys_table_arg;
   368	
   369		/* Check if we were booted by the EFI firmware */
   370		if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
   371			efi_exit(handle, EFI_INVALID_PARAMETER);
   372	
   373		status = efi_bs_call(handle_protocol, handle, &proto, (void **)&image);
   374		if (status != EFI_SUCCESS) {
   375			efi_printk("Failed to get handle for LOADED_IMAGE_PROTOCOL\n");
   376			efi_exit(handle, status);
   377		}
   378	
   379		image_base = efi_table_attr(image, image_base);
   380		image_offset = (void *)startup_32 - image_base;
   381	
   382		status = efi_allocate_pages(sizeof(struct boot_params),
   383					    (unsigned long *)&boot_params, ULONG_MAX);
   384		if (status != EFI_SUCCESS) {
   385			efi_printk("Failed to allocate lowmem for boot params\n");
   386			efi_exit(handle, status);
   387		}
   388	
   389		memset(boot_params, 0x0, sizeof(struct boot_params));
   390	
   391		hdr = &boot_params->hdr;
   392	
   393		/* Copy the second sector to boot_params */
   394		memcpy(&hdr->jump, image_base + 512, 512);
   395	
   396		/*
   397		 * Fill out some of the header fields ourselves because the
   398		 * EFI firmware loader doesn't load the first sector.
   399		 */
   400		hdr->root_flags	= 1;
   401		hdr->vid_mode	= 0xffff;
   402		hdr->boot_flag	= 0xAA55;
   403	
   404		hdr->type_of_loader = 0x21;
   405	
   406		/* Convert unicode cmdline to ascii */
   407		cmdline_ptr = efi_convert_cmdline(image, &options_size, ULONG_MAX);
   408		if (!cmdline_ptr)
   409			goto fail;
   410	
 > 411		efi_set_u64_split((u64)cmdline_ptr,
   412				  &hdr->cmd_line_ptr, &boot_params->ext_cmd_line_ptr);
   413	
   414		hdr->ramdisk_image = 0;
   415		hdr->ramdisk_size = 0;
   416	
   417		if (efi_is_native()) {
   418			status = efi_parse_options(cmdline_ptr);
   419			if (status != EFI_SUCCESS)
   420				goto fail2;
   421	
   422			if (!efi_noinitrd) {
   423				status = efi_load_initrd(image, &ramdisk_addr,
   424							 &ramdisk_size,
   425							 hdr->initrd_addr_max,
   426							 ULONG_MAX);
   427				if (status != EFI_SUCCESS)
   428					goto fail2;
   429				efi_set_u64_split(ramdisk_addr, &hdr->ramdisk_image,
   430						  &boot_params->ext_ramdisk_image);
   431				efi_set_u64_split(ramdisk_size, &hdr->ramdisk_size,
   432						  &boot_params->ext_ramdisk_size);
   433			}
   434		}
   435	
   436		efi_stub_entry(handle, sys_table_arg, boot_params);
   437		/* not reached */
   438	
   439	fail2:
   440		efi_free(options_size, (unsigned long)cmdline_ptr);
   441	fail:
   442		efi_free(sizeof(struct boot_params), (unsigned long)boot_params);
   443	
   444		efi_exit(handle, status);
   445	}
   446	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28397 bytes --]

[PATCH 02/10] efi/libstub: Add a helper function to split 64-bit values

[Arvind Sankar]

In several places 64-bit values need to be split up into two 32-bit
fields, in order to be backward-compatible with the old 32-bit ABIs.

Instead of open-coding this, add a helper function to set a 64-bit value
as two 32-bit fields.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/efistub.h  |  7 ++++++
 drivers/firmware/efi/libstub/gop.c      |  6 ++---
 drivers/firmware/efi/libstub/x86-stub.c | 32 +++++++++++--------------
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 5ff63230a1f1..e8aa70ba08d5 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -87,6 +87,13 @@ extern const efi_system_table_t *efi_system_table;
 		((handle = efi_get_handle_at((array), i)) || true);	\
 	     i++)
 
+static inline
+void efi_set_u64_split(u64 data, u32 *lo, u32 *hi)
+{
+	*lo = lower_32_bits(data);
+	*hi = upper_32_bits(data);
+}
+
 /*
  * Allocation types for calls to boottime->allocate_pages.
  */
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 216327d0b034..64cee0febae0 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -422,7 +422,6 @@ static efi_status_t setup_gop(struct screen_info *si, efi_guid_t *proto,
 	efi_graphics_output_protocol_t *gop;
 	efi_graphics_output_protocol_mode_t *mode;
 	efi_graphics_output_mode_info_t *info;
-	efi_physical_addr_t fb_base;
 
 	gop = find_gop(proto, size, handles);
 
@@ -442,9 +441,8 @@ static efi_status_t setup_gop(struct screen_info *si, efi_guid_t *proto,
 	si->lfb_width  = info->horizontal_resolution;
 	si->lfb_height = info->vertical_resolution;
 
-	fb_base		 = efi_table_attr(mode, frame_buffer_base);
-	si->lfb_base	 = lower_32_bits(fb_base);
-	si->ext_lfb_base = upper_32_bits(fb_base);
+	efi_set_u64_split(efi_table_attr(mode, frame_buffer_base),
+			  &si->lfb_base, &si->ext_lfb_base);
 	if (si->ext_lfb_base)
 		si->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
 
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index d4bafd7f6f9f..677b5a1e0543 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -408,9 +408,8 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	if (!cmdline_ptr)
 		goto fail;
 
-	hdr->cmd_line_ptr = (unsigned long)cmdline_ptr;
-	/* Fill in upper bits of command line address, NOP on 32 bit  */
-	boot_params->ext_cmd_line_ptr = (u64)(unsigned long)cmdline_ptr >> 32;
+	efi_set_u64_split((u64)cmdline_ptr,
+			  &hdr->cmd_line_ptr, &boot_params->ext_cmd_line_ptr);
 
 	hdr->ramdisk_image = 0;
 	hdr->ramdisk_size = 0;
@@ -427,10 +426,10 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 						 ULONG_MAX);
 			if (status != EFI_SUCCESS)
 				goto fail2;
-			hdr->ramdisk_image = ramdisk_addr & 0xffffffff;
-			hdr->ramdisk_size  = ramdisk_size & 0xffffffff;
-			boot_params->ext_ramdisk_image = (u64)ramdisk_addr >> 32;
-			boot_params->ext_ramdisk_size  = (u64)ramdisk_size >> 32;
+			efi_set_u64_split(ramdisk_addr, &hdr->ramdisk_image,
+					  &boot_params->ext_ramdisk_image);
+			efi_set_u64_split(ramdisk_size, &hdr->ramdisk_size,
+					  &boot_params->ext_ramdisk_size);
 		}
 	}
 
@@ -639,17 +638,14 @@ static efi_status_t exit_boot_func(struct efi_boot_memmap *map,
 				   : EFI32_LOADER_SIGNATURE;
 	memcpy(&p->efi->efi_loader_signature, signature, sizeof(__u32));
 
-	p->efi->efi_systab		= (unsigned long)efi_system_table;
+	efi_set_u64_split((u64)efi_system_table,
+			  &p->efi->efi_systab, &p->efi->efi_systab_hi);
 	p->efi->efi_memdesc_size	= *map->desc_size;
 	p->efi->efi_memdesc_version	= *map->desc_ver;
-	p->efi->efi_memmap		= (unsigned long)*map->map;
+	efi_set_u64_split((u64)*map->map,
+			  &p->efi->efi_memmap, &p->efi->efi_memmap_hi);
 	p->efi->efi_memmap_size		= *map->map_size;
 
-#ifdef CONFIG_X86_64
-	p->efi->efi_systab_hi		= (unsigned long)efi_system_table >> 32;
-	p->efi->efi_memmap_hi		= (unsigned long)*map->map >> 32;
-#endif
-
 	return EFI_SUCCESS;
 }
 
@@ -785,10 +781,10 @@ unsigned long efi_main(efi_handle_t handle,
 
 		status = efi_load_initrd_dev_path(&addr, &size, ULONG_MAX);
 		if (status == EFI_SUCCESS) {
-			hdr->ramdisk_image		= (u32)addr;
-			hdr->ramdisk_size 		= (u32)size;
-			boot_params->ext_ramdisk_image	= (u64)addr >> 32;
-			boot_params->ext_ramdisk_size 	= (u64)size >> 32;
+			efi_set_u64_split(addr, &hdr->ramdisk_image,
+					  &boot_params->ext_ramdisk_image);
+			efi_set_u64_split(size, &hdr->ramdisk_size,
+					  &boot_params->ext_ramdisk_size);
 		} else if (status != EFI_NOT_FOUND) {
 			efi_printk("efi_load_initrd_dev_path() failed!\n");
 			goto fail;
-- 
2.26.2

[PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Arvind Sankar]

Use pr_efi_err instead of bare efi_printk for error messages.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/x86-stub.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 677b5a1e0543..933205772d8c 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -49,7 +49,7 @@ preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
 	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, size,
 			     (void **)&rom);
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to allocate memory for 'rom'\n");
+		pr_efi_err("Failed to allocate memory for 'rom'\n");
 		return status;
 	}
 
@@ -65,7 +65,7 @@ preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
 				PCI_VENDOR_ID, 1, &rom->vendor);
 
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to read rom->vendor\n");
+		pr_efi_err("Failed to read rom->vendor\n");
 		goto free_struct;
 	}
 
@@ -73,7 +73,7 @@ preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
 				PCI_DEVICE_ID, 1, &rom->devid);
 
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to read rom->devid\n");
+		pr_efi_err("Failed to read rom->devid\n");
 		goto free_struct;
 	}
 
@@ -118,7 +118,7 @@ static void setup_efi_pci(struct boot_params *params)
 				     (void **)&pci_handle);
 
 		if (status != EFI_SUCCESS) {
-			efi_printk("Failed to allocate memory for 'pci_handle'\n");
+			pr_efi_err("Failed to allocate memory for 'pci_handle'\n");
 			return;
 		}
 
@@ -172,7 +172,7 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params)
 		return;
 
 	if (efi_table_attr(p, version) != 0x10000) {
-		efi_printk("Unsupported properties proto version\n");
+		pr_efi_err("Unsupported properties proto version\n");
 		return;
 	}
 
@@ -185,7 +185,7 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params)
 				     size + sizeof(struct setup_data),
 				     (void **)&new);
 		if (status != EFI_SUCCESS) {
-			efi_printk("Failed to allocate memory for 'properties'\n");
+			pr_efi_err("Failed to allocate memory for 'properties'\n");
 			return;
 		}
 
@@ -372,7 +372,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 
 	status = efi_bs_call(handle_protocol, handle, &proto, (void **)&image);
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to get handle for LOADED_IMAGE_PROTOCOL\n");
+		pr_efi_err("Failed to get handle for LOADED_IMAGE_PROTOCOL\n");
 		efi_exit(handle, status);
 	}
 
@@ -382,7 +382,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	status = efi_allocate_pages(sizeof(struct boot_params),
 				    (unsigned long *)&boot_params, ULONG_MAX);
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to allocate lowmem for boot params\n");
+		pr_efi_err("Failed to allocate lowmem for boot params\n");
 		efi_exit(handle, status);
 	}
 
@@ -749,7 +749,7 @@ unsigned long efi_main(efi_handle_t handle,
 					     hdr->kernel_alignment,
 					     LOAD_PHYSICAL_ADDR);
 		if (status != EFI_SUCCESS) {
-			efi_printk("efi_relocate_kernel() failed!\n");
+			pr_efi_err("efi_relocate_kernel() failed!\n");
 			goto fail;
 		}
 		/*
@@ -786,7 +786,7 @@ unsigned long efi_main(efi_handle_t handle,
 			efi_set_u64_split(size, &hdr->ramdisk_size,
 					  &boot_params->ext_ramdisk_size);
 		} else if (status != EFI_NOT_FOUND) {
-			efi_printk("efi_load_initrd_dev_path() failed!\n");
+			pr_efi_err("efi_load_initrd_dev_path() failed!\n");
 			goto fail;
 		}
 	}
@@ -813,13 +813,13 @@ unsigned long efi_main(efi_handle_t handle,
 
 	status = exit_boot(boot_params, handle);
 	if (status != EFI_SUCCESS) {
-		efi_printk("exit_boot() failed!\n");
+		pr_efi_err("exit_boot() failed!\n");
 		goto fail;
 	}
 
 	return bzimage_addr;
 fail:
-	efi_printk("efi_main() failed!\n");
+	pr_efi_err("efi_main() failed!\n");
 
 	efi_exit(handle, status);
 }
-- 
2.26.2

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Joe Perches]

On Wed, 2020-04-29 at 13:41 -0400, Arvind Sankar wrote:
> Use pr_efi_err instead of bare efi_printk for error messages.

Perhaps it'd be better to rename pr_efi_err to eri_err
to it's clearer it's a typical efi_ logging function.

$ git grep -w --name-only pr_efi_err | \
  xargs sed -i 's/\bpr_efi_err\b/efi_err/g'

Looking at code for efi_printk -> efi_char16_printk,
it's somewhat difficult to see where the "output_string"
function pointer is set.  Any clue?

> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
>  drivers/firmware/efi/libstub/x86-stub.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index 677b5a1e0543..933205772d8c 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -49,7 +49,7 @@ preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
>  	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, size,
>  			     (void **)&rom);
>  	if (status != EFI_SUCCESS) {
> -		efi_printk("Failed to allocate memory for 'rom'\n");
> +		pr_efi_err("Failed to allocate memory for 'rom'\n");
>  		return status;
>  	}
>  
> @@ -65,7 +65,7 @@ preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
>  				PCI_VENDOR_ID, 1, &rom->vendor);
>  
>  	if (status != EFI_SUCCESS) {
> -		efi_printk("Failed to read rom->vendor\n");
> +		pr_efi_err("Failed to read rom->vendor\n");
>  		goto free_struct;
>  	}
>  
> @@ -73,7 +73,7 @@ preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
>  				PCI_DEVICE_ID, 1, &rom->devid);
>  
>  	if (status != EFI_SUCCESS) {
> -		efi_printk("Failed to read rom->devid\n");
> +		pr_efi_err("Failed to read rom->devid\n");
>  		goto free_struct;
>  	}
>  
> @@ -118,7 +118,7 @@ static void setup_efi_pci(struct boot_params *params)
>  				     (void **)&pci_handle);
>  
>  		if (status != EFI_SUCCESS) {
> -			efi_printk("Failed to allocate memory for 'pci_handle'\n");
> +			pr_efi_err("Failed to allocate memory for 'pci_handle'\n");
>  			return;
>  		}
>  
> @@ -172,7 +172,7 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params)
>  		return;
>  
>  	if (efi_table_attr(p, version) != 0x10000) {
> -		efi_printk("Unsupported properties proto version\n");
> +		pr_efi_err("Unsupported properties proto version\n");
>  		return;
>  	}
>  
> @@ -185,7 +185,7 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params)
>  				     size + sizeof(struct setup_data),
>  				     (void **)&new);
>  		if (status != EFI_SUCCESS) {
> -			efi_printk("Failed to allocate memory for 'properties'\n");
> +			pr_efi_err("Failed to allocate memory for 'properties'\n");
>  			return;
>  		}
>  
> @@ -372,7 +372,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
>  
>  	status = efi_bs_call(handle_protocol, handle, &proto, (void **)&image);
>  	if (status != EFI_SUCCESS) {
> -		efi_printk("Failed to get handle for LOADED_IMAGE_PROTOCOL\n");
> +		pr_efi_err("Failed to get handle for LOADED_IMAGE_PROTOCOL\n");
>  		efi_exit(handle, status);
>  	}
>  
> @@ -382,7 +382,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
>  	status = efi_allocate_pages(sizeof(struct boot_params),
>  				    (unsigned long *)&boot_params, ULONG_MAX);
>  	if (status != EFI_SUCCESS) {
> -		efi_printk("Failed to allocate lowmem for boot params\n");
> +		pr_efi_err("Failed to allocate lowmem for boot params\n");
>  		efi_exit(handle, status);
>  	}
>  
> @@ -749,7 +749,7 @@ unsigned long efi_main(efi_handle_t handle,
>  					     hdr->kernel_alignment,
>  					     LOAD_PHYSICAL_ADDR);
>  		if (status != EFI_SUCCESS) {
> -			efi_printk("efi_relocate_kernel() failed!\n");
> +			pr_efi_err("efi_relocate_kernel() failed!\n");
>  			goto fail;
>  		}
>  		/*
> @@ -786,7 +786,7 @@ unsigned long efi_main(efi_handle_t handle,
>  			efi_set_u64_split(size, &hdr->ramdisk_size,
>  					  &boot_params->ext_ramdisk_size);
>  		} else if (status != EFI_NOT_FOUND) {
> -			efi_printk("efi_load_initrd_dev_path() failed!\n");
> +			pr_efi_err("efi_load_initrd_dev_path() failed!\n");
>  			goto fail;
>  		}
>  	}
> @@ -813,13 +813,13 @@ unsigned long efi_main(efi_handle_t handle,
>  
>  	status = exit_boot(boot_params, handle);
>  	if (status != EFI_SUCCESS) {
> -		efi_printk("exit_boot() failed!\n");
> +		pr_efi_err("exit_boot() failed!\n");
>  		goto fail;
>  	}
>  
>  	return bzimage_addr;
>  fail:
> -	efi_printk("efi_main() failed!\n");
> +	pr_efi_err("efi_main() failed!\n");
>  
>  	efi_exit(handle, status);
>  }

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Ard Biesheuvel]

On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
>
> On Wed, 2020-04-29 at 13:41 -0400, Arvind Sankar wrote:
> > Use pr_efi_err instead of bare efi_printk for error messages.
>
> Perhaps it'd be better to rename pr_efi_err to eri_err
> to it's clearer it's a typical efi_ logging function.
>
> $ git grep -w --name-only pr_efi_err | \
>   xargs sed -i 's/\bpr_efi_err\b/efi_err/g'
>

Yeah, pr_efi_err() is probably not the best name

> Looking at code for efi_printk -> efi_char16_printk,
> it's somewhat difficult to see where the "output_string"
> function pointer is set.  Any clue?
>

It is set by the firmware.

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Joe Perches]

On Wed, 2020-04-29 at 20:49 +0200, Ard Biesheuvel wrote:
> On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
> > Looking at code for efi_printk -> efi_char16_printk,
> > it's somewhat difficult to see where the "output_string"
> > function pointer is set.  Any clue?
> It is set by the firmware.

Sure, where in the code though?

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Ard Biesheuvel]

On Wed, 29 Apr 2020 at 20:57, Joe Perches <joe@perches.com> wrote:
>
> On Wed, 2020-04-29 at 20:49 +0200, Ard Biesheuvel wrote:
> > On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
> > > Looking at code for efi_printk -> efi_char16_printk,
> > > it's somewhat difficult to see where the "output_string"
> > > function pointer is set.  Any clue?
> > It is set by the firmware.
>
> Sure, where in the code though?
>

In which code? The firmware code?

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Joe Perches]

On Wed, 2020-04-29 at 20:59 +0200, Ard Biesheuvel wrote:
> On Wed, 29 Apr 2020 at 20:57, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2020-04-29 at 20:49 +0200, Ard Biesheuvel wrote:
> > > On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
> > > > Looking at code for efi_printk -> efi_char16_printk,
> > > > it's somewhat difficult to see where the "output_string"
> > > > function pointer is set.  Any clue?
> > > It is set by the firmware.
> > 
> > Sure, where in the code though?
> > 
> 
> In which code? The firmware code?

I presume it's set from a struct received from hardware/firmware
somewhere in drivers/firmware/efi, but it doesn't seem clear where.

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Ard Biesheuvel]

On Wed, 29 Apr 2020 at 21:47, Joe Perches <joe@perches.com> wrote:
>
> On Wed, 2020-04-29 at 20:59 +0200, Ard Biesheuvel wrote:
> > On Wed, 29 Apr 2020 at 20:57, Joe Perches <joe@perches.com> wrote:
> > > On Wed, 2020-04-29 at 20:49 +0200, Ard Biesheuvel wrote:
> > > > On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
> > > > > Looking at code for efi_printk -> efi_char16_printk,
> > > > > it's somewhat difficult to see where the "output_string"
> > > > > function pointer is set.  Any clue?
> > > > It is set by the firmware.
> > >
> > > Sure, where in the code though?
> > >
> >
> > In which code? The firmware code?
>
> I presume it's set from a struct received from hardware/firmware
> somewhere in drivers/firmware/efi, but it doesn't seem clear where.
>

It is a field in the EFI system table, which we dereference directly.

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Arvind Sankar]

On Wed, Apr 29, 2020 at 08:49:21PM +0200, Ard Biesheuvel wrote:
> On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
> >
> > On Wed, 2020-04-29 at 13:41 -0400, Arvind Sankar wrote:
> > > Use pr_efi_err instead of bare efi_printk for error messages.
> >
> > Perhaps it'd be better to rename pr_efi_err to eri_err
> > to it's clearer it's a typical efi_ logging function.
> >
> > $ git grep -w --name-only pr_efi_err | \
> >   xargs sed -i 's/\bpr_efi_err\b/efi_err/g'
> >
> 
> Yeah, pr_efi_err() is probably not the best name

Should I rename pr_efi/pr_efi_err to, say, efi_pr_info/efi_pr_error?

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Ard Biesheuvel]

On Wed, 29 Apr 2020 at 23:43, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Wed, Apr 29, 2020 at 08:49:21PM +0200, Ard Biesheuvel wrote:
> > On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
> > >
> > > On Wed, 2020-04-29 at 13:41 -0400, Arvind Sankar wrote:
> > > > Use pr_efi_err instead of bare efi_printk for error messages.
> > >
> > > Perhaps it'd be better to rename pr_efi_err to eri_err
> > > to it's clearer it's a typical efi_ logging function.
> > >
> > > $ git grep -w --name-only pr_efi_err | \
> > >   xargs sed -i 's/\bpr_efi_err\b/efi_err/g'
> > >
> >
> > Yeah, pr_efi_err() is probably not the best name
>
> Should I rename pr_efi/pr_efi_err to, say, efi_pr_info/efi_pr_error?

If you don't mind spinning another couple of patches, yes, that would
be helpful.

Let's use efi_pr_info and efi_pr_err to stay aligned with the ordinary
counterparts

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Arvind Sankar]

On Wed, Apr 29, 2020 at 11:45:17PM +0200, Ard Biesheuvel wrote:
> On Wed, 29 Apr 2020 at 23:43, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > On Wed, Apr 29, 2020 at 08:49:21PM +0200, Ard Biesheuvel wrote:
> > > On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
> > > >
> > > > On Wed, 2020-04-29 at 13:41 -0400, Arvind Sankar wrote:
> > > > > Use pr_efi_err instead of bare efi_printk for error messages.
> > > >
> > > > Perhaps it'd be better to rename pr_efi_err to eri_err
> > > > to it's clearer it's a typical efi_ logging function.
> > > >
> > > > $ git grep -w --name-only pr_efi_err | \
> > > >   xargs sed -i 's/\bpr_efi_err\b/efi_err/g'
> > > >
> > >
> > > Yeah, pr_efi_err() is probably not the best name
> >
> > Should I rename pr_efi/pr_efi_err to, say, efi_pr_info/efi_pr_error?
> 
> If you don't mind spinning another couple of patches, yes, that would
> be helpful.

No problem.

> 
> Let's use efi_pr_info and efi_pr_err to stay aligned with the ordinary
> counterparts

Right, for some reason I thought it was pr_error but it's actually
pr_err.

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Joe Perches]

On Wed, 2020-04-29 at 17:43 -0400, Arvind Sankar wrote:
> On Wed, Apr 29, 2020 at 08:49:21PM +0200, Ard Biesheuvel wrote:
> > On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
> > > On Wed, 2020-04-29 at 13:41 -0400, Arvind Sankar wrote:
> > > > Use pr_efi_err instead of bare efi_printk for error messages.
> > > 
> > > Perhaps it'd be better to rename pr_efi_err to eri_err
> > > so it's clearer it's a typical efi_ logging function.
> > > 
> > > $ git grep -w --name-only pr_efi_err | \
> > >   xargs sed -i 's/\bpr_efi_err\b/efi_err/g'
> > > 
> > 
> > Yeah, pr_efi_err() is probably not the best name
> 
> Should I rename pr_efi/pr_efi_err to, say, efi_pr_info/efi_pr_error?

Perhaps not use pr_ in the name at all.

I suggest:

pr_efi		-> efi_info (or efi_debug or efi_dbg)
		   (it is guarded by an efi_quiet flag, default: on)
pr_efi_err	-> efi_err

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Ard Biesheuvel]

On Wed, 29 Apr 2020 at 23:53, Joe Perches <joe@perches.com> wrote:
>
> On Wed, 2020-04-29 at 17:43 -0400, Arvind Sankar wrote:
> > On Wed, Apr 29, 2020 at 08:49:21PM +0200, Ard Biesheuvel wrote:
> > > On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
> > > > On Wed, 2020-04-29 at 13:41 -0400, Arvind Sankar wrote:
> > > > > Use pr_efi_err instead of bare efi_printk for error messages.
> > > >
> > > > Perhaps it'd be better to rename pr_efi_err to eri_err
> > > > so it's clearer it's a typical efi_ logging function.
> > > >
> > > > $ git grep -w --name-only pr_efi_err | \
> > > >   xargs sed -i 's/\bpr_efi_err\b/efi_err/g'
> > > >
> > >
> > > Yeah, pr_efi_err() is probably not the best name
> >
> > Should I rename pr_efi/pr_efi_err to, say, efi_pr_info/efi_pr_error?
>
> Perhaps not use pr_ in the name at all.
>
> I suggest:
>
> pr_efi          -> efi_info (or efi_debug or efi_dbg)
>                    (it is guarded by an efi_quiet flag, default: on)
> pr_efi_err      -> efi_err
>

Agreed. Shorter is better if there is no risk of confusion..

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Arvind Sankar]

On Wed, Apr 29, 2020 at 11:55:04PM +0200, Ard Biesheuvel wrote:
> On Wed, 29 Apr 2020 at 23:53, Joe Perches <joe@perches.com> wrote:
> >
> > On Wed, 2020-04-29 at 17:43 -0400, Arvind Sankar wrote:
> > > On Wed, Apr 29, 2020 at 08:49:21PM +0200, Ard Biesheuvel wrote:
> > > > On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
> > > > > On Wed, 2020-04-29 at 13:41 -0400, Arvind Sankar wrote:
> > > > > > Use pr_efi_err instead of bare efi_printk for error messages.
> > > > >
> > > > > Perhaps it'd be better to rename pr_efi_err to eri_err
> > > > > so it's clearer it's a typical efi_ logging function.
> > > > >
> > > > > $ git grep -w --name-only pr_efi_err | \
> > > > >   xargs sed -i 's/\bpr_efi_err\b/efi_err/g'
> > > > >
> > > >
> > > > Yeah, pr_efi_err() is probably not the best name
> > >
> > > Should I rename pr_efi/pr_efi_err to, say, efi_pr_info/efi_pr_error?
> >
> > Perhaps not use pr_ in the name at all.
> >
> > I suggest:
> >
> > pr_efi          -> efi_info (or efi_debug or efi_dbg)
> >                    (it is guarded by an efi_quiet flag, default: on)
> > pr_efi_err      -> efi_err
> >
> 
> Agreed. Shorter is better if there is no risk of confusion..

Ok, I'll use efi_info/efi_err. We could add debugging output as
efi_debug later, enabled if efi=debug is specified.

While we're here: most of the existing cases of pr_efi look like notice
or info level, except maybe these two, which probably should be at least
warnings?

drivers/firmware/efi/libstub/arm64-stub.c
62: pr_efi("EFI_RNG_PROTOCOL unavailable, no randomness supplied\n");

drivers/firmware/efi/libstub/efi-stub.c
254: pr_efi("Ignoring DTB from command line.\n");

Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages

[Ard Biesheuvel]

On Thu, 30 Apr 2020 at 00:21, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Wed, Apr 29, 2020 at 11:55:04PM +0200, Ard Biesheuvel wrote:
> > On Wed, 29 Apr 2020 at 23:53, Joe Perches <joe@perches.com> wrote:
> > >
> > > On Wed, 2020-04-29 at 17:43 -0400, Arvind Sankar wrote:
> > > > On Wed, Apr 29, 2020 at 08:49:21PM +0200, Ard Biesheuvel wrote:
> > > > > On Wed, 29 Apr 2020 at 20:47, Joe Perches <joe@perches.com> wrote:
> > > > > > On Wed, 2020-04-29 at 13:41 -0400, Arvind Sankar wrote:
> > > > > > > Use pr_efi_err instead of bare efi_printk for error messages.
> > > > > >
> > > > > > Perhaps it'd be better to rename pr_efi_err to eri_err
> > > > > > so it's clearer it's a typical efi_ logging function.
> > > > > >
> > > > > > $ git grep -w --name-only pr_efi_err | \
> > > > > >   xargs sed -i 's/\bpr_efi_err\b/efi_err/g'
> > > > > >
> > > > >
> > > > > Yeah, pr_efi_err() is probably not the best name
> > > >
> > > > Should I rename pr_efi/pr_efi_err to, say, efi_pr_info/efi_pr_error?
> > >
> > > Perhaps not use pr_ in the name at all.
> > >
> > > I suggest:
> > >
> > > pr_efi          -> efi_info (or efi_debug or efi_dbg)
> > >                    (it is guarded by an efi_quiet flag, default: on)
> > > pr_efi_err      -> efi_err
> > >
> >
> > Agreed. Shorter is better if there is no risk of confusion..
>
> Ok, I'll use efi_info/efi_err. We could add debugging output as
> efi_debug later, enabled if efi=debug is specified.
>
> While we're here: most of the existing cases of pr_efi look like notice
> or info level, except maybe these two, which probably should be at least
> warnings?
>
> drivers/firmware/efi/libstub/arm64-stub.c
> 62: pr_efi("EFI_RNG_PROTOCOL unavailable, no randomness supplied\n");
>

This should not be a warning. KASLR is enabled by default by the
distros, and many systems don't implement this protocol at all.

> drivers/firmware/efi/libstub/efi-stub.c
> 254: pr_efi("Ignoring DTB from command line.\n");

That could be upgraded to an error.

[PATCH 04/10] efi/gop: Use pr_efi_err for error messages

[Arvind Sankar]

Use pr_efi_err instead of bare efi_printk for error messages.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/gop.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 64cee0febae0..1d9bb40acd86 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -134,14 +134,14 @@ static u32 choose_mode_modenum(efi_graphics_output_protocol_t *gop)
 
 	max_mode = efi_table_attr(mode, max_mode);
 	if (cmdline.mode >= max_mode) {
-		efi_printk("Requested mode is invalid\n");
+		pr_efi_err("Requested mode is invalid\n");
 		return cur_mode;
 	}
 
 	status = efi_call_proto(gop, query_mode, cmdline.mode,
 				&info_size, &info);
 	if (status != EFI_SUCCESS) {
-		efi_printk("Couldn't get mode information\n");
+		pr_efi_err("Couldn't get mode information\n");
 		return cur_mode;
 	}
 
@@ -150,7 +150,7 @@ static u32 choose_mode_modenum(efi_graphics_output_protocol_t *gop)
 	efi_bs_call(free_pool, info);
 
 	if (pf == PIXEL_BLT_ONLY || pf >= PIXEL_FORMAT_MAX) {
-		efi_printk("Invalid PixelFormat\n");
+		pr_efi_err("Invalid PixelFormat\n");
 		return cur_mode;
 	}
 
@@ -222,7 +222,7 @@ static u32 choose_mode_res(efi_graphics_output_protocol_t *gop)
 			return m;
 	}
 
-	efi_printk("Couldn't find requested mode\n");
+	pr_efi_err("Couldn't find requested mode\n");
 
 	return cur_mode;
 }
@@ -316,7 +316,7 @@ static void set_mode(efi_graphics_output_protocol_t *gop)
 		return;
 
 	if (efi_call_proto(gop, set_mode, new_mode) != EFI_SUCCESS)
-		efi_printk("Failed to set requested mode\n");
+		pr_efi_err("Failed to set requested mode\n");
 }
 
 static void find_bits(u32 mask, u8 *pos, u8 *size)
-- 
2.26.2

[PATCH 05/10] efi/tpm: Use pr_efi_err for error messages

[Arvind Sankar]

Use pr_efi_err instead of bare efi_printk for error messages.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/tpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
index 1d59e103a2e3..8a16983fad98 100644
--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c
@@ -119,7 +119,7 @@ void efi_retrieve_tpm2_eventlog(void)
 			     sizeof(*log_tbl) + log_size, (void **)&log_tbl);
 
 	if (status != EFI_SUCCESS) {
-		efi_printk("Unable to allocate memory for event log\n");
+		pr_efi_err("Unable to allocate memory for event log\n");
 		return;
 	}
 
-- 
2.26.2

[PATCH 06/10] efi/x86: Move command-line initrd loading to efi_main

[Arvind Sankar]

Consolidate the initrd loading in efi_main.

The command line options now need to be parsed only once.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/x86-stub.c | 64 ++++++++++---------------
 1 file changed, 25 insertions(+), 39 deletions(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 933205772d8c..ee4241df4f29 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -22,6 +22,7 @@
 
 const efi_system_table_t *efi_system_table;
 extern u32 image_offset;
+static efi_loaded_image_t *image = NULL;
 
 static efi_status_t
 preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
@@ -355,7 +356,6 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 {
 	struct boot_params *boot_params;
 	struct setup_header *hdr;
-	efi_loaded_image_t *image;
 	void *image_base;
 	efi_guid_t proto = LOADED_IMAGE_PROTOCOL_GUID;
 	int options_size = 0;
@@ -414,30 +414,9 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	hdr->ramdisk_image = 0;
 	hdr->ramdisk_size = 0;
 
-	if (efi_is_native()) {
-		status = efi_parse_options(cmdline_ptr);
-		if (status != EFI_SUCCESS)
-			goto fail2;
-
-		if (!efi_noinitrd) {
-			status = efi_load_initrd(image, &ramdisk_addr,
-						 &ramdisk_size,
-						 hdr->initrd_addr_max,
-						 ULONG_MAX);
-			if (status != EFI_SUCCESS)
-				goto fail2;
-			efi_set_u64_split(ramdisk_addr, &hdr->ramdisk_image,
-					  &boot_params->ext_ramdisk_image);
-			efi_set_u64_split(ramdisk_size, &hdr->ramdisk_size,
-					  &boot_params->ext_ramdisk_size);
-		}
-	}
-
 	efi_stub_entry(handle, sys_table_arg, boot_params);
 	/* not reached */
 
-fail2:
-	efi_free(options_size, (unsigned long)cmdline_ptr);
 fail:
 	efi_free(sizeof(struct boot_params), (unsigned long)boot_params);
 
@@ -760,35 +739,42 @@ unsigned long efi_main(efi_handle_t handle,
 		image_offset = 0;
 	}
 
-	/*
-	 * efi_pe_entry() may have been called before efi_main(), in which
-	 * case this is the second time we parse the cmdline. This is ok,
-	 * parsing the cmdline multiple times does not have side-effects.
-	 */
 	cmdline_paddr = ((u64)hdr->cmd_line_ptr |
 			 ((u64)boot_params->ext_cmd_line_ptr << 32));
 	efi_parse_options((char *)cmdline_paddr);
 
 	/*
-	 * At this point, an initrd may already have been loaded, either by
-	 * the bootloader and passed via bootparams, or loaded from a initrd=
-	 * command line option by efi_pe_entry() above. In either case, we
-	 * permit an initrd loaded from the LINUX_EFI_INITRD_MEDIA_GUID device
-	 * path to supersede it.
+	 * At this point, an initrd may already have been loaded by the
+	 * bootloader and passed via bootparams. We permit an initrd loaded
+	 * from the LINUX_EFI_INITRD_MEDIA_GUID device path to supersede it.
+	 *
+	 * If the device path is not present, any command-line initrd=
+	 * arguments will be processed only if image is not NULL, which will be
+	 * the case only if we were loaded via the PE entry point.
 	 */
 	if (!efi_noinitrd) {
 		unsigned long addr, size;
 
 		status = efi_load_initrd_dev_path(&addr, &size, ULONG_MAX);
-		if (status == EFI_SUCCESS) {
-			efi_set_u64_split(addr, &hdr->ramdisk_image,
-					  &boot_params->ext_ramdisk_image);
-			efi_set_u64_split(size, &hdr->ramdisk_size,
-					  &boot_params->ext_ramdisk_size);
-		} else if (status != EFI_NOT_FOUND) {
-			pr_efi_err("efi_load_initrd_dev_path() failed!\n");
+		if (status == EFI_NOT_FOUND) {
+			if (efi_is_native() && image != NULL) {
+				status = efi_load_initrd(image, &addr, &size,
+							 hdr->initrd_addr_max,
+							 ULONG_MAX);
+			} else {
+				addr = size = 0;
+				status = EFI_SUCCESS;
+			}
+		}
+
+		if (status != EFI_SUCCESS) {
+			pr_efi_err("Failed to load initrd!\n");
 			goto fail;
 		}
+		efi_set_u64_split(addr, &hdr->ramdisk_image,
+				  &boot_params->ext_ramdisk_image);
+		efi_set_u64_split(size, &hdr->ramdisk_size,
+				  &boot_params->ext_ramdisk_size);
 	}
 
 	/*
-- 
2.26.2

[PATCH 07/10] efi/libstub: Unify initrd loading across architectures

[Arvind Sankar]

Factor out the initrd loading into a common function that can be called
both from the generic efi-stub.c and the x86-specific x86-stub.c.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 .../firmware/efi/libstub/efi-stub-helper.c    | 46 +++++++++++++++++--
 drivers/firmware/efi/libstub/efi-stub.c       | 12 +----
 drivers/firmware/efi/libstub/efistub.h        | 21 ++-------
 drivers/firmware/efi/libstub/x86-stub.c       | 13 +-----
 4 files changed, 52 insertions(+), 40 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 1c92ac231f94..2c0c2c34b4cc 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -331,6 +331,7 @@ static const struct {
  *		%EFI_OUT_OF_RESOURCES if memory allocation failed
  *		%EFI_LOAD_ERROR in all other cases
  */
+static
 efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
 				      unsigned long *load_size,
 				      unsigned long max)
@@ -343,9 +344,6 @@ efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
 	efi_handle_t handle;
 	efi_status_t status;
 
-	if (!load_addr || !load_size)
-		return EFI_INVALID_PARAMETER;
-
 	dp = (efi_device_path_protocol_t *)&initrd_dev_path;
 	status = efi_bs_call(locate_device_path, &lf2_proto_guid, &dp, &handle);
 	if (status != EFI_SUCCESS)
@@ -375,3 +373,45 @@ efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
 	*load_size = initrd_size;
 	return EFI_SUCCESS;
 }
+
+static
+efi_status_t efi_load_initrd_cmdline(efi_loaded_image_t *image,
+				     unsigned long *load_addr,
+				     unsigned long *load_size,
+				     unsigned long soft_limit,
+				     unsigned long hard_limit)
+{
+	if (!IS_ENABLED(CONFIG_EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER) ||
+	    (IS_ENABLED(CONFIG_X86) && (!efi_is_native() || image == NULL))) {
+		*load_addr = *load_size = 0;
+		return EFI_SUCCESS;
+	}
+
+	return handle_cmdline_files(image, L"initrd=", sizeof(L"initrd=") - 2,
+				    soft_limit, hard_limit,
+				    load_addr, load_size);
+}
+
+efi_status_t efi_load_initrd(efi_loaded_image_t *image,
+			     unsigned long *load_addr,
+			     unsigned long *load_size,
+			     unsigned long soft_limit,
+			     unsigned long hard_limit)
+{
+	efi_status_t status;
+
+	if (!load_addr || !load_size)
+		return EFI_INVALID_PARAMETER;
+
+	status = efi_load_initrd_dev_path(load_addr, load_size, hard_limit);
+	if (status == EFI_SUCCESS) {
+		pr_efi("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
+	} else if (status == EFI_NOT_FOUND) {
+		status = efi_load_initrd_cmdline(image, load_addr, load_size,
+						 soft_limit, hard_limit);
+		if (status == EFI_SUCCESS && *load_size > 0)
+			pr_efi("Loaded initrd from command line option\n");
+	}
+
+	return status;
+}
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index ee225b323687..d8f24f5c91bd 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -265,16 +265,8 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 
 	if (!efi_noinitrd) {
 		max_addr = efi_get_max_initrd_addr(dram_base, image_addr);
-		status = efi_load_initrd_dev_path(&initrd_addr, &initrd_size,
-						  max_addr);
-		if (status == EFI_SUCCESS) {
-			pr_efi("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
-		} else if (status == EFI_NOT_FOUND) {
-			status = efi_load_initrd(image, &initrd_addr, &initrd_size,
-						 ULONG_MAX, max_addr);
-			if (status == EFI_SUCCESS && initrd_size > 0)
-				pr_efi("Loaded initrd from command line option\n");
-		}
+		status = efi_load_initrd(image, &initrd_addr, &initrd_size,
+					 ULONG_MAX, max_addr);
 		if (status != EFI_SUCCESS)
 			pr_efi_err("Failed to load initrd!\n");
 	}
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index e8aa70ba08d5..dfdd7954bf58 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -677,21 +677,10 @@ static inline efi_status_t efi_load_dtb(efi_loaded_image_t *image,
 				    ULONG_MAX, ULONG_MAX, load_addr, load_size);
 }
 
-static inline efi_status_t efi_load_initrd(efi_loaded_image_t *image,
-					   unsigned long *load_addr,
-					   unsigned long *load_size,
-					   unsigned long soft_limit,
-					   unsigned long hard_limit)
-{
-	if (!IS_ENABLED(CONFIG_EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER))
-		return EFI_SUCCESS;
-
-	return handle_cmdline_files(image, L"initrd=", sizeof(L"initrd=") - 2,
-				    soft_limit, hard_limit, load_addr, load_size);
-}
-
-efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
-				      unsigned long *load_size,
-				      unsigned long max);
+efi_status_t efi_load_initrd(efi_loaded_image_t *image,
+			     unsigned long *load_addr,
+			     unsigned long *load_size,
+			     unsigned long soft_limit,
+			     unsigned long hard_limit);
 
 #endif
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index ee4241df4f29..1d3f94f1dafa 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -755,17 +755,8 @@ unsigned long efi_main(efi_handle_t handle,
 	if (!efi_noinitrd) {
 		unsigned long addr, size;
 
-		status = efi_load_initrd_dev_path(&addr, &size, ULONG_MAX);
-		if (status == EFI_NOT_FOUND) {
-			if (efi_is_native() && image != NULL) {
-				status = efi_load_initrd(image, &addr, &size,
-							 hdr->initrd_addr_max,
-							 ULONG_MAX);
-			} else {
-				addr = size = 0;
-				status = EFI_SUCCESS;
-			}
-		}
+		status = efi_load_initrd(image, &addr, &size,
+					 hdr->initrd_addr_max, ULONG_MAX);
 
 		if (status != EFI_SUCCESS) {
 			pr_efi_err("Failed to load initrd!\n");
-- 
2.26.2

[PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading

[Arvind Sankar]

Currently the EFI stub attempts to load initrd(s) specified on the
command line below hdr->initrd_addr_max (2G) and if that fails, falls
back to allocating at an unrestricted address.

The only case when loading at a low address helps is for the 32-bit
kernel, where the initrd must be copied by the kernel into lowmem if
it's not there already. The limit specified in hdr->initrd_addr_max is
insufficient to ensure this in any case, since lowmem by default will
extend to about 0.9G rather than 2G, and we don't attempt to load the
initrd in lowmem at all for the new device-path based initrd.

Simplify the code by dropping this optimization for the command line
initrd(s) as well.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 14 +++++---------
 drivers/firmware/efi/libstub/efi-stub.c        |  3 +--
 drivers/firmware/efi/libstub/efistub.h         |  8 +++-----
 drivers/firmware/efi/libstub/file.c            | 13 ++-----------
 drivers/firmware/efi/libstub/x86-stub.c        |  3 +--
 5 files changed, 12 insertions(+), 29 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 2c0c2c34b4cc..32768fa04b32 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -378,8 +378,7 @@ static
 efi_status_t efi_load_initrd_cmdline(efi_loaded_image_t *image,
 				     unsigned long *load_addr,
 				     unsigned long *load_size,
-				     unsigned long soft_limit,
-				     unsigned long hard_limit)
+				     unsigned long max)
 {
 	if (!IS_ENABLED(CONFIG_EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER) ||
 	    (IS_ENABLED(CONFIG_X86) && (!efi_is_native() || image == NULL))) {
@@ -388,27 +387,24 @@ efi_status_t efi_load_initrd_cmdline(efi_loaded_image_t *image,
 	}
 
 	return handle_cmdline_files(image, L"initrd=", sizeof(L"initrd=") - 2,
-				    soft_limit, hard_limit,
-				    load_addr, load_size);
+				    max, load_addr, load_size);
 }
 
 efi_status_t efi_load_initrd(efi_loaded_image_t *image,
 			     unsigned long *load_addr,
 			     unsigned long *load_size,
-			     unsigned long soft_limit,
-			     unsigned long hard_limit)
+			     unsigned long max)
 {
 	efi_status_t status;
 
 	if (!load_addr || !load_size)
 		return EFI_INVALID_PARAMETER;
 
-	status = efi_load_initrd_dev_path(load_addr, load_size, hard_limit);
+	status = efi_load_initrd_dev_path(load_addr, load_size, max);
 	if (status == EFI_SUCCESS) {
 		pr_efi("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
 	} else if (status == EFI_NOT_FOUND) {
-		status = efi_load_initrd_cmdline(image, load_addr, load_size,
-						 soft_limit, hard_limit);
+		status = efi_load_initrd_cmdline(image, load_addr, load_size, max);
 		if (status == EFI_SUCCESS && *load_size > 0)
 			pr_efi("Loaded initrd from command line option\n");
 	}
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index d8f24f5c91bd..930302d9415a 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -265,8 +265,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 
 	if (!efi_noinitrd) {
 		max_addr = efi_get_max_initrd_addr(dram_base, image_addr);
-		status = efi_load_initrd(image, &initrd_addr, &initrd_size,
-					 ULONG_MAX, max_addr);
+		status = efi_load_initrd(image, &initrd_addr, &initrd_size, max_addr);
 		if (status != EFI_SUCCESS)
 			pr_efi_err("Failed to load initrd!\n");
 	}
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index dfdd7954bf58..1ba0887818d9 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -663,8 +663,7 @@ efi_status_t efi_setup_gop(struct screen_info *si, efi_guid_t *proto,
 efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
 				  const efi_char16_t *optstr,
 				  int optstr_size,
-				  unsigned long soft_limit,
-				  unsigned long hard_limit,
+				  unsigned long max,
 				  unsigned long *load_addr,
 				  unsigned long *load_size);
 
@@ -674,13 +673,12 @@ static inline efi_status_t efi_load_dtb(efi_loaded_image_t *image,
 					unsigned long *load_size)
 {
 	return handle_cmdline_files(image, L"dtb=", sizeof(L"dtb=") - 2,
-				    ULONG_MAX, ULONG_MAX, load_addr, load_size);
+				    ULONG_MAX, load_addr, load_size);
 }
 
 efi_status_t efi_load_initrd(efi_loaded_image_t *image,
 			     unsigned long *load_addr,
 			     unsigned long *load_size,
-			     unsigned long soft_limit,
-			     unsigned long hard_limit);
+			     unsigned long max);
 
 #endif
diff --git a/drivers/firmware/efi/libstub/file.c b/drivers/firmware/efi/libstub/file.c
index 50aaf15f9ad5..7dee3c5d81fb 100644
--- a/drivers/firmware/efi/libstub/file.c
+++ b/drivers/firmware/efi/libstub/file.c
@@ -124,8 +124,7 @@ static int find_file_option(const efi_char16_t *cmdline, int cmdline_len,
 efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
 				  const efi_char16_t *optstr,
 				  int optstr_size,
-				  unsigned long soft_limit,
-				  unsigned long hard_limit,
+				  unsigned long max,
 				  unsigned long *load_addr,
 				  unsigned long *load_size)
 {
@@ -181,15 +180,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
 		    round_up(alloc_size, EFI_ALLOC_ALIGN)) {
 			unsigned long old_addr = alloc_addr;
 
-			status = EFI_OUT_OF_RESOURCES;
-			if (soft_limit < hard_limit)
-				status = efi_allocate_pages(alloc_size + size,
-							    &alloc_addr,
-							    soft_limit);
-			if (status == EFI_OUT_OF_RESOURCES)
-				status = efi_allocate_pages(alloc_size + size,
-							    &alloc_addr,
-							    hard_limit);
+			status = efi_allocate_pages(alloc_size + size, &alloc_addr, max);
 			if (status != EFI_SUCCESS) {
 				pr_efi_err("Failed to allocate memory for files\n");
 				goto err_close_file;
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 1d3f94f1dafa..85a924fecc87 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -755,8 +755,7 @@ unsigned long efi_main(efi_handle_t handle,
 	if (!efi_noinitrd) {
 		unsigned long addr, size;
 
-		status = efi_load_initrd(image, &addr, &size,
-					 hdr->initrd_addr_max, ULONG_MAX);
+		status = efi_load_initrd(image, &addr, &size, ULONG_MAX);
 
 		if (status != EFI_SUCCESS) {
 			pr_efi_err("Failed to load initrd!\n");
-- 
2.26.2

Re: [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading

[Ard Biesheuvel]

On Wed, 29 Apr 2020 at 19:41, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> Currently the EFI stub attempts to load initrd(s) specified on the
> command line below hdr->initrd_addr_max (2G) and if that fails, falls
> back to allocating at an unrestricted address.
>
> The only case when loading at a low address helps is for the 32-bit
> kernel, where the initrd must be copied by the kernel into lowmem if
> it's not there already. The limit specified in hdr->initrd_addr_max is
> insufficient to ensure this in any case, since lowmem by default will
> extend to about 0.9G rather than 2G, and we don't attempt to load the
> initrd in lowmem at all for the new device-path based initrd.
>
> Simplify the code by dropping this optimization for the command line
> initrd(s) as well.
>
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>

It is not really an optimization, unfortunately. Commit 47226ad4f4cfd
has the details, but in short, loading above 4 GB broke some
platforms, so below 4 GB had to remain the default.

This was 6 years ago, and so we might be able to revisit this, but
characterising it as a mere optimization is inaccurate.


> ---
>  drivers/firmware/efi/libstub/efi-stub-helper.c | 14 +++++---------
>  drivers/firmware/efi/libstub/efi-stub.c        |  3 +--
>  drivers/firmware/efi/libstub/efistub.h         |  8 +++-----
>  drivers/firmware/efi/libstub/file.c            | 13 ++-----------
>  drivers/firmware/efi/libstub/x86-stub.c        |  3 +--
>  5 files changed, 12 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index 2c0c2c34b4cc..32768fa04b32 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -378,8 +378,7 @@ static
>  efi_status_t efi_load_initrd_cmdline(efi_loaded_image_t *image,
>                                      unsigned long *load_addr,
>                                      unsigned long *load_size,
> -                                    unsigned long soft_limit,
> -                                    unsigned long hard_limit)
> +                                    unsigned long max)
>  {
>         if (!IS_ENABLED(CONFIG_EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER) ||
>             (IS_ENABLED(CONFIG_X86) && (!efi_is_native() || image == NULL))) {
> @@ -388,27 +387,24 @@ efi_status_t efi_load_initrd_cmdline(efi_loaded_image_t *image,
>         }
>
>         return handle_cmdline_files(image, L"initrd=", sizeof(L"initrd=") - 2,
> -                                   soft_limit, hard_limit,
> -                                   load_addr, load_size);
> +                                   max, load_addr, load_size);
>  }
>
>  efi_status_t efi_load_initrd(efi_loaded_image_t *image,
>                              unsigned long *load_addr,
>                              unsigned long *load_size,
> -                            unsigned long soft_limit,
> -                            unsigned long hard_limit)
> +                            unsigned long max)
>  {
>         efi_status_t status;
>
>         if (!load_addr || !load_size)
>                 return EFI_INVALID_PARAMETER;
>
> -       status = efi_load_initrd_dev_path(load_addr, load_size, hard_limit);
> +       status = efi_load_initrd_dev_path(load_addr, load_size, max);
>         if (status == EFI_SUCCESS) {
>                 pr_efi("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
>         } else if (status == EFI_NOT_FOUND) {
> -               status = efi_load_initrd_cmdline(image, load_addr, load_size,
> -                                                soft_limit, hard_limit);
> +               status = efi_load_initrd_cmdline(image, load_addr, load_size, max);
>                 if (status == EFI_SUCCESS && *load_size > 0)
>                         pr_efi("Loaded initrd from command line option\n");
>         }
> diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
> index d8f24f5c91bd..930302d9415a 100644
> --- a/drivers/firmware/efi/libstub/efi-stub.c
> +++ b/drivers/firmware/efi/libstub/efi-stub.c
> @@ -265,8 +265,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
>
>         if (!efi_noinitrd) {
>                 max_addr = efi_get_max_initrd_addr(dram_base, image_addr);
> -               status = efi_load_initrd(image, &initrd_addr, &initrd_size,
> -                                        ULONG_MAX, max_addr);
> +               status = efi_load_initrd(image, &initrd_addr, &initrd_size, max_addr);
>                 if (status != EFI_SUCCESS)
>                         pr_efi_err("Failed to load initrd!\n");
>         }
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index dfdd7954bf58..1ba0887818d9 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -663,8 +663,7 @@ efi_status_t efi_setup_gop(struct screen_info *si, efi_guid_t *proto,
>  efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
>                                   const efi_char16_t *optstr,
>                                   int optstr_size,
> -                                 unsigned long soft_limit,
> -                                 unsigned long hard_limit,
> +                                 unsigned long max,
>                                   unsigned long *load_addr,
>                                   unsigned long *load_size);
>
> @@ -674,13 +673,12 @@ static inline efi_status_t efi_load_dtb(efi_loaded_image_t *image,
>                                         unsigned long *load_size)
>  {
>         return handle_cmdline_files(image, L"dtb=", sizeof(L"dtb=") - 2,
> -                                   ULONG_MAX, ULONG_MAX, load_addr, load_size);
> +                                   ULONG_MAX, load_addr, load_size);
>  }
>
>  efi_status_t efi_load_initrd(efi_loaded_image_t *image,
>                              unsigned long *load_addr,
>                              unsigned long *load_size,
> -                            unsigned long soft_limit,
> -                            unsigned long hard_limit);
> +                            unsigned long max);
>
>  #endif
> diff --git a/drivers/firmware/efi/libstub/file.c b/drivers/firmware/efi/libstub/file.c
> index 50aaf15f9ad5..7dee3c5d81fb 100644
> --- a/drivers/firmware/efi/libstub/file.c
> +++ b/drivers/firmware/efi/libstub/file.c
> @@ -124,8 +124,7 @@ static int find_file_option(const efi_char16_t *cmdline, int cmdline_len,
>  efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
>                                   const efi_char16_t *optstr,
>                                   int optstr_size,
> -                                 unsigned long soft_limit,
> -                                 unsigned long hard_limit,
> +                                 unsigned long max,
>                                   unsigned long *load_addr,
>                                   unsigned long *load_size)
>  {
> @@ -181,15 +180,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
>                     round_up(alloc_size, EFI_ALLOC_ALIGN)) {
>                         unsigned long old_addr = alloc_addr;
>
> -                       status = EFI_OUT_OF_RESOURCES;
> -                       if (soft_limit < hard_limit)
> -                               status = efi_allocate_pages(alloc_size + size,
> -                                                           &alloc_addr,
> -                                                           soft_limit);
> -                       if (status == EFI_OUT_OF_RESOURCES)
> -                               status = efi_allocate_pages(alloc_size + size,
> -                                                           &alloc_addr,
> -                                                           hard_limit);
> +                       status = efi_allocate_pages(alloc_size + size, &alloc_addr, max);
>                         if (status != EFI_SUCCESS) {
>                                 pr_efi_err("Failed to allocate memory for files\n");
>                                 goto err_close_file;
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index 1d3f94f1dafa..85a924fecc87 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -755,8 +755,7 @@ unsigned long efi_main(efi_handle_t handle,
>         if (!efi_noinitrd) {
>                 unsigned long addr, size;
>
> -               status = efi_load_initrd(image, &addr, &size,
> -                                        hdr->initrd_addr_max, ULONG_MAX);
> +               status = efi_load_initrd(image, &addr, &size, ULONG_MAX);
>
>                 if (status != EFI_SUCCESS) {
>                         pr_efi_err("Failed to load initrd!\n");
> --
> 2.26.2
>

Re: [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading

[Arvind Sankar]

On Wed, Apr 29, 2020 at 09:05:51PM +0200, Ard Biesheuvel wrote:
> On Wed, 29 Apr 2020 at 19:41, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > Currently the EFI stub attempts to load initrd(s) specified on the
> > command line below hdr->initrd_addr_max (2G) and if that fails, falls
> > back to allocating at an unrestricted address.
> >
> > The only case when loading at a low address helps is for the 32-bit
> > kernel, where the initrd must be copied by the kernel into lowmem if
> > it's not there already. The limit specified in hdr->initrd_addr_max is
> > insufficient to ensure this in any case, since lowmem by default will
> > extend to about 0.9G rather than 2G, and we don't attempt to load the
> > initrd in lowmem at all for the new device-path based initrd.
> >
> > Simplify the code by dropping this optimization for the command line
> > initrd(s) as well.
> >
> > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> 
> It is not really an optimization, unfortunately. Commit 47226ad4f4cfd
> has the details, but in short, loading above 4 GB broke some
> platforms, so below 4 GB had to remain the default.
> 
> This was 6 years ago, and so we might be able to revisit this, but
> characterising it as a mere optimization is inaccurate.
> 

Drat. Ok so I guess this patch has to be dropped then.

[PATCH 09/10] efi/x86: Support builtin command line

[Arvind Sankar]

Add support for the x86 CMDLINE_BOOL and CMDLINE_OVERRIDE configuration
options.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/x86-stub.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 85a924fecc87..0faba30d6406 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -680,7 +680,6 @@ unsigned long efi_main(efi_handle_t handle,
 	unsigned long buffer_start, buffer_end;
 	struct setup_header *hdr = &boot_params->hdr;
 	efi_status_t status;
-	unsigned long cmdline_paddr;
 
 	efi_system_table = sys_table_arg;
 
@@ -739,9 +738,14 @@ unsigned long efi_main(efi_handle_t handle,
 		image_offset = 0;
 	}
 
-	cmdline_paddr = ((u64)hdr->cmd_line_ptr |
-			 ((u64)boot_params->ext_cmd_line_ptr << 32));
-	efi_parse_options((char *)cmdline_paddr);
+#ifdef CONFIG_CMDLINE_BOOL
+	efi_parse_options(CONFIG_CMDLINE);
+#endif
+	if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
+		unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
+					       ((u64)boot_params->ext_cmd_line_ptr << 32));
+		efi_parse_options((char *)cmdline_paddr);
+	}
 
 	/*
 	 * At this point, an initrd may already have been loaded by the
-- 
2.26.2

Re: [PATCH 09/10] efi/x86: Support builtin command line

[Ard Biesheuvel]

On Wed, 29 Apr 2020 at 19:41, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> Add support for the x86 CMDLINE_BOOL and CMDLINE_OVERRIDE configuration
> options.
>
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
>  drivers/firmware/efi/libstub/x86-stub.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index 85a924fecc87..0faba30d6406 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -680,7 +680,6 @@ unsigned long efi_main(efi_handle_t handle,
>         unsigned long buffer_start, buffer_end;
>         struct setup_header *hdr = &boot_params->hdr;
>         efi_status_t status;
> -       unsigned long cmdline_paddr;
>
>         efi_system_table = sys_table_arg;
>
> @@ -739,9 +738,14 @@ unsigned long efi_main(efi_handle_t handle,
>                 image_offset = 0;
>         }
>
> -       cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> -                        ((u64)boot_params->ext_cmd_line_ptr << 32));
> -       efi_parse_options((char *)cmdline_paddr);
> +#ifdef CONFIG_CMDLINE_BOOL
> +       efi_parse_options(CONFIG_CMDLINE);
> +#endif

Can we use IS_ENABLED() here as well?

> +       if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
> +               unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> +                                              ((u64)boot_params->ext_cmd_line_ptr << 32));
> +               efi_parse_options((char *)cmdline_paddr);
> +       }
>
>         /*
>          * At this point, an initrd may already have been loaded by the
> --
> 2.26.2
>

Re: [PATCH 09/10] efi/x86: Support builtin command line

[Arvind Sankar]

On Wed, Apr 29, 2020 at 09:07:32PM +0200, Ard Biesheuvel wrote:
> On Wed, 29 Apr 2020 at 19:41, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > Add support for the x86 CMDLINE_BOOL and CMDLINE_OVERRIDE configuration
> > options.
> >
> > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > ---
> >  drivers/firmware/efi/libstub/x86-stub.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> > index 85a924fecc87..0faba30d6406 100644
> > --- a/drivers/firmware/efi/libstub/x86-stub.c
> > +++ b/drivers/firmware/efi/libstub/x86-stub.c
> > @@ -680,7 +680,6 @@ unsigned long efi_main(efi_handle_t handle,
> >         unsigned long buffer_start, buffer_end;
> >         struct setup_header *hdr = &boot_params->hdr;
> >         efi_status_t status;
> > -       unsigned long cmdline_paddr;
> >
> >         efi_system_table = sys_table_arg;
> >
> > @@ -739,9 +738,14 @@ unsigned long efi_main(efi_handle_t handle,
> >                 image_offset = 0;
> >         }
> >
> > -       cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> > -                        ((u64)boot_params->ext_cmd_line_ptr << 32));
> > -       efi_parse_options((char *)cmdline_paddr);
> > +#ifdef CONFIG_CMDLINE_BOOL
> > +       efi_parse_options(CONFIG_CMDLINE);
> > +#endif
> 
> Can we use IS_ENABLED() here as well?

Unfortunately on x86, CONFIG_CMDLINE is not defined if
CONFIG_CMDLINE_BOOL isn't enabled. So turning this into an
IS_ENABLED(CONFIG_CMDLINE_BOOL) causes a compile error when it's
disabled due to CONFIG_CMDLINE being an undeclared symbol.

> 
> > +       if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
> > +               unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> > +                                              ((u64)boot_params->ext_cmd_line_ptr << 32));
> > +               efi_parse_options((char *)cmdline_paddr);
> > +       }
> >
> >         /*
> >          * At this point, an initrd may already have been loaded by the
> > --
> > 2.26.2
> >

Re: [PATCH 09/10] efi/x86: Support builtin command line

[Ard Biesheuvel]

On Wed, 29 Apr 2020 at 23:39, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Wed, Apr 29, 2020 at 09:07:32PM +0200, Ard Biesheuvel wrote:
> > On Wed, 29 Apr 2020 at 19:41, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > >
> > > Add support for the x86 CMDLINE_BOOL and CMDLINE_OVERRIDE configuration
> > > options.
> > >
> > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > ---
> > >  drivers/firmware/efi/libstub/x86-stub.c | 12 ++++++++----
> > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> > > index 85a924fecc87..0faba30d6406 100644
> > > --- a/drivers/firmware/efi/libstub/x86-stub.c
> > > +++ b/drivers/firmware/efi/libstub/x86-stub.c
> > > @@ -680,7 +680,6 @@ unsigned long efi_main(efi_handle_t handle,
> > >         unsigned long buffer_start, buffer_end;
> > >         struct setup_header *hdr = &boot_params->hdr;
> > >         efi_status_t status;
> > > -       unsigned long cmdline_paddr;
> > >
> > >         efi_system_table = sys_table_arg;
> > >
> > > @@ -739,9 +738,14 @@ unsigned long efi_main(efi_handle_t handle,
> > >                 image_offset = 0;
> > >         }
> > >
> > > -       cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> > > -                        ((u64)boot_params->ext_cmd_line_ptr << 32));
> > > -       efi_parse_options((char *)cmdline_paddr);
> > > +#ifdef CONFIG_CMDLINE_BOOL
> > > +       efi_parse_options(CONFIG_CMDLINE);
> > > +#endif
> >
> > Can we use IS_ENABLED() here as well?
>
> Unfortunately on x86, CONFIG_CMDLINE is not defined if
> CONFIG_CMDLINE_BOOL isn't enabled. So turning this into an
> IS_ENABLED(CONFIG_CMDLINE_BOOL) causes a compile error when it's
> disabled due to CONFIG_CMDLINE being an undeclared symbol.
>

What about

efi_parse_options(CONFIG_CMDLINE "");

?

Re: [PATCH 09/10] efi/x86: Support builtin command line

[Arvind Sankar]

On Wed, Apr 29, 2020 at 11:40:51PM +0200, Ard Biesheuvel wrote:
> On Wed, 29 Apr 2020 at 23:39, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > On Wed, Apr 29, 2020 at 09:07:32PM +0200, Ard Biesheuvel wrote:
> > > On Wed, 29 Apr 2020 at 19:41, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > > >
> > > > Add support for the x86 CMDLINE_BOOL and CMDLINE_OVERRIDE configuration
> > > > options.
> > > >
> > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > ---
> > > >  drivers/firmware/efi/libstub/x86-stub.c | 12 ++++++++----
> > > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> > > > index 85a924fecc87..0faba30d6406 100644
> > > > --- a/drivers/firmware/efi/libstub/x86-stub.c
> > > > +++ b/drivers/firmware/efi/libstub/x86-stub.c
> > > > @@ -680,7 +680,6 @@ unsigned long efi_main(efi_handle_t handle,
> > > >         unsigned long buffer_start, buffer_end;
> > > >         struct setup_header *hdr = &boot_params->hdr;
> > > >         efi_status_t status;
> > > > -       unsigned long cmdline_paddr;
> > > >
> > > >         efi_system_table = sys_table_arg;
> > > >
> > > > @@ -739,9 +738,14 @@ unsigned long efi_main(efi_handle_t handle,
> > > >                 image_offset = 0;
> > > >         }
> > > >
> > > > -       cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> > > > -                        ((u64)boot_params->ext_cmd_line_ptr << 32));
> > > > -       efi_parse_options((char *)cmdline_paddr);
> > > > +#ifdef CONFIG_CMDLINE_BOOL
> > > > +       efi_parse_options(CONFIG_CMDLINE);
> > > > +#endif
> > >
> > > Can we use IS_ENABLED() here as well?
> >
> > Unfortunately on x86, CONFIG_CMDLINE is not defined if
> > CONFIG_CMDLINE_BOOL isn't enabled. So turning this into an
> > IS_ENABLED(CONFIG_CMDLINE_BOOL) causes a compile error when it's
> > disabled due to CONFIG_CMDLINE being an undeclared symbol.
> >
> 
> What about
> 
> efi_parse_options(CONFIG_CMDLINE "");
> 
> ?

That's still a syntax error if CONFIG_CMDLINE is undefined, no? It's not
defined to be empty -- it's undefined. IS_ENABLED doesn't work on
string-valued options so I can't use IS_ENABLED(CONFIG_CMDLINE) either.

Re: [PATCH 09/10] efi/x86: Support builtin command line

[Ard Biesheuvel]

On Wed, 29 Apr 2020 at 23:48, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Wed, Apr 29, 2020 at 11:40:51PM +0200, Ard Biesheuvel wrote:
> > On Wed, 29 Apr 2020 at 23:39, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > >
> > > On Wed, Apr 29, 2020 at 09:07:32PM +0200, Ard Biesheuvel wrote:
> > > > On Wed, 29 Apr 2020 at 19:41, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > > > >
> > > > > Add support for the x86 CMDLINE_BOOL and CMDLINE_OVERRIDE configuration
> > > > > options.
> > > > >
> > > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > > > ---
> > > > >  drivers/firmware/efi/libstub/x86-stub.c | 12 ++++++++----
> > > > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> > > > > index 85a924fecc87..0faba30d6406 100644
> > > > > --- a/drivers/firmware/efi/libstub/x86-stub.c
> > > > > +++ b/drivers/firmware/efi/libstub/x86-stub.c
> > > > > @@ -680,7 +680,6 @@ unsigned long efi_main(efi_handle_t handle,
> > > > >         unsigned long buffer_start, buffer_end;
> > > > >         struct setup_header *hdr = &boot_params->hdr;
> > > > >         efi_status_t status;
> > > > > -       unsigned long cmdline_paddr;
> > > > >
> > > > >         efi_system_table = sys_table_arg;
> > > > >
> > > > > @@ -739,9 +738,14 @@ unsigned long efi_main(efi_handle_t handle,
> > > > >                 image_offset = 0;
> > > > >         }
> > > > >
> > > > > -       cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> > > > > -                        ((u64)boot_params->ext_cmd_line_ptr << 32));
> > > > > -       efi_parse_options((char *)cmdline_paddr);
> > > > > +#ifdef CONFIG_CMDLINE_BOOL
> > > > > +       efi_parse_options(CONFIG_CMDLINE);
> > > > > +#endif
> > > >
> > > > Can we use IS_ENABLED() here as well?
> > >
> > > Unfortunately on x86, CONFIG_CMDLINE is not defined if
> > > CONFIG_CMDLINE_BOOL isn't enabled. So turning this into an
> > > IS_ENABLED(CONFIG_CMDLINE_BOOL) causes a compile error when it's
> > > disabled due to CONFIG_CMDLINE being an undeclared symbol.
> > >
> >
> > What about
> >
> > efi_parse_options(CONFIG_CMDLINE "");
> >
> > ?
>
> That's still a syntax error if CONFIG_CMDLINE is undefined, no? It's not
> defined to be empty -- it's undefined. IS_ENABLED doesn't work on
> string-valued options so I can't use IS_ENABLED(CONFIG_CMDLINE) either.

I see. Not the end of the world to have to keep it as is, I was just curious.

[PATCH 10/10] efi/libstub: Check return value of efi_parse_options

[Arvind Sankar]

efi_parse_options can fail if it is unable to allocate space for a copy
of the command line. Check the return value to make sure it succeeded.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/efi-stub.c | 18 ++++++++++++++----
 drivers/firmware/efi/libstub/x86-stub.c | 12 ++++++++++--
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index 930302d9415a..a4399537b4e6 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -207,11 +207,21 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 
 	if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
 	    IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
-	    cmdline_size == 0)
-		efi_parse_options(CONFIG_CMDLINE);
+	    cmdline_size == 0) {
+		status = efi_parse_options(CONFIG_CMDLINE);
+		if (status != EFI_SUCCESS) {
+			pr_efi_err("Failed to parse options\n");
+			goto fail_free_cmdline;
+		}
+	}
 
-	if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && cmdline_size > 0)
-		efi_parse_options(cmdline_ptr);
+	if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && cmdline_size > 0) {
+		status = efi_parse_options(cmdline_ptr);
+		if (status != EFI_SUCCESS) {
+			pr_efi_err("Failed to parse options\n");
+			goto fail_free_cmdline;
+		}
+	}
 
 	pr_efi("Booting Linux Kernel...\n");
 
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 0faba30d6406..ca549f26f45d 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -739,12 +739,20 @@ unsigned long efi_main(efi_handle_t handle,
 	}
 
 #ifdef CONFIG_CMDLINE_BOOL
-	efi_parse_options(CONFIG_CMDLINE);
+	status = efi_parse_options(CONFIG_CMDLINE);
+	if (status != EFI_SUCCESS) {
+		pr_efi_err("Failed to parse options\n");
+		goto fail;
+	}
 #endif
 	if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
 		unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
 					       ((u64)boot_params->ext_cmd_line_ptr << 32));
-		efi_parse_options((char *)cmdline_paddr);
+		status = efi_parse_options((char *)cmdline_paddr);
+		if (status != EFI_SUCCESS) {
+			pr_efi_err("Failed to parse options\n");
+			goto fail;
+		}
 	}
 
 	/*
-- 
2.26.2

Re: [PATCH 10/10] efi/libstub: Check return value of efi_parse_options

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 11330 bytes --]

Hi Arvind,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on efi/next]
[also build test WARNING on next-20200429]
[cannot apply to v5.7-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Arvind-Sankar/efi-some-cleanups-refactoring-for-efi-next/20200430-051025
base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
config: arm-defconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 1ccde533425a4ba9d379510206ad680ff9702129)
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/firmware/efi/libstub/efi-stub.c:220:7: warning: variable 'si' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
                   if (status != EFI_SUCCESS) {
                       ^~~~~~~~~~~~~~~~~~~~~
   drivers/firmware/efi/libstub/efi-stub.c:339:19: note: uninitialized use occurs here
           free_screen_info(si);
                            ^~
   drivers/firmware/efi/libstub/efi-stub.c:220:3: note: remove the 'if' if its condition is always false
                   if (status != EFI_SUCCESS) {
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/firmware/efi/libstub/efi-stub.c:212:7: warning: variable 'si' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
                   if (status != EFI_SUCCESS) {
                       ^~~~~~~~~~~~~~~~~~~~~
   drivers/firmware/efi/libstub/efi-stub.c:339:19: note: uninitialized use occurs here
           free_screen_info(si);
                            ^~
   drivers/firmware/efi/libstub/efi-stub.c:212:3: note: remove the 'if' if its condition is always false
                   if (status != EFI_SUCCESS) {
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/firmware/efi/libstub/efi-stub.c:161:24: note: initialize the variable 'si' to silence this warning
           struct screen_info *si;
                                 ^
                                  = NULL
   2 warnings generated.

vim +220 drivers/firmware/efi/libstub/efi-stub.c

   119	
   120	/*
   121	 * This function handles the architcture specific differences between arm and
   122	 * arm64 regarding where the kernel image must be loaded and any memory that
   123	 * must be reserved. On failure it is required to free all
   124	 * all allocations it has made.
   125	 */
   126	efi_status_t handle_kernel_image(unsigned long *image_addr,
   127					 unsigned long *image_size,
   128					 unsigned long *reserve_addr,
   129					 unsigned long *reserve_size,
   130					 unsigned long dram_base,
   131					 efi_loaded_image_t *image);
   132	
   133	asmlinkage void __noreturn efi_enter_kernel(unsigned long entrypoint,
   134						    unsigned long fdt_addr,
   135						    unsigned long fdt_size);
   136	
   137	/*
   138	 * EFI entry point for the arm/arm64 EFI stubs.  This is the entrypoint
   139	 * that is described in the PE/COFF header.  Most of the code is the same
   140	 * for both archictectures, with the arch-specific code provided in the
   141	 * handle_kernel_image() function.
   142	 */
   143	efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
   144	{
   145		efi_loaded_image_t *image;
   146		efi_status_t status;
   147		unsigned long image_addr;
   148		unsigned long image_size = 0;
   149		unsigned long dram_base;
   150		/* addr/point and size pairs for memory management*/
   151		unsigned long initrd_addr = 0;
   152		unsigned long initrd_size = 0;
   153		unsigned long fdt_addr = 0;  /* Original DTB */
   154		unsigned long fdt_size = 0;
   155		char *cmdline_ptr = NULL;
   156		int cmdline_size = 0;
   157		efi_guid_t loaded_image_proto = LOADED_IMAGE_PROTOCOL_GUID;
   158		unsigned long reserve_addr = 0;
   159		unsigned long reserve_size = 0;
   160		enum efi_secureboot_mode secure_boot;
   161		struct screen_info *si;
   162		efi_properties_table_t *prop_tbl;
   163		unsigned long max_addr;
   164	
   165		efi_system_table = sys_table_arg;
   166	
   167		/* Check if we were booted by the EFI firmware */
   168		if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) {
   169			status = EFI_INVALID_PARAMETER;
   170			goto fail;
   171		}
   172	
   173		status = check_platform_features();
   174		if (status != EFI_SUCCESS)
   175			goto fail;
   176	
   177		/*
   178		 * Get a handle to the loaded image protocol.  This is used to get
   179		 * information about the running image, such as size and the command
   180		 * line.
   181		 */
   182		status = efi_system_table->boottime->handle_protocol(handle,
   183						&loaded_image_proto, (void *)&image);
   184		if (status != EFI_SUCCESS) {
   185			pr_efi_err("Failed to get loaded image protocol\n");
   186			goto fail;
   187		}
   188	
   189		dram_base = get_dram_base();
   190		if (dram_base == EFI_ERROR) {
   191			pr_efi_err("Failed to find DRAM base\n");
   192			status = EFI_LOAD_ERROR;
   193			goto fail;
   194		}
   195	
   196		/*
   197		 * Get the command line from EFI, using the LOADED_IMAGE
   198		 * protocol. We are going to copy the command line into the
   199		 * device tree, so this can be allocated anywhere.
   200		 */
   201		cmdline_ptr = efi_convert_cmdline(image, &cmdline_size, ULONG_MAX);
   202		if (!cmdline_ptr) {
   203			pr_efi_err("getting command line via LOADED_IMAGE_PROTOCOL\n");
   204			status = EFI_OUT_OF_RESOURCES;
   205			goto fail;
   206		}
   207	
   208		if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
   209		    IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
   210		    cmdline_size == 0) {
   211			status = efi_parse_options(CONFIG_CMDLINE);
   212			if (status != EFI_SUCCESS) {
   213				pr_efi_err("Failed to parse options\n");
   214				goto fail_free_cmdline;
   215			}
   216		}
   217	
   218		if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && cmdline_size > 0) {
   219			status = efi_parse_options(cmdline_ptr);
 > 220			if (status != EFI_SUCCESS) {
   221				pr_efi_err("Failed to parse options\n");
   222				goto fail_free_cmdline;
   223			}
   224		}
   225	
   226		pr_efi("Booting Linux Kernel...\n");
   227	
   228		si = setup_graphics();
   229	
   230		status = handle_kernel_image(&image_addr, &image_size,
   231					     &reserve_addr,
   232					     &reserve_size,
   233					     dram_base, image);
   234		if (status != EFI_SUCCESS) {
   235			pr_efi_err("Failed to relocate kernel\n");
   236			goto fail_free_cmdline;
   237		}
   238	
   239		efi_retrieve_tpm2_eventlog();
   240	
   241		/* Ask the firmware to clear memory on unclean shutdown */
   242		efi_enable_reset_attack_mitigation();
   243	
   244		secure_boot = efi_get_secureboot();
   245	
   246		/*
   247		 * Unauthenticated device tree data is a security hazard, so ignore
   248		 * 'dtb=' unless UEFI Secure Boot is disabled.  We assume that secure
   249		 * boot is enabled if we can't determine its state.
   250		 */
   251		if (!IS_ENABLED(CONFIG_EFI_ARMSTUB_DTB_LOADER) ||
   252		     secure_boot != efi_secureboot_mode_disabled) {
   253			if (strstr(cmdline_ptr, "dtb="))
   254				pr_efi("Ignoring DTB from command line.\n");
   255		} else {
   256			status = efi_load_dtb(image, &fdt_addr, &fdt_size);
   257	
   258			if (status != EFI_SUCCESS) {
   259				pr_efi_err("Failed to load device tree!\n");
   260				goto fail_free_image;
   261			}
   262		}
   263	
   264		if (fdt_addr) {
   265			pr_efi("Using DTB from command line\n");
   266		} else {
   267			/* Look for a device tree configuration table entry. */
   268			fdt_addr = (uintptr_t)get_fdt(&fdt_size);
   269			if (fdt_addr)
   270				pr_efi("Using DTB from configuration table\n");
   271		}
   272	
   273		if (!fdt_addr)
   274			pr_efi("Generating empty DTB\n");
   275	
   276		if (!efi_noinitrd) {
   277			max_addr = efi_get_max_initrd_addr(dram_base, image_addr);
   278			status = efi_load_initrd(image, &initrd_addr, &initrd_size, max_addr);
   279			if (status != EFI_SUCCESS)
   280				pr_efi_err("Failed to load initrd!\n");
   281		}
   282	
   283		efi_random_get_seed();
   284	
   285		/*
   286		 * If the NX PE data feature is enabled in the properties table, we
   287		 * should take care not to create a virtual mapping that changes the
   288		 * relative placement of runtime services code and data regions, as
   289		 * they may belong to the same PE/COFF executable image in memory.
   290		 * The easiest way to achieve that is to simply use a 1:1 mapping.
   291		 */
   292		prop_tbl = get_efi_config_table(EFI_PROPERTIES_TABLE_GUID);
   293		flat_va_mapping = prop_tbl &&
   294				  (prop_tbl->memory_protection_attribute &
   295				   EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA);
   296	
   297		/* hibernation expects the runtime regions to stay in the same place */
   298		if (!IS_ENABLED(CONFIG_HIBERNATION) && !efi_nokaslr && !flat_va_mapping) {
   299			/*
   300			 * Randomize the base of the UEFI runtime services region.
   301			 * Preserve the 2 MB alignment of the region by taking a
   302			 * shift of 21 bit positions into account when scaling
   303			 * the headroom value using a 32-bit random value.
   304			 */
   305			static const u64 headroom = EFI_RT_VIRTUAL_LIMIT -
   306						    EFI_RT_VIRTUAL_BASE -
   307						    EFI_RT_VIRTUAL_SIZE;
   308			u32 rnd;
   309	
   310			status = efi_get_random_bytes(sizeof(rnd), (u8 *)&rnd);
   311			if (status == EFI_SUCCESS) {
   312				virtmap_base = EFI_RT_VIRTUAL_BASE +
   313					       (((headroom >> 21) * rnd) >> (32 - 21));
   314			}
   315		}
   316	
   317		install_memreserve_table();
   318	
   319		status = allocate_new_fdt_and_exit_boot(handle, &fdt_addr,
   320							efi_get_max_fdt_addr(dram_base),
   321							initrd_addr, initrd_size,
   322							cmdline_ptr, fdt_addr, fdt_size);
   323		if (status != EFI_SUCCESS)
   324			goto fail_free_initrd;
   325	
   326		efi_enter_kernel(image_addr, fdt_addr, fdt_totalsize((void *)fdt_addr));
   327		/* not reached */
   328	
   329	fail_free_initrd:
   330		pr_efi_err("Failed to update FDT and exit boot services\n");
   331	
   332		efi_free(initrd_size, initrd_addr);
   333		efi_free(fdt_size, fdt_addr);
   334	
   335	fail_free_image:
   336		efi_free(image_size, image_addr);
   337		efi_free(reserve_size, reserve_addr);
   338	fail_free_cmdline:
   339		free_screen_info(si);
   340		efi_free(cmdline_size, (unsigned long)cmdline_ptr);
   341	fail:
   342		return status;
   343	}
   344	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51805 bytes --]

[PATCH v2 00/11] efi: some cleanups/refactoring for efi/next

[Arvind Sankar]

This series is on top of efi/next.

Patch 1 fixes the size allocated for x86 boot_params.
Patch 2 refactors the setting of various hi/lo 32-bit fields, mainly on x86.
Patch 3 renames pr_efi/pr_efi_err
Patches 4-6 convert the remaining uses of efi_printk to print error
messages to use efi_err instead.
Patch 7 updates dtb= ignored message to efi_err.
Patches 8-9 refactor initrd loading, moving it into efi-stub-helper.
Patch 10 adds support for x86 builtin command line.
Patch 11 adds error checking for efi_parse_options.

Changes from v1:
- Rename pr_efi/pr_efi_err
- Drop the soft_limit-removing patch
- Fix a couple of compile warnings

Arvind Sankar (11):
  efi/x86: Use correct size for boot_params
  efi/libstub: Add a helper function to split 64-bit values
  efi/libstub: Move pr_efi/pr_efi_err into efi namespace
  efi/x86: Use efi_err for error messages
  efi/gop: Use efi_err for error messages
  efi/tpm: Use efi_err for error messages
  efi/libstub: Upgrade ignored dtb= argument message to error
  efi/x86: Move command-line initrd loading to efi_main
  efi/libstub: Unify initrd loading across architectures
  efi/x86: Support builtin command line
  efi/libstub: Check return value of efi_parse_options

 drivers/firmware/efi/libstub/arm32-stub.c     |  12 +-
 drivers/firmware/efi/libstub/arm64-stub.c     |  14 +-
 .../firmware/efi/libstub/efi-stub-helper.c    |  46 ++++++-
 drivers/firmware/efi/libstub/efi-stub.c       |  63 ++++-----
 drivers/firmware/efi/libstub/efistub.h        |  32 ++---
 drivers/firmware/efi/libstub/fdt.c            |  16 +--
 drivers/firmware/efi/libstub/file.c           |  12 +-
 drivers/firmware/efi/libstub/gop.c            |  16 +--
 drivers/firmware/efi/libstub/pci.c            |   8 +-
 drivers/firmware/efi/libstub/relocate.c       |   2 +-
 drivers/firmware/efi/libstub/secureboot.c     |   4 +-
 drivers/firmware/efi/libstub/tpm.c            |   2 +-
 drivers/firmware/efi/libstub/x86-stub.c       | 122 ++++++++----------
 13 files changed, 186 insertions(+), 163 deletions(-)

-- 
2.26.2

[PATCH v2 01/11] efi/x86: Use correct size for boot_params

[Arvind Sankar]

struct boot_params is only 4096 bytes, not 16384. Fix this by using
sizeof(struct boot_params) instead of hardcoding the incorrect value.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/x86-stub.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 597793fe8d22..d4bafd7f6f9f 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -379,13 +379,14 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	image_base = efi_table_attr(image, image_base);
 	image_offset = (void *)startup_32 - image_base;
 
-	status = efi_allocate_pages(0x4000, (unsigned long *)&boot_params, ULONG_MAX);
+	status = efi_allocate_pages(sizeof(struct boot_params),
+				    (unsigned long *)&boot_params, ULONG_MAX);
 	if (status != EFI_SUCCESS) {
 		efi_printk("Failed to allocate lowmem for boot params\n");
 		efi_exit(handle, status);
 	}
 
-	memset(boot_params, 0x0, 0x4000);
+	memset(boot_params, 0x0, sizeof(struct boot_params));
 
 	hdr = &boot_params->hdr;
 
@@ -439,7 +440,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 fail2:
 	efi_free(options_size, (unsigned long)cmdline_ptr);
 fail:
-	efi_free(0x4000, (unsigned long)boot_params);
+	efi_free(sizeof(struct boot_params), (unsigned long)boot_params);
 
 	efi_exit(handle, status);
 }
-- 
2.26.2

[PATCH v2 02/11] efi/libstub: Add a helper function to split 64-bit values

[Arvind Sankar]

In several places 64-bit values need to be split up into two 32-bit
fields, in order to be backward-compatible with the old 32-bit ABIs.

Instead of open-coding this, add a helper function to set a 64-bit value
as two 32-bit fields.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/efistub.h  |  7 ++++++
 drivers/firmware/efi/libstub/gop.c      |  6 ++---
 drivers/firmware/efi/libstub/x86-stub.c | 32 +++++++++++--------------
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 5ff63230a1f1..e8aa70ba08d5 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -87,6 +87,13 @@ extern const efi_system_table_t *efi_system_table;
 		((handle = efi_get_handle_at((array), i)) || true);	\
 	     i++)
 
+static inline
+void efi_set_u64_split(u64 data, u32 *lo, u32 *hi)
+{
+	*lo = lower_32_bits(data);
+	*hi = upper_32_bits(data);
+}
+
 /*
  * Allocation types for calls to boottime->allocate_pages.
  */
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 216327d0b034..64cee0febae0 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -422,7 +422,6 @@ static efi_status_t setup_gop(struct screen_info *si, efi_guid_t *proto,
 	efi_graphics_output_protocol_t *gop;
 	efi_graphics_output_protocol_mode_t *mode;
 	efi_graphics_output_mode_info_t *info;
-	efi_physical_addr_t fb_base;
 
 	gop = find_gop(proto, size, handles);
 
@@ -442,9 +441,8 @@ static efi_status_t setup_gop(struct screen_info *si, efi_guid_t *proto,
 	si->lfb_width  = info->horizontal_resolution;
 	si->lfb_height = info->vertical_resolution;
 
-	fb_base		 = efi_table_attr(mode, frame_buffer_base);
-	si->lfb_base	 = lower_32_bits(fb_base);
-	si->ext_lfb_base = upper_32_bits(fb_base);
+	efi_set_u64_split(efi_table_attr(mode, frame_buffer_base),
+			  &si->lfb_base, &si->ext_lfb_base);
 	if (si->ext_lfb_base)
 		si->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
 
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index d4bafd7f6f9f..f91d4aab0156 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -408,9 +408,8 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	if (!cmdline_ptr)
 		goto fail;
 
-	hdr->cmd_line_ptr = (unsigned long)cmdline_ptr;
-	/* Fill in upper bits of command line address, NOP on 32 bit  */
-	boot_params->ext_cmd_line_ptr = (u64)(unsigned long)cmdline_ptr >> 32;
+	efi_set_u64_split((unsigned long)cmdline_ptr,
+			  &hdr->cmd_line_ptr, &boot_params->ext_cmd_line_ptr);
 
 	hdr->ramdisk_image = 0;
 	hdr->ramdisk_size = 0;
@@ -427,10 +426,10 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 						 ULONG_MAX);
 			if (status != EFI_SUCCESS)
 				goto fail2;
-			hdr->ramdisk_image = ramdisk_addr & 0xffffffff;
-			hdr->ramdisk_size  = ramdisk_size & 0xffffffff;
-			boot_params->ext_ramdisk_image = (u64)ramdisk_addr >> 32;
-			boot_params->ext_ramdisk_size  = (u64)ramdisk_size >> 32;
+			efi_set_u64_split(ramdisk_addr, &hdr->ramdisk_image,
+					  &boot_params->ext_ramdisk_image);
+			efi_set_u64_split(ramdisk_size, &hdr->ramdisk_size,
+					  &boot_params->ext_ramdisk_size);
 		}
 	}
 
@@ -639,17 +638,14 @@ static efi_status_t exit_boot_func(struct efi_boot_memmap *map,
 				   : EFI32_LOADER_SIGNATURE;
 	memcpy(&p->efi->efi_loader_signature, signature, sizeof(__u32));
 
-	p->efi->efi_systab		= (unsigned long)efi_system_table;
+	efi_set_u64_split((unsigned long)efi_system_table,
+			  &p->efi->efi_systab, &p->efi->efi_systab_hi);
 	p->efi->efi_memdesc_size	= *map->desc_size;
 	p->efi->efi_memdesc_version	= *map->desc_ver;
-	p->efi->efi_memmap		= (unsigned long)*map->map;
+	efi_set_u64_split((unsigned long)*map->map,
+			  &p->efi->efi_memmap, &p->efi->efi_memmap_hi);
 	p->efi->efi_memmap_size		= *map->map_size;
 
-#ifdef CONFIG_X86_64
-	p->efi->efi_systab_hi		= (unsigned long)efi_system_table >> 32;
-	p->efi->efi_memmap_hi		= (unsigned long)*map->map >> 32;
-#endif
-
 	return EFI_SUCCESS;
 }
 
@@ -785,10 +781,10 @@ unsigned long efi_main(efi_handle_t handle,
 
 		status = efi_load_initrd_dev_path(&addr, &size, ULONG_MAX);
 		if (status == EFI_SUCCESS) {
-			hdr->ramdisk_image		= (u32)addr;
-			hdr->ramdisk_size 		= (u32)size;
-			boot_params->ext_ramdisk_image	= (u64)addr >> 32;
-			boot_params->ext_ramdisk_size 	= (u64)size >> 32;
+			efi_set_u64_split(addr, &hdr->ramdisk_image,
+					  &boot_params->ext_ramdisk_image);
+			efi_set_u64_split(size, &hdr->ramdisk_size,
+					  &boot_params->ext_ramdisk_size);
 		} else if (status != EFI_NOT_FOUND) {
 			efi_printk("efi_load_initrd_dev_path() failed!\n");
 			goto fail;
-- 
2.26.2

[PATCH v2 03/11] efi/libstub: Move pr_efi/pr_efi_err into efi namespace

[Arvind Sankar]

Rename pr_efi to efi_info and pr_efi_err to efi_err to make it more
obvious that they are part of the EFI stub and not generic printk infra.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Suggested-by: Joe Perches <joe@perches.com>
---
 drivers/firmware/efi/libstub/arm32-stub.c | 12 ++++-----
 drivers/firmware/efi/libstub/arm64-stub.c | 14 +++++-----
 drivers/firmware/efi/libstub/efi-stub.c   | 32 +++++++++++------------
 drivers/firmware/efi/libstub/efistub.h    |  4 +--
 drivers/firmware/efi/libstub/fdt.c        | 16 ++++++------
 drivers/firmware/efi/libstub/file.c       | 12 ++++-----
 drivers/firmware/efi/libstub/pci.c        |  8 +++---
 drivers/firmware/efi/libstub/relocate.c   |  2 +-
 drivers/firmware/efi/libstub/secureboot.c |  4 +--
 9 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c
index 7826553af2ba..b038afe2ee7a 100644
--- a/drivers/firmware/efi/libstub/arm32-stub.c
+++ b/drivers/firmware/efi/libstub/arm32-stub.c
@@ -18,7 +18,7 @@ efi_status_t check_platform_features(void)
 	/* LPAE kernels need compatible hardware */
 	block = cpuid_feature_extract(CPUID_EXT_MMFR0, 0);
 	if (block < 5) {
-		pr_efi_err("This LPAE kernel is not supported by your CPU\n");
+		efi_err("This LPAE kernel is not supported by your CPU\n");
 		return EFI_UNSUPPORTED;
 	}
 	return EFI_SUCCESS;
@@ -120,7 +120,7 @@ static efi_status_t reserve_kernel_base(unsigned long dram_base,
 	 */
 	status = efi_get_memory_map(&map);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("reserve_kernel_base(): Unable to retrieve memory map.\n");
+		efi_err("reserve_kernel_base(): Unable to retrieve memory map.\n");
 		return status;
 	}
 
@@ -162,7 +162,7 @@ static efi_status_t reserve_kernel_base(unsigned long dram_base,
 					     (end - start) / EFI_PAGE_SIZE,
 					     &start);
 			if (status != EFI_SUCCESS) {
-				pr_efi_err("reserve_kernel_base(): alloc failed.\n");
+				efi_err("reserve_kernel_base(): alloc failed.\n");
 				goto out;
 			}
 			break;
@@ -219,7 +219,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 
 	status = reserve_kernel_base(kernel_base, reserve_addr, reserve_size);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Unable to allocate memory for uncompressed kernel.\n");
+		efi_err("Unable to allocate memory for uncompressed kernel.\n");
 		return status;
 	}
 
@@ -232,7 +232,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 	status = efi_relocate_kernel(image_addr, *image_size, *image_size,
 				     kernel_base + MAX_UNCOMP_KERNEL_SIZE, 0, 0);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to relocate kernel.\n");
+		efi_err("Failed to relocate kernel.\n");
 		efi_free(*reserve_size, *reserve_addr);
 		*reserve_size = 0;
 		return status;
@@ -244,7 +244,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 	 * address at which the zImage is loaded.
 	 */
 	if (*image_addr + *image_size > dram_base + ZIMAGE_OFFSET_LIMIT) {
-		pr_efi_err("Failed to relocate kernel, no low memory available.\n");
+		efi_err("Failed to relocate kernel, no low memory available.\n");
 		efi_free(*reserve_size, *reserve_addr);
 		*reserve_size = 0;
 		efi_free(*image_size, *image_addr);
diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index ba4db35015a3..7f6a57dec513 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -26,9 +26,9 @@ efi_status_t check_platform_features(void)
 	tg = (read_cpuid(ID_AA64MMFR0_EL1) >> ID_AA64MMFR0_TGRAN_SHIFT) & 0xf;
 	if (tg != ID_AA64MMFR0_TGRAN_SUPPORTED) {
 		if (IS_ENABLED(CONFIG_ARM64_64K_PAGES))
-			pr_efi_err("This 64 KB granular kernel is not supported by your CPU\n");
+			efi_err("This 64 KB granular kernel is not supported by your CPU\n");
 		else
-			pr_efi_err("This 16 KB granular kernel is not supported by your CPU\n");
+			efi_err("This 16 KB granular kernel is not supported by your CPU\n");
 		return EFI_UNSUPPORTED;
 	}
 	return EFI_SUCCESS;
@@ -59,18 +59,18 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 			status = efi_get_random_bytes(sizeof(phys_seed),
 						      (u8 *)&phys_seed);
 			if (status == EFI_NOT_FOUND) {
-				pr_efi("EFI_RNG_PROTOCOL unavailable, no randomness supplied\n");
+				efi_info("EFI_RNG_PROTOCOL unavailable, no randomness supplied\n");
 			} else if (status != EFI_SUCCESS) {
-				pr_efi_err("efi_get_random_bytes() failed\n");
+				efi_err("efi_get_random_bytes() failed\n");
 				return status;
 			}
 		} else {
-			pr_efi("KASLR disabled on kernel command line\n");
+			efi_info("KASLR disabled on kernel command line\n");
 		}
 	}
 
 	if (image->image_base != _text)
-		pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
+		efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
 
 	kernel_size = _edata - _text;
 	kernel_memsize = kernel_size + (_end - _edata);
@@ -102,7 +102,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 						    ULONG_MAX, min_kimg_align);
 
 		if (status != EFI_SUCCESS) {
-			pr_efi_err("Failed to relocate kernel\n");
+			efi_err("Failed to relocate kernel\n");
 			*reserve_size = 0;
 			return status;
 		}
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index ee225b323687..72ffd2670f99 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -69,7 +69,7 @@ static void install_memreserve_table(void)
 	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, sizeof(*rsv),
 			     (void **)&rsv);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to allocate memreserve entry!\n");
+		efi_err("Failed to allocate memreserve entry!\n");
 		return;
 	}
 
@@ -80,7 +80,7 @@ static void install_memreserve_table(void)
 	status = efi_bs_call(install_configuration_table,
 			     &memreserve_table_guid, rsv);
 	if (status != EFI_SUCCESS)
-		pr_efi_err("Failed to install memreserve config table!\n");
+		efi_err("Failed to install memreserve config table!\n");
 }
 
 static unsigned long get_dram_base(void)
@@ -182,13 +182,13 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 	status = efi_system_table->boottime->handle_protocol(handle,
 					&loaded_image_proto, (void *)&image);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to get loaded image protocol\n");
+		efi_err("Failed to get loaded image protocol\n");
 		goto fail;
 	}
 
 	dram_base = get_dram_base();
 	if (dram_base == EFI_ERROR) {
-		pr_efi_err("Failed to find DRAM base\n");
+		efi_err("Failed to find DRAM base\n");
 		status = EFI_LOAD_ERROR;
 		goto fail;
 	}
@@ -200,7 +200,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 	 */
 	cmdline_ptr = efi_convert_cmdline(image, &cmdline_size, ULONG_MAX);
 	if (!cmdline_ptr) {
-		pr_efi_err("getting command line via LOADED_IMAGE_PROTOCOL\n");
+		efi_err("getting command line via LOADED_IMAGE_PROTOCOL\n");
 		status = EFI_OUT_OF_RESOURCES;
 		goto fail;
 	}
@@ -213,7 +213,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 	if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && cmdline_size > 0)
 		efi_parse_options(cmdline_ptr);
 
-	pr_efi("Booting Linux Kernel...\n");
+	efi_info("Booting Linux Kernel...\n");
 
 	si = setup_graphics();
 
@@ -222,7 +222,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 				     &reserve_size,
 				     dram_base, image);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to relocate kernel\n");
+		efi_err("Failed to relocate kernel\n");
 		goto fail_free_cmdline;
 	}
 
@@ -241,42 +241,42 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 	if (!IS_ENABLED(CONFIG_EFI_ARMSTUB_DTB_LOADER) ||
 	     secure_boot != efi_secureboot_mode_disabled) {
 		if (strstr(cmdline_ptr, "dtb="))
-			pr_efi("Ignoring DTB from command line.\n");
+			efi_info("Ignoring DTB from command line.\n");
 	} else {
 		status = efi_load_dtb(image, &fdt_addr, &fdt_size);
 
 		if (status != EFI_SUCCESS) {
-			pr_efi_err("Failed to load device tree!\n");
+			efi_err("Failed to load device tree!\n");
 			goto fail_free_image;
 		}
 	}
 
 	if (fdt_addr) {
-		pr_efi("Using DTB from command line\n");
+		efi_info("Using DTB from command line\n");
 	} else {
 		/* Look for a device tree configuration table entry. */
 		fdt_addr = (uintptr_t)get_fdt(&fdt_size);
 		if (fdt_addr)
-			pr_efi("Using DTB from configuration table\n");
+			efi_info("Using DTB from configuration table\n");
 	}
 
 	if (!fdt_addr)
-		pr_efi("Generating empty DTB\n");
+		efi_info("Generating empty DTB\n");
 
 	if (!efi_noinitrd) {
 		max_addr = efi_get_max_initrd_addr(dram_base, image_addr);
 		status = efi_load_initrd_dev_path(&initrd_addr, &initrd_size,
 						  max_addr);
 		if (status == EFI_SUCCESS) {
-			pr_efi("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
+			efi_info("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
 		} else if (status == EFI_NOT_FOUND) {
 			status = efi_load_initrd(image, &initrd_addr, &initrd_size,
 						 ULONG_MAX, max_addr);
 			if (status == EFI_SUCCESS && initrd_size > 0)
-				pr_efi("Loaded initrd from command line option\n");
+				efi_info("Loaded initrd from command line option\n");
 		}
 		if (status != EFI_SUCCESS)
-			pr_efi_err("Failed to load initrd!\n");
+			efi_err("Failed to load initrd!\n");
 	}
 
 	efi_random_get_seed();
@@ -326,7 +326,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 	/* not reached */
 
 fail_free_initrd:
-	pr_efi_err("Failed to update FDT and exit boot services\n");
+	efi_err("Failed to update FDT and exit boot services\n");
 
 	efi_free(initrd_size, initrd_addr);
 	efi_free(fdt_size, fdt_addr);
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index e8aa70ba08d5..8c905a1be1b4 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -49,11 +49,11 @@ extern const efi_system_table_t *efi_system_table;
 #define efi_call_proto(inst, func, ...) inst->func(inst, ##__VA_ARGS__)
 #endif
 
-#define pr_efi(msg)		do {			\
+#define efi_info(msg)		do {			\
 	if (!efi_quiet) efi_printk("EFI stub: "msg);	\
 } while (0)
 
-#define pr_efi_err(msg) efi_printk("EFI stub: ERROR: "msg)
+#define efi_err(msg) efi_printk("EFI stub: ERROR: "msg)
 
 /* Helper macros for the usual case of using simple C variables: */
 #ifndef fdt_setprop_inplace_var
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 3074a5e27c65..11ecf3c4640e 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -39,7 +39,7 @@ static efi_status_t update_fdt(void *orig_fdt, unsigned long orig_fdt_size,
 	/* Do some checks on provided FDT, if it exists: */
 	if (orig_fdt) {
 		if (fdt_check_header(orig_fdt)) {
-			pr_efi_err("Device Tree header not valid!\n");
+			efi_err("Device Tree header not valid!\n");
 			return EFI_LOAD_ERROR;
 		}
 		/*
@@ -47,7 +47,7 @@ static efi_status_t update_fdt(void *orig_fdt, unsigned long orig_fdt_size,
 		 * configuration table:
 		 */
 		if (orig_fdt_size && fdt_totalsize(orig_fdt) > orig_fdt_size) {
-			pr_efi_err("Truncated device tree! foo!\n");
+			efi_err("Truncated device tree! foo!\n");
 			return EFI_LOAD_ERROR;
 		}
 	}
@@ -270,16 +270,16 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
 	 */
 	status = efi_get_memory_map(&map);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Unable to retrieve UEFI memory map.\n");
+		efi_err("Unable to retrieve UEFI memory map.\n");
 		return status;
 	}
 
-	pr_efi("Exiting boot services and installing virtual address map...\n");
+	efi_info("Exiting boot services and installing virtual address map...\n");
 
 	map.map = &memory_map;
 	status = efi_allocate_pages(MAX_FDT_SIZE, new_fdt_addr, max_addr);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Unable to allocate memory for new device tree.\n");
+		efi_err("Unable to allocate memory for new device tree.\n");
 		goto fail;
 	}
 
@@ -296,7 +296,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
 			    initrd_addr, initrd_size);
 
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Unable to construct new device tree.\n");
+		efi_err("Unable to construct new device tree.\n");
 		goto fail_free_new_fdt;
 	}
 
@@ -342,7 +342,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
 		return EFI_SUCCESS;
 	}
 
-	pr_efi_err("Exit boot services failed.\n");
+	efi_err("Exit boot services failed.\n");
 
 fail_free_new_fdt:
 	efi_free(MAX_FDT_SIZE, *new_fdt_addr);
@@ -363,7 +363,7 @@ void *get_fdt(unsigned long *fdt_size)
 		return NULL;
 
 	if (fdt_check_header(fdt) != 0) {
-		pr_efi_err("Invalid header detected on UEFI supplied FDT, ignoring ...\n");
+		efi_err("Invalid header detected on UEFI supplied FDT, ignoring ...\n");
 		return NULL;
 	}
 	*fdt_size = fdt_totalsize(fdt);
diff --git a/drivers/firmware/efi/libstub/file.c b/drivers/firmware/efi/libstub/file.c
index 50aaf15f9ad5..cc177152d0df 100644
--- a/drivers/firmware/efi/libstub/file.c
+++ b/drivers/firmware/efi/libstub/file.c
@@ -46,7 +46,7 @@ static efi_status_t efi_open_file(efi_file_protocol_t *volume,
 
 	status = volume->open(volume, &fh, fi->filename, EFI_FILE_MODE_READ, 0);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to open file: ");
+		efi_err("Failed to open file: ");
 		efi_char16_printk(fi->filename);
 		efi_printk("\n");
 		return status;
@@ -55,7 +55,7 @@ static efi_status_t efi_open_file(efi_file_protocol_t *volume,
 	info_sz = sizeof(struct finfo);
 	status = fh->get_info(fh, &info_guid, &info_sz, fi);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to get file info\n");
+		efi_err("Failed to get file info\n");
 		fh->close(fh);
 		return status;
 	}
@@ -75,13 +75,13 @@ static efi_status_t efi_open_volume(efi_loaded_image_t *image,
 	status = efi_bs_call(handle_protocol, image->device_handle, &fs_proto,
 			     (void **)&io);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to handle fs_proto\n");
+		efi_err("Failed to handle fs_proto\n");
 		return status;
 	}
 
 	status = io->open_volume(io, fh);
 	if (status != EFI_SUCCESS)
-		pr_efi_err("Failed to open volume\n");
+		efi_err("Failed to open volume\n");
 
 	return status;
 }
@@ -191,7 +191,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
 							    &alloc_addr,
 							    hard_limit);
 			if (status != EFI_SUCCESS) {
-				pr_efi_err("Failed to allocate memory for files\n");
+				efi_err("Failed to allocate memory for files\n");
 				goto err_close_file;
 			}
 
@@ -215,7 +215,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
 
 			status = file->read(file, &chunksize, addr);
 			if (status != EFI_SUCCESS) {
-				pr_efi_err("Failed to read file\n");
+				efi_err("Failed to read file\n");
 				goto err_close_file;
 			}
 			addr += chunksize;
diff --git a/drivers/firmware/efi/libstub/pci.c b/drivers/firmware/efi/libstub/pci.c
index b025e59b94df..60af51bed573 100644
--- a/drivers/firmware/efi/libstub/pci.c
+++ b/drivers/firmware/efi/libstub/pci.c
@@ -28,21 +28,21 @@ void efi_pci_disable_bridge_busmaster(void)
 
 	if (status != EFI_BUFFER_TOO_SMALL) {
 		if (status != EFI_SUCCESS && status != EFI_NOT_FOUND)
-			pr_efi_err("Failed to locate PCI I/O handles'\n");
+			efi_err("Failed to locate PCI I/O handles'\n");
 		return;
 	}
 
 	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, pci_handle_size,
 			     (void **)&pci_handle);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to allocate memory for 'pci_handle'\n");
+		efi_err("Failed to allocate memory for 'pci_handle'\n");
 		return;
 	}
 
 	status = efi_bs_call(locate_handle, EFI_LOCATE_BY_PROTOCOL, &pci_proto,
 			     NULL, &pci_handle_size, pci_handle);
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to locate PCI I/O handles'\n");
+		efi_err("Failed to locate PCI I/O handles'\n");
 		goto free_handle;
 	}
 
@@ -106,7 +106,7 @@ void efi_pci_disable_bridge_busmaster(void)
 		status = efi_call_proto(pci, pci.write, EfiPciIoWidthUint16,
 					PCI_COMMAND, 1, &command);
 		if (status != EFI_SUCCESS)
-			pr_efi_err("Failed to disable PCI busmastering\n");
+			efi_err("Failed to disable PCI busmastering\n");
 	}
 
 free_handle:
diff --git a/drivers/firmware/efi/libstub/relocate.c b/drivers/firmware/efi/libstub/relocate.c
index 1507d3c82884..93c04d6aaed1 100644
--- a/drivers/firmware/efi/libstub/relocate.c
+++ b/drivers/firmware/efi/libstub/relocate.c
@@ -157,7 +157,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
 					     min_addr);
 	}
 	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to allocate usable memory for kernel.\n");
+		efi_err("Failed to allocate usable memory for kernel.\n");
 		return status;
 	}
 
diff --git a/drivers/firmware/efi/libstub/secureboot.c b/drivers/firmware/efi/libstub/secureboot.c
index a765378ad18c..5efc524b14be 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -67,10 +67,10 @@ enum efi_secureboot_mode efi_get_secureboot(void)
 		return efi_secureboot_mode_disabled;
 
 secure_boot_enabled:
-	pr_efi("UEFI Secure Boot is enabled.\n");
+	efi_info("UEFI Secure Boot is enabled.\n");
 	return efi_secureboot_mode_enabled;
 
 out_efi_err:
-	pr_efi_err("Could not determine UEFI Secure Boot status.\n");
+	efi_err("Could not determine UEFI Secure Boot status.\n");
 	return efi_secureboot_mode_unknown;
 }
-- 
2.26.2

[PATCH v2 04/11] efi/x86: Use efi_err for error messages

[Arvind Sankar]

Use efi_err instead of bare efi_printk for error messages.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/x86-stub.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index f91d4aab0156..3800eb22232e 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -49,7 +49,7 @@ preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
 	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, size,
 			     (void **)&rom);
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to allocate memory for 'rom'\n");
+		efi_err("Failed to allocate memory for 'rom'\n");
 		return status;
 	}
 
@@ -65,7 +65,7 @@ preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
 				PCI_VENDOR_ID, 1, &rom->vendor);
 
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to read rom->vendor\n");
+		efi_err("Failed to read rom->vendor\n");
 		goto free_struct;
 	}
 
@@ -73,7 +73,7 @@ preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
 				PCI_DEVICE_ID, 1, &rom->devid);
 
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to read rom->devid\n");
+		efi_err("Failed to read rom->devid\n");
 		goto free_struct;
 	}
 
@@ -118,7 +118,7 @@ static void setup_efi_pci(struct boot_params *params)
 				     (void **)&pci_handle);
 
 		if (status != EFI_SUCCESS) {
-			efi_printk("Failed to allocate memory for 'pci_handle'\n");
+			efi_err("Failed to allocate memory for 'pci_handle'\n");
 			return;
 		}
 
@@ -172,7 +172,7 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params)
 		return;
 
 	if (efi_table_attr(p, version) != 0x10000) {
-		efi_printk("Unsupported properties proto version\n");
+		efi_err("Unsupported properties proto version\n");
 		return;
 	}
 
@@ -185,7 +185,7 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params)
 				     size + sizeof(struct setup_data),
 				     (void **)&new);
 		if (status != EFI_SUCCESS) {
-			efi_printk("Failed to allocate memory for 'properties'\n");
+			efi_err("Failed to allocate memory for 'properties'\n");
 			return;
 		}
 
@@ -372,7 +372,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 
 	status = efi_bs_call(handle_protocol, handle, &proto, (void **)&image);
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to get handle for LOADED_IMAGE_PROTOCOL\n");
+		efi_err("Failed to get handle for LOADED_IMAGE_PROTOCOL\n");
 		efi_exit(handle, status);
 	}
 
@@ -382,7 +382,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	status = efi_allocate_pages(sizeof(struct boot_params),
 				    (unsigned long *)&boot_params, ULONG_MAX);
 	if (status != EFI_SUCCESS) {
-		efi_printk("Failed to allocate lowmem for boot params\n");
+		efi_err("Failed to allocate lowmem for boot params\n");
 		efi_exit(handle, status);
 	}
 
@@ -749,7 +749,7 @@ unsigned long efi_main(efi_handle_t handle,
 					     hdr->kernel_alignment,
 					     LOAD_PHYSICAL_ADDR);
 		if (status != EFI_SUCCESS) {
-			efi_printk("efi_relocate_kernel() failed!\n");
+			efi_err("efi_relocate_kernel() failed!\n");
 			goto fail;
 		}
 		/*
@@ -786,7 +786,7 @@ unsigned long efi_main(efi_handle_t handle,
 			efi_set_u64_split(size, &hdr->ramdisk_size,
 					  &boot_params->ext_ramdisk_size);
 		} else if (status != EFI_NOT_FOUND) {
-			efi_printk("efi_load_initrd_dev_path() failed!\n");
+			efi_err("efi_load_initrd_dev_path() failed!\n");
 			goto fail;
 		}
 	}
@@ -813,13 +813,13 @@ unsigned long efi_main(efi_handle_t handle,
 
 	status = exit_boot(boot_params, handle);
 	if (status != EFI_SUCCESS) {
-		efi_printk("exit_boot() failed!\n");
+		efi_err("exit_boot() failed!\n");
 		goto fail;
 	}
 
 	return bzimage_addr;
 fail:
-	efi_printk("efi_main() failed!\n");
+	efi_err("efi_main() failed!\n");
 
 	efi_exit(handle, status);
 }
-- 
2.26.2

[PATCH v2 05/11] efi/gop: Use efi_err for error messages

[Arvind Sankar]

Use efi_err instead of bare efi_printk for error messages.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/gop.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 64cee0febae0..34c0cba2c8bf 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -134,14 +134,14 @@ static u32 choose_mode_modenum(efi_graphics_output_protocol_t *gop)
 
 	max_mode = efi_table_attr(mode, max_mode);
 	if (cmdline.mode >= max_mode) {
-		efi_printk("Requested mode is invalid\n");
+		efi_err("Requested mode is invalid\n");
 		return cur_mode;
 	}
 
 	status = efi_call_proto(gop, query_mode, cmdline.mode,
 				&info_size, &info);
 	if (status != EFI_SUCCESS) {
-		efi_printk("Couldn't get mode information\n");
+		efi_err("Couldn't get mode information\n");
 		return cur_mode;
 	}
 
@@ -150,7 +150,7 @@ static u32 choose_mode_modenum(efi_graphics_output_protocol_t *gop)
 	efi_bs_call(free_pool, info);
 
 	if (pf == PIXEL_BLT_ONLY || pf >= PIXEL_FORMAT_MAX) {
-		efi_printk("Invalid PixelFormat\n");
+		efi_err("Invalid PixelFormat\n");
 		return cur_mode;
 	}
 
@@ -222,7 +222,7 @@ static u32 choose_mode_res(efi_graphics_output_protocol_t *gop)
 			return m;
 	}
 
-	efi_printk("Couldn't find requested mode\n");
+	efi_err("Couldn't find requested mode\n");
 
 	return cur_mode;
 }
@@ -316,7 +316,7 @@ static void set_mode(efi_graphics_output_protocol_t *gop)
 		return;
 
 	if (efi_call_proto(gop, set_mode, new_mode) != EFI_SUCCESS)
-		efi_printk("Failed to set requested mode\n");
+		efi_err("Failed to set requested mode\n");
 }
 
 static void find_bits(u32 mask, u8 *pos, u8 *size)
-- 
2.26.2

[PATCH v2 06/11] efi/tpm: Use efi_err for error messages

[Arvind Sankar]

Use efi_err instead of bare efi_printk for error messages.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/tpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
index 1d59e103a2e3..09adcf51b75b 100644
--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c
@@ -119,7 +119,7 @@ void efi_retrieve_tpm2_eventlog(void)
 			     sizeof(*log_tbl) + log_size, (void **)&log_tbl);
 
 	if (status != EFI_SUCCESS) {
-		efi_printk("Unable to allocate memory for event log\n");
+		efi_err("Unable to allocate memory for event log\n");
 		return;
 	}
 
-- 
2.26.2

[PATCH v2 07/11] efi/libstub: Upgrade ignored dtb= argument message to error

[Arvind Sankar]

Use efi_err if we ignore a command-line dtb= argument, so that it shows
up even on a quiet boot.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/efi-stub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index 72ffd2670f99..cb02e8bb6b44 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -241,7 +241,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 	if (!IS_ENABLED(CONFIG_EFI_ARMSTUB_DTB_LOADER) ||
 	     secure_boot != efi_secureboot_mode_disabled) {
 		if (strstr(cmdline_ptr, "dtb="))
-			efi_info("Ignoring DTB from command line.\n");
+			efi_err("Ignoring DTB from command line.\n");
 	} else {
 		status = efi_load_dtb(image, &fdt_addr, &fdt_size);
 
-- 
2.26.2

[PATCH v2 08/11] efi/x86: Move command-line initrd loading to efi_main

[Arvind Sankar]

Consolidate the initrd loading in efi_main.

The command line options now need to be parsed only once.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/x86-stub.c | 64 ++++++++++---------------
 1 file changed, 25 insertions(+), 39 deletions(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 3800eb22232e..defeb6035109 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -22,6 +22,7 @@
 
 const efi_system_table_t *efi_system_table;
 extern u32 image_offset;
+static efi_loaded_image_t *image = NULL;
 
 static efi_status_t
 preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
@@ -355,7 +356,6 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 {
 	struct boot_params *boot_params;
 	struct setup_header *hdr;
-	efi_loaded_image_t *image;
 	void *image_base;
 	efi_guid_t proto = LOADED_IMAGE_PROTOCOL_GUID;
 	int options_size = 0;
@@ -414,30 +414,9 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	hdr->ramdisk_image = 0;
 	hdr->ramdisk_size = 0;
 
-	if (efi_is_native()) {
-		status = efi_parse_options(cmdline_ptr);
-		if (status != EFI_SUCCESS)
-			goto fail2;
-
-		if (!efi_noinitrd) {
-			status = efi_load_initrd(image, &ramdisk_addr,
-						 &ramdisk_size,
-						 hdr->initrd_addr_max,
-						 ULONG_MAX);
-			if (status != EFI_SUCCESS)
-				goto fail2;
-			efi_set_u64_split(ramdisk_addr, &hdr->ramdisk_image,
-					  &boot_params->ext_ramdisk_image);
-			efi_set_u64_split(ramdisk_size, &hdr->ramdisk_size,
-					  &boot_params->ext_ramdisk_size);
-		}
-	}
-
 	efi_stub_entry(handle, sys_table_arg, boot_params);
 	/* not reached */
 
-fail2:
-	efi_free(options_size, (unsigned long)cmdline_ptr);
 fail:
 	efi_free(sizeof(struct boot_params), (unsigned long)boot_params);
 
@@ -760,35 +739,42 @@ unsigned long efi_main(efi_handle_t handle,
 		image_offset = 0;
 	}
 
-	/*
-	 * efi_pe_entry() may have been called before efi_main(), in which
-	 * case this is the second time we parse the cmdline. This is ok,
-	 * parsing the cmdline multiple times does not have side-effects.
-	 */
 	cmdline_paddr = ((u64)hdr->cmd_line_ptr |
 			 ((u64)boot_params->ext_cmd_line_ptr << 32));
 	efi_parse_options((char *)cmdline_paddr);
 
 	/*
-	 * At this point, an initrd may already have been loaded, either by
-	 * the bootloader and passed via bootparams, or loaded from a initrd=
-	 * command line option by efi_pe_entry() above. In either case, we
-	 * permit an initrd loaded from the LINUX_EFI_INITRD_MEDIA_GUID device
-	 * path to supersede it.
+	 * At this point, an initrd may already have been loaded by the
+	 * bootloader and passed via bootparams. We permit an initrd loaded
+	 * from the LINUX_EFI_INITRD_MEDIA_GUID device path to supersede it.
+	 *
+	 * If the device path is not present, any command-line initrd=
+	 * arguments will be processed only if image is not NULL, which will be
+	 * the case only if we were loaded via the PE entry point.
 	 */
 	if (!efi_noinitrd) {
 		unsigned long addr, size;
 
 		status = efi_load_initrd_dev_path(&addr, &size, ULONG_MAX);
-		if (status == EFI_SUCCESS) {
-			efi_set_u64_split(addr, &hdr->ramdisk_image,
-					  &boot_params->ext_ramdisk_image);
-			efi_set_u64_split(size, &hdr->ramdisk_size,
-					  &boot_params->ext_ramdisk_size);
-		} else if (status != EFI_NOT_FOUND) {
-			efi_err("efi_load_initrd_dev_path() failed!\n");
+		if (status == EFI_NOT_FOUND) {
+			if (efi_is_native() && image != NULL) {
+				status = efi_load_initrd(image, &addr, &size,
+							 hdr->initrd_addr_max,
+							 ULONG_MAX);
+			} else {
+				addr = size = 0;
+				status = EFI_SUCCESS;
+			}
+		}
+
+		if (status != EFI_SUCCESS) {
+			efi_err("Failed to load initrd!\n");
 			goto fail;
 		}
+		efi_set_u64_split(addr, &hdr->ramdisk_image,
+				  &boot_params->ext_ramdisk_image);
+		efi_set_u64_split(size, &hdr->ramdisk_size,
+				  &boot_params->ext_ramdisk_size);
 	}
 
 	/*
-- 
2.26.2

[PATCH v2 09/11] efi/libstub: Unify initrd loading across architectures

[Arvind Sankar]

Factor out the initrd loading into a common function that can be called
both from the generic efi-stub.c and the x86-specific x86-stub.c.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 .../firmware/efi/libstub/efi-stub-helper.c    | 46 +++++++++++++++++--
 drivers/firmware/efi/libstub/efi-stub.c       | 12 +----
 drivers/firmware/efi/libstub/efistub.h        | 21 ++-------
 drivers/firmware/efi/libstub/x86-stub.c       | 13 +-----
 4 files changed, 52 insertions(+), 40 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 1c92ac231f94..7aac89e928ec 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -331,6 +331,7 @@ static const struct {
  *		%EFI_OUT_OF_RESOURCES if memory allocation failed
  *		%EFI_LOAD_ERROR in all other cases
  */
+static
 efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
 				      unsigned long *load_size,
 				      unsigned long max)
@@ -343,9 +344,6 @@ efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
 	efi_handle_t handle;
 	efi_status_t status;
 
-	if (!load_addr || !load_size)
-		return EFI_INVALID_PARAMETER;
-
 	dp = (efi_device_path_protocol_t *)&initrd_dev_path;
 	status = efi_bs_call(locate_device_path, &lf2_proto_guid, &dp, &handle);
 	if (status != EFI_SUCCESS)
@@ -375,3 +373,45 @@ efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
 	*load_size = initrd_size;
 	return EFI_SUCCESS;
 }
+
+static
+efi_status_t efi_load_initrd_cmdline(efi_loaded_image_t *image,
+				     unsigned long *load_addr,
+				     unsigned long *load_size,
+				     unsigned long soft_limit,
+				     unsigned long hard_limit)
+{
+	if (!IS_ENABLED(CONFIG_EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER) ||
+	    (IS_ENABLED(CONFIG_X86) && (!efi_is_native() || image == NULL))) {
+		*load_addr = *load_size = 0;
+		return EFI_SUCCESS;
+	}
+
+	return handle_cmdline_files(image, L"initrd=", sizeof(L"initrd=") - 2,
+				    soft_limit, hard_limit,
+				    load_addr, load_size);
+}
+
+efi_status_t efi_load_initrd(efi_loaded_image_t *image,
+			     unsigned long *load_addr,
+			     unsigned long *load_size,
+			     unsigned long soft_limit,
+			     unsigned long hard_limit)
+{
+	efi_status_t status;
+
+	if (!load_addr || !load_size)
+		return EFI_INVALID_PARAMETER;
+
+	status = efi_load_initrd_dev_path(load_addr, load_size, hard_limit);
+	if (status == EFI_SUCCESS) {
+		efi_info("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
+	} else if (status == EFI_NOT_FOUND) {
+		status = efi_load_initrd_cmdline(image, load_addr, load_size,
+						 soft_limit, hard_limit);
+		if (status == EFI_SUCCESS && *load_size > 0)
+			efi_info("Loaded initrd from command line option\n");
+	}
+
+	return status;
+}
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index cb02e8bb6b44..63541c2440ef 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -265,16 +265,8 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 
 	if (!efi_noinitrd) {
 		max_addr = efi_get_max_initrd_addr(dram_base, image_addr);
-		status = efi_load_initrd_dev_path(&initrd_addr, &initrd_size,
-						  max_addr);
-		if (status == EFI_SUCCESS) {
-			efi_info("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
-		} else if (status == EFI_NOT_FOUND) {
-			status = efi_load_initrd(image, &initrd_addr, &initrd_size,
-						 ULONG_MAX, max_addr);
-			if (status == EFI_SUCCESS && initrd_size > 0)
-				efi_info("Loaded initrd from command line option\n");
-		}
+		status = efi_load_initrd(image, &initrd_addr, &initrd_size,
+					 ULONG_MAX, max_addr);
 		if (status != EFI_SUCCESS)
 			efi_err("Failed to load initrd!\n");
 	}
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 8c905a1be1b4..874233cf8820 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -677,21 +677,10 @@ static inline efi_status_t efi_load_dtb(efi_loaded_image_t *image,
 				    ULONG_MAX, ULONG_MAX, load_addr, load_size);
 }
 
-static inline efi_status_t efi_load_initrd(efi_loaded_image_t *image,
-					   unsigned long *load_addr,
-					   unsigned long *load_size,
-					   unsigned long soft_limit,
-					   unsigned long hard_limit)
-{
-	if (!IS_ENABLED(CONFIG_EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER))
-		return EFI_SUCCESS;
-
-	return handle_cmdline_files(image, L"initrd=", sizeof(L"initrd=") - 2,
-				    soft_limit, hard_limit, load_addr, load_size);
-}
-
-efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
-				      unsigned long *load_size,
-				      unsigned long max);
+efi_status_t efi_load_initrd(efi_loaded_image_t *image,
+			     unsigned long *load_addr,
+			     unsigned long *load_size,
+			     unsigned long soft_limit,
+			     unsigned long hard_limit);
 
 #endif
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index defeb6035109..f1a134596b53 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -755,17 +755,8 @@ unsigned long efi_main(efi_handle_t handle,
 	if (!efi_noinitrd) {
 		unsigned long addr, size;
 
-		status = efi_load_initrd_dev_path(&addr, &size, ULONG_MAX);
-		if (status == EFI_NOT_FOUND) {
-			if (efi_is_native() && image != NULL) {
-				status = efi_load_initrd(image, &addr, &size,
-							 hdr->initrd_addr_max,
-							 ULONG_MAX);
-			} else {
-				addr = size = 0;
-				status = EFI_SUCCESS;
-			}
-		}
+		status = efi_load_initrd(image, &addr, &size,
+					 hdr->initrd_addr_max, ULONG_MAX);
 
 		if (status != EFI_SUCCESS) {
 			efi_err("Failed to load initrd!\n");
-- 
2.26.2

[PATCH v2 10/11] efi/x86: Support builtin command line

[Arvind Sankar]

Add support for the x86 CMDLINE_BOOL and CMDLINE_OVERRIDE configuration
options.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/x86-stub.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index f1a134596b53..c84c5678e3e1 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -680,7 +680,6 @@ unsigned long efi_main(efi_handle_t handle,
 	unsigned long buffer_start, buffer_end;
 	struct setup_header *hdr = &boot_params->hdr;
 	efi_status_t status;
-	unsigned long cmdline_paddr;
 
 	efi_system_table = sys_table_arg;
 
@@ -739,9 +738,14 @@ unsigned long efi_main(efi_handle_t handle,
 		image_offset = 0;
 	}
 
-	cmdline_paddr = ((u64)hdr->cmd_line_ptr |
-			 ((u64)boot_params->ext_cmd_line_ptr << 32));
-	efi_parse_options((char *)cmdline_paddr);
+#ifdef CONFIG_CMDLINE_BOOL
+	efi_parse_options(CONFIG_CMDLINE);
+#endif
+	if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
+		unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
+					       ((u64)boot_params->ext_cmd_line_ptr << 32));
+		efi_parse_options((char *)cmdline_paddr);
+	}
 
 	/*
 	 * At this point, an initrd may already have been loaded by the
-- 
2.26.2

[PATCH v2 11/11] efi/libstub: Check return value of efi_parse_options

[Arvind Sankar]

efi_parse_options can fail if it is unable to allocate space for a copy
of the command line. Check the return value to make sure it succeeded.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/efi-stub.c | 23 +++++++++++++++++------
 drivers/firmware/efi/libstub/x86-stub.c | 12 ++++++++++--
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index 63541c2440ef..c2484bf75c5d 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -207,11 +207,21 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 
 	if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
 	    IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
-	    cmdline_size == 0)
-		efi_parse_options(CONFIG_CMDLINE);
+	    cmdline_size == 0) {
+		status = efi_parse_options(CONFIG_CMDLINE);
+		if (status != EFI_SUCCESS) {
+			efi_err("Failed to parse options\n");
+			goto fail_free_cmdline;
+		}
+	}
 
-	if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && cmdline_size > 0)
-		efi_parse_options(cmdline_ptr);
+	if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && cmdline_size > 0) {
+		status = efi_parse_options(cmdline_ptr);
+		if (status != EFI_SUCCESS) {
+			efi_err("Failed to parse options\n");
+			goto fail_free_cmdline;
+		}
+	}
 
 	efi_info("Booting Linux Kernel...\n");
 
@@ -223,7 +233,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 				     dram_base, image);
 	if (status != EFI_SUCCESS) {
 		efi_err("Failed to relocate kernel\n");
-		goto fail_free_cmdline;
+		goto fail_free_screeninfo;
 	}
 
 	efi_retrieve_tpm2_eventlog();
@@ -326,8 +336,9 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 fail_free_image:
 	efi_free(image_size, image_addr);
 	efi_free(reserve_size, reserve_addr);
-fail_free_cmdline:
+fail_free_screeninfo:
 	free_screen_info(si);
+fail_free_cmdline:
 	efi_free(cmdline_size, (unsigned long)cmdline_ptr);
 fail:
 	return status;
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index c84c5678e3e1..37154bb93c59 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -739,12 +739,20 @@ unsigned long efi_main(efi_handle_t handle,
 	}
 
 #ifdef CONFIG_CMDLINE_BOOL
-	efi_parse_options(CONFIG_CMDLINE);
+	status = efi_parse_options(CONFIG_CMDLINE);
+	if (status != EFI_SUCCESS) {
+		efi_err("Failed to parse options\n");
+		goto fail;
+	}
 #endif
 	if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
 		unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
 					       ((u64)boot_params->ext_cmd_line_ptr << 32));
-		efi_parse_options((char *)cmdline_paddr);
+		status = efi_parse_options((char *)cmdline_paddr);
+		if (status != EFI_SUCCESS) {
+			efi_err("Failed to parse options\n");
+			goto fail;
+		}
 	}
 
 	/*
-- 
2.26.2

[PATCH 1/2] efi/libstub: efi_info/efi_err message neatening

[Joe Perches]

Use a standard style for these output logging messages.

Miscellanea:

o Use more common macro #defines with fmt, ##__VA_ARGS__
0 Remove trailing messages periods and odd ' uses
o Remove embedded function names and use %s, __func__

Signed-off-by: Joe Perches <joe@perches.com>
---

Perhaps these trivialities on top of this series?

 drivers/firmware/efi/libstub/arm32-stub.c | 10 +++++-----
 drivers/firmware/efi/libstub/efi-stub.c   |  2 +-
 drivers/firmware/efi/libstub/efistub.h    |  9 ++++++---
 drivers/firmware/efi/libstub/fdt.c        |  8 ++++----
 drivers/firmware/efi/libstub/pci.c        |  4 ++--
 drivers/firmware/efi/libstub/relocate.c   |  2 +-
 drivers/firmware/efi/libstub/secureboot.c |  4 ++--
 7 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c
index b038afe..5795781 100644
--- a/drivers/firmware/efi/libstub/arm32-stub.c
+++ b/drivers/firmware/efi/libstub/arm32-stub.c
@@ -120,7 +120,7 @@ static efi_status_t reserve_kernel_base(unsigned long dram_base,
 	 */
 	status = efi_get_memory_map(&map);
 	if (status != EFI_SUCCESS) {
-		efi_err("reserve_kernel_base(): Unable to retrieve memory map.\n");
+		efi_err("%s(): Unable to retrieve memory map\n", __func__);
 		return status;
 	}
 
@@ -162,7 +162,7 @@ static efi_status_t reserve_kernel_base(unsigned long dram_base,
 					     (end - start) / EFI_PAGE_SIZE,
 					     &start);
 			if (status != EFI_SUCCESS) {
-				efi_err("reserve_kernel_base(): alloc failed.\n");
+				efi_err("%s(): alloc failed\n", __func__);
 				goto out;
 			}
 			break;
@@ -219,7 +219,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 
 	status = reserve_kernel_base(kernel_base, reserve_addr, reserve_size);
 	if (status != EFI_SUCCESS) {
-		efi_err("Unable to allocate memory for uncompressed kernel.\n");
+		efi_err("Unable to allocate memory for uncompressed kernel\n");
 		return status;
 	}
 
@@ -232,7 +232,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 	status = efi_relocate_kernel(image_addr, *image_size, *image_size,
 				     kernel_base + MAX_UNCOMP_KERNEL_SIZE, 0, 0);
 	if (status != EFI_SUCCESS) {
-		efi_err("Failed to relocate kernel.\n");
+		efi_err("Failed to relocate kernel\n");
 		efi_free(*reserve_size, *reserve_addr);
 		*reserve_size = 0;
 		return status;
@@ -244,7 +244,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 	 * address at which the zImage is loaded.
 	 */
 	if (*image_addr + *image_size > dram_base + ZIMAGE_OFFSET_LIMIT) {
-		efi_err("Failed to relocate kernel, no low memory available.\n");
+		efi_err("Failed to relocate kernel, no low memory available\n");
 		efi_free(*reserve_size, *reserve_addr);
 		*reserve_size = 0;
 		efi_free(*image_size, *image_addr);
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index c2484b..19b42b 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -251,7 +251,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
 	if (!IS_ENABLED(CONFIG_EFI_ARMSTUB_DTB_LOADER) ||
 	     secure_boot != efi_secureboot_mode_disabled) {
 		if (strstr(cmdline_ptr, "dtb="))
-			efi_err("Ignoring DTB from command line.\n");
+			efi_err("Ignoring DTB from command line\n");
 	} else {
 		status = efi_load_dtb(image, &fdt_addr, &fdt_size);
 
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 874233..369262 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -49,11 +49,14 @@ extern const efi_system_table_t *efi_system_table;
 #define efi_call_proto(inst, func, ...) inst->func(inst, ##__VA_ARGS__)
 #endif
 
-#define efi_info(msg)		do {			\
-	if (!efi_quiet) efi_printk("EFI stub: "msg);	\
+#define efi_info(fmt, ...)						\
+do {									\
+	if (!efi_quiet)							\
+		efi_printk("EFI stub: " fmt, ##__VA_ARGS__);		\
 } while (0)
 
-#define efi_err(msg) efi_printk("EFI stub: ERROR: "msg)
+#define efi_err(fmt, ...)						\
+	efi_printk("EFI stub: ERROR: " fmt, ##__VA_ARGS__)
 
 /* Helper macros for the usual case of using simple C variables: */
 #ifndef fdt_setprop_inplace_var
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 11ecf3c..7c7257 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -270,7 +270,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
 	 */
 	status = efi_get_memory_map(&map);
 	if (status != EFI_SUCCESS) {
-		efi_err("Unable to retrieve UEFI memory map.\n");
+		efi_err("Unable to retrieve UEFI memory map\n");
 		return status;
 	}
 
@@ -279,7 +279,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
 	map.map = &memory_map;
 	status = efi_allocate_pages(MAX_FDT_SIZE, new_fdt_addr, max_addr);
 	if (status != EFI_SUCCESS) {
-		efi_err("Unable to allocate memory for new device tree.\n");
+		efi_err("Unable to allocate memory for new device tree\n");
 		goto fail;
 	}
 
@@ -296,7 +296,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
 			    initrd_addr, initrd_size);
 
 	if (status != EFI_SUCCESS) {
-		efi_err("Unable to construct new device tree.\n");
+		efi_err("Unable to construct new device tree\n");
 		goto fail_free_new_fdt;
 	}
 
@@ -342,7 +342,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
 		return EFI_SUCCESS;
 	}
 
-	efi_err("Exit boot services failed.\n");
+	efi_err("Exit boot services failed\n");
 
 fail_free_new_fdt:
 	efi_free(MAX_FDT_SIZE, *new_fdt_addr);
diff --git a/drivers/firmware/efi/libstub/pci.c b/drivers/firmware/efi/libstub/pci.c
index 60af51b..111c44b 100644
--- a/drivers/firmware/efi/libstub/pci.c
+++ b/drivers/firmware/efi/libstub/pci.c
@@ -28,7 +28,7 @@ void efi_pci_disable_bridge_busmaster(void)
 
 	if (status != EFI_BUFFER_TOO_SMALL) {
 		if (status != EFI_SUCCESS && status != EFI_NOT_FOUND)
-			efi_err("Failed to locate PCI I/O handles'\n");
+			efi_err("Failed to locate PCI I/O handles\n");
 		return;
 	}
 
@@ -42,7 +42,7 @@ void efi_pci_disable_bridge_busmaster(void)
 	status = efi_bs_call(locate_handle, EFI_LOCATE_BY_PROTOCOL, &pci_proto,
 			     NULL, &pci_handle_size, pci_handle);
 	if (status != EFI_SUCCESS) {
-		efi_err("Failed to locate PCI I/O handles'\n");
+		efi_err("Failed to locate PCI I/O handles\n");
 		goto free_handle;
 	}
 
diff --git a/drivers/firmware/efi/libstub/relocate.c b/drivers/firmware/efi/libstub/relocate.c
index 93c04d..62e2d6 100644
--- a/drivers/firmware/efi/libstub/relocate.c
+++ b/drivers/firmware/efi/libstub/relocate.c
@@ -157,7 +157,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
 					     min_addr);
 	}
 	if (status != EFI_SUCCESS) {
-		efi_err("Failed to allocate usable memory for kernel.\n");
+		efi_err("Failed to allocate usable memory for kernel\n");
 		return status;
 	}
 
diff --git a/drivers/firmware/efi/libstub/secureboot.c b/drivers/firmware/efi/libstub/secureboot.c
index 5efc524..796a31 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -67,10 +67,10 @@ enum efi_secureboot_mode efi_get_secureboot(void)
 		return efi_secureboot_mode_disabled;
 
 secure_boot_enabled:
-	efi_info("UEFI Secure Boot is enabled.\n");
+	efi_info("UEFI Secure Boot is enabled\n");
 	return efi_secureboot_mode_enabled;
 
 out_efi_err:
-	efi_err("Could not determine UEFI Secure Boot status.\n");
+	efi_err("Could not determine UEFI Secure Boot status\n");
 	return efi_secureboot_mode_unknown;
 }
-- 
2.26.0

[PATCH 2/2] efi/libstub: Correct comment typos

[Joe Perches]

Fix a couple typos in comments.

Signed-off-by: Joe Perches <joe@perches.com>
---

Perhaps these trivialities on top of this series?

drivers/firmware/efi/libstub/pci.c      | 2 +-
 drivers/firmware/efi/libstub/relocate.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/libstub/pci.c b/drivers/firmware/efi/libstub/pci.c
index 111c44b..17a53d8 100644
--- a/drivers/firmware/efi/libstub/pci.c
+++ b/drivers/firmware/efi/libstub/pci.c
@@ -69,7 +69,7 @@ void efi_pci_disable_bridge_busmaster(void)
 		 * access to the framebuffer. Drivers for true PCIe graphics
 		 * controllers that are behind a PCIe root port do not use
 		 * DMA to implement the GOP framebuffer anyway [although they
-		 * may use it in their implentation of Gop->Blt()], and so
+		 * may use it in their implementation of Gop->Blt()], and so
 		 * disabling DMA in the PCI bridge should not interfere with
 		 * normal operation of the device.
 		 */
diff --git a/drivers/firmware/efi/libstub/relocate.c b/drivers/firmware/efi/libstub/relocate.c
index 62e2d6..a7ad26 100644
--- a/drivers/firmware/efi/libstub/relocate.c
+++ b/drivers/firmware/efi/libstub/relocate.c
@@ -140,7 +140,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
 	 * The EFI firmware loader could have placed the kernel image
 	 * anywhere in memory, but the kernel has restrictions on the
 	 * max physical address it can run at.  Some architectures
-	 * also have a prefered address, so first try to relocate
+	 * also have a preferred address, so first try to relocate
 	 * to the preferred address.  If that fails, allocate as low
 	 * as possible while respecting the required alignment.
 	 */
-- 
2.26.0

Re: [PATCH 2/2] efi/libstub: Correct comment typos

[Ard Biesheuvel]

On Thu, 30 Apr 2020 at 21:12, Joe Perches <joe@perches.com> wrote:
>
> Fix a couple typos in comments.
>
> Signed-off-by: Joe Perches <joe@perches.com>

Thanks, I'll queue this one up

> ---
>
> Perhaps these trivialities on top of this series?
>
> drivers/firmware/efi/libstub/pci.c      | 2 +-
>  drivers/firmware/efi/libstub/relocate.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/pci.c b/drivers/firmware/efi/libstub/pci.c
> index 111c44b..17a53d8 100644
> --- a/drivers/firmware/efi/libstub/pci.c
> +++ b/drivers/firmware/efi/libstub/pci.c
> @@ -69,7 +69,7 @@ void efi_pci_disable_bridge_busmaster(void)
>                  * access to the framebuffer. Drivers for true PCIe graphics
>                  * controllers that are behind a PCIe root port do not use
>                  * DMA to implement the GOP framebuffer anyway [although they
> -                * may use it in their implentation of Gop->Blt()], and so
> +                * may use it in their implementation of Gop->Blt()], and so
>                  * disabling DMA in the PCI bridge should not interfere with
>                  * normal operation of the device.
>                  */
> diff --git a/drivers/firmware/efi/libstub/relocate.c b/drivers/firmware/efi/libstub/relocate.c
> index 62e2d6..a7ad26 100644
> --- a/drivers/firmware/efi/libstub/relocate.c
> +++ b/drivers/firmware/efi/libstub/relocate.c
> @@ -140,7 +140,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
>          * The EFI firmware loader could have placed the kernel image
>          * anywhere in memory, but the kernel has restrictions on the
>          * max physical address it can run at.  Some architectures
> -        * also have a prefered address, so first try to relocate
> +        * also have a preferred address, so first try to relocate
>          * to the preferred address.  If that fails, allocate as low
>          * as possible while respecting the required alignment.
>          */
> --
> 2.26.0
>

[trivial PATCH] efi/libstub: Reduce efi_printk object size

[Joe Perches]

Use a few more common kernel styles.

Trivially reduce efi_printk object size by using a dereference to
a temporary instead of multiple dereferences of the same object.

Use efi_printk(const char *str) and static or static const for its
internal variables.

Use the more common form of while instead of a for loop.

Change efi_char16_printk argument to const.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 16 ++++++++--------
 drivers/firmware/efi/libstub/efistub.h         |  6 +++---
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 1c92ac231f94..dfd72a4360ac 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -26,19 +26,19 @@ bool __pure __efi_soft_reserve_enabled(void)
 	return !efi_nosoftreserve;
 }
 
-void efi_printk(char *str)
+void efi_printk(const char *str)
 {
-	char *s8;
+	char s8;
 
-	for (s8 = str; *s8; s8++) {
-		efi_char16_t ch[2] = { 0 };
+	while ((s8 = *str++)) {
+		static efi_char16_t ch[2] = {0, 0};
 
-		ch[0] = *s8;
-		if (*s8 == '\n') {
-			efi_char16_t nl[2] = { '\r', 0 };
+		if (s8 == '\n') {
+			static const efi_char16_t nl[2] = { '\r', 0 };
 			efi_char16_printk(nl);
 		}
 
+		ch[0] = s8;
 		efi_char16_printk(ch);
 	}
 }
@@ -284,7 +284,7 @@ void *get_efi_config_table(efi_guid_t guid)
 	return NULL;
 }
 
-void efi_char16_printk(efi_char16_t *str)
+void efi_char16_printk(const efi_char16_t *str)
 {
 	efi_call_proto(efi_table_attr(efi_system_table, con_out),
 		       output_string, str);
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 5ff63230a1f1..a03a92c665f0 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -251,7 +251,7 @@ union efi_simple_text_output_protocol {
 	struct {
 		void *reset;
 		efi_status_t (__efiapi *output_string)(efi_simple_text_output_protocol_t *,
-						       efi_char16_t *);
+						       const efi_char16_t *);
 		void *test_string;
 	};
 	struct {
@@ -599,7 +599,7 @@ efi_status_t efi_exit_boot_services(void *handle,
 				    void *priv,
 				    efi_exit_boot_map_processing priv_func);
 
-void efi_char16_printk(efi_char16_t *);
+void efi_char16_printk(const efi_char16_t *str);
 
 efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
 					    unsigned long *new_fdt_addr,
@@ -624,7 +624,7 @@ efi_status_t check_platform_features(void);
 
 void *get_efi_config_table(efi_guid_t guid);
 
-void efi_printk(char *str);
+void efi_printk(const char *str);
 
 void efi_free(unsigned long size, unsigned long addr);
 

Re: [trivial PATCH] efi/libstub: Reduce efi_printk object size

[Ard Biesheuvel]

On Mon, 4 May 2020 at 20:29, Joe Perches <joe@perches.com> wrote:
>
> Use a few more common kernel styles.
>
> Trivially reduce efi_printk object size by using a dereference to
> a temporary instead of multiple dereferences of the same object.
>
> Use efi_printk(const char *str) and static or static const for its
> internal variables.
>
> Use the more common form of while instead of a for loop.
>
> Change efi_char16_printk argument to const.
>
> Signed-off-by: Joe Perches <joe@perches.com>

Thanks Joe.


> ---
>  drivers/firmware/efi/libstub/efi-stub-helper.c | 16 ++++++++--------
>  drivers/firmware/efi/libstub/efistub.h         |  6 +++---
>  2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index 1c92ac231f94..dfd72a4360ac 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -26,19 +26,19 @@ bool __pure __efi_soft_reserve_enabled(void)
>         return !efi_nosoftreserve;
>  }
>
> -void efi_printk(char *str)
> +void efi_printk(const char *str)
>  {
> -       char *s8;
> +       char s8;
>
> -       for (s8 = str; *s8; s8++) {
> -               efi_char16_t ch[2] = { 0 };
> +       while ((s8 = *str++)) {

I'm not sure I prefer the assignment-as-truth-value construct over the
original for () tbh

> +               static efi_char16_t ch[2] = {0, 0};
>

UEFI code could potentially be reentrant, so this should not be static.

> -               ch[0] = *s8;
> -               if (*s8 == '\n') {
> -                       efi_char16_t nl[2] = { '\r', 0 };
> +               if (s8 == '\n') {
> +                       static const efi_char16_t nl[2] = { '\r', 0 };
>                         efi_char16_printk(nl);

We cannot make this const, unfortunately (see below). But we can clean
this up by using L"\r" as the initializer.

>                 }
>
> +               ch[0] = s8;
>                 efi_char16_printk(ch);
>         }
>  }
> @@ -284,7 +284,7 @@ void *get_efi_config_table(efi_guid_t guid)
>         return NULL;
>  }
>
> -void efi_char16_printk(efi_char16_t *str)
> +void efi_char16_printk(const efi_char16_t *str)
>  {
>         efi_call_proto(efi_table_attr(efi_system_table, con_out),
>                        output_string, str);
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 5ff63230a1f1..a03a92c665f0 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -251,7 +251,7 @@ union efi_simple_text_output_protocol {
>         struct {
>                 void *reset;
>                 efi_status_t (__efiapi *output_string)(efi_simple_text_output_protocol_t *,
> -                                                      efi_char16_t *);
> +                                                      const efi_char16_t *);

This prototype comes straight from the UEFI specification, and even
though it is dumb that they forgot about const-qualified pointers
entirely, I would prefer not to deviate from this.

>                 void *test_string;
>         };
>         struct {
> @@ -599,7 +599,7 @@ efi_status_t efi_exit_boot_services(void *handle,
>                                     void *priv,
>                                     efi_exit_boot_map_processing priv_func);
>
> -void efi_char16_printk(efi_char16_t *);
> +void efi_char16_printk(const efi_char16_t *str);
>
>  efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
>                                             unsigned long *new_fdt_addr,
> @@ -624,7 +624,7 @@ efi_status_t check_platform_features(void);
>
>  void *get_efi_config_table(efi_guid_t guid);
>
> -void efi_printk(char *str);
> +void efi_printk(const char *str);
>
>  void efi_free(unsigned long size, unsigned long addr);
>
>

Re: [trivial PATCH] efi/libstub: Reduce efi_printk object size

[Joe Perches]

On Tue, 2020-05-05 at 09:50 +0200, Ard Biesheuvel wrote:
> On Mon, 4 May 2020 at 20:29, Joe Perches <joe@perches.com> wrote:
> > Use a few more common kernel styles.
> > 
> > Trivially reduce efi_printk object size by using a dereference to
> > a temporary instead of multiple dereferences of the same object.
> > 
> > Use efi_printk(const char *str) and static or static const for its
> > internal variables.
> > 
> > Use the more common form of while instead of a for loop.
> > 
> > Change efi_char16_printk argument to const.
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> 
> Thanks Joe.

No worries, it's not worth applying if
it's not good code.  Just ignore it.

cheers, Joe

Re: [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening

[Ard Biesheuvel]

On Thu, 30 Apr 2020 at 21:12, Joe Perches <joe@perches.com> wrote:
>
> Use a standard style for these output logging messages.
>
> Miscellanea:
>
> o Use more common macro #defines with fmt, ##__VA_ARGS__
> 0 Remove trailing messages periods and odd ' uses
> o Remove embedded function names and use %s, __func__
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>
> Perhaps these trivialities on top of this series?
>

The EFI printing routines don't actually support format strings.

Removing trailing periods is not really necessary IMO, but i'll take a
patch that fixes those weird quotes.

Thanks,
Ard.



>  drivers/firmware/efi/libstub/arm32-stub.c | 10 +++++-----
>  drivers/firmware/efi/libstub/efi-stub.c   |  2 +-
>  drivers/firmware/efi/libstub/efistub.h    |  9 ++++++---
>  drivers/firmware/efi/libstub/fdt.c        |  8 ++++----
>  drivers/firmware/efi/libstub/pci.c        |  4 ++--
>  drivers/firmware/efi/libstub/relocate.c   |  2 +-
>  drivers/firmware/efi/libstub/secureboot.c |  4 ++--
>  7 files changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c
> index b038afe..5795781 100644
> --- a/drivers/firmware/efi/libstub/arm32-stub.c
> +++ b/drivers/firmware/efi/libstub/arm32-stub.c
> @@ -120,7 +120,7 @@ static efi_status_t reserve_kernel_base(unsigned long dram_base,
>          */
>         status = efi_get_memory_map(&map);
>         if (status != EFI_SUCCESS) {
> -               efi_err("reserve_kernel_base(): Unable to retrieve memory map.\n");
> +               efi_err("%s(): Unable to retrieve memory map\n", __func__);
>                 return status;
>         }
>
> @@ -162,7 +162,7 @@ static efi_status_t reserve_kernel_base(unsigned long dram_base,
>                                              (end - start) / EFI_PAGE_SIZE,
>                                              &start);
>                         if (status != EFI_SUCCESS) {
> -                               efi_err("reserve_kernel_base(): alloc failed.\n");
> +                               efi_err("%s(): alloc failed\n", __func__);
>                                 goto out;
>                         }
>                         break;
> @@ -219,7 +219,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>
>         status = reserve_kernel_base(kernel_base, reserve_addr, reserve_size);
>         if (status != EFI_SUCCESS) {
> -               efi_err("Unable to allocate memory for uncompressed kernel.\n");
> +               efi_err("Unable to allocate memory for uncompressed kernel\n");
>                 return status;
>         }
>
> @@ -232,7 +232,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>         status = efi_relocate_kernel(image_addr, *image_size, *image_size,
>                                      kernel_base + MAX_UNCOMP_KERNEL_SIZE, 0, 0);
>         if (status != EFI_SUCCESS) {
> -               efi_err("Failed to relocate kernel.\n");
> +               efi_err("Failed to relocate kernel\n");
>                 efi_free(*reserve_size, *reserve_addr);
>                 *reserve_size = 0;
>                 return status;
> @@ -244,7 +244,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>          * address at which the zImage is loaded.
>          */
>         if (*image_addr + *image_size > dram_base + ZIMAGE_OFFSET_LIMIT) {
> -               efi_err("Failed to relocate kernel, no low memory available.\n");
> +               efi_err("Failed to relocate kernel, no low memory available\n");
>                 efi_free(*reserve_size, *reserve_addr);
>                 *reserve_size = 0;
>                 efi_free(*image_size, *image_addr);
> diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
> index c2484b..19b42b 100644
> --- a/drivers/firmware/efi/libstub/efi-stub.c
> +++ b/drivers/firmware/efi/libstub/efi-stub.c
> @@ -251,7 +251,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
>         if (!IS_ENABLED(CONFIG_EFI_ARMSTUB_DTB_LOADER) ||
>              secure_boot != efi_secureboot_mode_disabled) {
>                 if (strstr(cmdline_ptr, "dtb="))
> -                       efi_err("Ignoring DTB from command line.\n");
> +                       efi_err("Ignoring DTB from command line\n");
>         } else {
>                 status = efi_load_dtb(image, &fdt_addr, &fdt_size);
>
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 874233..369262 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -49,11 +49,14 @@ extern const efi_system_table_t *efi_system_table;
>  #define efi_call_proto(inst, func, ...) inst->func(inst, ##__VA_ARGS__)
>  #endif
>
> -#define efi_info(msg)          do {                    \
> -       if (!efi_quiet) efi_printk("EFI stub: "msg);    \
> +#define efi_info(fmt, ...)                                             \
> +do {                                                                   \
> +       if (!efi_quiet)                                                 \
> +               efi_printk("EFI stub: " fmt, ##__VA_ARGS__);            \
>  } while (0)
>
> -#define efi_err(msg) efi_printk("EFI stub: ERROR: "msg)
> +#define efi_err(fmt, ...)                                              \
> +       efi_printk("EFI stub: ERROR: " fmt, ##__VA_ARGS__)
>
>  /* Helper macros for the usual case of using simple C variables: */
>  #ifndef fdt_setprop_inplace_var
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index 11ecf3c..7c7257 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -270,7 +270,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
>          */
>         status = efi_get_memory_map(&map);
>         if (status != EFI_SUCCESS) {
> -               efi_err("Unable to retrieve UEFI memory map.\n");
> +               efi_err("Unable to retrieve UEFI memory map\n");
>                 return status;
>         }
>
> @@ -279,7 +279,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
>         map.map = &memory_map;
>         status = efi_allocate_pages(MAX_FDT_SIZE, new_fdt_addr, max_addr);
>         if (status != EFI_SUCCESS) {
> -               efi_err("Unable to allocate memory for new device tree.\n");
> +               efi_err("Unable to allocate memory for new device tree\n");
>                 goto fail;
>         }
>
> @@ -296,7 +296,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
>                             initrd_addr, initrd_size);
>
>         if (status != EFI_SUCCESS) {
> -               efi_err("Unable to construct new device tree.\n");
> +               efi_err("Unable to construct new device tree\n");
>                 goto fail_free_new_fdt;
>         }
>
> @@ -342,7 +342,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(void *handle,
>                 return EFI_SUCCESS;
>         }
>
> -       efi_err("Exit boot services failed.\n");
> +       efi_err("Exit boot services failed\n");
>
>  fail_free_new_fdt:
>         efi_free(MAX_FDT_SIZE, *new_fdt_addr);
> diff --git a/drivers/firmware/efi/libstub/pci.c b/drivers/firmware/efi/libstub/pci.c
> index 60af51b..111c44b 100644
> --- a/drivers/firmware/efi/libstub/pci.c
> +++ b/drivers/firmware/efi/libstub/pci.c
> @@ -28,7 +28,7 @@ void efi_pci_disable_bridge_busmaster(void)
>
>         if (status != EFI_BUFFER_TOO_SMALL) {
>                 if (status != EFI_SUCCESS && status != EFI_NOT_FOUND)
> -                       efi_err("Failed to locate PCI I/O handles'\n");
> +                       efi_err("Failed to locate PCI I/O handles\n");
>                 return;
>         }
>
> @@ -42,7 +42,7 @@ void efi_pci_disable_bridge_busmaster(void)
>         status = efi_bs_call(locate_handle, EFI_LOCATE_BY_PROTOCOL, &pci_proto,
>                              NULL, &pci_handle_size, pci_handle);
>         if (status != EFI_SUCCESS) {
> -               efi_err("Failed to locate PCI I/O handles'\n");
> +               efi_err("Failed to locate PCI I/O handles\n");
>                 goto free_handle;
>         }
>
> diff --git a/drivers/firmware/efi/libstub/relocate.c b/drivers/firmware/efi/libstub/relocate.c
> index 93c04d..62e2d6 100644
> --- a/drivers/firmware/efi/libstub/relocate.c
> +++ b/drivers/firmware/efi/libstub/relocate.c
> @@ -157,7 +157,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
>                                              min_addr);
>         }
>         if (status != EFI_SUCCESS) {
> -               efi_err("Failed to allocate usable memory for kernel.\n");
> +               efi_err("Failed to allocate usable memory for kernel\n");
>                 return status;
>         }
>
> diff --git a/drivers/firmware/efi/libstub/secureboot.c b/drivers/firmware/efi/libstub/secureboot.c
> index 5efc524..796a31 100644
> --- a/drivers/firmware/efi/libstub/secureboot.c
> +++ b/drivers/firmware/efi/libstub/secureboot.c
> @@ -67,10 +67,10 @@ enum efi_secureboot_mode efi_get_secureboot(void)
>                 return efi_secureboot_mode_disabled;
>
>  secure_boot_enabled:
> -       efi_info("UEFI Secure Boot is enabled.\n");
> +       efi_info("UEFI Secure Boot is enabled\n");
>         return efi_secureboot_mode_enabled;
>
>  out_efi_err:
> -       efi_err("Could not determine UEFI Secure Boot status.\n");
> +       efi_err("Could not determine UEFI Secure Boot status\n");
>         return efi_secureboot_mode_unknown;
>  }
> --
> 2.26.0
>

Re: [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening

[Joe Perches]

On Thu, 2020-04-30 at 21:29 +0200, Ard Biesheuvel wrote:
> On Thu, 30 Apr 2020 at 21:12, Joe Perches <joe@perches.com> wrote:
> > Use a standard style for these output logging messages.
> > 
> > Miscellanea:
> > 
> > o Use more common macro #defines with fmt, ##__VA_ARGS__
> > 0 Remove trailing messages periods and odd ' uses
> > o Remove embedded function names and use %s, __func__
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> > ---
> > 
> > Perhaps these trivialities on top of this series?
> > 
> 
> The EFI printing routines don't actually support format strings.

Thanks.  Then nevermind on that bit.

> Removing trailing periods is not really necessary IMO, but i'll take a
> patch that fixes those weird quotes.

<shrug>  Your choice.

There are 60+ uses of efi_err, 12 with a trailing period.

cheers, Joe

Re: [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening

[Arvind Sankar]

On Thu, Apr 30, 2020 at 09:29:46PM +0200, Ard Biesheuvel wrote:
> On Thu, 30 Apr 2020 at 21:12, Joe Perches <joe@perches.com> wrote:
> >
> > Use a standard style for these output logging messages.
> >
> > Miscellanea:
> >
> > o Use more common macro #defines with fmt, ##__VA_ARGS__
> > 0 Remove trailing messages periods and odd ' uses
> > o Remove embedded function names and use %s, __func__
> >
> > Signed-off-by: Joe Perches <joe@perches.com>
> > ---
> >
> > Perhaps these trivialities on top of this series?
> >
> 
> The EFI printing routines don't actually support format strings.
> 

The x86 real-mode bootup code actually has a printf.o that clocks in at
under 2k. We could add it in, and it would also be nice to move it into
lib or something, since at least alpha and powerpc implement something
very similar for boot-time messages.

Re: [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening

[Ard Biesheuvel]

On Thu, 30 Apr 2020 at 22:40, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Thu, Apr 30, 2020 at 09:29:46PM +0200, Ard Biesheuvel wrote:
> > On Thu, 30 Apr 2020 at 21:12, Joe Perches <joe@perches.com> wrote:
> > >
> > > Use a standard style for these output logging messages.
> > >
> > > Miscellanea:
> > >
> > > o Use more common macro #defines with fmt, ##__VA_ARGS__
> > > 0 Remove trailing messages periods and odd ' uses
> > > o Remove embedded function names and use %s, __func__
> > >
> > > Signed-off-by: Joe Perches <joe@perches.com>
> > > ---
> > >
> > > Perhaps these trivialities on top of this series?
> > >
> >
> > The EFI printing routines don't actually support format strings.
> >
>
> The x86 real-mode bootup code actually has a printf.o that clocks in at
> under 2k. We could add it in, and it would also be nice to move it into
> lib or something, since at least alpha and powerpc implement something
> very similar for boot-time messages.

Not being able to use format strings is really quite annoying, and I
did look into reusing the ordinary one (which is hairy), not realizing
that there is already a cut down version available.

So yes, that does sound like a great idea!

Re: [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next

[Ard Biesheuvel]

On Thu, 30 Apr 2020 at 20:28, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> This series is on top of efi/next.
>
> Patch 1 fixes the size allocated for x86 boot_params.
> Patch 2 refactors the setting of various hi/lo 32-bit fields, mainly on x86.
> Patch 3 renames pr_efi/pr_efi_err
> Patches 4-6 convert the remaining uses of efi_printk to print error
> messages to use efi_err instead.
> Patch 7 updates dtb= ignored message to efi_err.
> Patches 8-9 refactor initrd loading, moving it into efi-stub-helper.
> Patch 10 adds support for x86 builtin command line.
> Patch 11 adds error checking for efi_parse_options.
>
> Changes from v1:
> - Rename pr_efi/pr_efi_err
> - Drop the soft_limit-removing patch
> - Fix a couple of compile warnings
>
> Arvind Sankar (11):
>   efi/x86: Use correct size for boot_params
>   efi/libstub: Add a helper function to split 64-bit values
>   efi/libstub: Move pr_efi/pr_efi_err into efi namespace
>   efi/x86: Use efi_err for error messages
>   efi/gop: Use efi_err for error messages
>   efi/tpm: Use efi_err for error messages
>   efi/libstub: Upgrade ignored dtb= argument message to error
>   efi/x86: Move command-line initrd loading to efi_main
>   efi/libstub: Unify initrd loading across architectures
>   efi/x86: Support builtin command line
>   efi/libstub: Check return value of efi_parse_options
>

Thanks Arvind, I've queued these up now


>  drivers/firmware/efi/libstub/arm32-stub.c     |  12 +-
>  drivers/firmware/efi/libstub/arm64-stub.c     |  14 +-
>  .../firmware/efi/libstub/efi-stub-helper.c    |  46 ++++++-
>  drivers/firmware/efi/libstub/efi-stub.c       |  63 ++++-----
>  drivers/firmware/efi/libstub/efistub.h        |  32 ++---
>  drivers/firmware/efi/libstub/fdt.c            |  16 +--
>  drivers/firmware/efi/libstub/file.c           |  12 +-
>  drivers/firmware/efi/libstub/gop.c            |  16 +--
>  drivers/firmware/efi/libstub/pci.c            |   8 +-
>  drivers/firmware/efi/libstub/relocate.c       |   2 +-
>  drivers/firmware/efi/libstub/secureboot.c     |   4 +-
>  drivers/firmware/efi/libstub/tpm.c            |   2 +-
>  drivers/firmware/efi/libstub/x86-stub.c       | 122 ++++++++----------
>  13 files changed, 186 insertions(+), 163 deletions(-)
>
> --
> 2.26.2
>