[PATCH v2 00/35] vbe: Verified Boot for Embedded initial support
This adds the concept of a VBE method to U-Boot, along with an implementation of the 'VBE simple' method, basically a simple way of updating firmware in MMC from userspace and monitoring it from U-Boot. VBE simple is implemented in fwupd. U-Boot's role is to set up the device tree with the required firmware-update properties and provide the developer with information about the current VBE state. To that end this series includes a new 'vbe' command that allows VBE methods to be listed and examined. As part of this work, support for doing FDT fixups via the event interface is provided, along with the ability to write to the device tree via the ofnode interface. Another (significant) change is that bootmeths now have a 'global' flag, to allow the implementation of EFI bootmgr (and VBE) to be cleaned up. The 'system' bootdev is no-longer needed and these bootmeths are scanned first. Further work is needed to pull everything together, but this is a step along the way. Changes in v2: - Add new patch with some documentation and links for VBE - Add a reference to VBE documentation in the header file and commit - Make VBE a global bootmeth - Introduce the concept of global bootmeths (various patches) - Correct the clang / LTO / event bug from v1 Simon Glass (35): vbe: Add some documentation video: Renname vbe.h to vesa.h video: Rename structs and functions to avoid VBE dm: core: Split out the declaration of ofnode dm: core: Add a note about how livetree updates work dm: core: Introduce support for multiple trees dm: core: Move ofnode-writing test to ofnode dm: core: Swap parameters of ofnode_write_prop() dm: core: Tidy up ofnode-writing test dm: core: Prepare for updating the device tree with ofnode dm: core: Allow writing to a flat tree with ofnode dm: core: Add support for writing u32 with ofnode dm: core: Support sandbox with read interface bootstd: Drop delays in the tests bootstd: Fix comment in bootmeth test bootstd: Detect empty bootmeth ordering bootstd: Provide a bootmeth method to obtain state info bootstd: Tidy up var naming in bootdev_setup_iter_order() bootstd: Allow bootmeths to be marked as global bootstd: Allow EFI bootmgr to support an invalid bootflow bootstd: Allow the bootdev to be optional in bootflows bootstd: Tidy comments in bootflow_scan_bootdev() bootstd: Support bootflows with global bootmeths dm: core: Call dm_scan_other() when setting up for tests bootstd: Allow scanning for global bootmeths separately bootstd: Always create the EFI bootmgr bootmeth bootstd: Drop the system bootdev event: Change EVENT_SPY to global event: Add an event for device tree fixups vbe: Add initial support for VBE vbe: Support VBE simple bootstd: Add vbe bootmeth into sandbox bootstd: Update documentation bootstd: Check building without global bootmeths vbe: Add a new vbe command arch/sandbox/dts/sandbox.dtsi | 13 ++ arch/sandbox/dts/test.dts | 15 ++ arch/sandbox/include/asm/test.h | 11 + arch/x86/lib/bios.c | 12 +- arch/x86/lib/coreboot_table.c | 2 +- arch/x86/lib/fsp/fsp_graphics.c | 4 +- boot/Kconfig | 29 +++ boot/Makefile | 5 +- boot/bootdev-uclass.c | 15 +- boot/bootflow.c | 63 +- boot/bootmeth-uclass.c| 105 +++-- boot/bootmeth_distro.c| 14 ++ boot/bootmeth_efi_mgr.c | 35 ++- boot/bootstd-uclass.c | 13 +- boot/image-fdt.c | 11 + boot/system_bootdev.c | 66 -- boot/vbe.c| 119 ++ boot/vbe_simple.c | 315 ++ cmd/Kconfig | 10 + cmd/Makefile | 1 + cmd/bootflow.c| 12 +- cmd/bootmeth.c| 4 +- cmd/elf.c | 2 +- cmd/read.c| 3 +- cmd/vbe.c | 87 +++ common/event.c| 3 + configs/sandbox_flattree_defconfig| 2 + doc/develop/bootstd.rst | 89 +--- doc/develop/driver-model/livetree.rst | 60 - doc/develop/index.rst | 1 + doc/develop/vbe.rst | 26 +++ doc/usage/cmd/bootmeth.rst| 9 +- drivers/bios_emulator/atibios.c | 18 +- drivers/core/of_access.c | 57 - drivers/core/ofnode.c | 81 +++ drivers/pci/pci_rom.c | 14 +- drivers/video/broadwell_igd.c | 4 +- drivers/video/coreboot.c | 4 +- drivers/video/efi.c | 4 +- drivers/video/ivybridge_igd.c | 4 +- drivers/video/vesa.c | 4 +- include/bios_emul.h | 6 +- include/bootfl
[PATCH v2 01/35] vbe: Add some documentation
Add a few links to documents about Verified Boot for Embedded (VBE). These will be expanded as development proceeds. Signed-off-by: Simon Glass --- 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 000..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_iNzIj1FgPmKF37fpglAC47Tt5cvPBC5fvTdoK-GA5Zv1wifo/pub +.. _vbe-bootflows: https://docs.google.com/document/d/e/2PACX-1vR0OzhuyRJQ8kdeOibS3xB1rVFy3J4M_QKTM5-3vPIBNcdvR0W8EXu9ymG-yWfqthzWoM4JUNhqwydN/pub +.. _vbe-fwupdate: https://docs.google.com/document/d/e/2PACX-1vTnlIL17vVbl6TVoTHWYMED0bme7oHHNk-g5VGxblbPiKIdGDALE1HKId8Go5f0g1eziLsv4h9bocbk/pub -- 2.37.1.455.g008518b4e5-goog
[PATCH v2 02/35] video: Renname vbe.h to vesa.h
We want to use VBE to mean Verfiied Boot for Embedded in U-Boot. Rename
the existing VBE (Vesa BIOS extensions) to allow this.
Verified Boot for Embedded is documented doc/develop/vbe.rst
Signed-off-by: Simon Glass
---
Changes in v2:
- Add a reference to VBE documentation in the header file and commit
arch/x86/lib/bios.c | 2 +-
arch/x86/lib/coreboot_table.c | 2 +-
arch/x86/lib/fsp/fsp_graphics.c | 2 +-
cmd/elf.c | 2 +-
drivers/bios_emulator/atibios.c | 2 +-
drivers/pci/pci_rom.c | 2 +-
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/{vbe.h => vesa.h} | 4 ++--
lib/elf.c | 2 +-
13 files changed, 14 insertions(+), 14 deletions(-)
rename include/{vbe.h => vesa.h} (98%)
diff --git a/arch/x86/lib/bios.c b/arch/x86/lib/bios.c
index 98cc05de2eb..087539ba7db 100644
--- a/arch/x86/lib/bios.c
+++ b/arch/x86/lib/bios.c
@@ -10,7 +10,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
diff --git a/arch/x86/lib/coreboot_table.c b/arch/x86/lib/coreboot_table.c
index 6eab0452fda..05519d851a9 100644
--- a/arch/x86/lib/coreboot_table.c
+++ b/arch/x86/lib/coreboot_table.c
@@ -6,7 +6,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
diff --git a/arch/x86/lib/fsp/fsp_graphics.c b/arch/x86/lib/fsp/fsp_graphics.c
index 02fd05c9faf..6a7552e6956 100644
--- a/arch/x86/lib/fsp/fsp_graphics.c
+++ b/arch/x86/lib/fsp/fsp_graphics.c
@@ -9,7 +9,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
diff --git a/cmd/elf.c b/cmd/elf.c
index 2b33c50bd02..ce40d3f72a7 100644
--- a/cmd/elf.c
+++ b/cmd/elf.c
@@ -14,7 +14,7 @@
#include
#include
#ifdef CONFIG_X86
-#include
+#include
#include
#include
#include
diff --git a/drivers/bios_emulator/atibios.c b/drivers/bios_emulator/atibios.c
index cdc5ba6ad90..09da76bc5d9 100644
--- a/drivers/bios_emulator/atibios.c
+++ b/drivers/bios_emulator/atibios.c
@@ -51,7 +51,7 @@
#include
#include
#include
-#include
+#include
#include
#include "biosemui.h"
diff --git a/drivers/pci/pci_rom.c b/drivers/pci/pci_rom.c
index 73d15e797fc..ceeb59d1fe4 100644
--- a/drivers/pci/pci_rom.c
+++ b/drivers/pci/pci_rom.c
@@ -34,7 +34,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
diff --git a/drivers/video/broadwell_igd.c b/drivers/video/broadwell_igd.c
index 2551f162e8f..81f0fd8c019 100644
--- a/drivers/video/broadwell_igd.c
+++ b/drivers/video/broadwell_igd.c
@@ -11,7 +11,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
diff --git a/drivers/video/coreboot.c b/drivers/video/coreboot.c
index 7237542c076..3efc65daa2b 100644
--- a/drivers/video/coreboot.c
+++ b/drivers/video/coreboot.c
@@ -6,7 +6,7 @@
#include
#include
#include
-#include
+#include
#include
#include
diff --git a/drivers/video/efi.c b/drivers/video/efi.c
index 5f9031f2ec5..d60b6e27569 100644
--- a/drivers/video/efi.c
+++ b/drivers/video/efi.c
@@ -9,7 +9,7 @@
#include
#include
#include
-#include
+#include
#include
struct pixel {
diff --git a/drivers/video/ivybridge_igd.c b/drivers/video/ivybridge_igd.c
index 1aa5317dd5f..18672a18973 100644
--- a/drivers/video/ivybridge_igd.c
+++ b/drivers/video/ivybridge_igd.c
@@ -10,7 +10,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
diff --git a/drivers/video/vesa.c b/drivers/video/vesa.c
index 869e5469732..91da939e59b 100644
--- a/drivers/video/vesa.c
+++ b/drivers/video/vesa.c
@@ -7,7 +7,7 @@
#include
#include
#include
-#include
+#include
#include
#include
diff --git a/include/vbe.h b/include/vesa.h
similarity index 98%
rename from include/vbe.h
rename to include/vesa.h
index 1631260eb73..30df58a9f1b 100644
--- a/include/vbe.h
+++ b/include/vesa.h
@@ -7,8 +7,8 @@
* Contributors:
* IBM Corporation - initial implementation
*/
-#ifndef _VBE_H
-#define _VBE_H
+#ifndef _VESA_H
+#define _VESA_H
/* these structs are for input from and output to OF */
struct __packed vbe_screen_info {
diff --git a/lib/elf.c b/lib/elf.c
index d074e4e0a7d..0476b2614c3 100644
--- a/lib/elf.c
+++ b/lib/elf.c
@@ -11,7 +11,7 @@
#include
#include
#ifdef CONFIG_X86
-#include
+#include
#include
#include
#endif
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 03/35] video: Rename structs and functions to avoid VBE
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
---
(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, 0x, mi->video_mode,
0x, 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) & 0x;
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;
[PATCH v2 04/35] dm: core: Split out the declaration of ofnode
This is used by a lot of files, but ofnode.h needs to include a lot of
header files. This can create dependency cycles, particularly with
global_data.h which must include various declarations.
Split the core delcarations into a separate file to fix this.
Signed-off-by: Simon Glass
---
(no changes since v1)
include/dm/ofnode.h | 61 +-
include/dm/ofnode_decl.h | 72
2 files changed, 73 insertions(+), 60 deletions(-)
create mode 100644 include/dm/ofnode_decl.h
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index bb60433124b..346b09c7d96 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -19,41 +19,7 @@
struct resource;
-/**
- * typedef union ofnode_union ofnode - reference to a device tree node
- *
- * This union can hold either a straightforward pointer to a struct device_node
- * in the live device tree, or an offset within the flat device tree. In the
- * latter case, the pointer value is just the integer offset within the flat
DT.
- *
- * Thus we can reference nodes in both the live tree (once available) and the
- * flat tree (until then). Functions are available to translate between an
- * ofnode and either an offset or a `struct device_node *`.
- *
- * The reference can also hold a null offset, in which case the pointer value
- * here is NULL. This corresponds to a struct device_node * value of
- * NULL, or an offset of -1.
- *
- * There is no ambiguity as to whether ofnode holds an offset or a node
- * pointer: when the live tree is active it holds a node pointer, otherwise it
- * holds an offset. The value itself does not need to be unique and in theory
- * the same value could point to a valid device node or a valid offset. We
- * could arrange for a unique value to be used (e.g. by making the pointer
- * point to an offset within the flat device tree in the case of an offset) but
- * this increases code size slightly due to the subtraction. Since it offers no
- * real benefit, the approach described here seems best.
- *
- * For now these points use constant types, since we don't allow writing
- * the DT.
- *
- * @np: Pointer to device node, used for live tree
- * @of_offset: Pointer into flat device tree, used for flat tree. Note that
this
- * is not a really a pointer to a node: it is an offset value. See above.
- */
-typedef union ofnode_union {
- const struct device_node *np;
- long of_offset;
-} ofnode;
+#include
struct ofnode_phandle_args {
ofnode node;
@@ -61,31 +27,6 @@ struct ofnode_phandle_args {
uint32_t args[OF_MAX_PHANDLE_ARGS];
};
-/**
- * struct ofprop - reference to a property of a device tree node
- *
- * This struct hold the reference on one property of one node,
- * using struct ofnode and an offset within the flat device tree or either
- * a pointer to a struct property in the live device tree.
- *
- * Thus we can reference arguments in both the live tree and the flat tree.
- *
- * The property reference can also hold a null reference. This corresponds to
- * a struct property NULL pointer or an offset of -1.
- *
- * @node: Pointer to device node
- * @offset: Pointer into flat device tree, used for flat tree.
- * @prop: Pointer to property, used for live treee.
- */
-
-struct ofprop {
- ofnode node;
- union {
- int offset;
- const struct property *prop;
- };
-};
-
/**
* ofnode_to_np() - convert an ofnode to a live DT node pointer
*
diff --git a/include/dm/ofnode_decl.h b/include/dm/ofnode_decl.h
new file mode 100644
index 000..7c9e43e4ad8
--- /dev/null
+++ b/include/dm/ofnode_decl.h
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass
+ */
+
+#ifndef _DM_OFNODE_DECL_H
+#define _DM_OFNODE_DECL_H
+
+/**
+ * typedef union ofnode_union ofnode - reference to a device tree node
+ *
+ * This union can hold either a straightforward pointer to a struct device_node
+ * in the live device tree, or an offset within the flat device tree. In the
+ * latter case, the pointer value is just the integer offset within the flat
DT.
+ *
+ * Thus we can reference nodes in both the live tree (once available) and the
+ * flat tree (until then). Functions are available to translate between an
+ * ofnode and either an offset or a `struct device_node *`.
+ *
+ * The reference can also hold a null offset, in which case the pointer value
+ * here is NULL. This corresponds to a struct device_node * value of
+ * NULL, or an offset of -1.
+ *
+ * There is no ambiguity as to whether ofnode holds an offset or a node
+ * pointer: when the live tree is active it holds a node pointer, otherwise it
+ * holds an offset. The value itself does not need to be unique and in theory
+ * the same value could point to a valid device node or a valid offset. We
+ * could arrange for a unique value to be used (e.g. by making the pointer
+ * point to an off
[PATCH v2 05/35] dm: core: Add a note about how livetree updates work
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 --- (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 -- 2.37.1.455.g008518b4e5-goog
[PATCH v2 06/35] dm: core: Introduce support for multiple trees
At present ofnode only works with a single device tree, for the most part.
This is the control FDT used by U-Boot.
When booting an OS we may obtain a different device tree and want to
modify it. Add some initial support for this into the ofnode API.
Note that we don't permit aliases in this other device tree, since the
of_access implementation maintains a list of aliases collected at
start-up. Also, we don't need aliases to do fixups in the other FDT. So
make sure that flat tree and live tree processing are consistent in this
area.
Signed-off-by: Simon Glass
---
(no changes since v1)
doc/develop/driver-model/livetree.rst | 17 +++
drivers/core/of_access.c | 14 --
drivers/core/ofnode.c | 11 +
include/dm/of_access.h| 10 +++-
include/dm/ofnode.h | 28 +++
include/dm/ofnode_decl.h | 13 ++
include/of_live.h | 16 +++
lib/of_live.c | 14 +-
test/dm/ofnode.c | 67 +++
9 files changed, 171 insertions(+), 19 deletions(-)
diff --git a/doc/develop/driver-model/livetree.rst
b/doc/develop/driver-model/livetree.rst
index fb2969259d0..c29f29b205b 100644
--- a/doc/develop/driver-model/livetree.rst
+++ b/doc/develop/driver-model/livetree.rst
@@ -225,6 +225,23 @@ freed. Then the tree can be scanned for these 'separately
allocated' nodes and
properties before freeing the memory block.
+Multiple livetrees
+--
+
+The livetree implementation was originally designed for use with the control
+FDT. This means that the FDT fix-ups (ft_board_setup() and the like, must use
+a flat tree.
+
+It would be helpful to use livetree for fixups, since adding a lot of nodes and
+properties would involve less memory copying and be more efficient. As a step
+towards this, an `oftree` type has been introduced. It is normally set to
+oftree_default() but can be set to other values. Eventually this should allow
+the use of FDT fixups using the ofnode interface, instead of the low-level
+libfdt one.
+
+See dm_test_ofnode_root() for some examples.
+
+
Internal implementation
---
diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
index c20b19cb50f..0e5915a43e6 100644
--- a/drivers/core/of_access.c
+++ b/drivers/core/of_access.c
@@ -343,24 +343,30 @@ static struct device_node *__of_find_node_by_path(struct
device_node *parent,
#define for_each_property_of_node(dn, pp) \
for (pp = dn->properties; pp != NULL; pp = pp->next)
-struct device_node *of_find_node_opts_by_path(const char *path,
+struct device_node *of_find_node_opts_by_path(struct device_node *root,
+ const char *path,
const char **opts)
{
struct device_node *np = NULL;
struct property *pp;
const char *separator = strchr(path, ':');
+ if (!root)
+ root = gd->of_root;
if (opts)
*opts = separator ? separator + 1 : NULL;
if (strcmp(path, "/") == 0)
- return of_node_get(gd->of_root);
+ return of_node_get(root);
/* The path could begin with an alias */
if (*path != '/') {
int len;
const char *p = separator;
+ /* Only allow alias processing on the control FDT */
+ if (root != gd->of_root)
+ return NULL;
if (!p)
p = strchrnul(path, '/');
len = p - path;
@@ -383,7 +389,7 @@ struct device_node *of_find_node_opts_by_path(const char
*path,
/* Step down the tree matching path components */
if (!np)
- np = of_node_get(gd->of_root);
+ np = of_node_get(root);
while (np && *path == '/') {
struct device_node *tmp = np;
@@ -791,7 +797,7 @@ int of_alias_scan(void)
name = of_get_property(of_chosen, "stdout-path", NULL);
if (name)
- of_stdout = of_find_node_opts_by_path(name,
+ of_stdout = of_find_node_opts_by_path(NULL, name,
&of_stdout_options);
}
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index a59832ebbfb..bd41ef503c2 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -552,6 +552,17 @@ ofnode ofnode_path(const char *path)
return offset_to_ofnode(fdt_path_offset(gd->fdt_blob, path));
}
+ofnode ofnode_path_root(oftree tree, const char *path)
+{
+ if (of_live_active())
+ return np_to_ofnode(of_find_node_opts_by_path(tree.np, path,
+ NULL));
+ else if (*path != '/' && tree.fdt != gd->fdt_blob)
+ return ofnode_null(); /* Aliases
[PATCH v2 08/35] dm: core: Swap parameters of ofnode_write_prop()
It is normal for the length to come after the value in libfdt. Follow this
same convention with ofnode.
Signed-off-by: Simon Glass
---
(no changes since v1)
drivers/core/ofnode.c | 6 +++---
include/dm/ofnode.h | 6 +++---
test/dm/ofnode.c | 4 ++--
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index bd41ef503c2..1c9542a3567 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -1105,8 +1105,8 @@ ofnode ofnode_by_prop_value(ofnode from, const char
*propname,
}
}
-int ofnode_write_prop(ofnode node, const char *propname, int len,
- const void *value)
+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;
@@ -1161,7 +1161,7 @@ int ofnode_write_string(ofnode node, const char
*propname, const char *value)
debug("%s: %s = %s", __func__, propname, value);
- return ofnode_write_prop(node, propname, strlen(value) + 1, value);
+ return ofnode_write_prop(node, propname, value, strlen(value) + 1);
}
int ofnode_set_enabled(ofnode node, bool value)
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index d7ad5dccc14..071a9d63f67 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -1114,13 +1114,13 @@ int ofnode_device_is_compatible(ofnode node, const char
*compat);
*
* @node: The node for whose property should be set
* @propname: The name of the property to set
- * @len: The length of the new value of the property
* @value: The new value of the property (must be valid prior to calling
* the function)
+ * @len: The length of the new value of the property
* Return: 0 if successful, -ve on error
*/
-int ofnode_write_prop(ofnode node, const char *propname, int len,
- const void *value);
+int ofnode_write_prop(ofnode node, const char *propname, const void *value,
+ int len);
/**
* ofnode_write_string() - Set a string property of a ofnode
diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c
index b8d8e440dbc..0aeaaeb7f8c 100644
--- a/test/dm/ofnode.c
+++ b/test/dm/ofnode.c
@@ -576,8 +576,8 @@ static int dm_test_ofnode_livetree_writing(struct
unit_test_state *uts)
/* 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_assertok(ofnode_write_prop(node, "reg",
+ "\x00\x00\x00\x42\x00\x00\x01\x00", 8));
ut_asserteq(0x42, dev_read_addr(dev));
/* Test disabling devices */
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 07/35] dm: core: Move ofnode-writing test to ofnode
This fits better in the ofnode tests, so move it.
Signed-off-by: Simon Glass
---
(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
#include
#include
+#include
+#include
#include
+#include
#include
+#include
#include
#include
@@ -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
#include
#include
-#include
#include
#include
#include
@@ -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;
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 09/35] dm: core: Tidy up ofnode-writing test
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
---
(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);
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 10/35] dm: core: Prepare for updating the device tree with ofnode
Add some documentation and a new flag so that we can safely enabled using
the ofnode interface to write to the device tree.
Signed-off-by: Simon Glass
---
(no changes since v1)
doc/develop/driver-model/livetree.rst | 26 ++
include/test/test.h | 2 ++
test/test-main.c | 3 ++-
3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/doc/develop/driver-model/livetree.rst
b/doc/develop/driver-model/livetree.rst
index c29f29b205b..faf3eb5b5f0 100644
--- a/doc/develop/driver-model/livetree.rst
+++ b/doc/develop/driver-model/livetree.rst
@@ -224,6 +224,32 @@ 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.
+The ofnode_write\_...() functions also support writing to the flat tree. Care
+should be taken however, since this can change the position of node names and
+properties in the flat tree, thus affecting the live tree. Generally this does
+not matter, since when we fire up the live tree we don't ever use the flat tree
+again. But in the case of tests, this can cause a problem.
+
+The sandbox tests typically run with OF_LIVE enabled but with the actual live
+tree either present or absent. This is to make sure that the flat tree
functions
+work correctly even with OF_LIVE is enabled. But if a test modifies the flat
+device tree, then the live tree can become invalid. Any live tree tests that
run
+after that point will use a corrupted tree, e.g. with an incorrect property
name
+or worse. To deal with this we use a flag UT_TESTF_LIVE_OR_FLAT then ensures
+that tests which write to the flat tree are not run if OF_LIVE is enabled. Only
+the live tree version of the test is run, when OF_LIVE is enabled, with
+sandbox_flattree running the flat tree version.
+
+This is of course a work-around, even if a reasonable one. One solution to this
+problem would be to make a copy of the flat tree before the test and restore it
+afterwards, in the same memory location, so that the live tree pointers work
+again. Another would be to regenerate the live tree if a test modified the flat
+tree.
+
+Neither of these solutions is currently implemented, since the situation that
+causes the problem can only occur in sandbox tests, is somewhat esoteric and
+the UT_TESTF_LIVE_OR_FLAT flag deals with it in a reasonable way.
+
Multiple livetrees
--
diff --git a/include/test/test.h b/include/test/test.h
index 0104e189f63..c888d68b1ed 100644
--- a/include/test/test.h
+++ b/include/test/test.h
@@ -46,6 +46,8 @@ enum {
UT_TESTF_CONSOLE_REC= BIT(5), /* needs console recording */
/* do extra driver model init and uninit */
UT_TESTF_DM = BIT(6),
+ /* live or flat device tree, but not both in the same executable */
+ UT_TESTF_LIVE_OR_FLAT = BIT(4),
};
/**
diff --git a/test/test-main.c b/test/test-main.c
index ee38d1faea8..c0d0378c5d8 100644
--- a/test/test-main.c
+++ b/test/test-main.c
@@ -338,7 +338,8 @@ static int ut_run_test_live_flat(struct unit_test_state
*uts,
/* Run with the live tree if possible */
runs = 0;
if (CONFIG_IS_ENABLED(OF_LIVE)) {
- if (!(test->flags & UT_TESTF_FLAT_TREE)) {
+ if (!(test->flags &
+ (UT_TESTF_FLAT_TREE | UT_TESTF_LIVE_OR_FLAT))) {
uts->of_live = true;
ut_assertok(ut_run_test(uts, test, test->name));
runs++;
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 12/35] dm: core: Add support for writing u32 with ofnode
Add a new function to write an integer to an ofnode (live tree or
flat tree).
Signed-off-by: Simon Glass
---
(no changes since v1)
drivers/core/ofnode.c | 15 +++
include/dm/ofnode.h | 10 ++
test/dm/ofnode.c | 16
3 files changed, 41 insertions(+)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index b7a55589a1c..45ea84e9fb8 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -1126,6 +1126,21 @@ int ofnode_write_string(ofnode node, const char
*propname, const char *value)
return ofnode_write_prop(node, propname, value, strlen(value) + 1);
}
+int ofnode_write_u32(ofnode node, const char *propname, u32 value)
+{
+ fdt32_t *val;
+
+ assert(ofnode_valid(node));
+
+ log_debug("%s = %x", propname, value);
+ val = malloc(sizeof(*val));
+ if (!val)
+ return -ENOMEM;
+ *val = cpu_to_fdt32(value);
+
+ return ofnode_write_prop(node, propname, val, sizeof(value));
+}
+
int ofnode_set_enabled(ofnode node, bool value)
{
assert(ofnode_valid(node));
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 16c8890b097..7ce1e4c6d91 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -1154,6 +1154,16 @@ int ofnode_write_prop(ofnode node, const char *propname,
const void *value,
*/
int ofnode_write_string(ofnode node, const char *propname, const char *value);
+/**
+ * ofnode_write_u32() - Set an integer property of an ofnode
+ *
+ * @node: The node for whose string property should be set
+ * @propname: The name of the string property to set
+ * @value: The new value of the 32-bit integer property
+ * Return: 0 if successful, -ve on error
+ */
+int ofnode_write_u32(ofnode node, const char *propname, u32 value);
+
/**
* ofnode_set_enabled() - Enable or disable a device tree node given by its
* ofnode
diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c
index bd598d23e44..f80993f8927 100644
--- a/test/dm/ofnode.c
+++ b/test/dm/ofnode.c
@@ -585,3 +585,19 @@ static int dm_test_ofnode_livetree_writing(struct
unit_test_state *uts)
}
DM_TEST(dm_test_ofnode_livetree_writing,
UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT | UT_TESTF_LIVE_OR_FLAT);
+
+static int dm_test_ofnode_u32(struct unit_test_state *uts)
+{
+ ofnode node;
+
+ node = ofnode_path("/lcd");
+ ut_assert(ofnode_valid(node));
+ ut_asserteq(1366, ofnode_read_u32_default(node, "xres", 123));
+ ut_assertok(ofnode_write_u32(node, "xres", 1367));
+ ut_asserteq(1367, ofnode_read_u32_default(node, "xres", 123));
+ ut_assertok(ofnode_write_u32(node, "xres", 1366));
+
+ return 0;
+}
+DM_TEST(dm_test_ofnode_u32,
+ UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT | UT_TESTF_LIVE_OR_FLAT);
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 14/35] bootstd: Drop delays in the tests
Some tests go as far as booting a distribution. In this case a menu is
presented to the user, with a two-second timeout. This adds a total of
12 seconds to the test runs at present.
Avoid this by inserting a response using the console-recording feature.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/boot/bootflow.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 1ebb789e97b..a2ed8ac774f 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -16,6 +16,22 @@
#include
#include "bootstd_common.h"
+static int inject_response(struct unit_test_state *uts)
+{
+ /*
+* The image being booted presents a menu of options:
+*
+* Fedora-Workstation-armhfp-31-1.9 Boot Options.
+* 1: Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl)
+* Enter choice:
+*
+* Provide input for this, to avoid waiting two seconds for a timeout.
+*/
+ ut_asserteq(2, console_in_puts("1\n"));
+
+ return 0;
+}
+
/* Check 'bootflow scan/list' commands */
static int bootflow_cmd(struct unit_test_state *uts)
{
@@ -188,6 +204,7 @@ BOOTSTD_TEST(bootflow_cmd_info, UT_TESTF_DM |
UT_TESTF_SCAN_FDT);
static int bootflow_scan_boot(struct unit_test_state *uts)
{
console_record_reset_enable();
+ ut_assertok(inject_response(uts));
ut_assertok(run_command("bootflow scan -b", 0));
ut_assert_nextline(
"** Booting bootflow 'mmc1.bootdev.part_1' with syslinux");
@@ -351,6 +368,8 @@ static int bootflow_iter_disable(struct unit_test_state
*uts)
ut_assertok(bootstd_test_drop_bootdev_order(uts));
bootstd_clear_glob();
+ console_record_reset_enable();
+ ut_assertok(inject_response(uts));
ut_assertok(run_command("bootflow scan -lb", 0));
/* Try to boot the bootmgr flow, which will fail */
@@ -358,6 +377,7 @@ static int bootflow_iter_disable(struct unit_test_state
*uts)
ut_assertok(bootflow_scan_first(&iter, 0, &bflow));
ut_asserteq(3, iter.num_methods);
ut_asserteq_str("sandbox", iter.method->name);
+ ut_assertok(inject_response(uts));
ut_asserteq(-ENOTSUPP, bootflow_run_boot(&iter, &bflow));
ut_assert_skip_to_line("Boot method 'sandbox' failed and will not be
retried");
@@ -382,6 +402,8 @@ static int bootflow_cmd_boot(struct unit_test_state *uts)
ut_assert_console_end();
ut_assertok(run_command("bootflow select 0", 0));
ut_assert_console_end();
+
+ ut_assertok(inject_response(uts));
ut_asserteq(1, run_command("bootflow boot", 0));
ut_assert_nextline(
"** Booting bootflow 'mmc1.bootdev.part_1' with syslinux");
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 11/35] dm: core: Allow writing to a flat tree with ofnode
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
---
(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 de
[PATCH v2 19/35] bootstd: Allow bootmeths to be marked as global
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
---
(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_BOOTE
[PATCH v2 23/35] bootstd: Support bootflows with global bootmeths
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
---
(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 bootfl
[PATCH v2 24/35] dm: core: Call dm_scan_other() when setting up for tests
At present this function is not called, so tests miss out on any devices
created by it. Add it in so that tests can rely on these extra devices.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/test-main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/test/test-main.c b/test/test-main.c
index c0d0378c5d8..31837e57a8f 100644
--- a/test/test-main.c
+++ b/test/test-main.c
@@ -228,8 +228,10 @@ static int test_pre_run(struct unit_test_state *uts,
struct unit_test *test)
uts->start = mallinfo();
- if (test->flags & UT_TESTF_SCAN_PDATA)
+ if (test->flags & UT_TESTF_SCAN_PDATA) {
ut_assertok(dm_scan_plat(false));
+ ut_assertok(dm_scan_other(false));
+ }
if (test->flags & UT_TESTF_PROBE_TEST)
ut_assertok(do_autoprobe(uts));
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 25/35] bootstd: Allow scanning for global bootmeths separately
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
---
(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++) {
+
[PATCH v2 13/35] dm: core: Support sandbox with read interface
Update the 'read' command to work correctly with sandbox. Signed-off-by: Simon Glass --- (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 #include +#include #include 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); -- 2.37.1.455.g008518b4e5-goog
[PATCH v2 15/35] bootstd: Fix comment in bootmeth test
Correct the comment at the top of this file. Signed-off-by: Simon Glass --- (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 -- 2.37.1.455.g008518b4e5-goog
[PATCH v2 16/35] bootstd: Detect empty bootmeth ordering
If the ordering produces no entries, this is an error. Report it, so that
the caller doesn't try to continue with a NULL bootmeth.
This fixes a crash in the bootflow_iter test when running with the sandbox
'default' device tree, instead of the required 'test' one.
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/bootmeth-uclass.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c
index c040d5f92b2..b8ba4eca7ab 100644
--- a/boot/bootmeth-uclass.c
+++ b/boot/bootmeth-uclass.c
@@ -114,6 +114,8 @@ int bootmeth_setup_iter_order(struct bootflow_iter *iter)
}
count = upto;
}
+ if (!count)
+ return log_msg_ret("count2", -ENOENT);
iter->method_order = order;
iter->num_methods = count;
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 17/35] bootstd: Provide a bootmeth method to obtain state info
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
---
(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
#include
+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
+#include
#include
+#include
#include
#include
#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)));
+
[PATCH v2 18/35] bootstd: Tidy up var naming in bootdev_setup_iter_order()
Avoid using 'count' to mean either a count or an error, since this is
confusing. In fact, the called function never return 0, since that is an
error.
Use 'ret' instead.
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/bootdev-uclass.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c
index 1ede933c2f2..5683006c73c 100644
--- a/boot/bootdev-uclass.c
+++ b/boot/bootdev-uclass.c
@@ -604,14 +604,14 @@ int bootdev_setup_iter_order(struct bootflow_iter *iter,
struct udevice **devp)
log_debug("Expected %d bootdevs, found %d using aliases\n",
count, upto);
- count = build_order(bootstd, order, upto);
- if (count < 0) {
+ ret = build_order(bootstd, order, upto);
+ if (ret < 0) {
free(order);
- return log_msg_ret("build", count);
+ return log_msg_ret("build", ret);
}
+ iter->num_devs = ret;
iter->dev_order = order;
- iter->num_devs = count;
iter->cur_dev = 0;
dev = *order;
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 30/35] vbe: Add initial support for VBE
Create a new bootmeth for VBE along with a library to handle finding the
VBE methods.
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/Kconfig | 10
boot/Makefile | 2 +
boot/vbe.c| 119 ++
include/bootstd.h | 2 +
include/vbe.h | 48 +++
5 files changed, 181 insertions(+)
create mode 100644 boot/vbe.c
create mode 100644 include/vbe.h
diff --git a/boot/Kconfig b/boot/Kconfig
index b8adc64b9a7..094a790f3e8 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -392,6 +392,16 @@ config BOOTMETH_EFILOADER
This provides a way to try out standard boot on an existing boot flow.
+config BOOTMETH_VBE
+ bool "Bootdev support for Verified Boot for Embedded"
+ depends on FIT
+ default y
+ select BOOTMETH_GLOBAL
+ help
+ Enables support for VBE boot. This is a standard boot method which
+ supports selection of various firmware components, seleciton of an OS
to
+ boot as well as updating these using fwupd.
+
config BOOTMETH_SANDBOX
def_bool y
depends on SANDBOX
diff --git a/boot/Makefile b/boot/Makefile
index bab02be53fa..0378a9db27f 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -47,3 +47,5 @@ obj-$(CONFIG_CMD_ADTIMG) += image-android-dt.o
ifdef CONFIG_SPL_BUILD
obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o
endif
+
+obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o
diff --git a/boot/vbe.c b/boot/vbe.c
new file mode 100644
index 000..e6ee087dc24
--- /dev/null
+++ b/boot/vbe.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Verified Boot for Embedded (VBE) access functions
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * is_vbe() - Check if a device is a VBE method
+ *
+ * @dev: Device to check
+ * @return true if this is a VBE bootmth device, else false
+ */
+static bool is_vbe(struct udevice *dev)
+{
+ return !strncmp("vbe", dev->driver->name, 3);
+}
+
+int vbe_find_next_device(struct udevice **devp)
+{
+ for (uclass_find_next_device(devp);
+*devp;
+uclass_find_next_device(devp)) {
+ if (is_vbe(*devp))
+ return 0;
+ }
+
+ return 0;
+}
+
+int vbe_find_first_device(struct udevice **devp)
+{
+ uclass_find_first_device(UCLASS_BOOTMETH, devp);
+ if (*devp && is_vbe(*devp))
+ return 0;
+
+ return vbe_find_next_device(devp);
+}
+
+int vbe_list(void)
+{
+ struct bootstd_priv *std;
+ struct udevice *dev;
+ int ret;
+
+ ret = bootstd_get_priv(&std);
+ if (ret)
+ return ret;
+
+ printf("%3s %-3s %-15s %-15s %s\n", "#", "Sel", "Device", "Driver",
+ "Description");
+ printf("%3s %-3s %-15s %-15s %s\n", "---", "---", "--",
+ "--", "---");
+ for (ret = vbe_find_first_device(&dev); dev;
+ret = vbe_find_next_device(&dev)) {
+ const struct bootmeth_uc_plat *plat = dev_get_uclass_plat(dev);
+
+ printf("%3d %-3s %-15s %-15s %s\n", dev_seq(dev),
+ std->vbe_bootmeth == dev ? "*" : "", dev->name,
+ dev->driver->name, plat->desc);
+ }
+ printf("%3s %-3s %-15s %-15s %s\n", "---", "---", "--",
+ "--", "---");
+
+ return 0;
+}
+
+int vbe_select(struct udevice *dev)
+{
+ struct bootstd_priv *std;
+ int ret;
+
+ ret = bootstd_get_priv(&std);
+ if (ret)
+ return ret;
+ std->vbe_bootmeth = dev;
+
+ return 0;
+}
+
+int vbe_find_by_any(const char *name, struct udevice **devp)
+{
+ struct udevice *dev;
+ int ret, seq;
+ char *endp;
+
+ seq = simple_strtol(name, &endp, 16);
+
+ /* Select by name */
+ if (*endp) {
+ ret = uclass_get_device_by_name(UCLASS_BOOTMETH, name, &dev);
+ if (ret) {
+ printf("Cannot probe VBE bootmeth '%s' (err=%d)\n",
name,
+ ret);
+ return ret;
+ }
+
+ /* select by number */
+ } else {
+ ret = uclass_get_device_by_seq(UCLASS_BOOTMETH, seq, &dev);
+ if (ret) {
+ printf("Cannot find '%s' (err=%d)\n", name, ret);
+ return ret;
+ }
+ }
+
+ *devp = dev;
+
+ return 0;
+}
diff --git a/include/bootstd.h b/include/bootstd.h
index b002365f4f0..01be249d16e 100644
--- a/include/bootstd.h
+++ b/include/bootstd.h
@@ -26,6 +26,7 @@ struct udevice;
* @glob_head: Head for the global list of all bootflows across all bootdevs
* @bootmeth_count: Number of bootmeth devices in @bootmeth_order
* @bootmeth_order: List of bootmeth devic
[PATCH v2 20/35] bootstd: Allow EFI bootmgr to support an invalid bootflow
For most testing we don't want this bootmeth to actually do anything. For
the one test where we do, add a test hook to obtain the correct behaviour.
This will allow us to bind the device always, rather than just doing it
for this test.
Signed-off-by: Simon Glass
---
(no changes since v1)
arch/sandbox/include/asm/test.h | 11 +++
boot/bootmeth_efi_mgr.c | 32 ++--
test/boot/bootflow.c| 4
3 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h
index 015e96d53f8..53a036b3abf 100644
--- a/arch/sandbox/include/asm/test.h
+++ b/arch/sandbox/include/asm/test.h
@@ -304,4 +304,15 @@ int sandbox_cros_ec_get_pwm_duty(struct udevice *dev, uint
index, uint *duty);
*/
int sandbox_sdl_set_bpp(struct udevice *dev, enum video_log2_bpp l2bpp);
+/**
+ * sandbox_set_fake_efi_mgr_dev() - Control EFI bootmgr producing valid
bootflow
+ *
+ * This is only used for testing.
+ *
+ * @dev: efi_mgr bootmeth device
+ * @fake_dev: true to produce a valid bootflow when requested, false to produce
+ * an error
+ */
+void sandbox_set_fake_efi_mgr_dev(struct udevice *dev, bool fake_dev);
+
#endif
diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
index 08d9328af4e..2f327c1f8ce 100644
--- a/boot/bootmeth_efi_mgr.c
+++ b/boot/bootmeth_efi_mgr.c
@@ -15,6 +15,22 @@
#include
#include
+/**
+ * struct efi_mgr_priv - private info for the efi-mgr driver
+ *
+ * @fake_bootflow: Fake a valid bootflow for testing
+ */
+struct efi_mgr_priv {
+ bool fake_dev;
+};
+
+void sandbox_set_fake_efi_mgr_dev(struct udevice *dev, bool fake_dev)
+{
+ struct efi_mgr_priv *priv = dev_get_priv(dev);
+
+ priv->fake_dev = fake_dev;
+}
+
static int efi_mgr_check(struct udevice *dev, struct bootflow_iter *iter)
{
int ret;
@@ -29,13 +45,16 @@ static int efi_mgr_check(struct udevice *dev, struct
bootflow_iter *iter)
static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow)
{
- /*
-* Just assume there is something to boot since we don't have any way
-* of knowing in advance
-*/
- bflow->state = BOOTFLOWST_READY;
+ struct efi_mgr_priv *priv = dev_get_priv(dev);
- return 0;
+ if (priv->fake_dev) {
+ bflow->state = BOOTFLOWST_READY;
+ return 0;
+ }
+
+ /* To be implemented */
+
+ return -EINVAL;
}
static int efi_mgr_read_file(struct udevice *dev, struct bootflow *bflow,
@@ -84,4 +103,5 @@ U_BOOT_DRIVER(bootmeth_efi_mgr) = {
.of_match = efi_mgr_bootmeth_ids,
.ops= &efi_mgr_bootmeth_ops,
.bind = bootmeth_efi_mgr_bind,
+ .priv_auto = sizeof(struct efi_mgr_priv),
};
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index a2ed8ac774f..22eef40c0e3 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -11,6 +11,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -328,6 +330,8 @@ static int bootflow_system(struct unit_test_state *uts)
ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
ut_assertok(device_bind_driver(bootstd, "bootmeth_efi_mgr", "bootmgr",
&dev));
+ ut_assertok(device_probe(dev));
+ sandbox_set_fake_efi_mgr_dev(dev, true);
/* Add the system bootdev that it uses */
ut_assertok(device_bind_driver(bootstd, "system_bootdev",
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 21/35] bootstd: Allow the bootdev to be optional in bootflows
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
---
(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);
}
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 22/35] bootstd: Tidy comments in bootflow_scan_bootdev()
Fix a few nits in this function comment. Signed-off-by: Simon Glass --- (no changes since v1) include/bootflow.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/bootflow.h b/include/bootflow.h index c30ba042a48..4fa482a6784 100644 --- a/include/bootflow.h +++ b/include/bootflow.h @@ -169,9 +169,9 @@ int bootflow_iter_drop_bootmeth(struct bootflow_iter *iter, * If @flags includes BOOTFLOWF_ALL then bootflows with errors are returned too * * @dev: Boot device to scan, NULL to work through all of them until it - * finds one that * can supply a bootflow + * finds one that can supply a bootflow * @iter: Place to store private info (inited by this call) - * @flags: Flags for bootdev (enum bootflow_flags_t) + * @flags: Flags for iterator (enum bootflow_flags_t) * @bflow: Place to put the bootflow if found * Return: 0 if found, -ENODEV if no device, other -ve on other error * (iteration can continue) -- 2.37.1.455.g008518b4e5-goog
[PATCH v2 26/35] bootstd: Always create the EFI bootmgr bootmeth
Now that we can separate this out from the normal bootmeths, update the
code to create it always.
We cannot rely on the device tree to create this, since the EFI project
is quite opposed to having anything in the device tree that helps U-Boot
with its processing.
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/bootstd-uclass.c | 7 +--
test/boot/bootflow.c | 22 +++---
2 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/boot/bootstd-uclass.c b/boot/bootstd-uclass.c
index 3c6c32ae604..5107b6d4c7f 100644
--- a/boot/bootstd-uclass.c
+++ b/boot/bootstd-uclass.c
@@ -133,12 +133,7 @@ int dm_scan_other(bool pre_reloc_only)
return 0;
for (i = 0; i < n_ents; i++, drv++) {
- /*
-* Disable EFI Manager for now as no one uses it so it is
-* confusing
-*/
- if (drv->id == UCLASS_BOOTMETH &&
- strcmp("efi_mgr_bootmeth", drv->name)) {
+ if (drv->id == UCLASS_BOOTMETH) {
const char *name = drv->name;
if (!strncmp("bootmeth_", name, 9))
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 07b0517718e..b2f77972d1f 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -324,34 +324,26 @@ BOOTSTD_TEST(bootflow_iter, UT_TESTF_DM |
UT_TESTF_SCAN_FDT);
/* Check using the system bootdev */
static int bootflow_system(struct unit_test_state *uts)
{
- struct udevice *bootstd, *dev;
+ struct udevice *dev;
- /* Add the EFI bootmgr driver */
- ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
- ut_assertok(device_bind_driver(bootstd, "bootmeth_efi_mgr", "bootmgr",
- &dev));
- ut_assertok(device_probe(dev));
+ ut_assertok(uclass_get_device_by_name(UCLASS_BOOTMETH, "efi_mgr",
+ &dev));
sandbox_set_fake_efi_mgr_dev(dev, true);
- /* 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));
-
/* We should get a single 'bootmgr' method right at the end */
bootstd_clear_glob();
console_record_reset_enable();
ut_assertok(run_command("bootflow scan -l", 0));
ut_assert_skip_to_line(
- " 0 bootmgr ready (none) 0
");
+ " 0 efi_mgr ready (none) 0
");
ut_assert_skip_to_line("No more bootdevs");
- ut_assert_skip_to_line("(2 bootflows, 2 valid)");
+ ut_assert_skip_to_line("(6 bootflows, 6 valid)");
ut_assert_console_end();
return 0;
}
-BOOTSTD_TEST(bootflow_system, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+BOOTSTD_TEST(bootflow_system, UT_TESTF_DM | UT_TESTF_SCAN_PDATA |
+UT_TESTF_SCAN_FDT);
/* Check disabling a bootmethod if it requests it */
static int bootflow_iter_disable(struct unit_test_state *uts)
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 27/35] bootstd: Drop the system bootdev
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
---
(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..000
--- 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
- */
-
-#define LOG_CATEGORY UCLASS_BOOTSTD
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-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
#include
#include
-#include
#include
#include
#include
@@ -337,7 +336,7 @@ static int bootflow_system(struct unit_test_state *uts)
ut_assert_skip_to_line(
" 0 efi_mgr ready (none) 0
");
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();
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 28/35] event: Change EVENT_SPY to global
This creates static records at present, but it causes a problem with clang and LTO: the linker list records are sometimes dropped from the image. Fix this by making the records global. Update to use __used while we are here. Signed-off-by: Simon Glass --- (no changes since v1) include/event.h | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/event.h b/include/event.h index c00c4fb68dc..fb0734ed4e1 100644 --- a/include/event.h +++ b/include/event.h @@ -123,10 +123,13 @@ static inline const char *event_spy_id(struct evspy_info *spy) * The only solution I can think of is to mark linker-list entries as 'used' * using an attribute. This should be safe, since we don't actually want to drop * any of these. However this does slightly limit LTO's optimisation choices. + * + * Another issue has come up, only with clang: using 'static' makes it throw + * away the linker-list entry sometimes, e.g. with the EVT_FT_FIXUP entry in + * vbe_simple.c - so for now, make it global. */ #define EVENT_SPY(_type, _func) \ - static __attribute__((used)) ll_entry_declare(struct evspy_info, \ - _type, evspy_info) = \ + __used ll_entry_declare(struct evspy_info, _type, evspy_info) = \ _ESPY_REC(_type, _func) /** -- 2.37.1.455.g008518b4e5-goog
[PATCH v2 29/35] event: Add an event for device tree fixups
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
---
(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
#include
#include
+#include
#include
#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
+
/**
* 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 typeId Source
location
--
--
+EVT_FT_FIXUP bootmeth_vbe_simple_ft_fixupboot/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
--
2.37.1.455.g008518b4e5-goog
[PATCH v2 32/35] bootstd: Add vbe bootmeth into sandbox
Update sandbox to include the VBE bootmeth. Update a few existing tests to
take account of this change, specifically that the new bootmeth now
appears when scanning.
Signed-off-by: Simon Glass
---
(no changes since v1)
arch/sandbox/dts/sandbox.dtsi | 13 ++
arch/sandbox/dts/test.dts | 15 +++
test/boot/Makefile| 4 +++
test/boot/bootflow.c | 47 ---
test/boot/bootmeth.c | 25 ---
5 files changed, 96 insertions(+), 8 deletions(-)
diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index aa22b8765c8..56e6b38bfa7 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -12,6 +12,19 @@
chosen {
stdout-path = "/serial";
+
+ fwupd {
+ compatible = "simple-bus";
+ firmware {
+ compatible = "fwupd,vbe-simple";
+ cur-version = "1.2.3";
+ bootloader-version = "2022.01";
+ storage = "mmc1";
+ area-start = <0x0>;
+ area-size = <0x100>;
+ skip-offset = <0x8000>;
+ };
+ };
};
audio: audio-codec {
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index d1a8cc7bfb7..2761588f0da 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -1387,6 +1387,21 @@
compatible = "denx,u-boot-fdt-test";
reg = <9 1>;
};
+
+ fwupd {
+ compatible = "simple-bus";
+ firmware0 {
+ compatible = "fwupd,vbe-simple";
+ storage = "mmc1";
+ area-start = <0x400>;
+ area-size = <0x1000>;
+ skip-offset = <0x200>;
+ state-offset = <0x400>;
+ state-size = <0x40>;
+ version-offset = <0x800>;
+ version-size = <0x100>;
+ };
+ };
};
translation-test@8000 {
diff --git a/test/boot/Makefile b/test/boot/Makefile
index 1730792b5fa..9e9d5ae21f3 100644
--- a/test/boot/Makefile
+++ b/test/boot/Makefile
@@ -3,3 +3,7 @@
# Copyright 2021 Google LLC
obj-$(CONFIG_BOOTSTD) += bootdev.o bootstd_common.o bootflow.o bootmeth.o
+
+ifdef CONFIG_OF_LIVE
+obj-$(CONFIG_BOOTMETH_VBE_SIMPLE) += vbe_simple.o
+endif
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 39c2c33dead..d2c42e8b060 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -90,7 +91,7 @@ static int bootflow_cmd_glob(struct unit_test_state *uts)
ut_assertok(bootstd_test_drop_bootdev_order(uts));
console_record_reset_enable();
- ut_assertok(run_command("bootflow scan -l", 0));
+ ut_assertok(run_command("bootflow scan -lG", 0));
ut_assert_nextline("Scanning for bootflows in all bootdevs");
ut_assert_nextline("Seq Method State UclassPart Name
Filename");
ut_assert_nextlinen("---");
@@ -122,7 +123,7 @@ static int bootflow_cmd_scan_e(struct unit_test_state *uts)
ut_assertok(bootstd_test_drop_bootdev_order(uts));
console_record_reset_enable();
- ut_assertok(run_command("bootflow scan -ale", 0));
+ ut_assertok(run_command("bootflow scan -aleG", 0));
ut_assert_nextline("Scanning for bootflows in all bootdevs");
ut_assert_nextline("Seq Method State UclassPart Name
Filename");
ut_assert_nextlinen("---");
@@ -233,7 +234,7 @@ static int bootflow_iter(struct unit_test_state *uts)
/* The first device is mmc2.bootdev which has no media */
ut_asserteq(-EPROTONOSUPPORT,
- bootflow_scan_first(&iter, BOOTFLOWF_ALL, &bflow));
+ bootflow_scan_first(&iter, BOOTFLOWF_ALL |
BOOTFLOWF_SKIP_GLOBAL, &bflow));
ut_asserteq(2, iter.num_methods);
ut_asserteq(0, iter.cur_method);
ut_asserteq(0, iter.part);
@@ -242,7 +243,7 @@ static int bootflow_iter(struct unit_test_state *uts)
ut_asserteq(0, bflow.err);
/*
-* This shows MEDIA even though there is none, since int
+* This shows MEDIA even though there is none, since in
* bootdev_find_in_blk() we call part_get_info() which returns
* -EPROTONOSUPPORT. Ideally it would return -EEOPNOTSUPP and we would
* know.
@@ -384,6 +385,44 @@ static int bootflow_iter_disable(struct unit_test_state
*uts)
}
BOOT
[PATCH v2 31/35] vbe: Support VBE simple
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
---
(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 000..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
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+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 = 0x,
+
+ 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);
+
+
[PATCH v2 33/35] bootstd: Update documentation
Add some documentation updates, particularly about global bootmeths. Signed-off-by: Simon Glass --- (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 th
[PATCH v2 34/35] bootstd: Check building without global bootmeths
Use the sandbox_flattree build to check that everything works correctly
with BOOTMETH_GLOBAL disabled.
Update the tests as needed.
Signed-off-by: Simon Glass
---
(no changes since v1)
configs/sandbox_flattree_defconfig | 2 ++
test/boot/bootflow.c | 7 +++
test/boot/bootmeth.c | 24 +---
3 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/configs/sandbox_flattree_defconfig
b/configs/sandbox_flattree_defconfig
index a71ce77c403..6d62feeb08a 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -11,6 +11,7 @@ CONFIG_DISTRO_DEFAULTS=y
CONFIG_FIT=y
CONFIG_FIT_SIGNATURE=y
CONFIG_FIT_VERBOSE=y
+# CONFIG_BOOTMETH_VBE is not set
CONFIG_BOOTSTAGE=y
CONFIG_BOOTSTAGE_REPORT=y
CONFIG_BOOTSTAGE_FDT=y
@@ -206,6 +207,7 @@ CONFIG_RSA_VERIFY_WITH_PKEY=y
CONFIG_TPM=y
CONFIG_LZ4=y
CONFIG_ERRNO_STR=y
+# CONFIG_CMD_BOOTEFI_BOOTMGR is not set
CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
CONFIG_EFI_CAPSULE_ON_DISK=y
CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index d2c42e8b060..85305234e01 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -12,7 +12,9 @@
#include
#include
#include
+#ifdef CONFIG_SANDBOX
#include
+#endif
#include
#include
#include
@@ -321,6 +323,7 @@ static int bootflow_iter(struct unit_test_state *uts)
}
BOOTSTD_TEST(bootflow_iter, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+#if defined(CONFIG_SANDBOX) && defined(CONFIG_BOOTMETH_GLOBAL)
/* Check using the system bootdev */
static int bootflow_system(struct unit_test_state *uts)
{
@@ -344,6 +347,7 @@ static int bootflow_system(struct unit_test_state *uts)
}
BOOTSTD_TEST(bootflow_system, UT_TESTF_DM | UT_TESTF_SCAN_PDATA |
UT_TESTF_SCAN_FDT);
+#endif
/* Check disabling a bootmethod if it requests it */
static int bootflow_iter_disable(struct unit_test_state *uts)
@@ -388,6 +392,9 @@ BOOTSTD_TEST(bootflow_iter_disable, UT_TESTF_DM |
UT_TESTF_SCAN_FDT);
/* Check 'bootflow scan' with a bootmeth ordering including a global bootmeth
*/
static int bootflow_scan_glob_bootmeth(struct unit_test_state *uts)
{
+ if (!IS_ENABLED(CONFIG_BOOTMETH_GLOBAL))
+ return 0;
+
ut_assertok(bootstd_test_drop_bootdev_order(uts));
/*
diff --git a/test/boot/bootmeth.c b/test/boot/bootmeth.c
index ae216293d64..fb627313396 100644
--- a/test/boot/bootmeth.c
+++ b/test/boot/bootmeth.c
@@ -23,9 +23,11 @@ static int bootmeth_cmd_list(struct unit_test_state *uts)
ut_assert_nextlinen("---");
ut_assert_nextline("00 syslinuxSyslinux boot from
a block device");
ut_assert_nextline("11 efi EFI boot from an
.efi file");
- ut_assert_nextline(" glob2 firmware0 VBE simple");
+ if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL))
+ ut_assert_nextline(" glob2 firmware0 VBE
simple");
ut_assert_nextlinen("---");
- ut_assert_nextline("(3 bootmeths)");
+ ut_assert_nextline(IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) ?
+"(3 bootmeths)" : "(2 bootmeths)");
ut_assert_console_end();
return 0;
@@ -57,9 +59,11 @@ static int bootmeth_cmd_order(struct unit_test_state *uts)
ut_assert_nextlinen("---");
ut_assert_nextline("00 syslinuxSyslinux boot from
a block device");
ut_assert_nextline("-1 efi EFI boot from an
.efi file");
- ut_assert_nextline(" glob2 firmware0 VBE simple");
+ if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL))
+ ut_assert_nextline(" glob2 firmware0 VBE
simple");
ut_assert_nextlinen("---");
- ut_assert_nextline("(3 bootmeths)");
+ ut_assert_nextline(IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) ?
+"(3 bootmeths)" : "(2 bootmeths)");
ut_assert_console_end();
/* Check the -a flag with the reverse order */
@@ -70,9 +74,11 @@ static int bootmeth_cmd_order(struct unit_test_state *uts)
ut_assert_nextlinen("---");
ut_assert_nextline("10 syslinuxSyslinux boot from
a block device");
ut_assert_nextline("01 efi EFI boot from an
.efi file");
- ut_assert_nextline(" glob2 firmware0 VBE simple");
+ if (IS_ENABLED(CONFIG_BOOTMETH_GLOBAL))
+ ut_assert_nextline(" glob2 firmware0 VBE
simple");
ut_assert_nextlinen("---");
- ut_assert_nextline("(3 bootmeths)");
+ ut_assert_nextline(IS_ENABLED(CONFIG_BOOTMETH_GLOBAL) ?
+"(3 bootmeths)" : "(2 bootmeths)");
ut_assert_console_end();
/* Now reset the order to empty, which should show all of them again */
@@ -80,7 +86,8 @@ static int bootmeth_cmd_order(struct unit_test_state *uts)
ut_assert_console_end();
ut_assertnull(env_get(
[PATCH v2 35/35] vbe: Add a new vbe command
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
---
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 000..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
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+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 000..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
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#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_VERNUM0x00010002
+
+/* Basic test of reading nvdata and updating a fwupd node
[MVEBU] SPI flash offset was depecrated?
Hi Stefan and Pali, Sorry the question might be a bit trivial, but I could not find it in the source tree. It's been a long time and I'm not up to date on this subject. I recall a long time ago we needed to specify a SPI flash offset for the u-boot image so SPL would know where to load it from. I vaguely remember something about "return to BootROM and let the BootROM load the u-boot image"? Is this applicable only when we kwboot the u-boot-spl.kwb image, or also applicable to the u-boot payload on SPI flash? I saw Stefan flashing instruction in board/Marvell/db-88f6820-gp/README Update from original Marvell U-Boot to mainline U-Boot: --- The resulting image including the SPL binary with the full DDR setup is "u-boot-spl.kwb". To update the SPI NOR flash, please use the following command: => sf probe; tftpboot 200 db-88f6820-gp/u-boot-spl.kwb;\ sf update 200 0 6 ... But building this board I got 642K u-boot-spl.kwb. So perhaps the instructions above are out of date, or 6 meant to be just an example to be adjusted to the real image size? Could you point me in the right direction, ie. which Kconfig option deals with this at the moment, or is it automatically set when we build MVEBU -boot-spl.kwb? Thanks, Tony
Re: [MVEBU] SPI flash offset was depecrated?
On Saturday 30 July 2022 15:13:05 Tony Dinh wrote: > Hi Stefan and Pali, > > Sorry the question might be a bit trivial, but I could not find it in > the source tree. It's been a long time and I'm not up to date on this > subject. I recall a long time ago we needed to specify a SPI flash > offset for the u-boot image so SPL would know where to load it from. Hello! Now upstream U-Boot build process does not need to know offset where the proper U-Boot starts in SPI flash. SPL image determinate it at runtime (by looking into kwbimage header stored in SPI flash) and load proper U-Boot correctly. > I vaguely remember something about "return to BootROM and let the > BootROM load the u-boot image"? Is this applicable only when we kwboot > the u-boot-spl.kwb image, or also applicable to the u-boot payload on > SPI flash? Now upstream SPL can load proper U-Boot from SPI flash directly (if SPI flash driver is compiled into SPL) or it can "return to BootROM" and let BootROM to load proper U-Boot into RAM. This is configurable by defconfig options at compile time. We have tested that both options work fine. Using BootROM for loading U-Boot from SPI flash is slower than using dedicated SPL drivers (probably because BootROM read SPI at lower speed). But SPI flash drivers increase SPL binary (usage of BootROM does not). So you can choose which option you want -- either smaller SPL binary or faster U-Boot loading and booting. To disable SPI flash drivers in SPL, just disable option CONFIG_SPL_SPI. UART booting requires BootROM for loading proper U-Boot. But now upstream U-Boot generates universal u-boot-spl.kwb image which can be either flashed into SPI flash or booted via kwboot tool. You should _always_ flash u-boot-spl.kwb image into SPI flash memory. It is combined image of SPL and proper U-Boot with Marvell kwbimage header (required by BootROM). So never flash u-boot.bin or spl.bin separately. > I saw Stefan flashing instruction in board/Marvell/db-88f6820-gp/README > > Update from original Marvell U-Boot to mainline U-Boot: > --- > The resulting image including the SPL binary with the full DDR setup > is "u-boot-spl.kwb". > To update the SPI NOR flash, please use the following command: > => sf probe; tftpboot 200 db-88f6820-gp/u-boot-spl.kwb;\ > sf update 200 0 6 > ... > > But building this board I got 642K u-boot-spl.kwb. So perhaps the > instructions above are out of date, or 6 meant to be just an > example to be adjusted to the real image size? 6 should be there just "example" and you should replace it by real image size. > Could you point me in the right direction, ie. which Kconfig option > deals with this at the moment, or is it automatically set when we > build MVEBU -boot-spl.kwb? > > Thanks, > Tony Default Kconfig options for u-boot-spl.kwb should produce flashable SPI NOR image. It is option CONFIG_MVEBU_SPL_BOOT_DEVICE_SPI.
Re: [MVEBU] SPI flash offset was depecrated?
Hi Pali, "flash u-boot.bin or spl.bin separately" That's what we used to do many years ago! glad that we have a single flashable image now, much more robust. Thanks for a clear and detailed explanation! Tony On Sat, Jul 30, 2022 at 3:41 PM Pali Rohár wrote: > > On Saturday 30 July 2022 15:13:05 Tony Dinh wrote: > > Hi Stefan and Pali, > > > > Sorry the question might be a bit trivial, but I could not find it in > > the source tree. It's been a long time and I'm not up to date on this > > subject. I recall a long time ago we needed to specify a SPI flash > > offset for the u-boot image so SPL would know where to load it from. > > Hello! Now upstream U-Boot build process does not need to know offset > where the proper U-Boot starts in SPI flash. SPL image determinate it at > runtime (by looking into kwbimage header stored in SPI flash) and load > proper U-Boot correctly. > > > I vaguely remember something about "return to BootROM and let the > > BootROM load the u-boot image"? Is this applicable only when we kwboot > > the u-boot-spl.kwb image, or also applicable to the u-boot payload on > > SPI flash? > > Now upstream SPL can load proper U-Boot from SPI flash directly (if SPI > flash driver is compiled into SPL) or it can "return to BootROM" and let > BootROM to load proper U-Boot into RAM. > > This is configurable by defconfig options at compile time. We have > tested that both options work fine. Using BootROM for loading U-Boot > from SPI flash is slower than using dedicated SPL drivers (probably > because BootROM read SPI at lower speed). But SPI flash drivers increase > SPL binary (usage of BootROM does not). So you can choose which option > you want -- either smaller SPL binary or faster U-Boot loading and > booting. To disable SPI flash drivers in SPL, just disable option > CONFIG_SPL_SPI. > > UART booting requires BootROM for loading proper U-Boot. But now > upstream U-Boot generates universal u-boot-spl.kwb image which can be > either flashed into SPI flash or booted via kwboot tool. > > You should _always_ flash u-boot-spl.kwb image into SPI flash memory. It > is combined image of SPL and proper U-Boot with Marvell kwbimage header > (required by BootROM). So never flash u-boot.bin or spl.bin separately. > > > I saw Stefan flashing instruction in board/Marvell/db-88f6820-gp/README > > > > Update from original Marvell U-Boot to mainline U-Boot: > > --- > > The resulting image including the SPL binary with the full DDR setup > > is "u-boot-spl.kwb". > > To update the SPI NOR flash, please use the following command: > > => sf probe; tftpboot 200 db-88f6820-gp/u-boot-spl.kwb;\ > > sf update 200 0 6 > > ... > > > > But building this board I got 642K u-boot-spl.kwb. So perhaps the > > instructions above are out of date, or 6 meant to be just an > > example to be adjusted to the real image size? > > 6 should be there just "example" and you should replace it by real > image size. > > > Could you point me in the right direction, ie. which Kconfig option > > deals with this at the moment, or is it automatically set when we > > build MVEBU -boot-spl.kwb? > > > > Thanks, > > Tony > > Default Kconfig options for u-boot-spl.kwb should produce flashable > SPI NOR image. It is option CONFIG_MVEBU_SPL_BOOT_DEVICE_SPI.
Re: [MVEBU] SPI flash offset was depecrated?
Hi Pali,
Flashed to SPI and booted successfully! With CONFIG_SPL_SPI=y so SPL
loads u-boot payload.
My observation: dm-preloc is also required using SPL SPI driver method:
&spi0 {
u-boot,dm-pre-reloc;
};
Perhaps it should be implied in Kconfig with a DM_xxx symbol if possible.
Thanks,
Tony
On Sat, Jul 30, 2022 at 4:04 PM Tony Dinh wrote:
>
> Hi Pali,
>
> "flash u-boot.bin or spl.bin separately"
>
> That's what we used to do many years ago! glad that we have a single
> flashable image now, much more robust. Thanks for a clear and detailed
> explanation!
>
> Tony
>
> On Sat, Jul 30, 2022 at 3:41 PM Pali Rohár wrote:
> >
> > On Saturday 30 July 2022 15:13:05 Tony Dinh wrote:
> > > Hi Stefan and Pali,
> > >
> > > Sorry the question might be a bit trivial, but I could not find it in
> > > the source tree. It's been a long time and I'm not up to date on this
> > > subject. I recall a long time ago we needed to specify a SPI flash
> > > offset for the u-boot image so SPL would know where to load it from.
> >
> > Hello! Now upstream U-Boot build process does not need to know offset
> > where the proper U-Boot starts in SPI flash. SPL image determinate it at
> > runtime (by looking into kwbimage header stored in SPI flash) and load
> > proper U-Boot correctly.
> >
> > > I vaguely remember something about "return to BootROM and let the
> > > BootROM load the u-boot image"? Is this applicable only when we kwboot
> > > the u-boot-spl.kwb image, or also applicable to the u-boot payload on
> > > SPI flash?
> >
> > Now upstream SPL can load proper U-Boot from SPI flash directly (if SPI
> > flash driver is compiled into SPL) or it can "return to BootROM" and let
> > BootROM to load proper U-Boot into RAM.
> >
> > This is configurable by defconfig options at compile time. We have
> > tested that both options work fine. Using BootROM for loading U-Boot
> > from SPI flash is slower than using dedicated SPL drivers (probably
> > because BootROM read SPI at lower speed). But SPI flash drivers increase
> > SPL binary (usage of BootROM does not). So you can choose which option
> > you want -- either smaller SPL binary or faster U-Boot loading and
> > booting. To disable SPI flash drivers in SPL, just disable option
> > CONFIG_SPL_SPI.
> >
> > UART booting requires BootROM for loading proper U-Boot. But now
> > upstream U-Boot generates universal u-boot-spl.kwb image which can be
> > either flashed into SPI flash or booted via kwboot tool.
> >
> > You should _always_ flash u-boot-spl.kwb image into SPI flash memory. It
> > is combined image of SPL and proper U-Boot with Marvell kwbimage header
> > (required by BootROM). So never flash u-boot.bin or spl.bin separately.
> >
> > > I saw Stefan flashing instruction in board/Marvell/db-88f6820-gp/README
> > >
> > > Update from original Marvell U-Boot to mainline U-Boot:
> > > ---
> > > The resulting image including the SPL binary with the full DDR setup
> > > is "u-boot-spl.kwb".
> > > To update the SPI NOR flash, please use the following command:
> > > => sf probe; tftpboot 200 db-88f6820-gp/u-boot-spl.kwb;\
> > > sf update 200 0 6
> > > ...
> > >
> > > But building this board I got 642K u-boot-spl.kwb. So perhaps the
> > > instructions above are out of date, or 6 meant to be just an
> > > example to be adjusted to the real image size?
> >
> > 6 should be there just "example" and you should replace it by real
> > image size.
> >
> > > Could you point me in the right direction, ie. which Kconfig option
> > > deals with this at the moment, or is it automatically set when we
> > > build MVEBU -boot-spl.kwb?
> > >
> > > Thanks,
> > > Tony
> >
> > Default Kconfig options for u-boot-spl.kwb should produce flashable
> > SPI NOR image. It is option CONFIG_MVEBU_SPL_BOOT_DEVICE_SPI.
Re: [PATCH] Revert "fdtdec: drop needlessly convoluted CONFIG_PHANDLE_CHECK_SEQ"
Hi Tom, On Tue, 5 Jul 2022 at 10:42, Simon Glass wrote: > > Hi Rasmus, > > On Tue, 5 Jul 2022 at 07:47, Rasmus Villemoes > wrote: > > > > On 05/07/2022 11.47, Simon Glass wrote: > > > Hi Tom, > > > > > > On Sun, 3 Jul 2022 at 06:43, Tom Rini wrote: > > >> > > >> On Sun, Jul 03, 2022 at 02:32:42AM -0600, Simon Glass wrote: > > >>> Hi, > > >>> > > >>> On Sun, 3 Jul 2022 at 02:25, Simon Glass wrote: > > > > The fdt_path_offset() function is slow since it must scan the tree. > > This substantial overhead now applies to all boards. > > > > The original code may not be ideal but it is fit for purpose and is > > only > > needed on a few boards. > > > > We should revert this in time for the release. > > > > This reverts commit 26f981f295d00351b6f0c69b5317b254b2361cc0. > > > > Signed-off-by: Simon Glass > > --- > > > > configs/am65x_evm_a53_defconfig | 1 + > > configs/evb-ast2600_defconfig| 1 + > > configs/sama7g5ek_mmc1_defconfig | 1 + > > configs/sama7g5ek_mmc_defconfig | 1 + > > lib/Kconfig | 7 +++ > > lib/fdtdec.c | 7 +-- > > 6 files changed, 16 insertions(+), 2 deletions(-) > > >>> > > >>> Please also see the context here: > > >>> > > >>> https://patchwork.ozlabs.org/project/uboot/patch/2020142605.17034-1-a-govindr...@ti.com/ > > >> > > >> Previously when we've had issues of making the fast path slow, people > > >> have posted measurements of before/after in order to demonstrate the > > >> problem. Can we please get some logs of before/after and various > > >> possible solutions? Thanks. > > > > > > Well this code is not needed at all for all but four boards. > > > > Three. One board seems to enable that config for no reason at all. And > > it wouldn't work on that board, because the code was fragile and error > > prone. See my detailed explanations in the original patch thread. > > > > It does a > > > very expensive check of the DT and this can happen before relocation, > > > or is SPL. I don't have a board to test with at present, but I expect > > > it would cost 10s of milliseconds on AT91, for example. > > > > As I've said before, I prefer code which is correct out-of-the-box. I > > also prefer simpler code with less ifdefs (yeah yeah, IS_ENABLED...) and > > fewer configuration options to worry about. The three boards which have > > aliases where the leaf nodes have duplicate names also happen to enable > > some other unrelated magic config knob which happens to make the phandle > > comparison work. So at the very least the code should stop comparing > > phandles and just compare the node offsets; whether that check should be > > I don't disagree that this is a bit odd, but it is efficient. > > Another approach here would be to add better documentation since the > option doesn't quite work as advertised. > > > under a config knob can be discussed (certainly that config knob should > > not have PHANDLE in it), but, again, as I've said, it should be opt-out, > > and preferably with a build-time check that verifies that no two aliases > > point at same basenames. > > Opt-out means that everything pays the penalty, though. This is real > corner case. Arguably the device tree should be updated to avoid this > problem. > > > > > _If_ that 10s of milliseconds figure is true, there are other things one > > could do at build time. Say have some pass over the dtb which simply > > adds "u-boot,seq" properties to the target nodes of aliases, using that > > if present, or add a "back pointer" "u-boot,alias" property one could > > compare to name. > > That's a nice idea. > > The point of my revert was to get something in for the release. I > fully expect some sort of change to go in afterwards, but I don't > think people were aware of the impact of your patch (see context link > above). > > So I still favour a revert, for now. It looks like this didn't make it for the release. I'm not sure if this is causing problems. Shall I pick it up for the upcoming release? Regards, Simon
Re: [PATCH v2] spl: fit: Report fdt error for loading u-boot
Hi Bao Cheng,
On Sat, 30 Jul 2022 at 03:05, Su, Bao Cheng wrote:
>
> Commit 71551055cbdb ("spl: fit: Load devicetree when a Linux payload is
> found") made a change to not report the spl_fit_append_fdt error at all
> if next-stage image is u-boot.
>
> However for u-boot image without CONFIG_OF_EMBED, the error should be
> reported to uplevel caller. Otherwise, uplevel caller would think the
> fdt is already loaded which is obviously not true.
>
> Signed-off-by: Baocheng Su
> ---
>
> Changes in v2:
> - Fix the wrong wrapping
>
> common/spl/spl_fit.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index a35be52965..00404935cb 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -770,8 +770,12 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
> */
> if (os_takes_devicetree(spl_image->os)) {
> ret = spl_fit_append_fdt(spl_image, info, sector, &ctx);
> - if (ret < 0 && spl_image->os != IH_OS_U_BOOT)
> - return ret;
> + if (ret < 0) {
> + if (spl_image->os != IH_OS_U_BOOT)
> + return ret;
> + else if (!IS_ENABLED(CONFIG_OF_EMBED))
> + return ret;
This is a pretty unpleasant condition. I think we would be better to
report the error and let the caller figure it out.
There are no tests associated with this, so it is hard to know what is
actually going on.
If we must have this workaround, I suggest adding a Kconfig so boards
that need it can turn it on, and other boards can use normal
operation, which is to report errors.
Regards,
Simon
Re: [PATCH] bootstd: doc: Fix typos
On Fri, 29 Jul 2022 at 07:32, Paul Barker wrote: > > These typos were found while reading the docs. > > Signed-off-by: Paul Barker > --- > doc/develop/bootstd.rst | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Simon Glass Thank you. > > diff --git a/doc/develop/bootstd.rst b/doc/develop/bootstd.rst > index 5e9c0d282bb0..6488c997b65a 100644 > --- a/doc/develop/bootstd.rst > +++ b/doc/develop/bootstd.rst > @@ -21,7 +21,7 @@ feature is typically called `distro boot` (see > :doc:`distro`) because it is > a way for distributions to boot on any hardware. > > Traditionally U-Boot has relied on scripts to implement this feature. See > -disto_boodcmd_ for details. This is done because U-Boot has no native support > +distro_bootcmd_ for details. This is done because U-Boot has no native > support > for scanning devices. While the scripts work remarkably well, they can be > hard > to understand and extend, and the feature does not include tests. They are > also > making it difficult to move away from ad-hoc CONFIGs, since they are > implemented > @@ -51,7 +51,7 @@ BootLoaderSpec_ format. which looks something like this:: > initrd /initramfs-5.3.7-301.fc31.armv7hl.img > > As you can see it specifies a kernel, a ramdisk (initrd) and a directory from > -which to load devicetree files. The details are described in disto_boodcmd_. > +which to load devicetree files. The details are described in distro_bootcmd_. > > The bootflow is provided by the distro. It is not part of U-Boot. U-Boot's > job > is simply to interpret the file and carry out the instructions. This allows > @@ -632,7 +632,7 @@ Other ideas: >not need to specify things like `pxefile_addr_r` > > > -.. _disto_boodcmd: > https://github.com/u-boot/u-boot/blob/master/include/config_distro_bootcmd.h > +.. _distro_bootcmd: > https://github.com/u-boot/u-boot/blob/master/include/config_distro_bootcmd.h > .. _BootLoaderSpec: > http://www.freedesktop.org/wiki/Specifications/BootLoaderSpec/ > .. _distro_boot: https://github.com/u-boot/u-boot/blob/master/boot/distro.c > .. _bootflow_h: > https://github.com/u-boot/u-boot/blob/master/include/bootflow.h > -- > 2.25.1 >
Re: [PATCH v2] boot: allow bootmeth-distro without CONFIG_NET
On Thu, 28 Jul 2022 at 04:19, John Keeping wrote: > > Remove the dependency on CMD_PXE from BOOTMETH_DISTRO by introducing a > new hidden kconfig symbol to control whether pxe_utils is compiled, > allowing bootstd's distro method to be compiled without needing > networking support enabled. > > Signed-off-by: John Keeping > --- > v2: > - Fix MENU dependency for PXE_UTILS > > boot/Kconfig | 8 +++- > boot/Makefile | 3 +-- > cmd/Kconfig | 4 ++-- > 3 files changed, 10 insertions(+), 5 deletions(-) Reviewed-by: Simon Glass
Re: [RFCv1] doc: develop: Describe using CONFIG vs CFG for values
Hi Tom,
On Thu, 28 Jul 2022 at 07:20, Tom Rini wrote:
>
> Document how and when to use CONFIG or CFG namespace for options. There
> are times where Kconfig is not a great fit for our needs, so we want to
> use the CFG namespace instead.
>
> Signed-off-by: Tom Rini
> ---
> RFCv1:
> - This is essentially my idea on how to better handle the problem of
> CONFIG values that just don't fit in Kconfig because it makes much
> more sense to define them statically for a given SoC or calculate them
> from other values, and so on. One example here would be to revert
> c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig") and
> re-name these to CFG_SYS_.. instead. Another big example here would be
> a global search-and-replace of 's/CONFIG_HPS_/CFG_HPS_/g' as that's
> all tool-generated. Not all CONFIG_SYS_ options would get this as
> boolean choices are well handled in Kconfig, and that may not be clear
> enough in what I wrote here?
> ---
> doc/develop/build_configuration.rst | 36 +
> doc/develop/index.rst | 1 +
> 2 files changed, 37 insertions(+)
> create mode 100644 doc/develop/build_configuration.rst
I worry that CFG is confusing as it is too similar to CONFIG. Could we
have an entirely different prefix, e.g. SYS_? Also, why is a prefix
needed?
I have certainly seen things that are named CONFIG_... but are really
just SoC values. If they are addresses they should be in the
devicetree, but perhaps for early code in SPL that is not practical.
Re the revert, why? Are you worried that we are bloating Kconfig too
much with all these internal SoC settings?
What is the policy on using such things for a board? I would very much
like to avoid board-specific #defines like this.
Is your hope to drop all remaining ad-hoc CONFIGs in this way?
Regards,
Simon
Re: Replace make-fit-atf.py with binman. Was: migrate u-boot-rockchip.bin to binman and generate an image for SPI
Hi Quentin,
On Wed, 27 Jul 2022 at 04:34, Quentin Schulz
wrote:
>
> Hi Simon,
>
> On 7/26/22 21:58, Simon Glass wrote:
> > Hi Quentin,
> >
> > On Tue, 26 Jul 2022 at 03:08, Quentin Schulz
> > wrote:
> >>
> >> Hi Xavier,
> >>
> >> On 7/25/22 19:33, Xavier Drudis Ferran wrote:
> >>> El Mon, Jul 25, 2022 at 07:29:53PM +0200, Xavier Drudis Ferran deia:
>
> I copy here the rockchip-u-boot.dtsi file and then 2 patches on top of
> yours.
>
> >>>
> >>> Sorry I copied a dirty version that din't work. The patches were correct,
> >>> the dtsi wasn't.
> >>>
> >
> > [..]
> >
> >>
> +};
> +};
> +};
> +};
> simple-bin {
> filename = "u-boot-rockchip.bin";
> pad-byte = <0xff>;
> @@ -62,6 +131,7 @@
> #ifdef CONFIG_ARM64
> blob {
> filename = "u-boot.itb";
> +
> >>
> >> This is unfortunately not possible since binman parallelizes the build
> >> of images in the binman DT node. This means there is no guarantee the
> >> u-boot.itb file is generated before this section is worked on by binman.
> >> Or maybe I misunderstood the docs.
> >
> > You are correct, but this is something that has bothered me.
> >
> > At the moment we handle this by embedding the definition for one file
> > into the one that uses it. This is on the theory that there is no need
> > to actually write the embedded file, since it is a temporary file. In
> > fact binman does generate temporary files though.
> >
> > Is that good enough? If not I'd like to understand the need better,
> > before we make any changes.
> >
>
> For Rockchip, with the patch series from
> https://lore.kernel.org/u-boot/20220722113505.3875669-1-foss+ub...@0leil.net/,
> we now have two binaries:
> u-boot-rockchip.bin and u-boot-rockchip-spi.bin
>
> Both share the exact same u-boot.itb file (though at a different offset
> in the storage medium) embedded.
>
> This u-boot.itb is currently externally created via the Makefile +
> make_fit_atf.py prior to binman being called. This allows us to have a
> blob { filename = "u-boot.itb"; } in the binman DT node and that's
> enough for it to work fine.
>
> There's a desire to get rid of make_fit_atf.py in favor of binman. This
> means that we need to create this file in binman now.
>
> There's been some resistance to making binman not create idbloader.img
> image in the patch series mentioned above (it is *technically* created
> by binman but only in temporary files and then embedded in
> u-boot-rockchip*.bin). I assume the same resistance will be met for
> u-boot.itb.
>
> With how binman works currently and since we have u-boot.itb content
> embedded in at least two different images created by binman (might be
> even more once there's USB/NAND support?), we'd need to have the fit
> device tree node appear thrice (or more). One for u-boot.itb creation
> (because of people not wanting it disappear from binaries generated by
> U-Boot), one for embedding into u-boot-rockchip.bin and one for
> embedding into u-boot-rockchip-spi.bin.
> This increases the risk of not updating all fit device tree nodes at once.
> This is suboptimal in terms of build time because the image will
> effectively be created thrice (or more).
> This is also not the best for easy check of reproducibility since the
> content of the fit device tree node is technically not the same as
> u-boot.itb file (because it is the result of a different image built by
> binman).
>
> So I think the minimal implementation would be to allow to define an
> image/section (u-boot.itb) and to "#include" it inline in another
> section (where blob { filename = "u-boot.itb"; } currently is for
> u-boot-rockchip.bin and u-boot-rockchip-spi.bin). From reading the docs,
> I expected collection entry to be exactly this? But I couldn't make it work.
>
> Ideally, allowing binman to define a dependency from one image on
> another would mean we could still use the blob image/section, but
> safely, because the dependency is explicit so binman will know which
> image to build first. Phandles is what we're after on the Device Tree
> side I would assume. We could have something like: blob { image =
> <&u-boot-itb>; } for example. No idea how binman would create this
> dependency tree though :)
>
> Straight from my brain, lemme know if something needs to be clarified or
> rephrased.
Collections should allow this. Can you try running with threading
disabled (-T0)?
Another thought is that we could provide a way for binman to write a
section to a file in an official way, i.e. give it a filename
property.
Regards,
Simon
Re: [PATCH v2 0/7] Add support for cyclic function execution infrastruture
Hi Stefan, On Thu, 28 Jul 2022 at 01:09, Stefan Roese wrote: > > This patchset adds the basic infrastructure to periodically execute > code, e.g. all 100ms. Examples for such functions might be LED blinking > etc. The functions that are hooked into this cyclic list should be > small timewise as otherwise the execution of the other code that relies > on a high frequent polling (e.g. UART rx char ready check) might be > delayed too much. This patch also adds the Kconfig option > CONFIG_CYCLIC_MAX_CPU_TIME_US, which configures the max allowed time > for such a cyclic function. If it's execution time exceeds this time, > this cyclic function will get removed from the cyclic list. > > How is this cyclic functionality executed? > This patchset integrates the main function responsible for calling all > registered cyclic functions cyclic_run() into the common WATCHDOG_RESET > macro. This guarantees that cyclic_run() is executed very often, which > is necessary for the cyclic functions to get scheduled and executed at > their configured periods. > > This cyclic infrastructure will be used by a board specific function on > the NIC23 MIPS Octeon board, which needs to check periodically, if a > PCIe FLR has occurred. > > Ideas how to continue: > One idea is to rename WATCHDOG_RESET to something like SCHEDULE and > move the watchdog_reset call into this cyclic infrastructure as well. > Or to perhaps move the shell UART RX ready polling to a cyclic > function. > > It's also possible to extend the "cyclic" command, to support the > creation of periodically executed shell commands (for testing etc). > > Here the Azure build, without any issues: > https://dev.azure.com/sr0718/u-boot/_build/results?buildId=219&view=results > > Thanks, > Stefan > > Aaron Williams (1): > mips: octeon_nic23: Add PCIe FLR fixup via cyclic infrastructure > > Stefan Roese (6): > time: Import time_after64() and friends from Linux > cyclic: Add basic support for cyclic function execution infrastruture > cyclic: Integrate cyclic infrastructure into WATCHDOG_RESET > cyclic: Integrate cyclic functionality at bootup in board_r/f > cyclic: Add 'cyclic list' command > sandbox: Add cyclic demo function > > MAINTAINERS| 7 + > board/Marvell/octeon_nic23/board.c | 197 + > board/sandbox/sandbox.c| 15 +++ > cmd/Kconfig| 7 + > cmd/Makefile | 1 + > cmd/cyclic.c | 40 ++ > common/Kconfig | 20 +++ > common/Makefile| 1 + > common/board_f.c | 2 + > common/board_r.c | 2 + > common/cyclic.c| 112 > configs/octeon_nic23_defconfig | 3 + > configs/sandbox_defconfig | 3 + > fs/cramfs/uncompress.c | 2 +- > include/cyclic.h | 97 ++ > include/time.h | 19 +++ > include/watchdog.h | 23 +++- > 17 files changed, 547 insertions(+), 4 deletions(-) > create mode 100644 cmd/cyclic.c > create mode 100644 common/cyclic.c > create mode 100644 include/cyclic.h > > -- > 2.37.1 > This looks interesting. I like the idea of integrating the watchdog stuff too, since you are making use of it. Would it make sense to make use of the new event system, since it has static and dynamic handlers? Regards, Simon
[PATCH 0/7] fdt: Fix up tests and build warnings on newer distros
At present the code coverage for test_fdt is not working. This series fixes that and takes the opportunity to fix the pylint warnings in that file. Some newer Linux distributions are producing warnings about using distutils, so this series moves things over to setuptools. Simon Glass (7): dtoc: Tidy up fdt_tests RunTestCoverage() args dtoc: Tidy up fdt_tests RunTests() dtoc: Fix fdt test coverage dtoc: Move main program into its own function test_fdt: Convert to use argparse dtoc: Correct remaining pylint problems in test_fdt fdt: Move to setuptools scripts/dtc/README| 106 +++ scripts/dtc/pylibfdt/Makefile | 5 +- scripts/dtc/pylibfdt/setup.py | 60 ++- scripts/pylint.base | 2 +- tools/dtoc/test_fdt.py| 326 +++--- 5 files changed, 345 insertions(+), 154 deletions(-) create mode 100644 scripts/dtc/README -- 2.37.1.455.g008518b4e5-goog
[PATCH 1/7] dtoc: Tidy up fdt_tests RunTestCoverage() args
Pass the options args in rather than using the global various. Use snake
case and fix up comments to make pylint happy.
Signed-off-by: Simon Glass
---
tools/dtoc/test_fdt.py | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index 3baf4437cdd..ec257552efc 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -768,10 +768,14 @@ class TestFdtUtil(unittest.TestCase):
tools.outdir= old_outdir
-def RunTestCoverage():
-"""Run the tests and check that we get 100% coverage"""
+def run_test_coverage(build_dir):
+"""Run the tests and check that we get 100% coverage
+
+Args:
+build_dir (str): Directory containing the build output
+"""
test_util.run_test_coverage('tools/dtoc/test_fdt.py', None,
-['tools/patman/*.py', '*test_fdt.py'], options.build_dir)
+['tools/patman/*.py', '*test_fdt.py'], build_dir)
def RunTests(args):
@@ -811,4 +815,4 @@ if options.test:
ret_code = RunTests(args)
sys.exit(ret_code)
elif options.test_coverage:
-RunTestCoverage()
+run_test_coverage(options.build_dir)
--
2.37.1.455.g008518b4e5-goog
[PATCH 2/7] dtoc: Tidy up fdt_tests RunTests()
Pass the options args in rather than using the global variables. Use snake
case, fix up comments and use a ternary operator to make pylint happy.
Signed-off-by: Simon Glass
---
tools/dtoc/test_fdt.py | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index ec257552efc..b48819831d6 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -778,17 +778,20 @@ def run_test_coverage(build_dir):
['tools/patman/*.py', '*test_fdt.py'], build_dir)
-def RunTests(args):
+def run_tests(args, processes):
"""Run all the test we have for the fdt model
Args:
-args: List of positional args provided to fdt. This can hold a test
-name to execute (as in 'fdt -t testFdt', for example)
+args (list or str): List of positional args provided. This can hold a
+test name to execute (as in 'test_fdt -t testFdt', for example)
+processes (int): Number of processes to use (None means as many as
there
+are CPUs on the system. This must be set to 1 when running under
+the python3-coverage tool
Returns:
-Return code, 0 on success
+int: Return code, 0 on success
"""
-test_name = args and args[0] or None
+test_name = args[0] if args else None
result = test_util.run_test_suites(
'test_fdt', False, False, False, None, test_name, None,
[TestFdt, TestNode, TestProp, TestFdtUtil])
@@ -812,7 +815,7 @@ parser.add_option('-T', '--test-coverage',
action='store_true',
# Run our meagre tests
if options.test:
-ret_code = RunTests(args)
+ret_code = run_tests(args, options.processes)
sys.exit(ret_code)
elif options.test_coverage:
run_test_coverage(options.build_dir)
--
2.37.1.455.g008518b4e5-goog
[PATCH 3/7] dtoc: Fix fdt test coverage
Fix a bug that the --processes option was ignored, thus resulting in no
test coverage information being generated.
Signed-off-by: Simon Glass
Fixes: 42ae363ddd9 ("dtoc: Update fdt tests to use test_util")
---
tools/dtoc/test_fdt.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index b48819831d6..afa0bb58850 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -793,7 +793,7 @@ def run_tests(args, processes):
"""
test_name = args[0] if args else None
result = test_util.run_test_suites(
-'test_fdt', False, False, False, None, test_name, None,
+'test_fdt', False, False, False, processes, test_name, None,
[TestFdt, TestNode, TestProp, TestFdtUtil])
return (0 if result.wasSuccessful() else 1)
--
2.37.1.455.g008518b4e5-goog
[PATCH 4/7] dtoc: Move main program into its own function
Use a function for the main program so everything there doesn't look like
a global variable to pylint.
Signed-off-by: Simon Glass
---
tools/dtoc/test_fdt.py | 44 +++---
1 file changed, 24 insertions(+), 20 deletions(-)
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index afa0bb58850..e10fb528e81 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -799,23 +799,27 @@ def run_tests(args, processes):
return (0 if result.wasSuccessful() else 1)
-if __name__ != '__main__':
-sys.exit(1)
-
-parser = OptionParser()
-parser.add_option('-B', '--build-dir', type='string', default='b',
-help='Directory containing the build output')
-parser.add_option('-P', '--processes', type=int,
- help='set number of processes to use for running tests')
-parser.add_option('-t', '--test', action='store_true', dest='test',
- default=False, help='run tests')
-parser.add_option('-T', '--test-coverage', action='store_true',
-default=False, help='run tests and check for 100% coverage')
-(options, args) = parser.parse_args()
-
-# Run our meagre tests
-if options.test:
-ret_code = run_tests(args, options.processes)
-sys.exit(ret_code)
-elif options.test_coverage:
-run_test_coverage(options.build_dir)
+def main():
+"""Main program for this tool"""
+parser = OptionParser()
+parser.add_option('-B', '--build-dir', type='string', default='b',
+help='Directory containing the build output')
+parser.add_option('-P', '--processes', type=int,
+ help='set number of processes to use for running tests')
+parser.add_option('-t', '--test', action='store_true', dest='test',
+ default=False, help='run tests')
+parser.add_option('-T', '--test-coverage', action='store_true',
+default=False, help='run tests and check for 100%
coverage')
+(options, args) = parser.parse_args()
+
+# Run our meagre tests
+if options.test:
+ret_code = run_tests(args, options.processes)
+return ret_code
+if options.test_coverage:
+run_test_coverage(options.build_dir)
+return 0
+
+if __name__ == '__main__':
+sys.exit(main())
+sys.exit(1)
--
2.37.1.455.g008518b4e5-goog
[PATCH 5/7] test_fdt: Convert to use argparse
Drop the deprecated OptionParser.
Signed-off-by: Simon Glass
---
tools/dtoc/test_fdt.py | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index e10fb528e81..c38158edf32 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -4,7 +4,7 @@
# Written by Simon Glass
#
-from optparse import OptionParser
+from argparse import ArgumentParser
import glob
import os
import shutil
@@ -778,12 +778,11 @@ def run_test_coverage(build_dir):
['tools/patman/*.py', '*test_fdt.py'], build_dir)
-def run_tests(args, processes):
+def run_tests(names, processes):
"""Run all the test we have for the fdt model
Args:
-args (list or str): List of positional args provided. This can hold a
-test name to execute (as in 'test_fdt -t testFdt', for example)
+names (list of str): List of test names provided. Only the first is
used
processes (int): Number of processes to use (None means as many as
there
are CPUs on the system. This must be set to 1 when running under
the python3-coverage tool
@@ -791,7 +790,7 @@ def run_tests(args, processes):
Returns:
int: Return code, 0 on success
"""
-test_name = args[0] if args else None
+test_name = names[0] if names else None
result = test_util.run_test_suites(
'test_fdt', False, False, False, processes, test_name, None,
[TestFdt, TestNode, TestProp, TestFdtUtil])
@@ -801,23 +800,25 @@ def run_tests(args, processes):
def main():
"""Main program for this tool"""
-parser = OptionParser()
-parser.add_option('-B', '--build-dir', type='string', default='b',
-help='Directory containing the build output')
-parser.add_option('-P', '--processes', type=int,
- help='set number of processes to use for running tests')
-parser.add_option('-t', '--test', action='store_true', dest='test',
- default=False, help='run tests')
-parser.add_option('-T', '--test-coverage', action='store_true',
-default=False, help='run tests and check for 100%
coverage')
-(options, args) = parser.parse_args()
+parser = ArgumentParser()
+parser.add_argument('-B', '--build-dir', type=str, default='b',
+help='Directory containing the build output')
+parser.add_argument('-P', '--processes', type=int,
+help='set number of processes to use for running
tests')
+parser.add_argument('-t', '--test', action='store_true', dest='test',
+default=False, help='run tests')
+parser.add_argument('-T', '--test-coverage', action='store_true',
+default=False,
+help='run tests and check for 100% coverage')
+parser.add_argument('name', nargs='*')
+args = parser.parse_args()
# Run our meagre tests
-if options.test:
-ret_code = run_tests(args, options.processes)
+if args.test:
+ret_code = run_tests(args.name, args.processes)
return ret_code
-if options.test_coverage:
-run_test_coverage(options.build_dir)
+if args.test_coverage:
+run_test_coverage(args.build_dir)
return 0
if __name__ == '__main__':
--
2.37.1.455.g008518b4e5-goog
[PATCH 7/7] fdt: Move to setuptools
The distutils package is deprecated. The upstream libfdt repo uses
setuptools for building the pylibfdt module, so bring in that code,
suitably modified for U-Boot. Also bring in the README.
The modifications include setting the version correctly, making use of
the environment variables provided by the Makefile and various tweaks
to the directories.
Note that the version omits the minus character at the start of
EXTRAVERSION, since this creates a warning. The build is really just used
within U-Boot itself, so it doesn't matter too much if the version matches
upstream, or exactly matches U-Boot.
Signed-off-by: Simon Glass
---
scripts/dtc/README| 106 ++
scripts/dtc/pylibfdt/Makefile | 5 +-
scripts/dtc/pylibfdt/setup.py | 60 ---
3 files changed, 161 insertions(+), 10 deletions(-)
create mode 100644 scripts/dtc/README
diff --git a/scripts/dtc/README b/scripts/dtc/README
new file mode 100644
index 000..a48312a422c
--- /dev/null
+++ b/scripts/dtc/README
@@ -0,0 +1,106 @@
+The source tree contains the Device Tree Compiler (dtc) toolchain for
+working with device tree source and binary files and also libfdt, a
+utility library for reading and manipulating the binary format.
+
+DTC and LIBFDT are maintained by:
+
+David Gibson
+Jon Loeliger
+
+
+Python library
+--
+
+A Python library is also available. To build this you will need to install
+swig and Python development files. On Debian distributions:
+
+ sudo apt-get install swig python3-dev
+
+The library provides an Fdt class which you can use like this:
+
+$ PYTHONPATH=../pylibfdt python3
+>>> import libfdt
+>>> fdt = libfdt.Fdt(open('test_tree1.dtb', mode='rb').read())
+>>> node = fdt.path_offset('/subnode@1')
+>>> print(node)
+124
+>>> prop_offset = fdt.first_property_offset(node)
+>>> prop = fdt.get_property_by_offset(prop_offset)
+>>> print('%s=%s' % (prop.name, prop.as_str()))
+compatible=subnode1
+>>> node2 = fdt.path_offset('/')
+>>> print(fdt.getprop(node2, 'compatible').as_str())
+test_tree1
+
+You will find tests in tests/pylibfdt_tests.py showing how to use each
+method. Help is available using the Python help command, e.g.:
+
+$ cd pylibfdt
+$ python3 -c "import libfdt; help(libfdt)"
+
+If you add new features, please check code coverage:
+
+$ sudo apt-get install python3-coverage
+$ cd tests
+# It's just 'coverage' on most other distributions
+$ python3-coverage run pylibfdt_tests.py
+$ python3-coverage html
+# Open 'htmlcov/index.html' in your browser
+
+
+The library can be installed with pip from a local source tree:
+
+pip install . [--user|--prefix=/path/to/install_dir]
+
+Or directly from a remote git repo:
+
+pip install git+git://git.kernel.org/pub/scm/utils/dtc/dtc.git@main
+
+The install depends on libfdt shared library being installed on the host system
+first. Generally, using --user or --prefix is not necessary and pip will use
the
+default location for the Python installation which varies if the user is root
or
+not.
+
+You can also install everything via make if you like, but pip is recommended.
+
+To install both libfdt and pylibfdt you can use:
+
+make install [PREFIX=/path/to/install_dir]
+
+To disable building the python library, even if swig and Python are available,
+use:
+
+make NO_PYTHON=1
+
+
+More work remains to support all of libfdt, including access to numeric
+values.
+
+
+Adding a new function to libfdt.h
+-
+
+The shared library uses libfdt/version.lds to list the exported functions, so
+add your new function there. Check that your function works with pylibfdt. If
+it cannot be supported, put the declaration in libfdt.h behind #ifndef SWIG so
+that swig ignores it.
+
+
+Tests
+-
+
+Test files are kept in the tests/ directory. Use 'make check' to build and run
+all tests.
+
+If you want to adjust a test file, be aware that tree_tree1.dts is compiled
+and checked against a binary tree from assembler macros in trees.S. So
+if you change that file you must change tree.S also.
+
+
+Mailing list
+
+The following list is for discussion about dtc and libfdt implementation
+mailto:devicetree-compi...@vger.kernel.org
+
+Core device tree bindings are discussed on the devicetree-spec list:
+mailto:devicetree-s...@vger.kernel.org
diff --git a/scripts/dtc/pylibfdt/Makefile b/scripts/dtc/pylibfdt/Makefile
index 493995e3038..e442d5c2420 100644
--- a/scripts/dtc/pylibfdt/Makefile
+++ b/scripts/dtc/pylibfdt/Makefile
@@ -13,11 +13,14 @@ include $(LIBFDT_srcdir)/Makefile.libfdt
PYLIBFDT_srcs = $(addprefix $(LIBFDT_srcdir)/,$(LIBFDT_SRCS)) \
$(obj)/libfdt.i
+# create a version string compliant with PEP 440
+PEP_VERSION = $(VERSION)$(if $(PATCHLEVEL),.$(PATCHLEVEL)$(if
$(SUBLEVEL),.$(SUBLEVEL)))$(subst -,,$(EXTRAVERSION))
+
quiet_cmd_pymod = PYMOD $@
cmd_pymod = unset CROSS_COMPILE; unset CFLAGS; \
CC="
[PATCH 6/7] dtoc: Correct remaining pylint problems in test_fdt
Fix various camel-case and other naming problems. Update the pylint base
file to avoid regressions.
Signed-off-by: Simon Glass
---
scripts/pylint.base| 2 +-
tools/dtoc/test_fdt.py | 254 +++--
2 files changed, 142 insertions(+), 114 deletions(-)
diff --git a/scripts/pylint.base b/scripts/pylint.base
index 9ebebf47ab9..5203f2bb492 100644
--- a/scripts/pylint.base
+++ b/scripts/pylint.base
@@ -167,7 +167,7 @@ tools_binman_etype_x86_reset16_tpl -15.71
tools_binman_etype_x86_start16 -15.71
tools_binman_etype_x86_start16_spl -15.71
tools_binman_etype_x86_start16_tpl -15.71
-tools_binman_fdt_test 3.23
+tools_binman_fdt_test 10.00
tools_binman_fip_util 9.85
tools_binman_fip_util_test 10.00
tools_binman_fmap_util 6.88
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index c38158edf32..8a990b8bd77 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -1,11 +1,13 @@
#!/usr/bin/env python3
# SPDX-License-Identifier: GPL-2.0+
-# Copyright (c) 2018 Google, Inc
-# Written by Simon Glass
-#
+
+"""
+Tests for the Fdt module
+Copyright (c) 2018 Google, Inc
+Written by Simon Glass
+"""
from argparse import ArgumentParser
-import glob
import os
import shutil
import sys
@@ -22,16 +24,18 @@ sys.path.insert(2, os.path.join(our_path,
'../../scripts/dtc/pylibfdt'))
sys.path.insert(2, os.path.join(our_path,
'../../build-sandbox_spl/scripts/dtc/pylibfdt'))
+#pylint: disable=wrong-import-position
from dtoc import fdt
from dtoc import fdt_util
from dtoc.fdt_util import fdt32_to_cpu, fdt64_to_cpu
from dtoc.fdt import Type, BytesToValue
import libfdt
-from patman import command
from patman import test_util
from patman import tools
-def _GetPropertyValue(dtb, node, prop_name):
+#pylint: disable=protected-access
+
+def _get_property_value(dtb, node, prop_name):
"""Low-level function to get the property value based on its offset
This looks directly in the device tree at the property's offset to find
@@ -83,13 +87,13 @@ class TestFdt(unittest.TestCase):
def setUp(self):
self.dtb = fdt.FdtScan(find_dtb_file('dtoc_test_simple.dts'))
-def testFdt(self):
+def test_fdt(self):
"""Test that we can open an Fdt"""
self.dtb.Scan()
root = self.dtb.GetRoot()
self.assertTrue(isinstance(root, fdt.Node))
-def testGetNode(self):
+def test_get_node(self):
"""Test the GetNode() method"""
node = self.dtb.GetNode('/spl-test')
self.assertTrue(isinstance(node, fdt.Node))
@@ -103,27 +107,28 @@ class TestFdt(unittest.TestCase):
self.assertTrue(isinstance(node, fdt.Node))
self.assertEqual(0, node.Offset())
-def testFlush(self):
+def test_flush(self):
"""Check that we can flush the device tree out to its file"""
fname = self.dtb._fname
-with open(fname, 'rb') as fd:
-data = fd.read()
+with open(fname, 'rb') as inf:
+inf.read()
os.remove(fname)
with self.assertRaises(IOError):
-open(fname, 'rb')
+with open(fname, 'rb'):
+pass
self.dtb.Flush()
-with open(fname, 'rb') as fd:
-data = fd.read()
+with open(fname, 'rb') as inf:
+inf.read()
-def testPack(self):
+def test_pack(self):
"""Test that packing a device tree works"""
self.dtb.Pack()
-def testGetFdtRaw(self):
+def test_get_fdt_raw(self):
"""Tetst that we can access the raw device-tree data"""
self.assertTrue(isinstance(self.dtb.GetContents(), bytes))
-def testGetProps(self):
+def test_get_props(self):
"""Tests obtaining a list of properties"""
node = self.dtb.GetNode('/spl-test')
props = self.dtb.GetProps(node)
@@ -133,17 +138,19 @@ class TestFdt(unittest.TestCase):
'stringval', 'u-boot,dm-pre-reloc'],
sorted(props.keys()))
-def testCheckError(self):
+def test_check_error(self):
"""Tests the ChecKError() function"""
-with self.assertRaises(ValueError) as e:
+with self.assertRaises(ValueError) as exc:
fdt.CheckErr(-libfdt.NOTFOUND, 'hello')
-self.assertIn('FDT_ERR_NOTFOUND: hello', str(e.exception))
+self.assertIn('FDT_ERR_NOTFOUND: hello', str(exc.exception))
-def testGetFdt(self):
+def test_get_fdt(self):
+"""Test getting an Fdt object from a node"""
node = self.dtb.GetNode('/spl-test')
self.assertEqual(self.dtb, node.GetFdt())
-def testBytesToValue(self):
+def test_bytes_to_value(self):
+"""Test converting a string list into Python"""
self.assertEqual(BytesToValue(b'this\0is\0'),
(Type.STRING, ['this', 'is']))
@@ -163,11 +170,11 @@ class TestNode(unittest.TestCase):
self.node = self.dtb.GetNode(
