* [PATCH 1/4] efi_loader: description EFI_LOAD_FILE2_PROTOCOL
2020-10-03 11:57 [PATCH 0/4] efi_loader: fix EFI_LOAD_FILE2_PROTOCOL Heinrich Schuchardt
@ 2020-10-03 11:57 ` Heinrich Schuchardt
2020-10-04 5:14 ` Ilias Apalodimas
2020-10-03 11:57 ` [PATCH 2/4] efi_loader: illegal free in EFI_LOAD_FILE2_PROTOCOL Heinrich Schuchardt
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Heinrich Schuchardt @ 2020-10-03 11:57 UTC (permalink / raw)
To: u-boot
U-Boot offers a EFI_LOAD_FILE2_PROTOCOL which the Linux EFI stub can use to
load an initial RAM disk. Update the function comments of the
implementation.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
lib/efi_loader/efi_load_initrd.c | 42 +++++++++++++++++---------------
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
index 574a83d7e3..ff69e6eb79 100644
--- a/lib/efi_loader/efi_load_initrd.c
+++ b/lib/efi_loader/efi_load_initrd.c
@@ -47,9 +47,9 @@ static const struct efi_initrd_dp dp = {
/**
* get_file_size() - retrieve the size of initramfs, set efi status on error
*
- * @dev: device to read from. i.e "mmc"
- * @part: device partition. i.e "0:1"
- * @file: name fo file
+ * @dev: device to read from, e.g. "mmc"
+ * @part: device partition, e.g. "0:1"
+ * @file: name of file
* @status: EFI exit code in case of failure
*
* Return: size of file
@@ -78,15 +78,16 @@ out:
}
/**
- * load_file2() - get information about random number generation
+ * efi_load_file2initrd() - load initial RAM disk
+ *
+ * This function implements the LoadFile service of the EFI_LOAD_FILE2_PROTOCOL
+ * in order to load an initial RAM disk requested by the Linux kernel stub.
*
- * This function implement the LoadFile2() service in order to load an initram
- * disk requested by the Linux kernel stub.
* See the UEFI spec for details.
*
- * @this: loadfile2 protocol instance
- * @file_path: relative path of the file. "" in this case
- * @boot_policy: must be false for Loadfile2
+ * @this: EFI_LOAD_FILE2_PROTOCOL instance
+ * @file_path: media device path of the file, "" in this case
+ * @boot_policy: must be false
* @buffer_size: size of allocated buffer
* @buffer: buffer to load the file
*
@@ -128,7 +129,13 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
goto out;
}
- /* expect something like 'mmc 0:1 initrd.cpio.gz' */
+ /*
+ * expect a string with three space separated parts:
+ *
+ * * a block device type, e.g. "mmc"
+ * * a device and partition identifier, e.g. "0:1"
+ * * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
+ */
dev = strsep(&s, " ");
if (!dev)
goto out;
@@ -168,23 +175,20 @@ out:
}
/**
- * efi_initrd_register() - Register a handle and loadfile2 protocol
+ * efi_initrd_register() - create handle for loading initial RAM disk
*
- * This function creates a new handle and installs a linux specific GUID
- * to handle initram disk loading during boot.
- * See the UEFI spec for details.
+ * This function creates a new handle and installs a Linux specific vendor
+ * device path and an EFI_LOAD_FILE_2_PROTOCOL. Linux uses the device path
+ * to identify the handle and then calls the LoadFile service of the
+ * EFI_LOAD_FILE_2_PROTOCOL to read the initial RAM disk.
*
- * Return: status code
+ * Return: status code
*/
efi_status_t efi_initrd_register(void)
{
efi_handle_t efi_initrd_handle = NULL;
efi_status_t ret;
- /*
- * Set up the handle with the EFI_LOAD_FILE2_PROTOCOL which Linux may
- * use to load the initial ramdisk.
- */
ret = EFI_CALL(efi_install_multiple_protocol_interfaces
(&efi_initrd_handle,
/* initramfs */
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 1/4] efi_loader: description EFI_LOAD_FILE2_PROTOCOL
2020-10-03 11:57 ` [PATCH 1/4] efi_loader: description EFI_LOAD_FILE2_PROTOCOL Heinrich Schuchardt
@ 2020-10-04 5:14 ` Ilias Apalodimas
0 siblings, 0 replies; 8+ messages in thread
From: Ilias Apalodimas @ 2020-10-04 5:14 UTC (permalink / raw)
To: u-boot
On Sat, Oct 03, 2020 at 01:57:13PM +0200, Heinrich Schuchardt wrote:
> U-Boot offers a EFI_LOAD_FILE2_PROTOCOL which the Linux EFI stub can use to
> load an initial RAM disk. Update the function comments of the
> implementation.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> lib/efi_loader/efi_load_initrd.c | 42 +++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> index 574a83d7e3..ff69e6eb79 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -47,9 +47,9 @@ static const struct efi_initrd_dp dp = {
> /**
> * get_file_size() - retrieve the size of initramfs, set efi status on error
> *
> - * @dev: device to read from. i.e "mmc"
> - * @part: device partition. i.e "0:1"
> - * @file: name fo file
> + * @dev: device to read from, e.g. "mmc"
> + * @part: device partition, e.g. "0:1"
> + * @file: name of file
> * @status: EFI exit code in case of failure
> *
> * Return: size of file
> @@ -78,15 +78,16 @@ out:
> }
>
> /**
> - * load_file2() - get information about random number generation
> + * efi_load_file2initrd() - load initial RAM disk
> + *
> + * This function implements the LoadFile service of the EFI_LOAD_FILE2_PROTOCOL
> + * in order to load an initial RAM disk requested by the Linux kernel stub.
> *
> - * This function implement the LoadFile2() service in order to load an initram
> - * disk requested by the Linux kernel stub.
> * See the UEFI spec for details.
> *
> - * @this: loadfile2 protocol instance
> - * @file_path: relative path of the file. "" in this case
> - * @boot_policy: must be false for Loadfile2
> + * @this: EFI_LOAD_FILE2_PROTOCOL instance
> + * @file_path: media device path of the file, "" in this case
> + * @boot_policy: must be false
> * @buffer_size: size of allocated buffer
> * @buffer: buffer to load the file
> *
> @@ -128,7 +129,13 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
> goto out;
> }
>
> - /* expect something like 'mmc 0:1 initrd.cpio.gz' */
> + /*
> + * expect a string with three space separated parts:
> + *
> + * * a block device type, e.g. "mmc"
> + * * a device and partition identifier, e.g. "0:1"
> + * * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
> + */
> dev = strsep(&s, " ");
> if (!dev)
> goto out;
> @@ -168,23 +175,20 @@ out:
> }
>
> /**
> - * efi_initrd_register() - Register a handle and loadfile2 protocol
> + * efi_initrd_register() - create handle for loading initial RAM disk
> *
> - * This function creates a new handle and installs a linux specific GUID
> - * to handle initram disk loading during boot.
> - * See the UEFI spec for details.
> + * This function creates a new handle and installs a Linux specific vendor
> + * device path and an EFI_LOAD_FILE_2_PROTOCOL. Linux uses the device path
> + * to identify the handle and then calls the LoadFile service of the
> + * EFI_LOAD_FILE_2_PROTOCOL to read the initial RAM disk.
> *
> - * Return: status code
> + * Return: status code
> */
> efi_status_t efi_initrd_register(void)
> {
> efi_handle_t efi_initrd_handle = NULL;
> efi_status_t ret;
>
> - /*
> - * Set up the handle with the EFI_LOAD_FILE2_PROTOCOL which Linux may
> - * use to load the initial ramdisk.
> - */
> ret = EFI_CALL(efi_install_multiple_protocol_interfaces
> (&efi_initrd_handle,
> /* initramfs */
> --
> 2.28.0
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/4] efi_loader: illegal free in EFI_LOAD_FILE2_PROTOCOL
2020-10-03 11:57 [PATCH 0/4] efi_loader: fix EFI_LOAD_FILE2_PROTOCOL Heinrich Schuchardt
2020-10-03 11:57 ` [PATCH 1/4] efi_loader: description EFI_LOAD_FILE2_PROTOCOL Heinrich Schuchardt
@ 2020-10-03 11:57 ` Heinrich Schuchardt
2020-10-04 5:16 ` Ilias Apalodimas
2020-10-03 11:57 ` [PATCH 3/4] efi_selftest: enable printing hexadecimal numbers Heinrich Schuchardt
2020-10-03 11:57 ` [PATCH 4/4] efi_selftest: print CRC32 of initrd as hexadecimal Heinrich Schuchardt
3 siblings, 1 reply; 8+ messages in thread
From: Heinrich Schuchardt @ 2020-10-03 11:57 UTC (permalink / raw)
To: u-boot
strsep() changes the address that its first argument points to.
We cannot use the changed address as argument of free().
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
lib/efi_loader/efi_load_initrd.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
index ff69e6eb79..d517d686c3 100644
--- a/lib/efi_loader/efi_load_initrd.c
+++ b/lib/efi_loader/efi_load_initrd.c
@@ -98,19 +98,20 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
struct efi_device_path *file_path, bool boot_policy,
efi_uintn_t *buffer_size, void *buffer)
{
- const char *filespec = CONFIG_EFI_INITRD_FILESPEC;
+ char *filespec;
efi_status_t status = EFI_NOT_FOUND;
loff_t file_sz = 0, read_sz = 0;
char *dev, *part, *file;
- char *s;
+ char *pos;
int ret;
EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
buffer_size, buffer);
- s = strdup(filespec);
- if (!s)
+ filespec = strdup(CONFIG_EFI_INITRD_FILESPEC);
+ if (!filespec)
goto out;
+ pos = filespec;
if (!this || this != &efi_lf2_protocol ||
!buffer_size) {
@@ -136,13 +137,13 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
* * a device and partition identifier, e.g. "0:1"
* * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
*/
- dev = strsep(&s, " ");
+ dev = strsep(&pos, " ");
if (!dev)
goto out;
- part = strsep(&s, " ");
+ part = strsep(&pos, " ");
if (!part)
goto out;
- file = strsep(&s, " ");
+ file = strsep(&pos, " ");
if (!file)
goto out;
@@ -170,7 +171,7 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
}
out:
- free(s);
+ free(filespec);
return EFI_EXIT(status);
}
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4] efi_loader: illegal free in EFI_LOAD_FILE2_PROTOCOL
2020-10-03 11:57 ` [PATCH 2/4] efi_loader: illegal free in EFI_LOAD_FILE2_PROTOCOL Heinrich Schuchardt
@ 2020-10-04 5:16 ` Ilias Apalodimas
0 siblings, 0 replies; 8+ messages in thread
From: Ilias Apalodimas @ 2020-10-04 5:16 UTC (permalink / raw)
To: u-boot
On Sat, Oct 03, 2020 at 01:57:14PM +0200, Heinrich Schuchardt wrote:
> strsep() changes the address that its first argument points to.
> We cannot use the changed address as argument of free().
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> lib/efi_loader/efi_load_initrd.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> index ff69e6eb79..d517d686c3 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -98,19 +98,20 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
> struct efi_device_path *file_path, bool boot_policy,
> efi_uintn_t *buffer_size, void *buffer)
> {
> - const char *filespec = CONFIG_EFI_INITRD_FILESPEC;
> + char *filespec;
> efi_status_t status = EFI_NOT_FOUND;
> loff_t file_sz = 0, read_sz = 0;
> char *dev, *part, *file;
> - char *s;
> + char *pos;
> int ret;
>
> EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
> buffer_size, buffer);
>
> - s = strdup(filespec);
> - if (!s)
> + filespec = strdup(CONFIG_EFI_INITRD_FILESPEC);
> + if (!filespec)
> goto out;
> + pos = filespec;
>
> if (!this || this != &efi_lf2_protocol ||
> !buffer_size) {
> @@ -136,13 +137,13 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
> * * a device and partition identifier, e.g. "0:1"
> * * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
> */
> - dev = strsep(&s, " ");
> + dev = strsep(&pos, " ");
> if (!dev)
> goto out;
> - part = strsep(&s, " ");
> + part = strsep(&pos, " ");
> if (!part)
> goto out;
> - file = strsep(&s, " ");
> + file = strsep(&pos, " ");
> if (!file)
> goto out;
>
> @@ -170,7 +171,7 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
> }
>
> out:
> - free(s);
> + free(filespec);
> return EFI_EXIT(status);
> }
>
> --
> 2.28.0
>
Not changing the variable names would make this an one liner to read.
The changes do make sense though so
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/4] efi_selftest: enable printing hexadecimal numbers
2020-10-03 11:57 [PATCH 0/4] efi_loader: fix EFI_LOAD_FILE2_PROTOCOL Heinrich Schuchardt
2020-10-03 11:57 ` [PATCH 1/4] efi_loader: description EFI_LOAD_FILE2_PROTOCOL Heinrich Schuchardt
2020-10-03 11:57 ` [PATCH 2/4] efi_loader: illegal free in EFI_LOAD_FILE2_PROTOCOL Heinrich Schuchardt
@ 2020-10-03 11:57 ` Heinrich Schuchardt
2020-10-03 11:57 ` [PATCH 4/4] efi_selftest: print CRC32 of initrd as hexadecimal Heinrich Schuchardt
3 siblings, 0 replies; 8+ messages in thread
From: Heinrich Schuchardt @ 2020-10-03 11:57 UTC (permalink / raw)
To: u-boot
Add code to use %x in efi_st_print().
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
lib/efi_selftest/efi_selftest_console.c | 35 ++++++++++++++++---------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/lib/efi_selftest/efi_selftest_console.c b/lib/efi_selftest/efi_selftest_console.c
index 13f3ee6bc1..e3f4e39348 100644
--- a/lib/efi_selftest/efi_selftest_console.c
+++ b/lib/efi_selftest/efi_selftest_console.c
@@ -44,25 +44,28 @@ static void mac(void *pointer, u16 **buf)
}
/*
- * Print a pointer to an u16 string
+ * printx() - print hexadecimal number to an u16 string
*
- * @pointer: pointer
- * @buf: pointer to buffer address
- * on return position of terminating zero word
+ * @pointer: pointer
+ * @prec: minimum number of digits to print
+ * @buf: pointer to buffer address,
+ * on return position of terminating zero word
+ * @size: size of value to be printed in bytes
*/
-static void pointer(void *pointer, u16 **buf)
+static void printx(u64 p, int prec, u16 **buf)
{
int i;
u16 c;
- uintptr_t p = (uintptr_t)pointer;
u16 *pos = *buf;
- for (i = 8 * sizeof(p) - 4; i >= 0; i -= 4) {
- c = (p >> i) & 0x0f;
- c += '0';
- if (c > '9')
- c += 'a' - '9' - 1;
- *pos++ = c;
+ for (i = 2 * sizeof(p) - 1; i >= 0; --i) {
+ c = (p >> (4 * i)) & 0x0f;
+ if (c || pos != *buf || !i || i < prec) {
+ c += '0';
+ if (c > '9')
+ c += 'a' - '9' - 1;
+ *pos++ = c;
+ }
}
*pos = 0;
*buf = pos;
@@ -212,7 +215,9 @@ void efi_st_printc(int color, const char *fmt, ...)
break;
default:
--c;
- pointer(va_arg(args, void*), &pos);
+ printx((u64)va_arg(args, void *),
+ 2 * sizeof(void *), &pos);
+ break;
}
break;
case 's':
@@ -223,6 +228,10 @@ void efi_st_printc(int color, const char *fmt, ...)
case 'u':
uint2dec(va_arg(args, u32), prec, &pos);
break;
+ case 'x':
+ printx((u64)va_arg(args, unsigned int),
+ prec, &pos);
+ break;
default:
break;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] efi_selftest: print CRC32 of initrd as hexadecimal
2020-10-03 11:57 [PATCH 0/4] efi_loader: fix EFI_LOAD_FILE2_PROTOCOL Heinrich Schuchardt
` (2 preceding siblings ...)
2020-10-03 11:57 ` [PATCH 3/4] efi_selftest: enable printing hexadecimal numbers Heinrich Schuchardt
@ 2020-10-03 11:57 ` Heinrich Schuchardt
2020-10-04 5:16 ` Ilias Apalodimas
3 siblings, 1 reply; 8+ messages in thread
From: Heinrich Schuchardt @ 2020-10-03 11:57 UTC (permalink / raw)
To: u-boot
Print the CRC32 loaded via the EFI_LOAD_FILE2_PROTOCOL as a hexadecimal
number.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
lib/efi_selftest/efi_selftest_load_initrd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_selftest/efi_selftest_load_initrd.c b/lib/efi_selftest/efi_selftest_load_initrd.c
index e16163caca..fe060a6644 100644
--- a/lib/efi_selftest/efi_selftest_load_initrd.c
+++ b/lib/efi_selftest/efi_selftest_load_initrd.c
@@ -200,7 +200,7 @@ static int execute(void)
efi_st_error("Could not determine CRC32\n");
return EFI_ST_FAILURE;
}
- efi_st_printf("CRC32 %u\n", (unsigned int)crc32);
+ efi_st_printf("CRC32 %.8x\n", (unsigned int)crc32);
status = boottime->free_pool(buf);
if (status != EFI_SUCCESS) {
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] efi_selftest: print CRC32 of initrd as hexadecimal
2020-10-03 11:57 ` [PATCH 4/4] efi_selftest: print CRC32 of initrd as hexadecimal Heinrich Schuchardt
@ 2020-10-04 5:16 ` Ilias Apalodimas
0 siblings, 0 replies; 8+ messages in thread
From: Ilias Apalodimas @ 2020-10-04 5:16 UTC (permalink / raw)
To: u-boot
On Sat, Oct 03, 2020 at 01:57:16PM +0200, Heinrich Schuchardt wrote:
> Print the CRC32 loaded via the EFI_LOAD_FILE2_PROTOCOL as a hexadecimal
> number.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> lib/efi_selftest/efi_selftest_load_initrd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_selftest/efi_selftest_load_initrd.c b/lib/efi_selftest/efi_selftest_load_initrd.c
> index e16163caca..fe060a6644 100644
> --- a/lib/efi_selftest/efi_selftest_load_initrd.c
> +++ b/lib/efi_selftest/efi_selftest_load_initrd.c
> @@ -200,7 +200,7 @@ static int execute(void)
> efi_st_error("Could not determine CRC32\n");
> return EFI_ST_FAILURE;
> }
> - efi_st_printf("CRC32 %u\n", (unsigned int)crc32);
> + efi_st_printf("CRC32 %.8x\n", (unsigned int)crc32);
>
> status = boottime->free_pool(buf);
> if (status != EFI_SUCCESS) {
> --
> 2.28.0
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 8+ messages in thread