[PATCH 00/34] bootstd: Support recording images

This series provides a way to keep track of the images used in bootstd, including the type of each image.
At present this is sort-of handled by struct bootflow but in quite an ad-hoc way. The structure has become quite large and is hard to query. Future work will be able to reduce its size.
Ultimately the 'bootflow info' command may change to also show images as a list, but that is left for later, as this series is already fairly long. So for now, just introduce the concept and adjust bootstd to use it, with a simple command to list the images.
This series includes various alist enhancements, to make use of this new data structure a little easier.
Simon Glass (34): alist: Mention the error condition in alist_add_placeholder() alist: Add a comment for alist_init_struct() alist: Expand the comment for alist_get() alist: Add a way to get the next element alist: Add for-loop helpers alist: Add a function to empty the list alist: Add a way to efficiently filter an alist dm: core: Add a function to see if a device exists test: boot: Use a consistent name for the script bootmeth bootstd: Move bootflow-adding to bootstd bootstd: Move bootflow-clearing to bootstd bootstd: Add a function to get bootstd only if available bootstd: Drop the bootdev-specific list of bootflows bootstd: Move the bootflow list into an alist image: Add a new type for extlinux image: Add a new type for EFI apps image: Add a new type for logo images image: Add a new type for a command-line string test: Expand implementation of ut_list_has_dm_tests() test: Drop the duplicate line in setup_bootmenu_image() test: boot: Update bootflow_iter() for console checking bootstd: cros: Correct the x86-setup address bootstd: Maintain a list of images bootstd: Update bootmeth_alloc_file() to record images boot: pxe: Drop the duplicate comment on get_pxe_file() efi: Simplify reading files by using the common function bootmeth: Update the read_file() method to include a type efi: Check the filename-allocation in the network path boot: Update extlinux pxe_getfile_func() to include type boot: Update pxe bootmeth to record images Update bootmeth_alloc_other() to record images bootstd: Avoid showing an invalid buffer address bootstd: Update cros bootmeth to record images bootstd: Add a simple command to list images
boot/bootdev-uclass.c | 76 +++------- boot/bootflow.c | 37 +++-- boot/bootmeth-uclass.c | 54 ++++++- boot/bootmeth_android.c | 3 +- boot/bootmeth_cros.c | 33 +++- boot/bootmeth_efi.c | 16 +- boot/bootmeth_efi_mgr.c | 3 +- boot/bootmeth_extlinux.c | 7 +- boot/bootmeth_pxe.c | 21 ++- boot/bootmeth_qfw.c | 3 +- boot/bootmeth_sandbox.c | 3 +- boot/bootmeth_script.c | 7 +- boot/bootstd-uclass.c | 59 ++++++- boot/image.c | 4 + boot/pxe_utils.c | 36 ++--- boot/vbe_simple.c | 5 +- cmd/Kconfig | 9 ++ cmd/Makefile | 1 + cmd/bootdev.c | 2 +- cmd/bootflow.c | 13 +- cmd/bootstd.c | 65 ++++++++ cmd/pxe.c | 2 +- cmd/sysboot.c | 6 +- doc/develop/bootstd/overview.rst | 21 ++- doc/usage/cmd/bootstd.rst | 79 ++++++++++ doc/usage/index.rst | 1 + drivers/core/uclass.c | 11 ++ include/alist.h | 139 ++++++++++++++++- include/bootdev.h | 27 ---- include/bootflow.h | 44 ++++-- include/bootmeth.h | 20 ++- include/bootstd.h | 50 +++++- include/dm/uclass.h | 11 ++ include/image.h | 4 + include/pxe_utils.h | 14 +- lib/alist.c | 41 +++++ test/boot/bootflow.c | 110 +++++++++++++- test/dm/core.c | 22 +++ test/lib/alist.c | 253 +++++++++++++++++++++++++++++++ test/py/tests/test_ut.py | 5 +- test/test-main.c | 18 ++- 41 files changed, 1143 insertions(+), 192 deletions(-) create mode 100644 cmd/bootstd.c create mode 100644 doc/usage/cmd/bootstd.rst

Update the function comment to note that this function can return NULL if it runs out of memory.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/alist.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/alist.h b/include/alist.h index 68d268f01af..0343946bc4a 100644 --- a/include/alist.h +++ b/include/alist.h @@ -151,8 +151,9 @@ void *alist_ensure_ptr(struct alist *lst, uint index); * alist_add_placeholder() - Add a new item to the end of the list * * @lst: alist to add to - * Return: Pointer to the newly added position. Note that this is not inited so - * the caller must copy the requested struct to the returned pointer + * Return: Pointer to the newly added position, or NULL if out of memory. Note + * that this is not inited so the caller must copy the requested struct to the + * returned pointer */ void *alist_add_placeholder(struct alist *lst);

Comment this macro so that it is clear how to use it.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/alist.h | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/include/alist.h b/include/alist.h index 0343946bc4a..a727f1c7dfa 100644 --- a/include/alist.h +++ b/include/alist.h @@ -198,6 +198,12 @@ bool alist_expand_by(struct alist *lst, uint inc_by); */ bool alist_init(struct alist *lst, uint obj_size, uint alloc_size);
+/** + * alist_init_struct() - Typed version of alist_init() + * + * Use as: + * alist_init(&lst, struct my_struct); + */ #define alist_init_struct(_lst, _struct) \ alist_init(_lst, sizeof(_struct), 0)

Add a better description for this macro.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/alist.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/alist.h b/include/alist.h index a727f1c7dfa..2c78ede201e 100644 --- a/include/alist.h +++ b/include/alist.h @@ -116,7 +116,12 @@ static inline const void *alist_getd(struct alist *lst, uint index) return lst->data + index * lst->obj_size; }
-/** get an entry as a constant */ +/** + * alist_get() - get an entry as a constant + * + * Use as (to obtain element 2 of the list): + * const struct my_struct *ptr = alist_get(lst, 2, struct my_struct) + */ #define alist_get(_lst, _index, _struct) \ ((const _struct *)alist_get_ptr(_lst, _index))

Add a new function which returns the next element after the one provided, if it exists in the list.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/alist.h | 34 +++++++++++++++++++++++++++++++ lib/alist.c | 21 +++++++++++++++++++ test/lib/alist.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+)
diff --git a/include/alist.h b/include/alist.h index 2c78ede201e..97523af37a6 100644 --- a/include/alist.h +++ b/include/alist.h @@ -71,6 +71,21 @@ static inline bool alist_has(struct alist *lst, uint index) return index < lst->count; }
+/** + * alist_calc_index() - Calculate the index of an item in the list + * + * The returned element number will be -1 if the list is empty or the pointer + * pointers to before the list starts. + * + * If the pointer points to after the last item, the calculated element-number + * will be returned, even though it is greater than lst->count + * + * @lst: alist to check + * @ptr: pointer to check + * Return: element number of the pointer + */ +int alist_calc_index(const struct alist *lst, const void *ptr); + /** * alist_err() - Check if the alist is still valid * @@ -190,6 +205,25 @@ bool alist_expand_by(struct alist *lst, uint inc_by); #define alist_add(_lst, _obj) \ ((typeof(_obj) *)alist_add_ptr(_lst, &(_obj)))
+/** get next entry as a constant */ +#define alist_next(_lst, _objp) \ + ((const typeof(_objp))alist_next_ptrd(_lst, _objp)) + +/** get next entry, which can be written to */ +#define alist_nextw(_lst, _objp) \ + ((typeof(_objp))alist_next_ptrd(_lst, _objp)) + +/** + * alist_next_ptrd() - Get a pointer to the next list element + * + * This returns NULL if the requested element is beyond lst->count + * + * @lst: List to check + * @ptr: Pointer to current element (must be valid) + * Return: Pointer to next element, or NULL if @ptr is the last + */ +const void *alist_next_ptrd(const struct alist *lst, const void *ptr); + /** * alist_init() - Set up a new object list * diff --git a/lib/alist.c b/lib/alist.c index b7928cad520..7730fe0d473 100644 --- a/lib/alist.c +++ b/lib/alist.c @@ -106,6 +106,27 @@ const void *alist_get_ptr(const struct alist *lst, uint index) return lst->data + index * lst->obj_size; }
+int alist_calc_index(const struct alist *lst, const void *ptr) +{ + uint index; + + if (!lst->count || ptr < lst->data) + return -1; + + index = (ptr - lst->data) / lst->obj_size; + + return index; +} + +const void *alist_next_ptrd(const struct alist *lst, const void *ptr) +{ + int index = alist_calc_index(lst, ptr); + + assert(index != -1); + + return alist_get_ptr(lst, index + 1); +} + void *alist_ensure_ptr(struct alist *lst, uint index) { uint minsize = index + 1; diff --git a/test/lib/alist.c b/test/lib/alist.c index d41845c7e6c..96092affec9 100644 --- a/test/lib/alist.c +++ b/test/lib/alist.c @@ -240,3 +240,55 @@ static int lib_test_alist_add(struct unit_test_state *uts) return 0; } LIB_TEST(lib_test_alist_add, 0); + +/* Test alist_next() */ +static int lib_test_alist_next(struct unit_test_state *uts) +{ + const struct my_struct *ptr; + struct my_struct data, *ptr2; + struct alist lst; + ulong start; + + start = ut_check_free(); + + ut_assert(alist_init_struct(&lst, struct my_struct)); + data.val = 123; + data.other_val = 0; + alist_add(&lst, data); + + data.val = 321; + alist_add(&lst, data); + + data.val = 789; + alist_add(&lst, data); + + ptr = alist_get(&lst, 0, struct my_struct); + ut_assertnonnull(ptr); + ut_asserteq(123, ptr->val); + + ptr = alist_next(&lst, ptr); + ut_assertnonnull(ptr); + ut_asserteq(321, ptr->val); + + ptr2 = (struct my_struct *)ptr; + ptr2 = alist_nextw(&lst, ptr2); + ut_assertnonnull(ptr2); + + ptr = alist_next(&lst, ptr); + ut_assertnonnull(ptr); + ut_asserteq(789, ptr->val); + ut_asserteq_ptr(ptr, ptr2); + ptr2->val = 89; + ut_asserteq(89, ptr->val); + + ptr = alist_next(&lst, ptr); + ut_assertnull(ptr); + + alist_uninit(&lst); + + /* Check for memory leaks */ + ut_assertok(ut_check_delta(start)); + + return 0; +} +LIB_TEST(lib_test_alist_next, 0);

Add some macros which permit easy iteration through an alist, similar to those provided by the 'list' implementation.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/alist.h | 50 ++++++++++++++++++++++++++++++++ lib/alist.c | 7 +++++ test/lib/alist.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+)
diff --git a/include/alist.h b/include/alist.h index 97523af37a6..0090b9c0eb1 100644 --- a/include/alist.h +++ b/include/alist.h @@ -224,6 +224,56 @@ bool alist_expand_by(struct alist *lst, uint inc_by); */ const void *alist_next_ptrd(const struct alist *lst, const void *ptr);
+/** + * alist_chk_ptr() - Check whether a pointer is within a list + * + * Checks if the pointer points to an existing element of the list. The pointer + * must point to the start of an element, either in the list, or just outside of + * it. This function is only useful for handling for() loops + * + * Return: true if @ptr is within the list (0..count-1), else false + */ +bool alist_chk_ptr(const struct alist *lst, const void *ptr); + +/** + * alist_start() - Get the start of the list (first element) + * + * Note that this will always return ->data even if it is not NULL + * + * Usage: + * const struct my_struct *obj; # 'const' is optional + * + * alist_start(&lst, struct my_struct) + */ +#define alist_start(_lst, _struct) \ + ((_struct *)(_lst)->data) + +/** + * alist_end() - Get the end of the list (just after last element) + * + * Usage: + * const struct my_struct *obj; # 'const' is optional + * + * alist_end(&lst, struct my_struct) + */ +#define alist_end(_lst, _struct) \ + ((_struct *)(_lst)->data + (_lst)->count) + +/** + * alist_for_each() - Iterate over an alist (with constant pointer) + * + * Use as: + * const struct my_struct *obj; # 'const' is optional + * + * alist_for_each(obj, &lst) { + * obj->... + * } + */ +#define alist_for_each(_pos, _lst) \ + for (_pos = alist_start(_lst, typeof(*(_pos))); \ + _pos < alist_end(_lst, typeof(*(_pos))); \ + _pos++) + /** * alist_init() - Set up a new object list * diff --git a/lib/alist.c b/lib/alist.c index 7730fe0d473..1a4b4fb9c40 100644 --- a/lib/alist.c +++ b/lib/alist.c @@ -118,6 +118,13 @@ int alist_calc_index(const struct alist *lst, const void *ptr) return index; }
+bool alist_chk_ptr(const struct alist *lst, const void *ptr) +{ + int index = alist_calc_index(lst, ptr); + + return index >= 0 && index < lst->count; +} + const void *alist_next_ptrd(const struct alist *lst, const void *ptr) { int index = alist_calc_index(lst, ptr); diff --git a/test/lib/alist.c b/test/lib/alist.c index 96092affec9..1715a22584c 100644 --- a/test/lib/alist.c +++ b/test/lib/alist.c @@ -292,3 +292,77 @@ static int lib_test_alist_next(struct unit_test_state *uts) return 0; } LIB_TEST(lib_test_alist_next, 0); + +/* Test alist_for_each() */ +static int lib_test_alist_for_each(struct unit_test_state *uts) +{ + const struct my_struct *ptr; + struct my_struct data, *ptr2; + struct alist lst; + ulong start; + int sum; + + start = ut_check_free(); + + ut_assert(alist_init_struct(&lst, struct my_struct)); + ut_asserteq_ptr(NULL, alist_end(&lst, struct my_struct)); + + sum = 0; + alist_for_each(ptr, &lst) + sum++; + ut_asserteq(0, sum); + + alist_for_each(ptr, &lst) + sum++; + ut_asserteq(0, sum); + + /* add three items */ + data.val = 1; + data.other_val = 0; + alist_add(&lst, data); + + ptr = lst.data; + ut_asserteq_ptr(ptr + 1, alist_end(&lst, struct my_struct)); + + data.val = 2; + alist_add(&lst, data); + ut_asserteq_ptr(ptr + 2, alist_end(&lst, struct my_struct)); + + data.val = 3; + alist_add(&lst, data); + ut_asserteq_ptr(ptr + 3, alist_end(&lst, struct my_struct)); + + /* check alist_chk_ptr() */ + ut_asserteq(true, alist_chk_ptr(&lst, ptr + 2)); + ut_asserteq(false, alist_chk_ptr(&lst, ptr + 3)); + ut_asserteq(false, alist_chk_ptr(&lst, ptr + 4)); + ut_asserteq(true, alist_chk_ptr(&lst, ptr)); + ut_asserteq(false, alist_chk_ptr(&lst, ptr - 1)); + + /* sum all items */ + sum = 0; + alist_for_each(ptr, &lst) + sum += ptr->val; + ut_asserteq(6, sum); + + /* increment all items */ + alist_for_each(ptr2, &lst) + ptr2->val += 1; + + /* sum all items again */ + sum = 0; + alist_for_each(ptr, &lst) + sum += ptr->val; + ut_asserteq(9, sum); + + ptr = lst.data; + ut_asserteq_ptr(ptr + 3, alist_end(&lst, struct my_struct)); + + alist_uninit(&lst); + + /* Check for memory leaks */ + ut_assertok(ut_check_delta(start)); + + return 0; +} +LIB_TEST(lib_test_alist_for_each, 0);

Sometimes it is useful to empty the list without de-allocating any of the memory used, e.g. when the list will be re-populated immediately afterwards.
Add a new function for this.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/alist.h | 7 +++++++ lib/alist.c | 5 +++++ test/lib/alist.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+)
diff --git a/include/alist.h b/include/alist.h index 0090b9c0eb1..c639e42ab7d 100644 --- a/include/alist.h +++ b/include/alist.h @@ -274,6 +274,13 @@ bool alist_chk_ptr(const struct alist *lst, const void *ptr); _pos < alist_end(_lst, typeof(*(_pos))); \ _pos++)
+/** + * alist_empty() - Empty an alist + * + * This removes all entries from the list, without changing the allocated size + */ +void alist_empty(struct alist *lst); + /** * alist_init() - Set up a new object list * diff --git a/lib/alist.c b/lib/alist.c index 1a4b4fb9c40..32cd45b0b01 100644 --- a/lib/alist.c +++ b/lib/alist.c @@ -41,6 +41,11 @@ void alist_uninit(struct alist *lst) memset(lst, '\0', sizeof(struct alist)); }
+void alist_empty(struct alist *lst) +{ + lst->count = 0; +} + /** * alist_expand_to() - Expand a list to the given size * diff --git a/test/lib/alist.c b/test/lib/alist.c index 1715a22584c..87135bb3a51 100644 --- a/test/lib/alist.c +++ b/test/lib/alist.c @@ -358,6 +358,16 @@ static int lib_test_alist_for_each(struct unit_test_state *uts) ptr = lst.data; ut_asserteq_ptr(ptr + 3, alist_end(&lst, struct my_struct));
+ /* empty the list and try again */ + alist_empty(&lst); + ut_asserteq_ptr(ptr, alist_end(&lst, struct my_struct)); + ut_assertnull(alist_get(&lst, 0, struct my_struct)); + + sum = 0; + alist_for_each(ptr, &lst) + sum += ptr->val; + ut_asserteq(0, sum); + alist_uninit(&lst);
/* Check for memory leaks */ @@ -366,3 +376,35 @@ static int lib_test_alist_for_each(struct unit_test_state *uts) return 0; } LIB_TEST(lib_test_alist_for_each, 0); + +/* Test alist_empty() */ +static int lib_test_alist_empty(struct unit_test_state *uts) +{ + struct my_struct data; + struct alist lst; + ulong start; + + start = ut_check_free(); + + ut_assert(alist_init_struct(&lst, struct my_struct)); + ut_asserteq(0, lst.count); + data.val = 1; + data.other_val = 0; + alist_add(&lst, data); + ut_asserteq(1, lst.count); + ut_asserteq(4, lst.alloc); + + alist_empty(&lst); + ut_asserteq(0, lst.count); + ut_asserteq(4, lst.alloc); + ut_assertnonnull(lst.data); + ut_asserteq(sizeof(data), lst.obj_size); + + alist_uninit(&lst); + + /* Check for memory leaks */ + ut_assertok(ut_check_delta(start)); + + return 0; +} +LIB_TEST(lib_test_alist_empty, 0);

Unlike linked lists, it is inefficient to remove items from an alist, particularly if it is large. If most items need to be removed, then the time-complexity approaches O(n2).
Provide a way to do this efficiently, by working through the alist once and copying elements down.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/alist.h | 30 +++++++++++++++++ lib/alist.c | 8 +++++ test/lib/alist.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+)
diff --git a/include/alist.h b/include/alist.h index c639e42ab7d..b00d9ea97d6 100644 --- a/include/alist.h +++ b/include/alist.h @@ -274,6 +274,36 @@ bool alist_chk_ptr(const struct alist *lst, const void *ptr); _pos < alist_end(_lst, typeof(*(_pos))); \ _pos++)
+/** + * alist_for_each_filter() - version which sets up a 'from' pointer too + * + * This is used for filtering out information in the list. It works by iterating + * through the list, copying elements down over the top of elements to be + * deleted. + * + * In this example, 'from' iterates through the list from start to end,, 'to' + * also begins at the start, but only increments if the element at 'from' should + * be kept. This provides an O(n) filtering operation. Note that + * alist_update_end() must be called after the loop, to update the count. + * + * alist_for_each_filter(from, to, &lst) { + * if (from->val != 2) + * *to++ = *from; + * } + * alist_update_end(&lst, to); + */ +#define alist_for_each_filter(_pos, _from, _lst) \ + for (_pos = _from = alist_start(_lst, typeof(*(_pos))); \ + _pos < alist_end(_lst, typeof(*(_pos))); \ + _pos++) + +/** + * alist_update_end() - Set the element count based on a given pointer + * + * Set the given element as the final one + */ +void alist_update_end(struct alist *lst, const void *end); + /** * alist_empty() - Empty an alist * diff --git a/lib/alist.c b/lib/alist.c index 32cd45b0b01..4ce651f5c45 100644 --- a/lib/alist.c +++ b/lib/alist.c @@ -123,6 +123,14 @@ int alist_calc_index(const struct alist *lst, const void *ptr) return index; }
+void alist_update_end(struct alist *lst, const void *ptr) +{ + int index; + + index = alist_calc_index(lst, ptr); + lst->count = index == -1 ? 0 : index; +} + bool alist_chk_ptr(const struct alist *lst, const void *ptr) { int index = alist_calc_index(lst, ptr); diff --git a/test/lib/alist.c b/test/lib/alist.c index 87135bb3a51..0bf24578d2e 100644 --- a/test/lib/alist.c +++ b/test/lib/alist.c @@ -408,3 +408,88 @@ static int lib_test_alist_empty(struct unit_test_state *uts) return 0; } LIB_TEST(lib_test_alist_empty, 0); + +static int lib_test_alist_filter(struct unit_test_state *uts) +{ + struct my_struct *from, *to, *ptr; + struct my_struct data; + struct alist lst; + ulong start; + int count; + + start = ut_check_free(); + + ut_assert(alist_init_struct(&lst, struct my_struct)); + data.val = 1; + data.other_val = 0; + alist_add(&lst, data); + + data.val = 2; + alist_add(&lst, data); + + data.val = 3; + alist_add(&lst, data); + ptr = lst.data; + + /* filter out all values except 2 */ + alist_for_each_filter(from, to, &lst) { + if (from->val != 2) + *to++ = *from; + } + alist_update_end(&lst, to); + + ut_asserteq(2, lst.count); + ut_assertnonnull(lst.data); + + ut_asserteq(1, alist_get(&lst, 0, struct my_struct)->val); + ut_asserteq(3, alist_get(&lst, 1, struct my_struct)->val); + ut_asserteq_ptr(ptr + 3, from); + ut_asserteq_ptr(ptr + 2, to); + + /* filter out nothing */ + alist_for_each_filter(from, to, &lst) { + if (from->val != 2) + *to++ = *from; + } + alist_update_end(&lst, to); + ut_asserteq_ptr(ptr + 2, from); + ut_asserteq_ptr(ptr + 2, to); + + ut_asserteq(2, lst.count); + ut_assertnonnull(lst.data); + + ut_asserteq(1, alist_get(&lst, 0, struct my_struct)->val); + ut_asserteq(3, alist_get(&lst, 1, struct my_struct)->val); + + /* filter out everything */ + alist_for_each_filter(from, to, &lst) { + if (from->val == 2) + *to++ = *from; + } + alist_update_end(&lst, to); + ut_asserteq_ptr(ptr + 2, from); + ut_asserteq_ptr(ptr, to); + + /* filter out everything (nop) */ + count = 0; + alist_for_each_filter(from, to, &lst) { + if (from->val == 2) + *to++ = *from; + count++; + } + alist_update_end(&lst, to); + ut_asserteq_ptr(ptr, from); + ut_asserteq_ptr(ptr, to); + ut_asserteq(0, count); + + ut_asserteq(0, lst.count); + ut_assertnonnull(lst.data); + + alist_uninit(&lst); + + /* Check for memory leaks */ + ut_assertok(ut_check_delta(start)); + + return 0; +} +LIB_TEST(lib_test_alist_filter, 0);

All the uclass functions for finding a device end up creating a uclass if it doesn't exist. Add a function which instead returns NULL in this case.
This is useful when in the 'unbind' path, since we don't want to undo any unbinding which has already happened.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/uclass.c | 11 +++++++++++ include/dm/uclass.h | 11 +++++++++++ test/dm/core.c | 22 ++++++++++++++++++++++ 3 files changed, 44 insertions(+)
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 7ae0884a75e..f846a35d6b2 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -304,6 +304,17 @@ int uclass_find_device_by_name(enum uclass_id id, const char *name, return uclass_find_device_by_namelen(id, name, strlen(name), devp); }
+struct udevice *uclass_try_first_device(enum uclass_id id) +{ + struct uclass *uc; + + uc = uclass_find(id); + if (!uc || list_empty(&uc->dev_head)) + return NULL; + + return list_first_entry(&uc->dev_head, struct udevice, uclass_node); +} + int uclass_find_next_free_seq(struct uclass *uc) { struct udevice *dev; diff --git a/include/dm/uclass.h b/include/dm/uclass.h index 456eef7f2f3..c2793040923 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -435,6 +435,17 @@ int uclass_next_device_check(struct udevice **devp); int uclass_first_device_drvdata(enum uclass_id id, ulong driver_data, struct udevice **devp);
+/** + * uclass_try_first_device()- See if there is a device for a uclass + * + * If the uclass exists, this returns the first device on that uclass, without + * probing it. If the uclass does not exist, it gives up + * + * @id: Uclass ID to check + * Return: Pointer to device, if found, else NULL + */ +struct udevice *uclass_try_first_device(enum uclass_id id); + /** * uclass_probe_all() - Probe all devices based on an uclass ID * diff --git a/test/dm/core.c b/test/dm/core.c index e0c5b9e0017..7371d3ff426 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -1351,3 +1351,25 @@ static int dm_test_dev_get_mem(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_dev_get_mem, UTF_SCAN_FDT); + +/* Test uclass_try_first_device() */ +static int dm_test_try_first_device(struct unit_test_state *uts) +{ + struct udevice *dev; + + /* Check that it doesn't create a device or uclass */ + ut_assertnull(uclass_find(UCLASS_TEST)); + ut_assertnull(uclass_try_first_device(UCLASS_TEST)); + ut_assertnull(uclass_try_first_device(UCLASS_TEST)); + ut_assertnull(uclass_find(UCLASS_TEST)); + + /* Create a test device */ + ut_assertok(device_bind_by_name(uts->root, false, &driver_info_manual, + &dev)); + dev = uclass_try_first_device(UCLASS_TEST); + ut_assertnonnull(dev); + ut_asserteq(UCLASS_TEST, device_get_uclass_id(dev)); + + return 0; +} +DM_TEST(dm_test_try_first_device, 0);

In the bootflow tests the script bootmeth is bound with the name bootmeth_script whereas the others have a name without the bootmeth_ prefix. Adjust it to be the same.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/boot/bootflow.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c index 154dea70a59..8ea284098e7 100644 --- a/test/boot/bootflow.c +++ b/test/boot/bootflow.c @@ -542,7 +542,7 @@ static int prep_mmc_bootdev(struct unit_test_state *uts, const char *mmc_dev, /* Enable the script bootmeth too */ ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd)); ut_assertok(device_bind(bootstd, DM_DRIVER_REF(bootmeth_2script), - "bootmeth_script", 0, ofnode_null(), &dev)); + "script", 0, ofnode_null(), &dev));
/* Enable the cros bootmeth if needed */ if (IS_ENABLED(CONFIG_BOOTMETH_CROS) && bind_cros_android) {

This relates to more than just the bootdev, since there is a global list of bootflows. Move the function to the bootstd file and rename it.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootdev-uclass.c | 25 ------------------------- boot/bootstd-uclass.c | 25 +++++++++++++++++++++++++ cmd/bootflow.c | 2 +- include/bootdev.h | 15 --------------- include/bootstd.h | 17 +++++++++++++++++ 5 files changed, 43 insertions(+), 41 deletions(-)
diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index 807f8dfb064..f40bf0a6979 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -32,31 +32,6 @@ enum { BOOT_TARGETS_MAX_LEN = 100, };
-int bootdev_add_bootflow(struct bootflow *bflow) -{ - struct bootstd_priv *std; - struct bootflow *new; - int ret; - - ret = bootstd_get_priv(&std); - if (ret) - return ret; - - new = malloc(sizeof(*bflow)); - if (!new) - return log_msg_ret("bflow", -ENOMEM); - memcpy(new, bflow, sizeof(*bflow)); - - list_add_tail(&new->glob_node, &std->glob_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; -} - int bootdev_first_bootflow(struct udevice *dev, struct bootflow **bflowp) { struct bootdev_uc_plat *ucp = dev_get_uclass_plat(dev); diff --git a/boot/bootstd-uclass.c b/boot/bootstd-uclass.c index fdb8d69e320..bf6e49ad97a 100644 --- a/boot/bootstd-uclass.c +++ b/boot/bootstd-uclass.c @@ -61,6 +61,31 @@ void bootstd_clear_glob(void) bootstd_clear_glob_(std); }
+int bootstd_add_bootflow(struct bootflow *bflow) +{ + struct bootstd_priv *std; + struct bootflow *new; + int ret; + + ret = bootstd_get_priv(&std); + if (ret) + return ret; + + new = malloc(sizeof(*bflow)); + if (!new) + return log_msg_ret("bflow", -ENOMEM); + memcpy(new, bflow, sizeof(*bflow)); + + list_add_tail(&new->glob_node, &std->glob_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; +} + static int bootstd_remove(struct udevice *dev) { struct bootstd_priv *priv = dev_get_priv(dev); diff --git a/cmd/bootflow.c b/cmd/bootflow.c index 1588f277a4a..ff06ac3eb9f 100644 --- a/cmd/bootflow.c +++ b/cmd/bootflow.c @@ -207,7 +207,7 @@ static int do_bootflow_scan(struct cmd_tbl *cmdtp, int flag, int argc, bflow.err = ret; if (!ret) num_valid++; - ret = bootdev_add_bootflow(&bflow); + ret = bootstd_add_bootflow(&bflow); if (ret) { printf("Out of memory\n"); return CMD_RET_FAILURE; diff --git a/include/bootdev.h b/include/bootdev.h index ad4af0d1310..8db198dd56b 100644 --- a/include/bootdev.h +++ b/include/bootdev.h @@ -195,21 +195,6 @@ void bootdev_list(bool probe); */ void bootdev_clear_bootflows(struct udevice *dev);
-/** - * bootdev_add_bootflow() - Add a bootflow to the bootdev's list - * - * All fields in @bflow must be set up. Note that @bflow->dev is used to add the - * bootflow to that device. - * - * @dev: Bootdev device to add to - * @bflow: Bootflow to add. Note that fields within bflow must be allocated - * since this function takes over ownership of these. This functions makes - * a copy of @bflow itself (without allocating its fields again), so the - * caller must dispose of the memory used by the @bflow pointer itself - * Return: 0 if OK, -ENOMEM if out of memory - */ -int bootdev_add_bootflow(struct bootflow *bflow); - /** * bootdev_first_bootflow() - Get the first bootflow from a bootdev * diff --git a/include/bootstd.h b/include/bootstd.h index ac756e98d84..3fc93a4ec2e 100644 --- a/include/bootstd.h +++ b/include/bootstd.h @@ -105,4 +105,21 @@ void bootstd_clear_glob(void); */ int bootstd_prog_boot(void);
+/** + * bootstd_add_bootflow() - Add a bootflow to the bootdev's and global list + * + * All fields in @bflow must be set up. Note that @bflow->dev is used to add the + * bootflow to that device. + * + * The bootflow is also added to the global list of all bootflows + * + * @dev: Bootdev device to add to + * @bflow: Bootflow to add. Note that fields within bflow must be allocated + * since this function takes over ownership of these. This functions makes + * a copy of @bflow itself (without allocating its fields again), so the + * caller must dispose of the memory used by the @bflow pointer itself + * Return: 0 if OK, -ENOMEM if out of memory + */ +int bootstd_add_bootflow(struct bootflow *bflow); + #endif

This relates to more than just the bootdev, since there is a global list of bootflows. Move the function to the bootstd file and rename it.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootdev-uclass.c | 19 +++++-------------- boot/bootstd-uclass.c | 15 +++++++++++++++ cmd/bootflow.c | 2 +- include/bootdev.h | 10 ---------- include/bootstd.h | 10 ++++++++++ 5 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index f40bf0a6979..e9dda65417d 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -554,19 +554,6 @@ int bootdev_get_bootflow(struct udevice *dev, struct bootflow_iter *iter, return ops->get_bootflow(dev, iter, bflow); }
-void bootdev_clear_bootflows(struct udevice *dev) -{ - struct bootdev_uc_plat *ucp = dev_get_uclass_plat(dev); - - while (!list_empty(&ucp->bootflow_head)) { - struct bootflow *bflow; - - bflow = list_first_entry(&ucp->bootflow_head, struct bootflow, - bm_node); - bootflow_remove(bflow); - } -} - int bootdev_next_label(struct bootflow_iter *iter, struct udevice **devp, int *method_flagsp) { @@ -932,7 +919,11 @@ static int bootdev_post_bind(struct udevice *dev)
static int bootdev_pre_unbind(struct udevice *dev) { - bootdev_clear_bootflows(dev); + int ret; + + ret = bootstd_clear_bootflows_for_bootdev(dev); + if (ret) + return log_msg_ret("bun", ret);
return 0; } diff --git a/boot/bootstd-uclass.c b/boot/bootstd-uclass.c index bf6e49ad97a..596d3e5e41d 100644 --- a/boot/bootstd-uclass.c +++ b/boot/bootstd-uclass.c @@ -86,6 +86,21 @@ int bootstd_add_bootflow(struct bootflow *bflow) return 0; }
+int bootstd_clear_bootflows_for_bootdev(struct udevice *dev) +{ + struct bootdev_uc_plat *ucp = dev_get_uclass_plat(dev); + + while (!list_empty(&ucp->bootflow_head)) { + struct bootflow *bflow; + + bflow = list_first_entry(&ucp->bootflow_head, struct bootflow, + bm_node); + bootflow_remove(bflow); + } + + return 0; +} + static int bootstd_remove(struct udevice *dev) { struct bootstd_priv *priv = dev_get_priv(dev); diff --git a/cmd/bootflow.c b/cmd/bootflow.c index ff06ac3eb9f..5f656814b29 100644 --- a/cmd/bootflow.c +++ b/cmd/bootflow.c @@ -197,7 +197,7 @@ static int do_bootflow_scan(struct cmd_tbl *cmdtp, int flag, int argc, show_header(); } if (dev) - bootdev_clear_bootflows(dev); + bootstd_clear_bootflows_for_bootdev(dev); else bootstd_clear_glob(); for (i = 0, diff --git a/include/bootdev.h b/include/bootdev.h index 8db198dd56b..f9cae2fd1fd 100644 --- a/include/bootdev.h +++ b/include/bootdev.h @@ -185,16 +185,6 @@ int bootdev_find_in_blk(struct udevice *dev, struct udevice *blk, */ void bootdev_list(bool probe);
-/** - * bootdev_clear_bootflows() - Clear bootflows from a bootdev - * - * Each bootdev maintains a list of discovered bootflows. This provides a - * way to clear it. These bootflows are removed from the global list too. - * - * @dev: bootdev device to update - */ -void bootdev_clear_bootflows(struct udevice *dev); - /** * bootdev_first_bootflow() - Get the first bootflow from a bootdev * diff --git a/include/bootstd.h b/include/bootstd.h index 3fc93a4ec2e..4220ece785d 100644 --- a/include/bootstd.h +++ b/include/bootstd.h @@ -122,4 +122,14 @@ int bootstd_prog_boot(void); */ int bootstd_add_bootflow(struct bootflow *bflow);
+/** + * bootstd_clear_bootflows_for_bootdev() - Clear bootflows from a bootdev + * + * Each bootdev maintains a list of discovered bootflows. This provides a + * way to clear it. These bootflows are removed from the global list too. + * + * @dev: bootdev device to update + */ +int bootstd_clear_bootflows_for_bootdev(struct udevice *dev); + #endif

Provide a function which is safe to call in the 'unbind' path, which returns the bootstd priv data if available.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootstd-uclass.c | 11 +++++++++++ include/bootstd.h | 17 +++++++++++++++++ 2 files changed, 28 insertions(+)
diff --git a/boot/bootstd-uclass.c b/boot/bootstd-uclass.c index 596d3e5e41d..b2f80808c85 100644 --- a/boot/bootstd-uclass.c +++ b/boot/bootstd-uclass.c @@ -140,6 +140,17 @@ const char *const *const bootstd_get_prefixes(struct udevice *dev) return std->prefixes ? std->prefixes : default_prefixes; }
+struct bootstd_priv *bootstd_try_priv(void) +{ + struct udevice *dev; + + dev = uclass_try_first_device(UCLASS_BOOTSTD); + if (!dev || !device_active(dev)) + return NULL; + + return dev_get_priv(dev); +} + int bootstd_get_priv(struct bootstd_priv **stdp) { struct udevice *dev; diff --git a/include/bootstd.h b/include/bootstd.h index 4220ece785d..4535d91e2ad 100644 --- a/include/bootstd.h +++ b/include/bootstd.h @@ -89,6 +89,23 @@ const char *const *const bootstd_get_prefixes(struct udevice *dev); */ int bootstd_get_priv(struct bootstd_priv **stdp);
+/** + * bootstd_try_priv() - Try to get the (single) state for the bootstd system + * + * The state holds a global list of all bootflows that have been found. This + * function returns the state if available, but takes care not to create the + * device (or uclass) if it doesn't exist. + * + * This function is safe to use in the 'unbind' path. It will always return NULL + * unless the bootstd device is probed and ready, e.g. bootstd_get_priv() has + * previously been called. + * + * TODO(sjg@chromium.org): Consider adding a bootstd pointer to global_data + * + * Return: pointer if the device exists, else NULL + */ +struct bootstd_priv *bootstd_try_priv(void); + /** * bootstd_clear_glob() - Clear the global list of bootflows *

This list is only used by two functions, which can be updated to iterate through the global list. Take this approach, which allows the bootdev list to be dropped.
Overall this makes the code slightly more complicated, but will allow moving the bootflow list into an alist
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootdev-uclass.c | 61 +++++++++++++++++++++++++++---------------- boot/bootflow.c | 2 -- boot/bootstd-uclass.c | 17 +++++------- include/bootdev.h | 2 -- include/bootflow.h | 5 +--- include/bootstd.h | 2 +- 6 files changed, 48 insertions(+), 41 deletions(-)
diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index e9dda65417d..8f4a4bec129 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -32,30 +32,57 @@ enum { BOOT_TARGETS_MAX_LEN = 100, };
+struct bootflow *bootdev_next_bootflow_(struct bootstd_priv *std, + struct udevice *dev, + struct bootflow *prev) +{ + struct bootflow *bflow = prev; + + if (bflow) { + if (list_is_last(&bflow->glob_node, &std->glob_head)) + return NULL; + bflow = list_entry(bflow->glob_node.next, struct bootflow, + glob_node); + } else { + if (list_empty(&std->glob_head)) + return NULL; + + bflow = list_first_entry(&std->glob_head, struct bootflow, + glob_node); + } + + while (bflow->dev != dev) { + if (list_is_last(&bflow->glob_node, &std->glob_head)) + return NULL; + bflow = list_entry(bflow->glob_node.next, struct bootflow, + glob_node); + } + + return bflow; +} + int bootdev_first_bootflow(struct udevice *dev, struct bootflow **bflowp) { - struct bootdev_uc_plat *ucp = dev_get_uclass_plat(dev); + struct bootstd_priv *std = bootstd_try_priv(); + struct bootflow *bflow;
- if (list_empty(&ucp->bootflow_head)) + bflow = bootdev_next_bootflow_(std, dev, NULL); + if (!bflow) return -ENOENT; - - *bflowp = list_first_entry(&ucp->bootflow_head, struct bootflow, - bm_node); + *bflowp = bflow;
return 0; }
int bootdev_next_bootflow(struct bootflow **bflowp) { - struct bootflow *bflow = *bflowp; - struct bootdev_uc_plat *ucp = dev_get_uclass_plat(bflow->dev); - - *bflowp = NULL; + struct bootstd_priv *std = bootstd_try_priv(); + struct bootflow *bflow;
- if (list_is_last(&bflow->bm_node, &ucp->bootflow_head)) + bflow = bootdev_next_bootflow_(std, (*bflowp)->dev, *bflowp); + if (!bflow) return -ENOENT; - - *bflowp = list_entry(bflow->bm_node.next, struct bootflow, bm_node); + *bflowp = bflow;
return 0; } @@ -908,15 +935,6 @@ void bootdev_list_hunters(struct bootstd_priv *std) printf("(total hunters: %d)\n", n_ent); }
-static int bootdev_post_bind(struct udevice *dev) -{ - struct bootdev_uc_plat *ucp = dev_get_uclass_plat(dev); - - INIT_LIST_HEAD(&ucp->bootflow_head); - - return 0; -} - static int bootdev_pre_unbind(struct udevice *dev) { int ret; @@ -933,6 +951,5 @@ UCLASS_DRIVER(bootdev) = { .name = "bootdev", .flags = DM_UC_FLAG_SEQ_ALIAS, .per_device_plat_auto = sizeof(struct bootdev_uc_plat), - .post_bind = bootdev_post_bind, .pre_unbind = bootdev_pre_unbind, }; diff --git a/boot/bootflow.c b/boot/bootflow.c index 59d77d2385f..ae59d1a4092 100644 --- a/boot/bootflow.c +++ b/boot/bootflow.c @@ -476,8 +476,6 @@ void bootflow_free(struct bootflow *bflow)
void bootflow_remove(struct bootflow *bflow) { - if (bflow->dev) - list_del(&bflow->bm_node); list_del(&bflow->glob_node);
bootflow_free(bflow); diff --git a/boot/bootstd-uclass.c b/boot/bootstd-uclass.c index b2f80808c85..91e90bdf43c 100644 --- a/boot/bootstd-uclass.c +++ b/boot/bootstd-uclass.c @@ -77,25 +77,22 @@ int bootstd_add_bootflow(struct bootflow *bflow) memcpy(new, bflow, sizeof(*bflow));
list_add_tail(&new->glob_node, &std->glob_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; }
int bootstd_clear_bootflows_for_bootdev(struct udevice *dev) { - struct bootdev_uc_plat *ucp = dev_get_uclass_plat(dev); + struct bootstd_priv *std = bootstd_try_priv();
- while (!list_empty(&ucp->bootflow_head)) { + if (std) { struct bootflow *bflow; + struct list_head *pos;
- bflow = list_first_entry(&ucp->bootflow_head, struct bootflow, - bm_node); - bootflow_remove(bflow); + list_for_each(pos, &std->glob_head) { + bflow = list_entry(pos, struct bootflow, glob_node); + bootflow_remove(bflow); + } }
return 0; diff --git a/include/bootdev.h b/include/bootdev.h index f9cae2fd1fd..991b6229c1c 100644 --- a/include/bootdev.h +++ b/include/bootdev.h @@ -109,11 +109,9 @@ struct bootdev_hunter { * This is attached to each device in the bootdev uclass and accessible via * dev_get_uclass_plat(dev) * - * @bootflows: List of available bootflows for this bootdev * @piro: Priority of this bootdev */ struct bootdev_uc_plat { - struct list_head bootflow_head; enum bootdev_prio_t prio; };
diff --git a/include/bootflow.h b/include/bootflow.h index 4d2fc7b69b5..64d1d6c3786 100644 --- a/include/bootflow.h +++ b/include/bootflow.h @@ -56,12 +56,10 @@ enum bootflow_flags_t { /** * struct bootflow - information about a bootflow * - * This is connected into two separate linked lists: + * This is connected into a linked list: * - * bm_sibling - links all bootflows in the same bootdev * glob_sibling - links all bootflows in all bootdevs * - * @bm_node: Points to siblings in the same bootdev * @glob_node: Points to siblings in the global list (all bootdev) * @dev: Bootdev device which produced this bootflow, NULL for flows created by * BOOTMETHF_GLOBAL bootmeths @@ -92,7 +90,6 @@ enum bootflow_flags_t { * @bootmeth_priv: Private data for the bootmeth */ struct bootflow { - struct list_head bm_node; struct list_head glob_node; struct udevice *dev; struct udevice *blk; diff --git a/include/bootstd.h b/include/bootstd.h index 4535d91e2ad..8aff536e3cb 100644 --- a/include/bootstd.h +++ b/include/bootstd.h @@ -123,7 +123,7 @@ void bootstd_clear_glob(void); int bootstd_prog_boot(void);
/** - * bootstd_add_bootflow() - Add a bootflow to the bootdev's and global list + * bootstd_add_bootflow() - Add a bootflow to the global list * * All fields in @bflow must be set up. Note that @bflow->dev is used to add the * bootflow to that device.

Use an alist for this data structure as it is somewhat simpler to manage. This means that bootstd holds a simple list of bootflow structs and can drop it at will, without chasing down lists.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootdev-uclass.c | 47 +++++++++++++------------------------------ boot/bootflow.c | 16 ++++----------- boot/bootstd-uclass.c | 39 +++++++++++++++++------------------ cmd/bootdev.c | 2 +- include/bootflow.h | 11 +++++----- include/bootstd.h | 6 ++++-- 6 files changed, 47 insertions(+), 74 deletions(-)
diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index 8f4a4bec129..6ed59a206e2 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -32,41 +32,17 @@ enum { BOOT_TARGETS_MAX_LEN = 100, };
-struct bootflow *bootdev_next_bootflow_(struct bootstd_priv *std, - struct udevice *dev, - struct bootflow *prev) -{ - struct bootflow *bflow = prev; - - if (bflow) { - if (list_is_last(&bflow->glob_node, &std->glob_head)) - return NULL; - bflow = list_entry(bflow->glob_node.next, struct bootflow, - glob_node); - } else { - if (list_empty(&std->glob_head)) - return NULL; - - bflow = list_first_entry(&std->glob_head, struct bootflow, - glob_node); - } - - while (bflow->dev != dev) { - if (list_is_last(&bflow->glob_node, &std->glob_head)) - return NULL; - bflow = list_entry(bflow->glob_node.next, struct bootflow, - glob_node); - } - - return bflow; -} - int bootdev_first_bootflow(struct udevice *dev, struct bootflow **bflowp) { - struct bootstd_priv *std = bootstd_try_priv(); + struct bootstd_priv *std; struct bootflow *bflow; + int ret; + + ret = bootstd_get_priv(&std); + if (ret) + return log_msg_ret("bff", ret);
- bflow = bootdev_next_bootflow_(std, dev, NULL); + bflow = alist_getw(&std->bootflows, 0, struct bootflow); if (!bflow) return -ENOENT; *bflowp = bflow; @@ -76,10 +52,15 @@ int bootdev_first_bootflow(struct udevice *dev, struct bootflow **bflowp)
int bootdev_next_bootflow(struct bootflow **bflowp) { - struct bootstd_priv *std = bootstd_try_priv(); + struct bootstd_priv *std; struct bootflow *bflow; + int ret; + + ret = bootstd_get_priv(&std); + if (ret) + return log_msg_ret("bff", ret);
- bflow = bootdev_next_bootflow_(std, (*bflowp)->dev, *bflowp); + bflow = alist_nextw(&std->bootflows, *bflowp); if (!bflow) return -ENOENT; *bflowp = bflow; diff --git a/boot/bootflow.c b/boot/bootflow.c index ae59d1a4092..72b51ebd49b 100644 --- a/boot/bootflow.c +++ b/boot/bootflow.c @@ -55,11 +55,10 @@ int bootflow_first_glob(struct bootflow **bflowp) if (ret) return ret;
- if (list_empty(&std->glob_head)) + if (!std->bootflows.count) return -ENOENT;
- *bflowp = list_first_entry(&std->glob_head, struct bootflow, - glob_node); + *bflowp = alist_getw(&std->bootflows, 0, struct bootflow);
return 0; } @@ -67,20 +66,16 @@ int bootflow_first_glob(struct bootflow **bflowp) int bootflow_next_glob(struct bootflow **bflowp) { struct bootstd_priv *std; - struct bootflow *bflow = *bflowp; int ret;
ret = bootstd_get_priv(&std); if (ret) return ret;
- *bflowp = NULL; - - if (list_is_last(&bflow->glob_node, &std->glob_head)) + *bflowp = alist_nextw(&std->bootflows, *bflowp); + if (!*bflowp) return -ENOENT;
- *bflowp = list_entry(bflow->glob_node.next, struct bootflow, glob_node); - return 0; }
@@ -476,10 +471,7 @@ void bootflow_free(struct bootflow *bflow)
void bootflow_remove(struct bootflow *bflow) { - list_del(&bflow->glob_node); - bootflow_free(bflow); - free(bflow); }
#if CONFIG_IS_ENABLED(BOOTSTD_FULL) diff --git a/boot/bootstd-uclass.c b/boot/bootstd-uclass.c index 91e90bdf43c..5675719ee84 100644 --- a/boot/bootstd-uclass.c +++ b/boot/bootstd-uclass.c @@ -6,6 +6,7 @@ * Written by Simon Glass sjg@chromium.org */
+#include <alist.h> #include <bootflow.h> #include <bootstd.h> #include <dm.h> @@ -42,13 +43,11 @@ static int bootstd_of_to_plat(struct udevice *dev)
static void bootstd_clear_glob_(struct bootstd_priv *priv) { - while (!list_empty(&priv->glob_head)) { - struct bootflow *bflow; + struct bootflow *bflow;
- bflow = list_first_entry(&priv->glob_head, struct bootflow, - glob_node); + alist_for_each(bflow, &priv->bootflows) bootflow_remove(bflow); - } + alist_empty(&priv->bootflows); }
void bootstd_clear_glob(void) @@ -64,19 +63,15 @@ void bootstd_clear_glob(void) int bootstd_add_bootflow(struct bootflow *bflow) { struct bootstd_priv *std; - struct bootflow *new; int ret;
ret = bootstd_get_priv(&std); if (ret) return ret;
- new = malloc(sizeof(*bflow)); - if (!new) - return log_msg_ret("bflow", -ENOMEM); - memcpy(new, bflow, sizeof(*bflow)); - - list_add_tail(&new->glob_node, &std->glob_head); + bflow = alist_add(&std->bootflows, *bflow); + if (!bflow) + return log_msg_ret("bf2", -ENOMEM);
return 0; } @@ -84,16 +79,20 @@ int bootstd_add_bootflow(struct bootflow *bflow) int bootstd_clear_bootflows_for_bootdev(struct udevice *dev) { struct bootstd_priv *std = bootstd_try_priv(); + struct bootflow *from, *to;
- if (std) { - struct bootflow *bflow; - struct list_head *pos; + /* if bootstd does not exist we cannot have any bootflows */ + if (!std) + return 0;
- list_for_each(pos, &std->glob_head) { - bflow = list_entry(pos, struct bootflow, glob_node); - bootflow_remove(bflow); - } + /* Drop any bootflows that mention this dev */ + alist_for_each_filter(from, to, &std->bootflows) { + if (from->dev == dev) + bootflow_remove(from); + else + *to++ = *from; } + alist_update_end(&std->bootflows, to);
return 0; } @@ -165,7 +164,7 @@ static int bootstd_probe(struct udevice *dev) { struct bootstd_priv *std = dev_get_priv(dev);
- INIT_LIST_HEAD(&std->glob_head); + alist_init_struct(&std->bootflows, struct bootflow);
return 0; } diff --git a/cmd/bootdev.c b/cmd/bootdev.c index fa7285ba25e..4bc229e809a 100644 --- a/cmd/bootdev.c +++ b/cmd/bootdev.c @@ -81,7 +81,7 @@ static int do_bootdev_info(struct cmd_tbl *cmdtp, int flag, int argc,
dev = priv->cur_bootdev;
- /* Count the number of bootflows, including how many are valid*/ + /* Count the number of bootflows, including how many are valid */ num_valid = 0; for (ret = bootdev_first_bootflow(dev, &bflow), i = 0; !ret; diff --git a/include/bootflow.h b/include/bootflow.h index 64d1d6c3786..9b24fb5c3eb 100644 --- a/include/bootflow.h +++ b/include/bootflow.h @@ -56,11 +56,8 @@ enum bootflow_flags_t { /** * struct bootflow - information about a bootflow * - * This is connected into a linked list: + * All bootflows are listed in bootstd's bootflow alist in struct bootstd_priv * - * glob_sibling - links all bootflows in all bootdevs - * - * @glob_node: Points to siblings in the global list (all bootdev) * @dev: Bootdev device which produced this bootflow, NULL for flows created by * BOOTMETHF_GLOBAL bootmeths * @blk: Block device which contains this bootflow, NULL if this is a network @@ -90,7 +87,6 @@ enum bootflow_flags_t { * @bootmeth_priv: Private data for the bootmeth */ struct bootflow { - struct list_head glob_node; struct udevice *dev; struct udevice *blk; int part; @@ -390,7 +386,10 @@ const char *bootflow_state_get_name(enum bootflow_state_t state); /** * bootflow_remove() - Remove a bootflow and free its memory * - * This updates the linked lists containing the bootflow then frees it. + * This updates the 'global' linked list containing the bootflow, then frees it. + * It does not remove it from bootflows alist in struct bootstd_priv + * + * This does not free bflow itself, since this is assumed to be in an alist * * @bflow: Bootflow to remove */ diff --git a/include/bootstd.h b/include/bootstd.h index 8aff536e3cb..c5b902372c3 100644 --- a/include/bootstd.h +++ b/include/bootstd.h @@ -9,6 +9,7 @@ #ifndef __bootstd_h #define __bootstd_h
+#include <alist.h> #include <dm/ofnode_decl.h> #include <linux/list.h> #include <linux/types.h> @@ -30,7 +31,8 @@ struct udevice; * terminated) * @cur_bootdev: Currently selected bootdev (for commands) * @cur_bootflow: Currently selected bootflow (for commands) - * @glob_head: Head for the global list of all bootflows across all bootdevs + * @bootflows: (struct bootflow) Global list of all bootflows across all + * bootdevs * @bootmeth_count: Number of bootmeth devices in @bootmeth_order * @bootmeth_order: List of bootmeth devices to use, in order, NULL-terminated * @vbe_bootmeth: Currently selected VBE bootmeth, NULL if none @@ -44,7 +46,7 @@ struct bootstd_priv { const char **env_order; struct udevice *cur_bootdev; struct bootflow *cur_bootflow; - struct list_head glob_head; + struct alist bootflows; int bootmeth_count; struct udevice **bootmeth_order; struct udevice *vbe_bootmeth;

Add an image type for the extlinux.cfg file, which U-Boot supports reading.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index abac254e026..cf16fb7fbf7 100644 --- a/boot/image.c +++ b/boot/image.c @@ -183,6 +183,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat Device Tree ", }, { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" }, { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" }, + { IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" }, { -1, "", "", }, };
diff --git a/include/image.h b/include/image.h index c52fced9b40..606e7d49169 100644 --- a/include/image.h +++ b/include/image.h @@ -232,6 +232,7 @@ enum image_type_t { IH_TYPE_FDT_LEGACY, /* Binary Flat Device Tree Blob in a Legacy Image */ IH_TYPE_RENESAS_SPKG, /* Renesas SPKG image */ IH_TYPE_STARFIVE_SPL, /* StarFive SPL image */ + IH_TYPE_EXTLINUX_CFG, /* extlinux configuration-file */
IH_TYPE_COUNT, /* Number of image types */ };

On Thu, Oct 17, 2024 at 05:23:54PM -0600, Simon Glass wrote:
Add an image type for the extlinux.cfg file, which U-Boot supports reading.
Signed-off-by: Simon Glass sjg@chromium.org
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index abac254e026..cf16fb7fbf7 100644 --- a/boot/image.c +++ b/boot/image.c @@ -183,6 +183,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat Device Tree ", }, { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" }, { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" },
- { IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" }, { -1, "", "", },
};
This whole part of the series feels like we're abusing IH_TYPE_ far past what it's used for. Especially the command line string one. It seems like the main use is to be able to later on print something human friendly. Please try and figure out some other way to do that. Thanks.

Hi Tom,
On Thu, 17 Oct 2024 at 21:14, Tom Rini trini@konsulko.com wrote:
On Thu, Oct 17, 2024 at 05:23:54PM -0600, Simon Glass wrote:
Add an image type for the extlinux.cfg file, which U-Boot supports reading.
Signed-off-by: Simon Glass sjg@chromium.org
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index abac254e026..cf16fb7fbf7 100644 --- a/boot/image.c +++ b/boot/image.c @@ -183,6 +183,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat Device Tree ", }, { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" }, { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" },
{ IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" }, { -1, "", "", },
};
This whole part of the series feels like we're abusing IH_TYPE_ far past what it's used for. Especially the command line string one. It seems like the main use is to be able to later on print something human friendly. Please try and figure out some other way to do that. Thanks.
Yes I had the same thought, particularly with cmdline, as you say.
The obvious alternative is to just have a string which communicates the type. Then I can use IH_TYPE_INVALID, perhaps, with an extra string indicating what it really is.
But one advantage of the approach in this patch is that, for bootmeths which include a cmdline, it can be identified and added as a file. That includes zimage and ChromiumOS, but not EFI. It will allow me to get rid of the cmdline in struct bootflow, perhaps.
Regards, Simon

On Fri, Oct 18, 2024 at 08:57:02AM -0600, Simon Glass wrote:
Hi Tom,
On Thu, 17 Oct 2024 at 21:14, Tom Rini trini@konsulko.com wrote:
On Thu, Oct 17, 2024 at 05:23:54PM -0600, Simon Glass wrote:
Add an image type for the extlinux.cfg file, which U-Boot supports reading.
Signed-off-by: Simon Glass sjg@chromium.org
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index abac254e026..cf16fb7fbf7 100644 --- a/boot/image.c +++ b/boot/image.c @@ -183,6 +183,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat Device Tree ", }, { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" }, { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" },
{ IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" }, { -1, "", "", },
};
This whole part of the series feels like we're abusing IH_TYPE_ far past what it's used for. Especially the command line string one. It seems like the main use is to be able to later on print something human friendly. Please try and figure out some other way to do that. Thanks.
Yes I had the same thought, particularly with cmdline, as you say.
The obvious alternative is to just have a string which communicates the type. Then I can use IH_TYPE_INVALID, perhaps, with an extra string indicating what it really is.
But one advantage of the approach in this patch is that, for bootmeths which include a cmdline, it can be identified and added as a file. That includes zimage and ChromiumOS, but not EFI. It will allow me to get rid of the cmdline in struct bootflow, perhaps.
Yeah, this all sounds like IH_TYPE is the wrong direction to expand. Where you're grabbing these things from is where you might get a hint as to what these things are and that's where to take the store the knowledge for later.

Hi Tom,
On Fri, 18 Oct 2024 at 09:17, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 08:57:02AM -0600, Simon Glass wrote:
Hi Tom,
On Thu, 17 Oct 2024 at 21:14, Tom Rini trini@konsulko.com wrote:
On Thu, Oct 17, 2024 at 05:23:54PM -0600, Simon Glass wrote:
Add an image type for the extlinux.cfg file, which U-Boot supports reading.
Signed-off-by: Simon Glass sjg@chromium.org
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index abac254e026..cf16fb7fbf7 100644 --- a/boot/image.c +++ b/boot/image.c @@ -183,6 +183,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat Device Tree ", }, { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" }, { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" },
{ IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" }, { -1, "", "", },
};
This whole part of the series feels like we're abusing IH_TYPE_ far past what it's used for. Especially the command line string one. It seems like the main use is to be able to later on print something human friendly. Please try and figure out some other way to do that. Thanks.
Yes I had the same thought, particularly with cmdline, as you say.
The obvious alternative is to just have a string which communicates the type. Then I can use IH_TYPE_INVALID, perhaps, with an extra string indicating what it really is.
But one advantage of the approach in this patch is that, for bootmeths which include a cmdline, it can be identified and added as a file. That includes zimage and ChromiumOS, but not EFI. It will allow me to get rid of the cmdline in struct bootflow, perhaps.
Yeah, this all sounds like IH_TYPE is the wrong direction to expand. Where you're grabbing these things from is where you might get a hint as to what these things are and that's where to take the store the knowledge for later.
They are grabbed from several places: - extlinux comes from disk, with the extlinux bootmeth, but can also come from 'include' files within that scripts, also pxe - EFI apps from from disk with the EFI bootmeth - kernel, ramdisk and fdt some from most bootmeths - cmdline comes from extlinux (with zImage only) and ChromeOS, probably Android but I haven't looked - logo comes just from script so far, but I suppose UKI and FIT will have it eventually
Can you be more specific with your suggestion? I am not seeing an alternative right now.
Regards, Simon

On Fri, Oct 18, 2024 at 11:20:45AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 09:17, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 08:57:02AM -0600, Simon Glass wrote:
Hi Tom,
On Thu, 17 Oct 2024 at 21:14, Tom Rini trini@konsulko.com wrote:
On Thu, Oct 17, 2024 at 05:23:54PM -0600, Simon Glass wrote:
Add an image type for the extlinux.cfg file, which U-Boot supports reading.
Signed-off-by: Simon Glass sjg@chromium.org
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index abac254e026..cf16fb7fbf7 100644 --- a/boot/image.c +++ b/boot/image.c @@ -183,6 +183,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat Device Tree ", }, { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" }, { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" },
{ IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" }, { -1, "", "", },
};
This whole part of the series feels like we're abusing IH_TYPE_ far past what it's used for. Especially the command line string one. It seems like the main use is to be able to later on print something human friendly. Please try and figure out some other way to do that. Thanks.
Yes I had the same thought, particularly with cmdline, as you say.
The obvious alternative is to just have a string which communicates the type. Then I can use IH_TYPE_INVALID, perhaps, with an extra string indicating what it really is.
But one advantage of the approach in this patch is that, for bootmeths which include a cmdline, it can be identified and added as a file. That includes zimage and ChromiumOS, but not EFI. It will allow me to get rid of the cmdline in struct bootflow, perhaps.
Yeah, this all sounds like IH_TYPE is the wrong direction to expand. Where you're grabbing these things from is where you might get a hint as to what these things are and that's where to take the store the knowledge for later.
They are grabbed from several places:
- extlinux comes from disk, with the extlinux bootmeth, but can also
come from 'include' files within that scripts, also pxe
- EFI apps from from disk with the EFI bootmeth
- kernel, ramdisk and fdt some from most bootmeths
- cmdline comes from extlinux (with zImage only) and ChromeOS,
probably Android but I haven't looked
- logo comes just from script so far, but I suppose UKI and FIT will
have it eventually
Can you be more specific with your suggestion? I am not seeing an alternative right now.
I'm just saying that this is not what IH_TYPE is. As you said maybe you can store this string information elsewhere? And I'm suggesting that perhaps when you load a thing you'll then also know what the thing is.

Hi Tom,
On Fri, 18 Oct 2024 at 11:54, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 11:20:45AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 09:17, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 08:57:02AM -0600, Simon Glass wrote:
Hi Tom,
On Thu, 17 Oct 2024 at 21:14, Tom Rini trini@konsulko.com wrote:
On Thu, Oct 17, 2024 at 05:23:54PM -0600, Simon Glass wrote:
Add an image type for the extlinux.cfg file, which U-Boot supports reading.
Signed-off-by: Simon Glass sjg@chromium.org
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index abac254e026..cf16fb7fbf7 100644 --- a/boot/image.c +++ b/boot/image.c @@ -183,6 +183,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat Device Tree ", }, { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" }, { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" },
{ IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" }, { -1, "", "", },
};
This whole part of the series feels like we're abusing IH_TYPE_ far past what it's used for. Especially the command line string one. It seems like the main use is to be able to later on print something human friendly. Please try and figure out some other way to do that. Thanks.
Yes I had the same thought, particularly with cmdline, as you say.
The obvious alternative is to just have a string which communicates the type. Then I can use IH_TYPE_INVALID, perhaps, with an extra string indicating what it really is.
But one advantage of the approach in this patch is that, for bootmeths which include a cmdline, it can be identified and added as a file. That includes zimage and ChromiumOS, but not EFI. It will allow me to get rid of the cmdline in struct bootflow, perhaps.
Yeah, this all sounds like IH_TYPE is the wrong direction to expand. Where you're grabbing these things from is where you might get a hint as to what these things are and that's where to take the store the knowledge for later.
They are grabbed from several places:
- extlinux comes from disk, with the extlinux bootmeth, but can also
come from 'include' files within that scripts, also pxe
- EFI apps from from disk with the EFI bootmeth
- kernel, ramdisk and fdt some from most bootmeths
- cmdline comes from extlinux (with zImage only) and ChromeOS,
probably Android but I haven't looked
- logo comes just from script so far, but I suppose UKI and FIT will
have it eventually
Can you be more specific with your suggestion? I am not seeing an alternative right now.
I'm just saying that this is not what IH_TYPE is. As you said maybe you can store this string information elsewhere? And I'm suggesting that perhaps when you load a thing you'll then also know what the thing is.
Yes, I agree. 'Type' is really about the actual file that you are reading, not the data types of the bits within it, for example. That's the problem with my approach.
The other idea I had was to add a new thing to image.h (IH_DATA_xxx). But I wasn't sure it was worth it. Also, if you look at fit_image_load() it reads an image of a certain type out of an existing (containing) FIT. So we already have images within images.
How about I just put a data type in bootflow.h ? Then it will be separate from image.h and I'm not blurring the line any further between an image and its component parts? We can always change it later if we find a better way.
Regards, Simon

On Fri, Oct 18, 2024 at 12:19:26PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 11:54, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 11:20:45AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 09:17, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 08:57:02AM -0600, Simon Glass wrote:
Hi Tom,
On Thu, 17 Oct 2024 at 21:14, Tom Rini trini@konsulko.com wrote:
On Thu, Oct 17, 2024 at 05:23:54PM -0600, Simon Glass wrote:
> Add an image type for the extlinux.cfg file, which U-Boot supports > reading. > > Signed-off-by: Simon Glass sjg@chromium.org > --- > > boot/image.c | 1 + > include/image.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/boot/image.c b/boot/image.c > index abac254e026..cf16fb7fbf7 100644 > --- a/boot/image.c > +++ b/boot/image.c > @@ -183,6 +183,7 @@ static const table_entry_t uimage_type[] = { > { IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat Device Tree ", }, > { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" }, > { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" }, > + { IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" }, > { -1, "", "", }, > };
This whole part of the series feels like we're abusing IH_TYPE_ far past what it's used for. Especially the command line string one. It seems like the main use is to be able to later on print something human friendly. Please try and figure out some other way to do that. Thanks.
Yes I had the same thought, particularly with cmdline, as you say.
The obvious alternative is to just have a string which communicates the type. Then I can use IH_TYPE_INVALID, perhaps, with an extra string indicating what it really is.
But one advantage of the approach in this patch is that, for bootmeths which include a cmdline, it can be identified and added as a file. That includes zimage and ChromiumOS, but not EFI. It will allow me to get rid of the cmdline in struct bootflow, perhaps.
Yeah, this all sounds like IH_TYPE is the wrong direction to expand. Where you're grabbing these things from is where you might get a hint as to what these things are and that's where to take the store the knowledge for later.
They are grabbed from several places:
- extlinux comes from disk, with the extlinux bootmeth, but can also
come from 'include' files within that scripts, also pxe
- EFI apps from from disk with the EFI bootmeth
- kernel, ramdisk and fdt some from most bootmeths
- cmdline comes from extlinux (with zImage only) and ChromeOS,
probably Android but I haven't looked
- logo comes just from script so far, but I suppose UKI and FIT will
have it eventually
Can you be more specific with your suggestion? I am not seeing an alternative right now.
I'm just saying that this is not what IH_TYPE is. As you said maybe you can store this string information elsewhere? And I'm suggesting that perhaps when you load a thing you'll then also know what the thing is.
Yes, I agree. 'Type' is really about the actual file that you are reading, not the data types of the bits within it, for example. That's the problem with my approach.
The other idea I had was to add a new thing to image.h (IH_DATA_xxx). But I wasn't sure it was worth it. Also, if you look at fit_image_load() it reads an image of a certain type out of an existing (containing) FIT. So we already have images within images.
How about I just put a data type in bootflow.h ? Then it will be separate from image.h and I'm not blurring the line any further between an image and its component parts? We can always change it later if we find a better way.
Certainly somewhere else, and then lets move this to the part of the conversion on patch 23 where I suggest another way to handle these issues.

Add an image type for EFI applications, which U-Boot supports reading.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index cf16fb7fbf7..b13644b03dc 100644 --- a/boot/image.c +++ b/boot/image.c @@ -184,6 +184,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" }, { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" }, { IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" }, + { IH_TYPE_EFI, "efi", "EFI applicaiton" }, { -1, "", "", }, };
diff --git a/include/image.h b/include/image.h index 606e7d49169..a04925aa829 100644 --- a/include/image.h +++ b/include/image.h @@ -233,6 +233,7 @@ enum image_type_t { IH_TYPE_RENESAS_SPKG, /* Renesas SPKG image */ IH_TYPE_STARFIVE_SPL, /* StarFive SPL image */ IH_TYPE_EXTLINUX_CFG, /* extlinux configuration-file */ + IH_TYPE_EFI, /* EFI PE image */
IH_TYPE_COUNT, /* Number of image types */ };

From: Simon Glass sjg@chromium.org Date: Thu, 17 Oct 2024 17:23:55 -0600
Hi Simon,
Add an image type for EFI applications, which U-Boot supports reading.
Not necessarily a bad idea, but what would the image type be of a Linux kernel that is also an EFI application? See:
https://docs.kernel.org/admin-guide/efi-stub.html
Cheers,
Mark
Signed-off-by: Simon Glass sjg@chromium.org
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index cf16fb7fbf7..b13644b03dc 100644 --- a/boot/image.c +++ b/boot/image.c @@ -184,6 +184,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" }, { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" }, { IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" },
- { IH_TYPE_EFI, "efi", "EFI applicaiton" }, { -1, "", "", },
};
diff --git a/include/image.h b/include/image.h index 606e7d49169..a04925aa829 100644 --- a/include/image.h +++ b/include/image.h @@ -233,6 +233,7 @@ enum image_type_t { IH_TYPE_RENESAS_SPKG, /* Renesas SPKG image */ IH_TYPE_STARFIVE_SPL, /* StarFive SPL image */ IH_TYPE_EXTLINUX_CFG, /* extlinux configuration-file */
IH_TYPE_EFI, /* EFI PE image */
IH_TYPE_COUNT, /* Number of image types */
};
2.34.1

Hi Mark,
On Fri, 18 Oct 2024 at 03:03, Mark Kettenis mark.kettenis@xs4all.nl wrote:
From: Simon Glass sjg@chromium.org Date: Thu, 17 Oct 2024 17:23:55 -0600
Hi Simon,
Add an image type for EFI applications, which U-Boot supports reading.
Not necessarily a bad idea, but what would the image type be of a Linux kernel that is also an EFI application? See:
Hmm that's a good question. That is one of the challenges with EFI, that we don't know what we are booting. Is there a way to tell whether an EFI app is a kernel, or not?
For PXE/extlinux we have a 'kernel' line and in that case I set it to IH_TYPE_KERNEL
Cheers,
Mark
Signed-off-by: Simon Glass sjg@chromium.org
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index cf16fb7fbf7..b13644b03dc 100644 --- a/boot/image.c +++ b/boot/image.c @@ -184,6 +184,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" }, { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" }, { IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" },
{ IH_TYPE_EFI, "efi", "EFI applicaiton" }, { -1, "", "", },
};
diff --git a/include/image.h b/include/image.h index 606e7d49169..a04925aa829 100644 --- a/include/image.h +++ b/include/image.h @@ -233,6 +233,7 @@ enum image_type_t { IH_TYPE_RENESAS_SPKG, /* Renesas SPKG image */ IH_TYPE_STARFIVE_SPL, /* StarFive SPL image */ IH_TYPE_EXTLINUX_CFG, /* extlinux configuration-file */
IH_TYPE_EFI, /* EFI PE image */ IH_TYPE_COUNT, /* Number of image types */
};
2.34.1
Regards, Simon

Add an image type for logo images, which U-Boot supports reading within the PXE code.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index b13644b03dc..eaba9eb4332 100644 --- a/boot/image.c +++ b/boot/image.c @@ -185,6 +185,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_STARFIVE_SPL, "sfspl", "StarFive SPL Image" }, { IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" }, { IH_TYPE_EFI, "efi", "EFI applicaiton" }, + { IH_TYPE_LOGO, "logo", "Logo image" }, { -1, "", "", }, };
diff --git a/include/image.h b/include/image.h index a04925aa829..0d1adee499b 100644 --- a/include/image.h +++ b/include/image.h @@ -234,6 +234,7 @@ enum image_type_t { IH_TYPE_STARFIVE_SPL, /* StarFive SPL image */ IH_TYPE_EXTLINUX_CFG, /* extlinux configuration-file */ IH_TYPE_EFI, /* EFI PE image */ + IH_TYPE_LOGO, /* Image / logo */
IH_TYPE_COUNT, /* Number of image types */ };

Some bootmeths package a commandline into the bootflow. Add support for this as a file type.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image.c | 1 + include/image.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/boot/image.c b/boot/image.c index eaba9eb4332..f75b1e7841c 100644 --- a/boot/image.c +++ b/boot/image.c @@ -186,6 +186,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_EXTLINUX_CFG, "extlinux_cfg", "Extlinux configuration" }, { IH_TYPE_EFI, "efi", "EFI applicaiton" }, { IH_TYPE_LOGO, "logo", "Logo image" }, + { IH_TYPE_CMDLINE, "cmdline", "Command-line string" }, { -1, "", "", }, };
diff --git a/include/image.h b/include/image.h index 0d1adee499b..ff23fdbe22a 100644 --- a/include/image.h +++ b/include/image.h @@ -235,6 +235,7 @@ enum image_type_t { IH_TYPE_EXTLINUX_CFG, /* extlinux configuration-file */ IH_TYPE_EFI, /* EFI PE image */ IH_TYPE_LOGO, /* Image / logo */ + IH_TYPE_CMDLINE, /* OS command-line string */
IH_TYPE_COUNT, /* Number of image types */ };

This function assumes that all tests in a suite are being run. This means that it can sometimes call dm_test_restore() when it should not.
The impact of this is that it is not possible, for example, to run 'ut bootstd bootflow_cros' and then check the state of bootstd afterwards, since all devices are removed and recreated.
Update the function to take account of any selected test, to avoid this problem.
Add a comment for test_insert while we are here.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/test-main.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/test/test-main.c b/test/test-main.c index da5b07ce00b..ca6a073a8cb 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -240,15 +240,22 @@ static bool test_matches(const char *prefix, const char *test_name, * ut_list_has_dm_tests() - Check if a list of tests has driver model ones * * @tests: List of tests to run - * @count: Number of tests to ru + * @count: Number of tests to run + * @prefix: String prefix for the tests. Any tests that have this prefix will be + * printed without the prefix, so that it is easier to see the unique part + * of the test name. If NULL, no prefix processing is done + * @select_name: Name of a single test being run (from the list provided). If + * NULL all tests are being run * Return: true if any of the tests have the UTF_DM flag */ -static bool ut_list_has_dm_tests(struct unit_test *tests, int count) +static bool ut_list_has_dm_tests(struct unit_test *tests, int count, + const char *prefix, const char *select_name) { struct unit_test *test;
for (test = tests; test < tests + count; test++) { - if (test->flags & UTF_DM) + if (test_matches(prefix, test->name, select_name) && + (test->flags & UTF_DM)) return true; }
@@ -550,6 +557,9 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, * @count: Number of tests to run * @select_name: Name of a single test to run (from the list provided). If NULL * then all tests are run + * @test_insert: String describing a test to run after n other tests run, in the + * format n:name where n is the number of tests to run before this one and + * name is the name of the test to run * Return: 0 if all tests passed, -ENOENT if test @select_name was not found, * -EBADF if any failed */ @@ -646,7 +656,7 @@ int ut_run_list(const char *category, const char *prefix, int ret;
if (!CONFIG_IS_ENABLED(OF_PLATDATA) && - ut_list_has_dm_tests(tests, count)) { + ut_list_has_dm_tests(tests, count, prefix, select_name)) { has_dm_tests = true; /* * If we have no device tree, or it only has a root node, then

The mkimage call is done twice. Remove the duplicate.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/py/tests/test_ut.py | 2 -- 1 file changed, 2 deletions(-)
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py index 39aa1035e34..9166c8f6b6e 100644 --- a/test/py/tests/test_ut.py +++ b/test/py/tests/test_ut.py @@ -208,8 +208,6 @@ booti ${kernel_addr_r} ${ramdisk_addr_r} ${fdt_addr_r} cons, f'echo here {kernel} {symlink}') os.symlink(kernel, symlink)
- u_boot_utils.run_and_log( - cons, f'mkimage -C none -A arm -T script -d {cmd_fname} {scr_fname}') complete = True
except ValueError as exc:

This test checks console output so should have the UTF_CONSOLE flag. Add it.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/boot/bootflow.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c index 8ea284098e7..efb82ee628b 100644 --- a/test/boot/bootflow.c +++ b/test/boot/bootflow.c @@ -370,7 +370,7 @@ static int bootflow_iter(struct unit_test_state *uts)
return 0; } -BOOTSTD_TEST(bootflow_iter, UTF_DM | UTF_SCAN_FDT); +BOOTSTD_TEST(bootflow_iter, UTF_DM | UTF_SCAN_FDT | UTF_CONSOLE);
#if defined(CONFIG_SANDBOX) && defined(CONFIG_BOOTMETH_GLOBAL) /* Check using the system bootdev */

This should really use an address rather than the buffer. Update it in the command.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/bootflow.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/cmd/bootflow.c b/cmd/bootflow.c index 5f656814b29..3e9769e0d42 100644 --- a/cmd/bootflow.c +++ b/cmd/bootflow.c @@ -403,7 +403,8 @@ static int do_bootflow_info(struct cmd_tbl *cmdtp, int flag, int argc, puts("(none)"); putc('\n'); if (bflow->x86_setup) - printf("X86 setup: %p\n", bflow->x86_setup); + printf("X86 setup: %lx\n", + (ulong)map_to_sysmem(bflow->x86_setup)); printf("Logo: %s\n", bflow->logo ? simple_xtoa((ulong)map_to_sysmem(bflow->logo)) : "(none)"); if (bflow->logo) {

We want to keep track of images which are loaded, or those which could perhaps be loaded. This will make it easier to manage memory allocation, as well as permit removal of the EFI set_efi_bootdev() hack.
Add a list of these, attached to the bootflow. For now the list is empty.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootflow.c | 7 +++++++ include/bootflow.h | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+)
diff --git a/boot/bootflow.c b/boot/bootflow.c index 72b51ebd49b..10bda8eac79 100644 --- a/boot/bootflow.c +++ b/boot/bootflow.c @@ -455,10 +455,13 @@ void bootflow_init(struct bootflow *bflow, struct udevice *bootdev, bflow->dev = bootdev; bflow->method = meth; bflow->state = BOOTFLOWST_BASE; + alist_init_struct(&bflow->images, struct bootflow_img); }
void bootflow_free(struct bootflow *bflow) { + struct bootflow_img *img; + free(bflow->name); free(bflow->subdir); free(bflow->fname); @@ -467,6 +470,10 @@ void bootflow_free(struct bootflow *bflow) free(bflow->os_name); free(bflow->fdt_fname); free(bflow->bootmeth_priv); + + alist_for_each(img, &bflow->images) + free(img->fname); + alist_empty(&bflow->images); }
void bootflow_remove(struct bootflow *bflow) diff --git a/include/bootflow.h b/include/bootflow.h index 9b24fb5c3eb..3d9a9e2025a 100644 --- a/include/bootflow.h +++ b/include/bootflow.h @@ -7,7 +7,9 @@ #ifndef __bootflow_h #define __bootflow_h
+#include <alist.h> #include <bootdev.h> +#include <image.h> #include <dm/ofnode_decl.h> #include <linux/list.h>
@@ -85,6 +87,7 @@ enum bootflow_flags_t { * @cmdline: OS command line, or NULL if not known (allocated) * @x86_setup: Pointer to x86 setup block inside @buf, NULL if not present * @bootmeth_priv: Private data for the bootmeth + * @images: List of loaded images (struct bootstd_img) */ struct bootflow { struct udevice *dev; @@ -109,6 +112,24 @@ struct bootflow { char *cmdline; void *x86_setup; void *bootmeth_priv; + struct alist images; +}; + +/** + * struct bootflow_img - Information about an image which has been loaded + * + * This keeps track of a single, loaded image. + * + * @fname: Filename used to load the image (allocated) + * @type: Image type (IH_TYPE_...) + * @addr: Address to which the image was loaded, 0 if not yet loaded + * @size: Size of the image + */ +struct bootflow_img { + char *fname; + enum image_type_t type; + ulong addr; + ulong size; };
/**

On Thu, Oct 17, 2024 at 05:24:02PM -0600, Simon Glass wrote:
We want to keep track of images which are loaded, or those which could perhaps be loaded. This will make it easier to manage memory allocation, as well as permit removal of the EFI set_efi_bootdev() hack.
Please stop calling it a "hack" in commits, that just sets things up to argue. Thanks.

Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org:
We want to keep track of images which are loaded, or those which could perhaps be loaded. This will make it easier to manage memory allocation, as well as permit removal of the EFI set_efi_bootdev() hack.
Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows.
No matter how an EFI binary is loaded, we have to create the matching device path. This requires knowledge of the source device and the filepath or URL. Please, ensure that you gather all necessary information.
Best regards
Heinrich
Add a list of these, attached to the bootflow. For now the list is empty.
Signed-off-by: Simon Glass sjg@chromium.org
boot/bootflow.c | 7 +++++++ include/bootflow.h | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+)
diff --git a/boot/bootflow.c b/boot/bootflow.c index 72b51ebd49b..10bda8eac79 100644 --- a/boot/bootflow.c +++ b/boot/bootflow.c @@ -455,10 +455,13 @@ void bootflow_init(struct bootflow *bflow, struct udevice *bootdev, bflow->dev = bootdev; bflow->method = meth; bflow->state = BOOTFLOWST_BASE;
- alist_init_struct(&bflow->images, struct bootflow_img);
}
void bootflow_free(struct bootflow *bflow) {
- struct bootflow_img *img;
- free(bflow->name); free(bflow->subdir); free(bflow->fname);
@@ -467,6 +470,10 @@ void bootflow_free(struct bootflow *bflow) free(bflow->os_name); free(bflow->fdt_fname); free(bflow->bootmeth_priv);
- alist_for_each(img, &bflow->images)
free(img->fname);
- alist_empty(&bflow->images);
}
void bootflow_remove(struct bootflow *bflow) diff --git a/include/bootflow.h b/include/bootflow.h index 9b24fb5c3eb..3d9a9e2025a 100644 --- a/include/bootflow.h +++ b/include/bootflow.h @@ -7,7 +7,9 @@ #ifndef __bootflow_h #define __bootflow_h
+#include <alist.h> #include <bootdev.h> +#include <image.h> #include <dm/ofnode_decl.h> #include <linux/list.h>
@@ -85,6 +87,7 @@ enum bootflow_flags_t {
- @cmdline: OS command line, or NULL if not known (allocated)
- @x86_setup: Pointer to x86 setup block inside @buf, NULL if not present
- @bootmeth_priv: Private data for the bootmeth
- @images: List of loaded images (struct bootstd_img)
*/ struct bootflow { struct udevice *dev; @@ -109,6 +112,24 @@ struct bootflow { char *cmdline; void *x86_setup; void *bootmeth_priv;
- struct alist images;
+};
+/**
- struct bootflow_img - Information about an image which has been loaded
- This keeps track of a single, loaded image.
- @fname: Filename used to load the image (allocated)
- @type: Image type (IH_TYPE_...)
- @addr: Address to which the image was loaded, 0 if not yet loaded
- @size: Size of the image
- */
+struct bootflow_img {
- char *fname;
- enum image_type_t type;
- ulong addr;
- ulong size;
};
/**

Hi Heinrich,
On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org:
We want to keep track of images which are loaded, or those which could perhaps be loaded. This will make it easier to manage memory allocation, as well as permit removal of the EFI set_efi_bootdev() hack.
I'll change this 'hack' to 'feature'.
Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows.
Yes, this series is only going to help if bootstd is used. For ad-hoc use, EFI will need to rely on the above feature, at least until someone can think of another solution.
No matter how an EFI binary is loaded, we have to create the matching device path. This requires knowledge of the source device and the filepath or URL. Please, ensure that you gather all necessary information.
OK I'll send a little series which shows how this can work. I had thought it was understood already, but your comment helps explain why no one has done it yet. In brief, the main advantage of bootstd over the scripts is that it has a full understanding of what is going on.
Best regards
Heinrich
Add a list of these, attached to the bootflow. For now the list is empty.
Signed-off-by: Simon Glass sjg@chromium.org
boot/bootflow.c | 7 +++++++ include/bootflow.h | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+)
diff --git a/boot/bootflow.c b/boot/bootflow.c index 72b51ebd49b..10bda8eac79 100644 --- a/boot/bootflow.c +++ b/boot/bootflow.c @@ -455,10 +455,13 @@ void bootflow_init(struct bootflow *bflow, struct udevice *bootdev, bflow->dev = bootdev; bflow->method = meth; bflow->state = BOOTFLOWST_BASE;
- alist_init_struct(&bflow->images, struct bootflow_img);
}
void bootflow_free(struct bootflow *bflow) {
- struct bootflow_img *img;
free(bflow->name); free(bflow->subdir); free(bflow->fname); @@ -467,6 +470,10 @@ void bootflow_free(struct bootflow *bflow) free(bflow->os_name); free(bflow->fdt_fname); free(bflow->bootmeth_priv);
- alist_for_each(img, &bflow->images)
- free(img->fname);
- alist_empty(&bflow->images);
}
void bootflow_remove(struct bootflow *bflow) diff --git a/include/bootflow.h b/include/bootflow.h index 9b24fb5c3eb..3d9a9e2025a 100644 --- a/include/bootflow.h +++ b/include/bootflow.h @@ -7,7 +7,9 @@ #ifndef __bootflow_h #define __bootflow_h
+#include <alist.h> #include <bootdev.h> +#include <image.h> #include <dm/ofnode_decl.h> #include <linux/list.h>
@@ -85,6 +87,7 @@ enum bootflow_flags_t {
- @cmdline: OS command line, or NULL if not known (allocated)
- @x86_setup: Pointer to x86 setup block inside @buf, NULL if not present
- @bootmeth_priv: Private data for the bootmeth
- @images: List of loaded images (struct bootstd_img)
*/ struct bootflow { struct udevice *dev; @@ -109,6 +112,24 @@ struct bootflow { char *cmdline; void *x86_setup; void *bootmeth_priv;
- struct alist images;
+};
+/**
- struct bootflow_img - Information about an image which has been loaded
- This keeps track of a single, loaded image.
- @fname: Filename used to load the image (allocated)
- @type: Image type (IH_TYPE_...)
- @addr: Address to which the image was loaded, 0 if not yet loaded
- @size: Size of the image
- */
+struct bootflow_img {
- char *fname;
- enum image_type_t type;
- ulong addr;
- ulong size;
};
/**
Regards, SImon

On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote:
Hi Heinrich,
On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org:
We want to keep track of images which are loaded, or those which could perhaps be loaded. This will make it easier to manage memory allocation, as well as permit removal of the EFI set_efi_bootdev() hack.
I'll change this 'hack' to 'feature'.
Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows.
Yes, this series is only going to help if bootstd is used. For ad-hoc use, EFI will need to rely on the above feature, at least until someone can think of another solution.
Perhaps I need to try and be clearer here than I might have been in the past. The consensus among off the shelf free software operating systems is "just give me an EFI interface". This simplifies things on their end if regardless of architecture it's the same interface. This means that in U-Boot we need to treat EFI as one of the primary interfaces. Not a novelty. Not a "some people might use". It is a frequent and commonly used feature.

Hi Tom,
On Fri, 18 Oct 2024 at 10:33, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote:
Hi Heinrich,
On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org:
We want to keep track of images which are loaded, or those which could perhaps be loaded. This will make it easier to manage memory allocation, as well as permit removal of the EFI set_efi_bootdev() hack.
I'll change this 'hack' to 'feature'.
Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows.
Yes, this series is only going to help if bootstd is used. For ad-hoc use, EFI will need to rely on the above feature, at least until someone can think of another solution.
Perhaps I need to try and be clearer here than I might have been in the past. The consensus among off the shelf free software operating systems is "just give me an EFI interface". This simplifies things on their end if regardless of architecture it's the same interface. This means that in U-Boot we need to treat EFI as one of the primary interfaces. Not a novelty. Not a "some people might use". It is a frequent and commonly used feature.
Yes, EFI is everywhere and growing. All the more reason to tidy up this piece. I would like to see bootmgr use this new API, for example.
But how does this comment affect this patch?
Regards, Simon

On Fri, Oct 18, 2024 at 11:20:52AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 10:33, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote:
Hi Heinrich,
On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org:
We want to keep track of images which are loaded, or those which could perhaps be loaded. This will make it easier to manage memory allocation, as well as permit removal of the EFI set_efi_bootdev() hack.
I'll change this 'hack' to 'feature'.
Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows.
Yes, this series is only going to help if bootstd is used. For ad-hoc use, EFI will need to rely on the above feature, at least until someone can think of another solution.
Perhaps I need to try and be clearer here than I might have been in the past. The consensus among off the shelf free software operating systems is "just give me an EFI interface". This simplifies things on their end if regardless of architecture it's the same interface. This means that in U-Boot we need to treat EFI as one of the primary interfaces. Not a novelty. Not a "some people might use". It is a frequent and commonly used feature.
Yes, EFI is everywhere and growing. All the more reason to tidy up this piece. I would like to see bootmgr use this new API, for example.
But how does this comment affect this patch?
Because at the very high level, I wonder if I made a mistake a few years back. As I understand it, the nominal case is "bootefi bootmgr". I was saying at the time that perhaps bootstd can just fire that off, and move on. Now it seems like we're going along the path of re-inventing that, and not integrating well with it either.
So, to try and bring things back together. If U-Boot decides to load $FOO from device $BAR, at that common point is where we need to: - Is there an lmb for the location this is supposed to go to (for the if we know it, entire size)? - Note down everything else we know, now.
This means that we can note down enough stuff so that EFI can construct the path it needs. And if we're being told a filesystem, that filename is good enough for the IH_TYPE thing you're wanting, or at least a good chunk of it I hope.
It also means that since it's at the most common point, it doesn't matter if we were in an EFI application, a boot script, a bootmeth or someone at the cmdline doing "load mmc 0:1 /boot/Image $kernel_addr_r".

Hi Tom,
On Fri, 18 Oct 2024 at 12:04, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 11:20:52AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 10:33, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote:
Hi Heinrich,
On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org:
We want to keep track of images which are loaded, or those which could perhaps be loaded. This will make it easier to manage memory allocation, as well as permit removal of the EFI set_efi_bootdev() hack.
I'll change this 'hack' to 'feature'.
Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows.
Yes, this series is only going to help if bootstd is used. For ad-hoc use, EFI will need to rely on the above feature, at least until someone can think of another solution.
Perhaps I need to try and be clearer here than I might have been in the past. The consensus among off the shelf free software operating systems is "just give me an EFI interface". This simplifies things on their end if regardless of architecture it's the same interface. This means that in U-Boot we need to treat EFI as one of the primary interfaces. Not a novelty. Not a "some people might use". It is a frequent and commonly used feature.
Yes, EFI is everywhere and growing. All the more reason to tidy up this piece. I would like to see bootmgr use this new API, for example.
But how does this comment affect this patch?
Because at the very high level, I wonder if I made a mistake a few years back. As I understand it, the nominal case is "bootefi bootmgr". I was saying at the time that perhaps bootstd can just fire that off, and move on. Now it seems like we're going along the path of re-inventing that, and not integrating well with it either.
In what way are we re-inventing that? bootstd supports lots of different ways of booting, not just EFI. Also I hope that one day EFI will be implemented more as part of U-Boot than as a bolt-on, so will make use of bootflows, etc.
So, to try and bring things back together. If U-Boot decides to load $FOO from device $BAR, at that common point is where we need to:
- Is there an lmb for the location this is supposed to go to (for the if we know it, entire size)?
- Note down everything else we know, now.
Yes.
This means that we can note down enough stuff so that EFI can construct the path it needs. And if we're being told a filesystem, that filename is good enough for the IH_TYPE thing you're wanting, or at least a good chunk of it I hope.
You want me to ignore the type that I know (kernel, ramdisk, logo, etc.) and infer the type from a filename? Why?
For EFI there is only an EFI application. It will always just be a PE file. We don't really know what it is, as someone pointed out earlier. Maybe one day we will check to see if it is a UKI and pull things out of it. But then, it would be component parts (kernel, ramdisk, etc.) so I would want to add them as images.
It also means that since it's at the most common point, it doesn't matter if we were in an EFI application, a boot script, a bootmeth or someone at the cmdline doing "load mmc 0:1 /boot/Image $kernel_addr_r".
For that case (at the cmdline), bootstd is not currently running. Are you suggesting that bootstd could pick up these things and record them? If so, then yes, definitely, I want to do that. This series is the starting point for that. If you are suggesting something else, then I think I have lost you at this point.
Regards, Simon

On Fri, Oct 18, 2024 at 12:48:31PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 12:04, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 11:20:52AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 10:33, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote:
Hi Heinrich,
On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org: >We want to keep track of images which are loaded, or those which could >perhaps be loaded. This will make it easier to manage memory allocation, >as well as permit removal of the EFI set_efi_bootdev() hack.
I'll change this 'hack' to 'feature'.
Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows.
Yes, this series is only going to help if bootstd is used. For ad-hoc use, EFI will need to rely on the above feature, at least until someone can think of another solution.
Perhaps I need to try and be clearer here than I might have been in the past. The consensus among off the shelf free software operating systems is "just give me an EFI interface". This simplifies things on their end if regardless of architecture it's the same interface. This means that in U-Boot we need to treat EFI as one of the primary interfaces. Not a novelty. Not a "some people might use". It is a frequent and commonly used feature.
Yes, EFI is everywhere and growing. All the more reason to tidy up this piece. I would like to see bootmgr use this new API, for example.
But how does this comment affect this patch?
Because at the very high level, I wonder if I made a mistake a few years back. As I understand it, the nominal case is "bootefi bootmgr". I was saying at the time that perhaps bootstd can just fire that off, and move on. Now it seems like we're going along the path of re-inventing that, and not integrating well with it either.
In what way are we re-inventing that? bootstd supports lots of different ways of booting, not just EFI.
At the high level, bootflow scan is re-implementing "bootefi bootmgr". but handling non-EFI payloads.
Also I hope that one day EFI will be implemented more as part of U-Boot than as a bolt-on, so will make use of bootflows, etc.
And stuff like that is why I said what I said in here first. To me it sounds like you keep implying it's a hack that's not well integrated. When it's honestly at this point gotten more traction than FIT images have I think (as much as I wish FIT images had "won", it's like VHS vs Betamax, to bring in another technology metaphor).
So, to try and bring things back together. If U-Boot decides to load $FOO from device $BAR, at that common point is where we need to:
- Is there an lmb for the location this is supposed to go to (for the if we know it, entire size)?
- Note down everything else we know, now.
Yes.
This means that we can note down enough stuff so that EFI can construct the path it needs. And if we're being told a filesystem, that filename is good enough for the IH_TYPE thing you're wanting, or at least a good chunk of it I hope.
You want me to ignore the type that I know (kernel, ramdisk, logo, etc.) and infer the type from a filename? Why?
No, I want you to save and display the filename. That's probably much more useful when debugging than "kernel". If you actually know some other type information (ie extlinux.conf says ...) then yes, it too can be stored as that's useful too.
For EFI there is only an EFI application. It will always just be a PE file. We don't really know what it is, as someone pointed out earlier. Maybe one day we will check to see if it is a UKI and pull things out of it. But then, it would be component parts (kernel, ramdisk, etc.) so I would want to add them as images.
I don't see why yet, honestly.
It also means that since it's at the most common point, it doesn't matter if we were in an EFI application, a boot script, a bootmeth or someone at the cmdline doing "load mmc 0:1 /boot/Image $kernel_addr_r".
For that case (at the cmdline), bootstd is not currently running. Are you suggesting that bootstd could pick up these things and record them? If so, then yes, definitely, I want to do that. This series is the starting point for that. If you are suggesting something else, then I think I have lost you at this point.
Yes, I think I lost you somewhere, but I'm not sure how. What I am saying is that since everything at some point calls down to say fs_read() to read a file, that is the common point to note what we're doing. Not the load command, not the bootmeth, not the EFI_LOADER call.

Hi Tom,
On Fri, 18 Oct 2024 at 15:30, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 12:48:31PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 12:04, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 11:20:52AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 10:33, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote:
Hi Heinrich,
On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote: > > > > Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org: > >We want to keep track of images which are loaded, or those which could > >perhaps be loaded. This will make it easier to manage memory allocation, > >as well as permit removal of the EFI set_efi_bootdev() hack.
I'll change this 'hack' to 'feature'.
> > Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows.
Yes, this series is only going to help if bootstd is used. For ad-hoc use, EFI will need to rely on the above feature, at least until someone can think of another solution.
Perhaps I need to try and be clearer here than I might have been in the past. The consensus among off the shelf free software operating systems is "just give me an EFI interface". This simplifies things on their end if regardless of architecture it's the same interface. This means that in U-Boot we need to treat EFI as one of the primary interfaces. Not a novelty. Not a "some people might use". It is a frequent and commonly used feature.
Yes, EFI is everywhere and growing. All the more reason to tidy up this piece. I would like to see bootmgr use this new API, for example.
But how does this comment affect this patch?
Because at the very high level, I wonder if I made a mistake a few years back. As I understand it, the nominal case is "bootefi bootmgr". I was saying at the time that perhaps bootstd can just fire that off, and move on. Now it seems like we're going along the path of re-inventing that, and not integrating well with it either.
In what way are we re-inventing that? bootstd supports lots of different ways of booting, not just EFI.
At the high level, bootflow scan is re-implementing "bootefi bootmgr". but handling non-EFI payloads.
bootstd is about replacing the distro scripts, not bootmgr.
Also I hope that one day EFI will be implemented more as part of U-Boot than as a bolt-on, so will make use of bootflows, etc.
And stuff like that is why I said what I said in here first. To me it sounds like you keep implying it's a hack that's not well integrated. When it's honestly at this point gotten more traction than FIT images have I think (as much as I wish FIT images had "won", it's like VHS vs Betamax, to bring in another technology metaphor).
The 'hack' I was referring to is efi_set_bootdev(), not EFI_LOADER as a whole!
So, to try and bring things back together. If U-Boot decides to load $FOO from device $BAR, at that common point is where we need to:
- Is there an lmb for the location this is supposed to go to (for the if we know it, entire size)?
- Note down everything else we know, now.
Yes.
This means that we can note down enough stuff so that EFI can construct the path it needs. And if we're being told a filesystem, that filename is good enough for the IH_TYPE thing you're wanting, or at least a good chunk of it I hope.
You want me to ignore the type that I know (kernel, ramdisk, logo, etc.) and infer the type from a filename? Why?
No, I want you to save and display the filename. That's probably much more useful when debugging than "kernel". If you actually know some other type information (ie extlinux.conf says ...) then yes, it too can be stored as that's useful too.
The filename is already saved in bootflow->filename, and now it is in struct bootflow_img.
For EFI there is only an EFI application. It will always just be a PE file. We don't really know what it is, as someone pointed out earlier. Maybe one day we will check to see if it is a UKI and pull things out of it. But then, it would be component parts (kernel, ramdisk, etc.) so I would want to add them as images.
I don't see why yet, honestly.
For the cmdline, 'bootflow cmdline' allows editing it, for example. For a logo we can display it in the menu. The filename doesn't tell us what it is.
It also means that since it's at the most common point, it doesn't matter if we were in an EFI application, a boot script, a bootmeth or someone at the cmdline doing "load mmc 0:1 /boot/Image $kernel_addr_r".
For that case (at the cmdline), bootstd is not currently running. Are you suggesting that bootstd could pick up these things and record them? If so, then yes, definitely, I want to do that. This series is the starting point for that. If you are suggesting something else, then I think I have lost you at this point.
Yes, I think I lost you somewhere, but I'm not sure how. What I am saying is that since everything at some point calls down to say fs_read() to read a file, that is the common point to note what we're doing. Not the load command, not the bootmeth, not the EFI_LOADER call.
Perhaps what you are missing is that bootstd is a proper boot implementation for U-Boot, where U-Boot knows what is going on. Ignoring that information and just hooking into fs_read() is like throwing away the plan and trying to recreate it from photographs.
That said, yes I can hook into fs_read() and store that info, as it is better than nothing, if bootstd is not being used, or there is a script.
Regards, Simon

On Sat, Oct 19, 2024 at 05:49:40AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 15:30, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 12:48:31PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 12:04, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 11:20:52AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 10:33, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote: > Hi Heinrich, > > On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote: > > > > > > > > Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org: > > >We want to keep track of images which are loaded, or those which could > > >perhaps be loaded. This will make it easier to manage memory allocation, > > >as well as permit removal of the EFI set_efi_bootdev() hack. > > I'll change this 'hack' to 'feature'. > > > > > Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows. > > Yes, this series is only going to help if bootstd is used. For ad-hoc > use, EFI will need to rely on the above feature, at least until > someone can think of another solution.
Perhaps I need to try and be clearer here than I might have been in the past. The consensus among off the shelf free software operating systems is "just give me an EFI interface". This simplifies things on their end if regardless of architecture it's the same interface. This means that in U-Boot we need to treat EFI as one of the primary interfaces. Not a novelty. Not a "some people might use". It is a frequent and commonly used feature.
Yes, EFI is everywhere and growing. All the more reason to tidy up this piece. I would like to see bootmgr use this new API, for example.
But how does this comment affect this patch?
Because at the very high level, I wonder if I made a mistake a few years back. As I understand it, the nominal case is "bootefi bootmgr". I was saying at the time that perhaps bootstd can just fire that off, and move on. Now it seems like we're going along the path of re-inventing that, and not integrating well with it either.
In what way are we re-inventing that? bootstd supports lots of different ways of booting, not just EFI.
At the high level, bootflow scan is re-implementing "bootefi bootmgr". but handling non-EFI payloads.
bootstd is about replacing the distro scripts, not bootmgr.
And the distro scripts are functionally replaced by "bootefi bootmgr", outside of bootstd.
Also I hope that one day EFI will be implemented more as part of U-Boot than as a bolt-on, so will make use of bootflows, etc.
And stuff like that is why I said what I said in here first. To me it sounds like you keep implying it's a hack that's not well integrated. When it's honestly at this point gotten more traction than FIT images have I think (as much as I wish FIT images had "won", it's like VHS vs Betamax, to bring in another technology metaphor).
The 'hack' I was referring to is efi_set_bootdev(), not EFI_LOADER as a whole!
I wasn't clear enough, sorry. I didn't mean just in this series where you referred to storing the needed property as a hack but rather "bolt-on" in what I quoted and "tidy up this" and "tidy up that". I'm just saying what impression your words leave with me, and quite possibly others.
So, to try and bring things back together. If U-Boot decides to load $FOO from device $BAR, at that common point is where we need to:
- Is there an lmb for the location this is supposed to go to (for the if we know it, entire size)?
- Note down everything else we know, now.
Yes.
This means that we can note down enough stuff so that EFI can construct the path it needs. And if we're being told a filesystem, that filename is good enough for the IH_TYPE thing you're wanting, or at least a good chunk of it I hope.
You want me to ignore the type that I know (kernel, ramdisk, logo, etc.) and infer the type from a filename? Why?
No, I want you to save and display the filename. That's probably much more useful when debugging than "kernel". If you actually know some other type information (ie extlinux.conf says ...) then yes, it too can be stored as that's useful too.
The filename is already saved in bootflow->filename, and now it is in struct bootflow_img.
OK. But that's not generic enough.
For EFI there is only an EFI application. It will always just be a PE file. We don't really know what it is, as someone pointed out earlier. Maybe one day we will check to see if it is a UKI and pull things out of it. But then, it would be component parts (kernel, ramdisk, etc.) so I would want to add them as images.
I don't see why yet, honestly.
For the cmdline, 'bootflow cmdline' allows editing it, for example. For a logo we can display it in the menu. The filename doesn't tell us what it is.
There may, or may not, be further context clues about what a blob is, yes, and we should make use of them. But we may not have them. Frankly for a logo we probably need to have dealt with that upon initializing the display, so earlier in the process. But this is all getting away from the point I'm trying to make.
It also means that since it's at the most common point, it doesn't matter if we were in an EFI application, a boot script, a bootmeth or someone at the cmdline doing "load mmc 0:1 /boot/Image $kernel_addr_r".
For that case (at the cmdline), bootstd is not currently running. Are you suggesting that bootstd could pick up these things and record them? If so, then yes, definitely, I want to do that. This series is the starting point for that. If you are suggesting something else, then I think I have lost you at this point.
Yes, I think I lost you somewhere, but I'm not sure how. What I am saying is that since everything at some point calls down to say fs_read() to read a file, that is the common point to note what we're doing. Not the load command, not the bootmeth, not the EFI_LOADER call.
Perhaps what you are missing is that bootstd is a proper boot implementation for U-Boot, where U-Boot knows what is going on. Ignoring that information and just hooking into fs_read() is like throwing away the plan and trying to recreate it from photographs.
That said, yes I can hook into fs_read() and store that info, as it is better than nothing, if bootstd is not being used, or there is a script.
Maybe we need to throw out the plan and start over then? Since you seem to not be accounting for the rest of the common paths and assuming that your design can just be used as-is for them despite not taking them in to account in the design in the first place. Hooking in at around fs_read() or thereabouts is likely where we need to be at least starting some of this so that we do know information that we want to preserve. Similarly, there's the common path for "grab a file via HTTP(s)" and that is where we should be recording information for network loads. We can then augment that if it was a pxelinux.cfg file we grabbed or something else, but that's the common place to start gathering information.

Hi Tom,
On Sat, 19 Oct 2024 at 18:30, Tom Rini trini@konsulko.com wrote:
On Sat, Oct 19, 2024 at 05:49:40AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 15:30, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 12:48:31PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 12:04, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 11:20:52AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 10:33, Tom Rini trini@konsulko.com wrote: > > On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote: > > Hi Heinrich, > > > > On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote: > > > > > > > > > > > > Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org: > > > >We want to keep track of images which are loaded, or those which could > > > >perhaps be loaded. This will make it easier to manage memory allocation, > > > >as well as permit removal of the EFI set_efi_bootdev() hack. > > > > I'll change this 'hack' to 'feature'. > > > > > > > > Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows. > > > > Yes, this series is only going to help if bootstd is used. For ad-hoc > > use, EFI will need to rely on the above feature, at least until > > someone can think of another solution. > > Perhaps I need to try and be clearer here than I might have been in the > past. The consensus among off the shelf free software operating systems > is "just give me an EFI interface". This simplifies things on their end > if regardless of architecture it's the same interface. This means that > in U-Boot we need to treat EFI as one of the primary interfaces. Not a > novelty. Not a "some people might use". It is a frequent and commonly > used feature.
Yes, EFI is everywhere and growing. All the more reason to tidy up this piece. I would like to see bootmgr use this new API, for example.
But how does this comment affect this patch?
Because at the very high level, I wonder if I made a mistake a few years back. As I understand it, the nominal case is "bootefi bootmgr". I was saying at the time that perhaps bootstd can just fire that off, and move on. Now it seems like we're going along the path of re-inventing that, and not integrating well with it either.
In what way are we re-inventing that? bootstd supports lots of different ways of booting, not just EFI.
At the high level, bootflow scan is re-implementing "bootefi bootmgr". but handling non-EFI payloads.
bootstd is about replacing the distro scripts, not bootmgr.
And the distro scripts are functionally replaced by "bootefi bootmgr", outside of bootstd.
But that doesn't support anything other than EFI apps, so isn't useful for when EFI is not wanted.
Also I hope that one day EFI will be implemented more as part of U-Boot than as a bolt-on, so will make use of bootflows, etc.
And stuff like that is why I said what I said in here first. To me it sounds like you keep implying it's a hack that's not well integrated. When it's honestly at this point gotten more traction than FIT images have I think (as much as I wish FIT images had "won", it's like VHS vs Betamax, to bring in another technology metaphor).
The 'hack' I was referring to is efi_set_bootdev(), not EFI_LOADER as a whole!
I wasn't clear enough, sorry. I didn't mean just in this series where you referred to storing the needed property as a hack but rather "bolt-on" in what I quoted and "tidy up this" and "tidy up that". I'm just saying what impression your words leave with me, and quite possibly others.
OK I will try to be more gentle with my language.
So, to try and bring things back together. If U-Boot decides to load $FOO from device $BAR, at that common point is where we need to:
- Is there an lmb for the location this is supposed to go to (for the if we know it, entire size)?
- Note down everything else we know, now.
Yes.
This means that we can note down enough stuff so that EFI can construct the path it needs. And if we're being told a filesystem, that filename is good enough for the IH_TYPE thing you're wanting, or at least a good chunk of it I hope.
You want me to ignore the type that I know (kernel, ramdisk, logo, etc.) and infer the type from a filename? Why?
No, I want you to save and display the filename. That's probably much more useful when debugging than "kernel". If you actually know some other type information (ie extlinux.conf says ...) then yes, it too can be stored as that's useful too.
The filename is already saved in bootflow->filename, and now it is in struct bootflow_img.
OK. But that's not generic enough.
How about we see how things work out here rather than giving up at the start? It is pretty clear in my head, so far.
For EFI there is only an EFI application. It will always just be a PE file. We don't really know what it is, as someone pointed out earlier. Maybe one day we will check to see if it is a UKI and pull things out of it. But then, it would be component parts (kernel, ramdisk, etc.) so I would want to add them as images.
I don't see why yet, honestly.
For the cmdline, 'bootflow cmdline' allows editing it, for example. For a logo we can display it in the menu. The filename doesn't tell us what it is.
There may, or may not, be further context clues about what a blob is, yes, and we should make use of them. But we may not have them. Frankly for a logo we probably need to have dealt with that upon initializing the display, so earlier in the process. But this is all getting away from the point I'm trying to make.
It also means that since it's at the most common point, it doesn't matter if we were in an EFI application, a boot script, a bootmeth or someone at the cmdline doing "load mmc 0:1 /boot/Image $kernel_addr_r".
For that case (at the cmdline), bootstd is not currently running. Are you suggesting that bootstd could pick up these things and record them? If so, then yes, definitely, I want to do that. This series is the starting point for that. If you are suggesting something else, then I think I have lost you at this point.
Yes, I think I lost you somewhere, but I'm not sure how. What I am saying is that since everything at some point calls down to say fs_read() to read a file, that is the common point to note what we're doing. Not the load command, not the bootmeth, not the EFI_LOADER call.
Perhaps what you are missing is that bootstd is a proper boot implementation for U-Boot, where U-Boot knows what is going on. Ignoring that information and just hooking into fs_read() is like throwing away the plan and trying to recreate it from photographs.
That said, yes I can hook into fs_read() and store that info, as it is better than nothing, if bootstd is not being used, or there is a script.
Maybe we need to throw out the plan and start over then? Since you seem to not be accounting for the rest of the common paths and assuming that your design can just be used as-is for them despite not taking them in to account in the design in the first place. Hooking in at around fs_read() or thereabouts is likely where we need to be at least starting some of this so that we do know information that we want to preserve. Similarly, there's the common path for "grab a file via HTTP(s)" and that is where we should be recording information for network loads. We can then augment that if it was a pxelinux.cfg file we grabbed or something else, but that's the common place to start gathering information.
Here's what I see as the key point: bootstd knows what is being booted. It knows that the kernel is a kernel and where it came from. If there is a logo or cmdline, it knows about that too. It is sort-of the point of bootstd to make booting more automatic and deterministic. Where people are using scripts and ad-hoc ways of booting, they just aren't going to get the same level of control and visibility as bootstd can provide.
I think I recall having exactly this discussion before?
I will see if I can construct a small series to show what an ad-hoc bootflow might look like. It will at least collect loaded files into one place so they can be examined, etc.
Regards, Simon

On Tue, Oct 22, 2024 at 02:16:52PM +0200, Simon Glass wrote:
Hi Tom,
On Sat, 19 Oct 2024 at 18:30, Tom Rini trini@konsulko.com wrote:
On Sat, Oct 19, 2024 at 05:49:40AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 15:30, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 12:48:31PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 12:04, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 11:20:52AM -0600, Simon Glass wrote: > Hi Tom, > > On Fri, 18 Oct 2024 at 10:33, Tom Rini trini@konsulko.com wrote: > > > > On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote: > > > Hi Heinrich, > > > > > > On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote: > > > > > > > > > > > > > > > > Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org: > > > > >We want to keep track of images which are loaded, or those which could > > > > >perhaps be loaded. This will make it easier to manage memory allocation, > > > > >as well as permit removal of the EFI set_efi_bootdev() hack. > > > > > > I'll change this 'hack' to 'feature'. > > > > > > > > > > > Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows. > > > > > > Yes, this series is only going to help if bootstd is used. For ad-hoc > > > use, EFI will need to rely on the above feature, at least until > > > someone can think of another solution. > > > > Perhaps I need to try and be clearer here than I might have been in the > > past. The consensus among off the shelf free software operating systems > > is "just give me an EFI interface". This simplifies things on their end > > if regardless of architecture it's the same interface. This means that > > in U-Boot we need to treat EFI as one of the primary interfaces. Not a > > novelty. Not a "some people might use". It is a frequent and commonly > > used feature. > > Yes, EFI is everywhere and growing. All the more reason to tidy up > this piece. I would like to see bootmgr use this new API, for example. > > But how does this comment affect this patch?
Because at the very high level, I wonder if I made a mistake a few years back. As I understand it, the nominal case is "bootefi bootmgr". I was saying at the time that perhaps bootstd can just fire that off, and move on. Now it seems like we're going along the path of re-inventing that, and not integrating well with it either.
In what way are we re-inventing that? bootstd supports lots of different ways of booting, not just EFI.
At the high level, bootflow scan is re-implementing "bootefi bootmgr". but handling non-EFI payloads.
bootstd is about replacing the distro scripts, not bootmgr.
And the distro scripts are functionally replaced by "bootefi bootmgr", outside of bootstd.
But that doesn't support anything other than EFI apps, so isn't useful for when EFI is not wanted.
Yes, the distro scripts wanted to move more firmly / quickly in this direction. For all of the reasons they've elaborated before.
Also I hope that one day EFI will be implemented more as part of U-Boot than as a bolt-on, so will make use of bootflows, etc.
And stuff like that is why I said what I said in here first. To me it sounds like you keep implying it's a hack that's not well integrated. When it's honestly at this point gotten more traction than FIT images have I think (as much as I wish FIT images had "won", it's like VHS vs Betamax, to bring in another technology metaphor).
The 'hack' I was referring to is efi_set_bootdev(), not EFI_LOADER as a whole!
I wasn't clear enough, sorry. I didn't mean just in this series where you referred to storing the needed property as a hack but rather "bolt-on" in what I quoted and "tidy up this" and "tidy up that". I'm just saying what impression your words leave with me, and quite possibly others.
OK I will try to be more gentle with my language.
Thanks.
So, to try and bring things back together. If U-Boot decides to load $FOO from device $BAR, at that common point is where we need to:
- Is there an lmb for the location this is supposed to go to (for the if we know it, entire size)?
- Note down everything else we know, now.
Yes.
This means that we can note down enough stuff so that EFI can construct the path it needs. And if we're being told a filesystem, that filename is good enough for the IH_TYPE thing you're wanting, or at least a good chunk of it I hope.
You want me to ignore the type that I know (kernel, ramdisk, logo, etc.) and infer the type from a filename? Why?
No, I want you to save and display the filename. That's probably much more useful when debugging than "kernel". If you actually know some other type information (ie extlinux.conf says ...) then yes, it too can be stored as that's useful too.
The filename is already saved in bootflow->filename, and now it is in struct bootflow_img.
OK. But that's not generic enough.
How about we see how things work out here rather than giving up at the start? It is pretty clear in my head, so far.
I'd really rather not since it looks like it's starting in the wrong direction. I really do not understand why when we load the file / do the network request / read the flash area / etc is the wrong place to start recording the information about what we load.
For EFI there is only an EFI application. It will always just be a PE file. We don't really know what it is, as someone pointed out earlier. Maybe one day we will check to see if it is a UKI and pull things out of it. But then, it would be component parts (kernel, ramdisk, etc.) so I would want to add them as images.
I don't see why yet, honestly.
For the cmdline, 'bootflow cmdline' allows editing it, for example. For a logo we can display it in the menu. The filename doesn't tell us what it is.
There may, or may not, be further context clues about what a blob is, yes, and we should make use of them. But we may not have them. Frankly for a logo we probably need to have dealt with that upon initializing the display, so earlier in the process. But this is all getting away from the point I'm trying to make.
It also means that since it's at the most common point, it doesn't matter if we were in an EFI application, a boot script, a bootmeth or someone at the cmdline doing "load mmc 0:1 /boot/Image $kernel_addr_r".
For that case (at the cmdline), bootstd is not currently running. Are you suggesting that bootstd could pick up these things and record them? If so, then yes, definitely, I want to do that. This series is the starting point for that. If you are suggesting something else, then I think I have lost you at this point.
Yes, I think I lost you somewhere, but I'm not sure how. What I am saying is that since everything at some point calls down to say fs_read() to read a file, that is the common point to note what we're doing. Not the load command, not the bootmeth, not the EFI_LOADER call.
Perhaps what you are missing is that bootstd is a proper boot implementation for U-Boot, where U-Boot knows what is going on. Ignoring that information and just hooking into fs_read() is like throwing away the plan and trying to recreate it from photographs.
That said, yes I can hook into fs_read() and store that info, as it is better than nothing, if bootstd is not being used, or there is a script.
Maybe we need to throw out the plan and start over then? Since you seem to not be accounting for the rest of the common paths and assuming that your design can just be used as-is for them despite not taking them in to account in the design in the first place. Hooking in at around fs_read() or thereabouts is likely where we need to be at least starting some of this so that we do know information that we want to preserve. Similarly, there's the common path for "grab a file via HTTP(s)" and that is where we should be recording information for network loads. We can then augment that if it was a pxelinux.cfg file we grabbed or something else, but that's the common place to start gathering information.
Here's what I see as the key point: bootstd knows what is being booted. It knows that the kernel is a kernel and where it came from. If there is a logo or cmdline, it knows about that too. It is sort-of the point of bootstd to make booting more automatic and deterministic. Where people are using scripts and ad-hoc ways of booting, they just aren't going to get the same level of control and visibility as bootstd can provide.
I think I recall having exactly this discussion before?
I will see if I can construct a small series to show what an ad-hoc bootflow might look like. It will at least collect loaded files into one place so they can be examined, etc.
Yes, perhaps some series that shows what works / doesn't work and how it works would be helpful. I'd really like to see where you're starting your abstractions from.

Hi Tom,
On Tue, 22 Oct 2024 at 17:01, Tom Rini trini@konsulko.com wrote:
On Tue, Oct 22, 2024 at 02:16:52PM +0200, Simon Glass wrote:
Hi Tom,
On Sat, 19 Oct 2024 at 18:30, Tom Rini trini@konsulko.com wrote:
On Sat, Oct 19, 2024 at 05:49:40AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 15:30, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 12:48:31PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 12:04, Tom Rini trini@konsulko.com wrote: > > On Fri, Oct 18, 2024 at 11:20:52AM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Fri, 18 Oct 2024 at 10:33, Tom Rini trini@konsulko.com wrote: > > > > > > On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote: > > > > Hi Heinrich, > > > > > > > > On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote: > > > > > > > > > > > > > > > > > > > > Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org: > > > > > >We want to keep track of images which are loaded, or those which could > > > > > >perhaps be loaded. This will make it easier to manage memory allocation, > > > > > >as well as permit removal of the EFI set_efi_bootdev() hack. > > > > > > > > I'll change this 'hack' to 'feature'. > > > > > > > > > > > > > > Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows. > > > > > > > > Yes, this series is only going to help if bootstd is used. For ad-hoc > > > > use, EFI will need to rely on the above feature, at least until > > > > someone can think of another solution. > > > > > > Perhaps I need to try and be clearer here than I might have been in the > > > past. The consensus among off the shelf free software operating systems > > > is "just give me an EFI interface". This simplifies things on their end > > > if regardless of architecture it's the same interface. This means that > > > in U-Boot we need to treat EFI as one of the primary interfaces. Not a > > > novelty. Not a "some people might use". It is a frequent and commonly > > > used feature. > > > > Yes, EFI is everywhere and growing. All the more reason to tidy up > > this piece. I would like to see bootmgr use this new API, for example. > > > > But how does this comment affect this patch? > > Because at the very high level, I wonder if I made a mistake a few years > back. As I understand it, the nominal case is "bootefi bootmgr". I was > saying at the time that perhaps bootstd can just fire that off, and move > on. Now it seems like we're going along the path of re-inventing that, > and not integrating well with it either.
In what way are we re-inventing that? bootstd supports lots of different ways of booting, not just EFI.
At the high level, bootflow scan is re-implementing "bootefi bootmgr". but handling non-EFI payloads.
bootstd is about replacing the distro scripts, not bootmgr.
And the distro scripts are functionally replaced by "bootefi bootmgr", outside of bootstd.
But that doesn't support anything other than EFI apps, so isn't useful for when EFI is not wanted.
Yes, the distro scripts wanted to move more firmly / quickly in this direction. For all of the reasons they've elaborated before.
OK, well good luck to them, I suppose.
Also I hope that one day EFI will be implemented more as part of U-Boot than as a bolt-on, so will make use of bootflows, etc.
And stuff like that is why I said what I said in here first. To me it sounds like you keep implying it's a hack that's not well integrated. When it's honestly at this point gotten more traction than FIT images have I think (as much as I wish FIT images had "won", it's like VHS vs Betamax, to bring in another technology metaphor).
The 'hack' I was referring to is efi_set_bootdev(), not EFI_LOADER as a whole!
I wasn't clear enough, sorry. I didn't mean just in this series where you referred to storing the needed property as a hack but rather "bolt-on" in what I quoted and "tidy up this" and "tidy up that". I'm just saying what impression your words leave with me, and quite possibly others.
OK I will try to be more gentle with my language.
Thanks.
> So, to try and bring things back together. If U-Boot decides to load > $FOO from device $BAR, at that common point is where we need to: > - Is there an lmb for the location this is supposed to go to (for the if > we know it, entire size)? > - Note down everything else we know, now.
Yes.
> > This means that we can note down enough stuff so that EFI can construct > the path it needs. And if we're being told a filesystem, that filename > is good enough for the IH_TYPE thing you're wanting, or at least a good > chunk of it I hope.
You want me to ignore the type that I know (kernel, ramdisk, logo, etc.) and infer the type from a filename? Why?
No, I want you to save and display the filename. That's probably much more useful when debugging than "kernel". If you actually know some other type information (ie extlinux.conf says ...) then yes, it too can be stored as that's useful too.
The filename is already saved in bootflow->filename, and now it is in struct bootflow_img.
OK. But that's not generic enough.
How about we see how things work out here rather than giving up at the start? It is pretty clear in my head, so far.
I'd really rather not since it looks like it's starting in the wrong direction. I really do not understand why when we load the file / do the network request / read the flash area / etc is the wrong place to start recording the information about what we load.
It isn't that it is the wrong place, it's just that we know more when a bootmeth is in place. Are you thinking about the distro scripts or people's custom scripts, or something else?
For EFI there is only an EFI application. It will always just be a PE file. We don't really know what it is, as someone pointed out earlier. Maybe one day we will check to see if it is a UKI and pull things out of it. But then, it would be component parts (kernel, ramdisk, etc.) so I would want to add them as images.
I don't see why yet, honestly.
For the cmdline, 'bootflow cmdline' allows editing it, for example. For a logo we can display it in the menu. The filename doesn't tell us what it is.
There may, or may not, be further context clues about what a blob is, yes, and we should make use of them. But we may not have them. Frankly for a logo we probably need to have dealt with that upon initializing the display, so earlier in the process. But this is all getting away from the point I'm trying to make.
> It also means that since it's at the most common point, it doesn't > matter if we were in an EFI application, a boot script, a bootmeth or > someone at the cmdline doing "load mmc 0:1 /boot/Image $kernel_addr_r".
For that case (at the cmdline), bootstd is not currently running. Are you suggesting that bootstd could pick up these things and record them? If so, then yes, definitely, I want to do that. This series is the starting point for that. If you are suggesting something else, then I think I have lost you at this point.
Yes, I think I lost you somewhere, but I'm not sure how. What I am saying is that since everything at some point calls down to say fs_read() to read a file, that is the common point to note what we're doing. Not the load command, not the bootmeth, not the EFI_LOADER call.
Perhaps what you are missing is that bootstd is a proper boot implementation for U-Boot, where U-Boot knows what is going on. Ignoring that information and just hooking into fs_read() is like throwing away the plan and trying to recreate it from photographs.
That said, yes I can hook into fs_read() and store that info, as it is better than nothing, if bootstd is not being used, or there is a script.
Maybe we need to throw out the plan and start over then? Since you seem to not be accounting for the rest of the common paths and assuming that your design can just be used as-is for them despite not taking them in to account in the design in the first place. Hooking in at around fs_read() or thereabouts is likely where we need to be at least starting some of this so that we do know information that we want to preserve. Similarly, there's the common path for "grab a file via HTTP(s)" and that is where we should be recording information for network loads. We can then augment that if it was a pxelinux.cfg file we grabbed or something else, but that's the common place to start gathering information.
Here's what I see as the key point: bootstd knows what is being booted. It knows that the kernel is a kernel and where it came from. If there is a logo or cmdline, it knows about that too. It is sort-of the point of bootstd to make booting more automatic and deterministic. Where people are using scripts and ad-hoc ways of booting, they just aren't going to get the same level of control and visibility as bootstd can provide.
I think I recall having exactly this discussion before?
I will see if I can construct a small series to show what an ad-hoc bootflow might look like. It will at least collect loaded files into one place so they can be examined, etc.
Yes, perhaps some series that shows what works / doesn't work and how it works would be helpful. I'd really like to see where you're starting your abstractions from.
OK I'll send a version with an add-on patch for the 'load' command, with the concept of an ad-hoc bootflow.
Regards, Simon

On Tue, Oct 22, 2024 at 07:00:44PM +0200, Simon Glass wrote:
Hi Tom,
On Tue, 22 Oct 2024 at 17:01, Tom Rini trini@konsulko.com wrote:
On Tue, Oct 22, 2024 at 02:16:52PM +0200, Simon Glass wrote:
Hi Tom,
On Sat, 19 Oct 2024 at 18:30, Tom Rini trini@konsulko.com wrote:
On Sat, Oct 19, 2024 at 05:49:40AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 15:30, Tom Rini trini@konsulko.com wrote:
On Fri, Oct 18, 2024 at 12:48:31PM -0600, Simon Glass wrote: > Hi Tom, > > On Fri, 18 Oct 2024 at 12:04, Tom Rini trini@konsulko.com wrote: > > > > On Fri, Oct 18, 2024 at 11:20:52AM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Fri, 18 Oct 2024 at 10:33, Tom Rini trini@konsulko.com wrote: > > > > > > > > On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote: > > > > > Hi Heinrich, > > > > > > > > > > On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote: > > > > > > > > > > > > > > > > > > > > > > > > Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org: > > > > > > >We want to keep track of images which are loaded, or those which could > > > > > > >perhaps be loaded. This will make it easier to manage memory allocation, > > > > > > >as well as permit removal of the EFI set_efi_bootdev() hack. > > > > > > > > > > I'll change this 'hack' to 'feature'. > > > > > > > > > > > > > > > > > Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows. > > > > > > > > > > Yes, this series is only going to help if bootstd is used. For ad-hoc > > > > > use, EFI will need to rely on the above feature, at least until > > > > > someone can think of another solution. > > > > > > > > Perhaps I need to try and be clearer here than I might have been in the > > > > past. The consensus among off the shelf free software operating systems > > > > is "just give me an EFI interface". This simplifies things on their end > > > > if regardless of architecture it's the same interface. This means that > > > > in U-Boot we need to treat EFI as one of the primary interfaces. Not a > > > > novelty. Not a "some people might use". It is a frequent and commonly > > > > used feature. > > > > > > Yes, EFI is everywhere and growing. All the more reason to tidy up > > > this piece. I would like to see bootmgr use this new API, for example. > > > > > > But how does this comment affect this patch? > > > > Because at the very high level, I wonder if I made a mistake a few years > > back. As I understand it, the nominal case is "bootefi bootmgr". I was > > saying at the time that perhaps bootstd can just fire that off, and move > > on. Now it seems like we're going along the path of re-inventing that, > > and not integrating well with it either. > > In what way are we re-inventing that? bootstd supports lots of > different ways of booting, not just EFI.
At the high level, bootflow scan is re-implementing "bootefi bootmgr". but handling non-EFI payloads.
bootstd is about replacing the distro scripts, not bootmgr.
And the distro scripts are functionally replaced by "bootefi bootmgr", outside of bootstd.
But that doesn't support anything other than EFI apps, so isn't useful for when EFI is not wanted.
Yes, the distro scripts wanted to move more firmly / quickly in this direction. For all of the reasons they've elaborated before.
OK, well good luck to them, I suppose.
> Also I hope that one day EFI > will be implemented more as part of U-Boot than as a bolt-on, so will > make use of bootflows, etc.
And stuff like that is why I said what I said in here first. To me it sounds like you keep implying it's a hack that's not well integrated. When it's honestly at this point gotten more traction than FIT images have I think (as much as I wish FIT images had "won", it's like VHS vs Betamax, to bring in another technology metaphor).
The 'hack' I was referring to is efi_set_bootdev(), not EFI_LOADER as a whole!
I wasn't clear enough, sorry. I didn't mean just in this series where you referred to storing the needed property as a hack but rather "bolt-on" in what I quoted and "tidy up this" and "tidy up that". I'm just saying what impression your words leave with me, and quite possibly others.
OK I will try to be more gentle with my language.
Thanks.
> > So, to try and bring things back together. If U-Boot decides to load > > $FOO from device $BAR, at that common point is where we need to: > > - Is there an lmb for the location this is supposed to go to (for the if > > we know it, entire size)? > > - Note down everything else we know, now. > > Yes. > > > > > This means that we can note down enough stuff so that EFI can construct > > the path it needs. And if we're being told a filesystem, that filename > > is good enough for the IH_TYPE thing you're wanting, or at least a good > > chunk of it I hope. > > You want me to ignore the type that I know (kernel, ramdisk, logo, > etc.) and infer the type from a filename? Why?
No, I want you to save and display the filename. That's probably much more useful when debugging than "kernel". If you actually know some other type information (ie extlinux.conf says ...) then yes, it too can be stored as that's useful too.
The filename is already saved in bootflow->filename, and now it is in struct bootflow_img.
OK. But that's not generic enough.
How about we see how things work out here rather than giving up at the start? It is pretty clear in my head, so far.
I'd really rather not since it looks like it's starting in the wrong direction. I really do not understand why when we load the file / do the network request / read the flash area / etc is the wrong place to start recording the information about what we load.
It isn't that it is the wrong place, it's just that we know more when a bootmeth is in place. Are you thinking about the distro scripts or people's custom scripts, or something else?
[snip]
Yes, perhaps some series that shows what works / doesn't work and how it works would be helpful. I'd really like to see where you're starting your abstractions from.
OK I'll send a version with an add-on patch for the 'load' command, with the concept of an ad-hoc bootflow.
I want to combine these two (and I hope not drop any important context), to reply. A challenge is that for EFI, we need to know where we loaded something from. You hope that EFI_LOADER will make use of bootmeths at some future point, but that's not on your TODO list? What I'm saying is that in for example fs_read(), we have the common point between bootmeth, EFI_LOADER and "load" and already need to be doing sanity checks and can record what information we have now. Can bootmeth add to it? Sure. But I still don't see why this is the wrong place to start. And there are analogous points for network, flash, etc.

Hi Tom,
On Wed, 23 Oct 2024 at 20:41, Tom Rini trini@konsulko.com wrote:
On Tue, Oct 22, 2024 at 07:00:44PM +0200, Simon Glass wrote:
Hi Tom,
On Tue, 22 Oct 2024 at 17:01, Tom Rini trini@konsulko.com wrote:
On Tue, Oct 22, 2024 at 02:16:52PM +0200, Simon Glass wrote:
Hi Tom,
On Sat, 19 Oct 2024 at 18:30, Tom Rini trini@konsulko.com wrote:
On Sat, Oct 19, 2024 at 05:49:40AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 18 Oct 2024 at 15:30, Tom Rini trini@konsulko.com wrote: > > On Fri, Oct 18, 2024 at 12:48:31PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Fri, 18 Oct 2024 at 12:04, Tom Rini trini@konsulko.com wrote: > > > > > > On Fri, Oct 18, 2024 at 11:20:52AM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Fri, 18 Oct 2024 at 10:33, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > On Fri, Oct 18, 2024 at 09:01:03AM -0600, Simon Glass wrote: > > > > > > Hi Heinrich, > > > > > > > > > > > > On Thu, 17 Oct 2024 at 22:07, Heinrich Schuchardt xypron.glpk@gmx.de wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > Am 18. Oktober 2024 01:24:02 MESZ schrieb Simon Glass sjg@chromium.org: > > > > > > > >We want to keep track of images which are loaded, or those which could > > > > > > > >perhaps be loaded. This will make it easier to manage memory allocation, > > > > > > > >as well as permit removal of the EFI set_efi_bootdev() hack. > > > > > > > > > > > > I'll change this 'hack' to 'feature'. > > > > > > > > > > > > > > > > > > > > Please, keep in mind that files can be loaded manually, e.g. via the dhcp, the wget, and the loady commands. These are outside bootflows. > > > > > > > > > > > > Yes, this series is only going to help if bootstd is used. For ad-hoc > > > > > > use, EFI will need to rely on the above feature, at least until > > > > > > someone can think of another solution. > > > > > > > > > > Perhaps I need to try and be clearer here than I might have been in the > > > > > past. The consensus among off the shelf free software operating systems > > > > > is "just give me an EFI interface". This simplifies things on their end > > > > > if regardless of architecture it's the same interface. This means that > > > > > in U-Boot we need to treat EFI as one of the primary interfaces. Not a > > > > > novelty. Not a "some people might use". It is a frequent and commonly > > > > > used feature. > > > > > > > > Yes, EFI is everywhere and growing. All the more reason to tidy up > > > > this piece. I would like to see bootmgr use this new API, for example. > > > > > > > > But how does this comment affect this patch? > > > > > > Because at the very high level, I wonder if I made a mistake a few years > > > back. As I understand it, the nominal case is "bootefi bootmgr". I was > > > saying at the time that perhaps bootstd can just fire that off, and move > > > on. Now it seems like we're going along the path of re-inventing that, > > > and not integrating well with it either. > > > > In what way are we re-inventing that? bootstd supports lots of > > different ways of booting, not just EFI. > > At the high level, bootflow scan is re-implementing "bootefi bootmgr". > but handling non-EFI payloads.
bootstd is about replacing the distro scripts, not bootmgr.
And the distro scripts are functionally replaced by "bootefi bootmgr", outside of bootstd.
But that doesn't support anything other than EFI apps, so isn't useful for when EFI is not wanted.
Yes, the distro scripts wanted to move more firmly / quickly in this direction. For all of the reasons they've elaborated before.
OK, well good luck to them, I suppose.
> > Also I hope that one day EFI > > will be implemented more as part of U-Boot than as a bolt-on, so will > > make use of bootflows, etc. > > And stuff like that is why I said what I said in here first. To me it > sounds like you keep implying it's a hack that's not well integrated. > When it's honestly at this point gotten more traction than FIT images > have I think (as much as I wish FIT images had "won", it's like VHS vs > Betamax, to bring in another technology metaphor).
The 'hack' I was referring to is efi_set_bootdev(), not EFI_LOADER as a whole!
I wasn't clear enough, sorry. I didn't mean just in this series where you referred to storing the needed property as a hack but rather "bolt-on" in what I quoted and "tidy up this" and "tidy up that". I'm just saying what impression your words leave with me, and quite possibly others.
OK I will try to be more gentle with my language.
Thanks.
> > > So, to try and bring things back together. If U-Boot decides to load > > > $FOO from device $BAR, at that common point is where we need to: > > > - Is there an lmb for the location this is supposed to go to (for the if > > > we know it, entire size)? > > > - Note down everything else we know, now. > > > > Yes. > > > > > > > > This means that we can note down enough stuff so that EFI can construct > > > the path it needs. And if we're being told a filesystem, that filename > > > is good enough for the IH_TYPE thing you're wanting, or at least a good > > > chunk of it I hope. > > > > You want me to ignore the type that I know (kernel, ramdisk, logo, > > etc.) and infer the type from a filename? Why? > > No, I want you to save and display the filename. That's probably much > more useful when debugging than "kernel". If you actually know some > other type information (ie extlinux.conf says ...) then yes, it too can > be stored as that's useful too.
The filename is already saved in bootflow->filename, and now it is in struct bootflow_img.
OK. But that's not generic enough.
How about we see how things work out here rather than giving up at the start? It is pretty clear in my head, so far.
I'd really rather not since it looks like it's starting in the wrong direction. I really do not understand why when we load the file / do the network request / read the flash area / etc is the wrong place to start recording the information about what we load.
It isn't that it is the wrong place, it's just that we know more when a bootmeth is in place. Are you thinking about the distro scripts or people's custom scripts, or something else?
[snip]
Yes, perhaps some series that shows what works / doesn't work and how it works would be helpful. I'd really like to see where you're starting your abstractions from.
OK I'll send a version with an add-on patch for the 'load' command, with the concept of an ad-hoc bootflow.
I want to combine these two (and I hope not drop any important context), to reply. A challenge is that for EFI, we need to know where we loaded something from. You hope that EFI_LOADER will make use of bootmeths at some future point, but that's not on your TODO list? What I'm saying is that in for example fs_read(), we have the common point between bootmeth, EFI_LOADER and "load" and already need to be doing sanity checks and can record what information we have now. Can bootmeth add to it? Sure. But I still don't see why this is the wrong place to start. And there are analogous points for network, flash, etc.
There is already a bootmeth_efi which uses EFI_LOADER
But the point here is something different. I am not suggesting use of a bootmeth, but use of a bootflow. My idea is to use an ad-hoc bootflow which collects things not attached to an existing bootflow. I'll get some time to send the series again, with a few more patches at the end, to show the approach. But this series is the bones of it.
Regards, Simon

As a first step to recording images and where they came from, update this function to do so, since it is used by two bootmeths
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootmeth-uclass.c | 17 ++++++++++++++++- boot/bootmeth_extlinux.c | 2 +- boot/bootmeth_script.c | 3 ++- include/bootmeth.h | 5 ++++- 4 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c index 5b5fea39b3b..34fff004b43 100644 --- a/boot/bootmeth-uclass.c +++ b/boot/bootmeth-uclass.c @@ -6,6 +6,7 @@
#define LOG_CATEGORY UCLASS_BOOTSTD
+#include <alist.h> #include <blk.h> #include <bootflow.h> #include <bootmeth.h> @@ -326,8 +327,11 @@ int bootmeth_try_file(struct bootflow *bflow, struct blk_desc *desc, return 0; }
-int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align) +int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align, + enum image_type_t type) { + struct bootflow_img *img; + char *fname; void *buf; uint size; int ret; @@ -344,6 +348,17 @@ int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align) bflow->state = BOOTFLOWST_READY; bflow->buf = buf;
+ fname = strdup(bflow->fname); + if (!fname) + return log_msg_ret("alf", -ENOMEM); + img = alist_add_placeholder(&bflow->images); + if (!img) + return log_msg_ret("als", -ENOMEM); + img->fname = fname; + img->type = type; + img->addr = map_to_sysmem(buf); + img->size = size; + return 0; }
diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c index be8fbf4df63..755a80350d9 100644 --- a/boot/bootmeth_extlinux.c +++ b/boot/bootmeth_extlinux.c @@ -159,7 +159,7 @@ static int extlinux_read_bootflow(struct udevice *dev, struct bootflow *bflow) return log_msg_ret("try", ret); size = bflow->size;
- ret = bootmeth_alloc_file(bflow, 0x10000, 1); + ret = bootmeth_alloc_file(bflow, 0x10000, 1, IH_TYPE_EXTLINUX_CFG); if (ret) return log_msg_ret("read", ret);
diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c index c5cbf18c2e6..f01b22b064d 100644 --- a/boot/bootmeth_script.c +++ b/boot/bootmeth_script.c @@ -98,7 +98,8 @@ static int script_read_bootflow_file(struct udevice *bootstd, if (!bflow->subdir) return log_msg_ret("prefix", -ENOMEM);
- ret = bootmeth_alloc_file(bflow, 0x10000, ARCH_DMA_MINALIGN); + ret = bootmeth_alloc_file(bflow, 0x10000, ARCH_DMA_MINALIGN, + IH_TYPE_SCRIPT); if (ret) return log_msg_ret("read", ret);
diff --git a/include/bootmeth.h b/include/bootmeth.h index a08ebf005ad..322169c6b2e 100644 --- a/include/bootmeth.h +++ b/include/bootmeth.h @@ -7,6 +7,7 @@ #ifndef __bootmeth_h #define __bootmeth_h
+#include <image.h> #include <linux/bitops.h>
struct blk_desc; @@ -365,10 +366,12 @@ int bootmeth_try_file(struct bootflow *bflow, struct blk_desc *desc, * @bflow: Information about file to read * @size_limit: Maximum file size to permit * @align: Allocation alignment (1 for unaligned) + * @type: File type (IH_TYPE_...) * Return: 0 if OK, -E2BIG if file is too large, -ENOMEM if out of memory, * other -ve on other error */ -int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align); +int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align, + enum image_type_t type);
/** * bootmeth_alloc_other() - Allocate and read a file for a bootflow

This function is exported, so document it in the header file. Drop the duplicate comment in the C file.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/pxe_utils.c | 10 ---------- 1 file changed, 10 deletions(-)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index d6a4b2cb859..4fd26524286 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -133,16 +133,6 @@ static int get_relfile(struct pxe_context *ctx, const char *file_path, return 1; }
-/** - * get_pxe_file() - read a file - * - * The file is read and nul-terminated - * - * @ctx: PXE context - * @file_path: File path to read (relative to the PXE file) - * @file_addr: Address to load file to - * Returns 1 for success, or < 0 on error - */ int get_pxe_file(struct pxe_context *ctx, const char *file_path, ulong file_addr) {

The efiload_read_file() does similar things to a common function, so update it to use that instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootmeth_efi.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 2ad6d3b4ace..f3ddbe71405 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -110,19 +110,14 @@ static void set_efi_bootdev(struct blk_desc *desc, struct bootflow *bflow) static int efiload_read_file(struct bootflow *bflow, ulong addr) { struct blk_desc *desc = NULL; - loff_t bytes_read; + ulong size; int ret;
if (bflow->blk) desc = dev_get_uclass_plat(bflow->blk); - ret = bootmeth_setup_fs(bflow, desc); - if (ret) - return log_msg_ret("set", ret);
- ret = fs_read(bflow->fname, addr, 0, bflow->size, &bytes_read); - if (ret) - return log_msg_ret("read", ret); - bflow->buf = map_sysmem(addr, bflow->size); + ret = bootmeth_common_read_file(bflow->method, bflow, bflow->fname, + addr, &size);
set_efi_bootdev(desc, bflow);

On Thu, Oct 17, 2024 at 05:24:05PM -0600, Simon Glass wrote:
The efiload_read_file() does similar things to a common function, so update it to use that instead.
Signed-off-by: Simon Glass sjg@chromium.org
This is probably the first of a few commits that should read "bootmeth_efi:" not "efi:" I think.

We want to record the type of each file which is loaded. Add an new parameter for this, to the read_file() method. Update all users.
Make bootmeth_common_read_file() store information about the image that is read.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootmeth-uclass.c | 22 +++++++++++++++++++--- boot/bootmeth_android.c | 3 ++- boot/bootmeth_cros.c | 3 ++- boot/bootmeth_efi.c | 5 +++-- boot/bootmeth_efi_mgr.c | 3 ++- boot/bootmeth_extlinux.c | 2 +- boot/bootmeth_pxe.c | 6 +++--- boot/bootmeth_qfw.c | 3 ++- boot/bootmeth_sandbox.c | 3 ++- boot/vbe_simple.c | 5 +++-- include/bootmeth.h | 12 +++++++++--- 11 files changed, 48 insertions(+), 19 deletions(-)
diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c index 34fff004b43..726393e20b8 100644 --- a/boot/bootmeth-uclass.c +++ b/boot/bootmeth-uclass.c @@ -84,14 +84,15 @@ int bootmeth_boot(struct udevice *dev, struct bootflow *bflow) }
int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, ulong *sizep) + const char *file_path, ulong addr, + enum image_type_t type, ulong *sizep) { const struct bootmeth_ops *ops = bootmeth_get_ops(dev);
if (!ops->read_file) return -ENOSYS;
- return ops->read_file(dev, bflow, file_path, addr, sizep); + return ops->read_file(dev, bflow, file_path, addr, type, sizep); }
int bootmeth_get_bootflow(struct udevice *dev, struct bootflow *bflow) @@ -399,11 +400,14 @@ int bootmeth_alloc_other(struct bootflow *bflow, const char *fname, }
int bootmeth_common_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, ulong *sizep) + const char *file_path, ulong addr, + enum image_type_t type, ulong *sizep) { struct blk_desc *desc = NULL; + struct bootflow_img *img; loff_t len_read; loff_t size; + char *fname; int ret;
if (bflow->blk) @@ -428,6 +432,18 @@ int bootmeth_common_read_file(struct udevice *dev, struct bootflow *bflow, return ret; *sizep = len_read;
+ fname = strdup(bflow->fname); + if (!fname) + return log_msg_ret("crf", -ENOMEM); + + img = alist_add_placeholder(&bflow->images); + if (!img) + return log_msg_ret("cri", -ENOMEM); + img->fname = fname; + img->type = type; + img->addr = addr; + img->size = len_read; + return 0; }
diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c index 19b1f2c377b..0c7d3449ac7 100644 --- a/boot/bootmeth_android.c +++ b/boot/bootmeth_android.c @@ -298,7 +298,8 @@ static int android_read_bootflow(struct udevice *dev, struct bootflow *bflow) }
static int android_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, ulong *sizep) + const char *file_path, ulong addr, + enum image_type_t type, ulong *sizep) { /* * Reading individual files is not supported since we only diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c index 676f550ca25..3e60fd5da07 100644 --- a/boot/bootmeth_cros.c +++ b/boot/bootmeth_cros.c @@ -400,7 +400,8 @@ static int cros_read_bootflow(struct udevice *dev, struct bootflow *bflow) }
static int cros_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, ulong *sizep) + const char *file_path, ulong addr, + enum image_type_t type, ulong *sizep) { return -ENOSYS; } diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index f3ddbe71405..e1e378481b9 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -117,7 +117,7 @@ static int efiload_read_file(struct bootflow *bflow, ulong addr) desc = dev_get_uclass_plat(bflow->blk);
ret = bootmeth_common_read_file(bflow->method, bflow, bflow->fname, - addr, &size); + addr, IH_TYPE_EFI, &size);
set_efi_bootdev(desc, bflow);
@@ -189,7 +189,8 @@ static int distro_efi_try_bootflow_files(struct udevice *dev, /* Limit FDT files to 4MB */ size = SZ_4M; ret = bootmeth_common_read_file(dev, bflow, fname, - fdt_addr, &size); + fdt_addr, + IH_TYPE_FLATDT, &size); } }
diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c index 23ae1e610ac..b2b235d4915 100644 --- a/boot/bootmeth_efi_mgr.c +++ b/boot/bootmeth_efi_mgr.c @@ -74,7 +74,8 @@ static int efi_mgr_read_bootflow(struct udevice *dev, struct bootflow *bflow) }
static int efi_mgr_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, ulong *sizep) + const char *file_path, ulong addr, + enum image_type_t type, ulong *sizep) { /* Files are loaded by the 'bootefi bootmgr' command */
diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c index 755a80350d9..085ed9e456f 100644 --- a/boot/bootmeth_extlinux.c +++ b/boot/bootmeth_extlinux.c @@ -79,7 +79,7 @@ static int extlinux_getfile(struct pxe_context *ctx, const char *file_path, /* Allow up to 1GB */ *sizep = 1 << 30; ret = bootmeth_read_file(info->dev, info->bflow, file_path, addr, - sizep); + IH_TYPE_INVALID, sizep); if (ret) return log_msg_ret("read", ret);
diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c index 05c6bece2c1..74b0ad7d2e9 100644 --- a/boot/bootmeth_pxe.c +++ b/boot/bootmeth_pxe.c @@ -23,7 +23,7 @@ #include <pxe_utils.h>
static int extlinux_pxe_getfile(struct pxe_context *ctx, const char *file_path, - char *file_addr, ulong *sizep) + char *file_addr, ulong *sizep) { struct extlinux_info *info = ctx->userdata; ulong addr; @@ -34,7 +34,7 @@ static int extlinux_pxe_getfile(struct pxe_context *ctx, const char *file_path, /* Allow up to 1GB */ *sizep = 1 << 30; ret = bootmeth_read_file(info->dev, info->bflow, file_path, addr, - sizep); + IH_TYPE_INVALID, sizep); if (ret) return log_msg_ret("read", ret);
@@ -113,7 +113,7 @@ static int extlinux_pxe_read_bootflow(struct udevice *dev,
static int extlinux_pxe_read_file(struct udevice *dev, struct bootflow *bflow, const char *file_path, ulong addr, - ulong *sizep) + enum image_type_t type, ulong *sizep) { char *tftp_argv[] = {"tftp", NULL, NULL, NULL}; struct pxe_context *ctx = dev_get_priv(dev); diff --git a/boot/bootmeth_qfw.c b/boot/bootmeth_qfw.c index 2f8e00cf350..fb3cde4ab4b 100644 --- a/boot/bootmeth_qfw.c +++ b/boot/bootmeth_qfw.c @@ -52,7 +52,8 @@ static int qfw_read_bootflow(struct udevice *dev, struct bootflow *bflow) }
static int qfw_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, ulong *sizep) + const char *file_path, ulong addr, + enum image_type_t type, ulong *sizep) { return -ENOSYS; } diff --git a/boot/bootmeth_sandbox.c b/boot/bootmeth_sandbox.c index 26c713bb5f3..c40e8ae3304 100644 --- a/boot/bootmeth_sandbox.c +++ b/boot/bootmeth_sandbox.c @@ -27,7 +27,8 @@ static int sandbox_read_bootflow(struct udevice *dev, struct bootflow *bflow) }
static int sandbox_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, ulong *sizep) + const char *file_path, ulong addr, + enum image_type_t type, ulong *sizep) { return -ENOSYS; } diff --git a/boot/vbe_simple.c b/boot/vbe_simple.c index 189e86d2a22..c51b7d5805b 100644 --- a/boot/vbe_simple.c +++ b/boot/vbe_simple.c @@ -160,13 +160,14 @@ static int vbe_simple_read_bootflow(struct udevice *dev, struct bootflow *bflow) }
static int vbe_simple_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, ulong *sizep) + const char *file_path, ulong addr, + enum image_type_t type, ulong *sizep) { int ret;
if (vbe_phase() == VBE_PHASE_OS) { ret = bootmeth_common_read_file(dev, bflow, file_path, addr, - sizep); + type, sizep); if (ret) return log_msg_ret("os", ret); } diff --git a/include/bootmeth.h b/include/bootmeth.h index 322169c6b2e..78b2ce4123b 100644 --- a/include/bootmeth.h +++ b/include/bootmeth.h @@ -118,13 +118,15 @@ struct bootmeth_ops { * @bflow: Bootflow providing info on where to read from * @file_path: Path to file (may be absolute or relative) * @addr: Address to load file + * @type: File type (IH_TYPE_...) * @sizep: On entry provides the maximum permitted size; on exit * returns the size of the file * Return: 0 if OK, -ENOSPC if the file is too large for @sizep, other * -ve value if something else goes wrong */ int (*read_file)(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, ulong *sizep); + const char *file_path, ulong addr, + enum image_type_t type, ulong *sizep); #if CONFIG_IS_ENABLED(BOOTSTD_FULL) /** * readall() - read all files for a bootflow @@ -246,13 +248,15 @@ int bootmeth_set_bootflow(struct udevice *dev, struct bootflow *bflow, * @bflow: Bootflow providing info on where to read from * @file_path: Path to file (may be absolute or relative) * @addr: Address to load file + * @type: File type (IH_TYPE_...) * @sizep: On entry provides the maximum permitted size; on exit * returns the size of the file * Return: 0 if OK, -ENOSPC if the file is too large for @sizep, other * -ve value if something else goes wrong */ int bootmeth_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, ulong *sizep); + const char *file_path, ulong addr, + enum image_type_t type, ulong *sizep);
/** * bootmeth_read_all() - read all bootflow files @@ -398,11 +402,13 @@ int bootmeth_alloc_other(struct bootflow *bflow, const char *fname, * @bflow: Bootflow information * @file_path: Path to file * @addr: Address to load file to + * @type: File type (IH_TYPE_...) * @sizep: On entry, the maximum file size to accept, on exit the actual file * size read */ int bootmeth_common_read_file(struct udevice *dev, struct bootflow *bflow, - const char *file_path, ulong addr, ulong *sizep); + const char *file_path, ulong addr, + enum image_type_t type, ulong *sizep);
/** * bootmeth_get_bootflow() - Get a bootflow from a global bootmeth

If the filename cannot be set we should give up. Add the missing error check.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootmeth_efi.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index e1e378481b9..548d5f71b2a 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -269,6 +269,8 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow) if (!bootfile_name) return log_msg_ret("bootfile_name", ret); bflow->fname = strdup(bootfile_name); + if (!bflow->fname) + return log_msg_ret("fi0", -ENOMEM);
/* do the hideous EFI hack */ efi_set_bootdev("Net", "", bflow->fname, map_sysmem(addr, 0),

Add a file-type parameter to this function and update all users. Add a proper comment to the function which we are here.
This will allow tracking of the file types loaded by the extlinux bootmeth.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootmeth_extlinux.c | 5 +++-- boot/bootmeth_pxe.c | 5 +++-- boot/pxe_utils.c | 26 ++++++++++++++++---------- cmd/pxe.c | 2 +- cmd/sysboot.c | 6 ++++-- include/pxe_utils.h | 14 +++++++++++++- 6 files changed, 40 insertions(+), 18 deletions(-)
diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c index 085ed9e456f..ece52d2e662 100644 --- a/boot/bootmeth_extlinux.c +++ b/boot/bootmeth_extlinux.c @@ -68,7 +68,8 @@ static int extlinux_get_state_desc(struct udevice *dev, char *buf, int maxsize) }
static int extlinux_getfile(struct pxe_context *ctx, const char *file_path, - char *file_addr, ulong *sizep) + char *file_addr, enum image_type_t type, + ulong *sizep) { struct extlinux_info *info = ctx->userdata; ulong addr; @@ -79,7 +80,7 @@ static int extlinux_getfile(struct pxe_context *ctx, const char *file_path, /* Allow up to 1GB */ *sizep = 1 << 30; ret = bootmeth_read_file(info->dev, info->bflow, file_path, addr, - IH_TYPE_INVALID, sizep); + type, sizep); if (ret) return log_msg_ret("read", ret);
diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c index 74b0ad7d2e9..0643d280f21 100644 --- a/boot/bootmeth_pxe.c +++ b/boot/bootmeth_pxe.c @@ -23,7 +23,8 @@ #include <pxe_utils.h>
static int extlinux_pxe_getfile(struct pxe_context *ctx, const char *file_path, - char *file_addr, ulong *sizep) + char *file_addr, enum image_type_t type, + ulong *sizep) { struct extlinux_info *info = ctx->userdata; ulong addr; @@ -34,7 +35,7 @@ static int extlinux_pxe_getfile(struct pxe_context *ctx, const char *file_path, /* Allow up to 1GB */ *sizep = 1 << 30; ret = bootmeth_read_file(info->dev, info->bflow, file_path, addr, - IH_TYPE_INVALID, sizep); + type, sizep); if (ret) return log_msg_ret("read", ret);
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 4fd26524286..5188182f05f 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -97,7 +97,8 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len) * Returns 1 for success, or < 0 on error */ static int get_relfile(struct pxe_context *ctx, const char *file_path, - unsigned long file_addr, ulong *filesizep) + unsigned long file_addr, enum image_type_t type, + ulong *filesizep) { size_t path_len; char relfile[MAX_TFTP_PATH_LEN + 1]; @@ -124,7 +125,7 @@ static int get_relfile(struct pxe_context *ctx, const char *file_path,
sprintf(addr_buf, "%lx", file_addr);
- ret = ctx->getfile(ctx, relfile, addr_buf, &size); + ret = ctx->getfile(ctx, relfile, addr_buf, type, &size); if (ret < 0) return log_msg_ret("get", ret); if (filesizep) @@ -140,7 +141,8 @@ int get_pxe_file(struct pxe_context *ctx, const char *file_path, int err; char *buf;
- err = get_relfile(ctx, file_path, file_addr, &size); + err = get_relfile(ctx, file_path, file_addr, IH_TYPE_EXTLINUX_CFG, + &size); if (err < 0) return err;
@@ -189,13 +191,15 @@ int get_pxelinux_path(struct pxe_context *ctx, const char *file, * @file_path: File path to read (relative to the PXE file) * @envaddr_name: Name of environment variable which contains the address to * load to + * @type: File type * @filesizep: Returns the file size in bytes * Returns 1 on success, -ENOENT if @envaddr_name does not exist as an * environment variable, -EINVAL if its format is not valid hex, or other * value < 0 on other error */ static int get_relfile_envaddr(struct pxe_context *ctx, const char *file_path, - const char *envaddr_name, ulong *filesizep) + const char *envaddr_name, enum image_type_t type, + ulong *filesizep) { unsigned long file_addr; char *envaddr; @@ -207,7 +211,7 @@ static int get_relfile_envaddr(struct pxe_context *ctx, const char *file_path, if (strict_strtoul(envaddr, 16, &file_addr) < 0) return -EINVAL;
- return get_relfile(ctx, file_path, file_addr, filesizep); + return get_relfile(ctx, file_path, file_addr, type, filesizep); }
/** @@ -395,7 +399,7 @@ static void label_boot_fdtoverlay(struct pxe_context *ctx,
/* Load overlay file */ err = get_relfile_envaddr(ctx, overlayfile, "fdtoverlay_addr_r", - NULL); + IH_TYPE_FLATDT, NULL); if (err < 0) { printf("Failed loading overlay %s\n", overlayfile); goto skip_overlay; @@ -480,7 +484,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) }
if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r", - NULL) < 0) { + IH_TYPE_KERNEL, NULL) < 0) { printf("Skipping %s for failure retrieving kernel\n", label->name); return 1; @@ -506,7 +510,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) } else if (label->initrd) { ulong size; if (get_relfile_envaddr(ctx, label->initrd, "ramdisk_addr_r", - &size) < 0) { + IH_TYPE_RAMDISK, &size) < 0) { printf("Skipping %s for failure retrieving initrd\n", label->name); goto cleanup; @@ -651,7 +655,8 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label)
if (fdtfile) { int err = get_relfile_envaddr(ctx, fdtfile, - "fdt_addr_r", NULL); + "fdt_addr_r", + IH_TYPE_FLATDT, NULL);
free(fdtfilefree); if (err < 0) { @@ -1538,7 +1543,8 @@ void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg) if (IS_ENABLED(CONFIG_CMD_BMP)) { /* display BMP if available */ if (cfg->bmp) { - if (get_relfile(ctx, cfg->bmp, image_load_addr, NULL)) { + if (get_relfile(ctx, cfg->bmp, image_load_addr, + IH_TYPE_LOGO, NULL)) { #if defined(CONFIG_VIDEO) struct udevice *dev;
diff --git a/cmd/pxe.c b/cmd/pxe.c index 982e2b1e7ea..b64a4fce1a7 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -27,7 +27,7 @@ const char *pxe_default_paths[] = { };
static int do_get_tftp(struct pxe_context *ctx, const char *file_path, - char *file_addr, ulong *sizep) + char *file_addr, enum image_type_t type, ulong *sizep) { char *tftp_argv[] = {"tftp", NULL, NULL, NULL}; int ret; diff --git a/cmd/sysboot.c b/cmd/sysboot.c index 8a060780cab..71936cdb1cd 100644 --- a/cmd/sysboot.c +++ b/cmd/sysboot.c @@ -23,7 +23,8 @@ struct sysboot_info { };
static int sysboot_read_file(struct pxe_context *ctx, const char *file_path, - char *file_addr, ulong *sizep) + char *file_addr, enum image_type_t type, + ulong *sizep) { struct sysboot_info *info = ctx->userdata; loff_t len_read; @@ -110,7 +111,8 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; }
- if (get_pxe_file(&ctx, filename, pxefile_addr_r) < 0) { + if (get_pxe_file(&ctx, filename, pxefile_addr_r) + < 0) { printf("Error reading config file\n"); pxe_destroy_ctx(&ctx); return 1; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 68ac40b64ad..ec5d8af5709 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -3,6 +3,7 @@ #ifndef __PXE_UTILS_H #define __PXE_UTILS_H
+#include <image.h> #include <linux/list.h>
/* @@ -82,8 +83,19 @@ struct pxe_menu { };
struct pxe_context; + +/** + * Read a file + * + * @ctx: PXE context + * @file_path: Full path to filename to read + * @file_addr: String containing the to which to read the file + * @type: File type + * @fileszeip: Returns file size + */ typedef int (*pxe_getfile_func)(struct pxe_context *ctx, const char *file_path, - char *file_addr, ulong *filesizep); + char *file_addr, enum image_type_t type, + ulong *filesizep);
/** * struct pxe_context - context information for PXE parsing

Record images loaded by this bootmeth.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootmeth_pxe.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c index 0643d280f21..3e5008ab317 100644 --- a/boot/bootmeth_pxe.c +++ b/boot/bootmeth_pxe.c @@ -118,7 +118,8 @@ static int extlinux_pxe_read_file(struct udevice *dev, struct bootflow *bflow, { char *tftp_argv[] = {"tftp", NULL, NULL, NULL}; struct pxe_context *ctx = dev_get_priv(dev); - char file_addr[17]; + char file_addr[17], *fname; + struct bootflow_img *img; ulong size; int ret;
@@ -135,6 +136,17 @@ static int extlinux_pxe_read_file(struct udevice *dev, struct bootflow *bflow, return log_msg_ret("spc", -ENOSPC); *sizep = size;
+ fname = strdup(file_path); + if (!fname) + return log_msg_ret("pxf", -ENOMEM); + img = alist_add_placeholder(&bflow->images); + if (!img) + return log_msg_ret("pxi", -ENOMEM); + img->fname = fname; + img->type = type; + img->addr = addr; + img->size = size; + return 0; }

Update this function to add the image to the list.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootmeth-uclass.c | 15 ++++++++++++++- boot/bootmeth_script.c | 4 ++-- include/bootmeth.h | 3 ++- 3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c index 726393e20b8..fcfffa41533 100644 --- a/boot/bootmeth-uclass.c +++ b/boot/bootmeth-uclass.c @@ -364,9 +364,11 @@ int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align, }
int bootmeth_alloc_other(struct bootflow *bflow, const char *fname, - void **bufp, uint *sizep) + enum image_type_t type, void **bufp, uint *sizep) { struct blk_desc *desc = NULL; + struct bootflow_img *img; + char *img_fname; char path[200]; loff_t size; void *buf; @@ -393,6 +395,17 @@ int bootmeth_alloc_other(struct bootflow *bflow, const char *fname, if (ret) return log_msg_ret("all", ret);
+ img_fname = strdup(fname); + if (!img_fname) + return log_msg_ret("alf", -ENOMEM); + img = alist_add_placeholder(&bflow->images); + if (!img) + return log_msg_ret("als", -ENOMEM); + img->fname = img_fname; + img->type = type; + img->addr = map_to_sysmem(buf); + img->size = size; + *bufp = buf; *sizep = size;
diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c index f01b22b064d..e648e42a24e 100644 --- a/boot/bootmeth_script.c +++ b/boot/bootmeth_script.c @@ -107,8 +107,8 @@ static int script_read_bootflow_file(struct udevice *bootstd, if (ret) return log_msg_ret("inf", ret);
- ret = bootmeth_alloc_other(bflow, "boot.bmp", &bflow->logo, - &bflow->logo_size); + ret = bootmeth_alloc_other(bflow, "boot.bmp", IH_TYPE_LOGO, + &bflow->logo, &bflow->logo_size); /* ignore error */
return 0; diff --git a/include/bootmeth.h b/include/bootmeth.h index 78b2ce4123b..7382f76c0b8 100644 --- a/include/bootmeth.h +++ b/include/bootmeth.h @@ -386,12 +386,13 @@ int bootmeth_alloc_file(struct bootflow *bflow, uint size_limit, uint align, * * @bflow: Information about file to read * @fname: Filename to read from (within bootflow->subdir) + * @type: File type (IH_TYPE_...) * @bufp: Returns a pointer to the allocated buffer * @sizep: Returns the size of the buffer * Return: 0 if OK, -ENOMEM if out of memory, other -ve on other error */ int bootmeth_alloc_other(struct bootflow *bflow, const char *fname, - void **bufp, uint *sizep); + enum image_type_t type, void **bufp, uint *sizep);
/** * bootmeth_common_read_file() - Common handler for reading a file

When the buffer address is not set, say so, rather than showing an address which looks very strange, on sandbox.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/bootflow.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/cmd/bootflow.c b/cmd/bootflow.c index 3e9769e0d42..1c1146ce11e 100644 --- a/cmd/bootflow.c +++ b/cmd/bootflow.c @@ -393,7 +393,11 @@ static int do_bootflow_info(struct cmd_tbl *cmdtp, int flag, int argc, printf("Partition: %d\n", bflow->part); printf("Subdir: %s\n", bflow->subdir ? bflow->subdir : "(none)"); printf("Filename: %s\n", bflow->fname); - printf("Buffer: %lx\n", (ulong)map_to_sysmem(bflow->buf)); + printf("Buffer: "); + if (bflow->buf) + printf("%lx\n", (ulong)map_to_sysmem(bflow->buf)); + else + printf("(not loaded)\n"); printf("Size: %x (%d bytes)\n", bflow->size, bflow->size); printf("OS: %s\n", bflow->os_name ? bflow->os_name : "(none)"); printf("Cmdline: ");

Record images loaded by this bootmeth.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootmeth_cros.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c index 3e60fd5da07..10aefe046ca 100644 --- a/boot/bootmeth_cros.c +++ b/boot/bootmeth_cros.c @@ -206,6 +206,7 @@ static int cros_read_buf(struct bootflow *bflow, void *buf, ulong size, { struct blk_desc *desc = dev_get_uclass_plat(bflow->blk); ulong base, setup, cmdline, kern_base; + struct bootflow_img img; ulong num_blks; int ret;
@@ -243,8 +244,27 @@ static int cros_read_buf(struct bootflow *bflow, void *buf, ulong size, ret = copy_cmdline(map_sysmem(cmdline, 0), uuid, &bflow->cmdline); if (ret) return log_msg_ret("cmd", ret); + + img.fname = strdup("setup"); + if (!img.fname) + return log_msg_ret("cri", -ENOMEM); + img.type = IH_TYPE_X86_SETUP; + img.addr = setup; + img.size = 0x3000; + if (!alist_add(&bflow->images, img)) + return log_msg_ret("cri", -ENOMEM); + bflow->x86_setup = map_sysmem(setup, 0);
+ img.fname = strdup("cmdline"); + if (!img.fname) + return log_msg_ret("cri", -ENOMEM); + img.type = IH_TYPE_CMDLINE; + img.addr = cmdline; + img.size = 0x1000; + if (!alist_add(&bflow->images, img)) + return log_msg_ret("cri", -ENOMEM); + return 0; }
@@ -263,6 +283,7 @@ static int cros_read_info(struct bootflow *bflow, const char *uuid, struct udevice *blk = bflow->blk; struct blk_desc *desc = dev_get_uclass_plat(blk); ulong offset, size, before_base; + struct bootflow_img img; void *buf; int ret;
@@ -306,6 +327,15 @@ static int cros_read_info(struct bootflow *bflow, const char *uuid, } priv->info_buf = buf;
+ img.fname = strdup("kernel"); + if (!img.fname) + return log_msg_ret("cri", -ENOMEM); + img.type = IH_TYPE_KERNEL; + img.addr = 0; + img.size = priv->body_size; + if (!alist_add(&bflow->images, img)) + return log_msg_ret("cri", -ENOMEM); + return 0; }

Add a new 'bootstd images' command, which lists the images which have been loaded.
Update some existing tests to use it. Provide some documentation about images in general and this command in particular.
Use a more realistic kernel command-line to make the test easier to follow.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootflow.c | 12 ++++ cmd/Kconfig | 9 +++ cmd/Makefile | 1 + cmd/bootstd.c | 65 +++++++++++++++++++ doc/develop/bootstd/overview.rst | 21 +++++- doc/usage/cmd/bootstd.rst | 79 +++++++++++++++++++++++ doc/usage/index.rst | 1 + include/bootflow.h | 9 +++ test/boot/bootflow.c | 106 +++++++++++++++++++++++++++++++ test/py/tests/test_ut.py | 3 +- 10 files changed, 304 insertions(+), 2 deletions(-) create mode 100644 cmd/bootstd.c create mode 100644 doc/usage/cmd/bootstd.rst
diff --git a/boot/bootflow.c b/boot/bootflow.c index 10bda8eac79..13323c784ae 100644 --- a/boot/bootflow.c +++ b/boot/bootflow.c @@ -953,3 +953,15 @@ int bootflow_cmdline_auto(struct bootflow *bflow, const char *arg)
return 0; } + +int bootflow_get_seq(const struct bootflow *bflow) +{ + struct bootstd_priv *std; + int ret; + + ret = bootstd_get_priv(&std); + if (ret) + return ret; + + return alist_calc_index(&std->bootflows, bflow); +} diff --git a/cmd/Kconfig b/cmd/Kconfig index bff22b94de2..e67d488747c 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -321,6 +321,15 @@ config CMD_BOOTMETH
This command is not necessary for bootstd to work.
+config CMD_BOOTSTD + bool "bootstd" + depends on BOOTSTD + default y if BOOTSTD_FULL + help + Provide general information and control for bootstd. + + This command is not necessary for bootstd to work. + config BOOTM_EFI bool "Support booting UEFI FIT images" depends on EFI_BINARY_EXEC && CMD_BOOTM && FIT diff --git a/cmd/Makefile b/cmd/Makefile index 3c5bd56e912..50a9d7c0fd4 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -24,6 +24,7 @@ obj-$(CONFIG_BLK) += blk_common.o obj-$(CONFIG_CMD_BOOTDEV) += bootdev.o obj-$(CONFIG_CMD_BOOTFLOW) += bootflow.o obj-$(CONFIG_CMD_BOOTMETH) += bootmeth.o +obj-$(CONFIG_CMD_BOOTSTD) += bootstd.o obj-$(CONFIG_CMD_SOURCE) += source.o obj-$(CONFIG_CMD_BCB) += bcb.o obj-$(CONFIG_CMD_BDI) += bdinfo.o diff --git a/cmd/bootstd.c b/cmd/bootstd.c new file mode 100644 index 00000000000..ff98e1e6e01 --- /dev/null +++ b/cmd/bootstd.c @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * 'bootstd' command + * + * Copyright 2024 Google LLC + * Written by Simon Glass sjg@chromium.org + */ + +#include <bootdev.h> +#include <bootflow.h> +#include <bootmeth.h> +#include <bootstd.h> +#include <command.h> +#include <dm.h> +#include <malloc.h> +#include <dm/uclass-internal.h> + +static int do_bootstd_images(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + const struct bootflow *bflow; + struct bootstd_priv *std; + int ret, i; + + ret = bootstd_get_priv(&std); + if (ret) { + printf("Cannot get bootstd (err=%d)\n", ret); + return CMD_RET_FAILURE; + } + + printf("Seq Bootflow Type At Size Filename\n"); + printf("--- ------------------- -------------- -------- -------- ----------------\n"); + + /* + * Use the ordering if we have one, so long as we are not trying to list + * all bootmethds + */ + i = 0; + alist_for_each(bflow, &std->bootflows) { + const struct bootflow_img *img; + + alist_for_each(img, &bflow->images) { + printf("%3d %-20.20s %-15.15s ", + bootflow_get_seq(bflow), bflow->name, + genimg_get_type_short_name(img->type)); + if (img->addr) + printf("%8lx", img->addr); + else + printf("%8s", "-"); + printf(" %8lx %s\n", img->size, img->fname); + i++; + } + } + + printf("--- ------------------- -------------- -------- -------- ----------------\n"); + printf("(%d image%s)\n", i, i != 1 ? "s" : ""); + + return 0; +} + +U_BOOT_LONGHELP(bootstd, + "images - list loaded images"); + +U_BOOT_CMD_WITH_SUBCMDS(bootstd, "Standard-boot operation", bootstd_help_text, + U_BOOT_SUBCMD_MKENT(images, 1, 1, do_bootstd_images)); diff --git a/doc/develop/bootstd/overview.rst b/doc/develop/bootstd/overview.rst index a2913cd47be..052de7f8214 100644 --- a/doc/develop/bootstd/overview.rst +++ b/doc/develop/bootstd/overview.rst @@ -453,7 +453,7 @@ drivers are bound automatically. Command interface -----------------
-Three commands are available: +Four commands are available:
`bootdev` Allows listing of available bootdevs, selecting a particular one and @@ -468,6 +468,25 @@ Three commands are available: Allow listing of available bootmethds, setting the order in which they are tried and bootmeth specific configuration. See :doc:`/usage/cmd/bootmeth`
+`bootstd` + Allow access to standard boot itself, so far only for listing images across + all bootflows. See :doc:`/usage/cmd/bootstd` + +Images +------ + +Standard boot keeps track of images which can or have been loaded. These are +kept in a list attached to each bootflow. They can be listed using the +`bootstd images` command. + +For now most bootmeths load their images when scanning. Over time, some may +adjust to load them only when needed, but in this case the images will still +be visible. + +Once a bootflow has been selected, images for those that are not selected can +potentially be dropped from the memory map. For now, this is not implemented. + + .. _BootflowStates:
Bootflow states diff --git a/doc/usage/cmd/bootstd.rst b/doc/usage/cmd/bootstd.rst new file mode 100644 index 00000000000..7d933852a91 --- /dev/null +++ b/doc/usage/cmd/bootstd.rst @@ -0,0 +1,79 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +.. index:: + single: bootstd (command) + +bootstd command +=============== + +Synopsis +-------- + +:: + + bootstd images + +Description +----------- + +The `bootstd` command is used to manage standard boot. At present the only +functionality available is to look at the images which have been loaded, or +could be loaded should a particular bootflow be selected. + +See :doc:`/develop/bootstd/index` for more information. + +bootflow images +~~~~~~~~~~~~~~~ + +Lists the available images and their location in memory. + +Example +------- + +This shows listing images attached to various bootflows, then checking the +content of a few of them:: + + => bootflow scan + => bootflow list + Showing all bootflows + Seq Method State Uclass Part Name Filename + --- ----------- ------ -------- ---- ------------------------ ---------------- + 0 extlinux ready mmc 1 mmc1.bootdev.part_1 /extlinux/extlinux.conf + 1 script ready mmc 1 mmc4.bootdev.part_1 /boot/boot.scr + 2 cros ready mmc 2 mmc5.bootdev.part_2 + 3 cros ready mmc 4 mmc5.bootdev.part_4 + --- ----------- ------ -------- ---- ------------------------ ---------------- + (4 bootflows, 4 valid) + => + => bootstd images + Seq Bootflow Type At Size Filename + --- ------------------- -------------- -------- -------- ---------------- + 0 mmc1.bootdev.part_1 extlinux_cfg 8ed5a70 253 /extlinux/extlinux.conf + 1 mmc4.bootdev.part_1 script 8ed9550 c73 /boot/boot.scr + 1 mmc4.bootdev.part_1 logo 8eda2a0 5d42 boot.bmp + 2 mmc5.bootdev.part_2 x86_setup 8ee84d0 3000 setup + 2 mmc5.bootdev.part_2 cmdline 8ee84d0 1000 cmdline + 2 mmc5.bootdev.part_2 kernel - 4000 kernel + 3 mmc5.bootdev.part_4 x86_setup 8eeb4e0 3000 setup + 3 mmc5.bootdev.part_4 cmdline 8eeb4e0 1000 cmdline + 3 mmc5.bootdev.part_4 kernel - 4000 kernel + --- ------------------- -------------- -------- -------- ---------------- + (9 images) + => md 8eda2a0 10 + 08eda2a0: 5d424d42 00000000 008a0000 007c0000 BMB]..........|. + 08eda2b0: 00ac0000 002e0000 00010000 00000018 ................ + 08eda2c0: 5cb80000 0b130000 0b130000 00000000 ............... + 08eda2d0: 00000000 00000000 ff0000ff 00ff0000 ................ + => md 8ee84d0 10 + 08ee84d0: 544f4f42 414d495f 2f3d4547 696c6d76 BOOT_IMAGE=/vmli + 08ee84e0: 2d7a756e 35312e35 312d302e 672d3132 nuz-5.15.0-121-g + 08ee84f0: 72656e65 72206369 3d746f6f 7665642f eneric root=/dev + 08ee8500: 6d766e2f 316e3065 72203170 7571206f /nvme0n1p1 ro qu + +Return value +------------ + +The return value $? is always 0 (true). + + +.. BootflowStates_: diff --git a/doc/usage/index.rst b/doc/usage/index.rst index fcce125a611..d25c2a1e876 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -40,6 +40,7 @@ Shell commands cmd/bootm cmd/bootmenu cmd/bootmeth + cmd/bootstd cmd/bootz cmd/button cmd/cat diff --git a/include/bootflow.h b/include/bootflow.h index 3d9a9e2025a..2e6c7b559cb 100644 --- a/include/bootflow.h +++ b/include/bootflow.h @@ -586,4 +586,13 @@ int bootflow_cmdline_get_arg(struct bootflow *bflow, const char *arg, */ int bootflow_cmdline_auto(struct bootflow *bflow, const char *arg);
+/** + * bootflow_get_seq() - Get the sequence number of a bootflow + * + * Bootflows are numbered by their position in the bootstd list. + * + * Return: Sequence number of bootflow (0 = first) + */ +int bootflow_get_seq(const struct bootflow *bflow); + #endif diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c index efb82ee628b..30a3f366937 100644 --- a/test/boot/bootflow.c +++ b/test/boot/bootflow.c @@ -14,6 +14,7 @@ #include <dm.h> #include <efi_default_filename.h> #include <expo.h> +#include <mapmem.h> #ifdef CONFIG_SANDBOX #include <asm/test.h> #endif @@ -73,6 +74,14 @@ static int bootflow_cmd(struct unit_test_state *uts) ut_assert_nextline("(1 bootflow, 1 valid)"); ut_assert_console_end();
+ ut_assertok(run_command("bootstd images", 0)); + ut_assert_nextlinen("Seq"); + ut_assert_nextlinen("---"); + ut_assert_nextlinen(" 0 mmc1.bootdev.part_1 extlinux_cfg"); + ut_assert_nextlinen("---"); + ut_assert_nextline("(1 image)"); + ut_assert_console_end(); + return 0; } BOOTSTD_TEST(bootflow_cmd, UTF_DM | UTF_SCAN_FDT | UTF_CONSOLE); @@ -1185,6 +1194,19 @@ static int bootflow_cros(struct unit_test_state *uts) ut_assert_nextlinen("---"); ut_assert_skip_to_line("(3 bootflows, 3 valid)");
+ ut_assertok(run_command("bootstd images", 0)); + ut_assert_nextlinen("Seq"); + ut_assert_nextlinen("---"); + ut_assert_nextlinen(" 0 mmc1.bootdev.part_1 extlinux_cfg"); + ut_assert_nextlinen(" 1 mmc5.bootdev.part_2 x86_setup"); + ut_assert_nextlinen(" 1 mmc5.bootdev.part_2 cmdline"); + ut_assert_nextlinen(" 1 mmc5.bootdev.part_2 kernel - 4000 kernel"); + ut_assert_nextlinen(" 2 mmc5.bootdev.part_4 x86_setup"); + ut_assert_nextlinen(" 2 mmc5.bootdev.part_4 cmdline"); + ut_assert_nextlinen(" 2 mmc5.bootdev.part_4 kernel - 4000 kernel"); + ut_assert_nextlinen("---"); + ut_assert_nextline("(7 images)"); + ut_assert_console_end();
return 0; @@ -1213,3 +1235,87 @@ static int bootflow_android(struct unit_test_state *uts) return 0; } BOOTSTD_TEST(bootflow_android, UTF_CONSOLE); + +/* Check 'bootflow scan' provides a list of images */ +static int bootstd_images(struct unit_test_state *uts) +{ + static const char *order[] = {"mmc2", "mmc1", "mmc4", "mmc5", NULL}; + const struct legacy_img_hdr *hdr; + const struct bootflow_img *img; + const struct bootflow *bflow; + struct bootstd_priv *std; + const char **old_order; + struct udevice *dev; + ofnode root, node; + ulong data, len; + char *ptr; + + /* get access to the current bootflow */ + ut_assertok(bootstd_get_priv(&std)); + + ut_assertok(prep_mmc_bootdev(uts, "mmc4", true, &old_order)); + + /* bind mmc5 too, for cros */ + root = oftree_root(oftree_default()); + node = ofnode_find_subnode(root, "mmc5"); + ut_assert(ofnode_valid(node)); + ut_assertok(lists_bind_fdt(gd->dm_root, node, &dev, NULL, false)); + + std->bootdev_order = order; + ut_assertok(run_command("bootflow scan", 0)); + ut_assert_console_end(); + std->bootdev_order = old_order; + + ut_assertok(run_command("bootflow list", 0)); + ut_assert_skip_to_line("(4 bootflows, 4 valid)"); + + ut_assertok(run_command("bootstd images", 0)); + ut_assert_nextlinen("Seq"); + ut_assert_nextlinen("---"); + ut_assert_nextlinen(" 0 mmc1.bootdev.part_1 extlinux_cfg"); + ut_assert_nextlinen(" 1 mmc4.bootdev.part_1 script"); + ut_assert_nextlinen(" 1 mmc4.bootdev.part_1 logo"); + ut_assert_nextlinen(" 2 mmc5.bootdev.part_2 x86_setup"); + ut_assert_nextlinen(" 2 mmc5.bootdev.part_2 cmdline"); + ut_assert_nextlinen(" 2 mmc5.bootdev.part_2 kernel -"); + ut_assert_nextlinen(" 3 mmc5.bootdev.part_4 x86_setup"); + ut_assert_nextlinen(" 3 mmc5.bootdev.part_4 cmdline"); + ut_assert_nextlinen(" 3 mmc5.bootdev.part_4 kernel -"); + ut_assert_nextlinen("---"); + ut_assert_nextline("(9 images)"); + + /* check the first image */ + bflow = alist_get(&std->bootflows, 0, struct bootflow); + img = alist_get(&bflow->images, 0, struct bootflow_img); + ut_asserteq_strn("# extlinux.conf", map_sysmem(img->addr, 0)); + + /* check the second image */ + bflow = alist_get(&std->bootflows, 1, struct bootflow); + img = alist_get(&bflow->images, 0, struct bootflow_img); + + /* this is the length of the script in bytes */ + hdr = map_sysmem(img->addr, 0); + image_multi_getimg(hdr, 0, &data, &len); + ptr = (void *)data; + ut_asserteq_strn("# DO NOT EDIT THIS FILE", ptr); + + /* check the ChromiumOS images */ + bflow = alist_get(&std->bootflows, 2, struct bootflow); + img = alist_get(&bflow->images, 1, struct bootflow_img); + ptr = map_sysmem(img->addr, 0); + ut_asserteq_strn("BOOT_IMAGE=/vmlinuz-5.15.0-121-generic root=", ptr); + + /* + * the x86 setup is not a real binary, so just check that it is empty, + * so that if this changes in the future someone will notice and update + * this test + */ + img = alist_get(&bflow->images, 0, struct bootflow_img); + ptr = map_sysmem(img->addr, 0); + ut_asserteq(0, *(ulong *)ptr); + + ut_assert_console_end(); + + return 0; +} +BOOTSTD_TEST(bootstd_images, UTF_CONSOLE); diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py index 9166c8f6b6e..0508f933e3b 100644 --- a/test/py/tests/test_ut.py +++ b/test/py/tests/test_ut.py @@ -400,9 +400,10 @@ def setup_cros_image(cons): start, size, num, name = line.split(maxsplit=3) parts[int(num)] = Partition(int(start), int(size), name)
+ # Set up the kernel command-line dummy = os.path.join(cons.config.result_dir, 'dummy.txt') with open(dummy, 'wb') as outf: - outf.write(b'dummy\n') + outf.write(b'BOOT_IMAGE=/vmlinuz-5.15.0-121-generic root=/dev/nvme0n1p1 ro quiet splash vt.handoff=7')
# For now we just use dummy kernels. This limits testing to just detecting # a signed kernel. We could add support for the x86 data structures so that
participants (4)
-
Heinrich Schuchardt
-
Mark Kettenis
-
Simon Glass
-
Tom Rini