From 83d905b7b4fe1a1b63e385bdc43f17f4f2361110 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 19 Jan 2022 11:15:22 +0100 Subject: [PATCH 1/4] boot: Optimize unique title generation --- src/boot/efi/boot.c | 73 +++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 39 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index f49bd9e5f85..57ca183cf33 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -61,7 +61,6 @@ typedef struct { CHAR16 *options; CHAR16 key; EFI_STATUS (*call)(void); - BOOLEAN non_unique; UINTN tries_done; UINTN tries_left; CHAR16 *path; @@ -1717,88 +1716,84 @@ static void config_default_entry_select(Config *config) { config->timeout_sec = 10; } -static BOOLEAN find_nonunique(ConfigEntry **entries, UINTN entry_count) { - BOOLEAN non_unique = FALSE; +static BOOLEAN entries_unique(ConfigEntry **entries, BOOLEAN *unique, UINTN entry_count) { + BOOLEAN is_unique = TRUE; assert(entries); + assert(unique); for (UINTN i = 0; i < entry_count; i++) - entries[i]->non_unique = FALSE; - - for (UINTN i = 0; i < entry_count; i++) - for (UINTN k = 0; k < entry_count; k++) { - if (i == k) - continue; + for (UINTN k = i + 1; k < entry_count; k++) { if (StrCmp(entries[i]->title_show, entries[k]->title_show) != 0) continue; - non_unique = entries[i]->non_unique = entries[k]->non_unique = TRUE; + is_unique = unique[i] = unique[k] = FALSE; } - return non_unique; + return is_unique; } /* generate a unique title, avoiding non-distinguishable menu entries */ static void config_title_generate(Config *config) { assert(config); + BOOLEAN unique[config->entry_count]; + /* set title */ for (UINTN i = 0; i < config->entry_count; i++) { - FreePool(config->entries[i]->title_show); - config->entries[i]->title_show = xstrdup( - config->entries[i]->title ?: config->entries[i]->id); + assert(!config->entries[i]->title_show); + unique[i] = TRUE; + config->entries[i]->title_show = xstrdup(config->entries[i]->title ?: config->entries[i]->id); } - if (!find_nonunique(config->entries, config->entry_count)) + if (entries_unique(config->entries, unique, config->entry_count)) return; /* add version to non-unique titles */ for (UINTN i = 0; i < config->entry_count; i++) { - CHAR16 *s; - - if (!config->entries[i]->non_unique) + if (unique[i]) continue; + + unique[i] = TRUE; + if (!config->entries[i]->version) continue; - s = xpool_print(L"%s (%s)", config->entries[i]->title_show, config->entries[i]->version); - FreePool(config->entries[i]->title_show); - config->entries[i]->title_show = s; + _cleanup_freepool_ CHAR16 *t = config->entries[i]->title_show; + config->entries[i]->title_show = xpool_print(L"%s (%s)", t, config->entries[i]->version); } - if (!find_nonunique(config->entries, config->entry_count)) + if (entries_unique(config->entries, unique, config->entry_count)) return; /* add machine-id to non-unique titles */ for (UINTN i = 0; i < config->entry_count; i++) { - CHAR16 *s; - _cleanup_freepool_ CHAR16 *m = NULL; - - if (!config->entries[i]->non_unique) + if (unique[i]) continue; + + unique[i] = TRUE; + if (!config->entries[i]->machine_id) continue; - m = xstrdup(config->entries[i]->machine_id); - m[8] = '\0'; - s = xpool_print(L"%s (%s)", config->entries[i]->title_show, m); - FreePool(config->entries[i]->title_show); - config->entries[i]->title_show = s; + _cleanup_freepool_ CHAR16 *t = config->entries[i]->title_show; + config->entries[i]->title_show = xpool_print( + L"%s (%.*s)", + t, + StrnLen(config->entries[i]->machine_id, 8), + config->entries[i]->machine_id); } - if (!find_nonunique(config->entries, config->entry_count)) + if (entries_unique(config->entries, unique, config->entry_count)) return; /* add file name to non-unique titles */ for (UINTN i = 0; i < config->entry_count; i++) { - CHAR16 *s; - - if (!config->entries[i]->non_unique) + if (unique[i]) continue; - s = xpool_print(L"%s (%s)", config->entries[i]->title_show, config->entries[i]->id); - FreePool(config->entries[i]->title_show); - config->entries[i]->title_show = s; - config->entries[i]->non_unique = FALSE; + + _cleanup_freepool_ CHAR16 *t = config->entries[i]->title_show; + config->entries[i]->title_show = xpool_print(L"%s (%s)", t, config->entries[i]->id); } } From 20a734726ea2a18b265137a5b53654b9c413aa13 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 19 Jan 2022 12:10:32 +0100 Subject: [PATCH 2/4] boot: Use ASSERT_PTR --- src/boot/efi/util.h | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index b60814b4919..40ce98c6110 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -24,13 +24,6 @@ #define UINT64_MAX ((UINT64) -1) #endif -#define assert_alloc_ret(p) \ - ({ \ - void *_p = (p); \ - assert(_p); \ - _p; \ - }) - #define xnew_alloc(type, n, alloc) \ ({ \ UINTN _alloc_size; \ @@ -39,11 +32,11 @@ (type *) alloc(_alloc_size); \ }) -#define xallocate_pool(size) assert_alloc_ret(AllocatePool(size)) -#define xallocate_zero_pool(size) assert_alloc_ret(AllocateZeroPool(size)) -#define xreallocate_pool(p, old_size, new_size) assert_alloc_ret(ReallocatePool((p), (old_size), (new_size))) -#define xpool_print(fmt, ...) ((CHAR16 *) assert_alloc_ret(PoolPrint((fmt), ##__VA_ARGS__))) -#define xstrdup(str) ((CHAR16 *) assert_alloc_ret(StrDuplicate(str))) +#define xallocate_pool(size) ASSERT_PTR(AllocatePool(size)) +#define xallocate_zero_pool(size) ASSERT_PTR(AllocateZeroPool(size)) +#define xreallocate_pool(p, old_size, new_size) ASSERT_PTR(ReallocatePool((p), (old_size), (new_size))) +#define xpool_print(fmt, ...) ((CHAR16 *) ASSERT_PTR(PoolPrint((fmt), ##__VA_ARGS__))) +#define xstrdup(str) ((CHAR16 *) ASSERT_PTR(StrDuplicate(str))) #define xnew(type, n) xnew_alloc(type, (n), xallocate_pool) #define xnew0(type, n) xnew_alloc(type, (n), xallocate_zero_pool) From c323c2f63c9315bd705a1f8acf8f3abae06b5073 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 19 Jan 2022 13:28:32 +0100 Subject: [PATCH 3/4] boot: Also NUL-terminate for CHAR16 in file_reaad --- src/boot/efi/util.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index e023b97d2f0..362572cfadd 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -455,7 +455,7 @@ EFI_STATUS file_read(EFI_FILE *dir, const CHAR16 *name, UINTN off, UINTN size, C if (EFI_ERROR(err)) return err; - size = info->FileSize+1; + size = info->FileSize; } if (off > 0) { @@ -464,12 +464,16 @@ EFI_STATUS file_read(EFI_FILE *dir, const CHAR16 *name, UINTN off, UINTN size, C return err; } - buf = xallocate_pool(size + 1); + /* Allocate some extra bytes to guarantee the result is NUL-terminated for CHAR8 and CHAR16 strings. */ + UINTN extra = size % sizeof(CHAR16) + sizeof(CHAR16); + + buf = xallocate_pool(size + extra); err = handle->Read(handle, &size, buf); if (EFI_ERROR(err)) return err; - buf[size] = '\0'; + /* Note that handle->Read() changes size to reflect the actualy bytes read. */ + ZeroMem(buf + size, extra); *ret = TAKE_PTR(buf); if (ret_size) From 1926a1143913fa92a6bec7495f3ffe0c1e8254a1 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 26 Jan 2022 13:50:22 +0100 Subject: [PATCH 4/4] boot: Add screen resolution to print status --- src/boot/efi/boot.c | 4 +++- src/boot/efi/console.c | 27 ++++++++++++++++++++------- src/boot/efi/console.h | 1 + 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 57ca183cf33..eed5bcc9610 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -438,6 +438,7 @@ static void ps_bool(const CHAR16 *fmt, BOOLEAN value) { static void print_status(Config *config, CHAR16 *loaded_image_path) { UINT64 key; UINTN x_max, y_max; + UINT32 screen_width = 0, screen_height = 0; SecureBootMode secure; _cleanup_freepool_ CHAR16 *device_part_uuid = NULL; @@ -446,6 +447,7 @@ static void print_status(Config *config, CHAR16 *loaded_image_path) { clear_screen(COLOR_NORMAL); console_query_mode(&x_max, &y_max); + query_screen_resolution(&screen_width, &screen_height); secure = secure_boot_mode(); (void) efivar_get(LOADER_GUID, L"LoaderDevicePartUUID", &device_part_uuid); @@ -463,7 +465,7 @@ static void print_status(Config *config, CHAR16 *loaded_image_path) { Print(L" secure boot: %s (%s)\n", yes_no(IN_SET(secure, SECURE_BOOT_USER, SECURE_BOOT_DEPLOYED)), secure_boot_mode_to_string(secure)); ps_bool(L" shim: %s\n", shim_loaded()); ps_bool(L" TPM: %s\n", tpm_present()); - Print(L" console mode: %d/%d (%lu x %lu)\n", ST->ConOut->Mode->Mode, ST->ConOut->Mode->MaxMode - 1LL, x_max, y_max); + Print(L" console mode: %d/%d (%lux%lu @%ux%u)\n", ST->ConOut->Mode->Mode, ST->ConOut->Mode->MaxMode - 1LL, x_max, y_max, screen_width, screen_height); Print(L"\n--- Press any key to continue. ---\n\n"); console_key_read(&key, UINT64_MAX); diff --git a/src/boot/efi/console.c b/src/boot/efi/console.c index b8142c38b3f..3a0cf535e15 100644 --- a/src/boot/efi/console.c +++ b/src/boot/efi/console.c @@ -183,19 +183,32 @@ static EFI_STATUS change_mode(INT64 mode) { return err; } -static INT64 get_auto_mode(void) { - EFI_GRAPHICS_OUTPUT_PROTOCOL *GraphicsOutput; +EFI_STATUS query_screen_resolution(UINT32 *ret_w, UINT32 *ret_h) { EFI_STATUS err; + EFI_GRAPHICS_OUTPUT_PROTOCOL *go; - err = LibLocateProtocol(&GraphicsOutputProtocol, (void **)&GraphicsOutput); - if (!EFI_ERROR(err) && GraphicsOutput->Mode && GraphicsOutput->Mode->Info) { - EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *Info = GraphicsOutput->Mode->Info; + err = LibLocateProtocol(&GraphicsOutputProtocol, (void **) &go); + if (EFI_ERROR(err)) + return err; + + if (!go->Mode || !go->Mode->Info) + return EFI_DEVICE_ERROR; + + *ret_w = go->Mode->Info->HorizontalResolution; + *ret_h = go->Mode->Info->VerticalResolution; + return EFI_SUCCESS; +} + +static INT64 get_auto_mode(void) { + UINT32 screen_width, screen_height; + + if (!EFI_ERROR(query_screen_resolution(&screen_width, &screen_height))) { BOOLEAN keep = FALSE; /* Start verifying if we are in a resolution larger than Full HD * (1920x1080). If we're not, assume we're in a good mode and do not * try to change it. */ - if (Info->HorizontalResolution <= HORIZONTAL_MAX_OK && Info->VerticalResolution <= VERTICAL_MAX_OK) + if (screen_width <= HORIZONTAL_MAX_OK && screen_height <= VERTICAL_MAX_OK) keep = TRUE; /* For larger resolutions, calculate the ratio of the total screen * area to the text viewport area. If it's less than 10 times bigger, @@ -203,7 +216,7 @@ static INT64 get_auto_mode(void) { else { UINT64 text_area; UINTN x_max, y_max; - UINT64 screen_area = (UINT64)Info->HorizontalResolution * (UINT64)Info->VerticalResolution; + UINT64 screen_area = (UINT64)screen_width * (UINT64)screen_height; console_query_mode(&x_max, &y_max); text_area = SYSTEM_FONT_WIDTH * SYSTEM_FONT_HEIGHT * (UINT64)x_max * (UINT64)y_max; diff --git a/src/boot/efi/console.h b/src/boot/efi/console.h index 90086028c04..c27c19b39fb 100644 --- a/src/boot/efi/console.h +++ b/src/boot/efi/console.h @@ -29,3 +29,4 @@ enum { EFI_STATUS console_key_read(UINT64 *key, UINT64 timeout_usec); EFI_STATUS console_set_mode(INT64 mode); EFI_STATUS console_query_mode(UINTN *x_max, UINTN *y_max); +EFI_STATUS query_screen_resolution(UINT32 *ret_width, UINT32 *ret_height);