Re: [PATCH 1/3] abootcmd: Add load subcommand
Hi Roman,
Thank you for the patch.
On dim., mai 19, 2024 at 19:18, Roman Stratiienko
wrote:
> What it does:
> 1. Allocates the memory in HEAP to fit the partition
> 2. Loads partition into memory. In the following patch of this series,
>loading will be optimized to avoid loading an empty space.
> 3. Sets buffer start and buffer size value into environment variables
>abootimg__ptr and abootimg__size, respectively.
> and duplicate them as
>abootimg___ptr and abootimg___size.
>The latter two are needed to access by the AVB get_preloaded_partition.
>(see the next patch).
>
> Before this command, the boot script developer was responsible for
> allocating the memory manually by choosing the start and the end,
> which is far from good.
>
> Usage example:
>
> abootcmd load mmc 0 boot a
Should this be: abootimg load mmc 0 boot a ?
>
> Signed-off-by: Roman Stratiienko
> ---
> cmd/abootimg.c | 84 +-
> 1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/abootimg.c b/cmd/abootimg.c
> index a5321bab6a..808c9c4941 100644
> --- a/cmd/abootimg.c
> +++ b/cmd/abootimg.c
> @@ -8,7 +8,9 @@
> #include
> #include
> #include
> +#include
> #include
> +#include
>
> #define abootimg_addr() \
> (_abootimg_addr == -1 ? image_load_addr : _abootimg_addr)
> @@ -259,10 +261,81 @@ static int do_abootimg_dump(struct cmd_tbl *cmdtp, int
> flag, int argc,
> return CMD_RET_SUCCESS;
> }
>
> +static int do_abootimg_load(struct cmd_tbl *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + int time_start = get_timer(0);
On -next branch (where the patch will be applied), I see the following
build error:
cmd/abootimg.c: In function ‘do_abootimg_load’:
cmd/abootimg.c:279:26: error: implicit declaration of function ‘get_timer’
[-Wimplicit-function-declaration]
279 | int time_start = get_timer(0);
| ^
Please add the missing include: #include
Base: 7e52d6ccfb76 ("Merge patch series "FWU: Add support for FWU metadata
version 2"")
> + struct blk_desc *desc;
> + struct disk_partition info;
> + char buf[512] = { 0 };
> + void *addr;
> + int ret;
> +
> + if (argc < 4)
> + return CMD_RET_USAGE;
> + if (argc > 5)
> + return CMD_RET_USAGE;
Can we please do this in a single line, like done in other commands such
as do_abootimg_addr()?
> +
> + ret = blk_get_device_by_str(argv[1], argv[2], );
> + if (ret < 0) {
> + printf("Error: Failed to get device %s %s\n", argv[1], argv[2]);
> + return CMD_RET_FAILURE;
> + }
> +
> + if (argc == 5)
> + sprintf(buf, "%s_%s", argv[3], argv[4]);
> + else
> + sprintf(buf, "%s", argv[3]);
> +
> + ret = part_get_info_by_name(desc, buf, );
> + if (ret < 0) {
> + printf("Error: Failed to get partition %s\n", buf);
> + return CMD_RET_FAILURE;
> + }
> +
> + addr = (void *)memalign(4096, info.size * info.blksz);
Why 4096? If this is a known number, can we use a define for it please?
> + if (!addr) {
> + printf("Error: Failed to allocate memory\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + ret = blk_dread(desc, info.start, info.size, addr);
> + if (ret < 0) {
> + printf("Error: Failed to read partition %s\n", buf);
> + goto fail;
> + }
> +
> + sprintf(buf, "abootimg_%s_ptr", argv[3]);
> + env_set_hex(buf, (ulong)addr);
> +
> + sprintf(buf, "abootimg_%s_size", argv[3]);
> + env_set_hex(buf, info.size * info.blksz);
> +
> + if (argc == 5) {
> + sprintf(buf, "abootimg_%s_%s_ptr", argv[3], argv[4]);
> + env_set_hex(buf, (ulong)addr);
> +
> + sprintf(buf, "abootimg_%s_%s_size", argv[3], argv[4]);
> + env_set_hex(buf, info.size * info.blksz);
> + }
> +
> + int time_end = get_timer(0);
> +
> + printf("Loaded '%s' partition to address 0x%p (size: 0x%x) in %lu ms\n",
> +argv[3], addr, info.size * info.blksz, time_end - time_start);
This causes warnings on -next when building sandbox:
$ make sandbox_defconfig
$ make
cmd/abootimg.c:339:65: warning: format ‘%x’ expects argument of type ‘unsigned
int’, but argument 4 has type ‘long unsigned int’ [-Wformat=]
339 | printf("Loaded '%s' partition to address 0x%p (size: 0x%x) in
%lu ms\n",
|~^
| |
|
unsigned int
|%lx
340 |argv[3], addr, info.size * info.blksz, time_end -
time_start);
| ~~
|
[PATCH 1/3] abootcmd: Add load subcommand
What it does:
1. Allocates the memory in HEAP to fit the partition
2. Loads partition into memory. In the following patch of this series,
loading will be optimized to avoid loading an empty space.
3. Sets buffer start and buffer size value into environment variables
abootimg__ptr and abootimg__size, respectively.
and duplicate them as
abootimg___ptr and abootimg___size.
The latter two are needed to access by the AVB get_preloaded_partition.
(see the next patch).
Before this command, the boot script developer was responsible for
allocating the memory manually by choosing the start and the end,
which is far from good.
Usage example:
abootcmd load mmc 0 boot a
Signed-off-by: Roman Stratiienko
---
cmd/abootimg.c | 84 +-
1 file changed, 83 insertions(+), 1 deletion(-)
diff --git a/cmd/abootimg.c b/cmd/abootimg.c
index a5321bab6a..808c9c4941 100644
--- a/cmd/abootimg.c
+++ b/cmd/abootimg.c
@@ -8,7 +8,9 @@
#include
#include
#include
+#include
#include
+#include
#define abootimg_addr() \
(_abootimg_addr == -1 ? image_load_addr : _abootimg_addr)
@@ -259,10 +261,81 @@ static int do_abootimg_dump(struct cmd_tbl *cmdtp, int
flag, int argc,
return CMD_RET_SUCCESS;
}
+static int do_abootimg_load(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ int time_start = get_timer(0);
+ struct blk_desc *desc;
+ struct disk_partition info;
+ char buf[512] = { 0 };
+ void *addr;
+ int ret;
+
+ if (argc < 4)
+ return CMD_RET_USAGE;
+ if (argc > 5)
+ return CMD_RET_USAGE;
+
+ ret = blk_get_device_by_str(argv[1], argv[2], );
+ if (ret < 0) {
+ printf("Error: Failed to get device %s %s\n", argv[1], argv[2]);
+ return CMD_RET_FAILURE;
+ }
+
+ if (argc == 5)
+ sprintf(buf, "%s_%s", argv[3], argv[4]);
+ else
+ sprintf(buf, "%s", argv[3]);
+
+ ret = part_get_info_by_name(desc, buf, );
+ if (ret < 0) {
+ printf("Error: Failed to get partition %s\n", buf);
+ return CMD_RET_FAILURE;
+ }
+
+ addr = (void *)memalign(4096, info.size * info.blksz);
+ if (!addr) {
+ printf("Error: Failed to allocate memory\n");
+ return CMD_RET_FAILURE;
+ }
+
+ ret = blk_dread(desc, info.start, info.size, addr);
+ if (ret < 0) {
+ printf("Error: Failed to read partition %s\n", buf);
+ goto fail;
+ }
+
+ sprintf(buf, "abootimg_%s_ptr", argv[3]);
+ env_set_hex(buf, (ulong)addr);
+
+ sprintf(buf, "abootimg_%s_size", argv[3]);
+ env_set_hex(buf, info.size * info.blksz);
+
+ if (argc == 5) {
+ sprintf(buf, "abootimg_%s_%s_ptr", argv[3], argv[4]);
+ env_set_hex(buf, (ulong)addr);
+
+ sprintf(buf, "abootimg_%s_%s_size", argv[3], argv[4]);
+ env_set_hex(buf, info.size * info.blksz);
+ }
+
+ int time_end = get_timer(0);
+
+ printf("Loaded '%s' partition to address 0x%p (size: 0x%x) in %lu ms\n",
+ argv[3], addr, info.size * info.blksz, time_end - time_start);
+
+ return CMD_RET_SUCCESS;
+
+fail:
+ free(addr);
+ return CMD_RET_FAILURE;
+}
+
static struct cmd_tbl cmd_abootimg_sub[] = {
U_BOOT_CMD_MKENT(addr, 4, 1, do_abootimg_addr, "", ""),
U_BOOT_CMD_MKENT(dump, 2, 1, do_abootimg_dump, "", ""),
U_BOOT_CMD_MKENT(get, 5, 1, do_abootimg_get, "", ""),
+ U_BOOT_CMD_MKENT(load, 5, 1, do_abootimg_load, "", ""),
};
static int do_abootimg(struct cmd_tbl *cmdtp, int flag, int argc,
@@ -305,5 +378,14 @@ U_BOOT_CMD(
"- get address and size (hex) of DT blob in the image by index\n"
" : index number of desired DT blob in DTB area\n"
" [addr_var]: variable name to contain DT blob address\n"
- " [size_var]: variable name to contain DT blob size"
+ " [size_var]: variable name to contain DT blob size\n"
+ "abootimg load interface dev partition [slot_name]\n"
+ "- load boot image from device partition\n"
+ " memory is allocated in heap\n"
+ " address is stored in $abootimg__ptr\n"
+ " size is stored in $abootimg__size\n"
+ " interface: interface type (e.g. mmc, usb)\n"
+ " dev: device number (e.g. 0, 1)\n"
+ " partition: partition number (e.g. boot, dtb)\n"
+ " slot_suffix: slot name (e.g. a, b)"
);
--
2.40.1
