|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH v2 for-4.22] EFI: Fix boot from a device without a file system
When netbooting a unified Xen kernel image (via GRUB chainloader),
the resulting loaded_image->DeviceHandle does not support
SIMPLE_FILE_SYSTEM_PROTOCOL.
Instead of crashing via noreturn PrintErrMesg() in get_parent_handle(),
we defer calling this function until filesystem access is needed.
This way when booting UKI, get_parent_handle() is not called at all.
Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Suggested-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Szymon Acedański <accek@xxxxxxxxxxxxxxxxxxxxxx>
---
Changes in v2:
- Restructured along the lines Andrew and Marek both suggested on v1:
defer get_parent_handle() until the first call site that actually
needs a file.
This mirrors the existing lazy pattern in ARM's
allocate_module_file() in xen/arch/arm/efi/efi-boot.h, which was also
changed to use the new ensure_dir_handle() helper.
Tests performed:
- Same as v1: PXE-loaded GRUB chainloading UKI - failure without
patch, success with patch
- QEMU boot from EFI partition, with config, kernel and initrd on EFI
partition too - success with and without patch (halts later in both
cases due to unrelated issue)
- Cross-compiling ARM64 - success
xen/arch/arm/efi/efi-boot.h | 12 ++++----
xen/arch/x86/efi/efi-boot.h | 9 ++++--
xen/common/efi/boot.c | 57 ++++++++++++++++++++++++-------------
3 files changed, 50 insertions(+), 28 deletions(-)
diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
index ea59de47e7..069cc68b0a 100644
--- a/xen/arch/arm/efi/efi-boot.h
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -403,7 +403,7 @@ static void __init noreturn efi_arch_post_exit_boot(void)
}
static void __init efi_arch_cfg_file_early(const EFI_LOADED_IMAGE *image,
- EFI_FILE_HANDLE dir_handle,
+ EFI_FILE_HANDLE *dir_handle,
const char *section)
{
union string name;
@@ -419,8 +419,11 @@ static void __init efi_arch_cfg_file_early(const
EFI_LOADED_IMAGE *image,
name.s = get_value(&cfg, section, "dtb");
if ( name.s )
{
+ CHAR16 *fname;
+
split_string(name.s);
- read_file(dir_handle, s2w(&name), &dtbfile, NULL);
+ ensure_dir_handle(image, dir_handle, &fname);
+ read_file(*dir_handle, s2w(&name), &dtbfile, NULL);
efi_bs->FreePool(name.w);
}
}
@@ -430,7 +433,7 @@ static void __init efi_arch_cfg_file_early(const
EFI_LOADED_IMAGE *image,
}
static void __init efi_arch_cfg_file_late(const EFI_LOADED_IMAGE *image,
- EFI_FILE_HANDLE dir_handle,
+ EFI_FILE_HANDLE *dir_handle,
const char *section)
{
}
@@ -665,8 +668,7 @@ static int __init allocate_module_file(const
EFI_LOADED_IMAGE *loaded_image,
file_info->name_len = name_len;
/* Get the file system interface. */
- if ( !*dir_handle )
- *dir_handle = get_parent_handle(loaded_image, &fname);
+ ensure_dir_handle(loaded_image, dir_handle, &fname);
/* Load the binary in memory */
read_file(*dir_handle, s2w(&module_name), &module_binary, NULL);
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 42a2c46b5e..d738b839ee 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -284,13 +284,13 @@ static void __init noreturn efi_arch_post_exit_boot(void)
}
static void __init efi_arch_cfg_file_early(const EFI_LOADED_IMAGE *image,
- EFI_FILE_HANDLE dir_handle,
+ EFI_FILE_HANDLE *dir_handle,
const char *section)
{
}
static void __init efi_arch_cfg_file_late(const EFI_LOADED_IMAGE *image,
- EFI_FILE_HANDLE dir_handle,
+ EFI_FILE_HANDLE *dir_handle,
const char *section)
{
union string name;
@@ -304,9 +304,12 @@ static void __init efi_arch_cfg_file_late(const
EFI_LOADED_IMAGE *image,
name.s = get_value(&cfg, "global", "ucode");
if ( name.s )
{
+ CHAR16 *fname;
+
microcode_set_module(mbi.mods_count);
split_string(name.s);
- read_file(dir_handle, s2w(&name), &ucode, NULL);
+ ensure_dir_handle(image, dir_handle, &fname);
+ read_file(*dir_handle, s2w(&name), &ucode, NULL);
efi_bs->FreePool(name.w);
}
}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 9ea2183c0b..f90309624b 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -547,6 +547,17 @@ static EFI_FILE_HANDLE __init get_parent_handle(const
EFI_LOADED_IMAGE *loaded_i
return dir_handle;
}
+static void __init ensure_dir_handle(const EFI_LOADED_IMAGE *loaded_image,
+ EFI_FILE_HANDLE *dir_handle,
+ CHAR16 **file_name)
+{
+ if ( *dir_handle )
+ return;
+ *dir_handle = get_parent_handle(loaded_image, file_name);
+ if ( !*dir_handle )
+ blexit(L"Cannot load files without a usable file system");
+}
+
static CHAR16 *__init point_tail(CHAR16 *fn)
{
CHAR16 *tail = NULL;
@@ -1514,7 +1525,7 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
if ( use_cfg_file )
{
- EFI_FILE_HANDLE dir_handle;
+ EFI_FILE_HANDLE dir_handle = NULL;
EFI_HANDLE gop_handle;
UINTN depth, cols, rows;
@@ -1526,31 +1537,33 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
gop = efi_get_gop(&gop_handle);
- /* Get the file system interface. */
- dir_handle = get_parent_handle(loaded_image, &file_name);
-
/* Read and parse the config file. */
if ( read_section(loaded_image, L"config", &cfg, NULL) )
PrintStr(L"Using builtin config file\r\n");
- else if ( !cfg_file_name && file_name )
+ else
{
- CHAR16 *tail;
+ ensure_dir_handle(loaded_image, &dir_handle, &file_name);
- while ( (tail = point_tail(file_name)) != NULL )
+ if ( !cfg_file_name )
{
- wstrcpy(tail, L".cfg");
- if ( read_file(dir_handle, file_name, &cfg, NULL) )
- break;
- *tail = 0;
+ CHAR16 *tail;
+
+ while ( (tail = point_tail(file_name)) != NULL )
+ {
+ wstrcpy(tail, L".cfg");
+ if ( read_file(dir_handle, file_name, &cfg, NULL) )
+ break;
+ *tail = 0;
+ }
+ if ( !tail )
+ blexit(L"No configuration file found.");
+ PrintStr(L"Using configuration file '");
+ PrintStr(file_name);
+ PrintStr(L"'\r\n");
}
- if ( !tail )
- blexit(L"No configuration file found.");
- PrintStr(L"Using configuration file '");
- PrintStr(file_name);
- PrintStr(L"'\r\n");
+ else if ( !read_file(dir_handle, cfg_file_name, &cfg, NULL) )
+ blexit(L"Configuration file not found.");
}
- else if ( !read_file(dir_handle, cfg_file_name, &cfg, NULL) )
- blexit(L"Configuration file not found.");
pre_parse(&cfg);
if ( section.w )
@@ -1567,6 +1580,7 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
if ( !name.s )
break;
free_cfg();
+ ensure_dir_handle(loaded_image, &dir_handle, &file_name);
if ( !read_file(dir_handle, s2w(&name), &cfg, NULL) )
{
PrintStr(L"Chained configuration file '");
@@ -1578,13 +1592,14 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
efi_bs->FreePool(name.w);
}
- efi_arch_cfg_file_early(loaded_image, dir_handle, section.s);
+ efi_arch_cfg_file_early(loaded_image, &dir_handle, section.s);
option_str = name.s ? split_string(name.s) : NULL;
if ( !read_section(loaded_image, L"kernel", &kernel, option_str) &&
name.s )
{
+ ensure_dir_handle(loaded_image, &dir_handle, &file_name);
read_file(dir_handle, s2w(&name), &kernel, option_str);
efi_bs->FreePool(name.w);
}
@@ -1599,6 +1614,7 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
name.s = get_value(&cfg, section.s, "ramdisk");
if ( name.s )
{
+ ensure_dir_handle(loaded_image, &dir_handle, &file_name);
read_file(dir_handle, s2w(&name), &ramdisk, NULL);
efi_bs->FreePool(name.w);
}
@@ -1609,6 +1625,7 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
name.s = get_value(&cfg, section.s, "xsm");
if ( name.s )
{
+ ensure_dir_handle(loaded_image, &dir_handle, &file_name);
read_file(dir_handle, s2w(&name), &xsm, NULL);
efi_bs->FreePool(name.w);
}
@@ -1634,7 +1651,7 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
}
}
- efi_arch_cfg_file_late(loaded_image, dir_handle, section.s);
+ efi_arch_cfg_file_late(loaded_image, &dir_handle, section.s);
free_cfg();
--
2.53.0
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |