* [PATCH 1/8] fdtdec: Correct devicetree symbol for EFI app
2024-12-09 16:27 [PATCH 0/8] efi: Add docs and tests and a few tweaks Simon Glass
@ 2024-12-09 16:27 ` Simon Glass
2024-12-09 16:27 ` [PATCH 2/8] part_find: Enable for sandbox and correct radix and calls Simon Glass
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2024-12-09 16:27 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Matthew Garrett, Simon Glass, Lad Prabhakar, Marek Vasut,
Raymond Mao, Tom Rini
This should have a single underscore to match the declaration in
include/asm-generic/sections.h so fix it.
For some reason it was not picked up in CI
Signed-off-by: Simon Glass <sjg@chromium.org>
---
lib/fdtdec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index c1eb1c9f825..e87e537f0c3 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1238,7 +1238,7 @@ static void *fdt_find_separate(void)
fdt_blob = (ulong *)__bss_end;
#elif defined CONFIG_EFI_APP
/* FDT is in a separate section */
- fdt_blob = (ulong *)__dtb;
+ fdt_blob = (ulong *)_dtb;
#else
/* FDT is at end of image */
fdt_blob = (ulong *)_end;
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/8] part_find: Enable for sandbox and correct radix and calls
2024-12-09 16:27 [PATCH 0/8] efi: Add docs and tests and a few tweaks Simon Glass
2024-12-09 16:27 ` [PATCH 1/8] fdtdec: Correct devicetree symbol for EFI app Simon Glass
@ 2024-12-09 16:27 ` Simon Glass
2024-12-09 16:27 ` [PATCH 3/8] doc: test: Add docs and test for part_find Simon Glass
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2024-12-09 16:27 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Matthew Garrett, Simon Glass, AKASHI Takahiro, Dmitry Rokosov,
Francis Laniel, Heinrich Schuchardt, Ilias Apalodimas,
Jerome Forissier, Mattijs Korpershoek, Tom Rini
Enable this command for sandbox and the EFI app.
Recent work removed the if_type member. Update the code to use the new
method for obtaining the interface type.
Use hex so that large partition numbers (>=10) work correctly.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
cmd/Kconfig | 1 +
cmd/part_find.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 8fc76778d87..c8ca55194c0 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1552,6 +1552,7 @@ config CMD_PART
config CMD_PART_FIND
bool "part_find"
depends on PARTITIONS
+ default y if SANDBOX || EFI_APP
select HAVE_BLOCK_DEVICE
select PARTITION_UUIDS
select PARTITION_TYPE_GUID
diff --git a/cmd/part_find.c b/cmd/part_find.c
index 8c071d3c374..38919935641 100644
--- a/cmd/part_find.c
+++ b/cmd/part_find.c
@@ -107,7 +107,7 @@ static int part_find(int argc, char *const argv[])
&loader_part_no)) {
char env[256];
- ret = snprintf(env, sizeof(env), "%s %d:%d", blk_get_if_type_name(desc->if_type), desc->devnum, loader_part_no);
+ ret = snprintf(env, sizeof(env), "%s %x:%x", blk_get_if_type_name(desc->if_type), desc->devnum, loader_part_no);
if (ret < 0 || ret == sizeof(env))
return CMD_RET_FAILURE;
if (env_set("target_part", env))
@@ -123,7 +123,7 @@ static int part_find(int argc, char *const argv[])
break;
if (strcasecmp(argv[1], info.type_guid) == 0) {
char env[256];
- ret = snprintf(env, sizeof(env), "%s %d:%d", blk_get_if_type_name(desc->if_type), desc->devnum, i);
+ ret = snprintf(env, sizeof(env), "%s %x:%x", blk_get_uclass_name(desc->uclass_id), desc->devnum, i);
if (ret < 0 || ret == sizeof(env))
return CMD_RET_FAILURE;
env_set("target_part", env);
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 3/8] doc: test: Add docs and test for part_find
2024-12-09 16:27 [PATCH 0/8] efi: Add docs and tests and a few tweaks Simon Glass
2024-12-09 16:27 ` [PATCH 1/8] fdtdec: Correct devicetree symbol for EFI app Simon Glass
2024-12-09 16:27 ` [PATCH 2/8] part_find: Enable for sandbox and correct radix and calls Simon Glass
@ 2024-12-09 16:27 ` Simon Glass
2024-12-10 8:15 ` Heinrich Schuchardt
2024-12-09 16:27 ` [PATCH 4/8] efi: Drop config.h Simon Glass
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2024-12-09 16:27 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Matthew Garrett, Simon Glass, Emil Kronborg, Francis Laniel,
Heinrich Schuchardt, Hou Zhiqiang, Ilias Apalodimas,
Jerome Forissier, Jonas Karlman, Maxim Moskalets,
Rasmus Villemoes, Sebastian Reichel, Tom Rini, Wang Jie
Add some documentation and a test for this new command.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
doc/usage/cmd/part_find.rst | 119 ++++++++++++++++++++++++++++++++++++
doc/usage/index.rst | 1 +
test/cmd/Makefile | 1 +
test/cmd/part_find.c | 42 +++++++++++++
4 files changed, 163 insertions(+)
create mode 100644 doc/usage/cmd/part_find.rst
create mode 100644 test/cmd/part_find.c
diff --git a/doc/usage/cmd/part_find.rst b/doc/usage/cmd/part_find.rst
new file mode 100644
index 00000000000..fd5bd6578d5
--- /dev/null
+++ b/doc/usage/cmd/part_find.rst
@@ -0,0 +1,119 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+.. index::
+ single: part_find (command)
+
+part_find command
+=================
+
+Synopsis
+--------
+
+::
+
+ part_find <uuid>
+ part_find self
+
+Description
+-----------
+
+The `part_find` command is used to find a partition with a given type GUID. When
+it finds one, it sets the target_part environment variable to the corresponding
+``interface dev:part`` string.
+
+uuid
+ Universally Unique Identifier (UUID) to search, expressed as a string
+
+self
+ This is only permitted in the EFI app. It indicates that the required
+ partition is the one from which the app was started.
+
+Example
+-------
+
+This shows searching for an EFI system partition and looking at the files on
+that partition::
+
+ => host bind 1 mmc5.img
+ => part list host 0
+
+ Partition Map for host device 0 -- Partition Type: EFI
+
+ Part Start LBA End LBA Name
+ Attributes
+ Type GUID
+ Partition GUID
+ 1 0x0000202f 0x0000282e ""
+ attrs: 0x0000000000000000
+ type: ebd0a0a2-b9e5-4433-87c0-68b6b72699c7
+ (data)
+ guid: 6b1e51e3-427c-9f45-a947-e467b7216356
+ 2 0x0000002d 0x0000082c ""
+ attrs: 0x0000000000000000
+ type: fe3a2a5d-4f32-41a7-b725-accc3285a309
+ (cros-kern)
+ guid: dece619f-4876-e140-a6c9-8c208a0c9099
+ 3 0x0000202e 0x0000202e ""
+ attrs: 0x0000000000000000
+ type: 3cb8e202-3b7e-47dd-8a3c-7ff2a13cfcec
+ (cros-root)
+ guid: 078cee87-a195-ae4c-a974-8ba3a3d783b3
+ 4 0x0000082d 0x0000102c ""
+ attrs: 0x0000000000000000
+ type: fe3a2a5d-4f32-41a7-b725-accc3285a309
+ (cros-kern)
+ guid: 08d2f20f-d941-fc43-96f6-948931289d71
+ 5 0x0000202d 0x0000202d ""
+ attrs: 0x0000000000000000
+ type: 3cb8e202-3b7e-47dd-8a3c-7ff2a13cfcec
+ (cros-root)
+ guid: 0b23ba00-a11c-ed4e-8b49-5e8738899569
+ 6 0x00000029 0x00000029 ""
+ attrs: 0x0000000000000000
+ type: fe3a2a5d-4f32-41a7-b725-accc3285a309
+ (cros-kern)
+ guid: 6d8158a8-f82d-0d4d-8983-a3ada4eb9b73
+ 7 0x0000002a 0x0000002a ""
+ attrs: 0x0000000000000000
+ type: 3cb8e202-3b7e-47dd-8a3c-7ff2a13cfcec
+ (cros-root)
+ guid: 76e8f9b0-7db7-3844-8f18-21de93485211
+ 8 0x0000102d 0x0000182c ""
+ attrs: 0x0000000000000000
+ type: ebd0a0a2-b9e5-4433-87c0-68b6b72699c7
+ (data)
+ guid: 071dfd2d-173c-f64b-9474-3318665e1d24
+ 9 0x0000002b 0x0000002b ""
+ attrs: 0x0000000000000000
+ type: 2e0a753d-9e48-43b0-8337-b15192cb1b5e
+ (cros-rsrv)
+ guid: b9d078c3-bafa-cd48-b771-a0aaa18d5008
+ 10 0x0000002c 0x0000002c ""
+ attrs: 0x0000000000000000
+ type: 2e0a753d-9e48-43b0-8337-b15192cb1b5e
+ (cros-rsrv)
+ guid: 7b0c0234-1a29-0c4f-bceb-40fae8f7b27c
+ 11 0x00000028 0x00000028 ""
+ attrs: 0x0000000000000000
+ type: cab6e88e-abf3-4102-a07a-d4bb9be3c1d3
+ (cros-fw)
+ guid: aced715d-cd1f-394a-9e3e-24b54a7b1472
+ 12 0x0000182d 0x0000202c ""
+ attrs: 0x0000000000000000
+ type: c12a7328-f81f-11d2-ba4b-00a0c93ec93b
+ (system)
+ guid: e1672afd-75ee-d74e-be95-8726b12b5e74
+ => part_find c12a7328-f81f-11d2-ba4b-00a0c93ec93b
+ => print target_part
+ target_part=host 0:c
+ => ls $target_part
+ EFI/
+
+ 0 file(s), 1 dir(s)
+
+
+Return value
+------------
+
+The return value $? is set to 0 (true) if the command succeeds. If no partition
+could be found, the return value $? is set to 1 (false).
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 71970d5f2b3..64fb91bc5a6 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -95,6 +95,7 @@ Shell commands
cmd/mtrr
cmd/panic
cmd/part
+ cmd/part_find
cmd/pause
cmd/pinmux
cmd/printenv
diff --git a/test/cmd/Makefile b/test/cmd/Makefile
index 583e7c2eec4..dfc49af7bf3 100644
--- a/test/cmd/Makefile
+++ b/test/cmd/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_CMD_LOADM) += loadm.o
obj-$(CONFIG_CMD_MEMINFO) += meminfo.o
obj-$(CONFIG_CMD_MEMORY) += mem_copy.o
obj-$(CONFIG_CMD_MEM_SEARCH) += mem_search.o
+obj-$(CONFIG_CMD_PART_FIND) += part_find.o
ifdef CONFIG_CMD_PCI
obj-$(CONFIG_CMD_PCI_MPS) += pci_mps.o
endif
diff --git a/test/cmd/part_find.c b/test/cmd/part_find.c
new file mode 100644
index 00000000000..1663d4a654f
--- /dev/null
+++ b/test/cmd/part_find.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Test for 'part_find' command
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+#include <dm/ofnode.h>
+#include <dm/test.h>
+#include <test/cmd.h>
+#include <test/ut.h>
+
+/* Test 'part_find' command */
+static int cmd_test_part_find(struct unit_test_state *uts)
+{
+ struct udevice *dev;
+ ofnode root, node;
+
+ /* Enable the requested mmc node since we need a second bootflow */
+ 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));
+
+ ut_assertok(device_probe(dev));
+
+ ut_assertok(env_set("target_part", NULL));
+ ut_assertok(run_command("part_find c12a7328-f81f-11d2-ba4b-00a0c93ec93b", 0));
+ ut_assert_console_end();
+ ut_asserteq_str("mmc 5:c", env_get("target_part"));
+
+ ut_asserteq(1, run_command("part_find invalid", 0));
+ ut_asserteq_str("mmc 5:c", env_get("target_part"));
+
+ ut_assert_console_end();
+
+ return 0;
+}
+CMD_TEST(cmd_test_part_find, UTF_CONSOLE | UTF_DM | UTF_SCAN_FDT);
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 3/8] doc: test: Add docs and test for part_find
2024-12-09 16:27 ` [PATCH 3/8] doc: test: Add docs and test for part_find Simon Glass
@ 2024-12-10 8:15 ` Heinrich Schuchardt
2024-12-10 16:16 ` Simon Glass
0 siblings, 1 reply; 16+ messages in thread
From: Heinrich Schuchardt @ 2024-12-10 8:15 UTC (permalink / raw)
To: Simon Glass
Cc: Matthew Garrett, Emil Kronborg, Francis Laniel, Hou Zhiqiang,
Ilias Apalodimas, Jerome Forissier, Jonas Karlman,
Maxim Moskalets, Rasmus Villemoes, Sebastian Reichel, Tom Rini,
Wang Jie, U-Boot Mailing List
On 09.12.24 17:27, Simon Glass wrote:
> Add some documentation and a test for this new command.
Shouldn't this be two patches?
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> doc/usage/cmd/part_find.rst | 119 ++++++++++++++++++++++++++++++++++++
> doc/usage/index.rst | 1 +
> test/cmd/Makefile | 1 +
> test/cmd/part_find.c | 42 +++++++++++++
> 4 files changed, 163 insertions(+)
> create mode 100644 doc/usage/cmd/part_find.rst
> create mode 100644 test/cmd/part_find.c
>
> diff --git a/doc/usage/cmd/part_find.rst b/doc/usage/cmd/part_find.rst
> new file mode 100644
> index 00000000000..fd5bd6578d5
> --- /dev/null
> +++ b/doc/usage/cmd/part_find.rst
> @@ -0,0 +1,119 @@
> +.. SPDX-License-Identifier: GPL-2.0+:
This is not a valid SPDX identifier.
Cf. https://spdx.org/licenses/GPL-2.0-or-later.html
> +
> +.. index::
> + single: part_find (command)
> +
> +part_find command
> +=================
> +
> +Synopsis
> +--------
> +
> +::
> +
> + part_find <uuid>
> + part_find self
We already have a part command.
Remembering the command would be much easier if you would add the
functionality there.
> +
> +Description
> +-----------
> +
> +The `part_find` command is used to find a partition with a given type GUID. When
> +it finds one, it sets the target_part environment variable to the corresponding
> +``interface dev:part`` string.
> +
> +uuid
> + Universally Unique Identifier (UUID) to search, expressed as a string
> +
> +self
> + This is only permitted in the EFI app. It indicates that the required
> + partition is the one from which the app was started.
> +
> +Example
> +-------
> +
> +This shows searching for an EFI system partition and looking at the files on
> +that partition::
> +
> + => host bind 1 mmc5.img
> + => part list host 0
> +
> + Partition Map for host device 0 -- Partition Type: EFI
> +
> + Part Start LBA End LBA Name
> + Attributes
> + Type GUID
> + Partition GUID
> + 1 0x0000202f 0x0000282e ""
> + attrs: 0x0000000000000000
> + type: ebd0a0a2-b9e5-4433-87c0-68b6b72699c7
> + (data)
> + guid: 6b1e51e3-427c-9f45-a947-e467b7216356
> + 2 0x0000002d 0x0000082c ""
> + attrs: 0x0000000000000000
> + type: fe3a2a5d-4f32-41a7-b725-accc3285a309
> + (cros-kern)
> + guid: dece619f-4876-e140-a6c9-8c208a0c9099
> + 3 0x0000202e 0x0000202e ""
> + attrs: 0x0000000000000000
> + type: 3cb8e202-3b7e-47dd-8a3c-7ff2a13cfcec
> + (cros-root)
> + guid: 078cee87-a195-ae4c-a974-8ba3a3d783b3
> + 4 0x0000082d 0x0000102c ""
> + attrs: 0x0000000000000000
> + type: fe3a2a5d-4f32-41a7-b725-accc3285a309
> + (cros-kern)
> + guid: 08d2f20f-d941-fc43-96f6-948931289d71
> + 5 0x0000202d 0x0000202d ""
> + attrs: 0x0000000000000000
> + type: 3cb8e202-3b7e-47dd-8a3c-7ff2a13cfcec
> + (cros-root)
> + guid: 0b23ba00-a11c-ed4e-8b49-5e8738899569
> + 6 0x00000029 0x00000029 ""
> + attrs: 0x0000000000000000
> + type: fe3a2a5d-4f32-41a7-b725-accc3285a309
> + (cros-kern)
> + guid: 6d8158a8-f82d-0d4d-8983-a3ada4eb9b73
> + 7 0x0000002a 0x0000002a ""
> + attrs: 0x0000000000000000
> + type: 3cb8e202-3b7e-47dd-8a3c-7ff2a13cfcec
> + (cros-root)
> + guid: 76e8f9b0-7db7-3844-8f18-21de93485211
> + 8 0x0000102d 0x0000182c ""
> + attrs: 0x0000000000000000
> + type: ebd0a0a2-b9e5-4433-87c0-68b6b72699c7
> + (data)
> + guid: 071dfd2d-173c-f64b-9474-3318665e1d24
> + 9 0x0000002b 0x0000002b ""
> + attrs: 0x0000000000000000
> + type: 2e0a753d-9e48-43b0-8337-b15192cb1b5e
> + (cros-rsrv)
> + guid: b9d078c3-bafa-cd48-b771-a0aaa18d5008
> + 10 0x0000002c 0x0000002c ""
> + attrs: 0x0000000000000000
> + type: 2e0a753d-9e48-43b0-8337-b15192cb1b5e
> + (cros-rsrv)
> + guid: 7b0c0234-1a29-0c4f-bceb-40fae8f7b27c
> + 11 0x00000028 0x00000028 ""
> + attrs: 0x0000000000000000
> + type: cab6e88e-abf3-4102-a07a-d4bb9be3c1d3
> + (cros-fw)
> + guid: aced715d-cd1f-394a-9e3e-24b54a7b1472
> + 12 0x0000182d 0x0000202c ""
> + attrs: 0x0000000000000000
> + type: c12a7328-f81f-11d2-ba4b-00a0c93ec93b
> + (system)
> + guid: e1672afd-75ee-d74e-be95-8726b12b5e74
This excessive list is just distracting in the documentation. Two
partitions are enough.
> + => part_find c12a7328-f81f-11d2-ba4b-00a0c93ec93b
> + => print target_part
> + target_part=host 0:c
> + => ls $target_part
> + EFI/
> +
> + 0 file(s), 1 dir(s)
> +
> +
> +Return value
> +------------
> +
> +The return value $? is set to 0 (true) if the command succeeds. If no partition
> +could be found, the return value $? is set to 1 (false).
> diff --git a/doc/usage/index.rst b/doc/usage/index.rst
> index 71970d5f2b3..64fb91bc5a6 100644
> --- a/doc/usage/index.rst
> +++ b/doc/usage/index.rst
> @@ -95,6 +95,7 @@ Shell commands
> cmd/mtrr
> cmd/panic
> cmd/part
> + cmd/part_find
> cmd/pause
> cmd/pinmux
> cmd/printenv
> diff --git a/test/cmd/Makefile b/test/cmd/Makefile
> index 583e7c2eec4..dfc49af7bf3 100644
> --- a/test/cmd/Makefile
> +++ b/test/cmd/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_CMD_LOADM) += loadm.o
> obj-$(CONFIG_CMD_MEMINFO) += meminfo.o
> obj-$(CONFIG_CMD_MEMORY) += mem_copy.o
> obj-$(CONFIG_CMD_MEM_SEARCH) += mem_search.o
> +obj-$(CONFIG_CMD_PART_FIND) += part_find.o
> ifdef CONFIG_CMD_PCI
> obj-$(CONFIG_CMD_PCI_MPS) += pci_mps.o
> endif
> diff --git a/test/cmd/part_find.c b/test/cmd/part_find.c
> new file mode 100644
> index 00000000000..1663d4a654f
> --- /dev/null
> +++ b/test/cmd/part_find.c
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: GPL-2.0+
This is not a valid SPDX identifier.
Cf. https://spdx.org/licenses/GPL-2.0-or-later.html
> +/*
> + * Test for 'part_find' command
> + *
> + * Copyright 2024 Google LLC
> + * Written by Simon Glass <sjg@chromium.org>
> + */
> +
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
> +#include <dm/ofnode.h>
> +#include <dm/test.h>
> +#include <test/cmd.h>
> +#include <test/ut.h>
> +
> +/* Test 'part_find' command */
> +static int cmd_test_part_find(struct unit_test_state *uts)
> +{
> + struct udevice *dev;
> + ofnode root, node;
> +
> + /* Enable the requested mmc node since we need a second bootflow */
> + 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));
> +
> + ut_assertok(device_probe(dev));
> +
> + ut_assertok(env_set("target_part", NULL));
> + ut_assertok(run_command("part_find c12a7328-f81f-11d2-ba4b-00a0c93ec93b", 0));
> + ut_assert_console_end();
> + ut_asserteq_str("mmc 5:c", env_get("target_part"));
> +
> + ut_asserteq(1, run_command("part_find invalid", 0));
> + ut_asserteq_str("mmc 5:c", env_get("target_part"));
> +
> + ut_assert_console_end();
> +
> + return 0;
> +}
> +CMD_TEST(cmd_test_part_find, UTF_CONSOLE | UTF_DM | UTF_SCAN_FDT);
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 3/8] doc: test: Add docs and test for part_find
2024-12-10 8:15 ` Heinrich Schuchardt
@ 2024-12-10 16:16 ` Simon Glass
2024-12-10 17:08 ` Tom Rini
0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2024-12-10 16:16 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Matthew Garrett, Emil Kronborg, Francis Laniel, Hou Zhiqiang,
Ilias Apalodimas, Jerome Forissier, Jonas Karlman,
Maxim Moskalets, Rasmus Villemoes, Sebastian Reichel, Tom Rini,
Wang Jie, U-Boot Mailing List
Hi Heinrich,
On Tue, 10 Dec 2024 at 01:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 09.12.24 17:27, Simon Glass wrote:
> > Add some documentation and a test for this new command.
>
> Shouldn't this be two patches?
Often we put the new command, its docs and tests in the same commit,
since the question I always ask when looking at a command is, where
are the docs and tests!
>
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > doc/usage/cmd/part_find.rst | 119 ++++++++++++++++++++++++++++++++++++
> > doc/usage/index.rst | 1 +
> > test/cmd/Makefile | 1 +
> > test/cmd/part_find.c | 42 +++++++++++++
> > 4 files changed, 163 insertions(+)
> > create mode 100644 doc/usage/cmd/part_find.rst
> > create mode 100644 test/cmd/part_find.c
> >
> > diff --git a/doc/usage/cmd/part_find.rst b/doc/usage/cmd/part_find.rst
> > new file mode 100644
> > index 00000000000..fd5bd6578d5
> > --- /dev/null
> > +++ b/doc/usage/cmd/part_find.rst
> > @@ -0,0 +1,119 @@
> > +.. SPDX-License-Identifier: GPL-2.0+:
>
> This is not a valid SPDX identifier.
> Cf. https://spdx.org/licenses/GPL-2.0-or-later.html
I have seen this point made a few times, but I'm afraid I still don't
fully understand it:
The Licenses/README lists the licenses and GPL-2.0+ appears in there.
In the source tree:
$ git grep GPL-2.0+ |wc -l
13406
$ git grep GPL-2.0-or-later |wc -l
1847
I have to say I much prefer GPL-2.0+ as it is easier to remember.
But if we are planning to change, could you update checkpatch to throw
a warning?
Regards,
SImon
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/8] doc: test: Add docs and test for part_find
2024-12-10 16:16 ` Simon Glass
@ 2024-12-10 17:08 ` Tom Rini
2024-12-16 0:29 ` Simon Glass
0 siblings, 1 reply; 16+ messages in thread
From: Tom Rini @ 2024-12-10 17:08 UTC (permalink / raw)
To: Simon Glass
Cc: Heinrich Schuchardt, Matthew Garrett, Emil Kronborg,
Francis Laniel, Hou Zhiqiang, Ilias Apalodimas, Jerome Forissier,
Jonas Karlman, Maxim Moskalets, Rasmus Villemoes,
Sebastian Reichel, Wang Jie, U-Boot Mailing List
[-- Attachment #1: Type: text/plain, Size: 2073 bytes --]
On Tue, Dec 10, 2024 at 09:16:57AM -0700, Simon Glass wrote:
> Hi Heinrich,
>
> On Tue, 10 Dec 2024 at 01:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 09.12.24 17:27, Simon Glass wrote:
> > > Add some documentation and a test for this new command.
> >
> > Shouldn't this be two patches?
>
> Often we put the new command, its docs and tests in the same commit,
> since the question I always ask when looking at a command is, where
> are the docs and tests!
>
> >
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > > doc/usage/cmd/part_find.rst | 119 ++++++++++++++++++++++++++++++++++++
> > > doc/usage/index.rst | 1 +
> > > test/cmd/Makefile | 1 +
> > > test/cmd/part_find.c | 42 +++++++++++++
> > > 4 files changed, 163 insertions(+)
> > > create mode 100644 doc/usage/cmd/part_find.rst
> > > create mode 100644 test/cmd/part_find.c
> > >
> > > diff --git a/doc/usage/cmd/part_find.rst b/doc/usage/cmd/part_find.rst
> > > new file mode 100644
> > > index 00000000000..fd5bd6578d5
> > > --- /dev/null
> > > +++ b/doc/usage/cmd/part_find.rst
> > > @@ -0,0 +1,119 @@
> > > +.. SPDX-License-Identifier: GPL-2.0+:
> >
> > This is not a valid SPDX identifier.
> > Cf. https://spdx.org/licenses/GPL-2.0-or-later.html
>
> I have seen this point made a few times, but I'm afraid I still don't
> fully understand it:
>
> The Licenses/README lists the licenses and GPL-2.0+ appears in there.
> In the source tree:
>
> $ git grep GPL-2.0+ |wc -l
> 13406
> $ git grep GPL-2.0-or-later |wc -l
> 1847
>
> I have to say I much prefer GPL-2.0+ as it is easier to remember.
>
> But if we are planning to change, could you update checkpatch to throw
> a warning?
As I've said before too, GPL-2.0+ is deprecated by SPDX and
GPL-2.0-or-later is the correct tag. But we aren't, sadly, right now a
best practices example for SPDX anyhow and so it's not a deal breaker to
use the old tag, just something that should be avoided.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/8] doc: test: Add docs and test for part_find
2024-12-10 17:08 ` Tom Rini
@ 2024-12-16 0:29 ` Simon Glass
2024-12-16 8:05 ` Heinrich Schuchardt
0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2024-12-16 0:29 UTC (permalink / raw)
To: Tom Rini
Cc: Heinrich Schuchardt, Matthew Garrett, Emil Kronborg,
Francis Laniel, Hou Zhiqiang, Ilias Apalodimas, Jerome Forissier,
Jonas Karlman, Maxim Moskalets, Rasmus Villemoes,
Sebastian Reichel, Wang Jie, U-Boot Mailing List
Hi Tom,
On Tue, 10 Dec 2024 at 10:09, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Dec 10, 2024 at 09:16:57AM -0700, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Tue, 10 Dec 2024 at 01:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > >
> > > On 09.12.24 17:27, Simon Glass wrote:
> > > > Add some documentation and a test for this new command.
> > >
> > > Shouldn't this be two patches?
> >
> > Often we put the new command, its docs and tests in the same commit,
> > since the question I always ask when looking at a command is, where
> > are the docs and tests!
> >
> > >
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >
> > > > doc/usage/cmd/part_find.rst | 119 ++++++++++++++++++++++++++++++++++++
> > > > doc/usage/index.rst | 1 +
> > > > test/cmd/Makefile | 1 +
> > > > test/cmd/part_find.c | 42 +++++++++++++
> > > > 4 files changed, 163 insertions(+)
> > > > create mode 100644 doc/usage/cmd/part_find.rst
> > > > create mode 100644 test/cmd/part_find.c
> > > >
> > > > diff --git a/doc/usage/cmd/part_find.rst b/doc/usage/cmd/part_find.rst
> > > > new file mode 100644
> > > > index 00000000000..fd5bd6578d5
> > > > --- /dev/null
> > > > +++ b/doc/usage/cmd/part_find.rst
> > > > @@ -0,0 +1,119 @@
> > > > +.. SPDX-License-Identifier: GPL-2.0+:
> > >
> > > This is not a valid SPDX identifier.
> > > Cf. https://spdx.org/licenses/GPL-2.0-or-later.html
> >
> > I have seen this point made a few times, but I'm afraid I still don't
> > fully understand it:
> >
> > The Licenses/README lists the licenses and GPL-2.0+ appears in there.
> > In the source tree:
> >
> > $ git grep GPL-2.0+ |wc -l
> > 13406
> > $ git grep GPL-2.0-or-later |wc -l
> > 1847
> >
> > I have to say I much prefer GPL-2.0+ as it is easier to remember.
> >
> > But if we are planning to change, could you update checkpatch to throw
> > a warning?
>
> As I've said before too, GPL-2.0+ is deprecated by SPDX and
> GPL-2.0-or-later is the correct tag. But we aren't, sadly, right now a
> best practices example for SPDX anyhow and so it's not a deal breaker to
> use the old tag, just something that should be avoided.
OK, I will try to remember this.
At minimum, if this is important, Licenses/README should be updated to
drop the old license?
Heinrich, please update checkpatch to warn about this.
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/8] doc: test: Add docs and test for part_find
2024-12-16 0:29 ` Simon Glass
@ 2024-12-16 8:05 ` Heinrich Schuchardt
0 siblings, 0 replies; 16+ messages in thread
From: Heinrich Schuchardt @ 2024-12-16 8:05 UTC (permalink / raw)
To: Simon Glass, Tom Rini
Cc: Matthew Garrett, Emil Kronborg, Francis Laniel, Hou Zhiqiang,
Ilias Apalodimas, Jerome Forissier, Jonas Karlman,
Maxim Moskalets, Rasmus Villemoes, Sebastian Reichel, Wang Jie,
U-Boot Mailing List
Am 16. Dezember 2024 01:29:04 MEZ schrieb Simon Glass <sjg@chromium.org>:
>Hi Tom,
>
>On Tue, 10 Dec 2024 at 10:09, Tom Rini <trini@konsulko.com> wrote:
>>
>> On Tue, Dec 10, 2024 at 09:16:57AM -0700, Simon Glass wrote:
>> > Hi Heinrich,
>> >
>> > On Tue, 10 Dec 2024 at 01:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> > >
>> > > On 09.12.24 17:27, Simon Glass wrote:
>> > > > Add some documentation and a test for this new command.
>> > >
>> > > Shouldn't this be two patches?
>> >
>> > Often we put the new command, its docs and tests in the same commit,
>> > since the question I always ask when looking at a command is, where
>> > are the docs and tests!
>> >
>> > >
>> > > >
>> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
>> > > > ---
>> > > >
>> > > > doc/usage/cmd/part_find.rst | 119 ++++++++++++++++++++++++++++++++++++
>> > > > doc/usage/index.rst | 1 +
>> > > > test/cmd/Makefile | 1 +
>> > > > test/cmd/part_find.c | 42 +++++++++++++
>> > > > 4 files changed, 163 insertions(+)
>> > > > create mode 100644 doc/usage/cmd/part_find.rst
>> > > > create mode 100644 test/cmd/part_find.c
>> > > >
>> > > > diff --git a/doc/usage/cmd/part_find.rst b/doc/usage/cmd/part_find.rst
>> > > > new file mode 100644
>> > > > index 00000000000..fd5bd6578d5
>> > > > --- /dev/null
>> > > > +++ b/doc/usage/cmd/part_find.rst
>> > > > @@ -0,0 +1,119 @@
>> > > > +.. SPDX-License-Identifier: GPL-2.0+:
>> > >
>> > > This is not a valid SPDX identifier.
>> > > Cf. https://spdx.org/licenses/GPL-2.0-or-later.html
>> >
>> > I have seen this point made a few times, but I'm afraid I still don't
>> > fully understand it:
>> >
>> > The Licenses/README lists the licenses and GPL-2.0+ appears in there.
>> > In the source tree:
>> >
>> > $ git grep GPL-2.0+ |wc -l
>> > 13406
>> > $ git grep GPL-2.0-or-later |wc -l
>> > 1847
>> >
>> > I have to say I much prefer GPL-2.0+ as it is easier to remember.
>> >
>> > But if we are planning to change, could you update checkpatch to throw
>> > a warning?
>>
>> As I've said before too, GPL-2.0+ is deprecated by SPDX and
>> GPL-2.0-or-later is the correct tag. But we aren't, sadly, right now a
>> best practices example for SPDX anyhow and so it's not a deal breaker to
>> use the old tag, just something that should be avoided.
>
>OK, I will try to remember this.
>
>At minimum, if this is important, Licenses/README should be updated to
>drop the old license?
>
>Heinrich, please update checkpatch to warn about this.
>
>Regards,
>Simon
See
<https://lore.kernel.org/u-boot/20241215023732.68902-1-heinrich.schuchardt@canonical.com/>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/8] efi: Drop config.h
2024-12-09 16:27 [PATCH 0/8] efi: Add docs and tests and a few tweaks Simon Glass
` (2 preceding siblings ...)
2024-12-09 16:27 ` [PATCH 3/8] doc: test: Add docs and test for part_find Simon Glass
@ 2024-12-09 16:27 ` Simon Glass
2024-12-09 16:27 ` [PATCH 5/8] cmd: Refactor part_find() into separate functions Simon Glass
` (3 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2024-12-09 16:27 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Matthew Garrett, Simon Glass, Ilias Apalodimas, Joe Hershberger,
Ramon Fried, Tom Rini
We don't need to include this file anymore, so drop it.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
drivers/net/efi_net.c | 1 -
drivers/tpm/tpm2_efi.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/net/efi_net.c b/drivers/net/efi_net.c
index 370056040f3..abc69058437 100644
--- a/drivers/net/efi_net.c
+++ b/drivers/net/efi_net.c
@@ -4,7 +4,6 @@
*
*/
-#include <config.h>
#include <dm.h>
#include <efi.h>
#include <efi_api.h>
diff --git a/drivers/tpm/tpm2_efi.c b/drivers/tpm/tpm2_efi.c
index 2eb144891d8..c6e7db762b5 100644
--- a/drivers/tpm/tpm2_efi.c
+++ b/drivers/tpm/tpm2_efi.c
@@ -4,7 +4,6 @@
*
*/
-#include <config.h>
#include <dm.h>
#include <efi.h>
#include <efi_api.h>
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 5/8] cmd: Refactor part_find() into separate functions
2024-12-09 16:27 [PATCH 0/8] efi: Add docs and tests and a few tweaks Simon Glass
` (3 preceding siblings ...)
2024-12-09 16:27 ` [PATCH 4/8] efi: Drop config.h Simon Glass
@ 2024-12-09 16:27 ` Simon Glass
2024-12-09 16:27 ` [PATCH 6/8] addr_find: Use a simple lmb allocation Simon Glass
` (2 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2024-12-09 16:27 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Matthew Garrett, Simon Glass, Tom Rini
This function uses #ifdefs and ends up with a very large indent, Split
it into two separate functions, one for the EFI app and one for other
builds.
Drop the use of config.h while we are here.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
cmd/part_find.c | 153 +++++++++++++++++++++++++++++-------------------
1 file changed, 93 insertions(+), 60 deletions(-)
diff --git a/cmd/part_find.c b/cmd/part_find.c
index 38919935641..0864c3f1042 100644
--- a/cmd/part_find.c
+++ b/cmd/part_find.c
@@ -5,12 +5,10 @@
*/
#include <blk.h>
-#include <config.h>
#include <command.h>
#include <dm.h>
#include <env.h>
#include <part.h>
-#if defined(CONFIG_EFI) || defined(CONFIG_EFI_APP)
#include <efi.h>
#include <efi_api.h>
@@ -47,21 +45,86 @@ static bool partition_is_on_device(const struct efi_device_path *device,
}
return false;
}
-#endif
+
+/**
+ * part_self_find() - Check if a device contains the loaded-image path
+ *
+ * @udev: Block device to check
+ * @loaded_image_path: EFI path of the loaded image
+ * Return 0 if found, -ENOENT if not, other -ve value on error
+ */
+static int part_self_find(struct udevice *udev,
+ struct efi_device_path *loaded_image_path)
+{
+ struct blk_desc *desc = dev_get_uclass_plat(udev);
+
+ if (desc->uclass_id == UCLASS_EFI_MEDIA) {
+ struct efi_media_plat *plat = dev_get_plat(udev->parent);
+ u32 loader_part_no;
+
+ if (partition_is_on_device(plat->device_path, loaded_image_path,
+ &loader_part_no)) {
+ char env[256];
+ int ret;
+
+ ret = snprintf(env, sizeof(env), "%s %x:%x",
+ blk_get_uclass_name(desc->uclass_id),
+ desc->devnum, loader_part_no);
+ if (ret < 0 || ret == sizeof(env))
+ return -ENOSPC;
+ if (env_set("target_part", env))
+ return -ENOMEM;
+ return 0;
+ }
+ }
+
+ return -ENOENT;
+}
+
+/**
+ * part_blk_find() - Check if a device contains a partition with a type uuid
+ *
+ * @udev: Block device to check
+ * @uuid: UUID to search for (in string form)
+ * Return 0 if found, -ENOENT if not, other -ve value on error
+ */
+static int part_blk_find(struct udevice *udev, const char *uuid)
+{
+ struct blk_desc *desc = dev_get_uclass_plat(udev);
+ int i;
+
+ for (i = 1; i <= MAX_SEARCH_PARTITIONS; i++) {
+ struct disk_partition info;
+ int ret;
+
+ ret = part_get_info(desc, i, &info);
+ if (ret)
+ break;
+ if (strcasecmp(uuid, info.type_guid) == 0) {
+ char env[256];
+
+ ret = snprintf(env, sizeof(env), "%s %x:%x",
+ blk_get_uclass_name(desc->uclass_id),
+ desc->devnum, i);
+ if (ret < 0 || ret == sizeof(env))
+ return -ENOSPC;
+ debug("Setting target_part to %s\n", env);
+ if (env_set("target_part", env))
+ return -ENOMEM;
+ return 0;
+ }
+ }
+
+ return -ENOENT;
+}
static int part_find(int argc, char *const argv[])
{
-#if defined(CONFIG_EFI) || defined(CONFIG_EFI_APP)
efi_guid_t efi_devpath_guid = EFI_DEVICE_PATH_PROTOCOL_GUID;
struct efi_device_path *loaded_image_path = NULL;
- struct efi_boot_services *boot = efi_get_boot();
- struct efi_priv *priv = efi_get_priv();
bool part_self = false;
-#endif
struct driver *d = ll_entry_start(struct driver, driver);
const int n_ents = ll_entry_count(struct driver, driver);
- struct disk_partition info;
- struct blk_desc *desc;
struct driver *entry;
struct udevice *udev;
struct uclass *uc;
@@ -70,16 +133,20 @@ static int part_find(int argc, char *const argv[])
if (argc != 2)
return CMD_RET_USAGE;
-#if defined(CONFIG_EFI) || defined (CONFIG_EFI_APP)
- part_self = !strncmp(argv[1], "self", 6);
- if (part_self) {
- ret = boot->handle_protocol(priv->loaded_image->device_handle,
- &efi_devpath_guid,
- (void **)&loaded_image_path);
- if (ret)
- log_warning("failed to get device path for loaded image (ret=%d)", ret);
+ if (IS_ENABLED(CONFIG_EFI)) {
+ struct efi_boot_services *boot = efi_get_boot();
+ struct efi_priv *priv = efi_get_priv();
+
+ part_self = !strncmp(argv[1], "self", 6);
+ if (part_self) {
+ ret = boot->handle_protocol(priv->loaded_image->device_handle,
+ &efi_devpath_guid,
+ (void **)&loaded_image_path);
+ if (ret)
+ log_warning("failed to get device path for loaded image (ret=%d)",
+ ret);
+ }
}
-#endif
ret = uclass_get(UCLASS_BLK, &uc);
if (ret) {
@@ -90,50 +157,16 @@ static int part_find(int argc, char *const argv[])
if (entry->id != UCLASS_BLK)
continue;
uclass_foreach_dev(udev, uc) {
- int i;
-
if (udev->driver != entry)
continue;
- desc = dev_get_uclass_plat(udev);
-#if defined(CONFIG_EFI) || defined(CONFIG_EFI_APP)
- if (part_self) {
- if (desc->if_type == IF_TYPE_EFI_MEDIA) {
- struct efi_media_plat *plat =
- dev_get_plat(udev->parent);
- __u32 loader_part_no;
-
- if (partition_is_on_device(plat->device_path,
- loaded_image_path,
- &loader_part_no)) {
- char env[256];
-
- ret = snprintf(env, sizeof(env), "%s %x:%x", blk_get_if_type_name(desc->if_type), desc->devnum, loader_part_no);
- if (ret < 0 || ret == sizeof(env))
- return CMD_RET_FAILURE;
- if (env_set("target_part", env))
- return CMD_RET_FAILURE;
- return CMD_RET_SUCCESS;
- }
- }
- } else {
-#endif
- for (i = 1; i <= MAX_SEARCH_PARTITIONS; i++) {
- ret = part_get_info(desc, i, &info);
- if (ret)
- break;
- if (strcasecmp(argv[1], info.type_guid) == 0) {
- char env[256];
- ret = snprintf(env, sizeof(env), "%s %x:%x", blk_get_uclass_name(desc->uclass_id), desc->devnum, i);
- if (ret < 0 || ret == sizeof(env))
- return CMD_RET_FAILURE;
- env_set("target_part", env);
- debug("Setting target_part to %s\n", env);
- return CMD_RET_SUCCESS;
- }
- }
-#if defined(CONFIG_EFI) || defined(CONFIG_EFI_APP)
- }
-#endif
+ if (IS_ENABLED(CONFIG_EFI) && part_self)
+ ret = part_self_find(udev, loaded_image_path);
+ else
+ ret = part_blk_find(udev, argv[1]);
+ if (!ret)
+ return 0;
+ if (ret != -ENOENT)
+ break;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 6/8] addr_find: Use a simple lmb allocation
2024-12-09 16:27 [PATCH 0/8] efi: Add docs and tests and a few tweaks Simon Glass
` (4 preceding siblings ...)
2024-12-09 16:27 ` [PATCH 5/8] cmd: Refactor part_find() into separate functions Simon Glass
@ 2024-12-09 16:27 ` Simon Glass
2024-12-09 16:28 ` [PATCH 7/8] doc: test: Add docs and test for addr_find Simon Glass
2024-12-09 16:28 ` [PATCH 8/8] efi: Avoid a memory leak in efi_bind_block() on error path Simon Glass
7 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2024-12-09 16:27 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Matthew Garrett, Simon Glass, AKASHI Takahiro, Dmitry Rokosov,
Francis Laniel, Heinrich Schuchardt, Ilias Apalodimas,
Jerome Forissier, Mattijs Korpershoek, Tom Rini
There should be no need to parse the LMB tables manually. Use the
allocation-function provided instead. Adjust the argument checks while
we are here.
Also enable this command for sandbox and the EFI app, so it is built in
CI.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
cmd/Kconfig | 1 +
cmd/addr_find.c | 40 +++++++++++++---------------------------
2 files changed, 14 insertions(+), 27 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index c8ca55194c0..d4da504bea4 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -130,6 +130,7 @@ config CMD_ACPI
config CMD_ADDR_FIND
bool "addr_find"
+ default y if SANDBOX || EFI_APP
help
This command searches for an unused region of address space
sufficiently large to hold a file. If successful, it sets the
diff --git a/cmd/addr_find.c b/cmd/addr_find.c
index b187337d885..2c20b959031 100644
--- a/cmd/addr_find.c
+++ b/cmd/addr_find.c
@@ -16,12 +16,13 @@ DECLARE_GLOBAL_DATA_PTR;
int do_addr_find(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
{
- struct lmb_region *mem, *reserved;
const char *filename;
- struct lmb lmb;
+ phys_addr_t start;
loff_t size;
int ret;
- int i, j;
+
+ if (argc < 3)
+ return CMD_RET_USAGE;
if (!gd->fdt_blob) {
log_err("No FDT setup\n");
@@ -49,36 +50,21 @@ int do_addr_find(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
return CMD_RET_FAILURE;
}
- lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
- mem = &lmb.memory;
- reserved = &lmb.reserved;
-
- for (i = 0; i < mem->cnt; i++) {
- unsigned long long start, end;
+ start = lmb_alloc(size, SZ_2M);
+ if ((long)start < 0) {
+ log_err("Failed to find enough RAM for 0x%llx bytes\n", size);
- start = mem->region[i].base;
- end = mem->region[i].base + mem->region[i].size - 1;
- if ((start + size) > end)
- continue;
- for (j = 0; j < reserved->cnt; j++) {
- if ((reserved->region[j].base + reserved->region[j].size) < start)
- continue;
- if ((start + size) > reserved->region[j].base)
- start = reserved->region[j].base + reserved->region[j].size;
- }
- if ((start + size) <= end) {
- env_set_hex("loadaddr", start);
- debug("Set loadaddr to 0x%llx\n", start);
- return CMD_RET_SUCCESS;
- }
+ return CMD_RET_FAILURE;
}
- log_err("Failed to find enough RAM for 0x%llx bytes\n", size);
- return CMD_RET_FAILURE;
+ env_set_hex("loadaddr", start);
+ debug("Set loadaddr to %llx\n", (u64)start);
+
+ return 0;
}
U_BOOT_CMD(
- addr_find, 7, 1, do_addr_find,
+ addr_find, 4, 1, do_addr_find,
"find a load address suitable for a file",
"<interface> [<dev[:part]>] <filename>\n"
"- find a consecutive region of memory sufficiently large to hold\n"
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 7/8] doc: test: Add docs and test for addr_find
2024-12-09 16:27 [PATCH 0/8] efi: Add docs and tests and a few tweaks Simon Glass
` (5 preceding siblings ...)
2024-12-09 16:27 ` [PATCH 6/8] addr_find: Use a simple lmb allocation Simon Glass
@ 2024-12-09 16:28 ` Simon Glass
2024-12-09 16:28 ` [PATCH 8/8] efi: Avoid a memory leak in efi_bind_block() on error path Simon Glass
7 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2024-12-09 16:28 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Matthew Garrett, Simon Glass, Emil Kronborg, Francis Laniel,
Heinrich Schuchardt, Hou Zhiqiang, Ilias Apalodimas,
Jerome Forissier, Maxim Moskalets, Rasmus Villemoes,
Sebastian Reichel, Tom Rini
Add documentation and a test for this command.
Drop the use of config.h while we are here.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
cmd/addr_find.c | 1 -
doc/usage/cmd/addr_find.rst | 63 +++++++++++++++++++++++++++++++++++++
doc/usage/index.rst | 1 +
test/cmd/Makefile | 1 +
test/cmd/addr_find.c | 27 ++++++++++++++++
5 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 doc/usage/cmd/addr_find.rst
create mode 100644 test/cmd/addr_find.c
diff --git a/cmd/addr_find.c b/cmd/addr_find.c
index 2c20b959031..601b8b2566b 100644
--- a/cmd/addr_find.c
+++ b/cmd/addr_find.c
@@ -5,7 +5,6 @@
*/
#include <blk.h>
-#include <config.h>
#include <command.h>
#include <env.h>
#include <fs.h>
diff --git a/doc/usage/cmd/addr_find.rst b/doc/usage/cmd/addr_find.rst
new file mode 100644
index 00000000000..0d16ffd236b
--- /dev/null
+++ b/doc/usage/cmd/addr_find.rst
@@ -0,0 +1,63 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+.. index::
+ single: addr_find (command)
+
+addr_find command
+=================
+
+Synopsis
+--------
+
+::
+
+ addr_find <interface> [<dev[:part]> [<filename>]]
+
+Description
+-----------
+
+The addr_find command is used to find a consecutive region of memory
+sufficiently large to hold a file, ensuring that the memory is not currently in
+use for another file, etc.
+
+If successful, 'loadaddr' is set to the located address.
+
+The number of transferred bytes is saved in the environment variable filesize.
+The load address is saved in the environment variable fileaddr.
+
+interface
+ interface for accessing the block device (mmc, sata, scsi, usb, ....)
+
+dev
+ device number
+
+part
+ partition number, defaults to 0 (whole device)
+
+filename
+ path to file, defaults to environment variable 'bootfile'
+
+Example
+-------
+
+This shows obtaining an address suitable for a file on an mmc disk::
+
+ => ls mmc 1
+ extlinux/
+ 97135227 initramfs-5.3.7-301.fc31.armv7hl.img
+ dtb-5.3.7-301.fc31.armv7hl/
+ 12531628 vmlinuz-5.3.7-301.fc31.armv7hl
+
+ 2 file(s), 2 dir(s)
+
+ => addr_find mmc 1 vmlinuz-5.3.7-301.fc31.armv7hl
+ => print loadaddr
+ loadaddr=7c00000
+ =>
+
+
+Return value
+------------
+
+The return value $? is set to 0 (true) if the command succeeds. If no suitable
+address could be found, the return value $? is set to 1 (false).
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 64fb91bc5a6..9cbc9a2c9f9 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -24,6 +24,7 @@ Shell commands
:maxdepth: 1
cmd/acpi
+ cmd/addr_find
cmd/addrmap
cmd/armffa
cmd/askenv
diff --git a/test/cmd/Makefile b/test/cmd/Makefile
index dfc49af7bf3..912579e0c6b 100644
--- a/test/cmd/Makefile
+++ b/test/cmd/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_CMD_PAUSE) += test_pause.o
endif
obj-y += exit.o mem.o
obj-$(CONFIG_X86) += cpuid.o msr.o
+obj-$(CONFIG_CMD_ADDR_FIND) += addr_find.o
obj-$(CONFIG_CMD_ADDRMAP) += addrmap.o
obj-$(CONFIG_CMD_BDI) += bdinfo.o
obj-$(CONFIG_COREBOOT_SYSINFO) += coreboot.o
diff --git a/test/cmd/addr_find.c b/test/cmd/addr_find.c
new file mode 100644
index 00000000000..ce087759d9e
--- /dev/null
+++ b/test/cmd/addr_find.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Test for 'part_find' command
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+#include <dm/ofnode.h>
+#include <dm/test.h>
+#include <test/cmd.h>
+#include <test/ut.h>
+
+/* Test 'addr_find' command */
+static int cmd_test_addr_find(struct unit_test_state *uts)
+{
+ ut_assertok(env_set("loadaddr", NULL));
+ ut_assertok(run_command("addr_find mmc 1:1 vmlinuz-5.3.7-301.fc31.armv7hl", 0));
+ ut_assert_console_end();
+
+ ut_assertnonnull(env_get("loadaddr"));
+
+ return 0;
+}
+CMD_TEST(cmd_test_addr_find, UTF_CONSOLE | UTF_DM | UTF_SCAN_FDT);
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 8/8] efi: Avoid a memory leak in efi_bind_block() on error path
2024-12-09 16:27 [PATCH 0/8] efi: Add docs and tests and a few tweaks Simon Glass
` (6 preceding siblings ...)
2024-12-09 16:28 ` [PATCH 7/8] doc: test: Add docs and test for addr_find Simon Glass
@ 2024-12-09 16:28 ` Simon Glass
2024-12-10 8:10 ` Heinrich Schuchardt
7 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2024-12-09 16:28 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Matthew Garrett, Simon Glass, Ilias Apalodimas,
Heinrich Schuchardt, Janis Danisevskis, Tom Rini
Free the memory used if an error occurs.
Signed-off-by: Simon Glass <sjg@chromium.org>
Suggested-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
lib/efi/efi_app_init.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/efi/efi_app_init.c b/lib/efi/efi_app_init.c
index cc91e1d74b8..fec3f348cfb 100644
--- a/lib/efi/efi_app_init.c
+++ b/lib/efi/efi_app_init.c
@@ -60,8 +60,10 @@ int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio,
plat->handle = handle;
plat->blkio = blkio;
plat->device_path = malloc(device_path_len);
- if (!plat->device_path)
+ if (!plat->device_path) {
+ free(plat);
return log_msg_ret("path", -ENOMEM);
+ }
memcpy(plat->device_path, device_path, device_path_len);
ret = device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media",
plat, ofnode_null(), &dev);
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 8/8] efi: Avoid a memory leak in efi_bind_block() on error path
2024-12-09 16:28 ` [PATCH 8/8] efi: Avoid a memory leak in efi_bind_block() on error path Simon Glass
@ 2024-12-10 8:10 ` Heinrich Schuchardt
2024-12-16 15:15 ` Simon Glass
0 siblings, 1 reply; 16+ messages in thread
From: Heinrich Schuchardt @ 2024-12-10 8:10 UTC (permalink / raw)
To: Simon Glass
Cc: Matthew Garrett, Ilias Apalodimas, Janis Danisevskis, Tom Rini,
U-Boot Mailing List
On 09.12.24 17:28, Simon Glass wrote:
> Free the memory used if an error occurs.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Suggested-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>
> lib/efi/efi_app_init.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi/efi_app_init.c b/lib/efi/efi_app_init.c
> index cc91e1d74b8..fec3f348cfb 100644
> --- a/lib/efi/efi_app_init.c
> +++ b/lib/efi/efi_app_init.c
> @@ -60,8 +60,10 @@ int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio,
> plat->handle = handle;
> plat->blkio = blkio;
> plat->device_path = malloc(device_path_len);
> - if (!plat->device_path)
> + if (!plat->device_path) {
> + free(plat);
As of origin master origin/next (9dd0a9ecaa539ad) plat is a local
variable and not a pointer. You cannot free it.
If there is some prerequisite patch that is making plat a pointer, you
should add this correction to that patch.
Best regards
Heinrich
> return log_msg_ret("path", -ENOMEM);
> + }
> memcpy(plat->device_path, device_path, device_path_len);
> ret = device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media",
> plat, ofnode_null(), &dev);
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 8/8] efi: Avoid a memory leak in efi_bind_block() on error path
2024-12-10 8:10 ` Heinrich Schuchardt
@ 2024-12-16 15:15 ` Simon Glass
0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2024-12-16 15:15 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Matthew Garrett, Ilias Apalodimas, Janis Danisevskis, Tom Rini,
U-Boot Mailing List
Hi Heinrich,
On Tue, 10 Dec 2024 at 01:15, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 09.12.24 17:28, Simon Glass wrote:
> > Free the memory used if an error occurs.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > Suggested-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >
> > lib/efi/efi_app_init.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi/efi_app_init.c b/lib/efi/efi_app_init.c
> > index cc91e1d74b8..fec3f348cfb 100644
> > --- a/lib/efi/efi_app_init.c
> > +++ b/lib/efi/efi_app_init.c
> > @@ -60,8 +60,10 @@ int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio,
> > plat->handle = handle;
> > plat->blkio = blkio;
> > plat->device_path = malloc(device_path_len);
> > - if (!plat->device_path)
> > + if (!plat->device_path) {
> > + free(plat);
>
> As of origin master origin/next (9dd0a9ecaa539ad) plat is a local
> variable and not a pointer. You cannot free it.
>
> If there is some prerequisite patch that is making plat a pointer, you
> should add this correction to that patch.
This patch is for sjg/master as the original series has not landed in
Tom's tree (yet).
>
> Best regards
>
> Heinrich
>
> > return log_msg_ret("path", -ENOMEM);
> > + }
> > memcpy(plat->device_path, device_path, device_path_len);
> > ret = device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media",
> > plat, ofnode_null(), &dev);
>
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread