Add a few links to documents about Verified Boot for Embedded (VBE). These will be expanded as development proceeds.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Add new patch with some documentation and links for VBE

doc/develop/bootstd.rst | 1 + doc/develop/index.rst | 1 + doc/develop/vbe.rst | 26 ++++++++++++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 doc/develop/vbe.rst

diff --git a/doc/develop/bootstd.rst b/doc/develop/bootstd.rst index 5e9c0d282bb..dadd3473e5c 100644 --- a/doc/develop/bootstd.rst +++ b/doc/develop/bootstd.rst @@ -32,6 +32,7 @@ way to boot with U-Boot. The feature is extensible to different Operating Systems (such as Chromium OS) and devices (beyond just block and network devices). It supports EFI boot and EFI bootmgr too.

+Finally, standard boot supports the operation of :doc:`vbe`.

Bootflow -------- diff --git a/doc/develop/index.rst b/doc/develop/index.rst index 73741ceb6a2..6156474b47c 100644 --- a/doc/develop/index.rst +++ b/doc/develop/index.rst @@ -38,6 +38,7 @@ Implementation smbios spl uefi/index + vbe version

Debugging diff --git a/doc/develop/vbe.rst b/doc/develop/vbe.rst new file mode 100644 index 00000000000..8f147fd9360 --- /dev/null +++ b/doc/develop/vbe.rst @@ -0,0 +1,26 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Verified Boot for Embedded (VBE) +================================ + +Introduction +------------ + +VBE provides a standard boot mechanism for embedded systems. If defines +how firmware and Operating Systems are located, updated and verified. + +Within U-Boot, one or more VBE bootmeths implement the boot logic. For example, +the vbe-simple bootmeth handles finding the firmware (e.g. in MMC) and starting +it. Typically the bootmeth is started up in VPL and controls which SPL and +U-Boot binaries are loaded. + +A 'vbe' command provides access to various aspects of VBE's operation, including +listing methods and getting the status for a method. + +For a detailed overview of VBE, see vbe-intro_. A fuller description of +bootflows is at vbe-bootflows_ and the firmware-update mechanism is described at +vbe-fwupdate_. + +.. _vbe-intro: https://docs.google.com/document/d/e/2PACX-1vQjXLPWMIyVktaTMf8edHZYDrEvMYD_i... +.. _vbe-bootflows: https://docs.google.com/document/d/e/2PACX-1vR0OzhuyRJQ8kdeOibS3xB1rVFy3J4M_... +.. _vbe-fwupdate: https://docs.google.com/document/d/e/2PACX-1vTnlIL17vVbl6TVoTHWYMED0bme7oHHN...

Rename these to VESA, itself an abbreviation, to avoid a conflict with Verified Boot for Embedded.

Rename this to avoid referencing VBE.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

arch/x86/lib/bios.c | 10 +++++----- arch/x86/lib/fsp/fsp_graphics.c | 2 +- drivers/bios_emulator/atibios.c | 16 ++++++++-------- drivers/pci/pci_rom.c | 12 ++++++------ drivers/video/broadwell_igd.c | 2 +- drivers/video/coreboot.c | 2 +- drivers/video/efi.c | 2 +- drivers/video/ivybridge_igd.c | 2 +- drivers/video/vesa.c | 2 +- include/bios_emul.h | 6 +++--- include/vesa.h | 25 ++++++++++++++----------- 11 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/arch/x86/lib/bios.c b/arch/x86/lib/bios.c index 087539ba7db..94349ba8073 100644 --- a/arch/x86/lib/bios.c +++ b/arch/x86/lib/bios.c @@ -190,7 +190,7 @@ static void setup_realmode_idt(void) }

#ifdef CONFIG_FRAMEBUFFER_SET_VESA_MODE -static u8 vbe_get_mode_info(struct vbe_mode_info *mi) +static u8 vbe_get_mode_info(struct vesa_state *mi) { u16 buffer_seg; u16 buffer_adr; @@ -204,13 +204,13 @@ static u8 vbe_get_mode_info(struct vbe_mode_info *mi)

realmode_interrupt(0x10, VESA_GET_MODE_INFO, 0x0000, mi->video_mode, 0x0000, buffer_seg, buffer_adr); - memcpy(mi->mode_info_block, buffer, sizeof(struct vbe_mode_info)); + memcpy(mi->mode_info_block, buffer, sizeof(struct vesa_state)); mi->valid = true;

return 0; }

-static u8 vbe_set_mode(struct vbe_mode_info *mi) +static u8 vbe_set_mode(struct vesa_state *mi) { int video_mode = mi->video_mode;

@@ -225,7 +225,7 @@ static u8 vbe_set_mode(struct vbe_mode_info *mi) return 0; }

-static void vbe_set_graphics(int vesa_mode, struct vbe_mode_info *mode_info) +static void vbe_set_graphics(int vesa_mode, struct vesa_state *mode_info) { unsigned char *framebuffer;

@@ -249,7 +249,7 @@ static void vbe_set_graphics(int vesa_mode, struct vbe_mode_info *mode_info) #endif /* CONFIG_FRAMEBUFFER_SET_VESA_MODE */

void bios_run_on_x86(struct udevice *dev, unsigned long addr, int vesa_mode, - struct vbe_mode_info *mode_info) + struct vesa_state *mode_info) { pci_dev_t pcidev = dm_pci_get_bdf(dev); u32 num_dev; diff --git a/arch/x86/lib/fsp/fsp_graphics.c b/arch/x86/lib/fsp/fsp_graphics.c index 6a7552e6956..b07c666caf7 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 = vbe_setup_video_priv(vesa, uc_priv, plat); + ret = vesa_setup_video_priv(vesa, uc_priv, plat); if (ret) goto err;

diff --git a/drivers/bios_emulator/atibios.c b/drivers/bios_emulator/atibios.c index 09da76bc5d9..7ebead6bfad 100644 --- a/drivers/bios_emulator/atibios.c +++ b/drivers/bios_emulator/atibios.c @@ -83,13 +83,13 @@ static const void *bios_ptr(const void *buf, BE_VGAInfo *vga_info, }

static int atibios_debug_mode(BE_VGAInfo *vga_info, RMREGS *regs, - int vesa_mode, struct vbe_mode_info *mode_info) + int vesa_mode, struct vesa_state *mode_info) { void *buffer = (void *)(M.mem_base + vbe_offset); u16 buffer_seg = (((unsigned long)vbe_offset) >> 4) & 0xff00; u16 buffer_adr = ((unsigned long)vbe_offset) & 0xffff; struct vesa_mode_info *vm; - struct vbe_info *info; + struct vesa_bios_ext_info *info; const u16 *modes_bios, *ptr; u16 *modes; int size; @@ -140,7 +140,7 @@ static int atibios_debug_mode(BE_VGAInfo *vga_info, RMREGS *regs, int attr;

debug("Mode %x: ", mode); - memset(buffer, '\0', sizeof(struct vbe_mode_info)); + memset(buffer, '\0', sizeof(struct vesa_state)); regs->e.eax = VESA_GET_MODE_INFO; regs->e.ebx = 0; regs->e.ecx = mode; @@ -174,7 +174,7 @@ static int atibios_debug_mode(BE_VGAInfo *vga_info, RMREGS *regs, }

static int atibios_set_vesa_mode(RMREGS *regs, int vesa_mode, - struct vbe_mode_info *mode_info) + struct vesa_state *mode_info) { void *buffer = (void *)(M.mem_base + vbe_offset); u16 buffer_seg = (((unsigned long)vbe_offset) >> 4) & 0xff00; @@ -192,7 +192,7 @@ static int atibios_set_vesa_mode(RMREGS *regs, int vesa_mode, return -ENOSYS; }

- memset(buffer, '\0', sizeof(struct vbe_mode_info)); + memset(buffer, '\0', sizeof(struct vesa_state)); debug("VBE: Geting info for VESA mode %#04x\n", vesa_mode); regs->e.eax = VESA_GET_MODE_INFO; regs->e.ecx = vesa_mode; @@ -231,7 +231,7 @@ at this stage the controller has its I/O and memory space enabled and that all other controllers are in a disabled state. ****************************************************************************/ static void PCI_doBIOSPOST(struct udevice *pcidev, BE_VGAInfo *vga_info, - int vesa_mode, struct vbe_mode_info *mode_info) + int vesa_mode, struct vesa_state *mode_info) { RMREGS regs; RMSREGS sregs; @@ -416,7 +416,7 @@ image we can extract over the PCI bus. ****************************************************************************/ static int PCI_postController(struct udevice *pcidev, uchar *bios_rom, int bios_len, BE_VGAInfo *vga_info, - int vesa_mode, struct vbe_mode_info *mode_info) + int vesa_mode, struct vesa_state *mode_info) { u32 bios_image_len; uchar *mapped_bios; @@ -496,7 +496,7 @@ void biosemu_set_interrupt_handler(int intnum, int (*int_func)(void))

int biosemu_run(struct udevice *pcidev, uchar *bios_rom, int bios_len, BE_VGAInfo *vga_info, int clean_up, int vesa_mode, - struct vbe_mode_info *mode_info) + struct vesa_state *mode_info) { /*Post all the display controller BIOS'es*/ if (!PCI_postController(pcidev, bios_rom, bios_len, vga_info, diff --git a/drivers/pci/pci_rom.c b/drivers/pci/pci_rom.c index ceeb59d1fe4..27a24daa12a 100644 --- a/drivers/pci/pci_rom.c +++ b/drivers/pci/pci_rom.c @@ -202,7 +202,7 @@ static int pci_rom_load(struct pci_rom_header *rom_header, return 0; }

-struct vbe_mode_info mode_info; +struct vesa_state mode_info;

void setup_video(struct screen_info *screen_info) { @@ -326,9 +326,9 @@ err: }

#ifdef CONFIG_DM_VIDEO -int vbe_setup_video_priv(struct vesa_mode_info *vesa, - struct video_priv *uc_priv, - struct video_uc_plat *plat) +int vesa_setup_video_priv(struct vesa_mode_info *vesa, + struct video_priv *uc_priv, + struct video_uc_plat *plat) { if (!vesa->x_resolution) return log_msg_ret("No x resolution", -ENXIO); @@ -358,7 +358,7 @@ int vbe_setup_video_priv(struct vesa_mode_info *vesa, return 0; }

-int vbe_setup_video(struct udevice *dev, int (*int15_handler)(void)) +int vesa_setup_video(struct udevice *dev, int (*int15_handler)(void)) { struct video_uc_plat *plat = dev_get_uclass_plat(dev); struct video_priv *uc_priv = dev_get_uclass_priv(dev); @@ -378,7 +378,7 @@ int vbe_setup_video(struct udevice *dev, int (*int15_handler)(void)) return ret; }

- ret = vbe_setup_video_priv(&mode_info.vesa, uc_priv, plat); + ret = vesa_setup_video_priv(&mode_info.vesa, uc_priv, plat); if (ret) { if (ret == -ENFILE) { /* diff --git a/drivers/video/broadwell_igd.c b/drivers/video/broadwell_igd.c index 81f0fd8c019..6aa4e27071d 100644 --- a/drivers/video/broadwell_igd.c +++ b/drivers/video/broadwell_igd.c @@ -681,7 +681,7 @@ static int broadwell_igd_probe(struct udevice *dev) debug("%s: is_broadwell=%d\n", __func__, is_broadwell); ret = igd_pre_init(dev, is_broadwell); if (!ret) { - ret = vbe_setup_video(dev, broadwell_igd_int15_handler); + ret = vesa_setup_video(dev, broadwell_igd_int15_handler); if (ret) debug("failed to run video BIOS: %d\n", ret); } diff --git a/drivers/video/coreboot.c b/drivers/video/coreboot.c index 3efc65daa2b..d2d87c75c89 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 = vbe_setup_video_priv(vesa, uc_priv, plat); + ret = vesa_setup_video_priv(vesa, 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 d60b6e27569..b11e42c0ebf 100644 --- a/drivers/video/efi.c +++ b/drivers/video/efi.c @@ -149,7 +149,7 @@ static int efi_video_probe(struct udevice *dev) if (ret) goto err;

- ret = vbe_setup_video_priv(vesa, uc_priv, plat); + ret = vesa_setup_video_priv(vesa, uc_priv, plat); if (ret) goto err;

diff --git a/drivers/video/ivybridge_igd.c b/drivers/video/ivybridge_igd.c index 18672a18973..9264dd6770d 100644 --- a/drivers/video/ivybridge_igd.c +++ b/drivers/video/ivybridge_igd.c @@ -762,7 +762,7 @@ static int bd82x6x_video_probe(struct udevice *dev) rev = gma_func0_init(dev); if (rev < 0) return rev; - ret = vbe_setup_video(dev, int15_handler); + ret = vesa_setup_video(dev, int15_handler); if (ret) return ret;

diff --git a/drivers/video/vesa.c b/drivers/video/vesa.c index 91da939e59b..cac3bb0c331 100644 --- a/drivers/video/vesa.c +++ b/drivers/video/vesa.c @@ -17,7 +17,7 @@ static int vesa_video_probe(struct udevice *dev) ulong fbbase; int ret;

- ret = vbe_setup_video(dev, NULL); + ret = vesa_setup_video(dev, NULL); if (ret) return log_ret(ret);

diff --git a/include/bios_emul.h b/include/bios_emul.h index 72410dc7948..a7e6d73972c 100644 --- a/include/bios_emul.h +++ b/include/bios_emul.h @@ -36,14 +36,14 @@ typedef struct { u8 LowMem[1536]; } BE_VGAInfo;

-struct vbe_mode_info; +struct vesa_state;

int BootVideoCardBIOS(struct udevice *pcidev, BE_VGAInfo **pVGAInfo, int clean_up);

/* Run a BIOS ROM natively (only supported on x86 machines) */ void bios_run_on_x86(struct udevice *dev, unsigned long addr, int vesa_mode, - struct vbe_mode_info *mode_info); + struct vesa_state *mode_info);

/** * bios_set_interrupt_handler() - Install an interrupt handler for the BIOS @@ -61,6 +61,6 @@ int biosemu_setup(struct udevice *pcidev, BE_VGAInfo **pVGAInfo);

int biosemu_run(struct udevice *dev, uchar *bios_rom, int bios_len, BE_VGAInfo *vga_info, int clean_up, int vesa_mode, - struct vbe_mode_info *mode_info); + struct vesa_state *mode_info);

#endif diff --git a/include/vesa.h b/include/vesa.h index 30df58a9f1b..a42c1796863 100644 --- a/include/vesa.h +++ b/include/vesa.h @@ -11,7 +11,7 @@ #define _VESA_H

/* these structs are for input from and output to OF */ -struct __packed vbe_screen_info { +struct __packed vesa_screen_info { u8 display_type; /* 0=NONE, 1= analog, 2=digital */ u16 screen_width; u16 screen_height; @@ -22,7 +22,7 @@ struct __packed vbe_screen_info { u8 edid_block_zero[128]; };

-struct __packed vbe_screen_info_input { +struct __packed vesa_screen_info_input { u8 signature[4]; u16 size_reserved; u8 monitor_number; @@ -30,8 +30,11 @@ struct __packed vbe_screen_info_input { u8 color_depth; };

-/* these structs only store the required a subset of the VBE-defined fields */ -struct __packed vbe_info { +/* + * These structs only store the required subset of fields in Vesa BIOS + * Extensions + */ +struct __packed vesa_bios_ext_info { char signature[4]; u16 version; u32 oem_string_ptr; @@ -80,7 +83,7 @@ struct __packed vesa_mode_info { u8 reserved[206]; };

-struct vbe_mode_info { +struct vesa_state { u16 video_mode; bool valid; union { @@ -89,7 +92,7 @@ struct vbe_mode_info { }; };

-struct vbe_ddc_info { +struct vesa_ddc_info { u8 port_number; /* i.e. monitor number */ u8 edid_transfer_time; u8 ddc_level; @@ -101,13 +104,13 @@ struct vbe_ddc_info { #define VESA_SET_MODE 0x4f02 #define VESA_GET_CUR_MODE 0x4f03

-extern struct vbe_mode_info mode_info; +extern struct vesa_state mode_info;

struct video_priv; struct video_uc_plat; -int vbe_setup_video_priv(struct vesa_mode_info *vesa, - struct video_priv *uc_priv, - struct video_uc_plat *plat); -int vbe_setup_video(struct udevice *dev, int (*int15_handler)(void)); +int vesa_setup_video_priv(struct vesa_mode_info *vesa, + struct video_priv *uc_priv, + struct video_uc_plat *plat); +int vesa_setup_video(struct udevice *dev, int (*int15_handler)(void));

#endif

The unflattening algorithm results in a single block of memory being allocated for the whole tree. When writing new properties, these are allocated new memory outside that block. When the block is freed, the allocated properties remain.

Document how this works and the potential memory leak, as well as mentioning that updating the livetree is actually supported now.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

doc/develop/driver-model/livetree.rst | 17 +++++++++++++---- include/dm/ofnode.h | 3 ++- 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/doc/develop/driver-model/livetree.rst b/doc/develop/driver-model/livetree.rst index 9f654f3b894..fb2969259d0 100644 --- a/doc/develop/driver-model/livetree.rst +++ b/doc/develop/driver-model/livetree.rst @@ -211,9 +211,18 @@ using it in new code. Modifying the livetree ----------------------

-This is not currently supported. Once implemented it should provide a much -more efficient implementation for modification of the device tree than using -the flat tree. +This is supported in a limited way, with ofnode_write_prop() and related +functions. + +The unflattening algorithm results in a single block of memory being +allocated for the whole tree. When writing new properties, these are +allocated new memory outside that block. When the block is freed, the +allocated properties remain. This can result in a memory leak. + +The solution to this leak would be to add a flag for properties (and nodes when +support is provided for adding those) that indicates that they should be +freed. Then the tree can be scanned for these 'separately allocated' nodes and +properties before freeing the memory block.

Internal implementation @@ -281,6 +290,6 @@ Live tree support was introduced in U-Boot 2017.07. There is still quite a bit of work to do to flesh this out:

- tests for all access functions -- support for livetree modification +- more support for livetree modification - addition of more access functions as needed - support for livetree in SPL and before relocation (if desired) diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index 346b09c7d96..5a5309d79a7 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -1081,7 +1081,8 @@ int ofnode_device_is_compatible(ofnode node, const char *compat); * ofnode_write_prop() - Set a property of a ofnode * * Note that the value passed to the function is *not* allocated by the - * function itself, but must be allocated by the caller if necessary. + * function itself, but must be allocated by the caller if necessary. However + * it does allocate memory for the property struct and name. * * @node: The node for whose property should be set * @propname: The name of the property to set

This fits better in the ofnode tests, so move it.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

test/dm/ofnode.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++ test/dm/test-fdt.c | 53 ------------------------------------------- 2 files changed, 56 insertions(+), 53 deletions(-)

diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c index 6a252f3f504..b8d8e440dbc 100644 --- a/test/dm/ofnode.c +++ b/test/dm/ofnode.c @@ -4,8 +4,12 @@ #include <dm.h> #include <log.h> #include <of_live.h> +#include <dm/device-internal.h> +#include <dm/lists.h> #include <dm/of_extra.h> +#include <dm/root.h> #include <dm/test.h> +#include <dm/uclass-internal.h> #include <test/test.h> #include <test/ut.h>

@@ -536,3 +540,55 @@ static int dm_test_ofnode_root(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_ofnode_root, UT_TESTF_SCAN_FDT); + +static int dm_test_ofnode_livetree_writing(struct unit_test_state *uts) +{ + struct udevice *dev; + ofnode node; + + if (!of_live_active()) { + printf("Live tree not active; ignore test\n"); + return 0; + } + + /* Test enabling devices */ + + node = ofnode_path("/usb@2"); + + ut_assert(!of_device_is_available(ofnode_to_np(node))); + ofnode_set_enabled(node, true); + ut_assert(of_device_is_available(ofnode_to_np(node))); + + device_bind_driver_to_node(dm_root(), "usb_sandbox", "usb@2", node, + &dev); + ut_assertok(uclass_find_device_by_seq(UCLASS_USB, 2, &dev)); + + /* Test string property setting */ + + ut_assert(device_is_compatible(dev, "sandbox,usb")); + ofnode_write_string(node, "compatible", "gdsys,super-usb"); + ut_assert(device_is_compatible(dev, "gdsys,super-usb")); + ofnode_write_string(node, "compatible", "sandbox,usb"); + ut_assert(device_is_compatible(dev, "sandbox,usb")); + + /* Test setting generic properties */ + + /* Non-existent in DTB */ + ut_asserteq_64(FDT_ADDR_T_NONE, dev_read_addr(dev)); + /* reg = 0x42, size = 0x100 */ + ut_assertok(ofnode_write_prop(node, "reg", 8, + "\x00\x00\x00\x42\x00\x00\x01\x00")); + ut_asserteq(0x42, dev_read_addr(dev)); + + /* Test disabling devices */ + + device_remove(dev, DM_REMOVE_NORMAL); + device_unbind(dev); + + ut_assert(of_device_is_available(ofnode_to_np(node))); + ofnode_set_enabled(node, false); + ut_assert(!of_device_is_available(ofnode_to_np(node))); + + return 0; +} +DM_TEST(dm_test_ofnode_livetree_writing, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index f9e81747595..6118ad42ca8 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -17,7 +17,6 @@ #include <dm/devres.h> #include <dm/uclass-internal.h> #include <dm/util.h> -#include <dm/lists.h> #include <dm/of_access.h> #include <linux/ioport.h> #include <test/test.h> @@ -735,58 +734,6 @@ static int dm_test_fdt_remap_addr_name_live(struct unit_test_state *uts) DM_TEST(dm_test_fdt_remap_addr_name_live, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);

-static int dm_test_fdt_livetree_writing(struct unit_test_state *uts) -{ - struct udevice *dev; - ofnode node; - - if (!of_live_active()) { - printf("Live tree not active; ignore test\n"); - return 0; - } - - /* Test enabling devices */ - - node = ofnode_path("/usb@2"); - - ut_assert(!of_device_is_available(ofnode_to_np(node))); - ofnode_set_enabled(node, true); - ut_assert(of_device_is_available(ofnode_to_np(node))); - - device_bind_driver_to_node(dm_root(), "usb_sandbox", "usb@2", node, - &dev); - ut_assertok(uclass_find_device_by_seq(UCLASS_USB, 2, &dev)); - - /* Test string property setting */ - - ut_assert(device_is_compatible(dev, "sandbox,usb")); - ofnode_write_string(node, "compatible", "gdsys,super-usb"); - ut_assert(device_is_compatible(dev, "gdsys,super-usb")); - ofnode_write_string(node, "compatible", "sandbox,usb"); - ut_assert(device_is_compatible(dev, "sandbox,usb")); - - /* Test setting generic properties */ - - /* Non-existent in DTB */ - ut_asserteq_64(FDT_ADDR_T_NONE, dev_read_addr(dev)); - /* reg = 0x42, size = 0x100 */ - ut_assertok(ofnode_write_prop(node, "reg", 8, - "\x00\x00\x00\x42\x00\x00\x01\x00")); - ut_asserteq(0x42, dev_read_addr(dev)); - - /* Test disabling devices */ - - device_remove(dev, DM_REMOVE_NORMAL); - device_unbind(dev); - - ut_assert(of_device_is_available(ofnode_to_np(node))); - ofnode_set_enabled(node, false); - ut_assert(!of_device_is_available(ofnode_to_np(node))); - - return 0; -} -DM_TEST(dm_test_fdt_livetree_writing, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); - static int dm_test_fdt_disable_enable_by_path(struct unit_test_state *uts) { ofnode node;

Update this test to use the livetree flag so that special check can be avoided. Also drop a few blank lines.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

test/dm/ofnode.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c index 0aeaaeb7f8c..ce96f9d1ee2 100644 --- a/test/dm/ofnode.c +++ b/test/dm/ofnode.c @@ -546,13 +546,7 @@ static int dm_test_ofnode_livetree_writing(struct unit_test_state *uts) struct udevice *dev; ofnode node;

- if (!of_live_active()) { - printf("Live tree not active; ignore test\n"); - return 0; - } - /* Test enabling devices */ - node = ofnode_path("/usb@2");

ut_assert(!of_device_is_available(ofnode_to_np(node))); @@ -564,7 +558,6 @@ static int dm_test_ofnode_livetree_writing(struct unit_test_state *uts) ut_assertok(uclass_find_device_by_seq(UCLASS_USB, 2, &dev));

/* Test string property setting */ - ut_assert(device_is_compatible(dev, "sandbox,usb")); ofnode_write_string(node, "compatible", "gdsys,super-usb"); ut_assert(device_is_compatible(dev, "gdsys,super-usb")); @@ -581,7 +574,6 @@ static int dm_test_ofnode_livetree_writing(struct unit_test_state *uts) ut_asserteq(0x42, dev_read_addr(dev));

/* Test disabling devices */ - device_remove(dev, DM_REMOVE_NORMAL); device_unbind(dev);

@@ -591,4 +583,5 @@ static int dm_test_ofnode_livetree_writing(struct unit_test_state *uts)

return 0; } -DM_TEST(dm_test_ofnode_livetree_writing, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); +DM_TEST(dm_test_ofnode_livetree_writing, + UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT | UT_TESTF_LIVE_TREE);

In generally it is not permitted to implement an ofnode function only for flat tree or live tree. Both must be supported. Also the code for live tree access should be in of_access.c rather than ofnode.c which is really just for holding the API-conversion code.

Update ofnode_write_prop() accordingly and fix the test so it can work with flat tree too.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

drivers/core/of_access.c | 43 +++++++++++++++++++++++++++++++++ drivers/core/ofnode.c | 51 ++++------------------------------------ include/dm/of_access.h | 12 ++++++++++ include/dm/ofnode.h | 18 ++++++++++++++ test/dm/ofnode.c | 14 +++++------ 5 files changed, 85 insertions(+), 53 deletions(-)

diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c index 0e5915a43e6..a52f5a6b18b 100644 --- a/drivers/core/of_access.c +++ b/drivers/core/of_access.c @@ -887,3 +887,46 @@ struct device_node *of_get_stdout(void) { return of_stdout; } + +int of_write_prop(struct device_node *np, const char *propname, int len, + const void *value) +{ + struct property *pp; + struct property *pp_last = NULL; + struct property *new; + + if (!np) + return -EINVAL; + + for (pp = np->properties; pp; pp = pp->next) { + if (strcmp(pp->name, propname) == 0) { + /* Property exists -> change value */ + pp->value = (void *)value; + pp->length = len; + return 0; + } + pp_last = pp; + } + + if (!pp_last) + return -ENOENT; + + /* Property does not exist -> append new property */ + new = malloc(sizeof(struct property)); + if (!new) + return -ENOMEM; + + new->name = strdup(propname); + if (!new->name) { + free(new); + return -ENOMEM; + } + + new->value = (void *)value; + new->length = len; + new->next = NULL; + + pp_last->next = new; + + return 0; +} diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 1c9542a3567..b7a55589a1c 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -1108,55 +1108,17 @@ ofnode ofnode_by_prop_value(ofnode from, const char *propname, int ofnode_write_prop(ofnode node, const char *propname, const void *value, int len) { - const struct device_node *np = ofnode_to_np(node); - struct property *pp; - struct property *pp_last = NULL; - struct property *new; - - if (!of_live_active()) - return -ENOSYS; - - if (!np) - return -EINVAL; - - for (pp = np->properties; pp; pp = pp->next) { - if (strcmp(pp->name, propname) == 0) { - /* Property exists -> change value */ - pp->value = (void *)value; - pp->length = len; - return 0; - } - pp_last = pp; - } - - if (!pp_last) - return -ENOENT; - - /* Property does not exist -> append new property */ - new = malloc(sizeof(struct property)); - if (!new) - return -ENOMEM; - - new->name = strdup(propname); - if (!new->name) { - free(new); - return -ENOMEM; - } - - new->value = (void *)value; - new->length = len; - new->next = NULL; - - pp_last->next = new; + if (of_live_active()) + return of_write_prop(ofnode_to_npw(node), propname, len, value); + else + return fdt_setprop((void *)gd->fdt_blob, ofnode_to_offset(node), + propname, value, len);

return 0; }

int ofnode_write_string(ofnode node, const char *propname, const char *value) { - if (!of_live_active()) - return -ENOSYS; - assert(ofnode_valid(node));

debug("%s: %s = %s", __func__, propname, value); @@ -1166,9 +1128,6 @@ int ofnode_write_string(ofnode node, const char *propname, const char *value)

int ofnode_set_enabled(ofnode node, bool value) { - if (!of_live_active()) - return -ENOSYS; - assert(ofnode_valid(node));

if (value) diff --git a/include/dm/of_access.h b/include/dm/of_access.h index 078f2ea06cd..5b7821d0a1b 100644 --- a/include/dm/of_access.h +++ b/include/dm/of_access.h @@ -519,4 +519,16 @@ int of_alias_get_highest_id(const char *stem); */ struct device_node *of_get_stdout(void);

+/** + * of_write_prop() - Write a property to the device tree + * + * @np: device node to which the property value is to be written + * @propname: name of the property to write + * @value: value of the property + * @len: length of the property in bytes + * Returns: 0 if OK, -ve on error + */ +int of_write_prop(struct device_node *np, const char *propname, int len, + const void *value); + #endif diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index 071a9d63f67..16c8890b097 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -44,6 +44,24 @@ static inline const struct device_node *ofnode_to_np(ofnode node) return node.np; }

+/** + * ofnode_to_npw() - convert an ofnode to a writeable live DT node pointer + * + * This cannot be called if the reference contains an offset. + * + * @node: Reference containing struct device_node * (possibly invalid) + * Return: pointer to device node (can be NULL) + */ +static inline struct device_node *ofnode_to_npw(ofnode node) +{ +#ifdef OF_CHECKS + if (!of_live_active()) + return NULL; +#endif + /* Drop constant */ + return (struct device_node *)node.np; +} + /** * ofnode_to_offset() - convert an ofnode to a flat DT offset * diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c index ce96f9d1ee2..bd598d23e44 100644 --- a/test/dm/ofnode.c +++ b/test/dm/ofnode.c @@ -549,9 +549,9 @@ static int dm_test_ofnode_livetree_writing(struct unit_test_state *uts) /* Test enabling devices */ node = ofnode_path("/usb@2");

- ut_assert(!of_device_is_available(ofnode_to_np(node))); - ofnode_set_enabled(node, true); - ut_assert(of_device_is_available(ofnode_to_np(node))); + ut_assert(!ofnode_is_enabled(node)); + ut_assertok(ofnode_set_enabled(node, true)); + ut_asserteq(true, ofnode_is_enabled(node));

device_bind_driver_to_node(dm_root(), "usb_sandbox", "usb@2", node, &dev); @@ -577,11 +577,11 @@ static int dm_test_ofnode_livetree_writing(struct unit_test_state *uts) device_remove(dev, DM_REMOVE_NORMAL); device_unbind(dev);

- ut_assert(of_device_is_available(ofnode_to_np(node))); - ofnode_set_enabled(node, false); - ut_assert(!of_device_is_available(ofnode_to_np(node))); + ut_assert(ofnode_is_enabled(node)); + ut_assertok(ofnode_set_enabled(node, false)); + ut_assert(!ofnode_is_enabled(node));

return 0; } DM_TEST(dm_test_ofnode_livetree_writing, - UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT | UT_TESTF_LIVE_TREE); + UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT | UT_TESTF_LIVE_OR_FLAT);

Update the 'read' command to work correctly with sandbox.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

cmd/read.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmd/read.c b/cmd/read.c index 99c7e3854e1..fecfadaa1fa 100644 --- a/cmd/read.c +++ b/cmd/read.c @@ -10,6 +10,7 @@

#include <common.h> #include <command.h> +#include <mapmem.h> #include <part.h>

int do_read(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -45,7 +46,7 @@ int do_read(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; }

- addr = (void *)hextoul(argv[3], NULL); + addr = map_sysmem(hextoul(argv[3], NULL), 0); blk = hextoul(argv[4], NULL); cnt = hextoul(argv[5], NULL);

Correct the comment at the top of this file.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

test/boot/bootmeth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/boot/bootmeth.c b/test/boot/bootmeth.c index 07776c5368d..81421f550b5 100644 --- a/test/boot/bootmeth.c +++ b/test/boot/bootmeth.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0+ /* - * Test for bootdev functions. All start with 'bootdev' + * Test for bootdev functions. All start with 'bootmeth' * * Copyright 2021 Google LLC * Written by Simon Glass sjg@chromium.org

Some bootmeths can provide information about what is available to boot. For example, VBE simple provides access to the firmware state.

Add a new method for this, along with a sandbox test.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

boot/bootmeth-uclass.c | 10 ++++++++++ boot/bootmeth_distro.c | 14 ++++++++++++++ include/bootmeth.h | 38 +++++++++++++++++++++++++++++++++++++- test/boot/bootmeth.c | 18 ++++++++++++++++++ 4 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c index b8ba4eca7ab..1e276c0f26b 100644 --- a/boot/bootmeth-uclass.c +++ b/boot/bootmeth-uclass.c @@ -20,6 +20,16 @@

DECLARE_GLOBAL_DATA_PTR;

+int bootmeth_get_state_desc(struct udevice *dev, char *buf, int maxsize) +{ + const struct bootmeth_ops *ops = bootmeth_get_ops(dev); + + if (!ops->get_state_desc) + return -ENOSYS; + + return ops->get_state_desc(dev, buf, maxsize); +} + int bootmeth_check(struct udevice *dev, struct bootflow_iter *iter) { const struct bootmeth_ops *ops = bootmeth_get_ops(dev); diff --git a/boot/bootmeth_distro.c b/boot/bootmeth_distro.c index 2b41e654ade..fea09b2c2fb 100644 --- a/boot/bootmeth_distro.c +++ b/boot/bootmeth_distro.c @@ -22,6 +22,19 @@ #include <mmc.h> #include <pxe_utils.h>

+static int distro_get_state_desc(struct udevice *dev, char *buf, int maxsize) +{ + if (IS_ENABLED(CONFIG_SANDBOX)) { + int len; + + len = snprintf(buf, maxsize, "OK"); + + return len + 1 < maxsize ? 0 : -ENOSPC; + } + + return 0; +} + static int disto_getfile(struct pxe_context *ctx, const char *file_path, char *file_addr, ulong *sizep) { @@ -123,6 +136,7 @@ static int distro_bootmeth_bind(struct udevice *dev) }

static struct bootmeth_ops distro_bootmeth_ops = { + .get_state_desc = distro_get_state_desc, .check = distro_check, .read_bootflow = distro_read_bootflow, .read_file = bootmeth_common_read_file, diff --git a/include/bootmeth.h b/include/bootmeth.h index 484e503e338..4967031a0a8 100644 --- a/include/bootmeth.h +++ b/include/bootmeth.h @@ -24,7 +24,25 @@ struct bootmeth_uc_plat { /** struct bootmeth_ops - Operations for boot methods */ struct bootmeth_ops { /** - * check_supported() - check if a bootmeth supports this bootflow + * get_state_desc() - get detailed state information + * + * Prodecues a textual description of the state of the bootmeth. This + * can include newline characters if it extends to multiple lines. It + * must be a nul-terminated string. + * + * This may involve reading state from the system, e.g. some data in + * the firmware area. + * + * @dev: Bootmethod device to check + * @buf: Buffer to place the info in (terminator must fit) + * @maxsize: Size of buffer + * Returns: 0 if OK, -ENOSPC is buffer is too small, other -ve error if + * something else went wrong + */ + int (*get_state_desc)(struct udevice *dev, char *buf, int maxsize); + + /** + * check_supported() - check if a bootmeth supports this bootdev * * This is optional. If not provided, the bootdev is assumed to be * supported @@ -91,6 +109,24 @@ struct bootmeth_ops {

#define bootmeth_get_ops(dev) ((struct bootmeth_ops *)(dev)->driver->ops)

+/** + * bootmeth_get_state_desc() - get detailed state information + * + * Prodecues a textual description of the state of the bootmeth. This + * can include newline characters if it extends to multiple lines. It + * must be a nul-terminated string. + * + * This may involve reading state from the system, e.g. some data in + * the firmware area. + * + * @dev: Bootmethod device to check + * @buf: Buffer to place the info in (terminator must fit) + * @maxsize: Size of buffer + * Returns: 0 if OK, -ENOSPC is buffer is too small, other -ve error if + * something else went wrong + */ +int bootmeth_get_state_desc(struct udevice *dev, char *buf, int maxsize); + /** * bootmeth_check() - check if a bootmeth supports this bootflow * diff --git a/test/boot/bootmeth.c b/test/boot/bootmeth.c index 81421f550b5..5d2e87b1c95 100644 --- a/test/boot/bootmeth.c +++ b/test/boot/bootmeth.c @@ -7,7 +7,9 @@ */

#include <common.h> +#include <bootmeth.h> #include <bootstd.h> +#include <dm.h> #include <test/suites.h> #include <test/ut.h> #include "bootstd_common.h" @@ -120,3 +122,19 @@ static int bootmeth_env(struct unit_test_state *uts) return 0; } BOOTSTD_TEST(bootmeth_env, UT_TESTF_DM | UT_TESTF_SCAN_FDT); + +/* Check the get_state_desc() method */ +static int bootmeth_state(struct unit_test_state *uts) +{ + struct udevice *dev; + char buf[50]; + + ut_assertok(uclass_first_device(UCLASS_BOOTMETH, &dev)); + ut_assertnonnull(dev); + + ut_assertok(bootmeth_get_state_desc(dev, buf, sizeof(buf))); + ut_asserteq_str("OK", buf); + + return 0; +} +BOOTSTD_TEST(bootmeth_state, UT_TESTF_DM | UT_TESTF_SCAN_FDT);

The current way of handling things like EFI bootmgr is a bit odd, since that bootmeth handles selection of the bootdev itself. VBE needs to work the same way, so we should support it properly.

Add a flag that indicates that the bootmeth is global, rather than being invoked on each bootdev. Provide a helper to read a bootflow from the bootmeth.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

boot/Kconfig | 7 +++++++ boot/bootmeth-uclass.c | 14 ++++++++++++++ boot/bootmeth_efi_mgr.c | 3 ++- cmd/bootmeth.c | 4 +++- include/bootmeth.h | 23 +++++++++++++++++++++++ lib/efi_loader/Kconfig | 1 + 6 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/boot/Kconfig b/boot/Kconfig index 7a9ee483bc3..b8adc64b9a7 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -343,6 +343,13 @@ config BOOTSTD_BOOTCOMMAND standard boot does not support all of the features of distro boot yet.

+config BOOTMETH_GLOBAL + bool + help + Add support for global bootmeths. This feature is used by VBE and + EFI bootmgr, since they take full control over which bootdevs are + selected to boot. + config BOOTMETH_DISTRO bool "Bootdev support for distro boot" depends on CMD_PXE diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c index 1e276c0f26b..88bbb32c47f 100644 --- a/boot/bootmeth-uclass.c +++ b/boot/bootmeth-uclass.c @@ -71,6 +71,20 @@ int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow, return ops->read_file(dev, bflow, file_path, addr, sizep); }

+int bootmeth_get_bootflow(struct udevice *dev, struct bootflow *bflow) +{ + const struct bootmeth_ops *ops = bootmeth_get_ops(dev); + + if (!ops->read_bootflow) + return -ENOSYS; + memset(bflow, '\0', sizeof(*bflow)); + bflow->dev = NULL; + bflow->method = dev; + bflow->state = BOOTFLOWST_BASE; + + return ops->read_bootflow(dev, bflow); +} + /** * bootmeth_setup_iter_order() - Set up the ordering of bootmeths to scan * diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c index a6914466db7..08d9328af4e 100644 --- a/boot/bootmeth_efi_mgr.c +++ b/boot/bootmeth_efi_mgr.c @@ -61,6 +61,7 @@ static int bootmeth_efi_mgr_bind(struct udevice *dev) struct bootmeth_uc_plat *plat = dev_get_uclass_plat(dev);

plat->desc = "EFI bootmgr flow"; + plat->flags = BOOTMETHF_GLOBAL;

return 0; } @@ -77,7 +78,7 @@ static const struct udevice_id efi_mgr_bootmeth_ids[] = { { } };

-U_BOOT_DRIVER(bootmeth_zefi_mgr) = { +U_BOOT_DRIVER(bootmeth_efi_mgr) = { .name = "bootmeth_efi_mgr", .id = UCLASS_BOOTMETH, .of_match = efi_mgr_bootmeth_ids, diff --git a/cmd/bootmeth.c b/cmd/bootmeth.c index c9a27fe8ac6..9fbcccdba7e 100644 --- a/cmd/bootmeth.c +++ b/cmd/bootmeth.c @@ -69,7 +69,9 @@ static int do_bootmeth_list(struct cmd_tbl *cmdtp, int flag, int argc, } }

- if (order == -1) + if (ucp->flags & BOOTMETHF_GLOBAL) + printf("%5s", "glob"); + else if (order == -1) printf("%5s", "-"); else printf("%5x", order); diff --git a/include/bootmeth.h b/include/bootmeth.h index 4967031a0a8..c93367b0995 100644 --- a/include/bootmeth.h +++ b/include/bootmeth.h @@ -12,13 +12,24 @@ struct bootflow; struct bootflow_iter; struct udevice;

+/** + * enum bootmeth_flags - Flags for bootmeths + * + * @BOOTMETHF_GLOBAL: bootmeth handles bootdev selection automatically + */ +enum bootmeth_flags { + BOOTMETHF_GLOBAL = BIT(0), +}; + /** * struct bootmeth_uc_plat - information the uclass keeps about each bootmeth * * @desc: A long description of the bootmeth + * @flags: Flags for this bootmeth (enum bootmeth_flags) */ struct bootmeth_uc_plat { const char *desc; + int flags; };

/** struct bootmeth_ops - Operations for boot methods */ @@ -267,4 +278,16 @@ int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align); int bootmeth_common_read_file(struct udevice *dev, struct bootflow *bflow, const char *file_path, ulong addr, ulong *sizep);

+/** + * bootmeth_get_bootflow() - Get a bootflow from a global bootmeth + * + * Check the bootmeth for a bootflow which can be used. In this case the + * bootmeth handles all bootdev selection, etc. + * + * @dev: bootmeth device to read from + * @bflow: Bootflow information + * @return 0 on success, -ve if a bootflow could not be found or had an error + */ +int bootmeth_get_bootflow(struct udevice *dev, struct bootflow *bflow); + #endif diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index e3f2402d0e8..e4c51ed231d 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -37,6 +37,7 @@ if EFI_LOADER config CMD_BOOTEFI_BOOTMGR bool "UEFI Boot Manager" default y + select BOOTMETH_GLOBAL help Select this option if you want to select the UEFI binary to be booted via UEFI variables Boot####, BootOrder, and BootNext. This enables the

With global bootmeths we want to scan without a bootdev. Update the logic to allow this.

Change the bootflow command to show the bootdev only when valid.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

boot/bootdev-uclass.c | 7 +++++-- boot/bootflow.c | 3 ++- cmd/bootflow.c | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index 5683006c73c..13ac69eb392 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -36,7 +36,6 @@ enum {

int bootdev_add_bootflow(struct bootflow *bflow) { - struct bootdev_uc_plat *ucp = dev_get_uclass_plat(bflow->dev); struct bootstd_priv *std; struct bootflow *new; int ret; @@ -52,7 +51,11 @@ int bootdev_add_bootflow(struct bootflow *bflow) memcpy(new, bflow, sizeof(*bflow));

list_add_tail(&new->glob_node, &std->glob_head); - list_add_tail(&new->bm_node, &ucp->bootflow_head); + if (bflow->dev) { + struct bootdev_uc_plat *ucp = dev_get_uclass_plat(bflow->dev); + + list_add_tail(&new->bm_node, &ucp->bootflow_head); + }

return 0; } diff --git a/boot/bootflow.c b/boot/bootflow.c index 24ba3c34660..37bccb823a1 100644 --- a/boot/bootflow.c +++ b/boot/bootflow.c @@ -307,7 +307,8 @@ void bootflow_free(struct bootflow *bflow)

void bootflow_remove(struct bootflow *bflow) { - list_del(&bflow->bm_node); + if (bflow->dev) + list_del(&bflow->bm_node); list_del(&bflow->glob_node);

bootflow_free(bflow); diff --git a/cmd/bootflow.c b/cmd/bootflow.c index af4b9c37323..47899245ee8 100644 --- a/cmd/bootflow.c +++ b/cmd/bootflow.c @@ -69,8 +69,8 @@ static void show_bootflow(int index, struct bootflow *bflow, bool errors) { printf("%3x %-11s %-6s %-9.9s %4x %-25.25s %s\n", index, bflow->method->name, bootflow_state_get_name(bflow->state), - dev_get_uclass_name(dev_get_parent(bflow->dev)), bflow->part, - bflow->name, bflow->fname); + bflow->dev ? dev_get_uclass_name(dev_get_parent(bflow->dev)) : + "(none)", bflow->part, bflow->name, bflow->fname); if (errors) report_bootflow_err(bflow, bflow->err); }

Add support for handling this concept in bootflows. Update the 'bootflow' command to allow only the normal bootmeths to be used. This alllows skipping EFI bootmgr and VBE, for example.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

boot/bootflow.c | 58 +++++++++++++++++++++++++++++++++++++++------- cmd/bootflow.c | 8 +++++-- include/bootflow.h | 16 ++++++++++--- 3 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/boot/bootflow.c b/boot/bootflow.c index 37bccb823a1..08ea0336324 100644 --- a/boot/bootflow.c +++ b/boot/bootflow.c @@ -86,6 +86,7 @@ int bootflow_next_glob(struct bootflow **bflowp) void bootflow_iter_init(struct bootflow_iter *iter, int flags) { memset(iter, '\0', sizeof(*iter)); + iter->first_glob_method = -1; iter->flags = flags; }

@@ -115,11 +116,17 @@ int bootflow_iter_drop_bootmeth(struct bootflow_iter *iter, static void bootflow_iter_set_dev(struct bootflow_iter *iter, struct udevice *dev) { + struct bootmeth_uc_plat *ucp = dev_get_uclass_plat(iter->method); + iter->dev = dev; if ((iter->flags & (BOOTFLOWF_SHOW | BOOTFLOWF_SINGLE_DEV)) == BOOTFLOWF_SHOW) { if (dev) printf("Scanning bootdev '%s':\n", dev->name); + else if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && + ucp->flags & BOOTMETHF_GLOBAL) + printf("Scanning global bootmeth '%s':\n", + iter->method->name); else printf("No more bootdevs\n"); } @@ -133,8 +140,12 @@ static void bootflow_iter_set_dev(struct bootflow_iter *iter, static int iter_incr(struct bootflow_iter *iter) { struct udevice *dev; + bool inc_dev = true; + bool global; int ret;

+ global = iter->doing_global; + if (iter->err == BF_NO_MORE_DEVICES) return BF_NO_MORE_DEVICES;

@@ -144,6 +155,21 @@ static int iter_incr(struct bootflow_iter *iter) iter->method = iter->method_order[iter->cur_method]; return 0; } + + /* + * If we have finished scanning the global bootmeths, start the + * normal bootdev scan + */ + if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && global) { + iter->num_methods = iter->first_glob_method; + iter->doing_global = false; + + /* + * Don't move to the next dev as we haven't tried this + * one yet! + */ + inc_dev = false; + } }

/* No more bootmeths; start at the first one, and... */ @@ -169,14 +195,18 @@ static int iter_incr(struct bootflow_iter *iter) /* ...select next bootdev */ if (iter->flags & BOOTFLOWF_SINGLE_DEV) { ret = -ENOENT; - } else if (++iter->cur_dev == iter->num_devs) { - ret = -ENOENT; - bootflow_iter_set_dev(iter, NULL); } else { - dev = iter->dev_order[iter->cur_dev]; - ret = device_probe(dev); - if (!log_msg_ret("probe", ret)) - bootflow_iter_set_dev(iter, dev); + if (inc_dev) + iter->cur_dev++; + if (iter->cur_dev == iter->num_devs) { + ret = -ENOENT; + bootflow_iter_set_dev(iter, NULL); + } else { + dev = iter->dev_order[iter->cur_dev]; + ret = device_probe(dev); + if (!log_msg_ret("probe", ret)) + bootflow_iter_set_dev(iter, dev); + } }

/* if there are no more bootdevs, give up */ @@ -199,6 +229,15 @@ static int bootflow_check(struct bootflow_iter *iter, struct bootflow *bflow) struct udevice *dev; int ret;

+ if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && iter->doing_global) { + bootflow_iter_set_dev(iter, NULL); + ret = bootmeth_get_bootflow(iter->method, bflow); + if (ret) + return log_msg_ret("glob", ret); + + return 0; + } + dev = iter->dev; ret = bootdev_get_bootflow(dev, iter, bflow);

@@ -231,12 +270,13 @@ int bootflow_scan_bootdev(struct udevice *dev, struct bootflow_iter *iter, { int ret;

+ if (dev) + flags |= BOOTFLOWF_SKIP_GLOBAL; bootflow_iter_init(iter, flags);

ret = bootdev_setup_iter_order(iter, &dev); if (ret) return log_msg_ret("obdev", -ENODEV); - bootflow_iter_set_dev(iter, dev);

ret = bootmeth_setup_iter_order(iter); if (ret) @@ -244,6 +284,8 @@ int bootflow_scan_bootdev(struct udevice *dev, struct bootflow_iter *iter,

/* Find the first bootmeth (there must be at least one!) */ iter->method = iter->method_order[iter->cur_method]; + if (!IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) || !iter->doing_global) + bootflow_iter_set_dev(iter, dev);

ret = bootflow_check(iter, bflow); if (ret) { diff --git a/cmd/bootflow.c b/cmd/bootflow.c index 47899245ee8..313103d2775 100644 --- a/cmd/bootflow.c +++ b/cmd/bootflow.c @@ -95,7 +95,8 @@ static int do_bootflow_scan(struct cmd_tbl *cmdtp, int flag, int argc, struct bootflow_iter iter; struct udevice *dev; struct bootflow bflow; - bool all = false, boot = false, errors = false, list = false; + bool all = false, boot = false, errors = false, no_global = false; + bool list = false; int num_valid = 0; bool has_args; int ret, i; @@ -112,6 +113,7 @@ static int do_bootflow_scan(struct cmd_tbl *cmdtp, int flag, int argc, all = strchr(argv[1], 'a'); boot = strchr(argv[1], 'b'); errors = strchr(argv[1], 'e'); + no_global = strchr(argv[1], 'G'); list = strchr(argv[1], 'l'); argc--; argv++; @@ -137,6 +139,8 @@ static int do_bootflow_scan(struct cmd_tbl *cmdtp, int flag, int argc, flags |= BOOTFLOWF_SHOW; if (all) flags |= BOOTFLOWF_ALL; + if (no_global) + flags |= BOOTFLOWF_SKIP_GLOBAL;

/* * If we have a device, just scan for bootflows attached to that device @@ -383,7 +387,7 @@ static int do_bootflow_boot(struct cmd_tbl *cmdtp, int flag, int argc, #ifdef CONFIG_SYS_LONGHELP static char bootflow_help_text[] = #ifdef CONFIG_CMD_BOOTFLOW_FULL - "scan [-abel] [bdev] - scan for valid bootflows (-l list, -a all, -e errors, -b boot)\n" + "scan [-abeGl] [bdev] - scan for valid bootflows (-l list, -a all, -e errors, -b boot, -G no global)\n" "bootflow list [-e] - list scanned bootflows (-e errors)\n" "bootflow select [<num>|<name>] - select a bootflow\n" "bootflow info [-d] - show info on current bootflow (-d dump bootflow)\n" diff --git a/include/bootflow.h b/include/bootflow.h index 4fa482a6784..6aa3d1fff8d 100644 --- a/include/bootflow.h +++ b/include/bootflow.h @@ -77,12 +77,14 @@ struct bootflow { * @BOOTFLOWF_SHOW: Show each bootdev before scanning it * @BOOTFLOWF_ALL: Return bootflows with errors as well * @BOOTFLOWF_SINGLE_DEV: Just scan one bootmeth + * @BOOTFLOWF_SKIP_GLOBAL: Don't scan global bootmeths */ enum bootflow_flags_t { BOOTFLOWF_FIXED = 1 << 0, BOOTFLOWF_SHOW = 1 << 1, BOOTFLOWF_ALL = 1 << 2, BOOTFLOWF_SINGLE_DEV = 1 << 3, + BOOTFLOWF_SKIP_GLOBAL = 1 << 4, };

/** @@ -102,8 +104,10 @@ enum bootflow_flags_t { * updated to a larger value, no less than the number of available partitions. * This ensures that iteration works through all partitions on the bootdev. * - * @flags: Flags to use (see enum bootflow_flags_t) - * @dev: Current bootdev + * @flags: Flags to use (see enum bootflow_flags_t). If BOOTFLOWF_GLOBAL_FIRST is + * enabled then the global bootmeths are being scanned, otherwise we have + * moved onto the bootdevs + * @dev: Current bootdev, NULL if none * @part: Current partition number (0 for whole device) * @method: Current bootmeth * @max_part: Maximum hardware partition number in @dev, 0 if there is no @@ -117,7 +121,11 @@ enum bootflow_flags_t { * with the first one on the list * @num_methods: Number of bootmeth devices in @method_order * @cur_method: Current method number, an index into @method_order - * @method_order: List of bootmeth devices to use, in order + * @first_glob_method: First global method, if any, else -1 + * @method_order: List of bootmeth devices to use, in order. The normal methods + * appear first, then the global ones, if any + * @doing_global: true if we are iterating through the global bootmeths (which + * happens before the normal ones) */ struct bootflow_iter { int flags; @@ -131,7 +139,9 @@ struct bootflow_iter { struct udevice **dev_order; int num_methods; int cur_method; + int first_glob_method; struct udevice **method_order; + bool doing_global; };

/**

Typically we want to find and use global bootmeths first, since they have the best idea of how the system should boot. We then use normal bootmeths as a fallback.

Add the logic for this, putting global bootmeths at the end of the ordering. We can then easily scan the global bootmeths first, then drop them from the list for subsequent bootdev-centric scans.

This changes the ordering of global bootmeths, so update the bootflow_system() accordingly.

Drop the comment from bootmeth_setup_iter_order() since this is an exported function and it should be in the header file.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

boot/bootflow.c | 2 +- boot/bootmeth-uclass.c | 79 ++++++++++++++++++++++++++++++------------ include/bootmeth.h | 4 ++- test/boot/bootflow.c | 5 +-- 4 files changed, 64 insertions(+), 26 deletions(-)

diff --git a/boot/bootflow.c b/boot/bootflow.c index 08ea0336324..5d94a27ff84 100644 --- a/boot/bootflow.c +++ b/boot/bootflow.c @@ -278,7 +278,7 @@ int bootflow_scan_bootdev(struct udevice *dev, struct bootflow_iter *iter, if (ret) return log_msg_ret("obdev", -ENODEV);

- ret = bootmeth_setup_iter_order(iter); + ret = bootmeth_setup_iter_order(iter, !(flags & BOOTFLOWF_SKIP_GLOBAL)); if (ret) return log_msg_ret("obmeth", -ENODEV);

diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c index 88bbb32c47f..2d7652edeab 100644 --- a/boot/bootmeth-uclass.c +++ b/boot/bootmeth-uclass.c @@ -85,18 +85,7 @@ int bootmeth_get_bootflow(struct udevice *dev, struct bootflow *bflow) return ops->read_bootflow(dev, bflow); }

-/** - * bootmeth_setup_iter_order() - Set up the ordering of bootmeths to scan - * - * This sets up the ordering information in @iter, based on the selected - * ordering of the bootmethds in bootstd_priv->bootmeth_order. If there is no - * ordering there, then all bootmethods are added - * - * @iter: Iterator to update with the order - * Return: 0 if OK, -ENOENT if no bootdevs, -ENOMEM if out of memory, other -ve - * on other error - */ -int bootmeth_setup_iter_order(struct bootflow_iter *iter) +int bootmeth_setup_iter_order(struct bootflow_iter *iter, bool include_global) { struct bootstd_priv *std; struct udevice **order; @@ -119,31 +108,77 @@ int bootmeth_setup_iter_order(struct bootflow_iter *iter)

/* If we have an ordering, copy it */ if (IS_ENABLED(CONFIG_BOOTSTD_FULL) && std->bootmeth_count) { + int i; + + /* + * We don't support skipping global bootmeths. Instead, the user + * should omit them from the ordering + */ + if (!include_global) + return log_msg_ret("glob", -EPERM); memcpy(order, std->bootmeth_order, count * sizeof(struct bootmeth *)); + + if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL)) { + for (i = 0; i < count; i++) { + struct udevice *dev = order[i]; + struct bootmeth_uc_plat *ucp; + bool is_global; + + ucp = dev_get_uclass_plat(dev); + is_global = ucp->flags & + BOOTMETHF_GLOBAL; + if (is_global) { + iter->first_glob_method = i; + break; + } + } + } } else { struct udevice *dev; - int i, upto; + int i, upto, pass;

/* - * Get a list of bootmethods, in seq order (i.e. using aliases). - * There may be gaps so try to count up high enough to find them - * all. + * Do two passes, one to find the normal bootmeths and another + * to find the global ones, if required, The global ones go at + * the end. */ - for (i = 0, upto = 0; upto < count && i < 20 + count * 2; i++) { - ret = uclass_get_device_by_seq(UCLASS_BOOTMETH, i, - &dev); - if (!ret) - order[upto++] = dev; + for (pass = 0, upto = 0; pass < 1 + include_global; pass++) { + if (pass) + iter->first_glob_method = upto; + /* + * Get a list of bootmethods, in seq order (i.e. using + * aliases). There may be gaps so try to count up high + * enough to find them all. + */ + for (i = 0; upto < count && i < 20 + count * 2; i++) { + struct bootmeth_uc_plat *ucp; + bool is_global; + + ret = uclass_get_device_by_seq(UCLASS_BOOTMETH, + i, &dev); + if (ret) + continue; + ucp = dev_get_uclass_plat(dev); + is_global = + IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && + (ucp->flags & BOOTMETHF_GLOBAL); + if (pass ? is_global : !is_global) + order[upto++] = dev; + } } count = upto; } if (!count) return log_msg_ret("count2", -ENOENT);

+ if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) && include_global && + iter->first_glob_method != -1 && iter->first_glob_method != count) { + iter->cur_method = iter->first_glob_method; + iter->doing_global = true; + } iter->method_order = order; iter->num_methods = count; - iter->cur_method = 0;

return 0; } diff --git a/include/bootmeth.h b/include/bootmeth.h index c93367b0995..50ded055f3f 100644 --- a/include/bootmeth.h +++ b/include/bootmeth.h @@ -209,10 +209,12 @@ int bootmeth_boot(struct udevice *dev, struct bootflow *bflow); * ordering there, then all bootmethods are added * * @iter: Iterator to update with the order + * @include_global: true to add the global bootmeths, in which case they appear + * first * Return: 0 if OK, -ENOENT if no bootdevs, -ENOMEM if out of memory, other -ve * on other error */ -int bootmeth_setup_iter_order(struct bootflow_iter *iter); +int bootmeth_setup_iter_order(struct bootflow_iter *iter, bool include_global);

/** * bootmeth_set_order() - Set the bootmeth order diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c index 22eef40c0e3..07b0517718e 100644 --- a/test/boot/bootflow.c +++ b/test/boot/bootflow.c @@ -343,8 +343,9 @@ static int bootflow_system(struct unit_test_state *uts) bootstd_clear_glob(); console_record_reset_enable(); ut_assertok(run_command("bootflow scan -l", 0)); - ut_assert_skip_to_line(" 1 bootmgr ready bootstd 0 <NULL> <NULL>"); - ut_assert_nextline("No more bootdevs"); + ut_assert_skip_to_line( + " 0 bootmgr ready (none) 0 <NULL> <NULL>"); + ut_assert_skip_to_line("No more bootdevs"); ut_assert_skip_to_line("(2 bootflows, 2 valid)"); ut_assert_console_end();

This was a work-around for the fact that global bootmeths such as EFI bootmgr and VBE don't use a particular bootdev, or at least select it themselves so that we don't need to scan all bootdevs when using that bootmeth.

Drop the system bootdev entirely.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

boot/Makefile | 2 +- boot/bootstd-uclass.c | 6 ---- boot/system_bootdev.c | 66 ------------------------------------------- test/boot/bootflow.c | 7 +---- 4 files changed, 2 insertions(+), 79 deletions(-) delete mode 100644 boot/system_bootdev.c

diff --git a/boot/Makefile b/boot/Makefile index a70674259c1..bab02be53fa 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -19,7 +19,7 @@ obj-y += image.o image-board.o obj-$(CONFIG_ANDROID_AB) += android_ab.o obj-$(CONFIG_ANDROID_BOOT_IMAGE) += image-android.o image-android-dt.o

-obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootdev-uclass.o system_bootdev.o +obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootdev-uclass.o obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootflow.o obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootmeth-uclass.o obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootstd-uclass.o diff --git a/boot/bootstd-uclass.c b/boot/bootstd-uclass.c index 5107b6d4c7f..565c22a36e7 100644 --- a/boot/bootstd-uclass.c +++ b/boot/bootstd-uclass.c @@ -145,12 +145,6 @@ int dm_scan_other(bool pre_reloc_only) } }

- /* Create the system bootdev too */ - ret = device_bind_driver(bootstd, "system_bootdev", "system-bootdev", - &dev); - if (ret) - return log_msg_ret("sys", ret); - return 0; }

diff --git a/boot/system_bootdev.c b/boot/system_bootdev.c deleted file mode 100644 index 432d2034780..00000000000 --- a/boot/system_bootdev.c +++ /dev/null @@ -1,66 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * Bootdevice for system, used for bootmeths not tied to any partition device - * - * Copyright 2021 Google LLC - * Written by Simon Glass sjg@chromium.org - */ - -#define LOG_CATEGORY UCLASS_BOOTSTD - -#include <common.h> -#include <bootdev.h> -#include <bootflow.h> -#include <bootmeth.h> -#include <command.h> -#include <distro.h> -#include <dm.h> -#include <log.h> -#include <net.h> - -static int system_get_bootflow(struct udevice *dev, struct bootflow_iter *iter, - struct bootflow *bflow) -{ - int ret; - - /* Must be an bootstd device */ - ret = bootflow_iter_uses_system(iter); - if (ret) - return log_msg_ret("net", ret); - - ret = bootmeth_check(bflow->method, iter); - if (ret) - return log_msg_ret("check", ret); - - ret = bootmeth_read_bootflow(bflow->method, bflow); - if (ret) - return log_msg_ret("method", ret); - - return 0; -} - -static int system_bootdev_bind(struct udevice *dev) -{ - struct bootdev_uc_plat *ucp = dev_get_uclass_plat(dev); - - ucp->prio = BOOTDEVP_6_SYSTEM; - - return 0; -} - -struct bootdev_ops system_bootdev_ops = { - .get_bootflow = system_get_bootflow, -}; - -static const struct udevice_id system_bootdev_ids[] = { - { .compatible = "u-boot,bootdev-system" }, - { } -}; - -U_BOOT_DRIVER(system_bootdev) = { - .name = "system_bootdev", - .id = UCLASS_BOOTDEV, - .ops = &system_bootdev_ops, - .bind = system_bootdev_bind, - .of_match = system_bootdev_ids, -}; diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c index b2f77972d1f..39c2c33dead 100644 --- a/test/boot/bootflow.c +++ b/test/boot/bootflow.c @@ -12,7 +12,6 @@ #include <bootstd.h> #include <dm.h> #include <asm/test.h> -#include <dm/device-internal.h> #include <dm/lists.h> #include <test/suites.h> #include <test/ut.h> @@ -337,7 +336,7 @@ static int bootflow_system(struct unit_test_state *uts) ut_assert_skip_to_line( " 0 efi_mgr ready (none) 0 <NULL> <NULL>"); ut_assert_skip_to_line("No more bootdevs"); - ut_assert_skip_to_line("(6 bootflows, 6 valid)"); + ut_assert_skip_to_line("(5 bootflows, 5 valid)"); ut_assert_console_end();

return 0; @@ -358,10 +357,6 @@ static int bootflow_iter_disable(struct unit_test_state *uts) ut_assertok(device_bind_driver(bootstd, "bootmeth_sandbox", "sandbox", &dev));

- /* Add the system bootdev that it uses */ - ut_assertok(device_bind_driver(bootstd, "system_bootdev", - "system-bootdev", &dev)); - ut_assertok(bootstd_test_drop_bootdev_order(uts));

bootstd_clear_glob();

At present there is a confusing array of functions that handle the device tree fix-ups needed for booting an OS. We should be able to switch to using events to clean this up.

As a first step, create a new event type and call it from the standard place.

Note that this event uses the ofnode interface only, since this can support live tree which is more efficient when making lots of updates.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

boot/image-fdt.c | 11 +++++++++++ common/event.c | 3 +++ include/event.h | 14 ++++++++++++++ test/py/tests/test_event_dump.py | 1 + 4 files changed, 29 insertions(+)

diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 9db2cee9942..5e5b24674d3 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -21,6 +21,7 @@ #include <linux/libfdt.h> #include <mapmem.h> #include <asm/io.h> +#include <dm/ofnode.h> #include <tee/optee.h>

#ifndef CONFIG_SYS_FDT_PAD @@ -668,6 +669,16 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, goto err; } } + if (CONFIG_IS_ENABLED(EVENT)) { + struct event_ft_fixup fixup; + + fixup.tree = oftree_default(); + ret = event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup)); + if (ret) { + printf("ERROR: fdt fixup event failed: %d\n", ret); + goto err; + } + }

/* Delete the old LMB reservation */ if (lmb) diff --git a/common/event.c b/common/event.c index af1ed4121d8..3e345509783 100644 --- a/common/event.c +++ b/common/event.c @@ -35,6 +35,9 @@ const char *const type_name[] = {

/* init hooks */ "misc_init_f", + + /* fdt hooks */ + "ft_fixup", };

_Static_assert(ARRAY_SIZE(type_name) == EVT_COUNT, "event type_name size"); diff --git a/include/event.h b/include/event.h index fb0734ed4e1..e8f2f55c63d 100644 --- a/include/event.h +++ b/include/event.h @@ -10,6 +10,8 @@ #ifndef __event_h #define __event_h

+#include <dm/ofnode_decl.h> + /** * enum event_t - Types of events supported by U-Boot * @@ -29,6 +31,9 @@ enum event_t { /* Init hooks */ EVT_MISC_INIT_F,

+ /* Device tree fixups before booting */ + EVT_FT_FIXUP, + EVT_COUNT };

@@ -50,6 +55,15 @@ union event_data { struct event_dm { struct udevice *dev; } dm; + + /** + * struct event_ft_fixup - FDT fixup before booting + * + * @tree: tree to update + */ + struct event_ft_fixup { + oftree tree; + } ft_fixup; };

/** diff --git a/test/py/tests/test_event_dump.py b/test/py/tests/test_event_dump.py index b753e804ac3..17001777247 100644 --- a/test/py/tests/test_event_dump.py +++ b/test/py/tests/test_event_dump.py @@ -16,5 +16,6 @@ def test_event_dump(u_boot_console): out = util.run_and_log(cons, ['scripts/event_dump.py', sandbox]) expect = '''.*Event type Id Source location -------------------- ------------------------------ ------------------------------ +EVT_FT_FIXUP bootmeth_vbe_simple_ft_fixup boot/vbe_simple.c:.* EVT_MISC_INIT_F sandbox_misc_init_f .*arch/sandbox/cpu/start.c:''' assert re.match(expect, out, re.MULTILINE) is not None

Add support for VBE simple, which permits firmware update of a single image stored in MMC or another block device.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

boot/Kconfig | 12 ++ boot/Makefile | 1 + boot/vbe_simple.c | 315 +++++++++++++++++++++++++++++++ test/py/tests/test_event_dump.py | 2 +- 4 files changed, 329 insertions(+), 1 deletion(-) create mode 100644 boot/vbe_simple.c

diff --git a/boot/Kconfig b/boot/Kconfig index 094a790f3e8..f7a2d2acc7b 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -402,6 +402,18 @@ config BOOTMETH_VBE supports selection of various firmware components, seleciton of an OS to boot as well as updating these using fwupd.

+if BOOTMETH_VBE + +config BOOTMETH_VBE_SIMPLE + bool "Bootdev support for VBE 'simple' method" + default y + help + Enables support for VBE 'simple' boot. This allows updating a single + firmware image in boot media such as MMC. It does not support any sort + of rollback, recovery or A/B boot. + +endif # BOOTMETH_VBE + config BOOTMETH_SANDBOX def_bool y depends on SANDBOX diff --git a/boot/Makefile b/boot/Makefile index 0378a9db27f..87ca7b4434e 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -49,3 +49,4 @@ obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o endif

obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o +obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o diff --git a/boot/vbe_simple.c b/boot/vbe_simple.c new file mode 100644 index 00000000000..a395bc20a60 --- /dev/null +++ b/boot/vbe_simple.c @@ -0,0 +1,315 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Verified Boot for Embedded (VBE) 'simple' method + * + * Copyright 2022 Google LLC + * Written by Simon Glass sjg@chromium.org + */ + +#include <common.h> +#include <log.h> +#include <memalign.h> +#include <part.h> +#include <bootflow.h> +#include <bootmeth.h> +#include <dm.h> +#include <mmc.h> +#include <vbe.h> +#include <version_string.h> +#include <dm/device-internal.h> +#include <dm/ofnode.h> +#include <u-boot/crc.h> + +enum { + MAX_VERSION_LEN = 256, + + NVD_HDR_VER_SHIFT = 0, + NVD_HDR_VER_MASK = 0xf, + NVD_HDR_SIZE_SHIFT = 4, + NVD_HDR_SIZE_MASK = 0xf << NVD_HDR_SIZE_SHIFT, + + /* Firmware key-version is in the top 16 bits of fw_ver */ + FWVER_KEY_SHIFT = 16, + FWVER_FW_MASK = 0xffff, + + NVD_HDR_VER_CUR = 1, /* current version */ +}; + +/** struct simple_priv - information read from the device tree */ +struct simple_priv { + u32 area_start; + u32 area_size; + u32 skip_offset; + u32 state_offset; + u32 state_size; + u32 version_offset; + u32 version_size; + const char *storage; +}; + +/** struct simple_state - state information read from media + * + * @fw_version: Firmware version string + * @fw_vernum: Firmware version number + */ +struct simple_state { + char fw_version[MAX_VERSION_LEN]; + u32 fw_vernum; +}; + +/** struct simple_nvdata - storage format for non-volatile data */ +struct simple_nvdata { + u8 crc8; + u8 hdr; + u16 spare1; + u32 fw_vernum; + u8 spare2[0x38]; +}; + +static int simple_read_version(struct udevice *dev, struct blk_desc *desc, + u8 *buf, struct simple_state *state) +{ + struct simple_priv *priv = dev_get_priv(dev); + int start; + + if (priv->version_size > MMC_MAX_BLOCK_LEN) + return log_msg_ret("ver", -E2BIG); + + start = priv->area_start + priv->version_offset; + if (start & (MMC_MAX_BLOCK_LEN - 1)) + return log_msg_ret("get", -EBADF); + start /= MMC_MAX_BLOCK_LEN; + + if (blk_dread(desc, start, 1, buf) != 1) + return log_msg_ret("read", -EIO); + strlcpy(state->fw_version, buf, MAX_VERSION_LEN); + log_debug("version=%s\n", state->fw_version); + + return 0; +} + +static int simple_read_nvdata(struct udevice *dev, struct blk_desc *desc, + u8 *buf, struct simple_state *state) +{ + struct simple_priv *priv = dev_get_priv(dev); + uint hdr_ver, hdr_size, size, crc; + const struct simple_nvdata *nvd; + int start; + + if (priv->state_size > MMC_MAX_BLOCK_LEN) + return log_msg_ret("state", -E2BIG); + + start = priv->area_start + priv->state_offset; + if (start & (MMC_MAX_BLOCK_LEN - 1)) + return log_msg_ret("get", -EBADF); + start /= MMC_MAX_BLOCK_LEN; + + if (blk_dread(desc, start, 1, buf) != 1) + return log_msg_ret("read", -EIO); + nvd = (struct simple_nvdata *)buf; + hdr_ver = (nvd->hdr & NVD_HDR_VER_MASK) >> NVD_HDR_VER_SHIFT; + hdr_size = (nvd->hdr & NVD_HDR_SIZE_MASK) >> NVD_HDR_SIZE_SHIFT; + if (hdr_ver != NVD_HDR_VER_CUR) + return log_msg_ret("hdr", -EPERM); + size = 1 << hdr_size; + if (size > sizeof(*nvd)) + return log_msg_ret("sz", -ENOEXEC); + + crc = crc8(0, buf + 1, size - 1); + if (crc != nvd->crc8) + return log_msg_ret("crc", -EPERM); + state->fw_vernum = nvd->fw_vernum; + + log_debug("version=%s\n", state->fw_version); + + return 0; +} + +static int simple_read_state(struct udevice *dev, struct simple_state *state) +{ + ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN); + struct simple_priv *priv = dev_get_priv(dev); + struct blk_desc *desc; + char devname[16]; + const char *end; + int devnum; + int ret; + + /* First figure out the block device */ + log_debug("storage=%s\n", priv->storage); + devnum = trailing_strtoln_end(priv->storage, NULL, &end); + if (devnum == -1) + return log_msg_ret("num", -ENODEV); + if (end - priv->storage >= sizeof(devname)) + return log_msg_ret("end", -E2BIG); + strlcpy(devname, priv->storage, end - priv->storage + 1); + log_debug("dev=%s, %x\n", devname, devnum); + + desc = blk_get_dev(devname, devnum); + if (!desc) + return log_msg_ret("get", -ENXIO); + + ret = simple_read_version(dev, desc, buf, state); + if (ret) + return log_msg_ret("ver", ret); + + ret = simple_read_nvdata(dev, desc, buf, state); + if (ret) + return log_msg_ret("nvd", ret); + + return 0; +} + +static int vbe_simple_get_state_desc(struct udevice *dev, char *buf, + int maxsize) +{ + struct simple_state state; + int ret; + + ret = simple_read_state(dev, &state); + if (ret) + return log_msg_ret("read", ret); + + if (maxsize < 30) + return -ENOSPC; + snprintf(buf, maxsize, "Version: %s\nVernum: %x/%x", state.fw_version, + state.fw_vernum >> FWVER_KEY_SHIFT, + state.fw_vernum & FWVER_FW_MASK); + + return 0; +} + +static int vbe_simple_read_bootflow(struct udevice *dev, struct bootflow *bflow) +{ + /* To be implemented */ + + return -EINVAL; +} + +static struct bootmeth_ops bootmeth_vbe_simple_ops = { + .get_state_desc = vbe_simple_get_state_desc, + .read_bootflow = vbe_simple_read_bootflow, + .read_file = bootmeth_common_read_file, +}; + +int vbe_simple_fixup_node(ofnode node, struct simple_state *state) +{ + char *version; + int ret; + + version = strdup(state->fw_version); + if (!version) + return log_msg_ret("ver", -ENOMEM); + + ret = ofnode_write_string(node, "cur-version", version); + if (ret) + return log_msg_ret("ver", ret); + ret = ofnode_write_u32(node, "cur-vernum", state->fw_vernum); + if (ret) + return log_msg_ret("ver", ret); + ret = ofnode_write_string(node, "bootloader-version", version_string); + if (ret) + return log_msg_ret("fix", ret); + + return 0; +} + +/** + * bootmeth_vbe_simple_ft_fixup() - Write out all VBE simple data to the DT + * + * @ctx: Context for event + * @event: Event to process + * @return 0 if OK, -ve on error + */ +static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event) +{ + oftree tree = event->data.ft_fixup.tree; + struct udevice *dev; + ofnode node; + int ret; + + /* + * Ideally we would have driver model support for fixups, but that does + * not exist yet. It is a step too far to try to do this before VBE is + * in place. + */ + for (ret = vbe_find_first_device(&dev); dev; + ret = vbe_find_next_device(&dev)) { + struct simple_state state; + + if (strcmp("vbe_simple", dev->driver->name)) + continue; + + /* Check if there is a node to fix up */ + node = ofnode_path_root(tree, "/chosen/fwupd"); + if (!ofnode_valid(node)) + continue; + node = ofnode_find_subnode(node, dev->name); + if (!ofnode_valid(node)) + continue; + + log_debug("Fixing up: %s\n", dev->name); + ret = device_probe(dev); + if (ret) + return log_msg_ret("probe", ret); + ret = simple_read_state(dev, &state); + if (ret) + return log_msg_ret("read", ret); + + ret = vbe_simple_fixup_node(node, &state); + if (ret) + return log_msg_ret("fix", ret); + } + + return 0; +} +EVENT_SPY(EVT_FT_FIXUP, bootmeth_vbe_simple_ft_fixup); + +static int bootmeth_vbe_simple_probe(struct udevice *dev) +{ + struct simple_priv *priv = dev_get_priv(dev); + + memset(priv, '\0', sizeof(*priv)); + if (dev_read_u32(dev, "area-start", &priv->area_start) || + dev_read_u32(dev, "area-size", &priv->area_size) || + dev_read_u32(dev, "version-offset", &priv->version_offset) || + dev_read_u32(dev, "version-size", &priv->version_size) || + dev_read_u32(dev, "state-offset", &priv->state_offset) || + dev_read_u32(dev, "state-size", &priv->state_size)) + return log_msg_ret("read", -EINVAL); + dev_read_u32(dev, "skip-offset", &priv->skip_offset); + priv->storage = strdup(dev_read_string(dev, "storage")); + if (!priv->storage) + return log_msg_ret("str", -EINVAL); + + return 0; +} + +static int bootmeth_vbe_simple_bind(struct udevice *dev) +{ + struct bootmeth_uc_plat *plat = dev_get_uclass_plat(dev); + + plat->desc = IS_ENABLED(CONFIG_BOOTSTD_FULL) ? + "VBE simple" : "vbe-simple"; + plat->flags = BOOTMETHF_GLOBAL; + + return 0; +} + +#if CONFIG_IS_ENABLED(OF_REAL) +static const struct udevice_id generic_simple_vbe_simple_ids[] = { + { .compatible = "fwupd,vbe-simple" }, + { } +}; +#endif + +U_BOOT_DRIVER(vbe_simple) = { + .name = "vbe_simple", + .id = UCLASS_BOOTMETH, + .of_match = of_match_ptr(generic_simple_vbe_simple_ids), + .ops = &bootmeth_vbe_simple_ops, + .bind = bootmeth_vbe_simple_bind, + .probe = bootmeth_vbe_simple_probe, + .flags = DM_FLAG_PRE_RELOC, + .priv_auto = sizeof(struct simple_priv), +}; diff --git a/test/py/tests/test_event_dump.py b/test/py/tests/test_event_dump.py index 17001777247..bc54149e8f2 100644 --- a/test/py/tests/test_event_dump.py +++ b/test/py/tests/test_event_dump.py @@ -16,6 +16,6 @@ def test_event_dump(u_boot_console): out = util.run_and_log(cons, ['scripts/event_dump.py', sandbox]) expect = '''.*Event type Id Source location -------------------- ------------------------------ ------------------------------ -EVT_FT_FIXUP bootmeth_vbe_simple_ft_fixup boot/vbe_simple.c:.* +EVT_FT_FIXUP bootmeth_vbe_simple_ft_fixup .*boot/vbe_simple.c:.* EVT_MISC_INIT_F sandbox_misc_init_f .*arch/sandbox/cpu/start.c:''' assert re.match(expect, out, re.MULTILINE) is not None

Add some documentation updates, particularly about global bootmeths.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

doc/develop/bootstd.rst | 88 +++++++++++++++++++++++++++----------- doc/usage/cmd/bootmeth.rst | 9 ++-- 2 files changed, 68 insertions(+), 29 deletions(-)

diff --git a/doc/develop/bootstd.rst b/doc/develop/bootstd.rst index dadd3473e5c..b8773f8339d 100644 --- a/doc/develop/bootstd.rst +++ b/doc/develop/bootstd.rst @@ -90,6 +90,12 @@ bootflows. Note: it is possible to have a bootmeth that uses a partition or a whole device directly, but it is more common to use a filesystem.

+Note that some bootmeths are 'global', meaning that they select the bootdev +themselves. Examples include VBE and EFI boot manager. In this case, they +provide a `read_bootflow()` method which checks whatever bootdevs it likes, then +returns the bootflow, if found. Some of these bootmeths may be very slow, if +they scan a lot of devices. +

Boot process ------------ @@ -113,6 +119,9 @@ the following command:: which scans for available bootflows, optionally listing each find it finds (-l) and trying to boot it (-b).

+When global bootmeths are available, these are typically checked before the +above bootdev scanning. +

Controlling ordering -------------------- @@ -270,18 +279,8 @@ Standard boot requires a single instance of the bootstd device to make things work. This includes global information about the state of standard boot. See `struct bootstd_priv` for this structure, accessed with `bootstd_get_priv()`.

-Within the devicetree, if you add bootmeth devices or a system bootdev, they -should be children of the bootstd device. See `arch/sandbox/dts/test.dts` for -an example of this. - - -The system bootdev ------------------- - -Some bootmeths don't operate on individual bootdevs, but on the whole system. -For example, the EFI boot manager does its own device scanning and does not -make use of the bootdev devices. Such bootmeths can make use of the system -bootdev, typically considered last, after everything else has been tried. +Within the devicetree, if you add bootmeth devices, they should be children of +the bootstd device. See `arch/sandbox/dts/test.dts` for an example of this.

.. _`Automatic Devices`: @@ -292,12 +291,11 @@ Automatic devices It is possible to define all the required devices in the devicetree manually, but it is not necessary. The bootstd uclass includes a `dm_scan_other()` function which creates the bootstd device if not found. If no bootmeth devices -are found at all, it creates one for each available bootmeth driver as well as a -system bootdev. +are found at all, it creates one for each available bootmeth driver.

If your devicetree has any bootmeth device it must have all of them that you -want to use, as well as the system bootdev if needed, since no bootmeth devices -will be created automatically in that case. +want to use, since no bootmeth devices will be created automatically in that +case.

Using devicetree @@ -348,6 +346,7 @@ Bootmeth drivers are provided for: - distro boot from a disk (syslinux) - distro boot from a network (PXE) - EFI boot using bootefi + - VBE - EFI boot using boot manager

@@ -434,18 +433,23 @@ case, the iterator ends up with a `dev_order` array containing the bootdevs that are going to be used, with `num_devs` set to the number of bootdevs and `cur_dev` starting at 0.

-Next, the ordering of bootdevs is determined, by `bootmeth_setup_iter_order()`. +Next, the ordering of bootmeths is determined, by `bootmeth_setup_iter_order()`. By default the ordering is again by sequence number, i.e. the `/aliases` node, or failing that the order in the devicetree. But the `bootmeth order` command or `bootmeths` environment variable can be used to set up an ordering. If that has been done, the ordering is in `struct bootstd_priv`, so that ordering is simply copied into the iterator. Either way, the `method_order` array it set up, -along with `num_methods`. Then `cur_method` is set to 0. +along with `num_methods`. + +Note that global bootmeths are always put at the end of the ordering. If any are +present, `cur_method` is set to the first one, so that global bootmeths are done +first. Once all have been used, these bootmeths are dropped from the iteration. +When there are no global bootmeths, `cur_method` is set to 0.

At this point the iterator is ready to use, with the first bootdev and bootmeth -selected. All the other fields are 0. This means that the current partition is -0, which is taken to mean the whole device, since partition numbers start at 1. -It also means that `max_part` is 0, i.e. the maximum partition number we know +selected. Most of the other fields are 0. This means that the current partition +is 0, which is taken to mean the whole device, since partition numbers start at +1. It also means that `max_part` is 0, i.e. the maximum partition number we know about is 0, meaning that, as far as we know, there is no partition table on this bootdev.

@@ -456,6 +460,10 @@ If the `BOOTFLOWF_ALL` iterator flag is set, even errors are returned as incomplete bootflows, but normally an error results in moving onto the next iteration.

+Note that `bootflow_check()` handles global bootmeths explicitly, but calling +`bootmeth_get_bootflow()` on each one. The `doing_global` flag indicates when +the iterator is in that state. + The `bootflow_scan_next()` function handles moving onto the next iteration and checking it. In fact it sits in a loop doing that repeatedly until it finds something it wants to return. @@ -474,9 +482,10 @@ the least-sigificant digit on the right, counting like this: 0 0 2 0 1 0 0 1 1 - 0 1 1 + 0 1 2 1 0 0 1 0 1 + ... ======== ======= =======

The maximum value for `method` is `num_methods - 1` so when it exceeds that, it @@ -488,6 +497,31 @@ exceeds its maximum, then the next bootdev is used. In this way, iter_incr() works its way through all possibilities, moving forward one each time it is called.

+Note that global bootmeths introduce a subtlety into the above description. +When `doing_global` is true, the iteration takes place only among the bootmeths, +i.e. the last column above. The global bootmeths are at the end of the list. +Assuming that they are entries 3 and 4 in the list, the iteration then looks +like this: + + ======== ======= ======= ======================================= + bootdev part method notes + ======== ======= ======= ======================================= + . . 3 doing_global = true, method_count = 5 + . . 4 + 0 0 0 doing_global = false, method_count = 3 + 0 0 1 + 0 0 2 + 0 1 0 + 0 1 1 + 0 1 2 + 1 0 0 + 1 0 1 + ... + ======== ======= ======= ======================================= + +The changeover of the value of `doing_global` from true to false is handled in +`iter_incr()` as well. + There is no expectation that iteration will actually finish. Quite often a valid bootflow is found early on. With `bootflow scan -b`, that causes the bootflow to be immediately booted. Assuming it is successful, the iteration never @@ -517,17 +551,19 @@ method `bootdev_get_bootflow()` to ask the bootdev to return a bootflow. It passes the iterator to the bootdev method, so that function knows what we are talking about. At first, the bootflow is set up in the state `BOOTFLOWST_BASE`, with just the `method` and `dev` intiialised. But the bootdev may fill in more, -e.g. updating the state, depending on what it finds. +e.g. updating the state, depending on what it finds. For global bootmeths the +`bootmeth_get_bootflow()` function is called instead of +`bootdev_get_bootflow()`.

-Based on what the bootdev responds with, `bootflow_check()` either +Based on what the bootdev or bootmeth responds with, `bootflow_check()` either returns a valid bootflow, or a partial one with an error. A partial bootflow is one that has some fields set up, but did not reach the `BOOTFLOWST_READY` state. As noted before, if the `BOOTFLOWF_ALL` iterator flag is set, then all bootflows are returned, even partial ones. This can help with debugging.

So at this point you can see that total control over whether a bootflow can -be generated from a particular iteration, or not, rests with the bootdev. -Each one can adopt its own approach. +be generated from a particular iteration, or not, rests with the bootdev (or +global bootmeth). Each one can adopt its own approach.

Going down a level, what does the bootdev do in its `get_bootflow()` method? Let us consider the MMC bootdev. In that case the call to diff --git a/doc/usage/cmd/bootmeth.rst b/doc/usage/cmd/bootmeth.rst index 9fc7ebf0abf..29d8215a0c0 100644 --- a/doc/usage/cmd/bootmeth.rst +++ b/doc/usage/cmd/bootmeth.rst @@ -31,7 +31,9 @@ scanning bootdevs, each bootmeth is tried in turn to see if it can find a valid bootflow. You can use this command to adjust the order or even to omit some boomeths.

-The argument is a quoted list of bootmeths to use, by name. +The argument is a quoted list of bootmeths to use, by name. If global bootmeths +are included, they must be at the end, otherwise the scanning mechanism will not +work correctly.

bootmeth list @@ -47,14 +49,15 @@ Order Seq Name Description 1 1 efi EFI boot from an .efi file 2 2 pxe PXE boot from a network device 3 3 sandbox Sandbox boot for testing - 4 4 efi_mgr EFI bootmgr flow + glob 4 efi_mgr EFI bootmgr flow ===== === ================== =================================

The fields are as follows:

Order: The order in which these bootmeths are invoked for each bootdev. If this - shows as a hyphen, then the bootmeth is not in the current ordering. + shows as a hyphen, then the bootmeth is not in the current ordering. If it + shows as 'glob', then this is a global bootmeth and should be at the end.

Seq: The sequence number of the bootmeth, i.e. the normal ordering if none is set

Add a command to look at VBE methods and their status. Provide a test for all of this as well.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Make VBE a global bootmeth - Introduce the concept of global bootmeths (various patches) - Correct the clang / LTO / event bug from v1

cmd/Kconfig | 10 ++++ cmd/Makefile | 1 + cmd/vbe.c | 87 +++++++++++++++++++++++++++++++ test/boot/vbe_simple.c | 115 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 213 insertions(+) create mode 100644 cmd/vbe.c create mode 100644 test/boot/vbe_simple.c

diff --git a/cmd/Kconfig b/cmd/Kconfig index a8260aa170d..77cf0feb399 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -330,6 +330,16 @@ config BOOTM_RTEMS help Support booting RTEMS images via the bootm command.

+config CMD_VBE + bool "vbe - Verified Boot for Embedded" + depends on BOOTMETH_VBE + default y + help + Provides various subcommands related to VBE, such as listing the + available methods, looking at the state and changing which method + is used to boot. Updating the parameters is not currently + supported. + config BOOTM_VXWORKS bool "Support booting VxWorks OS images" depends on CMD_BOOTM diff --git a/cmd/Makefile b/cmd/Makefile index 5e43a1e022e..6e87522b62e 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -179,6 +179,7 @@ obj-$(CONFIG_CMD_FS_UUID) += fs_uuid.o obj-$(CONFIG_CMD_USB_MASS_STORAGE) += usb_mass_storage.o obj-$(CONFIG_CMD_USB_SDP) += usb_gadget_sdp.o obj-$(CONFIG_CMD_THOR_DOWNLOAD) += thordown.o +obj-$(CONFIG_CMD_VBE) += vbe.o obj-$(CONFIG_CMD_XIMG) += ximg.o obj-$(CONFIG_CMD_YAFFS2) += yaffs2.o obj-$(CONFIG_CMD_SPL) += spl.o diff --git a/cmd/vbe.c b/cmd/vbe.c new file mode 100644 index 00000000000..a5737edc047 --- /dev/null +++ b/cmd/vbe.c @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Verified Boot for Embedded (VBE) command + * + * Copyright 2022 Google LLC + * Written by Simon Glass sjg@chromium.org + */ + +#include <common.h> +#include <bootmeth.h> +#include <bootstd.h> +#include <command.h> +#include <vbe.h> + +static int do_vbe_list(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + vbe_list(); + + return 0; +} + +static int do_vbe_select(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct bootstd_priv *std; + struct udevice *dev; + int ret; + + ret = bootstd_get_priv(&std); + if (ret) + return CMD_RET_FAILURE; + if (argc < 2) { + std->vbe_bootmeth = NULL; + return 0; + } + if (vbe_find_by_any(argv[1], &dev)) + return CMD_RET_FAILURE; + + std->vbe_bootmeth = dev; + + return 0; +} + +static int do_vbe_info(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct bootstd_priv *std; + char buf[256]; + int ret, len; + + ret = bootstd_get_priv(&std); + if (ret) + return CMD_RET_FAILURE; + if (!std->vbe_bootmeth) { + printf("No VBE bootmeth selected\n"); + return CMD_RET_FAILURE; + } + ret = bootmeth_get_state_desc(std->vbe_bootmeth, buf, sizeof(buf)); + if (ret) { + printf("Failed (err=%d)\n", ret); + return CMD_RET_FAILURE; + } + len = strnlen(buf, sizeof(buf)); + if (len >= sizeof(buf)) { + printf("Buffer overflow\n"); + return CMD_RET_FAILURE; + } + + puts(buf); + if (buf[len] != '\n') + putc('\n'); + + return 0; +} + +#ifdef CONFIG_SYS_LONGHELP +static char vbe_help_text[] = + "list - list VBE bootmeths\n" + "vbe select - select a VBE bootmeth by sequence or name\n" + "vbe info - show information about a VBE bootmeth"; +#endif + +U_BOOT_CMD_WITH_SUBCMDS(vbe, "Verified Boot for Embedded", vbe_help_text, + U_BOOT_SUBCMD_MKENT(list, 1, 1, do_vbe_list), + U_BOOT_SUBCMD_MKENT(select, 2, 1, do_vbe_select), + U_BOOT_SUBCMD_MKENT(info, 2, 1, do_vbe_info)); diff --git a/test/boot/vbe_simple.c b/test/boot/vbe_simple.c new file mode 100644 index 00000000000..2f6979cafcf --- /dev/null +++ b/test/boot/vbe_simple.c @@ -0,0 +1,115 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test for vbe-simple bootmeth. All start with 'vbe_simple' + * + * Copyright 2023 Google LLC + * Written by Simon Glass sjg@chromium.org + */ + +#include <common.h> +#include <bootmeth.h> +#include <dm.h> +#include <image.h> +#include <memalign.h> +#include <mmc.h> +#include <of_live.h> +#include <vbe.h> +#include <version_string.h> +#include <linux/log2.h> +#include <test/suites.h> +#include <test/ut.h> +#include <u-boot/crc.h> +#include "bootstd_common.h" + +#define NVDATA_START_BLK ((0x400 + 0x400) / MMC_MAX_BLOCK_LEN) +#define VERSION_START_BLK ((0x400 + 0x800) / MMC_MAX_BLOCK_LEN) +#define TEST_VERSION "U-Boot v2022.04-local2" +#define TEST_VERNUM 0x00010002 + +/* Basic test of reading nvdata and updating a fwupd node in the device tree */ +static int vbe_simple_test_base(struct unit_test_state *uts) +{ + ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN); + const char *version, *bl_version; + struct event_ft_fixup fixup; + struct udevice *dev, *mmc; + struct device_node *np; + struct blk_desc *desc; + char fdt_buf[0x400]; + char info[100]; + int node_ofs; + ofnode node; + u32 vernum; + + /* Set up the version string */ + ut_assertok(uclass_get_device(UCLASS_MMC, 1, &mmc)); + desc = blk_get_by_device(mmc); + ut_assertnonnull(desc); + + memset(buf, '\0', MMC_MAX_BLOCK_LEN); + strcpy(buf, TEST_VERSION); + if (blk_dwrite(desc, VERSION_START_BLK, 1, buf) != 1) + return log_msg_ret("write", -EIO); + + /* Set up the nvdata */ + memset(buf, '\0', MMC_MAX_BLOCK_LEN); + buf[1] = ilog2(0x40) << 4 | 1; + *(u32 *)(buf + 4) = TEST_VERNUM; + buf[0] = crc8(0, buf + 1, 0x3f); + if (blk_dwrite(desc, NVDATA_START_BLK, 1, buf) != 1) + return log_msg_ret("write", -EIO); + + /* Read the version back */ + ut_assertok(vbe_find_by_any("firmware0", &dev)); + ut_assertok(bootmeth_get_state_desc(dev, info, sizeof(info))); + ut_asserteq_str("Version: " TEST_VERSION "\nVernum: 1/2", info); + + ut_assertok(fdt_create_empty_tree(fdt_buf, sizeof(fdt_buf))); + node_ofs = fdt_add_subnode(fdt_buf, 0, "chosen"); + ut_assert(node_ofs > 0); + + node_ofs = fdt_add_subnode(fdt_buf, node_ofs, "fwupd"); + ut_assert(node_ofs > 0); + + node_ofs = fdt_add_subnode(fdt_buf, node_ofs, "firmware0"); + ut_assert(node_ofs > 0); + + /* + * This can only work on the live tree, since the ofnode interface for + * flat tree assumes that ofnode points to the control FDT + */ + ut_assertok(unflatten_device_tree(fdt_buf, &np)); + + /* + * It would be better to call image_setup_libfdt() here, but that + * function does not allow passing an ofnode. We can pass fdt_buf but + * when it comes to send the evenr, it creates an ofnode that uses the + * control FDT, since it has no way of accessing the live tree created + * here. + * + * Two fix this we need: + * - image_setup_libfdt() is updated to use ofnode + * - ofnode updated to support access to an FDT other than the control + * FDT. This is partially implemented with live tree, but not with + * flat tree + */ + fixup.tree.np = np; + ut_assertok(event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup))); + + node = ofnode_path_root(fixup.tree, "/chosen/fwupd/firmware0"); + + version = ofnode_read_string(node, "cur-version"); + ut_assertnonnull(version); + ut_asserteq_str(TEST_VERSION, version); + + ut_assertok(ofnode_read_u32(node, "cur-vernum", &vernum)); + ut_asserteq(TEST_VERNUM, vernum); + + bl_version = ofnode_read_string(node, "bootloader-version"); + ut_assertnonnull(bl_version); + ut_asserteq_str(version_string, bl_version); + + return 0; +} +BOOTSTD_TEST(vbe_simple_test_base, UT_TESTF_DM | UT_TESTF_SCAN_FDT | + UT_TESTF_LIVE_TREE);