[PATCH v2 1/1] sandbox: fix building with CONFIG_SPL_TIMER=y
Building sandbox_defconfig with CONFIG_SPL_TIMER=y results in an error
include/dm/platdata.h:63:33: error: static assertion failed:
"Cannot use U_BOOT_DRVINFO with of-platdata.
Please use devicetree instead"
Add a missing condition in the sandbox driver.
Signed-off-by: Heinrich Schuchardt
---
v2:
don't create U_BOOT_DRVINFO for DT_PLAT_C
(as requested in review comment by Simon)
---
drivers/timer/sandbox_timer.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/timer/sandbox_timer.c b/drivers/timer/sandbox_timer.c
index c846bfb9f1..1da7e0c3a7 100644
--- a/drivers/timer/sandbox_timer.c
+++ b/drivers/timer/sandbox_timer.c
@@ -66,6 +66,8 @@ U_BOOT_DRIVER(sandbox_timer) = {
};
/* This is here in case we don't have a device tree */
+#if !CONFIG_IS_ENABLED(OF_PLATDATA)
U_BOOT_DRVINFO(sandbox_timer_non_fdt) = {
.name = "sandbox_timer",
};
+#endif
--
2.38.1
[PATCH 1/1] test: unit test for crc8
Add a unit test for the crc8() function.
Signed-off-by: Heinrich Schuchardt
---
test/lib/Makefile| 1 +
test/lib/test_crc8.c | 29 +
2 files changed, 30 insertions(+)
create mode 100644 test/lib/test_crc8.c
diff --git a/test/lib/Makefile b/test/lib/Makefile
index 7e7922fe3b..e0bd9e04e8 100644
--- a/test/lib/Makefile
+++ b/test/lib/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_UT_LIB_ASN1) += asn1.o
obj-$(CONFIG_UT_LIB_RSA) += rsa.o
obj-$(CONFIG_AES) += test_aes.o
obj-$(CONFIG_GETOPT) += getopt.o
+obj-$(CONFIG_CRC8) += test_crc8.o
obj-$(CONFIG_UT_LIB_CRYPT) += test_crypt.o
else
obj-$(CONFIG_SANDBOX) += kconfig_spl.o
diff --git a/test/lib/test_crc8.c b/test/lib/test_crc8.c
new file mode 100644
index 00..0dac97bc5b
--- /dev/null
+++ b/test/lib/test_crc8.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023, Heinrich Schuchardt
+ *
+ * Unit test for crc8
+ */
+
+#include
+#include
+#include
+
+static int lib_crc8(struct unit_test_state *uts) {
+ const char str[] = {0x20, 0xf4, 0xd8, 0x24, 0x6f, 0x41, 0x91, 0xae,
+ 0x46, 0x61, 0xf6, 0x55, 0xeb, 0x38, 0x47, 0x0f,
+ 0xec, 0xd8};
+ int actual1, actual2, actual3;
+ int expected1 = 0x47, expected2 = 0xea, expected3 = expected1;
+
+ actual1 = crc8(0, str, sizeof(str));
+ ut_asserteq(expected1, actual1);
+ actual2 = crc8(0, str, 7);
+ ut_asserteq(expected2, actual2);
+ actual3 = crc8(actual2, str + 7, sizeof(str) - 7);
+ ut_asserteq(expected3, actual3);
+
+ return 0;
+}
+
+LIB_TEST(lib_crc8, 0);
--
2.38.1
Re: [PATCH] doc: uefi: fix links
On 2/20/23 15:37, Vincent Stehlé wrote: Fix a couple of links so that they are rendered correctly with sphinx. Signed-off-by: Vincent Stehlé Cc: Heinrich Schuchardt Cc: Ilias Apalodimas Reviewed-by: Heinrich Schuchardt --- doc/develop/uefi/fwu_updates.rst | 3 ++- doc/develop/uefi/uefi.rst| 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/develop/uefi/fwu_updates.rst b/doc/develop/uefi/fwu_updates.rst index 72c850a7908..e4709d82b41 100644 --- a/doc/develop/uefi/fwu_updates.rst +++ b/doc/develop/uefi/fwu_updates.rst @@ -27,7 +27,8 @@ metadata. Individual drivers can be added based on the type of storage media, and its partitioning method. Details of the storage device containing the FWU metadata partitions are specified through a U-Boot specific device tree property `fwu-mdata-store`. Please refer to -U-Boot `doc `__ +U-Boot :download:`fwu-mdata-gpt.yaml +` for the device tree bindings. Enabling the FWU Multi Bank Update feature diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index a944c0fb803..ffe25ca2318 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -386,8 +386,8 @@ is because the FWU feature supports multiple partitions(banks) of updatable images, and the actual dfu alt number to which the image is to be written to is determined at runtime, based on the value of the update bank to which the image is to be written. For more information -on the FWU Multi Bank Update feature, please refer `doc -`__. +on the FWU Multi Bank Update feature, please refer to +:doc:`/develop/uefi/fwu_updates`. When using the FMP for FIT images, the image index value needs to be set to 1.
Re: [PATCH v6 6/6] doc: Add measured boot documentation
Am 22. Februar 2023 19:02:42 MEZ schrieb Eddie James : >Briefly describe the feature and specify the requirements. > >Signed-off-by: Eddie James >--- > doc/usage/index.rst | 1 + > doc/usage/measured_boot.rst | 23 +++ > 2 files changed, 24 insertions(+) > create mode 100644 doc/usage/measured_boot.rst > >diff --git a/doc/usage/index.rst b/doc/usage/index.rst >index cde7dcb14a..0cf78cb0e7 100644 >--- a/doc/usage/index.rst >+++ b/doc/usage/index.rst >@@ -12,6 +12,7 @@ Use U-Boot >partitions >cmdline >semihosting >+ measured_boot > > Shell commands > -- >diff --git a/doc/usage/measured_boot.rst b/doc/usage/measured_boot.rst >new file mode 100644 >index 00..8357b1f480 >--- /dev/null >+++ b/doc/usage/measured_boot.rst >@@ -0,0 +1,23 @@ >+.. SPDX-License-Identifier: GPL-2.0+ >+ >+Measured Boot >+= This completely misses o describe measured boot with UEFI. @Ilias, do you want to add that in a follow up patch? >+ >+U-Boot can perform a measured boot, the process of hashing various components >+of the boot process, extending the results in the TPM and logging the >+component's measurement in memory for the operating system to consume. >+ >+Requirements >+- >+ >+* A hardware TPM 2.0 supported by the U-Boot drivers >+* CONFIG_TPM=y >+* CONFIG_MEASURED_BOOT=y >+* Device-tree configuration of the TPM device to specify the memory area >+ for event logging. The TPM device node must either contain a phandle to >+ a reserved memory region or "linux,sml-base" and "linux,sml-size" >+ indicating the address and size of the memory region. An example can be >+ found in arch/sandbox/dts/test.dts >+* The operating system must also be configured to use the memory regions >+ specified in the U-Boot device-tree in order to make use of the event >+ log. Please, provide enough information such that a reader can set this up. This should include example code. Best regards Heinrich
Re: [PATCH RFC 0/3] FMP versioning support
Am 22. Februar 2023 11:40:33 MEZ schrieb Masahisa Kojima : >This series aims to add the versioning support >in FMP protocol implementation. > >EDK2 reference implementation utilizes the FMP Payload Header >inserted right before the capsule payload. With this series, >U-Boot also follows the EDK2 implementation. > >Note that this series is RFC and only tested with RAW image >with single image index. Hello Masahisa, Could you please describe if this change will be compatible with boards that are already using the existing U-Boot FMP implementation. Further please describe the benefit of the change. Best regards Heinrich > >[TODO] >- test with FIT image, authenticated capsule, multiple image index. >- enhance U-Boot mkeficapsule tool to insert FMP Payload Header > >Masahisa Kojima (3): > efi_loader: store firmware version into FmpState variable > efi_loader: versioning support in GetImageInfo > efi_loader: check lowest supported version in capsule update > > lib/efi_loader/efi_firmware.c | 269 ++ > 1 file changed, 242 insertions(+), 27 deletions(-) >
Re: [PATCH v5 30/44] lib: Add an SPL config for LIB_UUID
Am 22. Februar 2023 17:34:11 MEZ schrieb Simon Glass : >This is selected by PARTITION_UUIDS which has a separate option for SPL. >Add an SPL option for LIB_UUID also, so that we can keep them consistent. > >Also add one for PARTITION_TYPE_GUID to avoid a build error in part_efi.c >which wants to call a uuid function in SPL. > >Signed-off-by: Simon Glass >--- > >(no changes since v1) > > disk/Kconfig | 8 > lib/Kconfig | 4 > 2 files changed, 12 insertions(+) > >diff --git a/disk/Kconfig b/disk/Kconfig >index c9b9dbaf1a6..817b7c8c76d 100644 >--- a/disk/Kconfig >+++ b/disk/Kconfig >@@ -149,6 +149,7 @@ config SPL_PARTITION_UUIDS > bool "Enable support of UUID for partition in SPL" > depends on SPL_PARTITIONS > default y if SPL_EFI_PARTITION >+ select SPL_LIB_UUID > > config PARTITION_TYPE_GUID > bool "Enable support of GUID for partition type" >@@ -157,4 +158,11 @@ config PARTITION_TYPE_GUID > Activate the configuration of GUID type > for EFI partition > >+config SPL_PARTITION_TYPE_GUID Hello Simon, you have seen my series for using the partition type guid to load main U-Boot. I would like to put that change into v2023.04. If my series goes in first, we will have to add a patch to this series to adjust dependencies. Can we use the mmc driver in sandbox_spl for testing loading main U-Boot from mmc based on different criteria like partition number, partion_type_guid, file name? Best regards Heinrich >+ bool "Enable support of GUID for partition type (SPL)" >+ depends on SPL_EFI_PARTITION >+ help >+Activate the configuration of GUID type >+for EFI partition >+ > endmenu >diff --git a/lib/Kconfig b/lib/Kconfig >index 08318843231..4278b240554 100644 >--- a/lib/Kconfig >+++ b/lib/Kconfig >@@ -74,6 +74,10 @@ config HAVE_PRIVATE_LIBGCC > config LIB_UUID > bool > >+config SPL_LIB_UUID >+ depends on SPL >+ bool >+ > config SEMIHOSTING > bool "Support semihosting" > depends on ARM || RISCV
[PATCH] lib: bzip2: remove inlining
Compiling sandbox_defconfig with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y and
gcc 12.2.0-14ubuntu1 leads to a build error:
lib/bzip2/bzlib.c: In function 'BZ2_decompress':
lib/bzip2/bzlib.c:726:18: error: inlining failed in call to
'always_inline' 'BZ2_indexIntoF': function not considered for inlining
726 | __inline__ Int32 BZ2_indexIntoF ( Int32 indx, Int32 *cftab )
| ^
Leave it to the compiler if it inlines or not.
Signed-off-by: Heinrich Schuchardt
---
lib/bzip2/bzlib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/bzip2/bzlib.c b/lib/bzip2/bzlib.c
index bd589aa810..b28f14b540 100644
--- a/lib/bzip2/bzlib.c
+++ b/lib/bzip2/bzlib.c
@@ -723,7 +723,7 @@ void unRLE_obuf_to_output_FAST ( DState* s )
/*---*/
-__inline__ Int32 BZ2_indexIntoF ( Int32 indx, Int32 *cftab )
+Int32 BZ2_indexIntoF(Int32 indx, Int32 *cftab)
{
Int32 nb, na, mid;
nb = 0;
--
2.38.1
Re: [PATCH] efi_loader: efi_allocate_pages: check parameter pages
On 2/20/23 09:46, Peng Fan wrote:
On 2/20/2023 4:08 AM, Heinrich Schuchardt wrote:
On 2/16/23 11:53, Peng Fan (OSS) wrote:
From: Peng Fan
On i.MX8MM-EVK, when doing UEFI Capsule On Disk, we met such issue,
It will create Boot option for capsule on disk:
Boot:
VenHw(E61D73B9-A384-4ACC-AEAB-82E828F3628B)/eMMC(0x2)/eMMC(0x1)/HD(1,GPT,F5CC8412-CD9F-4C9E-A782-0E945461E89E,0x800,0x32000)
But when capsule update finished, the updated image booting will
trigger:
Loading Boot 'UEFI Capsule On Disk' failed
EFI boot manager: Cannot load any image
Found EFI removable media binary efi/boot/bootaa64.efi
"Synchronous Abort" handler, esr 0x9604
elr: 4029f40c lr : 402802f0 (reloc)
elr: bcd8b40c lr : bcd6c2f0
x0 : 02029ee86154940e x1 : bcd95458
x2 : 0010 x3 : bad31ad0
x4 : x5 : 02029ee86154940e
x6 : 07f0 x7 : 0007
x8 : 0009 x9 : 0008
x10: 0035 x11: 0010
x12: 0022 x13: 0001
x14: bacdedf0 x15: 0021
x16: bcd304d0 x17: 41a0
x18: bacebdb0 x19: b9c9f040
x20: bccecb28 x21: bcd95458
x22: 0001 x23: bad1f010
x24: bcdced70 x25: 1000
x26: b9c9e000 x27: 4000
x28: 0001 x29: bacdd030
Which place in the code do elr and lr relate to?
Have a look into u-boot.map to find the function and use objdump to
identify the exact code location in the *.o files.
If is the pages is 0, the efi_find_free_memory will return the next used
memory, check the parameter pages, and return EFI_INVALID_PARAMETER
if it
is 0.
Reviewed-by: Ye Li
Reported-by: Vincent Stehlé
Signed-off-by: Peng Fan
---
lib/efi_loader/efi_memory.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index b7bee98f79c..acca542033d 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -495,6 +495,9 @@ efi_status_t efi_allocate_pages(enum
efi_allocate_type type,
if (!memory)
return EFI_INVALID_PARAMETER;
+ if (!pages)
+ return EFI_INVALID_PARAMETER;
+
Looking at the UEFI specification this looks wrong. The EFI
specification does not forbid calling AllocatePages() with pages == 0.
So we should return EFI_SUCCESS.
EDK II returns EFI_NOT_FOUND for pages == 0. But this has no basis in
the specification.
Which function is the caller invoking AllocatePages() with pages = 0.
Where is the patch to fix it?
The call trace as below:
do_bootefi->do_efibootmgr->efi_bootmgr_load->try_load_entry
->efi_load_image->efi_load_image_from_path->efi_load_image_from_file
->efi_allocate_pages->efi_find_free_memory
There is an entry Boot:
VenHw(E61D73B9-A384-4ACC-AEAB-82E828F3628B)/eMMC(0x2)/eMMC(0x1)/HD(1,GPT,F5CC8412-CD9F-4C9E-A782-0E945461E89E,0x800,0x32000)
But the capsule1.bin has been removed during capsule update,
but the entry is still there.
The efi_file_size(f, &bs); will return bs as 0.
The issue is if len is 0, the efi_find_free_memory will return
memory that are already used by others.
The bug is in the caller not in AllocatePages:
AllocatePages() sets a pointer to a memory range of size 0.
We learnt in math the intersection of an empty set with any other set is
an empty set. A memory range of size 0 does not overlap with any other
memory range.
We have to fix the caller that requests 0 pages and then uses memory
outside of this zero byte area.
How can your problem be reproduced on QEMU?
Best regards
Heinrich
Regards,
Peng.
Best regards
Heinrich
switch (type) {
case EFI_ALLOCATE_ANY_PAGES:
/* Any page */
Re: [PATCH v2 03/13] efi: Support a 64-bit frame buffer address
On 2/22/23 20:12, Simon Glass wrote:
The current vesa structure only provides a 32-bit value for the frame
buffer. Many modern machines use an address outside the range.
It is still useful to have this common struct, but add a separate
frame-buffer address as well.
Add a comment for vesa_setup_video_priv() while we are here.
Signed-off-by: Simon Glass
---
(no changes since v1)
arch/x86/lib/fsp/fsp_graphics.c | 2 +-
drivers/pci/pci_rom.c | 10 ++
drivers/video/coreboot.c| 2 +-
drivers/video/efi.c | 34 +
include/vesa.h | 16 +++-
5 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/arch/x86/lib/fsp/fsp_graphics.c b/arch/x86/lib/fsp/fsp_graphics.c
index b07c666caf7..2bcc49f6051 100644
--- a/arch/x86/lib/fsp/fsp_graphics.c
+++ b/arch/x86/lib/fsp/fsp_graphics.c
@@ -106,7 +106,7 @@ static int fsp_video_probe(struct udevice *dev)
vesa->phys_base_ptr = dm_pci_read_bar32(dev, 2);
gd->fb_base = vesa->phys_base_ptr;
- ret = vesa_setup_video_priv(vesa, uc_priv, plat);
+ ret = vesa_setup_video_priv(vesa, vesa->phys_base_ptr, uc_priv, plat);
if (ret)
goto err;
diff --git a/drivers/pci/pci_rom.c b/drivers/pci/pci_rom.c
index 47b6e6e5bcf..f0dfe631490 100644
--- a/drivers/pci/pci_rom.c
+++ b/drivers/pci/pci_rom.c
@@ -325,7 +325,7 @@ err:
return ret;
}
-int vesa_setup_video_priv(struct vesa_mode_info *vesa,
+int vesa_setup_video_priv(struct vesa_mode_info *vesa, u64 fb,
struct video_priv *uc_priv,
struct video_uc_plat *plat)
{
@@ -348,9 +348,9 @@ int vesa_setup_video_priv(struct vesa_mode_info *vesa,
/* Use double buffering if enabled */
if (IS_ENABLED(CONFIG_VIDEO_COPY) && plat->base)
- plat->copy_base = vesa->phys_base_ptr;
+ plat->copy_base = fb;
else
- plat->base = vesa->phys_base_ptr;
+ plat->base = fb;
log_debug("base = %lx, copy_base = %lx\n", plat->base, plat->copy_base);
plat->size = vesa->bytes_per_scanline * vesa->y_resolution;
@@ -377,7 +377,9 @@ int vesa_setup_video(struct udevice *dev, int
(*int15_handler)(void))
return ret;
}
- ret = vesa_setup_video_priv(&mode_info.vesa, uc_priv, plat);
+ ret = vesa_setup_video_priv(&mode_info.vesa,
+ mode_info.vesa.phys_base_ptr, uc_priv,
+ plat);
if (ret) {
if (ret == -ENFILE) {
/*
diff --git a/drivers/video/coreboot.c b/drivers/video/coreboot.c
index d2d87c75c89..c586475e41e 100644
--- a/drivers/video/coreboot.c
+++ b/drivers/video/coreboot.c
@@ -57,7 +57,7 @@ static int coreboot_video_probe(struct udevice *dev)
goto err;
}
- ret = vesa_setup_video_priv(vesa, uc_priv, plat);
+ ret = vesa_setup_video_priv(vesa, vesa->phys_base_ptr, uc_priv, plat);
if (ret) {
ret = log_msg_ret("setup", ret);
goto err;
diff --git a/drivers/video/efi.c b/drivers/video/efi.c
index 0479f207032..169637c2882 100644
--- a/drivers/video/efi.c
+++ b/drivers/video/efi.c
@@ -5,6 +5,8 @@
* EFI framebuffer driver based on GOP
*/
+#define LOG_CATEGORY LOGC_EFI
+
#include
#include
#include
@@ -56,11 +58,12 @@ static void efi_find_pixel_bits(u32 mask, u8 *pos, u8 *size)
* Gets info from the graphics-output protocol
*
* @vesa: Place to put the mode information
+ * @fbp: Returns the address of the frame buffer
* @infop: Returns a pointer to the mode info
* Returns: 0 if OK, -ENOSYS if boot services are not available, -ENOTSUPP if
* the protocol is not supported by EFI
*/
-static int get_mode_info(struct vesa_mode_info *vesa,
+static int get_mode_info(struct vesa_mode_info *vesa, u64 *fbp,
struct efi_gop_mode_info **infop)
{
efi_guid_t efi_gop_guid = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID;
@@ -74,9 +77,13 @@ static int get_mode_info(struct vesa_mode_info *vesa,
ret = boot->locate_protocol(&efi_gop_guid, NULL, (void **)&gop);
if (ret)
return log_msg_ret("prot", -ENOTSUPP);
-
mode = gop->mode;
+ log_debug("maxmode %u, mode %u, info %p, size %lx, fb %lx, fb_size
%lx\n",
+ mode->max_mode, mode->mode, mode->info, mode->info_size,
+ (ulong)mode->fb_base, (ulong)mode->fb_size);
+
vesa->phys_base_ptr = mode->fb_base;
+ *fbp = mode->fb_base;
vesa->x_resolution = mode->info->width;
vesa->y_resolution = mode->info->height;
*infop = mode->info;
@@ -91,10 +98,11 @@ static int get_mode_info(struct vesa_mode_info *vesa,
* it into a vesa structure. It also returns the mode information.
*
* @vesa: Place to put the mode information
+ * @fbp: Returns the address of the f
Re: [PATCH v2 04/13] x86: Add a few more items to bdinfo
On 2/22/23 20:12, Simon Glass wrote:
Add the timer and vendor/model information.
Signed-off-by: Simon Glass
---
(no changes since v1)
arch/x86/lib/bdinfo.c | 4
1 file changed, 4 insertions(+)
diff --git a/arch/x86/lib/bdinfo.c b/arch/x86/lib/bdinfo.c
index 0cb79b01bd3..896de37dce4 100644
--- a/arch/x86/lib/bdinfo.c
+++ b/arch/x86/lib/bdinfo.c
@@ -16,6 +16,10 @@ DECLARE_GLOBAL_DATA_PTR;
void arch_print_bdinfo(void)
{
bdinfo_print_num_l("prev table", gd->arch.table);
+ bdinfo_print_num_l("clock_rate", gd->arch.clock_rate);
+ bdinfo_print_num_l("tsc_base", gd->arch.tsc_base);
+ bdinfo_print_num_l("vendor", gd->arch.x86_vendor);
Who would know which vendor uses which number?
Please, use cpu_vendor_name() and present a string.
Best regards
Heinrich
+ bdinfo_print_num_l("model", gd->arch.x86_model);
if (IS_ENABLED(CONFIG_EFI_STUB))
efi_show_bdinfo();
Re: [PATCH v2 05/13] efi: Use a fixed value for the timer clock
On 2/22/23 20:12, Simon Glass wrote:
It is not yet clear how to read the timer via EFI. The current value seems
much too high on a Framework laptop I tried. Adjust it to a lower
hard-coded value for now.
Signed-off-by: Simon Glass
---
(no changes since v1)
drivers/timer/tsc_timer.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
index 192c7b71a5a..1d2a3f20e4e 100644
--- a/drivers/timer/tsc_timer.c
+++ b/drivers/timer/tsc_timer.c
@@ -404,6 +404,10 @@ static void tsc_timer_ensure_setup(bool early)
if (!gd->arch.clock_rate) {
unsigned long fast_calibrate;
+ if (IS_ENABLED(CONFIG_EFI_APP)) {
This needs a code comment telling why you use this 2.75 GHz value.
Why would none of the methods in tsc_timer_ensure_setup() work correctly
in the EFI app?
Best regards
Heinrich
+ fast_calibrate = 2750;
+ goto done;
+ }
fast_calibrate = native_calibrate_tsc();
if (fast_calibrate)
goto done;
Re: [PATCH v2 06/13] efi: Support copy framebuffer
On 2/22/23 20:12, Simon Glass wrote:
Add support for this to EFI in case it becomes useful. At present it just
slows things down. You can enable CONFIG_VIDEO_COPY to turn it on.
Signed-off-by: Simon Glass
---
Changes in v2:
- Obtain copy framebuffer size from EFI instead of using a fixed value
- Dropping debugging printf()
- use new bootph-xxx tag
arch/x86/dts/efi-x86_app.dts | 1 +
drivers/video/efi.c | 25 -
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/arch/x86/dts/efi-x86_app.dts b/arch/x86/dts/efi-x86_app.dts
index 6d843a9820b..59e2e402d5e 100644
--- a/arch/x86/dts/efi-x86_app.dts
+++ b/arch/x86/dts/efi-x86_app.dts
@@ -27,6 +27,7 @@
};
efi-fb {
compatible = "efi-fb";
+ bootph-some-ram;
};
};
diff --git a/drivers/video/efi.c b/drivers/video/efi.c
index 169637c2882..a8e9282ab87 100644
--- a/drivers/video/efi.c
+++ b/drivers/video/efi.c
@@ -129,7 +129,6 @@ static int save_vesa_mode(struct vesa_mode_info *vesa, u64
*fbp)
struct efi_gop_mode_info *info;
int ret;
- printf("start\n");
if (IS_ENABLED(CONFIG_EFI_APP))
ret = get_mode_info(vesa, fbp, &info);
else
@@ -207,6 +206,29 @@ err:
return ret;
}
+static int efi_video_bind(struct udevice *dev)
+{
+ if (IS_ENABLED(CONFIG_VIDEO_COPY)) {
+ struct video_uc_plat *plat = dev_get_uclass_plat(dev);
+ struct vesa_mode_info vesa;
struct vesa_mode_info vesa = {0};
see below
+ int ret;
+ u64 fb;
+
+ /*
+* Initialise vesa_mode_info structure so we can figure out the
+* required framebuffer size. If something goes wrong, just do
+* without a copy framebuffer
+*/
+ ret = save_vesa_mode(&vesa, &fb);
+ if (!ret) {
+ plat->copy_size = vesa.bytes_per_scanline *
+ vesa.y_resolution;
If there is no EFI_GRAPHICS_OUTPUT_PROTOCOL, ret = -ENXIO but vesa is
*not* initialized. You don't want to use random bytes from the stack to
calculate copy_size and afterwards write into a non-existent framebuffer.
If you initialize vesa to zero, you can test if copy_size == 0 and in
this case return 1 to indicate that binding failed.
Best regards
Heinrich
+ }
+ }
+
+ return 0;
+}
+
static const struct udevice_id efi_video_ids[] = {
{ .compatible = "efi-fb" },
{ }
@@ -216,5 +238,6 @@ U_BOOT_DRIVER(efi_video) = {
.name = "efi_video",
.id = UCLASS_VIDEO,
.of_match = efi_video_ids,
+ .bind = efi_video_bind,
.probe = efi_video_probe,
};
Re: bootefi issue
On 2/23/23 14:38, Alexandre Ghiti wrote:
Hi Heinrich,
I fell into an issue in u-boot, and I struggle to find the correct
fix: I'm loading a kernel of 66MB at kernel_addr_r (=0x8400_, this
is the default value) and then I'm booting it using bootefi. But the
loaded kernel is overwritten by the fdt because u-boot loads the dtb
at 0x87f0_
(https://elixir.bootlin.com/u-boot/v2023.01/source/cmd/bootefi.c#L211:
I guess efi_allocate_pages does not fail because the efi allocator is
not aware of the kernel that was loaded there). So I have multiple
fixes and don't know which one is right:
1. bootefi could load the kernel first and then the fdt (so that the
efi allocator fails at 0x87f0_)
2. check before efi_allocate_pages that the "normal" allocator is free
there (I tried malloc_usable_size but it fails).
3. decrease kernel_addr_r...But I would disagree :)
Any idea?
Thanks,
Alex
Hello Alexandre,
this address for copying the device-tree dates back to 2016-04-11 when
AllocatePages() didn't work properly.
For riscv64 I see no reason why we should not use a high address.
@Ilias
Does ARM have any problem with the device-tree being in high memory?
Otherwise we should make a change as in the diff below.
Best regards
Heinrich
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 6618335ddf..08a0cfa764 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -210,19 +210,12 @@ static efi_status_t copy_fdt(void **fdtp)
*/
new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f0 +
fdt_size, 0);
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
+ ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
&new_fdt_addr);
if (ret != EFI_SUCCESS) {
- /* If we can't put it there, put it somewhere */
- new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
-&new_fdt_addr);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: Failed to reserve space for FDT\n");
- goto done;
- }
+ log_err("ERROR: Failed to reserve space for FDT\n");
+ goto done;
}
new_fdt = (void *)(uintptr_t)new_fdt_addr;
memcpy(new_fdt, fdt, fdt_totalsize(fdt));
Re: bootefi issue
On 2/23/23 16:57, Heinrich Schuchardt wrote:
On 2/23/23 14:38, Alexandre Ghiti wrote:
Hi Heinrich,
I fell into an issue in u-boot, and I struggle to find the correct
fix: I'm loading a kernel of 66MB at kernel_addr_r (=0x8400_, this
is the default value) and then I'm booting it using bootefi. But the
loaded kernel is overwritten by the fdt because u-boot loads the dtb
at 0x87f0_
(https://elixir.bootlin.com/u-boot/v2023.01/source/cmd/bootefi.c#L211:
I guess efi_allocate_pages does not fail because the efi allocator is
not aware of the kernel that was loaded there). So I have multiple
fixes and don't know which one is right:
1. bootefi could load the kernel first and then the fdt (so that the
efi allocator fails at 0x87f0_)
2. check before efi_allocate_pages that the "normal" allocator is free
there (I tried malloc_usable_size but it fails).
3. decrease kernel_addr_r...But I would disagree :)
Any idea?
Thanks,
Alex
Hello Alexandre,
this address for copying the device-tree dates back to 2016-04-11 when
AllocatePages() didn't work properly.
For riscv64 I see no reason why we should not use a high address.
@Ilias
Does ARM have any problem with the device-tree being in high memory?
Otherwise we should make a change as in the diff below.
Best regards
Heinrich
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 6618335ddf..08a0cfa764 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -210,19 +210,12 @@ static efi_status_t copy_fdt(void **fdtp)
*/
new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f0 +
fdt_size, 0);
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
+ ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
&new_fdt_addr);
if (ret != EFI_SUCCESS) {
- /* If we can't put it there, put it somewhere */
- new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
- EFI_ACPI_RECLAIM_MEMORY,
fdt_pages,
- &new_fdt_addr);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: Failed to reserve space for
FDT\n");
- goto done;
- }
+ log_err("ERROR: Failed to reserve space for FDT\n");
+ goto done;
}
new_fdt = (void *)(uintptr_t)new_fdt_addr;
memcpy(new_fdt, fdt, fdt_totalsize(fdt));
Ard gave me this information:
When booting via UEFI the device-tree is copied by the EFI stub to an
appropriate location. There are no restriction for U-Boot's choice.
For legacy booting ARMv7 before
ARM: 9012/1: move device tree mapping out of linear region
7a1be318f5795cb66fa0dc86b3ace427fe68057f
the device-tree had to be placed in a linear region of customizable
size. The general recommendation was to place the device-tree in the
second 128 MiB block.
I will convert the diff above into a patch.
Best regards
Heinrich
Re: [PATCH] lib: bzip2: remove inlining
On 2/23/23 17:30, Tom Rini wrote: On Thu, Feb 23, 2023 at 08:15:25AM +0100, Heinrich Schuchardt wrote: Compiling sandbox_defconfig with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y and gcc 12.2.0-14ubuntu1 leads to a build error: lib/bzip2/bzlib.c: In function 'BZ2_decompress': lib/bzip2/bzlib.c:726:18: error: inlining failed in call to 'always_inline' 'BZ2_indexIntoF': function not considered for inlining 726 | __inline__ Int32 BZ2_indexIntoF ( Int32 indx, Int32 *cftab ) | ^ Leave it to the compiler if it inlines or not. Signed-off-by: Heinrich Schuchardt What did previous compilers do here? If we're telling the compiler to always inline, presumably for good reason, we shouldn't just stop. Inlining may make the code a bit faster. But without inlining it would be smaller. Grep for BZ_GET_SMALL to find where the inlined function is used. In test/compression.c we check the result. 'ut compression' does not find a problem. Best regards Heinrich
Re: bootefi issue
On 2/23/23 17:24, Alexandre Ghiti wrote:
On Thu, Feb 23, 2023 at 4:57 PM Heinrich Schuchardt
wrote:
On 2/23/23 14:38, Alexandre Ghiti wrote:
Hi Heinrich,
I fell into an issue in u-boot, and I struggle to find the correct
fix: I'm loading a kernel of 66MB at kernel_addr_r (=0x8400_, this
is the default value) and then I'm booting it using bootefi. But the
loaded kernel is overwritten by the fdt because u-boot loads the dtb
at 0x87f0_
(https://elixir.bootlin.com/u-boot/v2023.01/source/cmd/bootefi.c#L211:
I guess efi_allocate_pages does not fail because the efi allocator is
not aware of the kernel that was loaded there). So I have multiple
fixes and don't know which one is right:
1. bootefi could load the kernel first and then the fdt (so that the
efi allocator fails at 0x87f0_)
2. check before efi_allocate_pages that the "normal" allocator is free
there (I tried malloc_usable_size but it fails).
3. decrease kernel_addr_r...But I would disagree :)
Any idea?
Thanks,
Alex
Hello Alexandre,
this address for copying the device-tree dates back to 2016-04-11 when
AllocatePages() didn't work properly.
For riscv64 I see no reason why we should not use a high address.
@Ilias
Does ARM have any problem with the device-tree being in high memory?
Otherwise we should make a change as in the diff below.
Best regards
Heinrich
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 6618335ddf..08a0cfa764 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -210,19 +210,12 @@ static efi_status_t copy_fdt(void **fdtp)
*/
new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f0 +
fdt_size, 0);
I guess we can entirely remove this ^ right?
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
+ ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
&new_fdt_addr);
if (ret != EFI_SUCCESS) {
- /* If we can't put it there, put it somewhere */
- new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
-&new_fdt_addr);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: Failed to reserve space for FDT\n");
- goto done;
- }
+ log_err("ERROR: Failed to reserve space for FDT\n");
+ goto done;
}
new_fdt = (void *)(uintptr_t)new_fdt_addr;
memcpy(new_fdt, fdt, fdt_totalsize(fdt));
What would prevent the efi allocator to return an address that
overwrites the loaded kernel?
Our AllocatePages() assigns high memory when called with
EFI_ALLOCATE_ANY_PAGES.
Currently we have a gap in U-Boot. Memory is not allocated by the 'load'
command. We need to merge the EFI memory allocator and lib/lmb.c.
A quick fix has been commit 06d514d77c37 ("lmb: consider EFI memory
map") but that does not cover everything.
Best regards
Heinrich
[PATCH 1/1] cmd: bootefi: allocate device-tree copy from high memory
The bootefi command creates a copy of the device-tree within the first
127 MiB of memory. This may lead to overwriting previously loaded binaries
(e.g. kernel, initrd).
Linux EFI stub itself copies U-Boot's copy of the device-tree. This means
there is not restriction for U-Boot to place the device-tree copy to any
address. (Restrictions existed for 32bit ARM before Linux commit
7a1be318f579 ("ARM: 9012/1: move device tree mapping out of linear region")
for legacy booting.
Reported-by: Alexandre Ghiti
Signed-off-by: Heinrich Schuchardt
---
cmd/bootefi.c | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 6618335ddf..aca4e99930 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -208,21 +208,12 @@ static efi_status_t copy_fdt(void **fdtp)
* Safe fdt location is at 127 MiB.
* On the sandbox convert from the sandbox address space.
*/
- new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f0 +
-fdt_size, 0);
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
+ ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
&new_fdt_addr);
if (ret != EFI_SUCCESS) {
- /* If we can't put it there, put it somewhere */
- new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
-&new_fdt_addr);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: Failed to reserve space for FDT\n");
- goto done;
- }
+ log_err("ERROR: Failed to reserve space for FDT\n");
+ goto done;
}
new_fdt = (void *)(uintptr_t)new_fdt_addr;
memcpy(new_fdt, fdt, fdt_totalsize(fdt));
--
2.38.1
[PATCH v2 1/1] cmd: bootefi: allocate device-tree copy from high memory
The bootefi command creates a copy of the device-tree within the first
127 MiB of memory. This may lead to overwriting previously loaded binaries
(e.g. kernel, initrd).
Linux EFI stub itself copies U-Boot's copy of the device-tree. This means
there is not restriction for U-Boot to place the device-tree copy to any
address. (Restrictions existed for 32bit ARM before Linux commit
7a1be318f579 ("ARM: 9012/1: move device tree mapping out of linear region")
for legacy booting.
Reported-by: Alexandre Ghiti
Signed-off-by: Heinrich Schuchardt
Tested-by: Alexandre Ghiti
---
v2:
remove superfluous comment
---
cmd/bootefi.c | 19 +++
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 6618335ddf..8aa15a64c8 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -204,25 +204,12 @@ static efi_status_t copy_fdt(void **fdtp)
fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
fdt_size = fdt_pages << EFI_PAGE_SHIFT;
- /*
-* Safe fdt location is at 127 MiB.
-* On the sandbox convert from the sandbox address space.
-*/
- new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f0 +
-fdt_size, 0);
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
+ ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
&new_fdt_addr);
if (ret != EFI_SUCCESS) {
- /* If we can't put it there, put it somewhere */
- new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
-&new_fdt_addr);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: Failed to reserve space for FDT\n");
- goto done;
- }
+ log_err("ERROR: Failed to reserve space for FDT\n");
+ goto done;
}
new_fdt = (void *)(uintptr_t)new_fdt_addr;
memcpy(new_fdt, fdt, fdt_totalsize(fdt));
--
2.38.1
[PATCH v2 1/1] cmd: bootefi: allocate device-tree copy from high memory
The bootefi command creates a copy of the device-tree within the first
127 MiB of memory. This may lead to overwriting previously loaded binaries
(e.g. kernel, initrd).
Linux EFI stub itself copies U-Boot's copy of the device-tree. This means
there is not restriction for U-Boot to place the device-tree copy to any
address. (Restrictions existed for 32bit ARM before Linux commit
7a1be318f579 ("ARM: 9012/1: move device tree mapping out of linear region")
for legacy booting.
Reported-by: Alexandre Ghiti
Signed-off-by: Heinrich Schuchardt
Tested-by: Alexandre Ghiti
---
v2:
remove superfluous comment
---
cmd/bootefi.c | 19 +++
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 6618335ddf..8aa15a64c8 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -204,25 +204,12 @@ static efi_status_t copy_fdt(void **fdtp)
fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
fdt_size = fdt_pages << EFI_PAGE_SHIFT;
- /*
-* Safe fdt location is at 127 MiB.
-* On the sandbox convert from the sandbox address space.
-*/
- new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f0 +
-fdt_size, 0);
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
+ ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
&new_fdt_addr);
if (ret != EFI_SUCCESS) {
- /* If we can't put it there, put it somewhere */
- new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
- ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
-&new_fdt_addr);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: Failed to reserve space for FDT\n");
- goto done;
- }
+ log_err("ERROR: Failed to reserve space for FDT\n");
+ goto done;
}
new_fdt = (void *)(uintptr_t)new_fdt_addr;
memcpy(new_fdt, fdt, fdt_totalsize(fdt));
--
2.38.1
Re: [PATCH] lib: bzip2: remove inlining
On 2/23/23 17:40, Tom Rini wrote: On Thu, Feb 23, 2023 at 05:39:08PM +0100, Heinrich Schuchardt wrote: On 2/23/23 17:30, Tom Rini wrote: On Thu, Feb 23, 2023 at 08:15:25AM +0100, Heinrich Schuchardt wrote: Compiling sandbox_defconfig with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y and gcc 12.2.0-14ubuntu1 leads to a build error: lib/bzip2/bzlib.c: In function 'BZ2_decompress': lib/bzip2/bzlib.c:726:18: error: inlining failed in call to 'always_inline' 'BZ2_indexIntoF': function not considered for inlining 726 | __inline__ Int32 BZ2_indexIntoF ( Int32 indx, Int32 *cftab ) | ^ Leave it to the compiler if it inlines or not. Signed-off-by: Heinrich Schuchardt What did previous compilers do here? If we're telling the compiler to always inline, presumably for good reason, we shouldn't just stop. Inlining may make the code a bit faster. But without inlining it would be smaller. Grep for BZ_GET_SMALL to find where the inlined function is used. In test/compression.c we check the result. 'ut compression' does not find a problem. OK, and I'm wondering if the compiler regressed. CONFIG_CC_OPTIMIZE_FOR_DEBUG is not used in any of our defconfigs and has been introduced long after bzip2. Typically CONFIG_CC_OPTIMIZE_FOR_DEBUG is used in conjunction with CONFIG_LTO=n. In this case the error does not occur. It seems that gcc with -Og -flto has an issue. Best regards Heinrich
Pull request for efi-2023-04-rc3-2
The following changes since commit 8c3acb726ef083d3d5de12f20318ee0e5070: Merge branch 'master' of https://source.denx.de/u-boot/custodians/u-boot-usb (2023-02-22 13:36:29 -0500) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2023-04-rc3-2 for you to fetch changes up to 0bb5d6b1e4105ec4215239958830a396658bfed8: cmd: bootefi: allocate device-tree copy from high memory (2023-02-24 07:44:35 +0100) Gitlab CI showed no issues: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15340 Pull request for efi-2023-04-rc3-2 Documentation: Fix links to fwu_updates.rst UEFI: Allocate device-tree copy from high memory in bootefi command Correct attributes checks in SetVariable() Other: Allow SPL to load main U-Boot via partition type GUID Test case for crc8 function ---- Heinrich Schuchardt (5): disk: accessors for conditional partition fields kconfig: new macro IF_ENABLED() spl: allow loading via partition type GUID test: unit test for crc8 cmd: bootefi: allocate device-tree copy from high memory Masahisa Kojima (1): efi_loader: update SetVariable attribute check Vincent Stehlé (1): doc: uefi: fix links cmd/bootefi.c| 19 +++ common/spl/Kconfig | 27 ++- common/spl/spl_mmc.c | 18 ++ doc/develop/uefi/fwu_updates.rst | 3 ++- doc/develop/uefi/uefi.rst| 4 ++-- include/linux/kconfig.h | 7 +++ include/part.h | 36 lib/efi_loader/efi_variable.c| 31 +-- test/lib/Makefile| 1 + test/lib/test_crc8.c | 29 + 10 files changed, 145 insertions(+), 30 deletions(-) create mode 100644 test/lib/test_crc8.c
Re: Pull request for efi-2023-04-rc3-2
On 2/24/23 15:42, Tom Rini wrote: On Fri, Feb 24, 2023 at 11:02:57AM +0100, Heinrich Schuchardt wrote: The following changes since commit 8c3acb726ef083d3d5de12f20318ee0e5070: Merge branch 'master' of https://source.denx.de/u-boot/custodians/u-boot-usb (2023-02-22 13:36:29 -0500) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2023-04-rc3-2 for you to fetch changes up to 0bb5d6b1e4105ec4215239958830a396658bfed8: cmd: bootefi: allocate device-tree copy from high memory (2023-02-24 07:44:35 +0100) Gitlab CI showed no issues: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15340 Pull request for efi-2023-04-rc3-2 Documentation: Fix links to fwu_updates.rst This is a fix. UEFI: Allocate device-tree copy from high memory in bootefi command This I think counts as a fix, but I'm a little confused about allocate in this context. Can you point me at the patch please? Correct attributes checks in SetVariable() This is a fix.. Other: Allow SPL to load main U-Boot via partition type GUID This is new functionality, and I didn't quite see that everyone agreed this was a good idea? Not for master at this point in the cycle. Would you take it for next? Best regards Heinrich Test case for crc8 function This I suppose can count as a fix.
Re: [PATCH v3 1/1] editorconfig: introduce .editorconfig
On 3/3/23 16:33, Dzmitry Sankouski wrote:
Current process of sending patches includes running checkpatch.pl
script for each patch, and fixing found style problems.
EditorConfig may help to prevent some style related problems
(like spaces vs tab indentation) on the fly.
Reviewed-by: Simon Glass
Signed-off-by: Dzmitry Sankouski
---
Changes in v3:
- add 'the' article in docs
- fix spacing
- add sign off tag
Changes in v2:
- add section in coding style rst doc
- unify Kconfig with other files
.editorconfig | 15 +++
.gitignore | 1 +
doc/develop/codingstyle.rst | 4
3 files changed, 20 insertions(+)
create mode 100644 .editorconfig
diff --git a/.editorconfig b/.editorconfig
new file mode 100644
index 00..df69cee160
--- /dev/null
+++ b/.editorconfig
@@ -0,0 +1,15 @@
+; This file is for unifying the coding style for different editors and IDEs.
+; Plugins are available for notepad++, emacs, vim, gedit,
+; textmate, visual studio, and more.
+;
+; See http://editorconfig.org for details.
+
+# Top-most EditorConfig file.
+root = true
+
+[{**.c, **.h, **Kconfig}]
+indent_style = tab
+indent_size = 8
+end_of_line = lf
+trim_trailing_whitespace = true
+insert_final_newline = true
For Python we follow PEP8 and use 4 spaces per indent. We use the same
for *.rst. Could you, please, add such a rule (possibly in a follow up
patch).
diff --git a/.gitignore b/.gitignore
index 3a4d056edf..ed8ca226fe 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,6 +7,7 @@
#
.*
!.checkpatch.conf
+!.editorconfig
Why would you want to ignore the file?
Best regards
Heinrich
*.a
*.asn1.[ch]
*.bin
diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst
index 1d5d0192b3..0bbac75d4e 100644
--- a/doc/develop/codingstyle.rst
+++ b/doc/develop/codingstyle.rst
@@ -27,6 +27,10 @@ The following rules apply:
more information, read :doc:`checkpatch`. Note that this should be done
*before* posting on the mailing list!
+* Some code style rules may be applied automatically by your editor using
+ the EditorConfig tool. Feel free to setup your editor to work with u-boot's
+ .editorconfig.
+
* Source files originating from different projects (for example the MTD
subsystem or the hush shell code from the BusyBox project) may, after
careful consideration, be exempted from these rules. For such files, the
Re: [PATCH v8 4/6] bootm: Support boot measurement
On 3/3/23 20:25, Eddie James wrote:
Add a configuration option to measure the boot through the bootm
function. Add the measurement state to the booti and bootz paths
as well.
Signed-off-by: Eddie James
Reviewed-by: Simon Glass
---
Changes since v6:
- Added comment for bootm_measure
- Fixed line length in bootm_measure
boot/Kconfig| 23
boot/bootm.c| 72 +
cmd/booti.c | 1 +
cmd/bootm.c | 2 ++
cmd/bootz.c | 1 +
include/bootm.h | 11
include/image.h | 1 +
7 files changed, 111 insertions(+)
diff --git a/boot/Kconfig b/boot/Kconfig
index 5f491625c8..d0d5e5794c 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -629,6 +629,29 @@ config LEGACY_IMAGE_FORMAT
loaded. If a board needs the legacy image format support in this
case, enable it here.
+config MEASURED_BOOT
+ bool "Measure boot images and configuration to TPM and event log"
+ depends on HASH && TPM_V2
+ help
+ This option enables measurement of the boot process. Measurement
+ involves creating cryptographic hashes of the binary images that
+ are booting and storing them in the TPM. In addition, a log of
+ these hashes is stored in memory for the OS to verify the booted
+ images and configuration. Enable this if the OS has configured
+ some memory area for the event log and you intend to use some
It cannot be the OS that configures the memory area for the measurement
as we don't even know which OSes will be booted at configuration time.
It must be the device-tree that defines if such a memory area exists and
where it is located.
Some of the OSes booted by U-Boot may have a driver to read such a
memory area. OSes lacking such a driver will ignore that memory area
which does not stop them from booting.
+ attestation tools on your system.
@Ilias:
This description does not make it clear that this setting does not
enable measured boot for the EFI sub-system. We will need some follow up
patch to point to the EFI relevant setting.
Best regards
Heinrich
+
+if MEASURED_BOOT
+ config MEASURE_DEVICETREE
+ bool "Measure the devicetree image"
+ default y if MEASURED_BOOT
+ help
+ On some platforms, the devicetree is not static as it may contain
+ random MAC addresses or other such data that changes each boot.
+ Therefore, it should not be measured into the TPM. In that case,
+ disable the measurement here.
+endif # MEASURED_BOOT
+
config SUPPORT_RAW_INITRD
bool "Enable raw initrd images"
help
diff --git a/boot/bootm.c b/boot/bootm.c
index 2eec60ec7b..e3ef18166d 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#if defined(CONFIG_CMD_USB)
#include
#endif
@@ -659,6 +660,73 @@ int bootm_process_cmdline_env(int flags)
return 0;
}
+int bootm_measure(struct bootm_headers *images)
+{
+ int ret = 0;
+
+ /* Skip measurement if EFI is going to do it */
+ if (images->os.os == IH_OS_EFI &&
+ IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL) &&
+ IS_ENABLED(CONFIG_BOOTM_EFI))
+ return ret;
+
+ if (IS_ENABLED(CONFIG_MEASURED_BOOT)) {
+ struct tcg2_event_log elog;
+ struct udevice *dev;
+ void *initrd_buf;
+ void *image_buf;
+ const char *s;
+ u32 rd_len;
+
+ elog.log_size = 0;
+ ret = tcg2_measurement_init(&dev, &elog);
+ if (ret)
+ return ret;
+
+ image_buf = map_sysmem(images->os.image_start,
+ images->os.image_len);
+ ret = tcg2_measure_data(dev, &elog, 8, images->os.image_len,
+ image_buf, EV_COMPACT_HASH,
+ strlen("linux") + 1, (u8 *)"linux");
+ if (ret)
+ goto unmap_image;
+
+ rd_len = images->rd_end - images->rd_start;
+ initrd_buf = map_sysmem(images->rd_start, rd_len);
+ ret = tcg2_measure_data(dev, &elog, 9, rd_len, initrd_buf,
+ EV_COMPACT_HASH, strlen("initrd") + 1,
+ (u8 *)"initrd");
+ if (ret)
+ goto unmap_initrd;
+
+ if (IS_ENABLED(CONFIG_MEASURE_DEVICETREE)) {
+ ret = tcg2_measure_data(dev, &elog, 0, images->ft_len,
+ (u8 *)images->ft_addr,
+ EV_TABLE_OF_DEVICES,
+ strlen("dts") + 1,
+ (u8 *)"dts");
+ if (ret)
+ goto
Re: [PATCH v8 6/6] doc: Add measured boot documentation
On 3/3/23 20:25, Eddie James wrote: Briefly describe the feature and specify the requirements. Signed-off-by: Eddie James Reviewed-by: Simon Glass --- doc/usage/index.rst | 1 + doc/usage/measured_boot.rst | 23 +++ 2 files changed, 24 insertions(+) create mode 100644 doc/usage/measured_boot.rst diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 840c20c934..1cb6988d8a 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -12,6 +12,7 @@ Use U-Boot partitions cmdline semihosting + measured_boot Shell commands -- diff --git a/doc/usage/measured_boot.rst b/doc/usage/measured_boot.rst new file mode 100644 index 00..8357b1f480 --- /dev/null +++ b/doc/usage/measured_boot.rst @@ -0,0 +1,23 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Measured Boot += + +U-Boot can perform a measured boot, the process of hashing various components This text is misleading: U-Boot does not perform measured boot. U-Boot performing measured boot would imply that it checks if the incoming values of the PCRs match expected values. This is not what is done with this series. U-Boot assists measured boot by extending PCRs and providing an event log. Best regards Heinrich +of the boot process, extending the results in the TPM and logging the +component's measurement in memory for the operating system to consume. + +Requirements +- + +* A hardware TPM 2.0 supported by the U-Boot drivers +* CONFIG_TPM=y +* CONFIG_MEASURED_BOOT=y +* Device-tree configuration of the TPM device to specify the memory area + for event logging. The TPM device node must either contain a phandle to + a reserved memory region or "linux,sml-base" and "linux,sml-size" + indicating the address and size of the memory region. An example can be + found in arch/sandbox/dts/test.dts +* The operating system must also be configured to use the memory regions + specified in the U-Boot device-tree in order to make use of the event + log.
[PATCH 1/1] efi_loader: describe term_get_char()
Add a function description.
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_console.c | 8
1 file changed, 8 insertions(+)
diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index 4317630907..d970b667a6 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -77,6 +77,14 @@ static struct simple_text_output_mode efi_con_mode = {
.cursor_visible = 1,
};
+/**
+ * term_get_char() - read a character from the console
+ *
+ * Wait for up to 100 ms to read a character from the console.
+ *
+ * @c: pointer to the buffer to receive the character
+ * Return: 0 on success, 1 otherwise
+ */
static int term_get_char(s32 *c)
{
u64 timeout;
--
2.39.2
[PATCH 1/1] doc: man-page for panic command
Provide a man-page for the panic command. Signed-off-by: Heinrich Schuchardt --- doc/usage/cmd/panic.rst | 33 + doc/usage/index.rst | 1 + 2 files changed, 34 insertions(+) create mode 100644 doc/usage/cmd/panic.rst diff --git a/doc/usage/cmd/panic.rst b/doc/usage/cmd/panic.rst new file mode 100644 index 00..115eba5bde --- /dev/null +++ b/doc/usage/cmd/panic.rst @@ -0,0 +1,33 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +panic command += + +Synopis +--- + +:: + +panic [message] + +Description +--- + +Display a message and reset the board. + +message +text to be displayed + +Examples + + +:: + +=> panic 'Unrecoverable error' +Unrecoverable error +resetting ... + +Configuration +- + +If CONFIG_PANIC_HANG=y, the user has to reset the board manually. diff --git a/doc/usage/index.rst b/doc/usage/index.rst index cde7dcb14a..aecb6cb762 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -64,6 +64,7 @@ Shell commands cmd/md cmd/mmc cmd/mtest + cmd/panic cmd/part cmd/pause cmd/pinmux -- 2.39.2
[PATCH 1/1] api: move API related config options into submenu
Kconfig settings that are related to the API for standalone applications should be in the API sub-menu and not on the top level. CONFIG_STANDALONE_LOAD_ADDR is only relevant if standalone example applications are built. Signed-off-by: Heinrich Schuchardt --- Kconfig | 8 api/Kconfig | 11 ++- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Kconfig b/Kconfig index a75cce7e28..e24376765d 100644 --- a/Kconfig +++ b/Kconfig @@ -602,14 +602,6 @@ config MP This provides an option to bringup different processors in multiprocessor cases. -config EXAMPLES - bool "Compile API examples" - depends on !SANDBOX - default y if ARCH_QEMU - help - U-Boot provides an API for standalone applications. Examples are - provided in directory examples/. - endmenu# General setup source "api/Kconfig" diff --git a/api/Kconfig b/api/Kconfig index d9362724e5..6072288f9b 100644 --- a/api/Kconfig +++ b/api/Kconfig @@ -10,9 +10,16 @@ config SYS_MMC_MAX_DEVICE depends on API default 1 -endmenu +config EXAMPLES + bool "Compile API examples" + depends on !SANDBOX + default y if ARCH_QEMU + help + U-Boot provides an API for standalone applications. Examples are + provided in directory examples/. config STANDALONE_LOAD_ADDR + depends on EXAMPLES hex "Address in memory to link standalone applications to" default 0x8020 if MIPS && 64BIT default 0x8c00 if SH @@ -30,3 +37,5 @@ config STANDALONE_LOAD_ADDR This option defines a board specific value for the address where standalone program gets loaded, thus overwriting the architecture dependent default settings. + +endmenu -- 2.39.2
Re: [PATCH 09/10] efi: Support showing tables
On 2/26/23 02:33, Simon Glass wrote:
Add a command to display the tables provided by EFI.
Signed-off-by: Simon Glass
---
cmd/efi.c | 40 +++-
doc/usage/cmd/efi.rst | 22 ++
2 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/cmd/efi.c b/cmd/efi.c
index c0384e0db28..86988d9e9e2 100644
--- a/cmd/efi.c
+++ b/cmd/efi.c
@@ -7,10 +7,12 @@
#include
#include
#include
+#include
#include
#include
#include
#include
+#include
#include
DECLARE_GLOBAL_DATA_PTR;
@@ -273,8 +275,43 @@ done:
return ret ? CMD_RET_FAILURE : 0;
}
+static int do_efi_tables(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
We already have function do_efi_show_tables() to print the list of EFI
configuration tables. Please, avoid code duplication.
+{
+ struct efi_system_table *systab;
+ int i;
+
+ if (IS_ENABLED(CONFIG_EFI_APP)) {
+ systab = efi_get_sys_table();
+ if (!systab) {
+ printf("Cannot read system table\n");
+ return CMD_RET_FAILURE;
+ }
+ } else {
+ int size;
+ int ret;
+
+ ret = efi_info_get(EFIET_SYS_TABLE, (void **)&systab, &size);
+ if (ret) {
+ printf("Cannot find EFI system table (err=%d)\n", ret);
+ return CMD_RET_FAILURE;
+ }
+ }
+ for (i = 0; i < systab->nr_tables; i++) {
+ struct efi_configuration_table *tab = &systab->tables[i];
+ char guid_str[37];
+
+ uuid_bin_to_str(tab->guid.b, guid_str, 1);
+ printf("%p %s %s\n", tab->table, guid_str,
+ uuid_guid_get_str(tab->guid.b));
do_efi_show_tables() does this with printf("%pUl (%pUs)\n",..).
+ }
+
+ return 0;
+}
+
static struct cmd_tbl efi_commands[] = {
U_BOOT_CMD_MKENT(mem, 1, 1, do_efi_mem, "", ""),
+ U_BOOT_CMD_MKENT(tables, 1, 1, do_efi_tables, "", ""),
};
static int do_efi(struct cmd_tbl *cmdtp, int flag, int argc, char *const
argv[])
@@ -298,5 +335,6 @@ static int do_efi(struct cmd_tbl *cmdtp, int flag, int
argc, char *const argv[])
U_BOOT_CMD(
efi, 3, 1, do_efi,
"EFI access",
- "mem [all]Dump memory information [include boot services]"
+ "mem [all]Dump memory information [include boot services]\n"
+ "tables Dump tables"
);
diff --git a/doc/usage/cmd/efi.rst b/doc/usage/cmd/efi.rst
index c029c423879..2424d3bb587 100644
--- a/doc/usage/cmd/efi.rst
+++ b/doc/usage/cmd/efi.rst
@@ -10,6 +10,7 @@ Synopsis
::
efi mem [all]
+efi tables
Description
---
@@ -54,6 +55,14 @@ Attributes
Shows a code for memory attributes. The key for this is shown below the
table.
+efi tables
+~~
+
+This shows a list of the EFI tables provided in the system table. These use
+GUIDs so it is not possible in general to show the name of a table. But some
+effort is made to provide a useful table, where the GUID is known by U-Boot.
+
+
Example
---
@@ -195,3 +204,16 @@ Example
f: uncached, write-coalescing, write-through, write-back
rf: uncached, write-coalescing, write-through, write-back, needs runtime
mapping
1: uncached
+
+
+=> efi tables
+1f8eda98 ee4e5898-3914-4259-9d6e-dc7bd79403cf EFI_LZMA_COMPRESSED
+1ff2ace0 05ad34ba-6f02-4214-952e-4da0398e2bb9 EFI_DXE_SERVICES
+1f8ea018 7739f24c-93d7-11d4-9a3a-0090273fc14d EFI_HOB_LIST
+1ff2bac0 4c19049f-4137-4dd3-9c10-8b97a83ffdfa EFI_MEMORY_TYPE
+1ff2cb10 49152e77-1ada-4764-b7a2-7afefed95e8b
This is what 'efi tables' / printf("%pUl (%pUs)\n",..). would output for
an unknown table:
8868e871-e4f1-11d3-bc22-0080c73c8881 (8868e871-e4f1-11d3-bc22-0080c73c8881)
Maybe uuid_guid_get_str() should return an empty string in case of an
unknown GUID.
+1f9ac018 060cc026-4c0d-4dda-8f41-595fef00a502
EFI_MEM_STATUS_CODE_REC
+1f9ab000 eb9d2d31-2d88-11d3-9a16-0090273fc14d EFI_SMBIOS
+1fb7e000 eb9d2d30-2d88-11d3-9a16-0090273fc14d EFI_GUID_EFI_ACPI1
+1fb7e014 8868e871-e4f1-11d3-bc22-0080c73c8881 EFI_GUID_EFI_ACPI2
+1e550018 dcfa911d-26eb-469f-a220-38b7dc461220
Who would care about the address of the table?
Best regards
Heinrich
Re: [PATCH 1/1] api: move API related config options into submenu
On 3/4/23 16:32, Tom Rini wrote: On Fri, Mar 03, 2023 at 11:31:22PM +0100, Heinrich Schuchardt wrote: Kconfig settings that are related to the API for standalone applications should be in the API sub-menu and not on the top level. CONFIG_STANDALONE_LOAD_ADDR is only relevant if standalone example applications are built. Signed-off-by: Heinrich Schuchardt --- Kconfig | 8 api/Kconfig | 11 ++- 2 files changed, 10 insertions(+), 9 deletions(-) Did you put this through CI? It's possible that some envs don't do "loadaddr=CONFIG_STANDALONE_LOAD_ADDR" and not enable API stuff anymore, but I think that's why I did what I did when migrating. Hello Tom, we should keep the main Kconfig menu clean of detail settings. I don't thin that there is an issue with the current patch. STANDALONE_LOAD_ADDR is not used for loadaddr: $ git grep -n STANDALONE_LOAD_ADDR (based on origin/master) api/Kconfig:15 config STANDALONE_LOAD_ADDR config.mk:79 export CONFIG_STANDALONE_LOAD_ADDR configs/display5_defconfig:33 CONFIG_STANDALONE_LOAD_ADDR=0x10001000 configs/display5_factory_defconfig:30 CONFIG_STANDALONE_LOAD_ADDR=0x10001000 configs/microchip_mpfs_icicle_defconfig:15 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv32_defconfig:12 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv32_smode_defconfig:13 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv32_spl_defconfig:15 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv64_defconfig:12 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv64_smode_defconfig:13 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv64_spl_defconfig:14 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/sifive_unleashed_defconfig:21 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/sifive_unmatched_defconfig:24 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/xtfpga_defconfig:12 CONFIG_STANDALONE_LOAD_ADDR=0x0080 examples/standalone/Makefile:45 LDFLAGS_STANDALONE += -Ttext $(CONFIG_STANDALONE_LOAD_ADDR) tools/patman/test_checkpatch.py:208 CONFIG_STANDALONE_LOAD_ADDR With the patch applied https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15474 showed no issues. Best regards Heinrich
qemu_arm_defconfig with LTO fails due to unaligned access
Hello Ilias, hello Tom, Tom tried to run qemu_arm_defconfig with CONFIG_LTO=y in gitlab. This failed as shown in protocol https://source.denx.de/u-boot/u-boot/-/jobs/589913/raw Executing 'HII database protocols' test_hii_database_new_package_list: data abort pc : [<7ff39b98>] lr : [<7ff87328>] reloc pc : [<0b98>]lr : [<0004e328>] sp : 7edf8cc0 ip : 000c fp : 7ffe60ec r10: r9 : 7eef8eb0 r8 : 7ffe0d02 r7 : r6 : 7ef0f8c8 r5 : 7ffe0cf0 r4 : 7ffe0cb4 r3 : 7ffe0cef r2 : r1 : r0 : Flags: nzcv IRQs off FIQs off Mode SVC_32 Code: e2403002 e3a0 e151 012fff1e (e1f320b2) UEFI image [0x:0x] '/\selftest' Resetting CPU .. Debugging shows: efi_hii_sibt_string_ucs2_block_next() calls u16_strnlen() for an unaligned u16 string. Here "ldrh r2, [r3, #2]!" is executed for unaligned r3. This should be allowable for SCTLR.A = 0. When the crash occurs SCRLR has value 0xc5187f. SCTLR.A is bit 1 with value 1. The implementation of allow_unaligned() in arch/arm/cpu/armv7 /sctlr.S should have set the flag to 0. arch/arm/cpu/armv7/sctlr.S is compiled (as demonstrated by adding #error to the code). If I remove the weak implementation of allow_unaligned() in lib/efi_loader/efi_setup.c, the error does not occur. Shouldn't building with LTO ignore the weak implementation? If I add a printf() statement to the weak implemenation, the printf() command is not executed but SCTLR 0xc5187d, SCTLR.A=0 The test passes as unaligned access is allowable. I was building inside the Docker image with the GCC downloaded by buildman (gcc-12.2.0-nolibc/arm-linux-gnueabi). To me this looks like a compiler issue. Best regards Heinrich
Re: qemu_arm_defconfig with LTO fails due to unaligned access
Am 8. März 2023 17:18:32 MEZ schrieb Tom Rini : >On Wed, Mar 08, 2023 at 05:12:26AM +0100, Heinrich Schuchardt wrote: >> Hello Ilias, hello Tom, >> >> Tom tried to run qemu_arm_defconfig with CONFIG_LTO=y in gitlab. This >> failed as shown in protocol >> https://source.denx.de/u-boot/u-boot/-/jobs/589913/raw >> >> Executing 'HII database protocols' >> test_hii_database_new_package_list: >> data abort >> pc : [<7ff39b98>] lr : [<7ff87328>] >> reloc pc : [<0b98>]lr : [<0004e328>] >> sp : 7edf8cc0 ip : 000c fp : 7ffe60ec >> r10: r9 : 7eef8eb0 r8 : 7ffe0d02 >> r7 : r6 : 7ef0f8c8 r5 : 7ffe0cf0 r4 : 7ffe0cb4 >> r3 : 7ffe0cef r2 : r1 : r0 : >> Flags: nzcv IRQs off FIQs off Mode SVC_32 >> Code: e2403002 e3a0 e151 012fff1e (e1f320b2) >> UEFI image [0x:0x] '/\selftest' >> Resetting CPU .. >> >> Debugging shows: >> >> efi_hii_sibt_string_ucs2_block_next() calls u16_strnlen() for an >> unaligned u16 string. Here "ldrh r2, [r3, #2]!" is executed for >> unaligned r3. This should be allowable for SCTLR.A = 0. >> >> When the crash occurs SCRLR has value 0xc5187f. SCTLR.A is bit 1 with >> value 1. >> >> The implementation of allow_unaligned() in >> arch/arm/cpu/armv7 /sctlr.S should have set the flag to 0. >> arch/arm/cpu/armv7/sctlr.S is compiled (as demonstrated by adding #error >> to the code). >> >> If I remove the weak implementation of allow_unaligned() in >> lib/efi_loader/efi_setup.c, the error does not occur. >> >> Shouldn't building with LTO ignore the weak implementation? >> >> If I add a printf() statement to the weak implemenation, the printf() >> command is not executed but >> >> SCTLR 0xc5187d, SCTLR.A=0 >> >> The test passes as unaligned access is allowable. >> >> I was building inside the Docker image with the GCC downloaded by >> buildman (gcc-12.2.0-nolibc/arm-linux-gnueabi). >> >> To me this looks like a compiler issue. > >Interesting, yes. It seems like it shouldn't be too hard to come up with >a condensed example where the assembly function isn't used but instead >the weak C function is. > >And as a work-around, re-doing the code so that path_to_uefi() just >checks for ARM && !ARM64 before calling allow_unaligned() and not doing >the weak function trick should also be fine. > We have more places in our code using weak functions. There many cases not covered by CI. Just looking at EFI may not be adequate. Best regards Heinrich
Re: [PATCH 2/2] efi: remove error in efi_disk_remove
On 3/8/23 14:26, Patrick Delaunay wrote:
EFI has no reason to block the driver remove when the associated EFI
resources failed to be released.
This patch avoids DM issue when an EFI resource can't be released,
for example if this resource wasn't created, for duplicated device name
(error EFI_ALREADY_STARTED).
Without this patch, the U-Boot device tree is not updated for "usb stop"
command because EFI stack can't free a resource; in usb_stop(), the
remove operation is stopped on first device_remove() error, including a
device_notify() error on any chil
The typical reason to return an error here is that the EFI device is
still in use, i.e. a protocol installed on the EFI handle is opened by a
child controller or driver. As long as the EFI handle cannot be removed
we must not remove the linked DM device or we corrupt our data model.
Best regards
Heinrich
And this remove error, returned by usb_stop(), is not managed in cmd/usb.c
and the next "usb start" command cause a crash because all the USB devices
need to be released before the next USB scan.
Signed-off-by: Patrick Delaunay
---
lib/efi_loader/efi_disk.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 8d53ba3bd27e..22a0035dcde2 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -767,16 +767,20 @@ int efi_disk_remove(void *ctx, struct event *event)
{
enum uclass_id id;
struct udevice *dev;
+ int ret = 0;
dev = event->data.dm.dev;
id = device_get_uclass_id(dev);
if (id == UCLASS_BLK)
- return efi_disk_delete_raw(dev);
+ ret = efi_disk_delete_raw(dev);
else if (id == UCLASS_PARTITION)
- return efi_disk_delete_part(dev);
- else
- return 0;
+ ret = efi_disk_delete_part(dev);
+
+ if (ret)
+ log_err("%s failed for %s uclass %u (%d)\n", __func__,
dev->name, id, ret);
+
+ return 0;
}
/**
Re: [PATCH 1/2] efi: remove error in efi_disk_probe
On 3/8/23 14:26, Patrick Delaunay wrote:
EFI has no reason to block the dm core device_probe() in the callback
efi_disk_probe() registered with EVT_DM_POST_PROBE.
This patch avoids to have error in DM core on device_probe()
ret = device_notify(dev, EVT_DM_POST_PROBE);
only because EFI is not able to create its instance/handle.
This should only occur if we are out of memory or if you call
efi_disk_probe() twice for the same device.
For example on usb start, when the SAME KEY (PID/VID) is present on
2 ports of the USB HUB, the 2nd key have the same EFI device path
with the call stack:
We need the HUB device with its USB port in the device path.
The way we currently create device paths is not good. We should traverse
the dm tree to the root and create a node for each dm device. The code
code for creating the individual nodes should be moved to uclasses.
@Simon: is that ok for you?
efi_disk_probe()
efi_disk_create_raw()
efi_disk_add_dev()
efi_install_multiple_protocol_interfaces()
EFI_ALREADY_STARTED
If we create the same device path for two USB devices, this is a bug we
must fix.
In case of error in probe, the 2nd key is unbound and deactivated for
the next usb commands even if the limitation is only for EFI.
This patch removes any return error in probe event callback;
if something occurs in EFI registration, the device is still probed.
Signed-off-by: Patrick Delaunay
---
lib/efi_loader/efi_disk.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index d2256713a8e7..8d53ba3bd27e 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -677,14 +677,18 @@ int efi_disk_probe(void *ctx, struct event *event)
desc = dev_get_uclass_plat(dev);
if (desc->uclass_id != UCLASS_EFI_LOADER) {
ret = efi_disk_create_raw(dev, agent_handle);
- if (ret)
- return -1;
+ if (ret) {
+ log_err("efi_disk_create_raw %s failed (%d)\n",
+ dev->name, ret);
This isn't a message a non-developer can easily understand.
+ return 0;
+ }
}
device_foreach_child(child, dev) {
ret = efi_disk_create_part(child, agent_handle);
if (ret)
- return -1;
+ log_err("efi_disk_create_part %s failed (%d)\n",
ditto.
Best regards
Heinrich
+ dev->name, ret);
}
return 0;
Re: [PATCH 2/2] efi: remove error in efi_disk_remove
On 3/9/23 11:59, Patrick DELAUNAY wrote: On 3/9/23 09:54, Heinrich Schuchardt wrote: On 3/8/23 14:26, Patrick Delaunay wrote: EFI has no reason to block the driver remove when the associated EFI resources failed to be released. This patch avoids DM issue when an EFI resource can't be released, for example if this resource wasn't created, for duplicated device name (error EFI_ALREADY_STARTED). Without this patch, the U-Boot device tree is not updated for "usb stop" command because EFI stack can't free a resource; in usb_stop(), the remove operation is stopped on first device_remove() error, including a device_notify() error on any chil The typical reason to return an error here is that the EFI device is still in use, i.e. a protocol installed on the EFI handle is opened by a child controller or driver. As long as the EFI handle cannot be removed we must not remove the linked DM device or we corrupt our data model. Best regards Heinrich Ok I get it now, Forget my serie Patrick Hello Patrick, thanks a lot for pointing out the issues with non-unique device paths. As it will take some time to clean up the device path generation patch 1 of the series may still make sense to avoid trouble for users using multiple USB devices. Best regards Heinrich
Pull request for efi-2023-04-rc4
The following changes since commit 6c1cdf158c4f3ccc8c5f9552f049a3386899dcd2: Merge tag 'dm-pull-10mar23' of https://source.denx.de/u-boot/custodians/u-boot-dm (2023-03-10 16:01:52 -0500) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2023-04-rc4 for you to fetch changes up to d3970e04e7125e37ea8c4f3f056b6f5ba868e5f7: efi_loader: describe term_get_char() (2023-03-13 13:56:14 +0100) Gitlab CI showed no issues: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15572 Pull request for efi-2023-04-rc4 Documentation: * man-page for panic command UEFI: * Correct parameter check for SetVariable() Other: * Provide unit test for crc8 ---- Heinrich Schuchardt (3): test: unit test for crc8 doc: man-page for panic command efi_loader: describe term_get_char() Masahisa Kojima (1): efi_loader: update SetVariable attribute check Vincent Stehlé (1): doc: uefi: fix links doc/develop/uefi/fwu_updates.rst | 3 ++- doc/develop/uefi/uefi.rst| 4 ++-- doc/usage/cmd/panic.rst | 33 + doc/usage/index.rst | 1 + lib/efi_loader/efi_console.c | 8 lib/efi_loader/efi_variable.c| 31 +-- test/lib/Makefile| 1 + test/lib/test_crc8.c | 29 + 8 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 doc/usage/cmd/panic.rst create mode 100644 test/lib/test_crc8.c
next: Pull request efi-next-20230313
The following changes since commit bcf343146ff365a88481b9a80920ed146c6dee5b: Merge tag 'dm-next-9mar23' of https://source.denx.de/u-boot/custodians/u-boot-dm into next (2023-03-09 11:22:50 -0500) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-next-20230313 for you to fetch changes up to 61a621054194216eefc1a6f5af0a63aa265bbaef: video: Add a note about the broken implementation (2023-03-13 13:53:01 +0100) Gitlab CI showed no issues: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15571 Pull request efi-next-20230313 UEFI: * Improve graphics support in EFI app Others: * x86: Add a few more items to bdinfo * video: Remove duplicate cursor-positioning function * video: Clear the vidconsole rather than the video Simon Glass (13): efi: video: Move payload code into a function efi: video: Return mode info for app also efi: Support a 64-bit frame buffer address x86: Add a few more items to bdinfo efi: Use a fixed value for the timer clock efi: Support copy framebuffer video: Allow a copy framebuffer with pre-allocated fb bbinfo: Show the size of the copy framebuffer efi: Adjust script to show pre-relocation output on terminal video: Remove duplicate cursor-positioning function video: Clear the vidconsole rather than the video efi: Add dhrystone, dcache and scroll lines to app video: Add a note about the broken implementation arch/x86/dts/efi-x86_app.dts | 1 + arch/x86/lib/bdinfo.c | 6 ++ arch/x86/lib/fsp/fsp_graphics.c | 2 +- cmd/bdinfo.c | 11 ++- cmd/cls.c | 20 -- configs/efi-x86_app64_defconfig | 3 + drivers/pci/pci_rom.c | 10 +-- drivers/timer/tsc_timer.c | 9 +++ drivers/video/coreboot.c | 2 +- drivers/video/efi.c | 138 -- drivers/video/vidconsole-uclass.c | 48 + drivers/video/video-uclass.c | 32 ++--- include/init.h| 3 + include/vesa.h| 16 - include/video.h | 2 + include/video_console.h | 9 +++ scripts/build-efi.sh | 2 + 17 files changed, 228 insertions(+), 86 deletions(-)
Re: [PATCH] arm: enable unaligned accesses by default if EFI is configured
On 3/17/23 14:42, Ilias Apalodimas wrote: Heinrich reports that compiling with LTO & EFI breaks armv7 and arm11* builds. The reason is that LTO (maybe a binutils bug?) replaces the asm version of allow_unaligned() with the __weak definition of the function. As a result EFI code ends up running with unaligned accesses disabled and eventually crashes. So let's enable unaligned access for those architectures during our start.S routines and avoid the linker issues. Reported-by: Heinrich Schuchardt The problem was originally reported by Tom. My contribution was to track it down to missing enabling of unaligned access due to a linker problem. Signed-off-by: Ilias Apalodimas --- arch/arm/cpu/arm11/Makefile | 4 arch/arm/cpu/arm11/sctlr.S | 25 - arch/arm/cpu/arm1136/start.S | 3 +++ arch/arm/cpu/arm1176/start.S | 3 +++ arch/arm/cpu/armv7/Makefile | 1 - arch/arm/cpu/armv7/sctlr.S | 22 -- arch/arm/cpu/armv7/start.S | 3 +++ include/asm-generic/unaligned.h | 4 lib/efi_loader/efi_device_path.c | 7 --- lib/efi_loader/efi_setup.c | 12 10 files changed, 9 insertions(+), 75 deletions(-) delete mode 100644 arch/arm/cpu/arm11/sctlr.S delete mode 100644 arch/arm/cpu/armv7/sctlr.S diff --git a/arch/arm/cpu/arm11/Makefile b/arch/arm/cpu/arm11/Makefile index 5dfa01ae8d05..5d721fce12b5 100644 --- a/arch/arm/cpu/arm11/Makefile +++ b/arch/arm/cpu/arm11/Makefile @@ -4,7 +4,3 @@ # Wolfgang Denk, DENX Software Engineering, w...@denx.de. obj-y = cpu.o - -ifneq ($(CONFIG_SPL_BUILD),y) -obj-$(CONFIG_EFI_LOADER) += sctlr.o -endif diff --git a/arch/arm/cpu/arm11/sctlr.S b/arch/arm/cpu/arm11/sctlr.S deleted file mode 100644 index 74a7fc4a25b6.. --- a/arch/arm/cpu/arm11/sctlr.S +++ /dev/null @@ -1,25 +0,0 @@ -/* SPDX-License-Identifier:GPL-2.0+ */ -/* - * Routines to access the system control register - * - * Copyright (c) 2019 Heinrich Schuchardt - */ - -#include - -/* - * void allow_unaligned(void) - allow unaligned access - * - * This routine sets the enable unaligned data support flag and clears the - * aligned flag in the system control register. - * After calling this routine unaligned access does no longer leads to a - * data abort or undefined behavior but is handled by the CPU. - * For details see the "ARM Architecture Reference Manual" for ARMv6. - */ -ENTRY(allow_unaligned) - mrc p15, 0, r0, c1, c0, 0 @ load system control register - orr r0, r0, #1 << 22 @ set unaligned data support flag - bic r0, r0, #2 @ clear aligned flag - mcr p15, 0, r0, c1, c0, 0 @ write system control register - bx lr @ return -ENDPROC(allow_unaligned) diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index 4bc27f637366..f2a18d8f3423 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S @@ -77,7 +77,10 @@ cpu_init_crit: mrc p15, 0, r0, c1, c0, 0 bic r0, r0, #0x2300 @ clear bits 13, 9:8 (--V- --RS) bic r0, r0, #0x0087 @ clear bits 7, 2:0 (B--- -CAM) +#if !CONFIG_IS_ENABLED(EFI_LOADER) + /* allow unaligned accesses since EFI requires it */ This comment line is only reached without UEFI support. So here you should explain why we forbid unaligned access without UEFI. Best regards Heinrich orr r0, r0, #0x0002 @ set bit 1 (A) Align +#endif orr r0, r0, #0x1000 @ set bit 12 (I) I-Cache mcr p15, 0, r0, c1, c0, 0 diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S index 78a9cc173a39..0c80160702ba 100644 --- a/arch/arm/cpu/arm1176/start.S +++ b/arch/arm/cpu/arm1176/start.S @@ -79,7 +79,10 @@ cpu_init_crit: mrc p15, 0, r0, c1, c0, 0 bic r0, r0, #0x2300 @ clear bits 13, 9:8 (--V- --RS) bic r0, r0, #0x0087 @ clear bits 7, 2:0 (B--- -CAM) +#if !CONFIG_IS_ENABLED(EFI_LOADER) + /* allow unailgned accesses since EFI requires it */ orr r0, r0, #0x0002 @ set bit 1 (A) Align +#endif orr r0, r0, #0x1000 @ set bit 12 (I) I-Cache /* Prepare to disable the MMU */ diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile index 653eef8ad79e..ca50f6e93e10 100644 --- a/arch/arm/cpu/armv7/Makefile +++ b/arch/arm/cpu/armv7/Makefile @@ -13,7 +13,6 @@ obj-y += syslib.o obj-$(CONFIG_SYS_ARM_MPU) += mpu_v7r.o ifneq ($(CONFIG_SPL_BUILD),y) -obj-$(CONFIG_EFI_LOADER) += sctlr.o obj-$(CONFIG_ARMV7_NONSEC) += exception_level.o endif diff --git a/arch/arm/cpu/armv7/sctlr.S b/arch/arm/cpu/armv7/sctlr.S deleted file mode 100644 index bd56e41afe18.. --- a/arch/arm/cpu/armv7/sctlr.S +++ /dev/null @@ -1,22 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0+ */ -/* - * Rout
[PATCH 1/1] dm: simplify uclass_pre_remove_device
Remove a superfluous logical constraint.
Signed-off-by: Heinrich Schuchardt
---
drivers/core/uclass.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index 1762a0796d..dce5b46fc7 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -789,14 +789,10 @@ int uclass_post_probe_device(struct udevice *dev)
int uclass_pre_remove_device(struct udevice *dev)
{
struct uclass *uc;
- int ret;
uc = dev->uclass;
- if (uc->uc_drv->pre_remove) {
- ret = uc->uc_drv->pre_remove(dev);
- if (ret)
- return ret;
- }
+ if (uc->uc_drv->pre_remove)
+ return uc->uc_drv->pre_remove(dev);
return 0;
}
--
2.39.2
[PATCH 1/1] efi_loader: move struct efi_device_path to efi.h
Avoid forward declaration of struct efi_device_path.
Signed-off-by: Heinrich Schuchardt
---
include/efi.h | 13 -
include/efi_api.h | 6 --
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/include/efi.h b/include/efi.h
index c3087d3da2..2f312da3cb 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -52,7 +52,18 @@
#define EFI32_LOADER_SIGNATURE "EL32"
#define EFI64_LOADER_SIGNATURE "EL64"
-struct efi_device_path;
+/**
+ * struct efi_device_path - device path protocol
+ *
+ * @type: device path type
+ * @sub_type: device path sub-type
+ * @length:length of the device path node including the header
+ */
+struct efi_device_path {
+ u8 type;
+ u8 sub_type;
+ u16 length;
+} __packed;
/*
* The EFI spec defines the EFI_GUID as
diff --git a/include/efi_api.h b/include/efi_api.h
index 2d18d25a71..2969884280 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -557,12 +557,6 @@ struct efi_loaded_image {
# define DEVICE_PATH_SUB_TYPE_INSTANCE_END0x01
# define DEVICE_PATH_SUB_TYPE_END 0xff
-struct efi_device_path {
- u8 type;
- u8 sub_type;
- u16 length;
-} __packed;
-
struct efi_mac_addr {
u8 addr[32];
} __packed;
--
2.39.2
[PATCH 0/2] efi_loader: efi_alloc()
The incumbent function efi_alloc() is unused. Replace dp_alloc() by a new function efi_alloc() that we can use more widely. Use it to simply efi_str_to_u16(). Heinrich Schuchardt (2): efi_loader: move dp_alloc() to efi_alloc() efi_loader: simplify efi_str_to_u16() include/efi_loader.h | 4 +-- lib/efi_loader/efi_device_path.c | 40 +++-- lib/efi_loader/efi_device_path_to_text.c | 5 ++- lib/efi_loader/efi_memory.c | 46 +--- 4 files changed, 42 insertions(+), 53 deletions(-) -- 2.39.2
[PATCH 1/2] efi_loader: move dp_alloc() to efi_alloc()
The incumbent function efi_alloc() is unused.
Replace dp_alloc() by a new function efi_alloc() that we can use more
widely.
Signed-off-by: Heinrich Schuchardt
---
include/efi_loader.h | 4 +--
lib/efi_loader/efi_device_path.c | 40 +--
lib/efi_loader/efi_memory.c | 46 +---
3 files changed, 40 insertions(+), 50 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 1542b4b625..cee04cbb9d 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -724,8 +724,8 @@ efi_status_t efi_next_variable_name(efi_uintn_t *size, u16
**buf,
* Return: size in pages
*/
#define efi_size_in_pages(size) (((size) + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT)
-/* Generic EFI memory allocator, call this to get memory */
-void *efi_alloc(uint64_t len, int memory_type);
+/* Allocate boot service data pool memory */
+void *efi_alloc(size_t len);
/* Allocate pages on the specified alignment */
void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align);
/* More specific EFI memory allocator, called by EFI payloads */
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 3b267b713e..9c560e034c 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -63,20 +63,6 @@ static bool is_sd(struct blk_desc *desc)
}
#endif
-static void *dp_alloc(size_t sz)
-{
- void *buf;
-
- if (efi_allocate_pool(EFI_BOOT_SERVICES_DATA, sz, &buf) !=
- EFI_SUCCESS) {
- debug("EFI: ERROR: out of memory in %s\n", __func__);
- return NULL;
- }
-
- memset(buf, 0, sz);
- return buf;
-}
-
/*
* Iterate to next block in device-path, terminating (returning NULL)
* at /End* node.
@@ -302,7 +288,7 @@ struct efi_device_path *efi_dp_dup(const struct
efi_device_path *dp)
if (!dp)
return NULL;
- ndp = dp_alloc(sz);
+ ndp = efi_alloc(sz);
if (!ndp)
return NULL;
memcpy(ndp, dp, sz);
@@ -346,7 +332,7 @@ efi_device_path *efi_dp_append_or_concatenate(const struct
efi_device_path *dp1,
/* both dp1 and dp2 are non-null */
unsigned sz1 = efi_dp_size(dp1);
unsigned sz2 = efi_dp_size(dp2);
- void *p = dp_alloc(sz1 + sz2 + end_size);
+ void *p = efi_alloc(sz1 + sz2 + end_size);
if (!p)
return NULL;
ret = p;
@@ -409,7 +395,7 @@ struct efi_device_path *efi_dp_append_node(const struct
efi_device_path *dp,
ret = efi_dp_dup(dp);
} else if (!dp) {
size_t sz = node->length;
- void *p = dp_alloc(sz + sizeof(END));
+ void *p = efi_alloc(sz + sizeof(END));
if (!p)
return NULL;
memcpy(p, node, sz);
@@ -418,7 +404,7 @@ struct efi_device_path *efi_dp_append_node(const struct
efi_device_path *dp,
} else {
/* both dp and node are non-null */
size_t sz = efi_dp_size(dp);
- void *p = dp_alloc(sz + node->length + sizeof(END));
+ void *p = efi_alloc(sz + node->length + sizeof(END));
if (!p)
return NULL;
memcpy(p, dp, sz);
@@ -439,7 +425,7 @@ struct efi_device_path *efi_dp_create_device_node(const u8
type,
if (length < sizeof(struct efi_device_path))
return NULL;
- ret = dp_alloc(length);
+ ret = efi_alloc(length);
if (!ret)
return ret;
ret->type = type;
@@ -461,7 +447,7 @@ struct efi_device_path *efi_dp_append_instance(
return efi_dp_dup(dpi);
sz = efi_dp_size(dp);
szi = efi_dp_instance_size(dpi);
- p = dp_alloc(sz + szi + 2 * sizeof(END));
+ p = efi_alloc(sz + szi + 2 * sizeof(END));
if (!p)
return NULL;
ret = p;
@@ -486,7 +472,7 @@ struct efi_device_path *efi_dp_get_next_instance(struct
efi_device_path **dp,
if (!dp || !*dp)
return NULL;
sz = efi_dp_instance_size(*dp);
- p = dp_alloc(sz + sizeof(END));
+ p = efi_alloc(sz + sizeof(END));
if (!p)
return NULL;
memcpy(p, *dp, sz + sizeof(END));
@@ -906,7 +892,7 @@ struct efi_device_path *efi_dp_from_part(struct blk_desc
*desc, int part)
{
void *buf, *start;
- start = buf = dp_alloc(dp_part_size(desc, part) + sizeof(END));
+ start = buf = efi_alloc(dp_part_size(desc, part) + sizeof(END));
if (!buf)
return NULL;
@@ -933,7 +919,7 @@ struct efi_device_path *efi_dp_part_node(struct blk_desc
*desc, int part)
dpsize = sizeof(struct efi_device_path_cdrom_path);
else
dpsize = sizeof(struct efi_device_path_hard_drive_p
[PATCH 2/2] efi_loader: simplify efi_str_to_u16()
Use efi_alloc() to allocate memory.
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_device_path_to_text.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c
b/lib/efi_loader/efi_device_path_to_text.c
index 9062058ac2..06e709dabc 100644
--- a/lib/efi_loader/efi_device_path_to_text.c
+++ b/lib/efi_loader/efi_device_path_to_text.c
@@ -32,11 +32,10 @@ static u16 *efi_str_to_u16(char *str)
{
efi_uintn_t len;
u16 *out, *dst;
- efi_status_t ret;
len = sizeof(u16) * (utf8_utf16_strlen(str) + 1);
- ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, len, (void **)&out);
- if (ret != EFI_SUCCESS)
+ out = efi_alloc(len);
+ if (!out)
return NULL;
dst = out;
utf8_utf16_strcpy(&dst, str);
--
2.39.2
[PATCH 0/2] efi_loader: fix device-path for USB devices
EFI device paths for block devices must be unique. If a non-unique device path is discovered, probing of the block device fails. Currently we use UsbClass() device path nodes. As multiple devices may have the same vendor and product id these are non-unique. Instead we should use Usb() device path nodes. They include the USB port on the parent hub and hence are unique. A USB storage device may contain multiple logical units. These can be modeled as Ctrl() nodes. Given a driver model tree root 0 root_driver root_driver usb0 ehci_generic |-- usb@1c1a000 usb_hub0 usb_hub `-- usb_hub usb_hub1 usb_hub `-- usb_hub usb_mass_s 2 usb_mass_storage `-- usb_mass_storage blk4 usb_storage_blk |-- usb_mass_storage.lun0 where the USB mass storage device is connected to port 4 of the 2nd hub, the device path for LUN 0 will be: /VenHw()/USB(0x0,0x0)/USB(0x1,0x0)/USB(0x4,0x0)/Ctrl(0x0) Heinrich Schuchardt (2): efi_loader: support for Ctrl() device path node efi_loader: fix device-path for USB devices include/efi_api.h| 6 lib/efi_loader/efi_device_path.c | 45 +--- lib/efi_loader/efi_device_path_to_text.c | 7 3 files changed, 46 insertions(+), 12 deletions(-) -- 2.39.2
[PATCH 1/2] efi_loader: support for Ctrl() device path node
* Add the definitions for Ctrl() device path nodes.
* Implement Ctrl() nodes in the device path to text protocol.
Signed-off-by: Heinrich Schuchardt
---
include/efi_api.h| 6 ++
lib/efi_loader/efi_device_path_to_text.c | 7 +++
2 files changed, 13 insertions(+)
diff --git a/include/efi_api.h b/include/efi_api.h
index 2d18d25a71..c57868abbd 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -570,6 +570,7 @@ struct efi_mac_addr {
#define DEVICE_PATH_TYPE_HARDWARE_DEVICE 0x01
# define DEVICE_PATH_SUB_TYPE_MEMORY 0x03
# define DEVICE_PATH_SUB_TYPE_VENDOR 0x04
+# define DEVICE_PATH_SUB_TYPE_CONTROLLER 0x05
struct efi_device_path_memory {
struct efi_device_path dp;
@@ -584,6 +585,11 @@ struct efi_device_path_vendor {
u8 vendor_data[];
} __packed;
+struct efi_device_path_controller {
+ struct efi_device_path dp;
+ u32 controller_number;
+} __packed;
+
#define DEVICE_PATH_TYPE_ACPI_DEVICE 0x02
# define DEVICE_PATH_SUB_TYPE_ACPI_DEVICE 0x01
diff --git a/lib/efi_loader/efi_device_path_to_text.c
b/lib/efi_loader/efi_device_path_to_text.c
index 9062058ac2..9c0b39311a 100644
--- a/lib/efi_loader/efi_device_path_to_text.c
+++ b/lib/efi_loader/efi_device_path_to_text.c
@@ -77,6 +77,13 @@ static char *dp_hardware(char *s, struct efi_device_path *dp)
s += sprintf(s, ")");
break;
}
+ case DEVICE_PATH_SUB_TYPE_CONTROLLER: {
+ struct efi_device_path_controller *cdp =
+ (struct efi_device_path_controller *)dp;
+
+ s += sprintf(s, "Ctrl(0x%0x)", cdp->controller_number);
+ break;
+ }
default:
s = dp_unknown(s, dp);
break;
--
2.39.2
[PATCH 2/2] efi_loader: fix device-path for USB devices
EFI device paths for block devices must be unique. If a non-unique device
path is discovered, probing of the block device fails.
Currently we use UsbClass() device path nodes. As multiple devices may
have the same vendor and product id these are non-unique. Instead we
should use Usb() device path nodes. They include the USB port on the
parent hub. Hence they are unique.
A USB storage device may contain multiple logical units. These can be
modeled as Ctrl() nodes.
Reported-by: Patrick Delaunay
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_device_path.c | 45 +++-
1 file changed, 33 insertions(+), 12 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 3b267b713e..b6dd575b13 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -147,7 +147,7 @@ struct efi_device_path *efi_dp_shorten(struct
efi_device_path *dp)
* in practice fallback.efi just uses MEDIA:HARD_DRIVE
* so not sure when we would see these other cases.
*/
- if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB_CLASS) ||
+ if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB) ||
EFI_DP_TYPE(dp, MEDIA_DEVICE, HARD_DRIVE_PATH) ||
EFI_DP_TYPE(dp, MEDIA_DEVICE, FILE_PATH))
return dp;
@@ -564,6 +564,11 @@ __maybe_unused static unsigned int dp_size(struct udevice
*dev)
return dp_size(dev->parent)
+ sizeof(struct efi_device_path_vendor) + 1;
#endif
+#ifdef CONFIG_USB
+ case UCLASS_MASS_STORAGE:
+ return dp_size(dev->parent)
+ + sizeof(struct efi_device_path_controller);
+#endif
#ifdef CONFIG_VIRTIO_BLK
case UCLASS_VIRTIO:
/*
@@ -585,7 +590,7 @@ __maybe_unused static unsigned int dp_size(struct udevice
*dev)
case UCLASS_MASS_STORAGE:
case UCLASS_USB_HUB:
return dp_size(dev->parent) +
- sizeof(struct efi_device_path_usb_class);
+ sizeof(struct efi_device_path_usb);
default:
/* just skip over unknown classes: */
return dp_size(dev->parent);
@@ -741,6 +746,19 @@ __maybe_unused static void *dp_fill(void *buf, struct
udevice *dev)
memcpy(&dp->ns_id, &ns_id, sizeof(ns_id));
return &dp[1];
}
+#endif
+#if defined(CONFIG_USB)
+ case UCLASS_MASS_STORAGE: {
+ struct blk_desc *desc = desc = dev_get_uclass_plat(dev);
+ struct efi_device_path_controller *dp =
+ dp_fill(buf, dev->parent);
+
+ dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
+ dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CONTROLLER;
+ dp->dp.length = sizeof(*dp);
+ dp->controller_number = desc->lun;
+ return &dp[1];
+ }
#endif
default:
debug("%s(%u) %s: unhandled parent class: %s (%u)\n",
@@ -767,19 +785,22 @@ __maybe_unused static void *dp_fill(void *buf, struct
udevice *dev)
#endif
case UCLASS_MASS_STORAGE:
case UCLASS_USB_HUB: {
- struct efi_device_path_usb_class *udp =
- dp_fill(buf, dev->parent);
- struct usb_device *udev = dev_get_parent_priv(dev);
- struct usb_device_descriptor *desc = &udev->descriptor;
+ struct efi_device_path_usb *udp = dp_fill(buf, dev->parent);
+
+ switch (device_get_uclass_id(dev->parent)) {
+ case UCLASS_USB_HUB: {
+ struct usb_device *udev = dev_get_parent_priv(dev);
+ udp->parent_port_number = udev->portnr;
+ break;
+ }
+ default:
+ udp->parent_port_number = 0;
+ }
udp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
- udp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS;
+ udp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_USB;
udp->dp.length = sizeof(*udp);
- udp->vendor_id = desc->idVendor;
- udp->product_id = desc->idProduct;
- udp->device_class= desc->bDeviceClass;
- udp->device_subclass = desc->bDeviceSubClass;
- udp->device_protocol = desc->bDeviceProtocol;
+ udp->usb_interface = 0;
return &udp[1];
}
--
2.39.2
Re: [PATCH v2 03/12] x86: Add return-value comment to cpu_jump_to_64bit()
On 3/10/23 21:48, Simon Glass wrote: This does not mention what it returns. Add the missing documentation. Signed-off-by: Simon Glass --- (no changes since v1) arch/x86/include/asm/cpu.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h index 3346012d335..aa03ef598e1 100644 --- a/arch/x86/include/asm/cpu.h +++ b/arch/x86/include/asm/cpu.h @@ -262,6 +262,7 @@ void cpu_call32(ulong code_seg32, ulong target, ulong table); * * @setup_base: Pointer to the setup.bin information for the kernel * @target: Pointer to the start of the kernel image + * Returns: -EFAULT if the kernel returned; otherwise does not return %s/Returns:/Return:/ See https://docs.kernel.org/doc-guide/kernel-doc.html#function-documentation Best regards Heinrich */ int cpu_jump_to_64bit(ulong setup_base, ulong target);
Re: [PATCH v2 05/12] x86: Exit EFI boot services before starting kernel
On 3/10/23 21:48, Simon Glass wrote:
When running the EFI app, we need to exit boot services before jumping
to Linux.
At some point it may be possible to jump to Linux and pass on the boot
services, so that the Linux efistub can be used. For now, this is not
implemented.
The EFI entry point expects the handle and system table. The system
table contains more then a pointer to boot services. If you respin the
series, consider updating the comment.
If you are running with CONFIG_EFI_API=y, you would have to
* install the device-tree as configuration table
* use LoadImage() to load the kernel image (e.g. from memory)
* start the image with StartImage().
Best regards
Heinrich
Signed-off-by: Simon Glass
---
(no changes since v1)
arch/x86/lib/bootm.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c
index 9beb376bb9c..61cb7bc6116 100644
--- a/arch/x86/lib/bootm.c
+++ b/arch/x86/lib/bootm.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -156,6 +157,23 @@ int boot_linux_kernel(ulong setup_base, ulong entry, bool
image_64bit)
#ifdef CONFIG_SYS_COREBOOT
timestamp_add_now(TS_U_BOOT_START_KERNEL);
#endif
+
+ /*
+* Exit EFI boot services just before jumping, after all console
+* output, since the console won't be available afterwards.
+*/
+ if (IS_ENABLED(CONFIG_EFI_APP)) {
+ int ret;
+
+ ret = efi_store_memory_map(efi_get_priv());
+ if (ret)
+ return ret;
+ printf("Exiting EFI boot services\n");
+ ret = efi_call_exit_boot_services();
+ if (ret)
+ return ret;
+ }
+
if (image_64bit) {
if (!cpu_has_64bit()) {
puts("Cannot boot 64-bit kernel on 32-bit machine\n");
Re: [PATCH v2 06/12] x86: Support zboot and bootm in the EFI app
On 3/10/23 21:48, Simon Glass wrote: These have been disabled due to the rudimentary support available. It is a little better now, so enable these options. Signed-off-by: Simon Glass Reviewed-by: Heinrich Schuchardt --- (no changes since v1) configs/efi-x86_app32_defconfig | 2 +- configs/efi-x86_app64_defconfig | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configs/efi-x86_app32_defconfig b/configs/efi-x86_app32_defconfig index 905f375a3ef..50975dbfaaf 100644 --- a/configs/efi-x86_app32_defconfig +++ b/configs/efi-x86_app32_defconfig @@ -19,7 +19,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_LAST_STAGE_INIT=y CONFIG_HUSH_PARSER=y CONFIG_SYS_PBSIZE=532 -# CONFIG_CMD_BOOTM is not set +CONFIG_CMD_BOOTZ=y CONFIG_CMD_PART=y # CONFIG_CMD_NET is not set CONFIG_CMD_TIME=y diff --git a/configs/efi-x86_app64_defconfig b/configs/efi-x86_app64_defconfig index f1cf43c1ef6..0fc358ddcdf 100644 --- a/configs/efi-x86_app64_defconfig +++ b/configs/efi-x86_app64_defconfig @@ -20,7 +20,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_LAST_STAGE_INIT=y CONFIG_HUSH_PARSER=y CONFIG_SYS_PBSIZE=532 -# CONFIG_CMD_BOOTM is not set +CONFIG_CMD_BOOTZ=y CONFIG_CMD_PART=y # CONFIG_CMD_NET is not set CONFIG_CMD_CACHE=y
Re: [PATCH v2 07/12] efi: Add another tranch of GUIDs
On 3/10/23 21:48, Simon Glass wrote:
Provide information about the GUIDs supplied by QEMU, so far as it is
known.
These values are used in the 'efi table' command as well as the printf
format string %sU
Signed-off-by: Simon Glass
---
Changes in v2:
- Update commit message to explain why these are needed
- Drop EFI_SMBIOS which exists as SMBIOS_TABLE_GUID
- Drop unwanted EFI_GUID_SNBIOS
- Drop EFI_GUID_EFI_ACPI2 which exists as EFI_ACPI_TABLE_GUID
include/efi_api.h | 22 ++
lib/uuid.c| 7 +++
2 files changed, 29 insertions(+)
diff --git a/include/efi_api.h b/include/efi_api.h
index 2d18d25a713..3dd48195885 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -1909,6 +1909,28 @@ struct efi_system_resource_table {
EFI_GUID(0x4aafd29d, 0x68df, 0x49ee, 0x8a, 0xa9, \
0x34, 0x7d, 0x37, 0x56, 0x65, 0xa7)
+#define EFI_LZMA_COMPRESSED \
+ EFI_GUID(0xee4e5898, 0x3914, 0x4259, 0x9d, 0x6e, \
+0xdc, 0x7b, 0xd7, 0x94, 0x03, 0xcf)
+#define EFI_DXE_SERVICES \
+ EFI_GUID(0x05ad34ba, 0x6f02, 0x4214, 0x95, 0x2e, \
+0x4d, 0xa0, 0x39, 0x8e, 0x2b, 0xb9)
+#define EFI_HOB_LIST \
+ EFI_GUID(0x7739f24c, 0x93d7, 0x11d4, 0x9a, 0x3a, \
+0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
+#define EFI_MEMORY_TYPE \
+ EFI_GUID(0x4c19049f, 0x4137, 0x4dd3, 0x9c, 0x10, \
+0x8b, 0x97, 0xa8, 0x3f, 0xfd, 0xfa)
+#define EFI_SMBIOS \
+ EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3, 0x9a, 0x16, \
+0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
+#define EFI_MEM_STATUS_CODE_REC \
+ EFI_GUID(0x060cc026, 0x4c0d, 0x4dda, 0x8f, 0x41, \
+0x59, 0x5f, 0xef, 0x00, 0xa5, 0x02)
+#define EFI_GUID_EFI_ACPI1 \
+ EFI_GUID(0xeb9d2d30, 0x2d88, 0x11d3, 0x9a, 0x16, \
+0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
+
/**
* struct win_certificate_uefi_guid - A certificate that encapsulates
* a GUID-specific signature
diff --git a/lib/uuid.c b/lib/uuid.c
index 465e1ac38f5..d86fab72626 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -255,6 +255,13 @@ static const struct {
EFI_CERT_TYPE_PKCS7_GUID,
},
#endif
+ { "EFI_LZMA_COMPRESSED", EFI_LZMA_COMPRESSED },
+ { "EFI_DXE_SERVICES", EFI_DXE_SERVICES },
+ { "EFI_HOB_LIST", EFI_HOB_LIST },
+ { "EFI_MEMORY_TYPE", EFI_MEMORY_TYPE },
+ { "EFI_SMBIOS", EFI_SMBIOS },
+ { "EFI_MEM_STATUS_CODE_REC", EFI_MEM_STATUS_CODE_REC },
+ { "EFI_GUID_EFI_ACPI1", EFI_GUID_EFI_ACPI1 },
As we do not want to unnecessarily increase the size of U-Boot for all
boards, please, consider adding #ifdef CONFIG_EFI_APP.
Best regards
Heinrich
};
/*
Re: [PATCH v2 08/12] efi: Include GUID names with EFI app and payload
On 3/10/23 21:48, Simon Glass wrote:
These are currently only available when running with EFI_LOADER.
Expand this to include the app and payload, since it is useful to be
able to decode things there.
Signed-off-by: Simon Glass
---
Changes in v2:
- Add new patch to enable GUID names with EFI app and payload
lib/uuid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/uuid.c b/lib/uuid.c
index d86fab72626..09793fb62bc 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -102,7 +102,7 @@ static const struct {
{"lvm", PARTITION_LINUX_LVM_GUID},
{"u-boot-env",PARTITION_U_BOOT_ENVIRONMENT},
#endif
-#ifdef CONFIG_CMD_EFIDEBUG
+#if defined(CONFIG_CMD_EFIDEBUG) || defined(CONFIG_EFI)
%s/CONFIG_EFI/CONFIG_EFI_APP)/
We don't need these when running EFI_STUB=y without EFI_LOADER).
Best regards
Heinrich
{
"Device Path",
EFI_DEVICE_PATH_PROTOCOL_GUID,
Re: [PATCH v2 09/12] doc: Add help for the efi command
On 3/10/23 21:48, Simon Glass wrote: This command currently has no help. Add some. Signed-off-by: Simon Glass --- (no changes since v1) doc/usage/cmd/efi.rst | 197 ++ doc/usage/index.rst | 1 + 2 files changed, 198 insertions(+) create mode 100644 doc/usage/cmd/efi.rst diff --git a/doc/usage/cmd/efi.rst b/doc/usage/cmd/efi.rst new file mode 100644 index 000..c029c423879 --- /dev/null +++ b/doc/usage/cmd/efi.rst @@ -0,0 +1,197 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. Copyright 2020, Heinrich Schuchardt + +efi command +=== + +Synopsis + + +:: + +efi mem [all] + +Description +--- + +The *efi* command provides information about the EFI environment U-Boot is +running in, when it is started from EFI. + +When running as an EFI app, this command queries EFI boot services for the +information. When running as an EFI payload, EFI boot services have been +stopped, so it uses the information collected by the boot stub before that +happened. + +efi mem +~~~ + +This shows the EFI memory map, sorted in order of physical address. + +This is normally a very large table. To help reduce the amount of detritus, +boot-time memory is normally merged with conventional memory. Use the 'all' +argument to show everything. + +The fields are as follows: + +# +Entry number (sequentially from 0) + +Type +Memory type. EFI has a large number of memory types. The type is shown in +the format : where in is the format number in hex and is the +name. + +Physical +Physical address + +Virtual +Virtual address + +Size +Size of memory area in bytes + +Attributes +Shows a code for memory attributes. The key for this is shown below the +table. + +Example +--- + +:: + +=> efi mem +EFI table at 0, memory map 1ad38b60, size 1260, key a79, version 1, descr. size 0x30 + # Type Physical VirtualSize Attributes + 0 7:conv 00 00 0a f + 0a 06 + 1 7:conv 10 00 70 f + 2 a:acpi_nvs 80 00 008000 f + 3 7:conv 808000 00 008000 f + 4 a:acpi_nvs 81 00 0f f + 5 7:conv 90 00 001efef000 f + 6 6:rt_data 001f8ef000 00 10 rf + 7 5:rt_code 001f9ef000 00 10 rf + 8 0:reserved 001faef000 00 08 f + 9 9:acpi_reclaim 001fb6f000 00 01 f +10 a:acpi_nvs 001fb7f000 00 08 f +11 7:conv 001fbff000 00 359000 f +12 6:rt_data 001ff58000 00 02 rf +13 a:acpi_nvs 001ff78000 00 088000 f + 002000 009000 +14 0:reserved 00b000 00 001000 1 + +Attributes key: + f: uncached, write-coalescing, write-through, write-back +rf: uncached, write-coalescing, write-through, write-back, needs runtime mapping + 1: uncached +*Some areas are merged (use 'all' to see) + + +=> efi mem all +EFI table at 0, memory map 1ad38bb0, size 1260, key a79, version 1, descr. size 0x30 + # Type Physical VirtualSize Attributes + 0 3:bs_code 00 00 001000 f + 1 7:conv 001000 00 09f000 f + 0a 06 + 2 7:conv 10 00 70 f + 3 a:acpi_nvs 80 00 008000 f + 4 7:conv 808000 00 008000 f + 5 a:acpi_nvs 81 00 0f f + 6 4:bs_data 90 00 c0 f + 7 7:conv 000150 00 000aa36000 f + 8 2:loader_data 000bf36000 00 001000 f + 9 4:bs_data 001bf36000 00 02 f +10 7:conv 001bf56000 00 00021e1000 f +11 1:loader_code 001e137000 00 0c4000 f +12 7:conv 001e1fb000 00 09b000 f +13 1:loader_code 001e296000 00 0e2000 f +14 7:conv 001e378000 00 05b000 f +15 4:bs_data 001e3d3000 00 01e000 f +16 7:conv 001e3f1000 00 016000 f +17 4:bs_data 001e407000 00 016000 f +18 2:loader_data 001e41d000 00 002000 f +19 4:bs_data 001e41f000 00 828000 f +20 3:bs_code 001ec47000 00 045000 f +21 4:bs_data 001ec8c000 00 001000 f +22 3:bs_code 001ec8d000 00
Re: [PATCH v2 10/12] efi: Split out table-listing code into a new file
On 3/10/23 21:49, Simon Glass wrote:
This code is used with EFI_LOADER but is also useful (with some
modifications) for the EFI app and payload. Move it into a shared
file.
Show the address of the table so it can be examined if needed. Also show
the table name as unknown if necessary. Our list of GUIDs is fairly
small.
Signed-off-by: Simon Glass
---
Changes in v2:
- Add new patch to split out table-listing code into a new file
cmd/Makefile | 2 +-
cmd/efi_common.c | 26 ++
cmd/efidebug.c | 6 +-
include/efi.h| 9 +
4 files changed, 37 insertions(+), 6 deletions(-)
create mode 100644 cmd/efi_common.c
diff --git a/cmd/Makefile b/cmd/Makefile
index 2d8bb4fc052..1c5c6f3c00c 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -63,7 +63,7 @@ obj-$(CONFIG_CMD_ECHO) += echo.o
obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
obj-$(CONFIG_CMD_EEPROM) += eeprom.o
obj-$(CONFIG_EFI) += efi.o
-obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o
+obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o efi_common.o
obj-$(CONFIG_CMD_EFICONFIG) += eficonfig.o
ifdef CONFIG_CMD_EFICONFIG
ifdef CONFIG_EFI_MM_COMM_TEE
diff --git a/cmd/efi_common.c b/cmd/efi_common.c
new file mode 100644
index 000..7eedf0726a7
--- /dev/null
+++ b/cmd/efi_common.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Common code for EFI commands
+ *
+ * Copyright 2023 Google LLC
+ * Written by Simon Glass
+ */
+
+#include
+#include
+#include
+#include
+
+void efi_show_tables(struct efi_system_table *systab)
+{
+ int i;
+
+ for (i = 0; i < systab->nr_tables; i++) {
+ struct efi_configuration_table *tab = &systab->tables[i];
+ char guid_str[37];
+
+ uuid_bin_to_str(tab->guid.b, guid_str, 1);
+ printf("%p %s %s\n", tab->table, guid_str,
+ uuid_guid_get_str(tab->guid.b) ?: "(unknown)");
Please, use %pUl to print the UUID string.
Best regards
Heinrich
+ }
+}
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index e6959ede930..9622430c475 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -649,11 +649,7 @@ static int do_efi_show_memmap(struct cmd_tbl *cmdtp, int
flag,
static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag,
int argc, char *const argv[])
{
- efi_uintn_t i;
-
- for (i = 0; i < systab.nr_tables; ++i)
- printf("%pUl (%pUs)\n",
- &systab.tables[i].guid, &systab.tables[i].guid);
+ efi_show_tables(&systab);
return CMD_RET_SUCCESS;
}
diff --git a/include/efi.h b/include/efi.h
index c3087d3da28..342dd52fed9 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -637,4 +637,13 @@ int efi_call_exit_boot_services(void);
int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, uint *keyp,
int *desc_sizep, uint *versionp);
+/**
+ * efi_show_tables() - Show a list of available tables
+ *
+ * Shows the address, GUID (and name where known) for each table
+ *
+ * @systab: System table containing the list of tables
+ */
+void efi_show_tables(struct efi_system_table *systab);
+
#endif /* _LINUX_EFI_H */
Re: [PATCH v2 11/12] efi: Support showing tables
On 3/10/23 21:49, Simon Glass wrote:
Add a command (for the app and payload) to display the tables provided
by EFI.
Signed-off-by: Simon Glass
---
Changes in v2:
- Make use of common code
cmd/Makefile | 2 +-
cmd/efi.c | 33 -
doc/usage/cmd/efi.rst | 22 ++
3 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/cmd/Makefile b/cmd/Makefile
index 1c5c6f3c00c..a0bfa2acefe 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -62,7 +62,7 @@ obj-$(CONFIG_CMD_EXTENSION) += extension_board.o
obj-$(CONFIG_CMD_ECHO) += echo.o
obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
obj-$(CONFIG_CMD_EEPROM) += eeprom.o
-obj-$(CONFIG_EFI) += efi.o
+obj-$(CONFIG_EFI) += efi.o efi_common.o
obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o efi_common.o
obj-$(CONFIG_CMD_EFICONFIG) += eficonfig.o
ifdef CONFIG_CMD_EFICONFIG
diff --git a/cmd/efi.c b/cmd/efi.c
index c0384e0db28..4d0edfa7f27 100644
--- a/cmd/efi.c
+++ b/cmd/efi.c
@@ -7,10 +7,12 @@
#include
#include
#include
+#include
#include
#include
#include
#include
+#include
#include
DECLARE_GLOBAL_DATA_PTR;
@@ -273,8 +275,36 @@ done:
return ret ? CMD_RET_FAILURE : 0;
}
+static int do_efi_tables(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
+{
+ struct efi_system_table *systab;
+
+ if (IS_ENABLED(CONFIG_EFI_APP)) {
+ systab = efi_get_sys_table();
+ if (!systab) {
+ printf("Cannot read system table\n");
+ return CMD_RET_FAILURE;
+ }
+ } else {
+ int size;
+ int ret;
+
+ ret = efi_info_get(EFIET_SYS_TABLE, (void **)&systab, &size);
+ if (ret) {
+ printf("Cannot find EFI system table (err=%d)\n", ret);
+ return CMD_RET_FAILURE;
Wouldn't U-Boot have failed earlier if there is no system table?
Best regards
Heinrich
+ }
+ }
+
+ efi_show_tables(systab);
+
+ return 0;
+}
+
static struct cmd_tbl efi_commands[] = {
U_BOOT_CMD_MKENT(mem, 1, 1, do_efi_mem, "", ""),
+ U_BOOT_CMD_MKENT(tables, 1, 1, do_efi_tables, "", ""),
};
static int do_efi(struct cmd_tbl *cmdtp, int flag, int argc, char *const
argv[])
@@ -298,5 +328,6 @@ static int do_efi(struct cmd_tbl *cmdtp, int flag, int
argc, char *const argv[])
U_BOOT_CMD(
efi, 3, 1, do_efi,
"EFI access",
- "mem [all]Dump memory information [include boot services]"
+ "mem [all]Dump memory information [include boot services]\n"
+ "tables Dump tables"
);
diff --git a/doc/usage/cmd/efi.rst b/doc/usage/cmd/efi.rst
index c029c423879..ef37ff2f4c1 100644
--- a/doc/usage/cmd/efi.rst
+++ b/doc/usage/cmd/efi.rst
@@ -10,6 +10,7 @@ Synopsis
::
efi mem [all]
+efi tables
Description
---
@@ -54,6 +55,14 @@ Attributes
Shows a code for memory attributes. The key for this is shown below the
table.
+efi tables
+~~
+
+This shows a list of the EFI tables provided in the system table. These use
+GUIDs so it is not possible in general to show the name of a table. But some
+effort is made to provide a useful table, where the GUID is known by U-Boot.
+
+
Example
---
@@ -195,3 +204,16 @@ Example
f: uncached, write-coalescing, write-through, write-back
rf: uncached, write-coalescing, write-through, write-back, needs runtime
mapping
1: uncached
+
+
+=> efi tables
+1f8edf98 ee4e5898-3914-4259-9d6e-dc7bd79403cf EFI_LZMA_COMPRESSED
+1ff2ace0 05ad34ba-6f02-4214-952e-4da0398e2bb9 EFI_DXE_SERVICES
+1f8ea018 7739f24c-93d7-11d4-9a3a-0090273fc14d EFI_HOB_LIST
+1ff2bac0 4c19049f-4137-4dd3-9c10-8b97a83ffdfa EFI_MEMORY_TYPE
+1ff2cb10 49152e77-1ada-4764-b7a2-7afefed95e8b (unknown)
+1f9ac018 060cc026-4c0d-4dda-8f41-595fef00a502
EFI_MEM_STATUS_CODE_REC
+1f9ab000 eb9d2d31-2d88-11d3-9a16-0090273fc14d SMBIOS table
+1fb7e000 eb9d2d30-2d88-11d3-9a16-0090273fc14d EFI_GUID_EFI_ACPI1
+1fb7e014 8868e871-e4f1-11d3-bc22-0080c73c8881 ACPI table
+1e654018 dcfa911d-26eb-469f-a220-38b7dc461220 (unknown)
Re: [PATCH 2/2] efi_loader: fix device-path for USB devices
On 3/19/23 20:29, Simon Glass wrote:
Hi Heinrich,
On Mon, 20 Mar 2023 at 04:25, Heinrich Schuchardt
wrote:
EFI device paths for block devices must be unique. If a non-unique device
path is discovered, probing of the block device fails.
Currently we use UsbClass() device path nodes. As multiple devices may
have the same vendor and product id these are non-unique. Instead we
should use Usb() device path nodes. They include the USB port on the
parent hub. Hence they are unique.
A USB storage device may contain multiple logical units. These can be
modeled as Ctrl() nodes.
Reported-by: Patrick Delaunay
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_device_path.c | 45 +++-
1 file changed, 33 insertions(+), 12 deletions(-)
Reviewed-by: Simon Glass
[..]
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 3b267b713e..b6dd575b13 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -147,7 +147,7 @@ struct efi_device_path *efi_dp_shorten(struct
efi_device_path *dp)
* in practice fallback.efi just uses MEDIA:HARD_DRIVE
* so not sure when we would see these other cases.
*/
- if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB_CLASS) ||
+ if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB) ||
EFI_DP_TYPE(dp, MEDIA_DEVICE, HARD_DRIVE_PATH) ||
EFI_DP_TYPE(dp, MEDIA_DEVICE, FILE_PATH))
return dp;
@@ -564,6 +564,11 @@ __maybe_unused static unsigned int dp_size(struct udevice
*dev)
return dp_size(dev->parent)
+ sizeof(struct efi_device_path_vendor) + 1;
#endif
+#ifdef CONFIG_USB
+ case UCLASS_MASS_STORAGE:
Can we do:
case UCLASS_MASS_STORAGE:
if (IS_ENABLED(CONFIG_USB)) {
...
}
?
That should be possible too. Didn't you want to get rid of IS_ENABLED()?
CONFIG_IS_ENABLED() would work here too.
The whole way that we create device paths is not consistent. We should
have a device path node for each DM device.
With v2023.07 I want to add
struct efi_device_path *(*get_dp_node)(struct udevice *dev);
to struct uclass_driver and move the generation of device path nodes to
the individual uclass drivers.
Best regards
Heinrich
and below too
+ return dp_size(dev->parent)
+ + sizeof(struct efi_device_path_controller);
+#endif
#ifdef CONFIG_VIRTIO_BLK
case UCLASS_VIRTIO:
/*
@@ -585,7 +590,7 @@ __maybe_unused static unsigned int dp_size(struct udevice
*dev)
case UCLASS_MASS_STORAGE:
case UCLASS_USB_HUB:
return dp_size(dev->parent) +
- sizeof(struct efi_device_path_usb_class);
+ sizeof(struct efi_device_path_usb);
default:
/* just skip over unknown classes: */
return dp_size(dev->parent);
@@ -741,6 +746,19 @@ __maybe_unused static void *dp_fill(void *buf, struct
udevice *dev)
memcpy(&dp->ns_id, &ns_id, sizeof(ns_id));
return &dp[1];
}
+#endif
+#if defined(CONFIG_USB)
+ case UCLASS_MASS_STORAGE: {
+ struct blk_desc *desc = desc = dev_get_uclass_plat(dev);
+ struct efi_device_path_controller *dp =
+ dp_fill(buf, dev->parent);
+
+ dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
+ dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CONTROLLER;
+ dp->dp.length = sizeof(*dp);
+ dp->controller_number = desc->lun;
+ return &dp[1];
+ }
#endif
[..]
Regards,
SImon
Re: [PATCH 1/2] efi_loader: move dp_alloc() to efi_alloc()
On 3/20/23 08:38, Ilias Apalodimas wrote:
Hi Heinrich,
On Sun, Mar 19, 2023 at 09:20:22AM +0100, Heinrich Schuchardt wrote:
The incumbent function efi_alloc() is unused.
Replace dp_alloc() by a new function efi_alloc() that we can use more
widely.
[...]
#include
#include
+#include
#include
#include
#include
@@ -533,27 +536,6 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type
type,
return EFI_SUCCESS;
}
-/**
- * efi_alloc() - allocate memory pages
- *
- * @len: size of the memory to be allocated
- * @memory_type: usage type of the allocated memory
- * Return: pointer to the allocated memory area or NULL
- */
-void *efi_alloc(uint64_t len, int memory_type)
-{
- uint64_t ret = 0;
- uint64_t pages = efi_size_in_pages(len);
- efi_status_t r;
-
- r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, memory_type, pages,
- &ret);
- if (r == EFI_SUCCESS)
- return (void*)(uintptr_t)ret;
-
- return NULL;
-}
-
/**
* efi_free_pages() - free memory pages
*
@@ -672,6 +654,28 @@ efi_status_t efi_allocate_pool(enum efi_memory_type
pool_type, efi_uintn_t size,
return r;
}
+/**
+ * efi_alloc() - allocate boot services data pool memory
+ *
+ * Allocate memory from pool and zero it out.
+ *
+ * @size: number of bytes to allocate
+ * Return: pointer to allocated memory or NULL
+ */
+void *efi_alloc(size_t size)
All our allocation related functions require the memory type to be passed.
If we want to default this to 'EFI_BOOT_SERVICES_DATA' I think we need to
change the name a bit to indicate that.
We have 13 instances of allocating EFI_BOOT_SERVICES_DATA from the pool
and only 2 instances (see below) of other memory types. So this is the
only case worth factoring out. I would prefer to keep identifiers short.
+{
+ void *buf;
+
+ if (efi_allocate_pool(EFI_BOOT_SERVICES_DATA, size, &buf) !=
Is there a reason we are using efi_allocate_pool instead of
efi_allocate_pages?
The UEFI specification explicitly requires pool memory in most places
where the caller has to free memory. Freeing pages requires to know the
size of the memory area while freeing from pool does not. Furthermore
when freeing from pool we can check if the memory was ever allocated.
In future we may want to make small allocations from pool more efficient
by not consuming a minimum of one page.
We could rethink why we are allocating configuration tables from pool in
the following instances:
lib/efi_loader/efi_tcg2.c:1703
create_final_event()
lib/efi_loader/efi_runtime.c:116
efi_init_runtime_supported()
Best regards
Heinrich
+ EFI_SUCCESS) {
+ log_err("out of memory");
+ return NULL;
+ }
+ memset(buf, 0, size);
+
+ return buf;
+}
+
/**
* efi_free_pool() - free memory from pool
*
--
2.39.2
Thanks
/Ilias
Re: [PATCH v3 1/4] efi_loader: store firmware version into FmpState variable
On 3/20/23 06:54, Masahisa Kojima wrote:
Firmware version management is not implemented in the current
FMP protocol.
EDK2 reference implementation capsule generation script inserts
the FMP Payload Header right before the payload, it contains the
firmware version and lowest supported version.
This commit utilizes the FMP Payload Header, read the header and
%s/read/reads/
stores the firmware version, lowest supported version,
last attempt version and last attempt status into "FmpState"
EFI non-volatile variable. indicates the image index,
since FMP protocol handles multiple image indexes.
This change is compatible with the existing FMP implementation.
This change does not mandate the FMP Payload Header.
If no FMP Payload Header is found in the capsule file, fw_version,
lowest supported version, last attempt version and last attempt
status is 0 and this is the same behavior as existing FMP
implementation.
Signed-off-by: Masahisa Kojima
---
Changes in v3:
- exclude CONFIG_FWU_MULTI_BANK_UPDATE case
- set image_type_id as a vendor field of FmpState variable
- set READ_ONLY flag for FmpState variable
- add error code for FIT image case
Changes in v2:
- modify indent
lib/efi_loader/efi_firmware.c | 224 ++
1 file changed, 201 insertions(+), 23 deletions(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 93e2b01c07..d6f3741024 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -36,6 +37,24 @@ struct fmp_payload_header {
u32 lowest_supported_version;
};
+/**
+ * struct fmp_state - fmp firmware update state
+ *
+ * This structure describes the state of the firmware update
+ * through FMP protocol.
+ *
+ * @fw_version:Firmware versions used
+ * @lowest_supported_version: Lowest supported version
+ * @last_attempt_version: Last attempt version
+ * @last_attempt_status: Last attempt status
+ */
+struct fmp_state {
+ u32 fw_version;
+ u32 lowest_supported_version;
+ u32 last_attempt_version;
+ u32 last_attempt_status;
+};
+
__weak void set_dfu_alt_info(char *interface, char *devstr)
{
env_set("dfu_alt_info", update_info.dfu_string);
@@ -102,6 +121,29 @@ efi_status_t EFIAPI
efi_firmware_set_package_info_unsupported(
return EFI_EXIT(EFI_UNSUPPORTED);
}
+/**
+ * efi_firmware_get_image_type_id - get image_type_id
+ * @image_index: image index
+ *
+ * Return the image_type_id identified by the image index.
+ *
+ * Return: pointer to the image_type_id, NULL if image_index is
invalid
+ */
+static
+efi_guid_t *efi_firmware_get_image_type_id(u8 image_index)
+{
+ int i;
+ struct efi_fw_image *fw_array;
+
+ fw_array = update_info.images;
+ for (i = 0; i < num_image_type_guids; i++) {
+ if (fw_array[i].image_index == image_index)
+ return &fw_array[i].image_type_id;
+ }
+
+ return NULL;
+}
+
/**
* efi_fill_image_desc_array - populate image descriptor array
* @image_info_size: Size of @image_info
@@ -182,6 +224,7 @@ static efi_status_t efi_fill_image_desc_array(
* efi_firmware_capsule_authenticate - authenticate the capsule if enabled
* @p_image: Pointer to new image
* @p_image_size: Pointer to size of new image
+ * @state Pointer to fmp state
*
* Authenticate the capsule if authentication is enabled.
* The image pointer and the image size are updated in case of success.
@@ -190,12 +233,11 @@ static efi_status_t efi_fill_image_desc_array(
*/
static
efi_status_t efi_firmware_capsule_authenticate(const void **p_image,
- efi_uintn_t *p_image_size)
+ efi_uintn_t *p_image_size,
+ struct fmp_state *state)
{
const void *image = *p_image;
efi_uintn_t image_size = *p_image_size;
- u32 fmp_hdr_signature;
- struct fmp_payload_header *header;
void *capsule_payload;
efi_status_t status;
efi_uintn_t capsule_payload_size;
@@ -209,8 +251,12 @@ efi_status_t efi_firmware_capsule_authenticate(const void
**p_image,
if (status == EFI_SECURITY_VIOLATION) {
printf("Capsule authentication check failed. Aborting
update\n");
+ state->last_attempt_status =
+ LAST_ATTEMPT_STATUS_ERROR_AUTH_ERROR;
return status;
} else if (status != EFI_SUCCESS) {
+ state->last_attempt_status =
+ LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
return status;
}
@@ -222,24 +268,130 @@ efi_status_t efi_fi
Re: [PATCH v3 3/4] efi_loader: check lowest supported version in capsule update
On 3/20/23 06:54, Masahisa Kojima wrote:
The FMP Payload Header which EDK2 capsule generation scripts
insert contains lowest supported version.
This commit reads the lowest supported version stored in the
"FmpState" EFI non-volatile variable, then check if the
firmware version of ongoing capsule is equal or greater than
the lowest supported version.
Signed-off-by: Masahisa Kojima
---
No changes since v2
Changes in v2:
- add error message when the firmware version is lower than
lowest supported version
lib/efi_loader/efi_firmware.c | 50 ++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 289456ecbb..8d6e32006d 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -391,6 +391,39 @@ void efi_firmware_parse_payload_header(const void
**p_image,
*p_image_size = image_size;
}
+/**
+ * efi_firmware_get_lowest_supported_version - get the lowest supported version
+ * @image_index: image_index
+ *
+ * Get the lowest supported version from FmpState variable.
+ *
+ * Return: lowest supported version, return 0 if reading
FmpState
+ * variable failed
+ */
+static
+u32 efi_firmware_get_lowest_supported_version(u8 image_index)
+{
+ u16 varname[13]; /* u"FmpState" */
+ efi_status_t ret;
+ efi_uintn_t size;
+ efi_guid_t *image_type_id;
+ struct fmp_state var_state = { 0 };
+
+ image_type_id = efi_firmware_get_image_type_id(image_index);
+ if (!image_type_id)
+ return 0;
+
+ efi_create_indexed_name(varname, sizeof(varname), "FmpState",
+ image_index);
+ size = sizeof(var_state);
+ ret = efi_get_variable_int(varname, image_type_id, NULL, &size,
+ &var_state, NULL);
+ if (ret != EFI_SUCCESS)
+ return 0;
+
+ return var_state.lowest_supported_version;
+}
+
/**
* efi_firmware_verify_image - verify image
* @p_image: Pointer to new image
@@ -398,7 +431,8 @@ void efi_firmware_parse_payload_header(const void **p_image,
* @image_index Image index
* @state Pointer to fmp state
*
- * Verify the capsule file
+ * Verify the capsule authentication and check if the fw_version
+ * is equal or greater than the lowest supported version.
*
* Return:status code
*/
@@ -409,10 +443,24 @@ efi_status_t efi_firmware_verify_image(const void
**p_image,
struct fmp_state *state)
{
efi_status_t ret;
+ u32 lowest_supported_version;
ret = efi_firmware_capsule_authenticate(p_image, p_image_size, state);
efi_firmware_parse_payload_header(p_image, p_image_size, state);
+ /* check lowest_supported_version if capsule authentication passes */
+ if (ret == EFI_SUCCESS) {
+ lowest_supported_version =
+ efi_firmware_get_lowest_supported_version(image_index);
+ if (lowest_supported_version > state->fw_version) {
+ printf("fw_version(%u) is too low(expected >%u). Aborting
update\n",
+ state->fw_version, lowest_supported_version);
Please, use log_warning() or log_err() instead of printf().
The addressee is a user not a developer:
"Firmware version %u too low. Expecting >= %u"
We should have one central place where upon exit we write a message
indicating that either the capsule update was successful or failed.
"Firmware updated to version %u" : "Firmware update failed"
Best regards
Heinrich
+ state->last_attempt_status =
+ LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION;
+ ret = EFI_INVALID_PARAMETER;
+ }
+ }
+
return ret;
}
Re: [PATCH 2/2] efi_loader: fix device-path for USB devices
On 3/20/23 19:39, Simon Glass wrote:
Hi Heinrich,
On Mon, 20 Mar 2023 at 09:58, Heinrich Schuchardt
wrote:
On 3/19/23 20:29, Simon Glass wrote:
Hi Heinrich,
On Mon, 20 Mar 2023 at 04:25, Heinrich Schuchardt
wrote:
EFI device paths for block devices must be unique. If a non-unique device
path is discovered, probing of the block device fails.
Currently we use UsbClass() device path nodes. As multiple devices may
have the same vendor and product id these are non-unique. Instead we
should use Usb() device path nodes. They include the USB port on the
parent hub. Hence they are unique.
A USB storage device may contain multiple logical units. These can be
modeled as Ctrl() nodes.
Reported-by: Patrick Delaunay
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_device_path.c | 45 +++-
1 file changed, 33 insertions(+), 12 deletions(-)
Reviewed-by: Simon Glass
[..]
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 3b267b713e..b6dd575b13 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -147,7 +147,7 @@ struct efi_device_path *efi_dp_shorten(struct
efi_device_path *dp)
* in practice fallback.efi just uses MEDIA:HARD_DRIVE
* so not sure when we would see these other cases.
*/
- if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB_CLASS) ||
+ if (EFI_DP_TYPE(dp, MESSAGING_DEVICE, MSG_USB) ||
EFI_DP_TYPE(dp, MEDIA_DEVICE, HARD_DRIVE_PATH) ||
EFI_DP_TYPE(dp, MEDIA_DEVICE, FILE_PATH))
return dp;
@@ -564,6 +564,11 @@ __maybe_unused static unsigned int dp_size(struct udevice
*dev)
return dp_size(dev->parent)
+ sizeof(struct efi_device_path_vendor) + 1;
#endif
+#ifdef CONFIG_USB
+ case UCLASS_MASS_STORAGE:
Can we do:
case UCLASS_MASS_STORAGE:
if (IS_ENABLED(CONFIG_USB)) {
...
}
?
That should be possible too. Didn't you want to get rid of IS_ENABLED()?
CONFIG_IS_ENABLED() would work here too.
I was wanting to get rid of CONFIG_IS_ENABLED() for things that don't
have an SPL Kconfig, then eventually get rid of CONFIG_IS_ENABLED().
I've got a bit lost in all the problems though.
The whole way that we create device paths is not consistent. We should
have a device path node for each DM device.
With v2023.07 I want to add
struct efi_device_path *(*get_dp_node)(struct udevice *dev);
to struct uclass_driver and move the generation of device path nodes to
the individual uclass drivers.
We could also send an event (EVT_EFI_GET_DP_NODE) and connect it that
way...ie would add less space to driver model. But yes it would be
good to line up EFI and DM a bit better.
The type of device-path node to be created is uclass specific:
For an NVMe device we will always create a NVMe() node.
For a block device we will always create a Ctrl() node with the LUN number.
...
For drivers that don't implement the method yet we can create a VenHw()
node per default with uclass-id and device number encoded in the node.
You suggested yourself that the DM and the EFI implementation should be
tightly integrated.
I cannot see what the use of an event should be. Why should each udevice
register to an event when all we need is a simple function like:
#if CONFIG_IS_ENABLED(EFI_LOADER)
struct efi_device_path *uclass_get_dp_node(struct udevice *dev)
{
struct uclass *uc;
struct efi_device_path_uboot *dp;
uc = dev->uclass;
if (uc->uc_drv->get_dp_node)
return uc->uc_drv->get_dp_node(dev);
dp = efi_alloc(sizeof(struct efi_device_path_uboot));
if (!dp)
return NULL;
dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
dp->dp.length = sizeof(struct efi_device_path_uboot);
dp->guid = efi_u_boot_guid;
dp->uclass_id = uc->uc_drv->id;
dp->seq_ = dev->seq_;
return &dp->dp;
}
#endif
Best regards
Heinrich
Re: [PATCH] efi_loader: define allow_unaligned as 'asm volatile'
On 3/20/23 18:13, Ilias Apalodimas wrote:
Tom reports that compiling with LTO & EFI breaks armv7 and arm11*
builds. The reason is that LTO (maybe a binutils bug?) replaces the
asm version of allow_unaligned() with the __weak definition of the
function. As a result EFI code ends up running with unaligned accesses
disabled and eventually crashes.
allow_unaligned() hardware specific variants are usually defined in .S
files. Switching those to inline assembly fixes the problem and the
linker keeps the correct version in the final binary
Reported-by: Tom Rini
Signed-off-by: Ilias Apalodimas
---
This is an alternative approach to
https://lore.kernel.org/u-boot/ZBRy1oSZ5iVFqrdV@hera/
since enabling unaligned accesses by default has been rejected in the past
arch/arm/cpu/arm11/Makefile | 4
arch/arm/cpu/arm11/cpu.c| 13 +
arch/arm/cpu/arm11/sctlr.S | 25 -
arch/arm/cpu/armv7/Makefile | 1 -
arch/arm/cpu/armv7/cpu.c| 11 +++
arch/arm/cpu/armv7/sctlr.S | 22 --
6 files changed, 24 insertions(+), 52 deletions(-)
delete mode 100644 arch/arm/cpu/arm11/sctlr.S
delete mode 100644 arch/arm/cpu/armv7/sctlr.S
diff --git a/arch/arm/cpu/arm11/Makefile b/arch/arm/cpu/arm11/Makefile
index 5dfa01ae8d05..5d721fce12b5 100644
--- a/arch/arm/cpu/arm11/Makefile
+++ b/arch/arm/cpu/arm11/Makefile
@@ -4,7 +4,3 @@
# Wolfgang Denk, DENX Software Engineering, w...@denx.de.
obj-y = cpu.o
-
-ifneq ($(CONFIG_SPL_BUILD),y)
-obj-$(CONFIG_EFI_LOADER) += sctlr.o
-endif
diff --git a/arch/arm/cpu/arm11/cpu.c b/arch/arm/cpu/arm11/cpu.c
index ffe35111d589..b19c4fc4a6c2 100644
--- a/arch/arm/cpu/arm11/cpu.c
+++ b/arch/arm/cpu/arm11/cpu.c
@@ -111,3 +111,16 @@ void enable_caches(void)
#endif
}
#endif
+
+#if !IS_ENABLED(CONFIG_SPL_BUILD)
+void allow_unaligned(void)
+{
+ asm volatile(
+ "mrc p15, 0, r0, c1, c0, 0\n" //load system control register
+ "orr r0, r0, #1 << 22\n" //set unaligned data support flag
+ "bic r0, r0, #2\n" //clear aligned flag
+ "mcr p15, 0, r0, c1, c0, 0\n" // write system control register
+ "bxlr\n" //return
+ );
+}
+#endif
diff --git a/arch/arm/cpu/arm11/sctlr.S b/arch/arm/cpu/arm11/sctlr.S
deleted file mode 100644
index 74a7fc4a25b6..
--- a/arch/arm/cpu/arm11/sctlr.S
+++ /dev/null
@@ -1,25 +0,0 @@
-/* SPDX-License-Identifier:GPL-2.0+ */
-/*
- * Routines to access the system control register
- *
- * Copyright (c) 2019 Heinrich Schuchardt
- */
-
-#include
-
-/*
- * void allow_unaligned(void) - allow unaligned access
- *
- * This routine sets the enable unaligned data support flag and clears the
- * aligned flag in the system control register.
- * After calling this routine unaligned access does no longer leads to a
- * data abort or undefined behavior but is handled by the CPU.
- * For details see the "ARM Architecture Reference Manual" for ARMv6.
- */
-ENTRY(allow_unaligned)
- mrc p15, 0, r0, c1, c0, 0 @ load system control register
- orr r0, r0, #1 << 22 @ set unaligned data support flag
- bic r0, r0, #2 @ clear aligned flag
- mcr p15, 0, r0, c1, c0, 0 @ write system control register
- bx lr @ return
-ENDPROC(allow_unaligned)
diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
index 653eef8ad79e..ca50f6e93e10 100644
--- a/arch/arm/cpu/armv7/Makefile
+++ b/arch/arm/cpu/armv7/Makefile
@@ -13,7 +13,6 @@ obj-y += syslib.o
obj-$(CONFIG_SYS_ARM_MPU) += mpu_v7r.o
ifneq ($(CONFIG_SPL_BUILD),y)
-obj-$(CONFIG_EFI_LOADER) += sctlr.o
obj-$(CONFIG_ARMV7_NONSEC) += exception_level.o
endif
diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c
index 68807d209978..9742fa65e3cf 100644
--- a/arch/arm/cpu/armv7/cpu.c
+++ b/arch/arm/cpu/armv7/cpu.c
@@ -83,3 +83,14 @@ int cleanup_before_linux(void)
{
return cleanup_before_linux_select(CBL_ALL);
}
+
+#if !IS_ENABLED(CONFIG_SPL_BUILD)
Why do we need this #if? The linker should eliminate unused functions.
Best regards
Heinrich
+void allow_unaligned(void)
+{
+ asm volatile(
+ "mrc p15, 0, r0, c1, c0, 0\n" //load system control register
+ "bic r0, r0, #2\n" //clear aligned flag
+ "mcr p15, 0, r0, c1, c0, 0\n" //write system control register
+ );
+}
+#endif
diff --git a/arch/arm/cpu/armv7/sctlr.S b/arch/arm/cpu/armv7/sctlr.S
deleted file mode 100644
index bd56e41afe18..
--- a/arch/arm/cpu/armv7/sctlr.S
+++ /dev/null
@@ -1,22 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * Routines to access the system control register
- *
- * Copyright (c) 2018 Heinrich Schuchardt
- */
-
-#include
-
-/*
- * void allow_unaligned(void) - all
Re: [PATCH] efi_driver: fix duplicate efiblk#0 issue
On 7/3/23 04:47, Masahisa Kojima wrote:
The devnum value of the blk_desc structure starts from 0,
current efi_bl_create_block_device() function creates
two "efiblk#0" devices for the cases that blk_find_max_devnum()
returns -ENODEV and blk_find_max_devnum() returns 0(one device
found in this case).
The devnum value for the "efiblk" name needs to be incremented.
Signed-off-by: Masahisa Kojima
---
lib/efi_driver/efi_block_device.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index add00eeebb..e37bfe6e80 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -129,6 +129,8 @@ efi_bl_create_block_device(efi_handle_t handle, void
*interface)
devnum = 0;
else if (devnum < 0)
return EFI_OUT_OF_RESOURCES;
+ else
+ devnum++; /* device found, note that devnum starts from 0 */
Shouldn't we simply use blk_next_free_devnum() instead of duplicating
the logic here?
Best regards
Heinrich
name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */
if (!name)
Re: [PATCH v2] efi_driver: fix duplicate efiblk#0 issue
On 7/3/23 08:08, Masahisa Kojima wrote:
The devnum value of the blk_desc structure starts from 0,
current efi_bl_create_block_device() function creates
two "efiblk#0" devices for the cases that blk_find_max_devnum()
returns -ENODEV and blk_find_max_devnum() returns 0(one device
found in this case).
This commit uses blk_next_free_devnum() instead of blk_find_max_devnum().
Signed-off-by: Masahisa Kojima
---
Changes in v2:
- uses blk_next_free_devnum() instead of blk_find_max_devnum()
lib/efi_driver/efi_block_device.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index add00eeebb..e3abd90275 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -124,10 +124,8 @@ efi_bl_create_block_device(efi_handle_t handle, void
*interface)
struct efi_block_io *io = interface;
struct efi_blk_plat *plat;
- devnum = blk_find_max_devnum(UCLASS_EFI_LOADER);
- if (devnum == -ENODEV)
- devnum = 0;
- else if (devnum < 0)
+ devnum = blk_next_free_devnum(UCLASS_EFI_LOADER);
+ if (devnum < 0)
return EFI_OUT_OF_RESOURCES;
Reviewed-by: Heinrich Schuchardt
name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */
Re: [PATCH v2] efi_driver: fix duplicate efiblk#0 issue
On 03.07.23 15:30, Simon Glass wrote:
Hi Masahisa,
On Mon, 3 Jul 2023 at 07:09, Masahisa Kojima wrote:
The devnum value of the blk_desc structure starts from 0,
current efi_bl_create_block_device() function creates
two "efiblk#0" devices for the cases that blk_find_max_devnum()
returns -ENODEV and blk_find_max_devnum() returns 0(one device
found in this case).
This commit uses blk_next_free_devnum() instead of blk_find_max_devnum().
Fixes: 05ef48a2484b ("efi_driver: EFI block driver")
Signed-off-by: Masahisa Kojima
---
Changes in v2:
- uses blk_next_free_devnum() instead of blk_find_max_devnum()
lib/efi_driver/efi_block_device.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index add00eeebb..e3abd90275 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -124,10 +124,8 @@ efi_bl_create_block_device(efi_handle_t handle, void
*interface)
struct efi_block_io *io = interface;
struct efi_blk_plat *plat;
- devnum = blk_find_max_devnum(UCLASS_EFI_LOADER);
Simon, this line was last changed by your patch
e33a5c6be55e ("blk: Switch over to using uclass IDs")
- if (devnum == -ENODEV)
- devnum = 0;
- else if (devnum < 0)
+ devnum = blk_next_free_devnum(UCLASS_EFI_LOADER);
This really should be an internal function but I see it was exported
as part of the virtio work.
How come the EFI and DM block devices are getting out of sync?
They never were in sync:
The bug dates back to Jan 2018:
05ef48a2484b ("efi_driver: EFI block driver")
Best regards
Heinrich
Anyway this function is munging around in the internals of the device
and should be fixed before it causes more problems.
For now, I suggest following what most other drivers so which is to
call blk_create_devicef() passing a devnum of -1.
+ if (devnum < 0)
return EFI_OUT_OF_RESOURCES;
name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */
--
2.34.1
Regards,
Simon
[PATCH 1/1] RISC-V: CONFIG_SPL_OPENSBI_SCRATCH_OPTIONS description
Describe which numeric values can be used for as scratch options for OpenSBI. Signed-off-by: Heinrich Schuchardt --- common/spl/Kconfig | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/spl/Kconfig b/common/spl/Kconfig index 2c042ad306..7f99889ec3 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -1524,8 +1524,10 @@ config SPL_OPENSBI_SCRATCH_OPTIONS default 0x1 depends on SPL_OPENSBI help - Options passed to fw_dynamic, for example SBI_SCRATCH_NO_BOOT_PRINTS or - SBI_SCRATCH_DEBUG_PRINTS. + This bitmap of options is passed from U-Boot SPL to OpenSBI. + As of OpenSBI 1.3 the following bits are defined: + - SBI_SCRATCH_NO_BOOT_PRINTS = 0x1 (Disable prints during boot) + - SBI_SCRATCH_DEBUG_PRINTS = 0x2 (Enable runtime debug prints) config SPL_TARGET string "Addtional build targets for 'make'" -- 2.40.1
[PATCH 1/1] tools: spkgimage: correct printf specifier
Compiling on armv7 results in:
tools/renesas_spkgimage.c: In function ‘spkgimage_parse_config_line’:
tools/renesas_spkgimage.c:76:66: warning: format ‘%ld’ expects
argument of type ‘long int’, but argument 3 has type ‘size_t’
{aka ‘unsigned int’} [-Wformat=]
76 | "config error: unknown keyword on line %ld\n",
|~~^
| |
| long int
|%d
77 | line_num);
|
| |
| size_t {aka unsigned int}
The correct printf specifier for size_t is '%zu'.
Fixes: afdfcb11f97c ("tools: spkgimage: add Renesas SPKG format")
Signed-off-by: Heinrich Schuchardt
---
tools/renesas_spkgimage.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/renesas_spkgimage.c b/tools/renesas_spkgimage.c
index fa0a468cc4..605d0ab149 100644
--- a/tools/renesas_spkgimage.c
+++ b/tools/renesas_spkgimage.c
@@ -73,7 +73,7 @@ static int spkgimage_parse_config_line(char *line, size_t
line_num)
conf.padding = check_range(name, value, 1, INT_MAX);
} else {
fprintf(stderr,
- "config error: unknown keyword on line %ld\n",
+ "config error: unknown keyword on line %zu\n",
line_num);
return -EINVAL;
}
--
2.40.1
Re: EFI Secure boot default keys
On 06.07.23 11:25, AKASHI Takahiro wrote:
On Thu, Jul 06, 2023 at 08:23:06AM +, Neil Jones wrote:
Please can someone describe the format of the file needed for the default /
built-in EFI secure boot keys (ubootefi.var)
The only docs I have found suggest its best to enroll the keys from within
u-boot onto some removable media, then copy this off and use this as the
default, this is not very helpful and doesn't work for me:
=> fatload mmc 0:1 ${loadaddr} PK.aut
2053 bytes read in 18 ms (111.3 KiB/s)
=> setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize PK
setenv - set environment variables
Usage:
setenv setenv [-f] name value ...
- [forcibly] set environment variable 'name' to 'value ...'
setenv [-f] name
- [forcibly] delete environment variable 'name'
my setenv doesn't support all the extra switches ? This is with 2022.04, all
other EFI options seem to be in this release and I can boot unsigned EFI images
ok.
Please turn on CONFIG_CMD_NVEDIT_EFI when building your U-Boot.
This option was disabled by the commit:
commit 3b728f8728fa (tag: efi-2020-01-rc1)
Author: Heinrich Schuchardt
Date: Sun Oct 6 15:44:22 2019 +0200
cmd: disable CMD_NVEDIT_EFI by default
The binary size of efi has grown much since in the past, though.
-Takahiro Akashi
Thanks, I have secure boot working now. A tool to generate the ubootefi.var
offline or even just a description of the file format would be very useful.
Thank you for the suggestion. While I'd like to defer to Heinrich,
the C definition of the file format can be found as struct efi_var_file
in include/efi_variable.h
Thanks!
I have noticed one issue when using ubootefi.var on mmc, when I switch boot
order it wipes out the keys and I have to re-enrol them:
=> fatls mmc 0:1
3040 ubootefi.var
1 file(s), 0 dir(s)
I'm not sure that secure boot related variables have been loaded
at this point.
This is during initial generation / enrollment of the variables
Hello Neil,
If you want to use secure boot, please either use
CONFIG_EFI_VARIABLES_PRESEED or CONFIG_EFI_MM_COMM_TEE.
U-Boot will never load the security database from file.
Without CONFIG_EFI_MM_COMM_TEE=y setting up the security database via
setenv -e is only usable for one time testing. After reboot the security
database will be gone (or fallback to the preseed for
CONFIG_EFI_VARIABLES_PRESEED=y).
Best regards
Heinrich
Anyhow, please try to enable CONFIG_EFI_VARIABLES_PRESEED with
EFI_VAR_FILE_NAME set. Otherwise, those variables will never be
restored.
(This is another topic that are not described in doc/develop/uefi.)
I have CONFIG_EFI_VARIABLES_PRESEED working, but while generating the file
ubootefi.var for the first time (without CONFIG_EFI_VARIABLES_PRESEED set) you
have to follow a specific order, or the file gets overwritten eg:
Working:
efidebug boot order 1 2
efidebug boot add -b 1 Signed mmc 0:1 /ImageSig.efi
efidebug boot add -b 2 UnSigned mmc 0:1 /Image
fatload mmc 0:1 ${loadaddr} PK.aut
setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize PK
fatload mmc 0:1 ${loadaddr} KEK.aut
setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize KEK
fatload mmc 0:1 ${loadaddr} DB.aut
setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize db
Failing:
setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize PK
fatload mmc 0:1 ${loadaddr} KEK.aut
setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize KEK
fatload mmc 0:1 ${loadaddr} DB.aut
setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize db
efidebug boot order 1 2 ### This command overwrites the keys just loaded
Are you sure that "env print -e" shows all the variables including
PK, KEK and db at this point?
Since I don't have enough time to examine this issue, can you please
try to trace efi_var_collect() in efi_var_file.c which is responsible
for enumerating all the non-volatile variables to be saved at each
SET_VARIABLE api call?
-Takahiro Akashi
Cheers,
Neil
Thanks,
-Takahiro Akashi
=> efidebug boot order 2 1
=> fatls mmc 0:1
440 ubootefi.var
(Size drops from 3040 to 440 bytes and keys have gone)
From: AKASHI Takahiro
Sent: 29 June 2023 02:01
To: Neil Jones
Cc: u-boot@lists.denx.de
Subject: Re: EFI Secure boot default keys
On Wed, Jun 28, 2023 at 04:26:58PM +, Neil Jones wrote:
Please can someone describe the format of the file needed for the default /
built-in EFI secure boot keys (ubootefi.var)
The only docs I have found suggest its best to enroll the keys from within
u-boot onto some removable media, then copy this off and use this as the
default, this is not very helpful and doesn't work for me:
=> fatload mmc 0:1 ${loadaddr} PK.aut
2053 bytes read in 18 ms (111.3 KiB/s)
=> setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize PK
setenv - set environment variables
Usage:
setenv setenv [-f] name value ...
- [
Re: EFI Secure boot default keys
On Wed, Jun 28, 2023 at 04:26:58PM +, Neil Jones wrote: Please can someone describe the format of the file needed for the default / built-in EFI secure boot keys (ubootefi.var) The only docs I have found suggest its best to enroll the keys from within u-boot onto some removable media, then copy this off and use this as the default, this is not very helpful and doesn't work for me: The file format is described in https://github.com/ARM-software/ebbr/blob/main/source/chapter5-variable-storage.rst U-Boot comes with tools/efivar.py to edit ubootefi.var. Best regards Heinrich
Re: [PATCH 0/4] introduce EFI_RAM_DISK_PROTOCOL
On 7/7/23 06:00, Masahisa Kojima wrote: This series introduces the EFI_RAM_DISK_PROTOCOL implementation. The major purpose of this series is a preparation for EFI HTTP(S) boot. Now U-Boot can download the distro installer ISO image via wget or tftpboot commands, but U-Boot can not mount the downloaded ISO image. By calling EFI_RAM_DISK_PROTOCOL->register API, user can mount the ISO image and boot the distro installer. If I understand you correctly, with your design RAM disks will only exist in the EFI sub-system. We strive to synchronize what is happening on the driver model level and on the UEFI level. I would prefer a design where we have a UCLASS_BLK driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of managing ram disk devices. A big issue we have is RAM management. malloc() can only assign limited amount of memory which is probably too small for the RAM disk you are looking at. We manage page sized memory in the EFI sub-system but this is not integrated with the LMB memory checks. The necessary sequence of development is probably: * Rework memory management * Implement ramdisks as UCLASS_BLK driver * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver. Best regards Heinrich Note that the installation process may not proceed after the distro installer calls ExitBootServices() since there is no hand-off process for the block device of memory mapped ISO image. The EFI_RAM_DISK_PROTOCOL was tested with the debian network installer[1] on qemu_arm64 platform. The example procedure is as follows. => tftpboot 4100 mini.iso => efidebug disk load 4100 $filesize After these commands, ISO image is mounted like: => efidebug dh 7eec5910 (efiblk#0) /RamDisk(0x4100,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0) Block IO 7eec6140 (efiblk#0:1) /RamDisk(0x4100,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800) Block IO 7eec62b0 (efiblk#0:2) /RamDisk(0x4100,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x1) Block IO System Partition Simple File System => dm tree blk 0 [ + ] efi_blk `-- efiblk#0 partition 0 [ + ] blk_partition |-- efiblk#0:1 partition 1 [ + ] blk_partition `-- efiblk#0:2 Debian can be successfully installed with this RAM disk on QEMU. [TODO] - udevices created in ./lib/efi_driver/efi_block_device.c::efi_bl_bind() are not removed when the efi_handle is removed. So after unload the RAM disk, udevices still exist. I plan to add a udevice removal process in ./lib/efi_driver/efi_uclass.c::efi_uc_stop(). In addition, I also plan to add unbind() callback in struct efi_driver_ops. [1] https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/mini.iso Masahisa Kojima (4): efi_loader: add RAM disk device path efi_loader: add EFI_RAM_DISK_PROTOCOL implementation cmd: efidebug: add RAM disk mount command efi_selftest: add EFI_RAM_DISK_PROTOCOL selftest cmd/efidebug.c | 117 ++ include/efi_api.h| 32 ++ include/efi_loader.h | 4 + lib/efi_driver/efi_uclass.c | 7 +- lib/efi_loader/Kconfig | 6 + lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_device_path_to_text.c | 14 + lib/efi_loader/efi_ram_disk.c| 334 +++ lib/efi_loader/efi_setup.c | 6 + lib/efi_selftest/Makefile| 1 + lib/efi_selftest/efi_selftest_ram_disk.c | 511 +++ lib/uuid.c | 4 + 12 files changed, 1035 insertions(+), 2 deletions(-) create mode 100644 lib/efi_loader/efi_ram_disk.c create mode 100644 lib/efi_selftest/efi_selftest_ram_disk.c base-commit: e2e2aea5733f0d23cd9593bbefe5c803c552dcb9
Re: [PATCH] efi_loader: Increase default variable store size to 32K
On 7/8/23 17:21, Alper Nebi Yasak wrote:
Debian's arm64 UEFI Secure Boot shim makes the EFI variable store run
out of space while mirroring its MOK database to variables. This can be
observed in QEMU like so:
$ tools/buildman/buildman -o build/qemu_arm64 --boards=qemu_arm64 -w
$ cd build/qemu_arm64
$ curl -L -o debian.iso \
https://cdimage.debian.org/debian-cd/current/arm64/iso-cd/debian-12.0.0-arm64-netinst.iso
$ qemu-system-aarch64 \
-nographic -bios u-boot.bin \
-machine virt -cpu cortex-a53 -m 1G -smp 2 \
-drive
if=virtio,file=debian.iso,index=0,format=raw,readonly=on,media=cdrom
[...]
=> # interrupt autoboot
=> env set -e -bs -nv -rt -guid 605dab50-e046-4300-abb6-3dd810dd8b23
SHIM_VERBOSE 1
=> boot
[...]
mok.c:296:mirror_one_esl() SetVariable("MokListXRT43", ... varsz=0x4C) = Out
of Resources
mok.c:452:mirror_mok_db() esd:0x7DB92D20 adj:0x30
Failed to set MokListXRT: Out of Resources
mok.c:767:mirror_one_mok_variable() mirror_mok_db("MokListXRT",
datasz=17328) returned Out of Resources
mok.c:812:mirror_one_mok_variable() returning Out of Resources
Could not create MokListXRT: Out of Resources
[...]
Welcome to GRUB!
This would normally be fine as shim would continue to run grubaa64.efi,
but shim's error handling code for this case has a bug [1] that causes a
synchronous abort on at least chromebook_kevin (but apparently not on
QEMU arm64).
Double the default variable store size so the variables fit. There is a
note about this value matching PcdFlashNvStorageVariableSize when
EFI_MM_COMM_TEE is enabled, so keep the old default in that case.
[1] https://github.com/rhboot/shim/pull/577
Signed-off-by: Alper Nebi Yasak
---
I'm not very familiar with EFI things, apologies if this default
should not be changed (consider this a bug report in that case).
lib/efi_loader/Kconfig | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index c5835e6ef61a..0660d1174902 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -96,7 +96,8 @@ endif
config EFI_VAR_BUF_SIZE
int "Memory size of the UEFI variable store"
- default 16384
+ default 16384 if EFI_MM_COMM_TEE
+ default 32768
https://learn.microsoft.com/en-us/previous-versions/windows/hardware/cert-program/windows-hardware-certification-requirements-for-client-and-server-systems
requires 64 KiB.
Best regards
Heinrich
range 4096 2147483647
help
This defines the size in bytes of the memory area reserved for keeping
@@ -106,7 +107,7 @@ config EFI_VAR_BUF_SIZE
match the value of PcdFlashNvStorageVariableSize used to compile the
StandAloneMM module.
- Minimum 4096, default 16384.
+ Minimum 4096, default 32768, or 16384 when using StandAloneMM.
config EFI_GET_TIME
bool "GetTime() runtime service"
Re: [PATCH] cmd: efidebug: add missing efi_free_pool for dh subcommand
On 6/29/23 11:08, Ilias Apalodimas wrote:
On Thu, 29 Jun 2023 at 05:14, Masahisa Kojima
wrote:
This adds the missing efi_free_pool call for dh subcommand.
Fixes: a80146205d0a ("cmd: efidebug: add dh command")
Signed-off-by: Masahisa Kojima
Reviewed-by: Heinrich Schuchardt
---
cmd/efidebug.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 9622430c47..0be3af3e76 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -486,6 +486,7 @@ static int do_efi_show_handles(struct cmd_tbl *cmdtp, int
flag,
if (guidcmp(guid[j], &efi_guid_device_path))
printf(" %pUs\n", guid[j]);
}
+ efi_free_pool(guid);
}
efi_free_pool(handles);
--
2.34.1
Reviewed-by: Ilias Apalodimas
Re: [PATCH] x86: Update docs link in bootparam header
On 7/7/23 17:41, Tom Rini wrote:
On Fri, Jul 07, 2023 at 07:51:42AM +0100, Paul Barker wrote:
After Linux commit ff61f0791ce9, x86 documentation was moved to
arch/x86 and the link in bootparam.h was broken.
Signed-off-by: Paul Barker
---
arch/x86/include/asm/bootparam.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h
index ea816ca74698..1f8ca569b880 100644
--- a/arch/x86/include/asm/bootparam.h
+++ b/arch/x86/include/asm/bootparam.h
@@ -62,7 +62,7 @@ struct setup_indirect {
/**
* struct setup_header - Information needed by Linux to boot
*
- * See https://www.kernel.org/doc/html/latest/x86/boot.html
+ * See https://docs.kernel.org/arch/x86/boot.html
This is also now:
https://www.kernel.org/doc/html/latest/arch/x86/boot.html
And tree-wide we have two examples of docs.kernel.org and the rest are
www.kernel.org/doc/html/latest for the base. We should be consistent
here, so I'm deferring to Heinrich.
In Linux' /Documentation/ they always uses
https://www.kernel.org/doc/html/latest. So let's stick to this.
Best regards
Heinrich
[PATCH 1/1] doc: harmonize Linux kernel documentation links
Linux internally uses https://www.kernel.org/doc/html/latest/ for documentation links. When referring to their documentation we should do the same. Signed-off-by: Heinrich Schuchardt --- doc/develop/docstyle.rst | 2 +- doc/usage/blkmap.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/develop/docstyle.rst b/doc/develop/docstyle.rst index f9ba83a559..50506d6857 100644 --- a/doc/develop/docstyle.rst +++ b/doc/develop/docstyle.rst @@ -20,7 +20,7 @@ We apply the following rules: * For documentation we use reStructured text conforming to the requirements of `Sphinx <https://www.sphinx-doc.org>`_. * For documentation within code we follow the Linux kernel guide - `Writing kernel-doc comments <https://docs.kernel.org/doc-guide/kernel-doc.html>`_. + `Writing kernel-doc comments <https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html>`_. * We try to stick to 80 columns per line in documents. * For tables we prefer simple tables over grid tables. We avoid list tables as they make the reStructured text documents hard to read. diff --git a/doc/usage/blkmap.rst b/doc/usage/blkmap.rst index dbfa8e5aad..7337ea507a 100644 --- a/doc/usage/blkmap.rst +++ b/doc/usage/blkmap.rst @@ -19,7 +19,7 @@ wherever they might be located. The implementation is loosely modeled on Linux's "Device Mapper" subsystem, see `kernel documentation`_ for more information. -.. _kernel documentation: https://docs.kernel.org/admin-guide/device-mapper/index.html +.. _kernel documentation: https://www.kernel.org/doc/html/latest/admin-guide/device-mapper/index.html Example: Netbooting an Ext4 Image -- 2.40.1
Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type
On 6/16/23 10:28, Stefan Herbrechtsmeier wrote:
From: Malte Schmidt
The data type of item_offset_list shall be UINT64 according to the UEFI [1]
specifications.
In include/efi_api.h the correct data type is used. The bug was probably
never noticed because of little endianness.
[1] https://uefi.org/specs/UEFI/2.10/index.html
Signed-off-by: Malte Schmidt
Signed-off-by: Stefan Herbrechtsmeier
---
tools/eficapsule.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/eficapsule.h b/tools/eficapsule.h
index 753fb73313..2099a2e9b8 100644
--- a/tools/eficapsule.h
+++ b/tools/eficapsule.h
@@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header {
uint32_t version;
uint16_t embedded_driver_count;
uint16_t payload_item_count;
- uint32_t item_offset_list[];
+ uint64_t item_offset_list[];
Defining the same structure in two places it bad practice.
https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/11
Reviewed-by: Heinrich Schuchardt
} __packed;
/* image_capsule_support */
Re: [PATCH v10 3/4] Boot var automatic management for removable medias
On 6/19/23 23:23, Raymond Mao wrote:
Changes for complying to EFI spec §3.5.1.1
'Removable Media Boot Behavior'.
Boot variables can be automatically generated during a removable
media is probed. At the same time, unused boot variables will be
detected and removed.
Please note that currently the function 'efi_disk_remove' has no
ability to distinguish below two scenarios
a) Unplugging of a removable media under U-Boot
b) U-Boot exiting and booting an OS
Thus currently the boot variables management is not added into
'efi_disk_remove' to avoid boot options being added/erased
repeatedly under scenario b) during power cycles
See TODO comments under function 'efi_disk_remove' for more details
Signed-off-by: Raymond Mao
---
Changes in v3
- Split the patch into moving and renaming functions and
individual patches for each changed functionality
Changes in v5
- Move function call of efi_bootmgr_update_media_device_boot_option()
from efi_init_variables() to efi_init_obj_list()
Changes in v6
- Revert unrelated changes
Changes in v7
- adapt the return code of function
efi_bootmgr_update_media_device_boot_option()
Changes in v8
- add a note in the commit message for future reference
Changes in v9
- amend the note text in the commit message
- add a TODO comment in 'efi_disk_remove'
Changes in v10
- fix typo and build failures with 'CONFIG_CMD_BOOTEFI_BOOTMGR=n'
lib/efi_loader/efi_disk.c | 18 ++
lib/efi_loader/efi_setup.c | 7 +++
2 files changed, 25 insertions(+)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index d2256713a8..911a4adfb1 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -687,6 +687,13 @@ int efi_disk_probe(void *ctx, struct event *event)
return -1;
}
+ /* only do the boot option management when UEFI sub-system is
initialized */
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) && efi_obj_list_initialized
== EFI_SUCCESS) {
+ ret = efi_bootmgr_update_media_device_boot_option();
+ if (ret != EFI_SUCCESS)
+ return -1;
+ }
+
return 0;
}
@@ -773,6 +780,17 @@ int efi_disk_remove(void *ctx, struct event *event)
return efi_disk_delete_part(dev);
else
return 0;
+
+ /*
+* TODO A flag to distinguish below 2 different scenarios of this
+* function call is needed:
+* a) Unplugging of a removable media under U-Boot
+* b) U-Boot exiting and booting an OS
+* In case of scenario a), efi_bootmgr_update_media_device_boot_option()
+* needs to be invoked here to update the boot options and remove the
+* unnecessary ones.
+*/
As in future we want to integrate more EFI drivers with the driver model
such a status should be in a global variable. It will have to be set in
ExitBootServices() before invoking dm_remove_devices_flags().
Reviewed-by: Heinrich Schuchardt
+
}
/**
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 58d4e13402..69c8b27730 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -245,6 +245,13 @@ efi_status_t efi_init_obj_list(void)
if (ret != EFI_SUCCESS)
goto out;
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
+ /* update boot option after variable service initialized */
+ ret = efi_bootmgr_update_media_device_boot_option();
+ if (ret != EFI_SUCCESS)
+ goto out;
+ }
+
/* Define supported languages */
ret = efi_init_platform_lang();
if (ret != EFI_SUCCESS)
Re: [PATCH 1/4 v2] efi_loader: reconnect drivers on failure
On 6/20/23 08:19, Ilias Apalodimas wrote: efi_disconnect_controller() doesn't reconnect drivers in case of failure. Reconnect the disconnected drivers properly Signed-off-by: Ilias Apalodimas Reviewed-by: Heinrich Schuchardt
Re: [PATCH 2/4 v2] efi_loader: check the status of disconnected drivers
On 6/20/23 08:19, Ilias Apalodimas wrote: efi_uninstall_protocol() calls efi_disconnect_all_drivers() but never checks the return value. Instead it tries to identify protocols that are still open after closing the ones that were opened with EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL, EFI_OPEN_PROTOCOL_GET_PROTOCOL and EFI_OPEN_PROTOCOL_TEST_PROTOCOL. Instead of doing that, check the return value early and exit if disconnecting the drivers failed. Also reconnect all the drivers of a handle if protocols are still found on the handle after disconnecting controllers and closing the remaining protocols. While at it fix a memory leak and properly free the opened protocol information when closing a protocol. Signed-off-by: Ilias Apalodimas Reviewed-by: Heinrich Schuchardt
Re: [PATCH] efi_loader: make efi_delete_handle() follow the EFI spec
On 6/26/23 12:04, Ilias Apalodimas wrote:
The EFI doesn't allow removal of handles, unless all hosted protocols
are cleanly removed. Our efi_delete_handle() is a bit intrusive.
Although it does try to delete protocols before removing a handle,
it doesn't care if that fails. Instead it only returns an error if the
handle is invalid. On top of that none of the callers of that function
check the return code.
So let's rewrite this in a way that fits the EFI spec better. Instead
of forcing the handle removal, gracefully uninstall all the handle
protocols. According to the EFI spec when the last protocol is removed
the handle will be deleted. Also switch all the callers and check the
return code. Some callers can't do anything useful apart from reporting
an error. The disk related functions on the other hand, can prevent a
medium that is being used by EFI from removal.
The only function that doesn't check the result is efi_delete_image().
But that function needs a bigger rework anyway, so we can clean it up in
the future
Signed-off-by: Ilias Apalodimas
---
Heinrich this needs to be applied on top of
https://lore.kernel.org/u-boot/20230620061932.113292-1-ilias.apalodi...@linaro.org/
cmd/bootefi.c | 19 ---
include/efi_loader.h | 2 +-
lib/efi_driver/efi_uclass.c | 13 ++---
lib/efi_loader/efi_boottime.c | 26 ++
lib/efi_loader/efi_disk.c | 17 +
5 files changed, 42 insertions(+), 35 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 8aa15a64c836..60b9e950ffdc 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -606,20 +606,6 @@ failure:
return ret;
}
-/**
- * bootefi_run_finish() - finish up after running an EFI test
- *
- * @loaded_image_info: Pointer to a struct which holds the loaded image info
- * @image_obj: Pointer to a struct which holds the loaded image object
- */
-static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj,
- struct efi_loaded_image *loaded_image_info)
-{
- efi_restore_gd();
- free(loaded_image_info->load_options);
- efi_delete_handle(&image_obj->header);
-}
-
/**
* do_efi_selftest() - execute EFI selftest
*
@@ -638,7 +624,10 @@ static int do_efi_selftest(void)
/* Execute the test */
ret = EFI_CALL(efi_selftest(&image_obj->header, &systab));
- bootefi_run_finish(image_obj, loaded_image_info);
+ efi_restore_gd();
+ free(loaded_image_info->load_options);
+ if (efi_delete_handle(&image_obj->header) != EFI_SUCCESS)
+ log_err("Failed to delete loaded image handle\n");
return ret != EFI_SUCCESS;
}
diff --git a/include/efi_loader.h b/include/efi_loader.h
index e530bcf15f76..43ccf49abec4 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -618,7 +618,7 @@ void efi_add_handle(efi_handle_t obj);
/* Create handle */
efi_status_t efi_create_handle(efi_handle_t *handle);
/* Delete handle */
-void efi_delete_handle(efi_handle_t obj);
+efi_status_t efi_delete_handle(efi_handle_t obj);
/* Call this to validate a handle and find the EFI object for it */
struct efi_object *efi_search_obj(const efi_handle_t handle);
/* Locate device_path handle */
diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
index 45f935198874..d8755541fc14 100644
--- a/lib/efi_driver/efi_uclass.c
+++ b/lib/efi_driver/efi_uclass.c
@@ -285,10 +285,8 @@ static efi_status_t efi_add_driver(struct driver *drv)
bp->ops = ops;
ret = efi_create_handle(&bp->bp.driver_binding_handle);
- if (ret != EFI_SUCCESS) {
- free(bp);
- goto out;
- }
+ if (ret != EFI_SUCCESS)
+ goto err;
bp->bp.image_handle = bp->bp.driver_binding_handle;
ret = efi_add_protocol(bp->bp.driver_binding_handle,
&efi_guid_driver_binding_protocol, bp);
@@ -299,11 +297,12 @@ static efi_status_t efi_add_driver(struct driver *drv)
if (ret != EFI_SUCCESS)
goto err;
}
-out:
- return ret;
+ return ret;
err:
- efi_delete_handle(bp->bp.driver_binding_handle);
+ if (bp->bp.driver_binding_handle &&
+ efi_delete_handle(bp->bp.driver_binding_handle) != EFI_SUCCESS)
+ log_err("Failed to delete handle\n");
You already log errors in efi_delete_handle(), please, avoid duplicate
log messages increasing code size (applies to several locations in this
patch).
Best regards
Heinrich
free(bp);
return ret;
}
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index d75a3336e3f1..5123055d7f5d 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -59,6 +59,10 @@ static efi_handle_t current_image;
static volatile gd_t *efi_gd, *app_gd;
#endif
+static efi_status_t efi_uninstall_protocol
+
Re: [PATCH v2] x86: Update docs link in bootparam header
On 7/9/23 11:44, Paul Barker wrote:
After Linux commit ff61f0791ce9, x86 documentation was moved to
arch/x86 and the link in bootparam.h was broken.
Signed-off-by: Paul Barker
Reviewed-by: Heinrich Schuchardt
---
arch/x86/include/asm/bootparam.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/bootparam.h b/arch/x86/include/asm/bootparam.h
index ea816ca74698..ac4865300f1b 100644
--- a/arch/x86/include/asm/bootparam.h
+++ b/arch/x86/include/asm/bootparam.h
@@ -62,7 +62,7 @@ struct setup_indirect {
/**
* struct setup_header - Information needed by Linux to boot
*
- * See https://www.kernel.org/doc/html/latest/x86/boot.html
+ * See https://www.kernel.org/doc/html/latest/arch/x86/boot.html
*/
struct setup_header {
__u8setup_sects;
base-commit: 0beb649053b86b2cfd5cf55a0fc68bc2fe91a430
Pull request efi-2023-07-rc7
Dear Tom, The following changes since commit 0beb649053b86b2cfd5cf55a0fc68bc2fe91a430: MAINTAINERS: correct at91 tree link (2023-07-07 11:37:09 -0400) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2023-07-rc7 for you to fetch changes up to e05a4b12a056fd7fe22e85715c8f5e5e811fb551: mkeficapsule: fix efi_firmware_management_capsule_header data type (2023-07-09 10:32:28 +0200) Gitlab CI showed no issues: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/16815 Pull request efi-2023-07-rc7 Documentation: * Fix links to Linux kernel documentation UEFI: * Fix memory leak in efidebug dh subcommand * Fix underflow when calculating remaining variable store size * Increase default variable store size to 64 KiB * mkeficapsule: fix efi_firmware_management_capsule_header data type Alper Nebi Yasak (2): efi_loader: Avoid underflow when calculating remaining var store size efi_loader: Increase default variable store size to 64KiB Heinrich Schuchardt (1): doc: harmonize Linux kernel documentation links Malte Schmidt (1): mkeficapsule: fix efi_firmware_management_capsule_header data type Masahisa Kojima (1): cmd: efidebug: add missing efi_free_pool for dh subcommand Paul Barker (1): x86: Update docs link in bootparam header arch/x86/include/asm/bootparam.h | 2 +- cmd/efidebug.c | 1 + doc/develop/docstyle.rst | 2 +- doc/usage/blkmap.rst | 2 +- lib/efi_loader/Kconfig | 5 +++-- lib/efi_loader/efi_var_mem.c | 4 tools/eficapsule.h | 2 +- 7 files changed, 12 insertions(+), 6 deletions(-)
Re: [PATCH 1/2] cmd: thordown: Add proper dependency for CMD_THOR_DOWNLOAD
Am 9. Juli 2023 15:09:57 MESZ schrieb Ashok Reddy Soma : >When CONFIG_CMD_USB and CONFIG_USB are disabled some compilation errors >are seen as below. Thanks for your patch. Currently we have no documentation for the thordown command. We should create a man page in /docs/usage/cmd/. Do you have any description of the usage of the command? Best regards Heinrich > >cmd/thordown.o: in function `usb_gadget_initialize': >include/linux/usb/gadget.h:981: undefined reference to `board_usb_init' >cmd/thordown.o: in function `do_thor_down': >cmd/thordown.c:68: undefined reference to `g_dnl_unregister' >cmd/thordown.o: in function `usb_gadget_release': >include/linux/usb/gadget.h:986: undefined reference to `board_usb_cleanup' >cmd/thordown.o: in function `do_thor_down': >cmd/thordown.c:41: undefined reference to `g_dnl_register' >cmd/thordown.c:48: undefined reference to `thor_init' >cmd/thordown.c:56: undefined reference to `thor_handle' >gnu/aarch64/lin/aarch64-linux/bin/aarch64-linux-gnu-ld.bfd: line 4: 8485 >Segmentation fault (core dumped) $CC --sysroot=$LIBC >--no-warn-rwx-segment "$@" >Makefile:1779: recipe for target 'u-boot' failed >make: *** [u-boot] Error 139 >make: *** Deleting file 'u-boot' > >Add dependency of CMD_USB for CONFIG_CMD_THOR_DOWNLOAD to fix the errors. > >Signed-off-by: Ashok Reddy Soma >--- > > cmd/Kconfig | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/cmd/Kconfig b/cmd/Kconfig >index 02e54f1e50..b44df9d67a 100644 >--- a/cmd/Kconfig >+++ b/cmd/Kconfig >@@ -526,6 +526,7 @@ config CMD_SPL_WRITE_SIZE > > config CMD_THOR_DOWNLOAD > bool "thor - TIZEN 'thor' download" >+ depends on CMD_USB > select DFU > help > Implements the 'thor' download protocol. This is a way of
Re: [PATCH v3 02/11] capsule: authenticate: Add capsule public key in platform's dtb
Am 9. Juli 2023 15:33:17 MESZ schrieb Sughosh Ganu :
>The EFI capsule authentication logic in u-boot expects the public key
>in the form of an EFI Signature List(ESL) to be provided as part of
>the platform's dtb. Currently, the embedding of the ESL file into the
>dtb needs to be done manually.
>
>Add a signature node in the u-boot dtsi file and include the public
>key through the capsule-key property. This file is per architecture,
>and is currently being added for sandbox and arm architectures. It
The device-tree compiler can pick up files from /include/. If the dtsi file is
not architecture specific, we should avoid code duplication.
We should treat all EFI architectures the same.
Best regards
Heinrich
>will have to be added for other architectures which need to enable
>capsule authentication support.
>
>The path to the ESL file is specified through the
>CONFIG_EFI_CAPSULE_ESL_FILE symbol.
>
>Signed-off-by: Sughosh Ganu
>---
>Changes since V2:
>* Add the public key ESL file through the u-boot.dtsi.
>* Add the dtsi files for sandbox and arm architectures.
>* Add a check in the Makefile that the ESL file path is not empty.
>
> arch/arm/dts/u-boot.dtsi | 17 +
> arch/sandbox/dts/u-boot.dtsi | 17 +
> lib/efi_loader/Kconfig | 11 +++
> lib/efi_loader/Makefile | 7 +++
> 4 files changed, 52 insertions(+)
> create mode 100644 arch/arm/dts/u-boot.dtsi
> create mode 100644 arch/sandbox/dts/u-boot.dtsi
>
>diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
>new file mode 100644
>index 00..60bd004937
>--- /dev/null
>+++ b/arch/arm/dts/u-boot.dtsi
>@@ -0,0 +1,17 @@
>+// SPDX-License-Identifier: GPL-2.0+
>+/*
>+ * Devicetree file with miscellaneous nodes that will be included
>+ * at build time into the DTB. Currently being used for including
>+ * capsule related information.
>+ *
>+ */
>+
>+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
>+/ {
>+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
>+ signature {
>+ capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
>+ };
>+#endif
>+};
>+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
>diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
>new file mode 100644
>index 00..60bd004937
>--- /dev/null
>+++ b/arch/sandbox/dts/u-boot.dtsi
>@@ -0,0 +1,17 @@
>+// SPDX-License-Identifier: GPL-2.0+
>+/*
>+ * Devicetree file with miscellaneous nodes that will be included
>+ * at build time into the DTB. Currently being used for including
>+ * capsule related information.
>+ *
>+ */
>+
>+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
>+/ {
>+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
>+ signature {
>+ capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
>+ };
>+#endif
>+};
>+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
>diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>index c5835e6ef6..1326a1d109 100644
>--- a/lib/efi_loader/Kconfig
>+++ b/lib/efi_loader/Kconfig
>@@ -234,6 +234,17 @@ config EFI_CAPSULE_MAX
> Select the max capsule index value used for capsule report
> variables. This value is used to create CapsuleMax variable.
>
>+config EFI_CAPSULE_ESL_FILE
>+ string "Path to the EFI Signature List File"
>+ default ""
>+ depends on EFI_CAPSULE_AUTHENTICATE
>+ help
>+Provides the absolute path to the EFI Signature List
>+file which will be embedded in the platform's device
>+tree and used for capsule authentication at the time
>+of capsule update.
>+
>+
> config EFI_DEVICE_PATH_TO_TEXT
> bool "Device path to text protocol"
> default y
>diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>index 13a35eae6c..9fb04720d9 100644
>--- a/lib/efi_loader/Makefile
>+++ b/lib/efi_loader/Makefile
>@@ -86,3 +86,10 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
>
> EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
> $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
>+
>+ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
>+EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_ESL_FILE))
>+ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
>+$(error .esl cerificate not found. Configure your CONFIG_EFI_CAPSULE_ESL_FILE)
>+endif
>+endif
Re: [PATCH v1] mkeficapsule: fix efi_firmware_management_capsule_header data type
Am 10. Juli 2023 08:26:37 MESZ schrieb AKASHI Takahiro
:
>On Sun, Jul 09, 2023 at 10:31:55AM +0200, Heinrich Schuchardt wrote:
>> On 6/16/23 10:28, Stefan Herbrechtsmeier wrote:
>> > From: Malte Schmidt
>> >
>> > The data type of item_offset_list shall be UINT64 according to the UEFI [1]
>> > specifications.
>> >
>> > In include/efi_api.h the correct data type is used. The bug was probably
>> > never noticed because of little endianness.
>> >
>> > [1] https://uefi.org/specs/UEFI/2.10/index.html
>> >
>> > Signed-off-by: Malte Schmidt
>> >
>> > Signed-off-by: Stefan Herbrechtsmeier
>> >
>> > ---
>> >
>> > tools/eficapsule.h | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/tools/eficapsule.h b/tools/eficapsule.h
>> > index 753fb73313..2099a2e9b8 100644
>> > --- a/tools/eficapsule.h
>> > +++ b/tools/eficapsule.h
>> > @@ -63,7 +63,7 @@ struct efi_firmware_management_capsule_header {
>> >uint32_t version;
>> >uint16_t embedded_driver_count;
>> >uint16_t payload_item_count;
>> > - uint32_t item_offset_list[];
>> > + uint64_t item_offset_list[];
>>
>> Defining the same structure in two places it bad practice.
>> https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/11
>
>I had a good reason for adding a tool-specific header, instead of
>using headers under 'include' dir, when I posted v7 of "efi_loader:
>capsule: improve capsule authentication support" patch.
>The cover letter says,
>===
>v7 (Nov 16, 2021)
> ...
>* define eficapsule.h and include it from mkeficapsule (patch#3)
> Hopefully, the tool can now compile on non-linux host.
>===
>
>If I correctly remember, this reflects the comment below and the
>succeeding discussions:
>https://lists.denx.de/pipermail/u-boot/2021-November/465859.html
>
>-Takahiro Akashi
We should be able to factor out a common header file which contains capsule
specific structures.
I understand that we have to test building on BSD.
Best regards
Heinrich
>
>
>> Reviewed-by: Heinrich Schuchardt
>>
>> > } __packed;
>> >
>> > /* image_capsule_support */
>>
Re: [PATCH 0/4] introduce EFI_RAM_DISK_PROTOCOL
On 7/11/23 21:13, Simon Glass wrote: Hi, On Tue, 11 Jul 2023 at 00:23, Masahisa Kojima wrote: On Mon, 10 Jul 2023 at 11:28, AKASHI Takahiro wrote: On Mon, Jul 10, 2023 at 11:13:12AM +0900, Masahisa Kojima wrote: On Fri, 7 Jul 2023 at 18:12, AKASHI Takahiro wrote: On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote: Hi Akashi-san, On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro wrote: On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote: On 7/7/23 06:00, Masahisa Kojima wrote: This series introduces the EFI_RAM_DISK_PROTOCOL implementation. The major purpose of this series is a preparation for EFI HTTP(S) boot. Now U-Boot can download the distro installer ISO image via wget or tftpboot commands, but U-Boot can not mount the downloaded ISO image. By calling EFI_RAM_DISK_PROTOCOL->register API, user can mount the ISO image and boot the distro installer. If I understand you correctly, with your design RAM disks will only exist in the EFI sub-system. Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot block device (and associated partition devices) thanks to your efi_driver framework. Read/Write the RAM disk is expected to be called from the EFI context, so An associated U-Boot block device should work as well on top of your RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk). That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's proper filesystem subsystem, is installed to the RAM disk object. I now realize that my RAM_DISK_PROTOCOL implementation happens to work thanks to the block cache. When I disable CONFIG_BLOCK_CACHE and load some EFI application(it does set 'app_gd') before calling RAM_DISK_PROTOCOL service, RAM_DISK_PROTOCOL does not work. ConnectController fails. native U-Boot can not access the RAM disk. I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an interface?) should work even under your current implementation. "fatls efiloader 0:2" does not work. I think "efiloader" device is designed to be accessed from EFI application only even if it is listed by "dm tree". I don't believe so. (the interface name may be "efi_blk" or "efiblk", though.) If the command fails, it's a bug. Must be fixed. "fatls efiloader 0:2" failed here: https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/block/blk-uclass.c#L139 This is because the parent is 'dm_root' for the device created by lib/efi_driver/efi_block_device.c, so uclass_id is different. The parent device of a UCLASS_BLK device MUST be a media uclass. It cannot be root. I believe EFI has some suitable classes. At some point we will remove the uclass_id from struct blk_desc and just use the parent's uclass ID. Hello Simon, we want to more tightly integrate EFI and DM. If an EFI program like iPXE creates a handle for an iSCSI drive with the block IO protocol the parent of that handle will be a network device and nothing having to do with a block device. Introducing dummy devices like you did for the sandbox host block devices will only make integration of EFI and DM more complicated. As I indicated years before you have to use blk_desc->uclass_id instead of the parent uclass_id to identify block devices in blk_get_devnum_by_uclass_idname() [1][2]. Removing class_id from struct blk_desc is a step against better future integration of EFI and DM. Best regards Heinrich [1] https://lore.kernel.org/all/20221003093550.106304-1-heinrich.schucha...@canonical.com/ [2] https://lore.kernel.org/all/20220802094933.69170-1-heinrich.schucha...@canonical.com/
Re: [PATCH v4 0/4] SPL NVMe support
On 12.07.23 15:06, mchit...@ventanamicro.com wrote:
Hi Tom,
On Tue, 2023-06-20 at 09:37 -0400, Tom Rini wrote:
On Sat, 03 Jun 2023 19:32:52 +0530, Mayuresh Chitale wrote:
This patchset adds support to load images of the SPL's next booting
stage from a NVMe device.
Changes in v4:
- Drop patch 4
- Modify patch 2 to use generic fs.h APIs
[...]
With one change, which is that the "disk/part.c" in 4/4 were not
required for
any platform in tree and also broke testcases, and so was dropped,
this has now
been applied to u-boot/next. If you can explain a bit more what the
problem you
had was, we can look in to it. I suspect you need to test for not
SPL_ENV_SUPPORT but ENV_SUPPORT itself.
Thanks.
When SPL_NVME is enabled the build breaks with the following error:
riscv64-unknown-linux-gnu-ld.bfd: disk/part.o: in function
`blk_get_device_part_str':
u-boot/disk/part.c:473: undefined reference to `env_get'
make[2]: *** [u-boot/scripts/Makefile.spl:527: spl/u-boot-spl] Error 1
make[1]: *** [/u-boot/Makefile:2053: spl/u-boot-spl] Error 2
One possible fix is:
if ((!IS_ENABLED(CONFIG_SPL) && IS_ENABLED(CONFIG_ENV_SUPPORT)) ||
(IS_ENABLED(CONFIG_SPL) && IS_ENABLED(CONFIG_SPL_ENV_SUPPORT)))
if (!dev_part_str || !strlen(dev_part_str)
|| !strcmp(dev_part_str, "-"))
dev_part_str = env_get("bootdevice");
I think CONFIG_SPL_ENV_SUPPORT should depend on CONFIG_ENV_SUPPORT in
common/spl/Kconfig.
Best regards
Heinrich
[PATCH 1/1] doc: riscv: running SiFive Unleashed image in QEMU
Describe how to run the SiFive Unleashed image in QEMU. Signed-off-by: Heinrich Schuchardt --- doc/board/sifive/unleashed.rst | 11 +++ 1 file changed, 11 insertions(+) diff --git a/doc/board/sifive/unleashed.rst b/doc/board/sifive/unleashed.rst index ce38b701d7..3daf7ffde2 100644 --- a/doc/board/sifive/unleashed.rst +++ b/doc/board/sifive/unleashed.rst @@ -574,4 +574,15 @@ Change DIP switches MSEL[3:0] are set to 0110 Power up the board. +Running in QEMU +--- + +QEMU provides the sifive_u (RISC-V Board compatible with SiFive U SDK) machine +that can be used for testing U-Boot: + +.. code-block:: bash + +qemu-system-riscv64 -M sifive_u -m 8G -nographic \ +-kernel u-boot.bin + [1] https://github.com/amarula/bsp-sifive -- 2.40.1
Re: [PATCH v2 1/6] efi_loader: add RAM disk device path
On 14.07.23 07:44, Masahisa Kojima wrote:
This is a preparation to add the EFI_RAM_DISK_PROTOCOL.
This commit adds the RAM disk device path structure
and text conversion to Device Path to Text Protocol.
Signed-off-by: Masahisa Kojima
---
No update since v1
include/efi_api.h| 19 +++
lib/efi_loader/efi_device_path_to_text.c | 14 ++
2 files changed, 33 insertions(+)
diff --git a/include/efi_api.h b/include/efi_api.h
index 55a4c989fc..4ee4a1b5e9 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -682,6 +682,7 @@ struct efi_device_path_uri {
# define DEVICE_PATH_SUB_TYPE_CDROM_PATH 0x02
# define DEVICE_PATH_SUB_TYPE_VENDOR_PATH0x03
# define DEVICE_PATH_SUB_TYPE_FILE_PATH 0x04
+# define DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH 0x09
struct efi_device_path_hard_drive_path {
struct efi_device_path dp;
@@ -705,6 +706,24 @@ struct efi_device_path_file_path {
u16 str[];
} __packed;
+/* This GUID defines a RAM Disk supporting a raw disk format in volatile
memory */
+#define EFI_VIRTUAL_DISK_GUID \
+ EFI_GUID(0x77ab535a, 0x45fc, 0x624b, \
+ 0x55, 0x60, 0xf7, 0xb2, 0x81, 0xd1, 0xf9, 0x6e)
+
+/* This GUID defines a RAM Disk supporting an ISO image in volatile memory */
+#define EFI_VIRTUAL_CD_GUID \
+ EFI_GUID(0x3d5abd30, 0x4175, 0x87ce, \
+0x6d, 0x64, 0xd2, 0xad, 0xe5, 0x23, 0xc4, 0xbb)
+
+struct efi_device_path_ram_disk_path {
+ struct efi_device_path dp;
+ u64 starting_address;
+ u64 ending_address;
+ efi_guid_t disk_type_guid;
+ u16 disk_instance;
+} __packed;
+
#define EFI_BLOCK_IO_PROTOCOL_GUID \
EFI_GUID(0x964e5b21, 0x6459, 0x11d2, \
0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
diff --git a/lib/efi_loader/efi_device_path_to_text.c
b/lib/efi_loader/efi_device_path_to_text.c
index 8c76d8be60..4395e79f33 100644
--- a/lib/efi_loader/efi_device_path_to_text.c
+++ b/lib/efi_loader/efi_device_path_to_text.c
@@ -324,6 +324,20 @@ static char *dp_media(char *s, struct efi_device_path *dp)
free(buffer);
break;
}
+ case DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH: {
+ struct efi_device_path_ram_disk_path *rddp =
+ (struct efi_device_path_ram_disk_path *)dp;
+ u64 start;
+ u64 end;
+
+ /* Copy from packed structure to aligned memory */
+ memcpy(&start, &rddp->starting_address, sizeof(start));
+ memcpy(&end, &rddp->ending_address, sizeof(end));
+
+ s += sprintf(s, "RamDisk(0x%llx,%llx,%pUl,0x%x)", start, end,
+&rddp->disk_type_guid, rddp->disk_instance);
If there is no alignment guarantee for starting_address, then the same
is true for disk_instance which may spread over two u64 blocks.
In case of DEVICE_PATH_SUB_TYPE_MEMORY we don't use memcpy() to align u64.
I don't think we call device_path_to_text before allow_unaligned().
There is a family of functions like get_unaligned_le64() if we should
ever need to a align a value. Or we could copy the whole device path node.
Best regards
Heinrich
+ break;
+ }
default:
s = dp_unknown(s, dp);
break;
Re: [PATCH] bootstd: Make efi_mgr bootmeth work for non-sandbox setups
Am 14. Juli 2023 21:56:02 MESZ schrieb Mark Kettenis :
>Enable the bootflow based on this bootmeth if the BootOrder EFI
>variable is set.
>
>Signed-off-by: Mark Kettenis
>---
> boot/bootmeth_efi_mgr.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
>diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
>index e9d973429f..db650861ff 100644
>--- a/boot/bootmeth_efi_mgr.c
>+++ b/boot/bootmeth_efi_mgr.c
>@@ -14,6 +14,8 @@
> #include
> #include
> #include
>+#include
>+#include
>
> /**
> * struct efi_mgr_priv - private info for the efi-mgr driver
>@@ -46,13 +48,21 @@ static int efi_mgr_check(struct udevice *dev, struct
>bootflow_iter *iter)
> static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
> {
> struct efi_mgr_priv *priv = dev_get_priv(dev);
>+ efi_uintn_t size;
>+ u16 *bootorder;
>
> if (priv->fake_dev) {
> bflow->state = BOOTFLOWST_READY;
> return 0;
> }
>
>- /* To be implemented */
>+ /* Enable this method if the "BootOrder" UEFI exists. */
>+ bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
>+ &size);
Are EFI variables already loaded when you hit this code?
Even if the variable Boot Order is not set we must boot EFI/BOOT/BOOT.EFI.
Best regards
Heinrich
>+ if (bootorder) {
>+ bflow->state = BOOTFLOWST_READY;
>+ return 0;
>+ }
>
> return -EINVAL;
> }
Re: [PATCH] bootstd: Make efi_mgr bootmeth work for non-sandbox setups
On 7/14/23 21:56, Mark Kettenis wrote:
Enable the bootflow based on this bootmeth if the BootOrder EFI
variable is set.
Signed-off-by: Mark Kettenis
---
boot/bootmeth_efi_mgr.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
index e9d973429f..db650861ff 100644
--- a/boot/bootmeth_efi_mgr.c
+++ b/boot/bootmeth_efi_mgr.c
@@ -14,6 +14,8 @@
#include
#include
#include
+#include
+#include
/**
* struct efi_mgr_priv - private info for the efi-mgr driver
@@ -46,13 +48,21 @@ static int efi_mgr_check(struct udevice *dev, struct
bootflow_iter *iter)
static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
{
struct efi_mgr_priv *priv = dev_get_priv(dev);
+ efi_uintn_t size;
+ u16 *bootorder;
if (priv->fake_dev) {
bflow->state = BOOTFLOWST_READY;
return 0;
}
- /* To be implemented */
+ /* Enable this method if the "BootOrder" UEFI exists. */
+ bootorder = efi_get_var(u"BootOrder", &efi_global_variable_guid,
+ &size);
+ if (bootorder) {
+ bflow->state = BOOTFLOWST_READY;
+ return 0;
+ }
return -EINVAL;
}
There is no device-tree with compatible "u-boot,efi-bootmgr". So this
looks like dead code up to now. Please, provide a unit test.
@Simon
Where is the documentation that describes the "u-boot,distro-efi" and
"u-boot,efi-bootmgr" boot methods? Current
doc/device-tree-bindings/bootmeth.txt is not sufficient and not included
in the generated online documentation.
doc/device-tree-bindings/bootmeth.txt should be converted to yaml format
so that it can be used for validation of device-trees.
Best regards
Heinrich
[PATCH] common: define time_t as 64bit
To avoid the year 2038 problem time_t must be 64bit on all architectures. Signed-off-by: Heinrich Schuchardt --- include/linux/types.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/types.h b/include/linux/types.h index baa2c491ea..9df930afd1 100644 --- a/include/linux/types.h +++ b/include/linux/types.h @@ -65,7 +65,7 @@ typedef __kernel_ptrdiff_tptrdiff_t; #ifndef _TIME_T #define _TIME_T -typedef __kernel_time_ttime_t; +typedef long long time_t; #endif #ifndef _CLOCK_T -- 2.40.1
[PATCH 1/1] test: avoid function name 'setup'
pytest 7.3.2 treats the function name 'setup' as a fixture [1].
This leads to errors like:
TypeError: setup() missing 2 required positional arguments:
'disk_img' and 'osindications'
Rename setup() to capsule_setup().
[1] How to run tests written for nose
https://docs.pytest.org/en/7.3.x/how-to/nose.html
Fixes: 482ef90aeb4c ("test: efi_capsule: refactor efi_capsule test")
Signed-off-by: Heinrich Schuchardt
---
test/py/tests/test_efi_capsule/capsule_common.py | 2 +-
.../test_efi_capsule/test_capsule_firmware_fit.py| 10 +-
.../test_efi_capsule/test_capsule_firmware_raw.py| 12 ++--
.../test_capsule_firmware_signed_fit.py | 12 ++--
.../test_capsule_firmware_signed_raw.py | 12 ++--
5 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/test/py/tests/test_efi_capsule/capsule_common.py
b/test/py/tests/test_efi_capsule/capsule_common.py
index 9eef6767a6..fc0d851c61 100644
--- a/test/py/tests/test_efi_capsule/capsule_common.py
+++ b/test/py/tests/test_efi_capsule/capsule_common.py
@@ -6,7 +6,7 @@
from capsule_defs import CAPSULE_DATA_DIR, CAPSULE_INSTALL_DIR
-def setup(u_boot_console, disk_img, osindications):
+def capsule_setup(u_boot_console, disk_img, osindications):
"""setup the test
Args:
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
b/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
index a3094c33f4..11bcdc2bb2 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_fit.py
@@ -8,7 +8,7 @@ This test verifies capsule-on-disk firmware update for FIT
images
import pytest
from capsule_common import (
-setup,
+capsule_setup,
init_content,
place_capsule_file,
exec_manual_update,
@@ -49,7 +49,7 @@ class TestEfiCapsuleFirmwareFit():
disk_img = efi_capsule_data
capsule_files = ['Test05']
with u_boot_console.log.section('Test Case 1-a, before reboot'):
-setup(u_boot_console, disk_img, '0x0004')
+capsule_setup(u_boot_console, disk_img, '0x0004')
init_content(u_boot_console, '10', 'u-boot.bin.old', 'Old')
init_content(u_boot_console, '15', 'u-boot.env.old', 'Old')
place_capsule_file(u_boot_console, capsule_files)
@@ -81,7 +81,7 @@ class TestEfiCapsuleFirmwareFit():
disk_img = efi_capsule_data
capsule_files = ['Test04']
with u_boot_console.log.section('Test Case 2-a, before reboot'):
-setup(u_boot_console, disk_img, '0x0004')
+capsule_setup(u_boot_console, disk_img, '0x0004')
init_content(u_boot_console, '10', 'u-boot.bin.old', 'Old')
init_content(u_boot_console, '15', 'u-boot.env.old', 'Old')
place_capsule_file(u_boot_console, capsule_files)
@@ -116,7 +116,7 @@ class TestEfiCapsuleFirmwareFit():
disk_img = efi_capsule_data
capsule_files = ['Test104']
with u_boot_console.log.section('Test Case 3-a, before reboot'):
-setup(u_boot_console, disk_img, '0x0004')
+capsule_setup(u_boot_console, disk_img, '0x0004')
init_content(u_boot_console, '10', 'u-boot.bin.old', 'Old')
init_content(u_boot_console, '15', 'u-boot.env.old', 'Old')
place_capsule_file(u_boot_console, capsule_files)
@@ -165,7 +165,7 @@ class TestEfiCapsuleFirmwareFit():
disk_img = efi_capsule_data
capsule_files = ['Test105']
with u_boot_console.log.section('Test Case 4-a, before reboot'):
-setup(u_boot_console, disk_img, '0x0004')
+capsule_setup(u_boot_console, disk_img, '0x0004')
init_content(u_boot_console, '10', 'u-boot.bin.old', 'Old')
place_capsule_file(u_boot_console, capsule_files)
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
index 80d791e3de..a5b5c8a385 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware_raw.py
@@ -8,7 +8,7 @@ This test verifies capsule-on-disk firmware update for raw
images
import pytest
from capsule_common import (
-setup,
+capsule_setup,
init_content,
place_capsule_file,
exec_manu
Re: [PATCH v10 3/4] Boot var automatic management for removable medias
On 7/9/23 10:56, Heinrich Schuchardt wrote:
On 6/19/23 23:23, Raymond Mao wrote:
Changes for complying to EFI spec §3.5.1.1
'Removable Media Boot Behavior'.
Boot variables can be automatically generated during a removable
media is probed. At the same time, unused boot variables will be
detected and removed.
Please note that currently the function 'efi_disk_remove' has no
ability to distinguish below two scenarios
a) Unplugging of a removable media under U-Boot
b) U-Boot exiting and booting an OS
Thus currently the boot variables management is not added into
'efi_disk_remove' to avoid boot options being added/erased
repeatedly under scenario b) during power cycles
See TODO comments under function 'efi_disk_remove' for more details
Signed-off-by: Raymond Mao
---
Changes in v3
- Split the patch into moving and renaming functions and
individual patches for each changed functionality
Changes in v5
- Move function call of efi_bootmgr_update_media_device_boot_option()
from efi_init_variables() to efi_init_obj_list()
Changes in v6
- Revert unrelated changes
Changes in v7
- adapt the return code of function
efi_bootmgr_update_media_device_boot_option()
Changes in v8
- add a note in the commit message for future reference
Changes in v9
- amend the note text in the commit message
- add a TODO comment in 'efi_disk_remove'
Changes in v10
- fix typo and build failures with 'CONFIG_CMD_BOOTEFI_BOOTMGR=n'
lib/efi_loader/efi_disk.c | 18 ++
lib/efi_loader/efi_setup.c | 7 +++
2 files changed, 25 insertions(+)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index d2256713a8..911a4adfb1 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -687,6 +687,13 @@ int efi_disk_probe(void *ctx, struct event *event)
return -1;
}
+ /* only do the boot option management when UEFI sub-system is
initialized */
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
efi_obj_list_initialized == EFI_SUCCESS) {
+ ret = efi_bootmgr_update_media_device_boot_option();
+ if (ret != EFI_SUCCESS)
+ return -1;
+ }
+
return 0;
}
@@ -773,6 +780,17 @@ int efi_disk_remove(void *ctx, struct event *event)
return efi_disk_delete_part(dev);
else
return 0;
+
+ /*
+ * TODO A flag to distinguish below 2 different scenarios of this
+ * function call is needed:
+ * a) Unplugging of a removable media under U-Boot
+ * b) U-Boot exiting and booting an OS
+ * In case of scenario a),
efi_bootmgr_update_media_device_boot_option()
+ * needs to be invoked here to update the boot options and remove
the
+ * unnecessary ones.
+ */
As in future we want to integrate more EFI drivers with the driver model
such a status should be in a global variable. It will have to be set in
ExitBootServices() before invoking dm_remove_devices_flags().
Reviewed-by: Heinrich Schuchardt
+
}
/**
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 58d4e13402..69c8b27730 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -245,6 +245,13 @@ efi_status_t efi_init_obj_list(void)
if (ret != EFI_SUCCESS)
goto out;
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
+ /* update boot option after variable service initialized */
+ ret = efi_bootmgr_update_media_device_boot_option();
+ if (ret != EFI_SUCCESS)
+ goto out;
+ }
+
/* Define supported languages */
ret = efi_init_platform_lang();
if (ret != EFI_SUCCESS)
This patch leads to a test failure in
test/py/tests/test_efi_secboot/test_signed.py::TestEfiSignedImage::test_efi_signed_image_auth2
No EFI system partition
Failed to persist EFI variables
Please, run 'make tests' before resubmitting.
Best regards
Heinrich
Pull request efi-2023-10-rc1
Dear Tom, The following changes since commit b3bbad816e97538c8c3b8acad7c7e134261cf3a3: Merge branch '2023-07-14-expo-initial-config-editor' (2023-07-14 13:26:42 -0400) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2023-10-rc1 for you to fetch changes up to 345a8b15acf228c4a429f6569c34cbc0232e76eb: doc: uefi: enhance anti-rollback documentation (2023-07-15 11:20:41 +0200) Gitlab CI showed no issues: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/16909 Pull request efi-2023-10-rc1 Documentation: * enhance UEFI anti-rollback documentation EFI: * Reconnect drivers if UninstallProtocol fails * Prefer short device paths for boot options * Fix error handling when updating boot options for block devices ---- Heinrich Schuchardt (1): README: remove Minicom comment Ilias Apalodimas (4): efi_loader: reconnect drivers on failure efi_loader: check the status of disconnected drivers efi_loader: fix the return codes of UninstallProtocol efi_selftests: add extra testcases on controller handling Masahisa Kojima (1): doc: uefi: enhance anti-rollback documentation Raymond Mao (3): Move bootorder and bootoption apis to lib Fix incorrect return code of boot option update Load option with short device path for boot vars README | 21 -- cmd/bootmenu.c | 4 +- cmd/eficonfig.c | 410 +--- doc/develop/uefi/uefi.rst | 7 + include/efi_config.h| 5 - include/efi_loader.h| 11 + lib/efi_loader/efi_bootmgr.c| 385 ++ lib/efi_loader/efi_boottime.c | 43 ++- lib/efi_loader/efi_helper.c | 25 ++ lib/efi_selftest/efi_selftest_controllers.c | 44 ++- 10 files changed, 514 insertions(+), 441 deletions(-)
[PATCH 1/1] part: eliminate part_get_info_by_name_type()
Since commit 56670d6fb83f ("disk: part: use common api to lookup part
driver") part_get_info_by_name_type() ignores the part_type parameter
used to restrict the partition table type.
omap_mmc_get_part_size() and part_get_info_by_name() are the only
consumers.
omap_mmc_get_part_size() calls with part_type = PART_TYPE_EFI because at
the time of implementation a speed up could be gained by passing the
partition table type. After 5 years experience without this restriction
it looks safe to keep it that way.
part_get_info_by_name() uses PART_TYPE_ALL.
Move the logic of part_get_info_by_name_type() to part_get_info_by_name()
and replace the function in omap_mmc_get_part_size().
Fixes: 56670d6fb83f ("disk: part: use common api to lookup part driver")
Signed-off-by: Heinrich Schuchardt
---
arch/arm/mach-omap2/utils.c | 3 +--
disk/part.c | 10 ++
include/part.h | 23 ---
3 files changed, 3 insertions(+), 33 deletions(-)
diff --git a/arch/arm/mach-omap2/utils.c b/arch/arm/mach-omap2/utils.c
index 6e6791fc65..7d938724f8 100644
--- a/arch/arm/mach-omap2/utils.c
+++ b/arch/arm/mach-omap2/utils.c
@@ -100,8 +100,7 @@ static u32 omap_mmc_get_part_size(const char *part)
return 0;
}
- /* Check only for EFI (GPT) partition table */
- res = part_get_info_by_name_type(dev_desc, part, &info, PART_TYPE_EFI);
+ res = part_get_info_by_name(dev_desc, part, &info);
if (res < 0)
return 0;
diff --git a/disk/part.c b/disk/part.c
index 35300df590..aeb4bf3b68 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -630,8 +630,8 @@ cleanup:
return ret;
}
-int part_get_info_by_name_type(struct blk_desc *dev_desc, const char *name,
- struct disk_partition *info, int part_type)
+int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
+ struct disk_partition *info)
{
struct part_driver *part_drv;
int ret;
@@ -662,12 +662,6 @@ int part_get_info_by_name_type(struct blk_desc *dev_desc,
const char *name,
return -ENOENT;
}
-int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
- struct disk_partition *info)
-{
- return part_get_info_by_name_type(dev_desc, name, info, PART_TYPE_ALL);
-}
-
/**
* Get partition info from device number and partition name.
*
diff --git a/include/part.h b/include/part.h
index be75c73549..29bdeeeccc 100644
--- a/include/part.h
+++ b/include/part.h
@@ -184,21 +184,6 @@ int blk_get_device_part_str(const char *ifname, const char
*dev_part_str,
struct blk_desc **dev_desc,
struct disk_partition *info, int allow_whole_dev);
-/**
- * part_get_info_by_name_type() - Search for a partition by name
- *for only specified partition type
- *
- * @param dev_desc - block device descriptor
- * @param gpt_name - the specified table entry name
- * @param info - returns the disk partition info
- * @param part_type - only search in partitions of this type
- *
- * Return: - the partition number on match (starting on 1), -1 on no match,
- * otherwise error
- */
-int part_get_info_by_name_type(struct blk_desc *dev_desc, const char *name,
- struct disk_partition *info, int part_type);
-
/**
* part_get_info_by_name() - Search for a partition by name
* among all available registered partitions
@@ -276,14 +261,6 @@ static inline int blk_get_device_part_str(const char
*ifname,
int allow_whole_dev)
{ *dev_desc = NULL; return -1; }
-static inline int part_get_info_by_name_type(struct blk_desc *dev_desc,
-const char *name,
-struct disk_partition *info,
-int part_type)
-{
- return -ENOENT;
-}
-
static inline int part_get_info_by_name(struct blk_desc *dev_desc,
const char *name,
struct disk_partition *info)
--
2.40.1
Re: [PATCH 1/1] test: avoid function name 'setup'
On 7/16/23 01:40, Simon Glass wrote:
Hi Heinrich,
On Sat, 15 Jul 2023 at 03:05, Heinrich Schuchardt
wrote:
pytest 7.3.2 treats the function name 'setup' as a fixture [1].
This leads to errors like:
TypeError: setup() missing 2 required positional arguments:
'disk_img' and 'osindications'
Rename setup() to capsule_setup().
[1] How to run tests written for nose
https://docs.pytest.org/en/7.3.x/how-to/nose.html
Fixes: 482ef90aeb4c ("test: efi_capsule: refactor efi_capsule test")
Signed-off-by: Heinrich Schuchardt
---
test/py/tests/test_efi_capsule/capsule_common.py | 2 +-
.../test_efi_capsule/test_capsule_firmware_fit.py| 10 +-
.../test_efi_capsule/test_capsule_firmware_raw.py| 12 ++--
.../test_capsule_firmware_signed_fit.py | 12 ++--
.../test_capsule_firmware_signed_raw.py | 12 ++--
5 files changed, 24 insertions(+), 24 deletions(-)
Reviewed-by: Simon Glass
Would it make sense to reduce the code duplication a little?
Thank you for reviewing.
Fixed 482ef90aeb4c ("test: efi_capsule: refactor efi_capsule test")
started the de-duplication effort.
Best regards
Heinrich
Re: [PATCH v2 1/6] efi_loader: add RAM disk device path
Am 18. Juli 2023 03:31:30 MESZ schrieb Masahisa Kojima
:
>Hi Heinrich,
>
>On Fri, 14 Jul 2023 at 22:44, Heinrich Schuchardt wrote:
>>
>> On 14.07.23 07:44, Masahisa Kojima wrote:
>> > This is a preparation to add the EFI_RAM_DISK_PROTOCOL.
>> > This commit adds the RAM disk device path structure
>> > and text conversion to Device Path to Text Protocol.
>> >
>> > Signed-off-by: Masahisa Kojima
>> > ---
>> > No update since v1
>> >
>> > include/efi_api.h| 19 +++
>> > lib/efi_loader/efi_device_path_to_text.c | 14 ++
>> > 2 files changed, 33 insertions(+)
>> >
>> > diff --git a/include/efi_api.h b/include/efi_api.h
>> > index 55a4c989fc..4ee4a1b5e9 100644
>> > --- a/include/efi_api.h
>> > +++ b/include/efi_api.h
>> > @@ -682,6 +682,7 @@ struct efi_device_path_uri {
>> > # define DEVICE_PATH_SUB_TYPE_CDROM_PATH 0x02
>> > # define DEVICE_PATH_SUB_TYPE_VENDOR_PATH 0x03
>> > # define DEVICE_PATH_SUB_TYPE_FILE_PATH0x04
>> > +# define DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH 0x09
>> >
>> > struct efi_device_path_hard_drive_path {
>> > struct efi_device_path dp;
>> > @@ -705,6 +706,24 @@ struct efi_device_path_file_path {
>> > u16 str[];
>> > } __packed;
>> >
>> > +/* This GUID defines a RAM Disk supporting a raw disk format in volatile
>> > memory */
>> > +#define EFI_VIRTUAL_DISK_GUID \
>> > + EFI_GUID(0x77ab535a, 0x45fc, 0x624b, \
>> > + 0x55, 0x60, 0xf7, 0xb2, 0x81, 0xd1, 0xf9, 0x6e)
>> > +
>> > +/* This GUID defines a RAM Disk supporting an ISO image in volatile
>> > memory */
>> > +#define EFI_VIRTUAL_CD_GUID \
>> > + EFI_GUID(0x3d5abd30, 0x4175, 0x87ce, \
>> > + 0x6d, 0x64, 0xd2, 0xad, 0xe5, 0x23, 0xc4, 0xbb)
>> > +
>> > +struct efi_device_path_ram_disk_path {
>> > + struct efi_device_path dp;
>> > + u64 starting_address;
>> > + u64 ending_address;
>> > + efi_guid_t disk_type_guid;
>> > + u16 disk_instance;
>> > +} __packed;
>> > +
>> > #define EFI_BLOCK_IO_PROTOCOL_GUID \
>> > EFI_GUID(0x964e5b21, 0x6459, 0x11d2, \
>> >0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>> > diff --git a/lib/efi_loader/efi_device_path_to_text.c
>> > b/lib/efi_loader/efi_device_path_to_text.c
>> > index 8c76d8be60..4395e79f33 100644
>> > --- a/lib/efi_loader/efi_device_path_to_text.c
>> > +++ b/lib/efi_loader/efi_device_path_to_text.c
>> > @@ -324,6 +324,20 @@ static char *dp_media(char *s, struct efi_device_path
>> > *dp)
>> > free(buffer);
>> > break;
>> > }
>> > + case DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH: {
>> > + struct efi_device_path_ram_disk_path *rddp =
>> > + (struct efi_device_path_ram_disk_path *)dp;
>> > + u64 start;
>> > + u64 end;
>> > +
>> > + /* Copy from packed structure to aligned memory */
>> > + memcpy(&start, &rddp->starting_address, sizeof(start));
>> > + memcpy(&end, &rddp->ending_address, sizeof(end));
>> > +
>> > + s += sprintf(s, "RamDisk(0x%llx,%llx,%pUl,0x%x)", start, end,
>> > + &rddp->disk_type_guid, rddp->disk_instance);
>>
>> If there is no alignment guarantee for starting_address, then the same
>> is true for disk_instance which may spread over two u64 blocks.
>
>disk_instance is a u16 field, so it is aligned.
A preceding node, e.g. VenHw(), may have an uneven length?
>
>>
>> In case of DEVICE_PATH_SUB_TYPE_MEMORY we don't use memcpy() to align u64.
>>
>> I don't think we call device_path_to_text before allow_unaligned().
>>
>> There is a family of functions like get_unaligned_le64() if we should
>> ever need to a align a value. Or we could copy the whole device path node.
>
>OK, I will use get_unaligned_le64().
Why do you think this is necessary?
Best regards
Heinrich
>
>Thanks,
>Masahisa Kojima
>
>>
>> Best regards
>>
>> Heinrich
>>
>> > + break;
>> > + }
>> > default:
>> > s = dp_unknown(s, dp);
>> > break;
>>
Re: [PATCH v10 3/4] Boot var automatic management for removable medias
On 18.07.23 11:03, Masahisa Kojima wrote:
Hi Raymond,
On Tue, 18 Jul 2023 at 05:23, Raymond Mao wrote:
Hi Heinrich,
I run 'make tests' with the patches locally but no errors observed from
'test_signed.py', below are few lines from the output console:
```
test/py/tests/test_efi_secboot/test_signed.py
[ 90%]
```
I think the test cases are just skipped, not executed.
I guess that some required packages are missing on the host machine,
such as sbsigntool, efitools and libguestfs-tools.
build-sandbox/test-log.html will help to analyze why the tests are skipped.
The following document describes prerequisites to run python tests.
https://u-boot.readthedocs.io/en/latest/develop/py_testing.html
'make tests' runs many test cases and takes time.
'./test/py/test.py --bd sandbox --build -k efi_secboot' will run only
the efi_secboot test
and it will be useful for debugging purposes.
Thanks,
Masahisa Kojima
To get more output describing why tests are skipped or fail use
export PYTEST_ADDOPTS="-ra"
https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/16948
shows the errors with the suggested patch.
Best regards
Heinrich
Regards,
Raymond
On Sat, 15 Jul 2023 at 05:19, Heinrich Schuchardt wrote:
On 7/9/23 10:56, Heinrich Schuchardt wrote:
On 6/19/23 23:23, Raymond Mao wrote:
Changes for complying to EFI spec §3.5.1.1
'Removable Media Boot Behavior'.
Boot variables can be automatically generated during a removable
media is probed. At the same time, unused boot variables will be
detected and removed.
Please note that currently the function 'efi_disk_remove' has no
ability to distinguish below two scenarios
a) Unplugging of a removable media under U-Boot
b) U-Boot exiting and booting an OS
Thus currently the boot variables management is not added into
'efi_disk_remove' to avoid boot options being added/erased
repeatedly under scenario b) during power cycles
See TODO comments under function 'efi_disk_remove' for more details
Signed-off-by: Raymond Mao
---
Changes in v3
- Split the patch into moving and renaming functions and
individual patches for each changed functionality
Changes in v5
- Move function call of efi_bootmgr_update_media_device_boot_option()
from efi_init_variables() to efi_init_obj_list()
Changes in v6
- Revert unrelated changes
Changes in v7
- adapt the return code of function
efi_bootmgr_update_media_device_boot_option()
Changes in v8
- add a note in the commit message for future reference
Changes in v9
- amend the note text in the commit message
- add a TODO comment in 'efi_disk_remove'
Changes in v10
- fix typo and build failures with 'CONFIG_CMD_BOOTEFI_BOOTMGR=n'
lib/efi_loader/efi_disk.c | 18 ++
lib/efi_loader/efi_setup.c | 7 +++
2 files changed, 25 insertions(+)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index d2256713a8..911a4adfb1 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -687,6 +687,13 @@ int efi_disk_probe(void *ctx, struct event *event)
return -1;
}
+/* only do the boot option management when UEFI sub-system is
initialized */
+if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
efi_obj_list_initialized == EFI_SUCCESS) {
+ret = efi_bootmgr_update_media_device_boot_option();
+if (ret != EFI_SUCCESS)
+return -1;
+}
+
return 0;
}
@@ -773,6 +780,17 @@ int efi_disk_remove(void *ctx, struct event *event)
return efi_disk_delete_part(dev);
else
return 0;
+
+/*
+ * TODO A flag to distinguish below 2 different scenarios of this
+ * function call is needed:
+ * a) Unplugging of a removable media under U-Boot
+ * b) U-Boot exiting and booting an OS
+ * In case of scenario a),
efi_bootmgr_update_media_device_boot_option()
+ * needs to be invoked here to update the boot options and remove
the
+ * unnecessary ones.
+ */
As in future we want to integrate more EFI drivers with the driver model
such a status should be in a global variable. It will have to be set in
ExitBootServices() before invoking dm_remove_devices_flags().
Reviewed-by: Heinrich Schuchardt
+
}
/**
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 58d4e13402..69c8b27730 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -245,6 +245,13 @@ efi_status_t efi_init_obj_list(void)
if (ret != EFI_SUCCESS)
goto out;
+if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
+/* update boot option after variable service initialized */
+ret = efi_bootmgr_update_media_device_boot_option();
+if (ret != EFI_SUCCESS)
+
Re: [PATCH] efi_loader: Allow also empty capsule to be process
On 13.07.23 16:35, Michal Simek wrote:
Empty capsule are also allowed to be process. Without it updated images
can't change their Image Acceptance state from no to yes.
Is there any documentation describing the usage of empty capsule to set
the image acceptance state?
Best regards
Heinrich
Signed-off-by: Michal Simek
---
lib/efi_loader/efi_capsule.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 7a6f195cbc02..93e83e5f04c3 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -752,7 +752,8 @@ efi_status_t EFIAPI efi_update_capsule(
log_debug("Capsule[%d] (guid:%pUs)\n",
i, &capsule->capsule_guid);
if (!guidcmp(&capsule->capsule_guid,
-&efi_guid_firmware_management_capsule_id)) {
+&efi_guid_firmware_management_capsule_id) ||
+ fwu_empty_capsule(capsule)) {
ret = efi_capsule_update_firmware(capsule);
} else {
log_err("Unsupported capsule type: %pUs\n",
Re: EFI breaks USB dual port detection - our observations
Am 18. Juli 2023 20:37:04 MESZ schrieb Suniel Mahesh : >Hi, > >I am testing the USB infrastructure on a Rockchip RK3328 based >roc-rk3328-cc target. > >The USB tree on the device is as follows: > >=> usb tree >USB device tree: > 1 Hub (480 Mb/s, 0mA) > u-boot EHCI Host Controller > > 1 Hub (12 Mb/s, 0mA) > U-Boot Root Hub (OHCI) > > 1 Hub (5 Gb/s, 0mA) > U-Boot XHCI Host Controller > > 1 Hub (480 Mb/s, 0mA) > U-Boot Root Hub (DWC2) > >For some reason I am unable to detect storage device on DWC2 when two USB >drives >are simultaneously connected on EHCI and DWC2. Though it shows 2 storage >devices >when i do a "usb start/reset" and it gives usb_mass_storage.lun0 failed >message as >shown below. On which U-Boot version are you? Cf. https://github.com/trini/u-boot/commit/180b7118bed8433f9cfe4b5ad62c6b0d901924f5 Best regards Heinrich > >=> usb start >starting USB... >Bus usb@ff5c: USB EHCI 1.00 >Bus usb@ff5d: USB OHCI 1.0 >Bus usb@ff60: generic_phy_get_bulk : no phys property >Register 2000140 NbrPorts 2 >Starting the controller >USB XHCI 1.10 >Bus usb@ff58: USB DWC2 >scanning bus usb@ff5c for devices... >2 USB Device(s) found >scanning bus usb@ff5d for devices... 1 USB Device(s) found >scanning bus usb@ff60 for devices... 1 USB Device(s) found >scanning bus usb@ff58 for devices... >Adding disk for usb_mass_storage.lun0 failed >(err=-9223372036854775788/0x8014) >device 'usb_mass_storage.lun0' failed to unbind >1 USB Device(s) found >device 'usb_mass_storage.lun0' failed to unbind > scanning usb for storage devices... 2 Storage Device(s) found > >=> usb tree >USB device tree: > 1 Hub (480 Mb/s, 0mA) > | u-boot EHCI Host Controller > | > +-2 Mass Storage (480 Mb/s, 224mA) > SanDisk Dual Drive 040130e3ee554b7078843f4eb331646 > > 1 Hub (12 Mb/s, 0mA) > U-Boot Root Hub > > 1 Hub (5 Gb/s, 0mA) > U-Boot XHCI Host Controller > > 1 Hub (480 Mb/s, 0mA) > U-Boot Root Hub > >call trace: (detailed log: >https://gist.github.com/sunielmahesh/10088ea7b536cc9898ef787d9db43721) > >efi_install_multiple_protocol_interfaces_int >device path >/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USB(0x0,0x0)/USB(0x1,0x0)/Ctrl(0x0) >09576e91-6d3f-11d2-8e39-00a0c969723b, fcf1bae8, >fcf1bae0efi_locate_device_path: ret = EFI_SUCCESS >efi_install_multiple_protocol_interfaces_int: ret=0, dp->type:7f >Path >/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USB(0x0,0x0)/USB(0x1,0x0)/Ctrl(0x0) >already installed >install failed 8014 > >However, an interesting observation is that if i disable CONFIG_EFI_LOADER, >then I am able to detect the storage devices >without any usb_mass_storage.lun0 failed message: > >=> usb reset >resetting USB... >Bus usb@ff5c: USB EHCI 1.00 >Bus usb@ff5d: USB OHCI 1.0 >Bus usb@ff60: generic_phy_get_bulk : no phys property >Register 2000140 NbrPorts 2 >Starting the controller >USB XHCI 1.10 >Bus usb@ff58: USB DWC2 >scanning bus usb@ff5c for devices... 2 USB Device(s) found >scanning bus usb@ff5d for devices... 1 USB Device(s) found >scanning bus usb@ff60 for devices... 1 USB Device(s) found >scanning bus usb@ff58 for devices... 2 USB Device(s) found > scanning usb for storage devices... 2 Storage Device(s) found >=> usb tree >USB device tree: > 1 Hub (480 Mb/s, 0mA) > | u-boot EHCI Host Controller > | > +-2 Mass Storage (480 Mb/s, 224mA) > SanDisk Dual Drive 040130e3ee554b7078843f4eb331646 > > 1 Hub (12 Mb/s, 0mA) > U-Boot Root Hub > > 1 Hub (5 Gb/s, 0mA) > U-Boot XHCI Host Controller > > 1 Hub (480 Mb/s, 0mA) > | U-Boot Root Hub > | > +-2 Mass Storage (480 Mb/s, 224mA) > SanDisk Dual Drive 04019c9b2e1a58f24ee318c3c123aa5 > >Please share you thoughts. > >Thanks and Regards
Re: [RFC] efi_driver: fix a parent issue in efi-created block devices
On 7/19/23 03:08, Simon Glass wrote:
Hi AKASHI,
On Tue, 18 Jul 2023 at 18:22, AKASHI Takahiro
wrote:
An EFI application may create an EFI block device (EFI_BLOCK_IO_PROTOCOL) in
EFI world, which in turn generates a corresponding U-Boot block device based on
U-Boot's Driver Model.
The latter device, however, doesn't work as U-Boot proper block device
due to an issue in efi_driver's implementation. We saw discussions in the past,
most recently in [1].
[1] https://lists.denx.de/pipermail/u-boot/2023-July/522565.html
This RFC patch tries to address (part of) the issue.
If it is considered acceptable, I will create a formal patch.
Withtout this patch,
===8<===
=> env set efi_selftest 'block device'
=> bootefi selftest
...
Summary: 0 failures
=> dm tree
Class Index Probed DriverName
---
root 0 [ + ] root_driver root_driver
...
bootmeth 7 [ ] vbe_simple| `-- vbe_simple
blk 0 [ + ] efi_blk `-- efiblk#0
partition 0 [ + ] blk_partition `-- efiblk#0:1
=> ls efiloader 0:1
** Bad device specification efiloader 0 **
Couldn't find partition efiloader 0:1
===>8===
With this patch applied, efiblk#0(:1) now gets accessible.
===8<===
=> env set efi_selftest 'block device'
=> bootefi selftest
...
Summary: 0 failures
=> dm tree
Class Index Probed DriverName
---
root 0 [ + ] root_driver root_driver
...
bootmeth 7 [ ] vbe_simple| `-- vbe_simple
efi 0 [ + ] EFI block driver `--
/VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)
blk 0 [ + ] efi_blk `-- efiblk#0
partition 0 [ + ] blk_partition `-- efiblk#0:1
=> ls efiloader 0:1
13 hello.txt
7 u-boot.txt
2 file(s), 0 dir(s)
===>8===
Signed-off-by: AKASHI Takahiro
---
include/efi_driver.h | 2 +-
lib/efi_driver/efi_block_device.c| 17 -
lib/efi_driver/efi_uclass.c | 8 +++-
lib/efi_selftest/efi_selftest_block_device.c | 2 ++
4 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/include/efi_driver.h b/include/efi_driver.h
index 63a95e4cf800..ed66796f3519 100644
--- a/include/efi_driver.h
+++ b/include/efi_driver.h
@@ -42,7 +42,7 @@ struct efi_driver_ops {
const efi_guid_t *child_protocol;
efi_status_t (*init)(struct efi_driver_binding_extended_protocol
*this);
efi_status_t (*bind)(struct efi_driver_binding_extended_protocol *this,
-efi_handle_t handle, void *interface);
+efi_handle_t handle, void *interface, char *name);
};
#endif /* _EFI_DRIVER_H */
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index add00eeebbea..43b7ed7c973c 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -115,9 +115,9 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t
blknr, lbaint_t blkcnt,
* Return: status code
*/
static efi_status_t
-efi_bl_create_block_device(efi_handle_t handle, void *interface)
+efi_bl_create_block_device(efi_handle_t handle, void *interface, struct
udevice *parent)
{
- struct udevice *bdev = NULL, *parent = dm_root();
+ struct udevice *bdev = NULL;
efi_status_t ret;
int devnum;
char *name;
@@ -181,7 +181,7 @@ err:
*/
static efi_status_t efi_bl_bind(
struct efi_driver_binding_extended_protocol *this,
- efi_handle_t handle, void *interface)
+ efi_handle_t handle, void *interface, char *name)
{
efi_status_t ret = EFI_SUCCESS;
struct efi_object *obj = efi_search_obj(handle);
@@ -191,8 +191,15 @@ static efi_status_t efi_bl_bind(
if (!obj || !interface)
return EFI_INVALID_PARAMETER;
- if (!handle->dev)
- ret = efi_bl_create_block_device(handle, interface);
+ if (!handle->dev) {
+ struct udevice *parent;
+
+ ret = device_bind_driver(dm_root(), "EFI block driver", name,
&parent);
Can you use a non-root device as the parent?
The parent of an iSCSI drive handled by iPXE will be a network
interface. For a RAM drive there will be no physical parent at all.
The design bug is in blk_get_devnum_by_uclass_idname() which instead of
looking at blk_desc->uclass_id looks at the parent.
When will you fix this Simon?
Best regards
Heinrich
+ if (!ret)
+ ret = efi_bl_create_block_device(handle, interface,
parent);
+ else
+ ret = EFI_DEVICE_ERROR;
+ }
return ret;
}
diff --git a/lib/efi_driver/efi_u
Re: [PATCH 1/1] part: eliminate part_get_info_by_name_type()
On 7/19/23 03:08, Simon Glass wrote:
Hi Heinrich,
On Sun, 16 Jul 2023 at 05:34, Heinrich Schuchardt
wrote:
Since commit 56670d6fb83f ("disk: part: use common api to lookup part
driver") part_get_info_by_name_type() ignores the part_type parameter
used to restrict the partition table type.
omap_mmc_get_part_size() and part_get_info_by_name() are the only
consumers.
omap_mmc_get_part_size() calls with part_type = PART_TYPE_EFI because at
the time of implementation a speed up could be gained by passing the
partition table type. After 5 years experience without this restriction
it looks safe to keep it that way.
part_get_info_by_name() uses PART_TYPE_ALL.
Move the logic of part_get_info_by_name_type() to part_get_info_by_name()
and replace the function in omap_mmc_get_part_size().
Fixes: 56670d6fb83f ("disk: part: use common api to lookup part driver")
Signed-off-by: Heinrich Schuchardt
---
arch/arm/mach-omap2/utils.c | 3 +--
disk/part.c | 10 ++
include/part.h | 23 ---
3 files changed, 3 insertions(+), 33 deletions(-)
That explains the strange behaviour I saw when fiddling with this a
month or so back, thanks.
Reviewed-by: Simon Glass
Is there a test to update for this?
Thanks for reviewing.
As we don't change any existing behavior there is no test to update.
Best regards
Heinrich
