* [PATCH 00/10] efi: some cleanups/refactoring for efi/next @ 2020-04-29 17:41 Arvind Sankar 2020-04-29 17:41 ` [PATCH 01/10] efi/x86: Use correct size for boot_params Arvind Sankar ` (11 more replies) 0 siblings, 12 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 01/10] efi/x86: Use correct size for boot_params 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar @ 2020-04-29 17:41 ` Arvind Sankar 2020-04-29 17:41 ` [PATCH 02/10] efi: Add a helper function to split 64-bit values Arvind Sankar ` (10 subsequent siblings) 11 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 02/10] efi: Add a helper function to split 64-bit values 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar 2020-04-29 17:41 ` [PATCH 01/10] efi/x86: Use correct size for boot_params Arvind Sankar @ 2020-04-29 17:41 ` Arvind Sankar 2020-04-29 23:51 ` kbuild test robot 2020-04-29 17:41 ` [PATCH 02/10] efi/libstub: " Arvind Sankar ` (9 subsequent siblings) 11 siblings, 1 reply; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH 02/10] efi: Add a helper function to split 64-bit values 2020-04-29 17:41 ` [PATCH 02/10] efi: Add a helper function to split 64-bit values Arvind Sankar @ 2020-04-29 23:51 ` kbuild test robot 0 siblings, 0 replies; 57+ messages in thread From: kbuild test robot @ 2020-04-29 23:51 UTC (permalink / raw) To: Arvind Sankar, Ard Biesheuvel; +Cc: kbuild-all, linux-efi, x86, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 02/10] efi/libstub: Add a helper function to split 64-bit values 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar 2020-04-29 17:41 ` [PATCH 01/10] efi/x86: Use correct size for boot_params Arvind Sankar 2020-04-29 17:41 ` [PATCH 02/10] efi: Add a helper function to split 64-bit values Arvind Sankar @ 2020-04-29 17:41 ` Arvind Sankar 2020-04-29 17:41 ` [PATCH 03/10] efi/x86: Use pr_efi_err for error messages Arvind Sankar ` (8 subsequent siblings) 11 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (2 preceding siblings ...) 2020-04-29 17:41 ` [PATCH 02/10] efi/libstub: " Arvind Sankar @ 2020-04-29 17:41 ` Arvind Sankar 2020-04-29 18:47 ` Joe Perches 2020-04-29 17:41 ` [PATCH 04/10] efi/gop: " Arvind Sankar ` (7 subsequent siblings) 11 siblings, 1 reply; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 17:41 ` [PATCH 03/10] efi/x86: Use pr_efi_err for error messages Arvind Sankar @ 2020-04-29 18:47 ` Joe Perches 2020-04-29 18:49 ` Ard Biesheuvel 0 siblings, 1 reply; 57+ messages in thread From: Joe Perches @ 2020-04-29 18:47 UTC (permalink / raw) To: Arvind Sankar, Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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); > } ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 18:47 ` Joe Perches @ 2020-04-29 18:49 ` Ard Biesheuvel 2020-04-29 18:57 ` Joe Perches 2020-04-29 21:43 ` Arvind Sankar 0 siblings, 2 replies; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-29 18:49 UTC (permalink / raw) To: Joe Perches; +Cc: Arvind Sankar, linux-efi, X86 ML, Linux Kernel Mailing List 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. ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 18:49 ` Ard Biesheuvel @ 2020-04-29 18:57 ` Joe Perches 2020-04-29 18:59 ` Ard Biesheuvel 2020-04-29 21:43 ` Arvind Sankar 1 sibling, 1 reply; 57+ messages in thread From: Joe Perches @ 2020-04-29 18:57 UTC (permalink / raw) To: Ard Biesheuvel Cc: Arvind Sankar, linux-efi, X86 ML, Linux Kernel Mailing List 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? ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 18:57 ` Joe Perches @ 2020-04-29 18:59 ` Ard Biesheuvel 2020-04-29 19:47 ` Joe Perches 0 siblings, 1 reply; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-29 18:59 UTC (permalink / raw) To: Joe Perches; +Cc: Arvind Sankar, linux-efi, X86 ML, Linux Kernel Mailing List 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? ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 18:59 ` Ard Biesheuvel @ 2020-04-29 19:47 ` Joe Perches 2020-04-29 19:48 ` Ard Biesheuvel 0 siblings, 1 reply; 57+ messages in thread From: Joe Perches @ 2020-04-29 19:47 UTC (permalink / raw) To: Ard Biesheuvel Cc: Arvind Sankar, linux-efi, X86 ML, Linux Kernel Mailing List 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. ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 19:47 ` Joe Perches @ 2020-04-29 19:48 ` Ard Biesheuvel 0 siblings, 0 replies; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-29 19:48 UTC (permalink / raw) To: Joe Perches; +Cc: Arvind Sankar, linux-efi, X86 ML, Linux Kernel Mailing List 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. ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 18:49 ` Ard Biesheuvel 2020-04-29 18:57 ` Joe Perches @ 2020-04-29 21:43 ` Arvind Sankar 2020-04-29 21:45 ` Ard Biesheuvel 2020-04-29 21:53 ` Joe Perches 1 sibling, 2 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 21:43 UTC (permalink / raw) To: Ard Biesheuvel Cc: Joe Perches, Arvind Sankar, linux-efi, X86 ML, Linux Kernel Mailing List 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? ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 21:43 ` Arvind Sankar @ 2020-04-29 21:45 ` Ard Biesheuvel 2020-04-29 21:51 ` Arvind Sankar 2020-04-29 21:53 ` Joe Perches 1 sibling, 1 reply; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-29 21:45 UTC (permalink / raw) To: Arvind Sankar; +Cc: Joe Perches, linux-efi, X86 ML, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 21:45 ` Ard Biesheuvel @ 2020-04-29 21:51 ` Arvind Sankar 0 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 21:51 UTC (permalink / raw) To: Ard Biesheuvel Cc: Arvind Sankar, Joe Perches, linux-efi, X86 ML, Linux Kernel Mailing List 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. ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 21:43 ` Arvind Sankar 2020-04-29 21:45 ` Ard Biesheuvel @ 2020-04-29 21:53 ` Joe Perches 2020-04-29 21:55 ` Ard Biesheuvel 1 sibling, 1 reply; 57+ messages in thread From: Joe Perches @ 2020-04-29 21:53 UTC (permalink / raw) To: Arvind Sankar, Ard Biesheuvel Cc: linux-efi, X86 ML, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 21:53 ` Joe Perches @ 2020-04-29 21:55 ` Ard Biesheuvel 2020-04-29 22:20 ` Arvind Sankar 0 siblings, 1 reply; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-29 21:55 UTC (permalink / raw) To: Joe Perches; +Cc: Arvind Sankar, linux-efi, X86 ML, Linux Kernel Mailing List 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.. ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 21:55 ` Ard Biesheuvel @ 2020-04-29 22:20 ` Arvind Sankar 2020-04-30 15:14 ` Ard Biesheuvel 0 siblings, 1 reply; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 22:20 UTC (permalink / raw) To: Ard Biesheuvel Cc: Joe Perches, Arvind Sankar, linux-efi, X86 ML, Linux Kernel Mailing List 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"); ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/10] efi/x86: Use pr_efi_err for error messages 2020-04-29 22:20 ` Arvind Sankar @ 2020-04-30 15:14 ` Ard Biesheuvel 0 siblings, 0 replies; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-30 15:14 UTC (permalink / raw) To: Arvind Sankar; +Cc: Joe Perches, linux-efi, X86 ML, Linux Kernel Mailing List 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. ^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 04/10] efi/gop: Use pr_efi_err for error messages 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (3 preceding siblings ...) 2020-04-29 17:41 ` [PATCH 03/10] efi/x86: Use pr_efi_err for error messages Arvind Sankar @ 2020-04-29 17:41 ` Arvind Sankar 2020-04-29 17:41 ` [PATCH 05/10] efi/tpm: " Arvind Sankar ` (6 subsequent siblings) 11 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 05/10] efi/tpm: Use pr_efi_err for error messages 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (4 preceding siblings ...) 2020-04-29 17:41 ` [PATCH 04/10] efi/gop: " Arvind Sankar @ 2020-04-29 17:41 ` Arvind Sankar 2020-04-29 17:41 ` [PATCH 06/10] efi/x86: Move command-line initrd loading to efi_main Arvind Sankar ` (5 subsequent siblings) 11 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 06/10] efi/x86: Move command-line initrd loading to efi_main 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (5 preceding siblings ...) 2020-04-29 17:41 ` [PATCH 05/10] efi/tpm: " Arvind Sankar @ 2020-04-29 17:41 ` Arvind Sankar 2020-04-29 17:41 ` [PATCH 07/10] efi/libstub: Unify initrd loading across architectures Arvind Sankar ` (4 subsequent siblings) 11 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 07/10] efi/libstub: Unify initrd loading across architectures 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (6 preceding siblings ...) 2020-04-29 17:41 ` [PATCH 06/10] efi/x86: Move command-line initrd loading to efi_main Arvind Sankar @ 2020-04-29 17:41 ` Arvind Sankar 2020-04-29 17:41 ` [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading Arvind Sankar ` (3 subsequent siblings) 11 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (7 preceding siblings ...) 2020-04-29 17:41 ` [PATCH 07/10] efi/libstub: Unify initrd loading across architectures Arvind Sankar @ 2020-04-29 17:41 ` Arvind Sankar 2020-04-29 19:05 ` Ard Biesheuvel 2020-04-29 17:41 ` [PATCH 09/10] efi/x86: Support builtin command line Arvind Sankar ` (2 subsequent siblings) 11 siblings, 1 reply; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading 2020-04-29 17:41 ` [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading Arvind Sankar @ 2020-04-29 19:05 ` Ard Biesheuvel 2020-04-29 21:33 ` Arvind Sankar 0 siblings, 1 reply; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-29 19:05 UTC (permalink / raw) To: Arvind Sankar; +Cc: linux-efi, X86 ML, Linux Kernel Mailing List 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 > ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading 2020-04-29 19:05 ` Ard Biesheuvel @ 2020-04-29 21:33 ` Arvind Sankar 0 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 21:33 UTC (permalink / raw) To: Ard Biesheuvel Cc: Arvind Sankar, linux-efi, X86 ML, Linux Kernel Mailing List 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. ^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 09/10] efi/x86: Support builtin command line 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (8 preceding siblings ...) 2020-04-29 17:41 ` [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading Arvind Sankar @ 2020-04-29 17:41 ` Arvind Sankar 2020-04-29 19:07 ` Ard Biesheuvel 2020-04-29 17:41 ` [PATCH 10/10] efi/libstub: Check return value of efi_parse_options Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar 11 siblings, 1 reply; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH 09/10] efi/x86: Support builtin command line 2020-04-29 17:41 ` [PATCH 09/10] efi/x86: Support builtin command line Arvind Sankar @ 2020-04-29 19:07 ` Ard Biesheuvel 2020-04-29 21:39 ` Arvind Sankar 0 siblings, 1 reply; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-29 19:07 UTC (permalink / raw) To: Arvind Sankar; +Cc: linux-efi, X86 ML, Linux Kernel Mailing List 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 > ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 09/10] efi/x86: Support builtin command line 2020-04-29 19:07 ` Ard Biesheuvel @ 2020-04-29 21:39 ` Arvind Sankar 2020-04-29 21:40 ` Ard Biesheuvel 0 siblings, 1 reply; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 21:39 UTC (permalink / raw) To: Ard Biesheuvel Cc: Arvind Sankar, linux-efi, X86 ML, Linux Kernel Mailing List 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 > > ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 09/10] efi/x86: Support builtin command line 2020-04-29 21:39 ` Arvind Sankar @ 2020-04-29 21:40 ` Ard Biesheuvel 2020-04-29 21:48 ` Arvind Sankar 0 siblings, 1 reply; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-29 21:40 UTC (permalink / raw) To: Arvind Sankar; +Cc: linux-efi, X86 ML, Linux Kernel Mailing List 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 ""); ? ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 09/10] efi/x86: Support builtin command line 2020-04-29 21:40 ` Ard Biesheuvel @ 2020-04-29 21:48 ` Arvind Sankar 2020-04-29 21:51 ` Ard Biesheuvel 0 siblings, 1 reply; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 21:48 UTC (permalink / raw) To: Ard Biesheuvel Cc: Arvind Sankar, linux-efi, X86 ML, Linux Kernel Mailing List 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. ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 09/10] efi/x86: Support builtin command line 2020-04-29 21:48 ` Arvind Sankar @ 2020-04-29 21:51 ` Ard Biesheuvel 0 siblings, 0 replies; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-29 21:51 UTC (permalink / raw) To: Arvind Sankar; +Cc: linux-efi, X86 ML, Linux Kernel Mailing List 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. ^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 10/10] efi/libstub: Check return value of efi_parse_options 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (9 preceding siblings ...) 2020-04-29 17:41 ` [PATCH 09/10] efi/x86: Support builtin command line Arvind Sankar @ 2020-04-29 17:41 ` Arvind Sankar 2020-04-30 4:31 ` kbuild test robot 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar 11 siblings, 1 reply; 57+ messages in thread From: Arvind Sankar @ 2020-04-29 17:41 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH 10/10] efi/libstub: Check return value of efi_parse_options 2020-04-29 17:41 ` [PATCH 10/10] efi/libstub: Check return value of efi_parse_options Arvind Sankar @ 2020-04-30 4:31 ` kbuild test robot 0 siblings, 0 replies; 57+ messages in thread From: kbuild test robot @ 2020-04-30 4:31 UTC (permalink / raw) To: Arvind Sankar, Ard Biesheuvel Cc: kbuild-all, clang-built-linux, linux-efi, x86, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (10 preceding siblings ...) 2020-04-29 17:41 ` [PATCH 10/10] efi/libstub: Check return value of efi_parse_options Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 01/11] efi/x86: Use correct size for boot_params Arvind Sankar ` (12 more replies) 11 siblings, 13 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v2 01/11] efi/x86: Use correct size for boot_params 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 02/11] efi/libstub: Add a helper function to split 64-bit values Arvind Sankar ` (11 subsequent siblings) 12 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v2 02/11] efi/libstub: Add a helper function to split 64-bit values 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 01/11] efi/x86: Use correct size for boot_params Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 03/11] efi/libstub: Move pr_efi/pr_efi_err into efi namespace Arvind Sankar ` (10 subsequent siblings) 12 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v2 03/11] efi/libstub: Move pr_efi/pr_efi_err into efi namespace 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 01/11] efi/x86: Use correct size for boot_params Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 02/11] efi/libstub: Add a helper function to split 64-bit values Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 04/11] efi/x86: Use efi_err for error messages Arvind Sankar ` (9 subsequent siblings) 12 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel, Joe Perches 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v2 04/11] efi/x86: Use efi_err for error messages 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (2 preceding siblings ...) 2020-04-30 18:28 ` [PATCH v2 03/11] efi/libstub: Move pr_efi/pr_efi_err into efi namespace Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 05/11] efi/gop: " Arvind Sankar ` (8 subsequent siblings) 12 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v2 05/11] efi/gop: Use efi_err for error messages 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (3 preceding siblings ...) 2020-04-30 18:28 ` [PATCH v2 04/11] efi/x86: Use efi_err for error messages Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 06/11] efi/tpm: " Arvind Sankar ` (7 subsequent siblings) 12 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v2 06/11] efi/tpm: Use efi_err for error messages 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (4 preceding siblings ...) 2020-04-30 18:28 ` [PATCH v2 05/11] efi/gop: " Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 07/11] efi/libstub: Upgrade ignored dtb= argument message to error Arvind Sankar ` (6 subsequent siblings) 12 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v2 07/11] efi/libstub: Upgrade ignored dtb= argument message to error 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (5 preceding siblings ...) 2020-04-30 18:28 ` [PATCH v2 06/11] efi/tpm: " Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 08/11] efi/x86: Move command-line initrd loading to efi_main Arvind Sankar ` (5 subsequent siblings) 12 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v2 08/11] efi/x86: Move command-line initrd loading to efi_main 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (6 preceding siblings ...) 2020-04-30 18:28 ` [PATCH v2 07/11] efi/libstub: Upgrade ignored dtb= argument message to error Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 09/11] efi/libstub: Unify initrd loading across architectures Arvind Sankar ` (4 subsequent siblings) 12 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v2 09/11] efi/libstub: Unify initrd loading across architectures 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (7 preceding siblings ...) 2020-04-30 18:28 ` [PATCH v2 08/11] efi/x86: Move command-line initrd loading to efi_main Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 10/11] efi/x86: Support builtin command line Arvind Sankar ` (3 subsequent siblings) 12 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v2 10/11] efi/x86: Support builtin command line 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (8 preceding siblings ...) 2020-04-30 18:28 ` [PATCH v2 09/11] efi/libstub: Unify initrd loading across architectures Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 11/11] efi/libstub: Check return value of efi_parse_options Arvind Sankar ` (2 subsequent siblings) 12 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v2 11/11] efi/libstub: Check return value of efi_parse_options 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (9 preceding siblings ...) 2020-04-30 18:28 ` [PATCH v2 10/11] efi/x86: Support builtin command line Arvind Sankar @ 2020-04-30 18:28 ` Arvind Sankar 2020-04-30 19:12 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Joe Perches 2020-05-04 8:17 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Ard Biesheuvel 12 siblings, 0 replies; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 18:28 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86, linux-kernel 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (10 preceding siblings ...) 2020-04-30 18:28 ` [PATCH v2 11/11] efi/libstub: Check return value of efi_parse_options Arvind Sankar @ 2020-04-30 19:12 ` Joe Perches 2020-04-30 19:12 ` [PATCH 2/2] efi/libstub: Correct comment typos Joe Perches 2020-04-30 19:29 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Ard Biesheuvel 2020-05-04 8:17 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Ard Biesheuvel 12 siblings, 2 replies; 57+ messages in thread From: Joe Perches @ 2020-04-30 19:12 UTC (permalink / raw) To: Arvind Sankar, linux-kernel; +Cc: Ard Biesheuvel, linux-efi, x86 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 2/2] efi/libstub: Correct comment typos 2020-04-30 19:12 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Joe Perches @ 2020-04-30 19:12 ` Joe Perches 2020-04-30 19:30 ` Ard Biesheuvel 2020-04-30 19:29 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Ard Biesheuvel 1 sibling, 1 reply; 57+ messages in thread From: Joe Perches @ 2020-04-30 19:12 UTC (permalink / raw) To: Arvind Sankar, linux-kernel; +Cc: Ard Biesheuvel, linux-efi, x86 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH 2/2] efi/libstub: Correct comment typos 2020-04-30 19:12 ` [PATCH 2/2] efi/libstub: Correct comment typos Joe Perches @ 2020-04-30 19:30 ` Ard Biesheuvel 2020-05-04 18:29 ` [trivial PATCH] efi/libstub: Reduce efi_printk object size Joe Perches 0 siblings, 1 reply; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-30 19:30 UTC (permalink / raw) To: Joe Perches; +Cc: Arvind Sankar, Linux Kernel Mailing List, linux-efi, X86 ML 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 > ^ permalink raw reply [flat|nested] 57+ messages in thread
* [trivial PATCH] efi/libstub: Reduce efi_printk object size 2020-04-30 19:30 ` Ard Biesheuvel @ 2020-05-04 18:29 ` Joe Perches 2020-05-05 7:50 ` Ard Biesheuvel 0 siblings, 1 reply; 57+ messages in thread From: Joe Perches @ 2020-05-04 18:29 UTC (permalink / raw) To: Ard Biesheuvel Cc: Arvind Sankar, Linux Kernel Mailing List, linux-efi, X86 ML 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); ^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [trivial PATCH] efi/libstub: Reduce efi_printk object size 2020-05-04 18:29 ` [trivial PATCH] efi/libstub: Reduce efi_printk object size Joe Perches @ 2020-05-05 7:50 ` Ard Biesheuvel 2020-05-05 8:01 ` Joe Perches 0 siblings, 1 reply; 57+ messages in thread From: Ard Biesheuvel @ 2020-05-05 7:50 UTC (permalink / raw) To: Joe Perches; +Cc: Arvind Sankar, Linux Kernel Mailing List, linux-efi, X86 ML 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); > > ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [trivial PATCH] efi/libstub: Reduce efi_printk object size 2020-05-05 7:50 ` Ard Biesheuvel @ 2020-05-05 8:01 ` Joe Perches 0 siblings, 0 replies; 57+ messages in thread From: Joe Perches @ 2020-05-05 8:01 UTC (permalink / raw) To: Ard Biesheuvel Cc: Arvind Sankar, Linux Kernel Mailing List, linux-efi, X86 ML 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 ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening 2020-04-30 19:12 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Joe Perches 2020-04-30 19:12 ` [PATCH 2/2] efi/libstub: Correct comment typos Joe Perches @ 2020-04-30 19:29 ` Ard Biesheuvel 2020-04-30 19:38 ` Joe Perches 2020-04-30 20:40 ` Arvind Sankar 1 sibling, 2 replies; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-30 19:29 UTC (permalink / raw) To: Joe Perches; +Cc: Arvind Sankar, Linux Kernel Mailing List, linux-efi, X86 ML 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 > ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening 2020-04-30 19:29 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Ard Biesheuvel @ 2020-04-30 19:38 ` Joe Perches 2020-04-30 20:40 ` Arvind Sankar 1 sibling, 0 replies; 57+ messages in thread From: Joe Perches @ 2020-04-30 19:38 UTC (permalink / raw) To: Ard Biesheuvel Cc: Arvind Sankar, Linux Kernel Mailing List, linux-efi, X86 ML 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 ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening 2020-04-30 19:29 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Ard Biesheuvel 2020-04-30 19:38 ` Joe Perches @ 2020-04-30 20:40 ` Arvind Sankar 2020-04-30 20:42 ` Ard Biesheuvel 1 sibling, 1 reply; 57+ messages in thread From: Arvind Sankar @ 2020-04-30 20:40 UTC (permalink / raw) To: Ard Biesheuvel Cc: Joe Perches, Arvind Sankar, Linux Kernel Mailing List, linux-efi, X86 ML 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. ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening 2020-04-30 20:40 ` Arvind Sankar @ 2020-04-30 20:42 ` Ard Biesheuvel 0 siblings, 0 replies; 57+ messages in thread From: Ard Biesheuvel @ 2020-04-30 20:42 UTC (permalink / raw) To: Arvind Sankar; +Cc: Joe Perches, Linux Kernel Mailing List, linux-efi, X86 ML 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! ^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar ` (11 preceding siblings ...) 2020-04-30 19:12 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Joe Perches @ 2020-05-04 8:17 ` Ard Biesheuvel 12 siblings, 0 replies; 57+ messages in thread From: Ard Biesheuvel @ 2020-05-04 8:17 UTC (permalink / raw) To: Arvind Sankar; +Cc: linux-efi, X86 ML, Linux Kernel Mailing List 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 > ^ permalink raw reply [flat|nested] 57+ messages in thread
end of thread, other threads:[~2020-05-05 8:01 UTC | newest] Thread overview: 57+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar 2020-04-29 17:41 ` [PATCH 01/10] efi/x86: Use correct size for boot_params Arvind Sankar 2020-04-29 17:41 ` [PATCH 02/10] efi: Add a helper function to split 64-bit values Arvind Sankar 2020-04-29 23:51 ` kbuild test robot 2020-04-29 17:41 ` [PATCH 02/10] efi/libstub: " Arvind Sankar 2020-04-29 17:41 ` [PATCH 03/10] efi/x86: Use pr_efi_err for error messages Arvind Sankar 2020-04-29 18:47 ` Joe Perches 2020-04-29 18:49 ` Ard Biesheuvel 2020-04-29 18:57 ` Joe Perches 2020-04-29 18:59 ` Ard Biesheuvel 2020-04-29 19:47 ` Joe Perches 2020-04-29 19:48 ` Ard Biesheuvel 2020-04-29 21:43 ` Arvind Sankar 2020-04-29 21:45 ` Ard Biesheuvel 2020-04-29 21:51 ` Arvind Sankar 2020-04-29 21:53 ` Joe Perches 2020-04-29 21:55 ` Ard Biesheuvel 2020-04-29 22:20 ` Arvind Sankar 2020-04-30 15:14 ` Ard Biesheuvel 2020-04-29 17:41 ` [PATCH 04/10] efi/gop: " Arvind Sankar 2020-04-29 17:41 ` [PATCH 05/10] efi/tpm: " Arvind Sankar 2020-04-29 17:41 ` [PATCH 06/10] efi/x86: Move command-line initrd loading to efi_main Arvind Sankar 2020-04-29 17:41 ` [PATCH 07/10] efi/libstub: Unify initrd loading across architectures Arvind Sankar 2020-04-29 17:41 ` [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading Arvind Sankar 2020-04-29 19:05 ` Ard Biesheuvel 2020-04-29 21:33 ` Arvind Sankar 2020-04-29 17:41 ` [PATCH 09/10] efi/x86: Support builtin command line Arvind Sankar 2020-04-29 19:07 ` Ard Biesheuvel 2020-04-29 21:39 ` Arvind Sankar 2020-04-29 21:40 ` Ard Biesheuvel 2020-04-29 21:48 ` Arvind Sankar 2020-04-29 21:51 ` Ard Biesheuvel 2020-04-29 17:41 ` [PATCH 10/10] efi/libstub: Check return value of efi_parse_options Arvind Sankar 2020-04-30 4:31 ` kbuild test robot 2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 01/11] efi/x86: Use correct size for boot_params Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 02/11] efi/libstub: Add a helper function to split 64-bit values Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 03/11] efi/libstub: Move pr_efi/pr_efi_err into efi namespace Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 04/11] efi/x86: Use efi_err for error messages Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 05/11] efi/gop: " Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 06/11] efi/tpm: " Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 07/11] efi/libstub: Upgrade ignored dtb= argument message to error Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 08/11] efi/x86: Move command-line initrd loading to efi_main Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 09/11] efi/libstub: Unify initrd loading across architectures Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 10/11] efi/x86: Support builtin command line Arvind Sankar 2020-04-30 18:28 ` [PATCH v2 11/11] efi/libstub: Check return value of efi_parse_options Arvind Sankar 2020-04-30 19:12 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Joe Perches 2020-04-30 19:12 ` [PATCH 2/2] efi/libstub: Correct comment typos Joe Perches 2020-04-30 19:30 ` Ard Biesheuvel 2020-05-04 18:29 ` [trivial PATCH] efi/libstub: Reduce efi_printk object size Joe Perches 2020-05-05 7:50 ` Ard Biesheuvel 2020-05-05 8:01 ` Joe Perches 2020-04-30 19:29 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Ard Biesheuvel 2020-04-30 19:38 ` Joe Perches 2020-04-30 20:40 ` Arvind Sankar 2020-04-30 20:42 ` Ard Biesheuvel 2020-05-04 8:17 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).