* [PATCH v2 01/33] bloblist: add api to get blob with size
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
@ 2025-01-06 14:31 ` Simon Glass
2025-01-06 14:31 ` [PATCH v2 02/33] bloblist: Drop BSD license Simon Glass
` (33 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:31 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Raymond Mao, Ilias Apalodimas, Simon Glass, Levi Yun,
Patrick Rudolph, Tom Rini
From: Raymond Mao <raymond.mao@linaro.org>
bloblist_find function only returns the pointer of blob data,
which is fine for those self-describing data like FDT.
But as a common scenario, an interface is needed to retrieve both
the pointer and the size of the blob data.
Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
common/bloblist.c | 17 +++++++++++++++--
include/bloblist.h | 18 ++++++++++++++++++
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c
index e8acfc74331..ecce3da9331 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -223,14 +223,27 @@ static int bloblist_ensurerec(uint tag, struct bloblist_rec **recp, int size,
}
void *bloblist_find(uint tag, int size)
+{
+ void *blob = NULL;
+ int blob_size;
+
+ blob = bloblist_get_blob(tag, &blob_size);
+
+ if (size && size != blob_size)
+ return NULL;
+
+ return blob;
+}
+
+void *bloblist_get_blob(uint tag, int *size)
{
struct bloblist_rec *rec;
rec = bloblist_findrec(tag);
if (!rec)
return NULL;
- if (size && size != rec->size)
- return NULL;
+
+ *size = rec->size;
return (void *)rec + rec_hdr_size(rec);
}
diff --git a/include/bloblist.h b/include/bloblist.h
index 1e1ca34aa92..7d462e71914 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -251,6 +251,24 @@ static inline void *bloblist_check_magic(ulong addr)
return ptr;
}
+#if CONFIG_IS_ENABLED(BLOBLIST)
+/**
+ * bloblist_get_blob() - Find a blob and get the size of it
+ *
+ * Searches the bloblist and returns the blob with the matching tag
+ *
+ * @tag: Tag to search for (enum bloblist_tag_t)
+ * @size: Size of the blob found
+ * Return: pointer to bloblist if found, or NULL if not found
+ */
+void *bloblist_get_blob(uint tag, int *size);
+#else
+static inline void *bloblist_get_blob(uint tag, int *size)
+{
+ return NULL;
+}
+#endif
+
/**
* bloblist_find() - Find a blob
*
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 02/33] bloblist: Drop BSD license
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
2025-01-06 14:31 ` [PATCH v2 01/33] bloblist: add api to get blob with size Simon Glass
@ 2025-01-06 14:31 ` Simon Glass
2025-01-07 10:36 ` Ilias Apalodimas
2025-01-06 14:31 ` [PATCH v2 03/33] bloblist: Make BLOBLIST_ALLOC the default Simon Glass
` (32 subsequent siblings)
34 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:31 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Ilias Apalodimas, Levi Yun, Patrick Rudolph,
Raymond Mao, Tom Rini
Linaro has decided to write their own version of bloblist from scratch,
so there is no point in having a non-GPL license on this file. Change
it, to better fit with the U-Boot project.
Signed-off-by: Simon Glass <sjg@chromium.org>
Link: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/22215
---
(no changes since v1)
common/bloblist.c | 2 +-
include/bloblist.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c
index ecce3da9331..c441498ec56 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0+ BSD-3-Clause
+// SPDX-License-Identifier: GPL-2.0+
/*
* Copyright 2018 Google, Inc
* Written by Simon Glass <sjg@chromium.org>
diff --git a/include/bloblist.h b/include/bloblist.h
index 7d462e71914..be7303e1504 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0+ BSD-3-Clause */
+/* SPDX-License-Identifier: GPL-2.0+ */
/*
* This provides a standard way of passing information between boot phases
* (TPL -> SPL -> U-Boot proper.)
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 02/33] bloblist: Drop BSD license
2025-01-06 14:31 ` [PATCH v2 02/33] bloblist: Drop BSD license Simon Glass
@ 2025-01-07 10:36 ` Ilias Apalodimas
2025-01-07 12:04 ` Simon Glass
0 siblings, 1 reply; 60+ messages in thread
From: Ilias Apalodimas @ 2025-01-07 10:36 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Levi Yun, Patrick Rudolph, Raymond Mao,
Tom Rini
On Mon, 6 Jan 2025 at 16:32, Simon Glass <sjg@chromium.org> wrote:
>
> Linaro has decided to write their own version of bloblist from scratch,
> so there is no point in having a non-GPL license on this file. Change
> it, to better fit with the U-Boot project.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Link: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/22215
NAK on the commit message. I explained why it was wrong and
inappropriate in the previous version
/Ilias
>
> ---
>
> (no changes since v1)
>
> common/bloblist.c | 2 +-
> include/bloblist.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/common/bloblist.c b/common/bloblist.c
> index ecce3da9331..c441498ec56 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -1,4 +1,4 @@
> -// SPDX-License-Identifier: GPL-2.0+ BSD-3-Clause
> +// SPDX-License-Identifier: GPL-2.0+
> /*
> * Copyright 2018 Google, Inc
> * Written by Simon Glass <sjg@chromium.org>
> diff --git a/include/bloblist.h b/include/bloblist.h
> index 7d462e71914..be7303e1504 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0+ BSD-3-Clause */
> +/* SPDX-License-Identifier: GPL-2.0+ */
> /*
> * This provides a standard way of passing information between boot phases
> * (TPL -> SPL -> U-Boot proper.)
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 02/33] bloblist: Drop BSD license
2025-01-07 10:36 ` Ilias Apalodimas
@ 2025-01-07 12:04 ` Simon Glass
0 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-07 12:04 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: U-Boot Mailing List, Levi Yun, Patrick Rudolph, Raymond Mao,
Tom Rini
Hi Ilias,
On Tue, 7 Jan 2025 at 03:37, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Mon, 6 Jan 2025 at 16:32, Simon Glass <sjg@chromium.org> wrote:
> >
> > Linaro has decided to write their own version of bloblist from scratch,
> > so there is no point in having a non-GPL license on this file. Change
> > it, to better fit with the U-Boot project.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > Link: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/22215
>
> NAK on the commit message. I explained why it was wrong and
> inappropriate in the previous version
I didn't mean to resend this patch. Let's keep the discussion to the
original patch.
Regards,
Simon
>
> /Ilias
> >
> > ---
> >
> > (no changes since v1)
> >
> > common/bloblist.c | 2 +-
> > include/bloblist.h | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index ecce3da9331..c441498ec56 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -1,4 +1,4 @@
> > -// SPDX-License-Identifier: GPL-2.0+ BSD-3-Clause
> > +// SPDX-License-Identifier: GPL-2.0+
> > /*
> > * Copyright 2018 Google, Inc
> > * Written by Simon Glass <sjg@chromium.org>
> > diff --git a/include/bloblist.h b/include/bloblist.h
> > index 7d462e71914..be7303e1504 100644
> > --- a/include/bloblist.h
> > +++ b/include/bloblist.h
> > @@ -1,4 +1,4 @@
> > -/* SPDX-License-Identifier: GPL-2.0+ BSD-3-Clause */
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > /*
> > * This provides a standard way of passing information between boot phases
> > * (TPL -> SPL -> U-Boot proper.)
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 03/33] bloblist: Make BLOBLIST_ALLOC the default
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
2025-01-06 14:31 ` [PATCH v2 01/33] bloblist: add api to get blob with size Simon Glass
2025-01-06 14:31 ` [PATCH v2 02/33] bloblist: Drop BSD license Simon Glass
@ 2025-01-06 14:31 ` Simon Glass
2025-01-06 21:41 ` Tom Rini
2025-01-06 14:32 ` [PATCH v2 04/33] abuf: Provide a way to get the buffer address Simon Glass
` (31 subsequent siblings)
34 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:31 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Alex Shumsky, Alper Nebi Yasak, Bin Meng,
Chen-Yu Tsai, Dragan Simic, Ilias Apalodimas, Jagan Teki,
Jerome Forissier, Jiaxun Yang, Jonas Karlman, Joshua Watt,
Kever Yang, Mattijs Korpershoek, Patrick Rudolph, Quentin Schulz,
Stefan Roese, Tom Rini
We want to encourage people to use an allocated bloblist since it is
more flexible than a fixed one. Make this the default, being sure not to
change existing users.
The unit tests require BLOBLIST_FIXED so add a dependency in the
Makefile to avoid build errors.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
common/Kconfig | 15 ++++++++-------
configs/chromebook_bob_defconfig | 1 +
configs/chromebook_coral_defconfig | 1 +
configs/chromebook_kevin_defconfig | 1 +
configs/chromebook_samus_tpl_defconfig | 1 +
configs/qemu-x86_64_defconfig | 1 +
test/common/Makefile | 4 ++++
7 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig
index e8d89bf6eb9..a00387a4609 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -1039,16 +1039,10 @@ if BLOBLIST
choice
prompt "Bloblist location"
+ default BLOBLIST_FIXED if SANDBOX
help
Select the location of the bloblist, via various means.
-config BLOBLIST_FIXED
- bool "Place bloblist at a fixed address in memory"
- help
- Select this to used a fixed memory address for the bloblist. If the
- bloblist exists at this address from a previous phase, it used as is.
- If not it is created at this address in U-Boot.
-
config BLOBLIST_ALLOC
bool "Allocate bloblist"
help
@@ -1056,6 +1050,13 @@ config BLOBLIST_ALLOC
specify a fixed address on systems where this is unknown or can
change at runtime.
+config BLOBLIST_FIXED
+ bool "Place bloblist at a fixed address in memory"
+ help
+ Select this to used a fixed memory address for the bloblist. If the
+ bloblist exists at this address from a previous phase, it used as is.
+ If not it is created at this address in U-Boot.
+
endchoice
config BLOBLIST_ADDR
diff --git a/configs/chromebook_bob_defconfig b/configs/chromebook_bob_defconfig
index decac2e1935..072311465d9 100644
--- a/configs/chromebook_bob_defconfig
+++ b/configs/chromebook_bob_defconfig
@@ -36,6 +36,7 @@ CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-gru-bob.dtb"
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_BOARD_EARLY_INIT_R=y
CONFIG_BLOBLIST=y
+CONFIG_BLOBLIST_FIXED=y
CONFIG_BLOBLIST_ADDR=0x100000
CONFIG_BLOBLIST_SIZE=0x1000
CONFIG_SPL_MAX_SIZE=0x1e000
diff --git a/configs/chromebook_coral_defconfig b/configs/chromebook_coral_defconfig
index 0fb73049738..b1999b4ef20 100644
--- a/configs/chromebook_coral_defconfig
+++ b/configs/chromebook_coral_defconfig
@@ -44,6 +44,7 @@ CONFIG_LOGF_FUNC=y
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_BLOBLIST=y
# CONFIG_TPL_BLOBLIST is not set
+CONFIG_BLOBLIST_FIXED=y
CONFIG_BLOBLIST_ADDR=0x100000
CONFIG_BLOBLIST_SIZE=0x30000
CONFIG_SPL_NO_BSS_LIMIT=y
diff --git a/configs/chromebook_kevin_defconfig b/configs/chromebook_kevin_defconfig
index 5bbea6c42a8..13c0998dbab 100644
--- a/configs/chromebook_kevin_defconfig
+++ b/configs/chromebook_kevin_defconfig
@@ -37,6 +37,7 @@ CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-gru-kevin.dtb"
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_BOARD_EARLY_INIT_R=y
CONFIG_BLOBLIST=y
+CONFIG_BLOBLIST_FIXED=y
CONFIG_BLOBLIST_ADDR=0x100000
CONFIG_BLOBLIST_SIZE=0x1000
CONFIG_SPL_MAX_SIZE=0x1e000
diff --git a/configs/chromebook_samus_tpl_defconfig b/configs/chromebook_samus_tpl_defconfig
index fc524da5480..42337d7a11e 100644
--- a/configs/chromebook_samus_tpl_defconfig
+++ b/configs/chromebook_samus_tpl_defconfig
@@ -34,6 +34,7 @@ CONFIG_SYS_CONSOLE_INFO_QUIET=y
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_MISC_INIT_R=y
CONFIG_BLOBLIST=y
+CONFIG_BLOBLIST_FIXED=y
CONFIG_BLOBLIST_ADDR=0xff7c0000
CONFIG_BLOBLIST_SIZE=0x1000
CONFIG_SPL_NO_BSS_LIMIT=y
diff --git a/configs/qemu-x86_64_defconfig b/configs/qemu-x86_64_defconfig
index 812b20687e5..f93721fceb8 100644
--- a/configs/qemu-x86_64_defconfig
+++ b/configs/qemu-x86_64_defconfig
@@ -34,6 +34,7 @@ CONFIG_SPL_LOG=y
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_PCI_INIT_R=y
CONFIG_BLOBLIST=y
+CONFIG_BLOBLIST_FIXED=y
CONFIG_BLOBLIST_ADDR=0x10000
CONFIG_SPL_NO_BSS_LIMIT=y
CONFIG_SPL_BOARD_INIT=y
diff --git a/test/common/Makefile b/test/common/Makefile
index 53c4f16164d..95bd00741a3 100644
--- a/test/common/Makefile
+++ b/test/common/Makefile
@@ -1,9 +1,13 @@
# SPDX-License-Identifier: GPL-2.0+
obj-y += cmd_ut_common.o
obj-$(CONFIG_AUTOBOOT) += test_autoboot.o
+
ifneq ($(CONFIG_$(XPL_)BLOBLIST),)
+ifdef CONFIG_BLOBLIST_FIXED
obj-$(CONFIG_$(XPL_)CMDLINE) += bloblist.o
endif
+endif
+
obj-$(CONFIG_CYCLIC) += cyclic.o
obj-$(CONFIG_EVENT_DYNAMIC) += event.o
obj-y += cread.o
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 03/33] bloblist: Make BLOBLIST_ALLOC the default
2025-01-06 14:31 ` [PATCH v2 03/33] bloblist: Make BLOBLIST_ALLOC the default Simon Glass
@ 2025-01-06 21:41 ` Tom Rini
2025-01-09 15:01 ` Simon Glass
0 siblings, 1 reply; 60+ messages in thread
From: Tom Rini @ 2025-01-06 21:41 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Alex Shumsky, Alper Nebi Yasak, Bin Meng,
Chen-Yu Tsai, Dragan Simic, Ilias Apalodimas, Jagan Teki,
Jerome Forissier, Jiaxun Yang, Jonas Karlman, Joshua Watt,
Kever Yang, Mattijs Korpershoek, Patrick Rudolph, Quentin Schulz,
Stefan Roese
[-- Attachment #1: Type: text/plain, Size: 909 bytes --]
On Mon, Jan 06, 2025 at 07:31:59AM -0700, Simon Glass wrote:
> We want to encourage people to use an allocated bloblist since it is
> more flexible than a fixed one. Make this the default, being sure not to
> change existing users.
>
> The unit tests require BLOBLIST_FIXED so add a dependency in the
> Makefile to avoid build errors.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
[snip]
> diff --git a/common/Kconfig b/common/Kconfig
> index e8d89bf6eb9..a00387a4609 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -1039,16 +1039,10 @@ if BLOBLIST
>
> choice
> prompt "Bloblist location"
> + default BLOBLIST_FIXED if SANDBOX
> help
> Select the location of the bloblist, via various means.
This should just be "default BLOBLIST_ALLOC" and sandbox platforms that
are using the bloblist can be like the other platforms using a fixed
address.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 03/33] bloblist: Make BLOBLIST_ALLOC the default
2025-01-06 21:41 ` Tom Rini
@ 2025-01-09 15:01 ` Simon Glass
2025-01-09 16:42 ` Tom Rini
0 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-09 15:01 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Alex Shumsky, Alper Nebi Yasak, Bin Meng,
Chen-Yu Tsai, Dragan Simic, Ilias Apalodimas, Jagan Teki,
Jerome Forissier, Jiaxun Yang, Jonas Karlman, Joshua Watt,
Kever Yang, Mattijs Korpershoek, Patrick Rudolph, Quentin Schulz,
Stefan Roese
Hi Tom,
On Mon, 6 Jan 2025 at 14:41, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Jan 06, 2025 at 07:31:59AM -0700, Simon Glass wrote:
>
> > We want to encourage people to use an allocated bloblist since it is
> > more flexible than a fixed one. Make this the default, being sure not to
> > change existing users.
> >
> > The unit tests require BLOBLIST_FIXED so add a dependency in the
> > Makefile to avoid build errors.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> [snip]
> > diff --git a/common/Kconfig b/common/Kconfig
> > index e8d89bf6eb9..a00387a4609 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -1039,16 +1039,10 @@ if BLOBLIST
> >
> > choice
> > prompt "Bloblist location"
> > + default BLOBLIST_FIXED if SANDBOX
> > help
> > Select the location of the bloblist, via various means.
>
> This should just be "default BLOBLIST_ALLOC" and sandbox platforms that
> are using the bloblist can be like the other platforms using a fixed
> address.
We don't need 'default BLOBLIST_ALLOC' as it is now first, so is the
default. I would like the 'fixed' thing to be later on so that people
know we want to stop using it.
All the sandbox platforms use BLOBLIST_FIXED. I'm not sure what change
you are wanting me to make to this patch?
Regards,
Simon
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 03/33] bloblist: Make BLOBLIST_ALLOC the default
2025-01-09 15:01 ` Simon Glass
@ 2025-01-09 16:42 ` Tom Rini
2025-01-10 13:38 ` Simon Glass
0 siblings, 1 reply; 60+ messages in thread
From: Tom Rini @ 2025-01-09 16:42 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Alex Shumsky, Alper Nebi Yasak, Bin Meng,
Chen-Yu Tsai, Dragan Simic, Ilias Apalodimas, Jagan Teki,
Jerome Forissier, Jiaxun Yang, Jonas Karlman, Joshua Watt,
Kever Yang, Mattijs Korpershoek, Patrick Rudolph, Quentin Schulz,
Stefan Roese
[-- Attachment #1: Type: text/plain, Size: 1606 bytes --]
On Thu, Jan 09, 2025 at 08:01:17AM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Mon, 6 Jan 2025 at 14:41, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Jan 06, 2025 at 07:31:59AM -0700, Simon Glass wrote:
> >
> > > We want to encourage people to use an allocated bloblist since it is
> > > more flexible than a fixed one. Make this the default, being sure not to
> > > change existing users.
> > >
> > > The unit tests require BLOBLIST_FIXED so add a dependency in the
> > > Makefile to avoid build errors.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > [snip]
> > > diff --git a/common/Kconfig b/common/Kconfig
> > > index e8d89bf6eb9..a00387a4609 100644
> > > --- a/common/Kconfig
> > > +++ b/common/Kconfig
> > > @@ -1039,16 +1039,10 @@ if BLOBLIST
> > >
> > > choice
> > > prompt "Bloblist location"
> > > + default BLOBLIST_FIXED if SANDBOX
> > > help
> > > Select the location of the bloblist, via various means.
> >
> > This should just be "default BLOBLIST_ALLOC" and sandbox platforms that
> > are using the bloblist can be like the other platforms using a fixed
> > address.
>
> We don't need 'default BLOBLIST_ALLOC' as it is now first, so is the
> default. I would like the 'fixed' thing to be later on so that people
> know we want to stop using it.
>
> All the sandbox platforms use BLOBLIST_FIXED. I'm not sure what change
> you are wanting me to make to this patch?
I'm wanting you to not re-order the list to change the default and
instead use the default keyword to change the default.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 03/33] bloblist: Make BLOBLIST_ALLOC the default
2025-01-09 16:42 ` Tom Rini
@ 2025-01-10 13:38 ` Simon Glass
0 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-10 13:38 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Alex Shumsky, Alper Nebi Yasak, Bin Meng,
Chen-Yu Tsai, Dragan Simic, Ilias Apalodimas, Jagan Teki,
Jerome Forissier, Jiaxun Yang, Jonas Karlman, Joshua Watt,
Kever Yang, Mattijs Korpershoek, Patrick Rudolph, Quentin Schulz,
Stefan Roese
Hi Tom,
On Thu, 9 Jan 2025 at 09:42, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jan 09, 2025 at 08:01:17AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 6 Jan 2025 at 14:41, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Jan 06, 2025 at 07:31:59AM -0700, Simon Glass wrote:
> > >
> > > > We want to encourage people to use an allocated bloblist since it is
> > > > more flexible than a fixed one. Make this the default, being sure not to
> > > > change existing users.
> > > >
> > > > The unit tests require BLOBLIST_FIXED so add a dependency in the
> > > > Makefile to avoid build errors.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > [snip]
> > > > diff --git a/common/Kconfig b/common/Kconfig
> > > > index e8d89bf6eb9..a00387a4609 100644
> > > > --- a/common/Kconfig
> > > > +++ b/common/Kconfig
> > > > @@ -1039,16 +1039,10 @@ if BLOBLIST
> > > >
> > > > choice
> > > > prompt "Bloblist location"
> > > > + default BLOBLIST_FIXED if SANDBOX
> > > > help
> > > > Select the location of the bloblist, via various means.
> > >
> > > This should just be "default BLOBLIST_ALLOC" and sandbox platforms that
> > > are using the bloblist can be like the other platforms using a fixed
> > > address.
> >
> > We don't need 'default BLOBLIST_ALLOC' as it is now first, so is the
> > default. I would like the 'fixed' thing to be later on so that people
> > know we want to stop using it.
> >
> > All the sandbox platforms use BLOBLIST_FIXED. I'm not sure what change
> > you are wanting me to make to this patch?
>
> I'm wanting you to not re-order the list to change the default and
> instead use the default keyword to change the default.
Oh, OK I see.
Regards,
Simon
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 04/33] abuf: Provide a way to get the buffer address
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (2 preceding siblings ...)
2025-01-06 14:31 ` [PATCH v2 03/33] bloblist: Make BLOBLIST_ALLOC the default Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 05/33] abuf: Allow use in host tools Simon Glass
` (30 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Tom Rini
In many cases it is useful to get the address of a buffer, e.g. when
booting from it. Add a function to handle this.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
include/abuf.h | 8 ++++++++
lib/abuf.c | 6 ++++++
test/lib/abuf.c | 4 +++-
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/include/abuf.h b/include/abuf.h
index be98ec78c86..76e314b9a47 100644
--- a/include/abuf.h
+++ b/include/abuf.h
@@ -42,6 +42,14 @@ static inline size_t abuf_size(const struct abuf *abuf)
return abuf->size;
}
+/**
+ * abuf_addr() - Get the address of a buffer's data
+ *
+ * @abuf: Buffer to check
+ * Return: address of buffer
+ */
+ulong abuf_addr(const struct abuf *abuf);
+
/**
* abuf_set() - set the (unallocated) data in a buffer
*
diff --git a/lib/abuf.c b/lib/abuf.c
index 937c3df351e..8156177c773 100644
--- a/lib/abuf.c
+++ b/lib/abuf.c
@@ -26,6 +26,12 @@ void abuf_map_sysmem(struct abuf *abuf, ulong addr, size_t size)
{
abuf_set(abuf, map_sysmem(addr, size), size);
}
+
+ulong abuf_addr(const struct abuf *abuf)
+{
+ return map_to_sysmem(abuf->data);
+}
+
#else
/* copied from lib/string.c for convenience */
static char *memdup(const void *src, size_t len)
diff --git a/test/lib/abuf.c b/test/lib/abuf.c
index 7c0481ab610..5d61f9261c6 100644
--- a/test/lib/abuf.c
+++ b/test/lib/abuf.c
@@ -46,7 +46,7 @@ static int lib_test_abuf_set(struct unit_test_state *uts)
}
LIB_TEST(lib_test_abuf_set, 0);
-/* Test abuf_map_sysmem() */
+/* Test abuf_map_sysmem() and abuf_addr() */
static int lib_test_abuf_map_sysmem(struct unit_test_state *uts)
{
struct abuf buf;
@@ -60,6 +60,8 @@ static int lib_test_abuf_map_sysmem(struct unit_test_state *uts)
ut_asserteq(TEST_DATA_LEN, buf.size);
ut_asserteq(false, buf.alloced);
+ ut_asserteq(addr, abuf_addr(&buf));
+
return 0;
}
LIB_TEST(lib_test_abuf_map_sysmem, 0);
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 05/33] abuf: Allow use in host tools
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (3 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 04/33] abuf: Provide a way to get the buffer address Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 06/33] abuf: Provide a constant buffer Simon Glass
` (29 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Tom Rini
Some header files included on the host are moving to use abuf, so adjust
the header-inclusion to bring in size_t correctly.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
include/abuf.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/abuf.h b/include/abuf.h
index 76e314b9a47..de21cefade4 100644
--- a/include/abuf.h
+++ b/include/abuf.h
@@ -9,7 +9,11 @@
#ifndef __ABUF_H
#define __ABUF_H
+#ifdef USE_HOSTCC
+#include <sys/types.h>
+#else
#include <linux/types.h>
+#endif
/**
* struct abuf - buffer that can be allocated and freed
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 06/33] abuf: Provide a constant buffer
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (4 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 05/33] abuf: Allow use in host tools Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 07/33] cpu: Provide a way to get the physical-address size Simon Glass
` (28 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Tom Rini
Add a new initialiser which can accept a constant pointer.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
include/abuf.h | 13 +++++++++++++
lib/abuf.c | 6 ++++++
test/lib/abuf.c | 22 ++++++++++++++++++++++
3 files changed, 41 insertions(+)
diff --git a/include/abuf.h b/include/abuf.h
index de21cefade4..62ff6499a0c 100644
--- a/include/abuf.h
+++ b/include/abuf.h
@@ -157,6 +157,19 @@ void abuf_init_move(struct abuf *abuf, void *data, size_t size);
*/
void abuf_init_set(struct abuf *abuf, void *data, size_t size);
+/**
+ * abuf_init_const() - Set up a new const abuf
+ *
+ * Inits a new abuf and sets up its (unallocated) data. The only current
+ * difference between this and abuf_init_set() is the 'data' parameter is a
+ * const pointer. At some point a flag could be used to indicate const-ness.
+ *
+ * @abuf: abuf to set up
+ * @data: New contents of abuf
+ * @size: New size of abuf
+ */
+void abuf_init_const(struct abuf *abuf, const void *data, size_t size);
+
/**
* abuf_uninit() - Free any memory used by an abuf
*
diff --git a/lib/abuf.c b/lib/abuf.c
index 8156177c773..61adf7fc6b1 100644
--- a/lib/abuf.c
+++ b/lib/abuf.c
@@ -119,6 +119,12 @@ void abuf_init_set(struct abuf *abuf, void *data, size_t size)
abuf_set(abuf, data, size);
}
+void abuf_init_const(struct abuf *abuf, const void *data, size_t size)
+{
+ /* for now there is no flag indicating that the abuf data is constant */
+ abuf_init_set(abuf, (void *)data, size);
+}
+
void abuf_init_move(struct abuf *abuf, void *data, size_t size)
{
abuf_init_set(abuf, data, size);
diff --git a/test/lib/abuf.c b/test/lib/abuf.c
index 5d61f9261c6..b38690fe1a9 100644
--- a/test/lib/abuf.c
+++ b/test/lib/abuf.c
@@ -46,6 +46,28 @@ static int lib_test_abuf_set(struct unit_test_state *uts)
}
LIB_TEST(lib_test_abuf_set, 0);
+/* Test abuf_init_const() */
+static int lib_test_abuf_init_const(struct unit_test_state *uts)
+{
+ struct abuf buf;
+ ulong start;
+ void *ptr;
+
+ start = ut_check_free();
+
+ ptr = map_sysmem(0x100, 0);
+
+ abuf_init_const(&buf, ptr, 10);
+ ut_asserteq_ptr(ptr, buf.data);
+ ut_asserteq(10, buf.size);
+
+ /* No memory should have been allocated */
+ ut_assertok(ut_check_delta(start));
+
+ return 0;
+}
+LIB_TEST(lib_test_abuf_init_const, 0);
+
/* Test abuf_map_sysmem() and abuf_addr() */
static int lib_test_abuf_map_sysmem(struct unit_test_state *uts)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 07/33] cpu: Provide a way to get the physical-address size
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (5 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 06/33] abuf: Provide a constant buffer Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 08/33] serial: Support info() method in ns16550 xPL with UPL Simon Glass
` (27 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Andy Shevchenko, Bin Meng, Hou Zhiqiang,
Philip Oberfichtner, Tom Rini
This concept exists on x86. Declare it as a generic function so that the
value can be accessed by UPL.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
arch/x86/include/asm/cpu.h | 9 ---------
arch/x86/lib/bdinfo.c | 1 +
include/cpu.h | 14 ++++++++++++++
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 8c1ef4c8cc1..fd389d4024c 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -284,15 +284,6 @@ u32 cpu_get_family_model(void);
*/
u32 cpu_get_stepping(void);
-/**
- * cpu_phys_address_size() - Get the physical address size in bits
- *
- * This is 32 for older CPUs but newer ones may support 36.
- *
- * Return: address size (typically 32 or 36)
- */
-int cpu_phys_address_size(void);
-
void board_final_init(void);
void board_final_cleanup(void);
diff --git a/arch/x86/lib/bdinfo.c b/arch/x86/lib/bdinfo.c
index 2a78f578dee..bd2cf0b9fcb 100644
--- a/arch/x86/lib/bdinfo.c
+++ b/arch/x86/lib/bdinfo.c
@@ -5,6 +5,7 @@
* Copyright 2021 Google LLC
*/
+#include <cpu.h>
#include <efi.h>
#include <init.h>
#include <asm/cpu.h>
diff --git a/include/cpu.h b/include/cpu.h
index 0018910d61f..d0cd104c05a 100644
--- a/include/cpu.h
+++ b/include/cpu.h
@@ -179,4 +179,18 @@ struct udevice *cpu_get_current_dev(void);
* @return 0 if OK, -ve on error
*/
int cpu_release_core(const struct udevice *dev, phys_addr_t addr);
+
+/**
+ * cpu_phys_address_size() - Get the physical-address size for the CPU
+ *
+ * x86 CPUs have a setting which indicates how many bits of address space are
+ * available on the CPU. This is 32 for older CPUs but newer ones may support 36
+ * or more.
+ *
+ * For non-x86 CPUs the result may simply be 32 for 32-bit CPUS or 64 for 64-bit
+ *
+ * Return: address size (typically 32 or 36)
+ */
+int cpu_phys_address_size(void);
+
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 08/33] serial: Support info() method in ns16550 xPL with UPL
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (6 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 07/33] cpu: Provide a way to get the physical-address size Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 09/33] mkimage: Update map_to_sysmem() to match its prototype Simon Glass
` (26 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Jonas Karlman, Quentin Schulz, Rasmus Villemoes,
Stefan Roese, Tom Rini
UPL needs to pass the serial details onto the next stage, so adjust the
condition to support this.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
drivers/serial/ns16550.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 3f6860f3916..5126cf8bc1a 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -473,7 +473,7 @@ static int ns16550_serial_getinfo(struct udevice *dev,
struct ns16550_plat *plat = com_port->plat;
/* save code size */
- if (!not_xpl())
+ if (!not_xpl() && !CONFIG_IS_ENABLED(UPL_OUT))
return -ENOSYS;
info->type = SERIAL_CHIP_16550_COMPATIBLE;
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 09/33] mkimage: Update map_to_sysmem() to match its prototype
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (7 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 08/33] serial: Support info() method in ns16550 xPL with UPL Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 10/33] x86: Enable meminfo command Simon Glass
` (25 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Joe Hershberger, Marek Vasut, Stefano Babic,
Tom Rini
Update the version of this function in mkimage so that it uses a const
pointer, as is done in the mapmem.h header file.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
tools/mkimage.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/mkimage.h b/tools/mkimage.h
index d92a3ff8117..15741f250fd 100644
--- a/tools/mkimage.h
+++ b/tools/mkimage.h
@@ -37,7 +37,7 @@ static inline void *map_sysmem(ulong paddr, unsigned long len)
return (void *)(uintptr_t)paddr;
}
-static inline ulong map_to_sysmem(void *ptr)
+static inline ulong map_to_sysmem(const void *ptr)
{
return (ulong)(uintptr_t)ptr;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 10/33] x86: Enable meminfo command
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (8 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 09/33] mkimage: Update map_to_sysmem() to match its prototype Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 11/33] x86: Show the timestamp counter with bdinfo Simon Glass
` (24 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Bin Meng, Dmitry Rokosov, Heinrich Schuchardt,
Ilias Apalodimas, Jerome Forissier, Matthew Garrett,
Mattijs Korpershoek, Tom Rini
Enable this command for x86 boards as it is quite useful for seeing
where memory is.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Fix 'Emable' typo
- Enable the command in cmd/Kconfig instead
cmd/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 8fc76778d87..64c6e980a09 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -905,14 +905,14 @@ config MD5SUM_VERIFY
config CMD_MEMINFO
bool "meminfo"
- default y if SANDBOX
+ default y if SANDBOX || X86
help
Display memory information.
config CMD_MEMINFO_MAP
bool "- with memory map"
depends on CMD_MEMINFO
- default y if SANDBOX
+ default y if SANDBOX || X86
help
Shows a memory map, in addition to just the DRAM size. This allows
seeing where U-Boot's memory area is, at the top of DRAM, as well as
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 11/33] x86: Show the timestamp counter with bdinfo
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (9 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 10/33] x86: Enable meminfo command Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 12/33] ofnode: Use 4K for a default tree-size Simon Glass
` (23 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Bin Meng, Tom Rini
Add a line to the 'bdinfo' command which shows the current value of the
TSC.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
arch/x86/lib/bdinfo.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/lib/bdinfo.c b/arch/x86/lib/bdinfo.c
index bd2cf0b9fcb..4b016d4a0fc 100644
--- a/arch/x86/lib/bdinfo.c
+++ b/arch/x86/lib/bdinfo.c
@@ -33,6 +33,8 @@ void arch_print_bdinfo(void)
bdinfo_print_num_l(" high start", gd->arch.table_start_high);
bdinfo_print_num_l(" high end", gd->arch.table_end_high);
+ bdinfo_print_num_ll("tsc", rdtsc());
+
if (IS_ENABLED(CONFIG_EFI_STUB))
efi_show_bdinfo();
}
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 12/33] ofnode: Use 4K for a default tree-size
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (10 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 11/33] x86: Show the timestamp counter with bdinfo Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-08 18:33 ` Tom Rini
2025-01-06 14:32 ` [PATCH v2 13/33] ofnode: Indicate when out of space in a few places Simon Glass
` (22 subsequent siblings)
34 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Christian Marangi, Jonas Karlman, Quentin Schulz,
Tom Rini
At some point it would be nice to have the ofnode API automatically
expand the tree as required, to accommodate new nodes. For now, expand
the default size so that UPL can be supported.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
drivers/core/ofnode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 950895e72a9..ec03f6f550d 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -118,7 +118,7 @@ int oftree_new(oftree *treep)
return log_msg_ret("liv", ret);
tree = oftree_from_np(root);
} else {
- const int size = 1024;
+ const int size = 4096;
void *fdt;
ret = check_tree_count();
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 12/33] ofnode: Use 4K for a default tree-size
2025-01-06 14:32 ` [PATCH v2 12/33] ofnode: Use 4K for a default tree-size Simon Glass
@ 2025-01-08 18:33 ` Tom Rini
2025-01-09 12:37 ` Simon Glass
0 siblings, 1 reply; 60+ messages in thread
From: Tom Rini @ 2025-01-08 18:33 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Christian Marangi, Jonas Karlman,
Quentin Schulz
[-- Attachment #1: Type: text/plain, Size: 497 bytes --]
On Mon, Jan 06, 2025 at 07:32:08AM -0700, Simon Glass wrote:
> At some point it would be nice to have the ofnode API automatically
> expand the tree as required, to accommodate new nodes. For now, expand
> the default size so that UPL can be supported.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
> drivers/core/ofnode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Why not have oftree_new take a size parameter?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 12/33] ofnode: Use 4K for a default tree-size
2025-01-08 18:33 ` Tom Rini
@ 2025-01-09 12:37 ` Simon Glass
0 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-09 12:37 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Christian Marangi, Jonas Karlman,
Quentin Schulz
Hi Tom,
On Wed, 8 Jan 2025 at 11:33, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Jan 06, 2025 at 07:32:08AM -0700, Simon Glass wrote:
>
> > At some point it would be nice to have the ofnode API automatically
> > expand the tree as required, to accommodate new nodes. For now, expand
> > the default size so that UPL can be supported.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> > drivers/core/ofnode.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Why not have oftree_new take a size parameter?
Well, the real solution is as in the commit message. This commit is
just kicking it down the road, but we need to make progress on
livetree, as the logic to automatically expand a flattree could be a
bit painful. I'm not sure if someone other than me is interested in
working on it.
I just felt that 1K was a bit small to be useful.
Regards,
Simon
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 13/33] ofnode: Indicate when out of space in a few places
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (11 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 12/33] ofnode: Use 4K for a default tree-size Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-08 18:25 ` Tom Rini
2025-01-06 14:32 ` [PATCH v2 14/33] ofnode: Update of_add_subnode() to indicate name is alloced Simon Glass
` (21 subsequent siblings)
34 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Christian Marangi, Jonas Karlman, Quentin Schulz,
Tom Rini
Update ofnode_add_subnode() and ofnode_add_prop() to return a suitable
error when space is exhausted in the FDT. This makes it easier to see
what is going wrong.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
drivers/core/ofnode.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index ec03f6f550d..5b8be218d3b 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -1607,9 +1607,10 @@ ofnode ofnode_by_prop_value(ofnode from, const char *propname,
int ofnode_write_prop(ofnode node, const char *propname, const void *value,
int len, bool copy)
{
+ int ret;
+
if (of_live_active()) {
void *newval;
- int ret;
if (copy) {
newval = malloc(len);
@@ -1623,8 +1624,12 @@ int ofnode_write_prop(ofnode node, const char *propname, const void *value,
free(newval);
return ret;
} else {
- return fdt_setprop(ofnode_to_fdt(node), ofnode_to_offset(node),
- propname, value, len);
+ ret = fdt_setprop(ofnode_to_fdt(node), ofnode_to_offset(node),
+ propname, value, len);
+ if (ret)
+ return ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EINVAL;
+
+ return 0;
}
}
@@ -1897,7 +1902,7 @@ int ofnode_add_subnode(ofnode node, const char *name, ofnode *subnodep)
ret = -EEXIST;
}
if (offset < 0)
- return -EINVAL;
+ return offset == -FDT_ERR_NOSPACE ? -ENOSPC : -EINVAL;
subnode = noffset_to_ofnode(node, offset);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 14/33] ofnode: Update of_add_subnode() to indicate name is alloced
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (12 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 13/33] ofnode: Indicate when out of space in a few places Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 15/33] boot: Rename fit_image_get_data() Simon Glass
` (20 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Christian Marangi, Tom Rini
This function allocates memory for the node name, so mention this in the
function comment.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
include/dm/ofnode.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 0787758926f..024ae4cd289 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -1702,7 +1702,7 @@ static inline int ofnode_read_bootscript_flash(u64 *bootscr_flash_offset,
* of_add_subnode() - add a new subnode to a node
*
* @parent: parent node to add to
- * @name: name of subnode
+ * @name: name of subnode (allocated by this function)
* @nodep: returns pointer to new subnode (valid if the function returns 0
* or -EEXIST)
* Returns 0 if OK, -EEXIST if already exists, -ENOMEM if out of memory, other
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 15/33] boot: Rename fit_image_get_data()
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (13 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 14/33] ofnode: Update of_add_subnode() to indicate name is alloced Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 16/33] boot: Rename fit_image_get_data_and_size() Simon Glass
` (19 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Heinrich Schuchardt, Alexander Dahl,
Alexander Kochetkov, Alexander Sverdlin, Bin Meng,
Devarsh Thakkar, Hugo Cornelis, Igor Opaniuk, Ilias Apalodimas,
Janne Grunau, Julien Masson, Laurent Pinchart, Leo Yu-Chi Liang,
Lukasz Majewski, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Maxim Moskalets, Nikhil M Jain,
Paul-Erwan Rio, Raymond Mao, Roman Stratiienko, Sughosh Ganu,
Tom Rini
This function can only be used with FITs that use embedded data. Rename
it so this is clear.
Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
(no changes since v1)
arch/arm/cpu/armv8/sec_firmware.c | 4 ++--
arch/arm/mach-k3/r5/sysfw-loader.c | 2 +-
arch/x86/lib/bootm.c | 6 +++---
boot/image-fit.c | 10 +++++-----
common/spl/spl_fit.c | 2 +-
common/splash_source.c | 4 +++-
common/update.c | 2 +-
include/image.h | 4 ++--
tools/image-host.c | 4 ++--
9 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
index 44372cbe4a1..b7c73f288bd 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -128,8 +128,8 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
return -EINVAL;
}
- if (fit_image_get_data(sec_firmware_img, ld_node_off,
- &data, &size)) {
+ if (fit_image_get_emb_data(sec_firmware_img, ld_node_off,
+ &data, &size)) {
printf("SEC Loadable: Can't get subimage data/size");
return -ENOENT;
}
diff --git a/arch/arm/mach-k3/r5/sysfw-loader.c b/arch/arm/mach-k3/r5/sysfw-loader.c
index 188731e673d..c323d2f78f8 100644
--- a/arch/arm/mach-k3/r5/sysfw-loader.c
+++ b/arch/arm/mach-k3/r5/sysfw-loader.c
@@ -115,7 +115,7 @@ static int fit_get_data_by_name(const void *fit, int images, const char *name,
if (node_offset < 0)
return -ENOENT;
- return fit_image_get_data(fit, node_offset, addr, size);
+ return fit_image_get_emb_data(fit, node_offset, addr, size);
}
static void k3_start_system_controller(int rproc_id, bool rproc_loaded,
diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c
index 5fb46112ccb..3305560aa06 100644
--- a/arch/x86/lib/bootm.c
+++ b/arch/x86/lib/bootm.c
@@ -106,9 +106,9 @@ static int boot_prep_linux(struct bootm_headers *images)
is_zimage = 1;
#if defined(CONFIG_FIT)
} else if (images->fit_uname_os && is_zimage) {
- ret = fit_image_get_data(images->fit_hdr_os,
- images->fit_noffset_os,
- (const void **)&data, &len);
+ ret = fit_image_get_emb_data(images->fit_hdr_os,
+ images->fit_noffset_os,
+ (const void **)&data, &len);
if (ret) {
puts("Can't get image data/size!\n");
goto error;
diff --git a/boot/image-fit.c b/boot/image-fit.c
index 7d56f0b5e6e..4b116ba5891 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -902,13 +902,13 @@ int fit_image_get_entry(const void *fit, int noffset, ulong *entry)
}
/**
- * fit_image_get_data - get data property and its size for a given component image node
+ * fit_image_get_emb_data - get data property and its size for a given component image node
* @fit: pointer to the FIT format image header
* @noffset: component image node offset
* @data: double pointer to void, will hold data property's data address
* @size: pointer to size_t, will hold data property's data size
*
- * fit_image_get_data() finds data property in a given component image node.
+ * fit_image_get_emb_data() finds data property in a given component image node.
* If the property is found its data start address and size are returned to
* the caller.
*
@@ -916,8 +916,8 @@ int fit_image_get_entry(const void *fit, int noffset, ulong *entry)
* 0, on success
* -1, on failure
*/
-int fit_image_get_data(const void *fit, int noffset,
- const void **data, size_t *size)
+int fit_image_get_emb_data(const void *fit, int noffset, const void **data,
+ size_t *size)
{
int len;
@@ -1074,7 +1074,7 @@ int fit_image_get_data_and_size(const void *fit, int noffset,
*size = len;
}
} else {
- ret = fit_image_get_data(fit, noffset, data, size);
+ ret = fit_image_get_emb_data(fit, noffset, data, size);
}
return ret;
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 3160f573bfb..097c731d9a3 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -282,7 +282,7 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset,
src = src_ptr + overhead;
} else {
/* Embedded data */
- if (fit_image_get_data(fit, node, &data, &length)) {
+ if (fit_image_get_emb_data(fit, node, &data, &length)) {
puts("Cannot get image data/size\n");
return -ENOENT;
}
diff --git a/common/splash_source.c b/common/splash_source.c
index f43e7cc1be7..5ac32a2f995 100644
--- a/common/splash_source.c
+++ b/common/splash_source.c
@@ -396,7 +396,9 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr)
/* Extract the splash data from FIT */
/* 1. Test if splash is in FIT internal data. */
- if (!fit_image_get_data(fit_header, node_offset, &internal_splash_data, &internal_splash_size))
+ if (!fit_image_get_emb_data(fit_header, node_offset,
+ &internal_splash_data,
+ &internal_splash_size))
memmove((void *)(uintptr_t)bmp_load_addr, internal_splash_data, internal_splash_size);
/* 2. Test if splash is in FIT external data with fixed position. */
else if (!fit_image_get_data_position(fit_header, node_offset, &external_splash_addr))
diff --git a/common/update.c b/common/update.c
index 6801b49479d..d149ca18e78 100644
--- a/common/update.c
+++ b/common/update.c
@@ -200,7 +200,7 @@ static int update_fit_getparams(const void *fit, int noffset, ulong *addr,
{
const void *data;
- if (fit_image_get_data(fit, noffset, &data, (size_t *)size))
+ if (fit_image_get_emb_data(fit, noffset, &data, (size_t *)size))
return 1;
if (fit_image_get_load(fit, noffset, (ulong *)fladdr))
diff --git a/include/image.h b/include/image.h
index de44dc09d07..e5241648b5e 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1196,8 +1196,8 @@ int fit_image_get_type(const void *fit, int noffset, uint8_t *type);
int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp);
int fit_image_get_load(const void *fit, int noffset, ulong *load);
int fit_image_get_entry(const void *fit, int noffset, ulong *entry);
-int fit_image_get_data(const void *fit, int noffset,
- const void **data, size_t *size);
+int fit_image_get_emb_data(const void *fit, int noffset, const void **data,
+ size_t *size);
int fit_image_get_data_offset(const void *fit, int noffset, int *data_offset);
int fit_image_get_data_position(const void *fit, int noffset,
int *data_position);
diff --git a/tools/image-host.c b/tools/image-host.c
index 5e01b853c50..007a94f72d5 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -574,7 +574,7 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
}
/* Get image data and data length */
- if (fit_image_get_data(fit, image_noffset, &data, &size)) {
+ if (fit_image_get_emb_data(fit, image_noffset, &data, &size)) {
fprintf(stderr, "Can't get image data/size\n");
return -1;
}
@@ -654,7 +654,7 @@ int fit_image_add_verification_data(const char *keydir, const char *keyfile,
int noffset;
/* Get image data and data length */
- if (fit_image_get_data(fit, image_noffset, &data, &size)) {
+ if (fit_image_get_emb_data(fit, image_noffset, &data, &size)) {
fprintf(stderr, "Can't get image data/size\n");
return -1;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 16/33] boot: Rename fit_image_get_data_and_size()
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (14 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 15/33] boot: Rename fit_image_get_data() Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf Simon Glass
` (18 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Ahelenia Ziemiańska, Dmitry Gerasimov,
Dragan Simic, Heinrich Schuchardt, Igor Opaniuk, Julien Masson,
Marek Vasut, Mattijs Korpershoek, Maxim Moskalets, Paul-Erwan Rio,
Peter Korsgaard, Raymond Mao, Roman Stratiienko, Sughosh Ganu,
Tom Rini
This function is really just getting the data. The size comes along for
the ride. In fact this function is only reliable way to obtain the data
for an image in a FIT, since the FIT may use external data.
Rename it to fit_image_get_data()
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
boot/image-board.c | 4 ++--
boot/image-fit.c | 20 +++++++++-----------
cmd/ximg.c | 3 +--
include/image.h | 4 ++--
tools/fit_image.c | 2 +-
5 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/boot/image-board.c b/boot/image-board.c
index d018d790a49..8e6b2974dcf 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -1087,8 +1087,8 @@ fallback:
}
/* get script subimage data address and length */
- if (fit_image_get_data_and_size(fit_hdr, noffset,
- &fit_data, &fit_len)) {
+ if (fit_image_get_data(fit_hdr, noffset, &fit_data,
+ &fit_len)) {
puts("Could not find script subimage data\n");
return 1;
}
diff --git a/boot/image-fit.c b/boot/image-fit.c
index 4b116ba5891..e9653452eae 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -509,7 +509,7 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
fit_image_get_comp(fit, image_noffset, &comp);
printf("%s Compression: %s\n", p, genimg_get_comp_name(comp));
- ret = fit_image_get_data_and_size(fit, image_noffset, &data, &size);
+ ret = fit_image_get_data(fit, image_noffset, &data, &size);
if (!tools_build()) {
printf("%s Data Start: ", p);
@@ -1031,14 +1031,14 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
}
/**
- * fit_image_get_data_and_size - get data and its size including
+ * fit_image_get_data - get data and its size including
* both embedded and external data
* @fit: pointer to the FIT format image header
* @noffset: component image node offset
* @data: double pointer to void, will hold data property's data address
* @size: pointer to size_t, will hold data property's data size
*
- * fit_image_get_data_and_size() finds data and its size including
+ * fit_image_get_data() finds data and its size including
* both embedded and external data. If the property is found
* its data start address and size are returned to the caller.
*
@@ -1046,8 +1046,8 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
* 0, on success
* otherwise, on failure
*/
-int fit_image_get_data_and_size(const void *fit, int noffset,
- const void **data, size_t *size)
+int fit_image_get_data(const void *fit, int noffset, const void **data,
+ size_t *size)
{
bool external_data = false;
int offset;
@@ -1432,7 +1432,7 @@ int fit_image_verify(const void *fit, int image_noffset)
goto err;
}
/* Get image data and data length */
- if (fit_image_get_data_and_size(fit, image_noffset, &data, &size)) {
+ if (fit_image_get_data(fit, image_noffset, &data, &size)) {
err_msg = "Can't get image data/size";
goto err;
}
@@ -1781,8 +1781,7 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
}
/* search in this config's kernel FDT */
- if (fit_image_get_data_and_size(fit, kfdt_noffset,
- &fdt, &sz)) {
+ if (fit_image_get_data(fit, kfdt_noffset, &fdt, &sz)) {
debug("Failed to get fdt \"%s\".\n", kfdt_name);
continue;
}
@@ -1941,7 +1940,7 @@ static int fit_get_data_tail(const void *fit, int noffset,
if (!fit_image_verify(fit, noffset))
return -EINVAL;
- if (fit_image_get_data_and_size(fit, noffset, data, size))
+ if (fit_image_get_data(fit, noffset, data, size))
return -ENOENT;
if (!fit_get_desc(fit, noffset, &desc))
@@ -2197,8 +2196,7 @@ int fit_image_load(struct bootm_headers *images, ulong addr,
bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL_OK);
/* get image data address and length */
- if (fit_image_get_data_and_size(fit, noffset,
- (const void **)&buf, &size)) {
+ if (fit_image_get_data(fit, noffset, (const void **)&buf, &size)) {
printf("Could not find %s subimage data!\n", prop_name);
bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA);
return -ENOENT;
diff --git a/cmd/ximg.c b/cmd/ximg.c
index 1c96f5a0a1f..29d7c3279b3 100644
--- a/cmd/ximg.c
+++ b/cmd/ximg.c
@@ -161,8 +161,7 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
}
/* get subimage/external data address and length */
- if (fit_image_get_data_and_size(fit_hdr, noffset,
- &fit_data, &fit_len)) {
+ if (fit_image_get_data(fit_hdr, noffset, &fit_data, &fit_len)) {
puts("Could not find script subimage data\n");
return 1;
}
diff --git a/include/image.h b/include/image.h
index e5241648b5e..12f52919812 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1204,8 +1204,8 @@ int fit_image_get_data_position(const void *fit, int noffset,
int fit_image_get_data_size(const void *fit, int noffset, int *data_size);
int fit_image_get_data_size_unciphered(const void *fit, int noffset,
size_t *data_size);
-int fit_image_get_data_and_size(const void *fit, int noffset,
- const void **data, size_t *size);
+int fit_image_get_data(const void *fit, int noffset, const void **data,
+ size_t *size);
/**
* fit_get_data_node() - Get verified image data for an image
diff --git a/tools/fit_image.c b/tools/fit_image.c
index 0fccfbb4ebd..caed8d5f901 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -876,7 +876,7 @@ static int fit_image_extract(
int ret;
/* get the data address and size of component at offset "image_noffset" */
- ret = fit_image_get_data_and_size(fit, image_noffset, &file_data, &file_size);
+ ret = fit_image_get_data(fit, image_noffset, &file_data, &file_size);
if (ret) {
fprintf(stderr, "Could not get component information\n");
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (15 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 16/33] boot: Rename fit_image_get_data_and_size() Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-08 18:25 ` Tom Rini
2025-01-06 14:32 ` [PATCH v2 18/33] boot: Use fit_image_get_data() to get data Simon Glass
` (17 subsequent siblings)
34 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Alexander Dahl, Alexander Kochetkov,
Alexander Sverdlin, Bin Meng, Caleb Connolly, Devarsh Thakkar,
Heinrich Schuchardt, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Janne Grunau, Julien Masson, Laurent Pinchart,
Leo Yu-Chi Liang, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Maxim Moskalets, Nikhil M Jain,
Paul-Erwan Rio, Raymond Mao, Roman Stratiienko, Sughosh Ganu,
Tom Rini
This function uses separate arguments for data and size. Use the new
abuf instead, so that they are paired and in one place. In some cases it
also saves an argument, thus potentially reducing code size.
Move the prototype to the header file while here.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
arch/arm/cpu/armv8/sec_firmware.c | 12 +++++-----
arch/arm/mach-k3/r5/sysfw-loader.c | 11 +++++++++-
arch/x86/lib/bootm.c | 7 ++++--
boot/image-fit.c | 35 +++++++++++-------------------
common/spl/spl_fit.c | 8 ++++---
common/splash_source.c | 11 ++++------
include/image.h | 20 +++++++++++++++--
tools/Makefile | 3 ++-
tools/image-host.c | 23 ++++++++++----------
9 files changed, 74 insertions(+), 56 deletions(-)
diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
index b7c73f288bd..e768859acfb 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -83,10 +83,8 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
{
phys_addr_t sec_firmware_loadable_addr = 0;
int conf_node_off, ld_node_off, images;
- const void *data;
- size_t size;
- ulong load;
const char *name, *str, *type;
+ ulong load;
int len;
conf_node_off = fit_conf_get_node(sec_firmware_img, NULL);
@@ -114,6 +112,8 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
for (str = name; str && ((str - name) < len);
str = strchr(str, '\0') + 1) {
+ struct abuf buf;
+
printf("%s: '%s'\n", type, str);
ld_node_off = fdt_subnode_offset(sec_firmware_img, images, str);
if (ld_node_off < 0) {
@@ -129,7 +129,7 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
}
if (fit_image_get_emb_data(sec_firmware_img, ld_node_off,
- &data, &size)) {
+ &buf)) {
printf("SEC Loadable: Can't get subimage data/size");
return -ENOENT;
}
@@ -147,9 +147,9 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
/* Copy loadable to secure memory and flush dcache */
debug("%s copied to address 0x%p\n",
FIT_LOADABLE_PROP, (void *)sec_firmware_loadable_addr);
- memcpy((void *)sec_firmware_loadable_addr, data, size);
+ memcpy((void *)sec_firmware_loadable_addr, buf.data, buf.size);
flush_dcache_range(sec_firmware_loadable_addr,
- sec_firmware_loadable_addr + size);
+ sec_firmware_loadable_addr + buf.size);
/* Populate loadable address only for Trusted OS */
if (!strcmp(str, "trustedOS@1")) {
diff --git a/arch/arm/mach-k3/r5/sysfw-loader.c b/arch/arm/mach-k3/r5/sysfw-loader.c
index c323d2f78f8..1895fb385a5 100644
--- a/arch/arm/mach-k3/r5/sysfw-loader.c
+++ b/arch/arm/mach-k3/r5/sysfw-loader.c
@@ -110,12 +110,21 @@ static int fit_get_data_by_name(const void *fit, int images, const char *name,
const void **addr, size_t *size)
{
int node_offset;
+ struct abuf buf;
+ int ret;
node_offset = fdt_subnode_offset(fit, images, name);
if (node_offset < 0)
return -ENOENT;
- return fit_image_get_emb_data(fit, node_offset, addr, size);
+ ret = fit_image_get_emb_data(fit, node_offset, &buf);
+ if (ret)
+ return ret;
+
+ *addr = buf.data;
+ *size = buf.size;
+
+ return 0;
}
static void k3_start_system_controller(int rproc_id, bool rproc_loaded,
diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c
index 3305560aa06..16980c15081 100644
--- a/arch/x86/lib/bootm.c
+++ b/arch/x86/lib/bootm.c
@@ -106,13 +106,16 @@ static int boot_prep_linux(struct bootm_headers *images)
is_zimage = 1;
#if defined(CONFIG_FIT)
} else if (images->fit_uname_os && is_zimage) {
+ struct abuf buf;
+
ret = fit_image_get_emb_data(images->fit_hdr_os,
- images->fit_noffset_os,
- (const void **)&data, &len);
+ images->fit_noffset_os, &buf);
if (ret) {
puts("Can't get image data/size!\n");
goto error;
}
+ data = buf.data;
+ len = buf.size;
is_zimage = 1;
#endif
}
diff --git a/boot/image-fit.c b/boot/image-fit.c
index e9653452eae..e9b4152eb84 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -33,6 +33,7 @@
DECLARE_GLOBAL_DATA_PTR;
#endif /* !USE_HOSTCC*/
+#include <abuf.h>
#include <bootm.h>
#include <image.h>
#include <bootstage.h>
@@ -901,34 +902,20 @@ int fit_image_get_entry(const void *fit, int noffset, ulong *entry)
return fit_image_get_address(fit, noffset, FIT_ENTRY_PROP, entry);
}
-/**
- * fit_image_get_emb_data - get data property and its size for a given component image node
- * @fit: pointer to the FIT format image header
- * @noffset: component image node offset
- * @data: double pointer to void, will hold data property's data address
- * @size: pointer to size_t, will hold data property's data size
- *
- * fit_image_get_emb_data() finds data property in a given component image node.
- * If the property is found its data start address and size are returned to
- * the caller.
- *
- * returns:
- * 0, on success
- * -1, on failure
- */
-int fit_image_get_emb_data(const void *fit, int noffset, const void **data,
- size_t *size)
+int fit_image_get_emb_data(const void *fit, int noffset, struct abuf *buf)
{
+ const void *data;
int len;
- *data = fdt_getprop(fit, noffset, FIT_DATA_PROP, &len);
- if (*data == NULL) {
+ data = fdt_getprop(fit, noffset, FIT_DATA_PROP, &len);
+ if (!data) {
fit_get_debug(fit, noffset, FIT_DATA_PROP, len);
- *size = 0;
+ abuf_init(buf);
return -1;
}
- *size = len;
+ abuf_init_const(buf, data, len);
+
return 0;
}
@@ -1074,7 +1061,11 @@ int fit_image_get_data(const void *fit, int noffset, const void **data,
*size = len;
}
} else {
- ret = fit_image_get_emb_data(fit, noffset, data, size);
+ struct abuf buf;
+
+ ret = fit_image_get_emb_data(fit, noffset, &buf);
+ *data = buf.data;
+ *size = buf.size;
}
return ret;
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 097c731d9a3..172153b75c2 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -214,7 +214,6 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset,
void *src;
ulong overhead;
uint8_t image_comp = -1, type = -1;
- const void *data;
const void *fit = ctx->fit;
bool external_data = false;
@@ -281,14 +280,17 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset,
src_ptr, offset, (unsigned long)length);
src = src_ptr + overhead;
} else {
+ struct abuf buf;
+
/* Embedded data */
- if (fit_image_get_emb_data(fit, node, &data, &length)) {
+ if (fit_image_get_emb_data(fit, node, &buf)) {
puts("Cannot get image data/size\n");
return -ENOENT;
}
+ src = buf.data;
+ length = buf.size;
debug("Embedded data: dst=%lx, size=%lx\n", load_addr,
(unsigned long)length);
- src = (void *)data; /* cast away const */
}
if (CONFIG_IS_ENABLED(FIT_SIGNATURE)) {
diff --git a/common/splash_source.c b/common/splash_source.c
index 5ac32a2f995..37648b8b9f3 100644
--- a/common/splash_source.c
+++ b/common/splash_source.c
@@ -347,8 +347,6 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr)
int res;
int node_offset;
const char *splash_file;
- const void *internal_splash_data;
- size_t internal_splash_size;
int external_splash_addr;
int external_splash_size;
bool is_splash_external = false;
@@ -356,6 +354,7 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr)
const u32 *fit_header;
u32 fit_size;
const size_t header_size = sizeof(struct legacy_img_hdr);
+ struct abuf buf;
/* Read in image header */
res = splash_storage_read_raw(location, bmp_load_addr, header_size);
@@ -396,12 +395,10 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr)
/* Extract the splash data from FIT */
/* 1. Test if splash is in FIT internal data. */
- if (!fit_image_get_emb_data(fit_header, node_offset,
- &internal_splash_data,
- &internal_splash_size))
- memmove((void *)(uintptr_t)bmp_load_addr, internal_splash_data, internal_splash_size);
+ if (!fit_image_get_emb_data(fit_header, node_offset, &buf)) {
+ memmove((void *)(uintptr_t)bmp_load_addr, buf.data, buf.size);
/* 2. Test if splash is in FIT external data with fixed position. */
- else if (!fit_image_get_data_position(fit_header, node_offset, &external_splash_addr))
+ } else if (!fit_image_get_data_position(fit_header, node_offset, &external_splash_addr))
is_splash_external = true;
/* 3. Test if splash is in FIT external data with offset. */
else if (!fit_image_get_data_offset(fit_header, node_offset, &external_splash_addr)) {
diff --git a/include/image.h b/include/image.h
index 12f52919812..29ac7a69ae9 100644
--- a/include/image.h
+++ b/include/image.h
@@ -20,6 +20,7 @@
#include <stdbool.h>
/* Define this to avoid #ifdefs later on */
+struct abuf;
struct fdt_region;
#ifdef USE_HOSTCC
@@ -1196,8 +1197,23 @@ int fit_image_get_type(const void *fit, int noffset, uint8_t *type);
int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp);
int fit_image_get_load(const void *fit, int noffset, ulong *load);
int fit_image_get_entry(const void *fit, int noffset, ulong *entry);
-int fit_image_get_emb_data(const void *fit, int noffset, const void **data,
- size_t *size);
+
+/**
+ * fit_image_get_emb_data() - get embedded data for a component-image node
+ * @fit: pointer to the FIT format image header
+ * @noffset: component image node offset
+ * @buf: returns data (inited by this function)
+ *
+ * fit_image_get_emb_data() finds data property in a given component image node.
+ * If the property is found its data start address and size are returned to
+ * the caller.
+ *
+ * returns:
+ * 0, on success
+ * -1, on failure
+ */
+int fit_image_get_emb_data(const void *fit, int noffset, struct abuf *buf);
+
int fit_image_get_data_offset(const void *fit, int noffset, int *data_offset);
int fit_image_get_data_position(const void *fit, int noffset,
int *data_position);
diff --git a/tools/Makefile b/tools/Makefile
index ee08a9675df..d0adfe1f58d 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -71,7 +71,8 @@ ifneq ($(CONFIG_CMD_BOOTEFI_SELFTEST)$(CONFIG_FWU_MDATA_GPT_BLK),)
hostprogs-y += file2include
endif
-FIT_OBJS-y := fit_common.o fit_image.o image-host.o generated/boot/image-fit.o
+FIT_OBJS-y := fit_common.o fit_image.o image-host.o generated/boot/image-fit.o \
+ generated/lib/abuf.o
FIT_SIG_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := image-sig-host.o generated/boot/image-fit-sig.o
FIT_CIPHER_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := generated/boot/image-cipher.o
diff --git a/tools/image-host.c b/tools/image-host.c
index 007a94f72d5..d428f655591 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -9,6 +9,7 @@
*/
#include "mkimage.h"
+#include <abuf.h>
#include <bootm.h>
#include <fdt_region.h>
#include <image.h>
@@ -509,7 +510,7 @@ int fit_image_write_cipher(void *fit, int image_noffset, int noffset,
static int
fit_image_process_cipher(const char *keydir, void *keydest, void *fit,
const char *image_name, int image_noffset,
- int node_noffset, const void *data, size_t size,
+ int node_noffset, struct abuf *buf,
const char *cmdname)
{
struct image_cipher_info info;
@@ -524,7 +525,7 @@ fit_image_process_cipher(const char *keydir, void *keydest, void *fit,
if (ret)
goto out;
- ret = info.cipher->encrypt(&info, data, size,
+ ret = info.cipher->encrypt(&info, buf->data, buf->size,
&data_ciphered, &data_ciphered_len);
if (ret)
goto out;
@@ -546,7 +547,7 @@ fit_image_process_cipher(const char *keydir, void *keydest, void *fit,
}
ret = fit_image_write_cipher(fit, image_noffset, node_noffset,
- data, size,
+ buf->data, buf->size,
data_ciphered, data_ciphered_len);
out:
@@ -562,9 +563,8 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
const char *cmdname)
{
const char *image_name;
- const void *data;
- size_t size;
int cipher_node_offset, len;
+ struct abuf buf;
/* Get image name */
image_name = fit_get_name(fit, image_noffset, NULL);
@@ -574,7 +574,7 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
}
/* Get image data and data length */
- if (fit_image_get_emb_data(fit, image_noffset, &data, &size)) {
+ if (fit_image_get_emb_data(fit, image_noffset, &buf)) {
fprintf(stderr, "Can't get image data/size\n");
return -1;
}
@@ -605,7 +605,7 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
if (!IMAGE_ENABLE_ENCRYPT || !keydir)
return 0;
return fit_image_process_cipher(keydir, keydest, fit, image_name,
- image_noffset, cipher_node_offset, data, size, cmdname);
+ image_noffset, cipher_node_offset, &buf, cmdname);
}
/**
@@ -649,12 +649,11 @@ int fit_image_add_verification_data(const char *keydir, const char *keyfile,
const char *cmdname, const char* algo_name)
{
const char *image_name;
- const void *data;
- size_t size;
+ struct abuf buf;
int noffset;
/* Get image data and data length */
- if (fit_image_get_emb_data(fit, image_noffset, &data, &size)) {
+ if (fit_image_get_emb_data(fit, image_noffset, &buf)) {
fprintf(stderr, "Can't get image data/size\n");
return -1;
}
@@ -677,12 +676,12 @@ int fit_image_add_verification_data(const char *keydir, const char *keyfile,
if (!strncmp(node_name, FIT_HASH_NODENAME,
strlen(FIT_HASH_NODENAME))) {
ret = fit_image_process_hash(fit, image_name, noffset,
- data, size);
+ buf.data, buf.size);
} else if (IMAGE_ENABLE_SIGN && (keydir || keyfile) &&
!strncmp(node_name, FIT_SIG_NODENAME,
strlen(FIT_SIG_NODENAME))) {
ret = fit_image_process_sig(keydir, keyfile, keydest,
- fit, image_name, noffset, data, size,
+ fit, image_name, noffset, buf.data, buf.size,
comment, require_keys, engine_id, cmdname,
algo_name);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf
2025-01-06 14:32 ` [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf Simon Glass
@ 2025-01-08 18:25 ` Tom Rini
2025-01-09 12:36 ` Simon Glass
0 siblings, 1 reply; 60+ messages in thread
From: Tom Rini @ 2025-01-08 18:25 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Alexander Dahl, Alexander Kochetkov,
Alexander Sverdlin, Bin Meng, Caleb Connolly, Devarsh Thakkar,
Heinrich Schuchardt, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Janne Grunau, Julien Masson, Laurent Pinchart,
Leo Yu-Chi Liang, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Maxim Moskalets, Nikhil M Jain,
Paul-Erwan Rio, Raymond Mao, Roman Stratiienko, Sughosh Ganu
[-- Attachment #1: Type: text/plain, Size: 462 bytes --]
On Mon, Jan 06, 2025 at 07:32:13AM -0700, Simon Glass wrote:
> This function uses separate arguments for data and size. Use the new
> abuf instead, so that they are paired and in one place. In some cases it
> also saves an argument, thus potentially reducing code size.
This is one of the commits that globally increases size in both full
U-Boot and SPL/etc.
Is all of the "abuf" changes just a "tidy up" that increases the code a
bit?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf
2025-01-08 18:25 ` Tom Rini
@ 2025-01-09 12:36 ` Simon Glass
2025-01-09 15:09 ` Tom Rini
0 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-09 12:36 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Alexander Dahl, Alexander Kochetkov,
Alexander Sverdlin, Bin Meng, Caleb Connolly, Devarsh Thakkar,
Heinrich Schuchardt, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Janne Grunau, Julien Masson, Laurent Pinchart,
Leo Yu-Chi Liang, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Maxim Moskalets, Nikhil M Jain,
Paul-Erwan Rio, Raymond Mao, Roman Stratiienko, Sughosh Ganu
Hi Tom,
On Wed, 8 Jan 2025 at 11:25, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Jan 06, 2025 at 07:32:13AM -0700, Simon Glass wrote:
>
> > This function uses separate arguments for data and size. Use the new
> > abuf instead, so that they are paired and in one place. In some cases it
> > also saves an argument, thus potentially reducing code size.
>
> This is one of the commits that globally increases size in both full
> U-Boot and SPL/etc.
>
> Is all of the "abuf" changes just a "tidy up" that increases the code a
> bit?
Yes, a tidy-up which I hope will help overall. I have been thinking
for a while of how to avoid having addr/size and ptr/size passed
everywhere. For now abuf seems to provide some sort of solution.
I see this:
18: boot: Update fit_image_get_emb_data to use abuf
aarch64: (for 1/1 boards) all +4.0 bss -24.0 spl/u-boot-spl:all
+16.0 spl/u-boot-spl:text +16.0 text +28.0
so growth on firefly-rk3399 but not with rk3288. I am not sure if the
growth will tail off as there are more users, though. We might even be
able to be more clever with static inlines.
Regards,
Simon
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf
2025-01-09 12:36 ` Simon Glass
@ 2025-01-09 15:09 ` Tom Rini
2025-01-09 15:14 ` Simon Glass
0 siblings, 1 reply; 60+ messages in thread
From: Tom Rini @ 2025-01-09 15:09 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Alexander Dahl, Alexander Kochetkov,
Alexander Sverdlin, Bin Meng, Caleb Connolly, Devarsh Thakkar,
Heinrich Schuchardt, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Janne Grunau, Julien Masson, Laurent Pinchart,
Leo Yu-Chi Liang, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Maxim Moskalets, Nikhil M Jain,
Paul-Erwan Rio, Raymond Mao, Roman Stratiienko, Sughosh Ganu
[-- Attachment #1: Type: text/plain, Size: 4488 bytes --]
On Thu, Jan 09, 2025 at 05:36:03AM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Wed, 8 Jan 2025 at 11:25, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Jan 06, 2025 at 07:32:13AM -0700, Simon Glass wrote:
> >
> > > This function uses separate arguments for data and size. Use the new
> > > abuf instead, so that they are paired and in one place. In some cases it
> > > also saves an argument, thus potentially reducing code size.
> >
> > This is one of the commits that globally increases size in both full
> > U-Boot and SPL/etc.
> >
> > Is all of the "abuf" changes just a "tidy up" that increases the code a
> > bit?
>
> Yes, a tidy-up which I hope will help overall. I have been thinking
> for a while of how to avoid having addr/size and ptr/size passed
> everywhere. For now abuf seems to provide some sort of solution.
>
> I see this:
>
> 18: boot: Update fit_image_get_emb_data to use abuf
> aarch64: (for 1/1 boards) all +4.0 bss -24.0 spl/u-boot-spl:all
> +16.0 spl/u-boot-spl:text +16.0 text +28.0
>
> so growth on firefly-rk3399 but not with rk3288. I am not sure if the
> growth will tail off as there are more users, though. We might even be
> able to be more clever with static inlines.
Yeah, lets not do this now then and worry about some "clean up" later
when we can show that it does, or does not, improve size. And there's
something wrong with your numbers:
01: Fix neighbor discovery ethernet address saving
aarch64: w+ firefly-rk3399
+(firefly-rk3399) Image 'simple-bin' is missing external blobs and is non-functional: atf-bl31
+(firefly-rk3399)
+(firefly-rk3399) /binman/simple-bin/fit/images/@atf-SEQ/atf-bl31 (atf-bl31):
+(firefly-rk3399) See the documentation for your board. You may need to build ARM Trusted
+(firefly-rk3399) Firmware and build with BL31=/path/to/bl31.bin
+(firefly-rk3399) Image 'simple-bin' is missing optional external blobs but is still functional: tee-os
+(firefly-rk3399) /binman/simple-bin/fit/images/@tee-SEQ/tee-os (tee-os):
+(firefly-rk3399) See the documentation for your board. You may need to build Open Portable
+(firefly-rk3399) Trusted Execution Environment (OP-TEE) and build with TEE=/path/to/tee.bin
+(firefly-rk3399) Some images are invalid
37: dm: core: Provide ofnode_find_subnode_unit()
aarch64: (for 1/1 boards) all +324.0 bss +32.0 spl/u-boot-spl:all +16.0 spl/u-boot-spl:text +16.0 text +292.0
firefly-rk3399 : all +324 bss +32 spl/u-boot-spl:all +16 spl/u-boot-spl:text +16 text +292
u-boot: add: 6/-1, grow: 4/-4 bytes: 516/-224 (292)
function old new delta
ofnode_name_eq_unit - 160 +160
ofnode_find_subnode_unit - 116 +116
fit_image_get_data 80 176 +96
fit_image_get_emb_data - 84 +84
ofnode_write_prop 224 236 +12
ofnode_add_subnode 232 244 +12
abuf_init_const - 12 +12
abuf_init - 12 +12
abuf_addr - 8 +8
fit_image_print 780 784 +4
image_locate_script 696 692 -4
fit_image_load 1584 1580 -4
fit_image_verify 176 164 -12
ofnode_find_subnode 140 116 -24
fit_image_get_data_and_size 180 - -180
spl-u-boot-spl: add: 3/-1, grow: 0/-1 bytes: 108/-92 (16)
function old new delta
fit_image_get_emb_data - 84 +84
abuf_init_const - 12 +12
abuf_init - 12 +12
load_simple_fit 580 568 -12
fit_image_get_data 80 - -80
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf
2025-01-09 15:09 ` Tom Rini
@ 2025-01-09 15:14 ` Simon Glass
2025-01-09 18:08 ` Tom Rini
0 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-09 15:14 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Alexander Dahl, Alexander Kochetkov,
Alexander Sverdlin, Bin Meng, Caleb Connolly, Devarsh Thakkar,
Heinrich Schuchardt, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Janne Grunau, Julien Masson, Laurent Pinchart,
Leo Yu-Chi Liang, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Maxim Moskalets, Nikhil M Jain,
Paul-Erwan Rio, Raymond Mao, Roman Stratiienko, Sughosh Ganu
Hi Tom,
On Thu, 9 Jan 2025 at 08:10, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jan 09, 2025 at 05:36:03AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 8 Jan 2025 at 11:25, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Jan 06, 2025 at 07:32:13AM -0700, Simon Glass wrote:
> > >
> > > > This function uses separate arguments for data and size. Use the new
> > > > abuf instead, so that they are paired and in one place. In some cases it
> > > > also saves an argument, thus potentially reducing code size.
> > >
> > > This is one of the commits that globally increases size in both full
> > > U-Boot and SPL/etc.
> > >
> > > Is all of the "abuf" changes just a "tidy up" that increases the code a
> > > bit?
> >
> > Yes, a tidy-up which I hope will help overall. I have been thinking
> > for a while of how to avoid having addr/size and ptr/size passed
> > everywhere. For now abuf seems to provide some sort of solution.
> >
> > I see this:
> >
> > 18: boot: Update fit_image_get_emb_data to use abuf
> > aarch64: (for 1/1 boards) all +4.0 bss -24.0 spl/u-boot-spl:all
> > +16.0 spl/u-boot-spl:text +16.0 text +28.0
> >
> > so growth on firefly-rk3399 but not with rk3288. I am not sure if the
> > growth will tail off as there are more users, though. We might even be
> > able to be more clever with static inlines.
>
> Yeah, lets not do this now then and worry about some "clean up" later
> when we can show that it does, or does not, improve size.
Oh.
> And there's
> something wrong with your numbers:
> 01: Fix neighbor discovery ethernet address saving
> aarch64: w+ firefly-rk3399
> +(firefly-rk3399) Image 'simple-bin' is missing external blobs and is non-functional: atf-bl31
> +(firefly-rk3399)
> +(firefly-rk3399) /binman/simple-bin/fit/images/@atf-SEQ/atf-bl31 (atf-bl31):
> +(firefly-rk3399) See the documentation for your board. You may need to build ARM Trusted
> +(firefly-rk3399) Firmware and build with BL31=/path/to/bl31.bin
> +(firefly-rk3399) Image 'simple-bin' is missing optional external blobs but is still functional: tee-os
> +(firefly-rk3399) /binman/simple-bin/fit/images/@tee-SEQ/tee-os (tee-os):
> +(firefly-rk3399) See the documentation for your board. You may need to build Open Portable
> +(firefly-rk3399) Trusted Execution Environment (OP-TEE) and build with TEE=/path/to/tee.bin
> +(firefly-rk3399) Some images are invalid
> 37: dm: core: Provide ofnode_find_subnode_unit()
> aarch64: (for 1/1 boards) all +324.0 bss +32.0 spl/u-boot-spl:all +16.0 spl/u-boot-spl:text +16.0 text +292.0
> firefly-rk3399 : all +324 bss +32 spl/u-boot-spl:all +16 spl/u-boot-spl:text +16 text +292
> u-boot: add: 6/-1, grow: 4/-4 bytes: 516/-224 (292)
> function old new delta
> ofnode_name_eq_unit - 160 +160
> ofnode_find_subnode_unit - 116 +116
> fit_image_get_data 80 176 +96
> fit_image_get_emb_data - 84 +84
> ofnode_write_prop 224 236 +12
> ofnode_add_subnode 232 244 +12
> abuf_init_const - 12 +12
> abuf_init - 12 +12
> abuf_addr - 8 +8
> fit_image_print 780 784 +4
> image_locate_script 696 692 -4
> fit_image_load 1584 1580 -4
> fit_image_verify 176 164 -12
> ofnode_find_subnode 140 116 -24
> fit_image_get_data_and_size 180 - -180
> spl-u-boot-spl: add: 3/-1, grow: 0/-1 bytes: 108/-92 (16)
> function old new delta
> fit_image_get_emb_data - 84 +84
> abuf_init_const - 12 +12
> abuf_init - 12 +12
> load_simple_fit 580 568 -12
> fit_image_get_data 80 - -80
Yes, that's the whole series, so not related to this change.
I elected to have two versions of ofnode_find_subnode() to avoid the
size growth in the previous version. But the cost is larger size
growth when OF_LIVE is used.
Without OF_LIVE, the size growth is tiny.
So...what to do?
Regards,
Simon
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf
2025-01-09 15:14 ` Simon Glass
@ 2025-01-09 18:08 ` Tom Rini
2025-01-10 13:39 ` Simon Glass
0 siblings, 1 reply; 60+ messages in thread
From: Tom Rini @ 2025-01-09 18:08 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Alexander Dahl, Alexander Kochetkov,
Alexander Sverdlin, Bin Meng, Caleb Connolly, Devarsh Thakkar,
Heinrich Schuchardt, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Janne Grunau, Julien Masson, Laurent Pinchart,
Leo Yu-Chi Liang, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Maxim Moskalets, Nikhil M Jain,
Paul-Erwan Rio, Raymond Mao, Roman Stratiienko, Sughosh Ganu
[-- Attachment #1: Type: text/plain, Size: 5701 bytes --]
On Thu, Jan 09, 2025 at 08:14:53AM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Thu, 9 Jan 2025 at 08:10, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jan 09, 2025 at 05:36:03AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 8 Jan 2025 at 11:25, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Mon, Jan 06, 2025 at 07:32:13AM -0700, Simon Glass wrote:
> > > >
> > > > > This function uses separate arguments for data and size. Use the new
> > > > > abuf instead, so that they are paired and in one place. In some cases it
> > > > > also saves an argument, thus potentially reducing code size.
> > > >
> > > > This is one of the commits that globally increases size in both full
> > > > U-Boot and SPL/etc.
> > > >
> > > > Is all of the "abuf" changes just a "tidy up" that increases the code a
> > > > bit?
> > >
> > > Yes, a tidy-up which I hope will help overall. I have been thinking
> > > for a while of how to avoid having addr/size and ptr/size passed
> > > everywhere. For now abuf seems to provide some sort of solution.
> > >
> > > I see this:
> > >
> > > 18: boot: Update fit_image_get_emb_data to use abuf
> > > aarch64: (for 1/1 boards) all +4.0 bss -24.0 spl/u-boot-spl:all
> > > +16.0 spl/u-boot-spl:text +16.0 text +28.0
> > >
> > > so growth on firefly-rk3399 but not with rk3288. I am not sure if the
> > > growth will tail off as there are more users, though. We might even be
> > > able to be more clever with static inlines.
> >
> > Yeah, lets not do this now then and worry about some "clean up" later
> > when we can show that it does, or does not, improve size.
>
> Oh.
>
> > And there's
> > something wrong with your numbers:
> > 01: Fix neighbor discovery ethernet address saving
> > aarch64: w+ firefly-rk3399
> > +(firefly-rk3399) Image 'simple-bin' is missing external blobs and is non-functional: atf-bl31
> > +(firefly-rk3399)
> > +(firefly-rk3399) /binman/simple-bin/fit/images/@atf-SEQ/atf-bl31 (atf-bl31):
> > +(firefly-rk3399) See the documentation for your board. You may need to build ARM Trusted
> > +(firefly-rk3399) Firmware and build with BL31=/path/to/bl31.bin
> > +(firefly-rk3399) Image 'simple-bin' is missing optional external blobs but is still functional: tee-os
> > +(firefly-rk3399) /binman/simple-bin/fit/images/@tee-SEQ/tee-os (tee-os):
> > +(firefly-rk3399) See the documentation for your board. You may need to build Open Portable
> > +(firefly-rk3399) Trusted Execution Environment (OP-TEE) and build with TEE=/path/to/tee.bin
> > +(firefly-rk3399) Some images are invalid
> > 37: dm: core: Provide ofnode_find_subnode_unit()
> > aarch64: (for 1/1 boards) all +324.0 bss +32.0 spl/u-boot-spl:all +16.0 spl/u-boot-spl:text +16.0 text +292.0
> > firefly-rk3399 : all +324 bss +32 spl/u-boot-spl:all +16 spl/u-boot-spl:text +16 text +292
> > u-boot: add: 6/-1, grow: 4/-4 bytes: 516/-224 (292)
> > function old new delta
> > ofnode_name_eq_unit - 160 +160
> > ofnode_find_subnode_unit - 116 +116
> > fit_image_get_data 80 176 +96
> > fit_image_get_emb_data - 84 +84
> > ofnode_write_prop 224 236 +12
> > ofnode_add_subnode 232 244 +12
> > abuf_init_const - 12 +12
> > abuf_init - 12 +12
> > abuf_addr - 8 +8
> > fit_image_print 780 784 +4
> > image_locate_script 696 692 -4
> > fit_image_load 1584 1580 -4
> > fit_image_verify 176 164 -12
> > ofnode_find_subnode 140 116 -24
> > fit_image_get_data_and_size 180 - -180
> > spl-u-boot-spl: add: 3/-1, grow: 0/-1 bytes: 108/-92 (16)
> > function old new delta
> > fit_image_get_emb_data - 84 +84
> > abuf_init_const - 12 +12
> > abuf_init - 12 +12
> > load_simple_fit 580 568 -12
> > fit_image_get_data 80 - -80
>
> Yes, that's the whole series, so not related to this change.
Yes, that's the whole series including this change, so it's related to
this change.
> I elected to have two versions of ofnode_find_subnode() to avoid the
> size growth in the previous version. But the cost is larger size
> growth when OF_LIVE is used.
>
> Without OF_LIVE, the size growth is tiny.
And even worse in SPL, somehow. But you want more OF_LIVE users, not
less, yes?
> So...what to do?
Well, if you drop the abuf changes for now, SPL won't change at all for
most platforms and that'll be an improvement.
And I'm going to keep complaining about size growth here because a
non-trivial subset of users just wants things to boot quickly and be
small.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf
2025-01-09 18:08 ` Tom Rini
@ 2025-01-10 13:39 ` Simon Glass
2025-01-10 16:17 ` Tom Rini
0 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-10 13:39 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Alexander Dahl, Alexander Kochetkov,
Alexander Sverdlin, Bin Meng, Caleb Connolly, Devarsh Thakkar,
Heinrich Schuchardt, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Janne Grunau, Julien Masson, Laurent Pinchart,
Leo Yu-Chi Liang, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Maxim Moskalets, Nikhil M Jain,
Paul-Erwan Rio, Raymond Mao, Roman Stratiienko, Sughosh Ganu
Hi Tom,
On Thu, 9 Jan 2025 at 11:08, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jan 09, 2025 at 08:14:53AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 9 Jan 2025 at 08:10, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jan 09, 2025 at 05:36:03AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Wed, 8 Jan 2025 at 11:25, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Mon, Jan 06, 2025 at 07:32:13AM -0700, Simon Glass wrote:
> > > > >
> > > > > > This function uses separate arguments for data and size. Use the new
> > > > > > abuf instead, so that they are paired and in one place. In some cases it
> > > > > > also saves an argument, thus potentially reducing code size.
> > > > >
> > > > > This is one of the commits that globally increases size in both full
> > > > > U-Boot and SPL/etc.
> > > > >
> > > > > Is all of the "abuf" changes just a "tidy up" that increases the code a
> > > > > bit?
> > > >
> > > > Yes, a tidy-up which I hope will help overall. I have been thinking
> > > > for a while of how to avoid having addr/size and ptr/size passed
> > > > everywhere. For now abuf seems to provide some sort of solution.
> > > >
> > > > I see this:
> > > >
> > > > 18: boot: Update fit_image_get_emb_data to use abuf
> > > > aarch64: (for 1/1 boards) all +4.0 bss -24.0 spl/u-boot-spl:all
> > > > +16.0 spl/u-boot-spl:text +16.0 text +28.0
> > > >
> > > > so growth on firefly-rk3399 but not with rk3288. I am not sure if the
> > > > growth will tail off as there are more users, though. We might even be
> > > > able to be more clever with static inlines.
> > >
> > > Yeah, lets not do this now then and worry about some "clean up" later
> > > when we can show that it does, or does not, improve size.
> >
> > Oh.
> >
> > > And there's
> > > something wrong with your numbers:
> > > 01: Fix neighbor discovery ethernet address saving
> > > aarch64: w+ firefly-rk3399
> > > +(firefly-rk3399) Image 'simple-bin' is missing external blobs and is non-functional: atf-bl31
> > > +(firefly-rk3399)
> > > +(firefly-rk3399) /binman/simple-bin/fit/images/@atf-SEQ/atf-bl31 (atf-bl31):
> > > +(firefly-rk3399) See the documentation for your board. You may need to build ARM Trusted
> > > +(firefly-rk3399) Firmware and build with BL31=/path/to/bl31.bin
> > > +(firefly-rk3399) Image 'simple-bin' is missing optional external blobs but is still functional: tee-os
> > > +(firefly-rk3399) /binman/simple-bin/fit/images/@tee-SEQ/tee-os (tee-os):
> > > +(firefly-rk3399) See the documentation for your board. You may need to build Open Portable
> > > +(firefly-rk3399) Trusted Execution Environment (OP-TEE) and build with TEE=/path/to/tee.bin
> > > +(firefly-rk3399) Some images are invalid
> > > 37: dm: core: Provide ofnode_find_subnode_unit()
> > > aarch64: (for 1/1 boards) all +324.0 bss +32.0 spl/u-boot-spl:all +16.0 spl/u-boot-spl:text +16.0 text +292.0
> > > firefly-rk3399 : all +324 bss +32 spl/u-boot-spl:all +16 spl/u-boot-spl:text +16 text +292
> > > u-boot: add: 6/-1, grow: 4/-4 bytes: 516/-224 (292)
> > > function old new delta
> > > ofnode_name_eq_unit - 160 +160
> > > ofnode_find_subnode_unit - 116 +116
> > > fit_image_get_data 80 176 +96
> > > fit_image_get_emb_data - 84 +84
> > > ofnode_write_prop 224 236 +12
> > > ofnode_add_subnode 232 244 +12
> > > abuf_init_const - 12 +12
> > > abuf_init - 12 +12
> > > abuf_addr - 8 +8
> > > fit_image_print 780 784 +4
> > > image_locate_script 696 692 -4
> > > fit_image_load 1584 1580 -4
> > > fit_image_verify 176 164 -12
> > > ofnode_find_subnode 140 116 -24
> > > fit_image_get_data_and_size 180 - -180
> > > spl-u-boot-spl: add: 3/-1, grow: 0/-1 bytes: 108/-92 (16)
> > > function old new delta
> > > fit_image_get_emb_data - 84 +84
> > > abuf_init_const - 12 +12
> > > abuf_init - 12 +12
> > > load_simple_fit 580 568 -12
> > > fit_image_get_data 80 - -80
> >
> > Yes, that's the whole series, so not related to this change.
>
> Yes, that's the whole series including this change, so it's related to
> this change.
Right, but it is due to ofnode_find_subnode(), etc.
>
> > I elected to have two versions of ofnode_find_subnode() to avoid the
> > size growth in the previous version. But the cost is larger size
> > growth when OF_LIVE is used.
> >
> > Without OF_LIVE, the size growth is tiny.
>
> And even worse in SPL, somehow. But you want more OF_LIVE users, not
> less, yes?
Well, OF_LIVE is always quite a bit larger, at least at the moment. It
has both Linux's of_access stuff and libfdt. It's not OF_LIVE I am
bothered about, but I do want people using ofnode. Unfortunately
people still send patches which use libfdt directly.
>
> > So...what to do?
>
> Well, if you drop the abuf changes for now, SPL won't change at all for
> most platforms and that'll be an improvement.
Yes, I'll look at that. This is one of many examples where I have a
problem and realise that we need a nicer way of dealing with it, then
implement it in the series, but then the series loses focus. So then I
take it out again, then forget about it until next time, but I never
actually make the change.
>
> And I'm going to keep complaining about size growth here because a
> non-trivial subset of users just wants things to boot quickly and be
> small.
Yes, you won't get any complaints from me on that. I did propose some
automated checking a few years back, but it never went anywhere.
Regards,
SImon
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf
2025-01-10 13:39 ` Simon Glass
@ 2025-01-10 16:17 ` Tom Rini
2025-01-10 19:23 ` Simon Glass
0 siblings, 1 reply; 60+ messages in thread
From: Tom Rini @ 2025-01-10 16:17 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Alexander Dahl, Alexander Kochetkov,
Alexander Sverdlin, Bin Meng, Caleb Connolly, Devarsh Thakkar,
Heinrich Schuchardt, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Janne Grunau, Julien Masson, Laurent Pinchart,
Leo Yu-Chi Liang, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Maxim Moskalets, Nikhil M Jain,
Paul-Erwan Rio, Raymond Mao, Roman Stratiienko, Sughosh Ganu
[-- Attachment #1: Type: text/plain, Size: 7536 bytes --]
On Fri, Jan 10, 2025 at 06:39:11AM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Thu, 9 Jan 2025 at 11:08, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jan 09, 2025 at 08:14:53AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 9 Jan 2025 at 08:10, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Jan 09, 2025 at 05:36:03AM -0700, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Wed, 8 Jan 2025 at 11:25, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Mon, Jan 06, 2025 at 07:32:13AM -0700, Simon Glass wrote:
> > > > > >
> > > > > > > This function uses separate arguments for data and size. Use the new
> > > > > > > abuf instead, so that they are paired and in one place. In some cases it
> > > > > > > also saves an argument, thus potentially reducing code size.
> > > > > >
> > > > > > This is one of the commits that globally increases size in both full
> > > > > > U-Boot and SPL/etc.
> > > > > >
> > > > > > Is all of the "abuf" changes just a "tidy up" that increases the code a
> > > > > > bit?
> > > > >
> > > > > Yes, a tidy-up which I hope will help overall. I have been thinking
> > > > > for a while of how to avoid having addr/size and ptr/size passed
> > > > > everywhere. For now abuf seems to provide some sort of solution.
> > > > >
> > > > > I see this:
> > > > >
> > > > > 18: boot: Update fit_image_get_emb_data to use abuf
> > > > > aarch64: (for 1/1 boards) all +4.0 bss -24.0 spl/u-boot-spl:all
> > > > > +16.0 spl/u-boot-spl:text +16.0 text +28.0
> > > > >
> > > > > so growth on firefly-rk3399 but not with rk3288. I am not sure if the
> > > > > growth will tail off as there are more users, though. We might even be
> > > > > able to be more clever with static inlines.
> > > >
> > > > Yeah, lets not do this now then and worry about some "clean up" later
> > > > when we can show that it does, or does not, improve size.
> > >
> > > Oh.
> > >
> > > > And there's
> > > > something wrong with your numbers:
> > > > 01: Fix neighbor discovery ethernet address saving
> > > > aarch64: w+ firefly-rk3399
> > > > +(firefly-rk3399) Image 'simple-bin' is missing external blobs and is non-functional: atf-bl31
> > > > +(firefly-rk3399)
> > > > +(firefly-rk3399) /binman/simple-bin/fit/images/@atf-SEQ/atf-bl31 (atf-bl31):
> > > > +(firefly-rk3399) See the documentation for your board. You may need to build ARM Trusted
> > > > +(firefly-rk3399) Firmware and build with BL31=/path/to/bl31.bin
> > > > +(firefly-rk3399) Image 'simple-bin' is missing optional external blobs but is still functional: tee-os
> > > > +(firefly-rk3399) /binman/simple-bin/fit/images/@tee-SEQ/tee-os (tee-os):
> > > > +(firefly-rk3399) See the documentation for your board. You may need to build Open Portable
> > > > +(firefly-rk3399) Trusted Execution Environment (OP-TEE) and build with TEE=/path/to/tee.bin
> > > > +(firefly-rk3399) Some images are invalid
> > > > 37: dm: core: Provide ofnode_find_subnode_unit()
> > > > aarch64: (for 1/1 boards) all +324.0 bss +32.0 spl/u-boot-spl:all +16.0 spl/u-boot-spl:text +16.0 text +292.0
> > > > firefly-rk3399 : all +324 bss +32 spl/u-boot-spl:all +16 spl/u-boot-spl:text +16 text +292
> > > > u-boot: add: 6/-1, grow: 4/-4 bytes: 516/-224 (292)
> > > > function old new delta
> > > > ofnode_name_eq_unit - 160 +160
> > > > ofnode_find_subnode_unit - 116 +116
> > > > fit_image_get_data 80 176 +96
> > > > fit_image_get_emb_data - 84 +84
> > > > ofnode_write_prop 224 236 +12
> > > > ofnode_add_subnode 232 244 +12
> > > > abuf_init_const - 12 +12
> > > > abuf_init - 12 +12
> > > > abuf_addr - 8 +8
> > > > fit_image_print 780 784 +4
> > > > image_locate_script 696 692 -4
> > > > fit_image_load 1584 1580 -4
> > > > fit_image_verify 176 164 -12
> > > > ofnode_find_subnode 140 116 -24
> > > > fit_image_get_data_and_size 180 - -180
> > > > spl-u-boot-spl: add: 3/-1, grow: 0/-1 bytes: 108/-92 (16)
> > > > function old new delta
> > > > fit_image_get_emb_data - 84 +84
> > > > abuf_init_const - 12 +12
> > > > abuf_init - 12 +12
> > > > load_simple_fit 580 568 -12
> > > > fit_image_get_data 80 - -80
> > >
> > > Yes, that's the whole series, so not related to this change.
> >
> > Yes, that's the whole series including this change, so it's related to
> > this change.
>
> Right, but it is due to ofnode_find_subnode(), etc.
>
> >
> > > I elected to have two versions of ofnode_find_subnode() to avoid the
> > > size growth in the previous version. But the cost is larger size
> > > growth when OF_LIVE is used.
> > >
> > > Without OF_LIVE, the size growth is tiny.
> >
> > And even worse in SPL, somehow. But you want more OF_LIVE users, not
> > less, yes?
>
> Well, OF_LIVE is always quite a bit larger, at least at the moment. It
> has both Linux's of_access stuff and libfdt. It's not OF_LIVE I am
> bothered about, but I do want people using ofnode. Unfortunately
> people still send patches which use libfdt directly.
>
> >
> > > So...what to do?
> >
> > Well, if you drop the abuf changes for now, SPL won't change at all for
> > most platforms and that'll be an improvement.
>
> Yes, I'll look at that. This is one of many examples where I have a
> problem and realise that we need a nicer way of dealing with it, then
> implement it in the series, but then the series loses focus. So then I
> take it out again, then forget about it until next time, but I never
> actually make the change.
I hate to start to derail this, but refactor for "nicer code" is very
much subjective. Especially when it also grows the code (and it's not
clear that wider usage would result in shrinkage). So yes, this really
needs to be put aside and also part of why I keep asking for one thing
at a time.
> > And I'm going to keep complaining about size growth here because a
> > non-trivial subset of users just wants things to boot quickly and be
> > small.
>
> Yes, you won't get any complaints from me on that. I did propose some
> automated checking a few years back, but it never went anywhere.
It be great if buildman size comparison had some way to csv the output.
That's what's missing imo from being able to have some automation or
even just nicer tooling.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf
2025-01-10 16:17 ` Tom Rini
@ 2025-01-10 19:23 ` Simon Glass
2025-01-10 21:15 ` Tom Rini
0 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-10 19:23 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Alexander Dahl, Alexander Kochetkov,
Alexander Sverdlin, Bin Meng, Caleb Connolly, Devarsh Thakkar,
Heinrich Schuchardt, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Janne Grunau, Julien Masson, Laurent Pinchart,
Leo Yu-Chi Liang, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Maxim Moskalets, Nikhil M Jain,
Paul-Erwan Rio, Raymond Mao, Roman Stratiienko, Sughosh Ganu
Hi Tom,
On Fri, 10 Jan 2025 at 09:17, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Jan 10, 2025 at 06:39:11AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 9 Jan 2025 at 11:08, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jan 09, 2025 at 08:14:53AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 9 Jan 2025 at 08:10, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Thu, Jan 09, 2025 at 05:36:03AM -0700, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Wed, 8 Jan 2025 at 11:25, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Mon, Jan 06, 2025 at 07:32:13AM -0700, Simon Glass wrote:
> > > > > > >
> > > > > > > > This function uses separate arguments for data and size. Use the new
> > > > > > > > abuf instead, so that they are paired and in one place. In some cases it
> > > > > > > > also saves an argument, thus potentially reducing code size.
> > > > > > >
> > > > > > > This is one of the commits that globally increases size in both full
> > > > > > > U-Boot and SPL/etc.
> > > > > > >
> > > > > > > Is all of the "abuf" changes just a "tidy up" that increases the code a
> > > > > > > bit?
> > > > > >
> > > > > > Yes, a tidy-up which I hope will help overall. I have been thinking
> > > > > > for a while of how to avoid having addr/size and ptr/size passed
> > > > > > everywhere. For now abuf seems to provide some sort of solution.
> > > > > >
> > > > > > I see this:
> > > > > >
> > > > > > 18: boot: Update fit_image_get_emb_data to use abuf
> > > > > > aarch64: (for 1/1 boards) all +4.0 bss -24.0 spl/u-boot-spl:all
> > > > > > +16.0 spl/u-boot-spl:text +16.0 text +28.0
> > > > > >
> > > > > > so growth on firefly-rk3399 but not with rk3288. I am not sure if the
> > > > > > growth will tail off as there are more users, though. We might even be
> > > > > > able to be more clever with static inlines.
> > > > >
> > > > > Yeah, lets not do this now then and worry about some "clean up" later
> > > > > when we can show that it does, or does not, improve size.
> > > >
> > > > Oh.
> > > >
> > > > > And there's
> > > > > something wrong with your numbers:
> > > > > 01: Fix neighbor discovery ethernet address saving
> > > > > aarch64: w+ firefly-rk3399
> > > > > +(firefly-rk3399) Image 'simple-bin' is missing external blobs and is non-functional: atf-bl31
> > > > > +(firefly-rk3399)
> > > > > +(firefly-rk3399) /binman/simple-bin/fit/images/@atf-SEQ/atf-bl31 (atf-bl31):
> > > > > +(firefly-rk3399) See the documentation for your board. You may need to build ARM Trusted
> > > > > +(firefly-rk3399) Firmware and build with BL31=/path/to/bl31.bin
> > > > > +(firefly-rk3399) Image 'simple-bin' is missing optional external blobs but is still functional: tee-os
> > > > > +(firefly-rk3399) /binman/simple-bin/fit/images/@tee-SEQ/tee-os (tee-os):
> > > > > +(firefly-rk3399) See the documentation for your board. You may need to build Open Portable
> > > > > +(firefly-rk3399) Trusted Execution Environment (OP-TEE) and build with TEE=/path/to/tee.bin
> > > > > +(firefly-rk3399) Some images are invalid
> > > > > 37: dm: core: Provide ofnode_find_subnode_unit()
> > > > > aarch64: (for 1/1 boards) all +324.0 bss +32.0 spl/u-boot-spl:all +16.0 spl/u-boot-spl:text +16.0 text +292.0
> > > > > firefly-rk3399 : all +324 bss +32 spl/u-boot-spl:all +16 spl/u-boot-spl:text +16 text +292
> > > > > u-boot: add: 6/-1, grow: 4/-4 bytes: 516/-224 (292)
> > > > > function old new delta
> > > > > ofnode_name_eq_unit - 160 +160
> > > > > ofnode_find_subnode_unit - 116 +116
> > > > > fit_image_get_data 80 176 +96
> > > > > fit_image_get_emb_data - 84 +84
> > > > > ofnode_write_prop 224 236 +12
> > > > > ofnode_add_subnode 232 244 +12
> > > > > abuf_init_const - 12 +12
> > > > > abuf_init - 12 +12
> > > > > abuf_addr - 8 +8
> > > > > fit_image_print 780 784 +4
> > > > > image_locate_script 696 692 -4
> > > > > fit_image_load 1584 1580 -4
> > > > > fit_image_verify 176 164 -12
> > > > > ofnode_find_subnode 140 116 -24
> > > > > fit_image_get_data_and_size 180 - -180
> > > > > spl-u-boot-spl: add: 3/-1, grow: 0/-1 bytes: 108/-92 (16)
> > > > > function old new delta
> > > > > fit_image_get_emb_data - 84 +84
> > > > > abuf_init_const - 12 +12
> > > > > abuf_init - 12 +12
> > > > > load_simple_fit 580 568 -12
> > > > > fit_image_get_data 80 - -80
> > > >
> > > > Yes, that's the whole series, so not related to this change.
> > >
> > > Yes, that's the whole series including this change, so it's related to
> > > this change.
> >
> > Right, but it is due to ofnode_find_subnode(), etc.
> >
> > >
> > > > I elected to have two versions of ofnode_find_subnode() to avoid the
> > > > size growth in the previous version. But the cost is larger size
> > > > growth when OF_LIVE is used.
> > > >
> > > > Without OF_LIVE, the size growth is tiny.
> > >
> > > And even worse in SPL, somehow. But you want more OF_LIVE users, not
> > > less, yes?
> >
> > Well, OF_LIVE is always quite a bit larger, at least at the moment. It
> > has both Linux's of_access stuff and libfdt. It's not OF_LIVE I am
> > bothered about, but I do want people using ofnode. Unfortunately
> > people still send patches which use libfdt directly.
> >
> > >
> > > > So...what to do?
> > >
> > > Well, if you drop the abuf changes for now, SPL won't change at all for
> > > most platforms and that'll be an improvement.
> >
> > Yes, I'll look at that. This is one of many examples where I have a
> > problem and realise that we need a nicer way of dealing with it, then
> > implement it in the series, but then the series loses focus. So then I
> > take it out again, then forget about it until next time, but I never
> > actually make the change.
>
> I hate to start to derail this, but refactor for "nicer code" is very
> much subjective. Especially when it also grows the code (and it's not
> clear that wider usage would result in shrinkage). So yes, this really
> needs to be put aside and also part of why I keep asking for one thing
> at a time.
Yes I very much agree with this.
>
> > > And I'm going to keep complaining about size growth here because a
> > > non-trivial subset of users just wants things to boot quickly and be
> > > small.
> >
> > Yes, you won't get any complaints from me on that. I did propose some
> > automated checking a few years back, but it never went anywhere.
>
> It be great if buildman size comparison had some way to csv the output.
> That's what's missing imo from being able to have some automation or
> even just nicer tooling.
What sort of tooling could we have? I would like something in CI which
reports code-size changes in a useful way, perhaps failing if the
delta is too large for more than x boards. WDYT?
Regards,
Simon
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf
2025-01-10 19:23 ` Simon Glass
@ 2025-01-10 21:15 ` Tom Rini
0 siblings, 0 replies; 60+ messages in thread
From: Tom Rini @ 2025-01-10 21:15 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Alexander Dahl, Alexander Kochetkov,
Alexander Sverdlin, Bin Meng, Caleb Connolly, Devarsh Thakkar,
Heinrich Schuchardt, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Janne Grunau, Julien Masson, Laurent Pinchart,
Leo Yu-Chi Liang, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Maxim Moskalets, Nikhil M Jain,
Paul-Erwan Rio, Raymond Mao, Roman Stratiienko, Sughosh Ganu
[-- Attachment #1: Type: text/plain, Size: 1105 bytes --]
On Fri, Jan 10, 2025 at 12:23:42PM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Fri, 10 Jan 2025 at 09:17, Tom Rini <trini@konsulko.com> wrote:
[snip]
> > It be great if buildman size comparison had some way to csv the output.
> > That's what's missing imo from being able to have some automation or
> > even just nicer tooling.
>
> What sort of tooling could we have? I would like something in CI which
> reports code-size changes in a useful way, perhaps failing if the
> delta is too large for more than x boards. WDYT?
I don't want it making automated decisions (for example when platforms
enable features they grow, a lot, on purpose) but generating an output
that can be imported elsewhere would be good. I want something that can
be thrown in a database (or added to a database). Setting aside the
challenge of LTO, and looking at what I see in a recent before/after, it
would be good to query and see that for ~60 platforms mpc8xxx_gpio_probe
grew by 44 bytes. Which platforms that is only matters if it's not clear
from the log which commit that's likely from.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 18/33] boot: Use fit_image_get_data() to get data
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (16 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 17/33] boot: Update fit_image_get_emb_data to use abuf Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 19/33] boot: Update fit_image_get_data() to use abuf Simon Glass
` (16 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Alexander Sverdlin, Bin Meng, Devarsh Thakkar,
Heinrich Schuchardt, Ilias Apalodimas, Janne Grunau,
Lukasz Majewski, Marek Vasut, Matthew Garrett,
Mattijs Korpershoek, Nikhil M Jain, Sughosh Ganu, Tom Rini
Use this function instead of fit_image_get_emb_data() data, since it
works will FITs that use external data.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Fix unbalanced {}
arch/arm/cpu/armv8/sec_firmware.c | 12 ++++++------
arch/arm/mach-k3/r5/sysfw-loader.c | 9 +--------
arch/x86/lib/bootm.c | 9 +++------
common/splash_source.c | 20 ++++++--------------
common/update.c | 2 +-
5 files changed, 17 insertions(+), 35 deletions(-)
diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
index e768859acfb..e3f8a6dcd60 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -83,6 +83,8 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
{
phys_addr_t sec_firmware_loadable_addr = 0;
int conf_node_off, ld_node_off, images;
+ const void *data;
+ size_t size;
const char *name, *str, *type;
ulong load;
int len;
@@ -112,8 +114,6 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
for (str = name; str && ((str - name) < len);
str = strchr(str, '\0') + 1) {
- struct abuf buf;
-
printf("%s: '%s'\n", type, str);
ld_node_off = fdt_subnode_offset(sec_firmware_img, images, str);
if (ld_node_off < 0) {
@@ -128,8 +128,8 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
return -EINVAL;
}
- if (fit_image_get_emb_data(sec_firmware_img, ld_node_off,
- &buf)) {
+ if (fit_image_get_data(sec_firmware_img, ld_node_off,
+ &data, &size)) {
printf("SEC Loadable: Can't get subimage data/size");
return -ENOENT;
}
@@ -147,9 +147,9 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
/* Copy loadable to secure memory and flush dcache */
debug("%s copied to address 0x%p\n",
FIT_LOADABLE_PROP, (void *)sec_firmware_loadable_addr);
- memcpy((void *)sec_firmware_loadable_addr, buf.data, buf.size);
+ memcpy((void *)sec_firmware_loadable_addr, data, size);
flush_dcache_range(sec_firmware_loadable_addr,
- sec_firmware_loadable_addr + buf.size);
+ sec_firmware_loadable_addr + size);
/* Populate loadable address only for Trusted OS */
if (!strcmp(str, "trustedOS@1")) {
diff --git a/arch/arm/mach-k3/r5/sysfw-loader.c b/arch/arm/mach-k3/r5/sysfw-loader.c
index 1895fb385a5..f0ad7559b97 100644
--- a/arch/arm/mach-k3/r5/sysfw-loader.c
+++ b/arch/arm/mach-k3/r5/sysfw-loader.c
@@ -117,14 +117,7 @@ static int fit_get_data_by_name(const void *fit, int images, const char *name,
if (node_offset < 0)
return -ENOENT;
- ret = fit_image_get_emb_data(fit, node_offset, &buf);
- if (ret)
- return ret;
-
- *addr = buf.data;
- *size = buf.size;
-
- return 0;
+ return fit_image_get_data(fit, node_offset, addr, size);
}
static void k3_start_system_controller(int rproc_id, bool rproc_loaded,
diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c
index 16980c15081..2a7933cdaf8 100644
--- a/arch/x86/lib/bootm.c
+++ b/arch/x86/lib/bootm.c
@@ -106,16 +106,13 @@ static int boot_prep_linux(struct bootm_headers *images)
is_zimage = 1;
#if defined(CONFIG_FIT)
} else if (images->fit_uname_os && is_zimage) {
- struct abuf buf;
-
- ret = fit_image_get_emb_data(images->fit_hdr_os,
- images->fit_noffset_os, &buf);
+ ret = fit_image_get_data(images->fit_hdr_os,
+ images->fit_noffset_os,
+ (const void **)&data, &len);
if (ret) {
puts("Can't get image data/size!\n");
goto error;
}
- data = buf.data;
- len = buf.size;
is_zimage = 1;
#endif
}
diff --git a/common/splash_source.c b/common/splash_source.c
index 37648b8b9f3..2df78a4f2d7 100644
--- a/common/splash_source.c
+++ b/common/splash_source.c
@@ -347,6 +347,8 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr)
int res;
int node_offset;
const char *splash_file;
+ const void *internal_splash_data;
+ size_t internal_splash_size;
int external_splash_addr;
int external_splash_size;
bool is_splash_external = false;
@@ -354,7 +356,6 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr)
const u32 *fit_header;
u32 fit_size;
const size_t header_size = sizeof(struct legacy_img_hdr);
- struct abuf buf;
/* Read in image header */
res = splash_storage_read_raw(location, bmp_load_addr, header_size);
@@ -394,19 +395,10 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr)
}
/* Extract the splash data from FIT */
- /* 1. Test if splash is in FIT internal data. */
- if (!fit_image_get_emb_data(fit_header, node_offset, &buf)) {
- memmove((void *)(uintptr_t)bmp_load_addr, buf.data, buf.size);
- /* 2. Test if splash is in FIT external data with fixed position. */
- } else if (!fit_image_get_data_position(fit_header, node_offset, &external_splash_addr))
- is_splash_external = true;
- /* 3. Test if splash is in FIT external data with offset. */
- else if (!fit_image_get_data_offset(fit_header, node_offset, &external_splash_addr)) {
- /* Align data offset to 4-byte boundary */
- fit_size = ALIGN(fdt_totalsize(fit_header), 4);
- /* External splash offset means the offset by end of FIT header */
- external_splash_addr += location->offset + fit_size;
- is_splash_external = true;
+ if (!fit_image_get_data(fit_header, node_offset, &internal_splash_data,
+ &internal_splash_size)) {
+ memmove((void *)(uintptr_t)bmp_load_addr, internal_splash_data,
+ internal_splash_size);
} else {
printf("Failed to get splash image from FIT\n");
return -ENODATA;
diff --git a/common/update.c b/common/update.c
index d149ca18e78..6801b49479d 100644
--- a/common/update.c
+++ b/common/update.c
@@ -200,7 +200,7 @@ static int update_fit_getparams(const void *fit, int noffset, ulong *addr,
{
const void *data;
- if (fit_image_get_emb_data(fit, noffset, &data, (size_t *)size))
+ if (fit_image_get_data(fit, noffset, &data, (size_t *)size))
return 1;
if (fit_image_get_load(fit, noffset, (ulong *)fladdr))
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 19/33] boot: Update fit_image_get_data() to use abuf
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (17 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 18/33] boot: Use fit_image_get_data() to get data Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 20/33] test: Fix inpected typo in upl test Simon Glass
` (15 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Ahelenia Ziemiańska, Alexander Sverdlin,
Bin Meng, Devarsh Thakkar, Dmitry Gerasimov, Dragan Simic,
Heinrich Schuchardt, Igor Opaniuk, Ilias Apalodimas, Janne Grunau,
Julien Masson, Laurent Pinchart, Lukasz Majewski, Marek Vasut,
Matthew Garrett, Mattijs Korpershoek, Maxim Moskalets,
Michael Walle, Nikhil M Jain, Paul-Erwan Rio, Peter Korsgaard,
Raymond Mao, Roman Stratiienko, Sughosh Ganu, Tom Rini
This function uses separate arguments for data and size. Use the new
abuf instead, so that they are paired and in one place. In some cases it
also saves an argument, thus potentially reducing code size.
Move the prototype to the header file while here.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
arch/arm/cpu/armv8/sec_firmware.c | 11 +++--
arch/arm/mach-k3/r5/sysfw-loader.c | 9 +++-
arch/x86/lib/bootm.c | 7 ++-
boot/image-board.c | 11 +++--
boot/image-fit.c | 71 +++++++++++-------------------
cmd/ximg.c | 13 +++---
common/splash_source.c | 9 ++--
common/update.c | 7 +--
include/image.h | 19 +++++++-
lib/abuf.c | 11 ++++-
tools/fit_image.c | 8 ++--
11 files changed, 93 insertions(+), 83 deletions(-)
diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
index e3f8a6dcd60..555ab50625a 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -83,8 +83,6 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
{
phys_addr_t sec_firmware_loadable_addr = 0;
int conf_node_off, ld_node_off, images;
- const void *data;
- size_t size;
const char *name, *str, *type;
ulong load;
int len;
@@ -114,6 +112,8 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
for (str = name; str && ((str - name) < len);
str = strchr(str, '\0') + 1) {
+ struct abuf buf;
+
printf("%s: '%s'\n", type, str);
ld_node_off = fdt_subnode_offset(sec_firmware_img, images, str);
if (ld_node_off < 0) {
@@ -128,8 +128,7 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
return -EINVAL;
}
- if (fit_image_get_data(sec_firmware_img, ld_node_off,
- &data, &size)) {
+ if (fit_image_get_data(sec_firmware_img, ld_node_off, &buf)) {
printf("SEC Loadable: Can't get subimage data/size");
return -ENOENT;
}
@@ -147,9 +146,9 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
/* Copy loadable to secure memory and flush dcache */
debug("%s copied to address 0x%p\n",
FIT_LOADABLE_PROP, (void *)sec_firmware_loadable_addr);
- memcpy((void *)sec_firmware_loadable_addr, data, size);
+ memcpy((void *)sec_firmware_loadable_addr, buf.data, buf.size);
flush_dcache_range(sec_firmware_loadable_addr,
- sec_firmware_loadable_addr + size);
+ sec_firmware_loadable_addr + buf.size);
/* Populate loadable address only for Trusted OS */
if (!strcmp(str, "trustedOS@1")) {
diff --git a/arch/arm/mach-k3/r5/sysfw-loader.c b/arch/arm/mach-k3/r5/sysfw-loader.c
index f0ad7559b97..ca98d310450 100644
--- a/arch/arm/mach-k3/r5/sysfw-loader.c
+++ b/arch/arm/mach-k3/r5/sysfw-loader.c
@@ -117,7 +117,14 @@ static int fit_get_data_by_name(const void *fit, int images, const char *name,
if (node_offset < 0)
return -ENOENT;
- return fit_image_get_data(fit, node_offset, addr, size);
+ ret = fit_image_get_data(fit, node_offset, &buf);
+ if (ret)
+ return ret;
+
+ *addr = buf.data;
+ *size = buf.size;
+
+ return 0;
}
static void k3_start_system_controller(int rproc_id, bool rproc_loaded,
diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c
index 2a7933cdaf8..bbd8ee7870f 100644
--- a/arch/x86/lib/bootm.c
+++ b/arch/x86/lib/bootm.c
@@ -106,13 +106,16 @@ static int boot_prep_linux(struct bootm_headers *images)
is_zimage = 1;
#if defined(CONFIG_FIT)
} else if (images->fit_uname_os && is_zimage) {
+ struct abuf buf;
+
ret = fit_image_get_data(images->fit_hdr_os,
- images->fit_noffset_os,
- (const void **)&data, &len);
+ images->fit_noffset_os, &buf);
if (ret) {
puts("Can't get image data/size!\n");
goto error;
}
+ data = buf.data;
+ len = buf.size;
is_zimage = 1;
#endif
}
diff --git a/boot/image-board.c b/boot/image-board.c
index 8e6b2974dcf..f8083fcb3f9 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -964,9 +964,7 @@ int image_locate_script(void *buf, int size, const char *fit_uname,
const char *confname, char **datap, uint *lenp)
{
const struct legacy_img_hdr *hdr;
- const void *fit_data;
const void *fit_hdr;
- size_t fit_len;
int noffset;
int verify;
ulong len;
@@ -1024,6 +1022,8 @@ int image_locate_script(void *buf, int size, const char *fit_uname,
if (!IS_ENABLED(CONFIG_FIT)) {
goto exit_image_format;
} else {
+ struct abuf abuf;
+
fit_hdr = buf;
if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) {
puts("Bad FIT image format\n");
@@ -1087,14 +1087,13 @@ fallback:
}
/* get script subimage data address and length */
- if (fit_image_get_data(fit_hdr, noffset, &fit_data,
- &fit_len)) {
+ if (fit_image_get_data(fit_hdr, noffset, &abuf)) {
puts("Could not find script subimage data\n");
return 1;
}
- data = (u32 *)fit_data;
- len = (ulong)fit_len;
+ data = abuf.data;
+ len = abuf.size;
}
break;
default:
diff --git a/boot/image-fit.c b/boot/image-fit.c
index e9b4152eb84..8e69a3e1897 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -475,9 +475,8 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
{
char *desc;
uint8_t type, arch, os, comp = IH_COMP_NONE;
- size_t size;
ulong load, entry;
- const void *data;
+ struct abuf buf;
int noffset;
int ndepth;
int ret;
@@ -510,24 +509,21 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
fit_image_get_comp(fit, image_noffset, &comp);
printf("%s Compression: %s\n", p, genimg_get_comp_name(comp));
- ret = fit_image_get_data(fit, image_noffset, &data, &size);
+ ret = fit_image_get_data(fit, image_noffset, &buf);
if (!tools_build()) {
printf("%s Data Start: ", p);
- if (ret) {
+ if (ret)
printf("unavailable\n");
- } else {
- void *vdata = (void *)data;
-
- printf("0x%08lx\n", (ulong)map_to_sysmem(vdata));
- }
+ else
+ printf("0x%08lx\n", abuf_addr(&buf));
}
printf("%s Data Size: ", p);
if (ret)
printf("unavailable\n");
else
- genimg_print_size(size);
+ genimg_print_size(buf.size);
/* Remaining, type dependent properties */
if ((type == IH_TYPE_KERNEL) || (type == IH_TYPE_STANDALONE) ||
@@ -1017,24 +1013,7 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
return 0;
}
-/**
- * fit_image_get_data - get data and its size including
- * both embedded and external data
- * @fit: pointer to the FIT format image header
- * @noffset: component image node offset
- * @data: double pointer to void, will hold data property's data address
- * @size: pointer to size_t, will hold data property's data size
- *
- * fit_image_get_data() finds data and its size including
- * both embedded and external data. If the property is found
- * its data start address and size are returned to the caller.
- *
- * returns:
- * 0, on success
- * otherwise, on failure
- */
-int fit_image_get_data(const void *fit, int noffset, const void **data,
- size_t *size)
+int fit_image_get_data(const void *fit, int noffset, struct abuf *buf)
{
bool external_data = false;
int offset;
@@ -1056,16 +1035,10 @@ int fit_image_get_data(const void *fit, int noffset, const void **data,
if (external_data) {
debug("External Data\n");
ret = fit_image_get_data_size(fit, noffset, &len);
- if (!ret) {
- *data = fit + offset;
- *size = len;
- }
+ if (!ret)
+ abuf_init_const(buf, fit + offset, len);
} else {
- struct abuf buf;
-
- ret = fit_image_get_emb_data(fit, noffset, &buf);
- *data = buf.data;
- *size = buf.size;
+ ret = fit_image_get_emb_data(fit, noffset, buf);
}
return ret;
@@ -1410,9 +1383,8 @@ error:
int fit_image_verify(const void *fit, int image_noffset)
{
const char *name = fit_get_name(fit, image_noffset, NULL);
- const void *data;
- size_t size;
char *err_msg = "";
+ struct abuf buf;
if (IS_ENABLED(CONFIG_FIT_SIGNATURE) && strchr(name, '@')) {
/*
@@ -1423,13 +1395,13 @@ int fit_image_verify(const void *fit, int image_noffset)
goto err;
}
/* Get image data and data length */
- if (fit_image_get_data(fit, image_noffset, &data, &size)) {
+ if (fit_image_get_data(fit, image_noffset, &buf)) {
err_msg = "Can't get image data/size";
goto err;
}
return fit_image_verify_with_data(fit, image_noffset, gd_fdt_blob(),
- data, size);
+ buf.data, buf.size);
err:
printf("error!\n%s in '%s' image node\n", err_msg,
@@ -1739,8 +1711,8 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
const char *kfdt_name;
int kfdt_noffset, compat_noffset;
const char *cur_fdt_compat;
+ struct abuf buf;
int len;
- size_t sz;
int i;
if (ndepth > 1)
@@ -1772,10 +1744,11 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
}
/* search in this config's kernel FDT */
- if (fit_image_get_data(fit, kfdt_noffset, &fdt, &sz)) {
+ if (fit_image_get_data(fit, kfdt_noffset, &buf)) {
debug("Failed to get fdt \"%s\".\n", kfdt_name);
continue;
}
+ fdt = buf.data;
compat_noffset = 0; /* search kFDT under root node */
}
@@ -1923,6 +1896,7 @@ int fit_conf_get_prop_node(const void *fit, int noffset, const char *prop_name,
static int fit_get_data_tail(const void *fit, int noffset,
const void **data, size_t *size)
{
+ struct abuf buf;
char *desc;
if (noffset < 0)
@@ -1931,12 +1905,15 @@ static int fit_get_data_tail(const void *fit, int noffset,
if (!fit_image_verify(fit, noffset))
return -EINVAL;
- if (fit_image_get_data(fit, noffset, data, size))
+ if (fit_image_get_data(fit, noffset, &buf))
return -ENOENT;
if (!fit_get_desc(fit, noffset, &desc))
printf("%s\n", desc);
+ *data = buf.data;
+ *size = buf.size;
+
return 0;
}
@@ -2047,6 +2024,7 @@ int fit_image_load(struct bootm_headers *images, ulong addr,
const char *fit_uname;
const char *fit_uname_config;
const char *fit_base_uname_config;
+ struct abuf abuf;
const void *fit;
void *buf;
void *loadbuf;
@@ -2187,12 +2165,15 @@ int fit_image_load(struct bootm_headers *images, ulong addr,
bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL_OK);
/* get image data address and length */
- if (fit_image_get_data(fit, noffset, (const void **)&buf, &size)) {
+ if (fit_image_get_data(fit, noffset, &abuf)) {
printf("Could not find %s subimage data!\n", prop_name);
bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA);
return -ENOENT;
}
+ buf = abuf.data;
+ size = abuf.size;
+
/* Decrypt data before uncompress/move */
if (IS_ENABLED(CONFIG_FIT_CIPHER) && IMAGE_ENABLE_DECRYPT) {
puts(" Decrypting Data ... ");
diff --git a/cmd/ximg.c b/cmd/ximg.c
index 29d7c3279b3..ad85806fdcb 100644
--- a/cmd/ximg.c
+++ b/cmd/ximg.c
@@ -44,8 +44,6 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
const char *uname = NULL;
const void* fit_hdr;
int noffset;
- const void *fit_data;
- size_t fit_len;
#endif
#ifdef CONFIG_GZIP
uint unc_len = CONFIG_SYS_XIMG_LEN;
@@ -122,7 +120,9 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
break;
#endif
#if defined(CONFIG_FIT)
- case IMAGE_FORMAT_FIT:
+ case IMAGE_FORMAT_FIT: {
+ struct abuf buf;
+
if (uname == NULL) {
puts("No FIT subimage unit name\n");
return 1;
@@ -161,7 +161,7 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
}
/* get subimage/external data address and length */
- if (fit_image_get_data(fit_hdr, noffset, &fit_data, &fit_len)) {
+ if (fit_image_get_data(fit_hdr, noffset, &buf)) {
puts("Could not find script subimage data\n");
return 1;
}
@@ -169,9 +169,10 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
if (fit_image_get_comp(fit_hdr, noffset, &comp))
comp = IH_COMP_NONE;
- data = (ulong)fit_data;
- len = (ulong)fit_len;
+ data = abuf_addr(&buf);
+ len = buf.size;
break;
+ }
#endif
default:
puts("Invalid image type for imxtract\n");
diff --git a/common/splash_source.c b/common/splash_source.c
index 2df78a4f2d7..95a88731c31 100644
--- a/common/splash_source.c
+++ b/common/splash_source.c
@@ -347,8 +347,6 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr)
int res;
int node_offset;
const char *splash_file;
- const void *internal_splash_data;
- size_t internal_splash_size;
int external_splash_addr;
int external_splash_size;
bool is_splash_external = false;
@@ -356,6 +354,7 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr)
const u32 *fit_header;
u32 fit_size;
const size_t header_size = sizeof(struct legacy_img_hdr);
+ struct abuf buf;
/* Read in image header */
res = splash_storage_read_raw(location, bmp_load_addr, header_size);
@@ -395,10 +394,8 @@ static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr)
}
/* Extract the splash data from FIT */
- if (!fit_image_get_data(fit_header, node_offset, &internal_splash_data,
- &internal_splash_size)) {
- memmove((void *)(uintptr_t)bmp_load_addr, internal_splash_data,
- internal_splash_size);
+ if (!fit_image_get_emb_data(fit_header, node_offset, &buf)) {
+ memmove((void *)(uintptr_t)bmp_load_addr, buf.data, buf.size);
} else {
printf("Failed to get splash image from FIT\n");
return -ENODATA;
diff --git a/common/update.c b/common/update.c
index 6801b49479d..6d0b979cc97 100644
--- a/common/update.c
+++ b/common/update.c
@@ -6,6 +6,7 @@
* Bartlomiej Sieka <tur@semihalf.com>
*/
+#include <abuf.h>
#include <cpu_func.h>
#include <image.h>
#include <linux/printk.h>
@@ -198,15 +199,15 @@ static int update_flash(ulong addr_source, ulong addr_first, ulong size)
static int update_fit_getparams(const void *fit, int noffset, ulong *addr,
ulong *fladdr, ulong *size)
{
- const void *data;
+ struct abuf buf;
- if (fit_image_get_data(fit, noffset, &data, (size_t *)size))
+ if (fit_image_get_data(fit, noffset, &buf))
return 1;
if (fit_image_get_load(fit, noffset, (ulong *)fladdr))
return 1;
- *addr = (ulong)data;
+ *addr = abuf_addr(&buf);
return 0;
}
diff --git a/include/image.h b/include/image.h
index 29ac7a69ae9..19f5a7b2d83 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1220,8 +1220,23 @@ int fit_image_get_data_position(const void *fit, int noffset,
int fit_image_get_data_size(const void *fit, int noffset, int *data_size);
int fit_image_get_data_size_unciphered(const void *fit, int noffset,
size_t *data_size);
-int fit_image_get_data(const void *fit, int noffset, const void **data,
- size_t *size);
+
+/**
+ * fit_image_get_data() - Get a node's data and size
+ *
+ * @fit: pointer to the FIT format image header
+ * @noffset: component image node offset
+ * @buf: returns data (inited by this function)
+ *
+ * fit_image_get_data_and_size() finds data and its size including
+ * both embedded and external data. If the property is found
+ * its data start address and size are returned to the caller.
+ *
+ * returns:
+ * 0, on success
+ * otherwise, on failure
+ */
+int fit_image_get_data(const void *fit, int noffset, struct abuf *buf);
/**
* fit_get_data_node() - Get verified image data for an image
diff --git a/lib/abuf.c b/lib/abuf.c
index 61adf7fc6b1..1999b2c1f5f 100644
--- a/lib/abuf.c
+++ b/lib/abuf.c
@@ -21,18 +21,25 @@ void abuf_set(struct abuf *abuf, void *data, size_t size)
abuf->size = size;
}
-#ifndef USE_HOSTCC
void abuf_map_sysmem(struct abuf *abuf, ulong addr, size_t size)
{
+#ifdef USE_HOSTCC
+ abuf_set(abuf, (void *)addr, size);
+#else
abuf_set(abuf, map_sysmem(addr, size), size);
+#endif
}
ulong abuf_addr(const struct abuf *abuf)
{
+#ifdef USE_HOSTCC
+ return (ulong)abuf->data;
+#else
return map_to_sysmem(abuf->data);
+#endif
}
-#else
+#ifdef USE_HOSTCC
/* copied from lib/string.c for convenience */
static char *memdup(const void *src, size_t len)
{
diff --git a/tools/fit_image.c b/tools/fit_image.c
index caed8d5f901..dfcfa66afa7 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -15,6 +15,7 @@
#include "imagetool.h"
#include "fit_common.h"
+#include <abuf.h>
#include "mkimage.h"
#include <image.h>
#include <string.h>
@@ -871,19 +872,18 @@ static int fit_image_extract(
int image_noffset,
const char *file_name)
{
- const void *file_data;
- size_t file_size = 0;
+ struct abuf buf;
int ret;
/* get the data address and size of component at offset "image_noffset" */
- ret = fit_image_get_data(fit, image_noffset, &file_data, &file_size);
+ ret = fit_image_get_data(fit, image_noffset, &buf);
if (ret) {
fprintf(stderr, "Could not get component information\n");
return ret;
}
/* save the "file_data" into the file specified by "file_name" */
- return imagetool_save_subimage(file_name, (ulong) file_data, file_size);
+ return imagetool_save_subimage(file_name, abuf_addr(&buf), buf.size);
}
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 20/33] test: Fix inpected typo in upl test
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (18 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 19/33] boot: Update fit_image_get_data() to use abuf Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 21/33] emulation: fdt: Relax condition for OF_HAS_PRIOR_STAGE Simon Glass
` (14 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Tom Rini
Fix a typo in the test comment.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
test/py/tests/test_upl.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/py/tests/test_upl.py b/test/py/tests/test_upl.py
index 3164bda6b71..90125c4dc1b 100644
--- a/test/py/tests/test_upl.py
+++ b/test/py/tests/test_upl.py
@@ -17,7 +17,7 @@ def test_upl_handoff(u_boot_console):
proper and runs a test to check that the parameters are correct.
The entire FIT is loaded into memory in SPL (in upl_load_from_image()) so
- that it can be inpected in upl_test_info_norun
+ that it can be inspected in upl_test_info_norun
"""
cons = u_boot_console
ram = os.path.join(cons.config.build_dir, 'ram.bin')
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 21/33] emulation: fdt: Relax condition for OF_HAS_PRIOR_STAGE
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (19 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 20/33] test: Fix inpected typo in upl test Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 21:40 ` Tom Rini
2025-01-06 14:32 ` [PATCH v2 22/33] emulation: Use bloblist to hold tables Simon Glass
` (13 subsequent siblings)
34 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Caleb Connolly, Jan Kiszka, Jerry Van Baren,
Marek Mojík, Marek Vasut, Raymond Mao, Sumit Garg,
This contributor prefers not to receive mails, Tom Rini
QEMU always gets its devicetree from the OF_BOARD mechanism so we should
not depend on !BLOBLIST here.
The logic of PRIOR_STAGE is quite a mess, unfortuantely. We should rely
only standard passage to receive things from the prior stage. QEMU
should implement standard passage to provide its devicetree to U-Boot
However Linaro has blocked my patch to provide devicetree additions[1],
so little breath should be held in respect of either change.
[1] https://lore.kernel.org/qemu-devel/20210926183410.256484-1-sjg@chromium.org/
Signed-off-by: Simon Glass <sjg@chromium.org>
Fixes: 2b71470628c dts: OF_HAS_PRIOR_STAGE should depend on !BLOBLIST
---
(no changes since v1)
dts/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dts/Kconfig b/dts/Kconfig
index 41a758e83a6..fc25a854708 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -191,7 +191,7 @@ config OF_BOARD
config OF_HAS_PRIOR_STAGE
bool
- depends on !BLOBLIST
+ depends on !BLOBLIST || ARCH_QEMU
help
Indicates that a prior stage of the firmware (before U-Boot proper)
makes use of device tree and this board normally boots with that prior
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 21/33] emulation: fdt: Relax condition for OF_HAS_PRIOR_STAGE
2025-01-06 14:32 ` [PATCH v2 21/33] emulation: fdt: Relax condition for OF_HAS_PRIOR_STAGE Simon Glass
@ 2025-01-06 21:40 ` Tom Rini
2025-01-09 15:01 ` Simon Glass
0 siblings, 1 reply; 60+ messages in thread
From: Tom Rini @ 2025-01-06 21:40 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Caleb Connolly, Jan Kiszka, Jerry Van Baren,
Marek Mojík, Marek Vasut, Raymond Mao, Sumit Garg,
This contributor prefers not to receive mails
[-- Attachment #1: Type: text/plain, Size: 1815 bytes --]
On Mon, Jan 06, 2025 at 07:32:17AM -0700, Simon Glass wrote:
> QEMU always gets its devicetree from the OF_BOARD mechanism so we should
> not depend on !BLOBLIST here.
>
> The logic of PRIOR_STAGE is quite a mess, unfortuantely. We should rely
> only standard passage to receive things from the prior stage. QEMU
> should implement standard passage to provide its devicetree to U-Boot
> However Linaro has blocked my patch to provide devicetree additions[1],
> so little breath should be held in respect of either change.
>
> [1] https://lore.kernel.org/qemu-devel/20210926183410.256484-1-sjg@chromium.org/
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Fixes: 2b71470628c dts: OF_HAS_PRIOR_STAGE should depend on !BLOBLIST
> ---
>
> (no changes since v1)
>
> dts/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dts/Kconfig b/dts/Kconfig
> index 41a758e83a6..fc25a854708 100644
> --- a/dts/Kconfig
> +++ b/dts/Kconfig
> @@ -191,7 +191,7 @@ config OF_BOARD
>
> config OF_HAS_PRIOR_STAGE
> bool
> - depends on !BLOBLIST
> + depends on !BLOBLIST || ARCH_QEMU
> help
> Indicates that a prior stage of the firmware (before U-Boot proper)
> makes use of device tree and this board normally boots with that prior
Since OF_HAS_PRIOR_STAGE has no prompt, it shouldn't have a depends line
either really. I believe 2b71470628c is part of an attempt to say that
if you have bloblist enabled then that's how the previous loader needs
to pass the device tree. But indeed that's not quite how to enforce that
kind of demand, so we should just drop the restriction here (and some
other time fix the "imply OF_HAS_PRIOR_STAGE" to be "select
OF_HAS_PRIOR_STAGE" because it's not prompted, imply is the wrong
keyword.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 21/33] emulation: fdt: Relax condition for OF_HAS_PRIOR_STAGE
2025-01-06 21:40 ` Tom Rini
@ 2025-01-09 15:01 ` Simon Glass
2025-01-09 15:14 ` Tom Rini
0 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-09 15:01 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Caleb Connolly, Jan Kiszka, Jerry Van Baren,
Marek Mojík, Marek Vasut, Raymond Mao, Sumit Garg,
This contributor prefers not to receive mails
Hi Tom,
On Mon, 6 Jan 2025 at 14:40, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Jan 06, 2025 at 07:32:17AM -0700, Simon Glass wrote:
> > QEMU always gets its devicetree from the OF_BOARD mechanism so we should
> > not depend on !BLOBLIST here.
> >
> > The logic of PRIOR_STAGE is quite a mess, unfortuantely. We should rely
> > only standard passage to receive things from the prior stage. QEMU
> > should implement standard passage to provide its devicetree to U-Boot
> > However Linaro has blocked my patch to provide devicetree additions[1],
> > so little breath should be held in respect of either change.
> >
> > [1] https://lore.kernel.org/qemu-devel/20210926183410.256484-1-sjg@chromium.org/
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > Fixes: 2b71470628c dts: OF_HAS_PRIOR_STAGE should depend on !BLOBLIST
> > ---
> >
> > (no changes since v1)
> >
> > dts/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/dts/Kconfig b/dts/Kconfig
> > index 41a758e83a6..fc25a854708 100644
> > --- a/dts/Kconfig
> > +++ b/dts/Kconfig
> > @@ -191,7 +191,7 @@ config OF_BOARD
> >
> > config OF_HAS_PRIOR_STAGE
> > bool
> > - depends on !BLOBLIST
> > + depends on !BLOBLIST || ARCH_QEMU
> > help
> > Indicates that a prior stage of the firmware (before U-Boot proper)
> > makes use of device tree and this board normally boots with that prior
>
> Since OF_HAS_PRIOR_STAGE has no prompt, it shouldn't have a depends line
> either really. I believe 2b71470628c is part of an attempt to say that
> if you have bloblist enabled then that's how the previous loader needs
> to pass the device tree. But indeed that's not quite how to enforce that
> kind of demand, so we should just drop the restriction here (and some
> other time fix the "imply OF_HAS_PRIOR_STAGE" to be "select
> OF_HAS_PRIOR_STAGE" because it's not prompted, imply is the wrong
> keyword.
So, just drop the 'depends' line?
I don't understand all this prior-stage stuff and I believe it is the
Wrong Direction. We should just say where the DT comes from.
Regards,
Simon
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 21/33] emulation: fdt: Relax condition for OF_HAS_PRIOR_STAGE
2025-01-09 15:01 ` Simon Glass
@ 2025-01-09 15:14 ` Tom Rini
2025-01-10 13:38 ` Simon Glass
0 siblings, 1 reply; 60+ messages in thread
From: Tom Rini @ 2025-01-09 15:14 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Caleb Connolly, Jan Kiszka, Jerry Van Baren,
Marek Mojík, Marek Vasut, Raymond Mao, Sumit Garg,
This contributor prefers not to receive mails
[-- Attachment #1: Type: text/plain, Size: 2426 bytes --]
On Thu, Jan 09, 2025 at 08:01:22AM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Mon, 6 Jan 2025 at 14:40, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Jan 06, 2025 at 07:32:17AM -0700, Simon Glass wrote:
> > > QEMU always gets its devicetree from the OF_BOARD mechanism so we should
> > > not depend on !BLOBLIST here.
> > >
> > > The logic of PRIOR_STAGE is quite a mess, unfortuantely. We should rely
> > > only standard passage to receive things from the prior stage. QEMU
> > > should implement standard passage to provide its devicetree to U-Boot
> > > However Linaro has blocked my patch to provide devicetree additions[1],
> > > so little breath should be held in respect of either change.
> > >
> > > [1] https://lore.kernel.org/qemu-devel/20210926183410.256484-1-sjg@chromium.org/
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > Fixes: 2b71470628c dts: OF_HAS_PRIOR_STAGE should depend on !BLOBLIST
> > > ---
> > >
> > > (no changes since v1)
> > >
> > > dts/Kconfig | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/dts/Kconfig b/dts/Kconfig
> > > index 41a758e83a6..fc25a854708 100644
> > > --- a/dts/Kconfig
> > > +++ b/dts/Kconfig
> > > @@ -191,7 +191,7 @@ config OF_BOARD
> > >
> > > config OF_HAS_PRIOR_STAGE
> > > bool
> > > - depends on !BLOBLIST
> > > + depends on !BLOBLIST || ARCH_QEMU
> > > help
> > > Indicates that a prior stage of the firmware (before U-Boot proper)
> > > makes use of device tree and this board normally boots with that prior
> >
> > Since OF_HAS_PRIOR_STAGE has no prompt, it shouldn't have a depends line
> > either really. I believe 2b71470628c is part of an attempt to say that
> > if you have bloblist enabled then that's how the previous loader needs
> > to pass the device tree. But indeed that's not quite how to enforce that
> > kind of demand, so we should just drop the restriction here (and some
> > other time fix the "imply OF_HAS_PRIOR_STAGE" to be "select
> > OF_HAS_PRIOR_STAGE" because it's not prompted, imply is the wrong
> > keyword.
>
> So, just drop the 'depends' line?
Correct.
> I don't understand all this prior-stage stuff and I believe it is the
> Wrong Direction. We should just say where the DT comes from.
Yes, but we're all keen to not rehash years worth of discussion here
either.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 21/33] emulation: fdt: Relax condition for OF_HAS_PRIOR_STAGE
2025-01-09 15:14 ` Tom Rini
@ 2025-01-10 13:38 ` Simon Glass
0 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-10 13:38 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Caleb Connolly, Jan Kiszka, Jerry Van Baren,
Marek Mojík, Marek Vasut, Raymond Mao, Sumit Garg,
This contributor prefers not to receive mails
On Thu, 9 Jan 2025 at 08:14, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jan 09, 2025 at 08:01:22AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 6 Jan 2025 at 14:40, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Jan 06, 2025 at 07:32:17AM -0700, Simon Glass wrote:
> > > > QEMU always gets its devicetree from the OF_BOARD mechanism so we should
> > > > not depend on !BLOBLIST here.
> > > >
> > > > The logic of PRIOR_STAGE is quite a mess, unfortuantely. We should rely
> > > > only standard passage to receive things from the prior stage. QEMU
> > > > should implement standard passage to provide its devicetree to U-Boot
> > > > However Linaro has blocked my patch to provide devicetree additions[1],
> > > > so little breath should be held in respect of either change.
> > > >
> > > > [1] https://lore.kernel.org/qemu-devel/20210926183410.256484-1-sjg@chromium.org/
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > Fixes: 2b71470628c dts: OF_HAS_PRIOR_STAGE should depend on !BLOBLIST
> > > > ---
> > > >
> > > > (no changes since v1)
> > > >
> > > > dts/Kconfig | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/dts/Kconfig b/dts/Kconfig
> > > > index 41a758e83a6..fc25a854708 100644
> > > > --- a/dts/Kconfig
> > > > +++ b/dts/Kconfig
> > > > @@ -191,7 +191,7 @@ config OF_BOARD
> > > >
> > > > config OF_HAS_PRIOR_STAGE
> > > > bool
> > > > - depends on !BLOBLIST
> > > > + depends on !BLOBLIST || ARCH_QEMU
> > > > help
> > > > Indicates that a prior stage of the firmware (before U-Boot proper)
> > > > makes use of device tree and this board normally boots with that prior
> > >
> > > Since OF_HAS_PRIOR_STAGE has no prompt, it shouldn't have a depends line
> > > either really. I believe 2b71470628c is part of an attempt to say that
> > > if you have bloblist enabled then that's how the previous loader needs
> > > to pass the device tree. But indeed that's not quite how to enforce that
> > > kind of demand, so we should just drop the restriction here (and some
> > > other time fix the "imply OF_HAS_PRIOR_STAGE" to be "select
> > > OF_HAS_PRIOR_STAGE" because it's not prompted, imply is the wrong
> > > keyword.
> >
> > So, just drop the 'depends' line?
>
> Correct.
OK
>
> > I don't understand all this prior-stage stuff and I believe it is the
> > Wrong Direction. We should just say where the DT comes from.
>
> Yes, but we're all keen to not rehash years worth of discussion here
> either.
Indeed.
Regards,
SImon
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 22/33] emulation: Use bloblist to hold tables
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (20 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 21/33] emulation: fdt: Relax condition for OF_HAS_PRIOR_STAGE Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-09 22:28 ` Tom Rini
2025-01-06 14:32 ` [PATCH v2 23/33] x86: Create more space for SPL with qemu-x86_64 Simon Glass
` (12 subsequent siblings)
34 siblings, 1 reply; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Anand Moon, Bin Meng, Heinrich Schuchardt,
Ilias Apalodimas, Jerome Forissier, Masahisa Kojima, Michal Simek,
Patrick Delaunay, Patrick Rudolph, Raymond Mao, Sughosh Ganu,
Tom Rini, Tuomas Tynkkynen
QEMU can have its own internal ACPI and SMBIOS tables. At present U-Boot
copies out the SMBIOS tables but points directly to the ACPI ones.
The ACPI tables are not aligned on a 4KB boundary, which means that UPL
cannot use them directly, since it uses a reserved-memory node for the
tables and that it assumed (by EDK2) to be 4KB-aligned.
On x86, QEMU completely takes over the generation of these tables, thus
making it difficult to use any common code.
Adjust the logic to fit within the existing table-generation code. Use a
bloblist always and ensure that the ACPI tables is placed in an aligned
region. Set a size of 8K for QEMU. This does not actually put all the
tables in one place, for QEMU, since it currently adds a pointer to the
tables in QFW.
On ARM, enable bloblist so that SMBIOS tables can be added to the
bloblist.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
arch/x86/lib/tables.c | 2 +-
configs/qemu-x86_64_defconfig | 1 -
configs/qemu_arm64_defconfig | 2 ++
configs/qemu_arm_defconfig | 2 ++
drivers/misc/qfw_acpi.c | 37 ++++++++++++++++++++++++++--
drivers/misc/qfw_smbios.c | 45 ++++++++++++++++-------------------
lib/Kconfig | 1 +
lib/Makefile | 5 ++++
8 files changed, 67 insertions(+), 28 deletions(-)
diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
index 45a70e92763..5fc7dc75377 100644
--- a/arch/x86/lib/tables.c
+++ b/arch/x86/lib/tables.c
@@ -61,7 +61,7 @@ static struct table_info table_list[] = {
#ifdef CONFIG_GENERATE_ACPI_TABLE
{ "acpi", write_acpi_tables, BLOBLISTT_ACPI_TABLES, 0x10000, 0x1000},
#endif
-#if defined(CONFIG_GENERATE_SMBIOS_TABLE) && !defined(CONFIG_QFW_SMBIOS)
+#ifdef CONFIG_GENERATE_SMBIOS_TABLE
{ "smbios", write_smbios_table, BLOBLISTT_SMBIOS_TABLES, 0x1000, 0x100},
#endif
};
diff --git a/configs/qemu-x86_64_defconfig b/configs/qemu-x86_64_defconfig
index f93721fceb8..3c0fc7a34fe 100644
--- a/configs/qemu-x86_64_defconfig
+++ b/configs/qemu-x86_64_defconfig
@@ -33,7 +33,6 @@ CONFIG_LOGF_FUNC=y
CONFIG_SPL_LOG=y
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_PCI_INIT_R=y
-CONFIG_BLOBLIST=y
CONFIG_BLOBLIST_FIXED=y
CONFIG_BLOBLIST_ADDR=0x10000
CONFIG_SPL_NO_BSS_LIMIT=y
diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
index 8dffb91e93b..8a529acfba3 100644
--- a/configs/qemu_arm64_defconfig
+++ b/configs/qemu_arm64_defconfig
@@ -27,6 +27,8 @@ CONFIG_USE_PREBOOT=y
# CONFIG_DISPLAY_CPUINFO is not set
# CONFIG_DISPLAY_BOARDINFO is not set
CONFIG_PCI_INIT_R=y
+CONFIG_BLOBLIST=y
+CONFIG_BLOBLIST_SIZE_RELOC=0x2000
CONFIG_CMD_SMBIOS=y
CONFIG_CMD_BOOTZ=y
CONFIG_CMD_BOOTEFI_SELFTEST=y
diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
index cc4f4540fd5..1dfa946f17d 100644
--- a/configs/qemu_arm_defconfig
+++ b/configs/qemu_arm_defconfig
@@ -28,6 +28,8 @@ CONFIG_USE_PREBOOT=y
# CONFIG_DISPLAY_CPUINFO is not set
# CONFIG_DISPLAY_BOARDINFO is not set
CONFIG_PCI_INIT_R=y
+CONFIG_BLOBLIST=y
+CONFIG_BLOBLIST_SIZE_RELOC=0x2000
CONFIG_CMD_BOOTEFI_SELFTEST=y
CONFIG_CMD_NVEDIT_EFI=y
CONFIG_CMD_DFU=y
diff --git a/drivers/misc/qfw_acpi.c b/drivers/misc/qfw_acpi.c
index 7ffed1e8c02..0d0cf764689 100644
--- a/drivers/misc/qfw_acpi.c
+++ b/drivers/misc/qfw_acpi.c
@@ -7,6 +7,7 @@
#define LOG_CATEGORY UCLASS_QFW
#include <acpi/acpi_table.h>
+#include <bloblist.h>
#include <errno.h>
#include <malloc.h>
#include <mapmem.h>
@@ -160,6 +161,15 @@ ulong write_acpi_tables(ulong addr)
struct bios_linker_entry *entry;
uint32_t size;
struct udevice *dev;
+ struct acpi_ctx *ctx;
+
+ ctx = malloc(sizeof(*ctx));
+ if (!ctx) {
+ printf("error: out of memory for acpi ctx\n");
+ return addr;
+ }
+
+ acpi_setup_ctx(ctx, addr);
ret = qfw_get_dev(&dev);
if (ret) {
@@ -257,6 +267,29 @@ ulong acpi_get_rsdp_addr(void)
return file->addr;
}
+void acpi_write_rsdp(struct acpi_rsdp *rsdp, struct acpi_rsdt *rsdt,
+ struct acpi_xsdt *xsdt)
+{
+ memset(rsdp, 0, sizeof(struct acpi_rsdp));
+
+ memcpy(rsdp->signature, RSDP_SIG, 8);
+ memcpy(rsdp->oem_id, OEM_ID, 6);
+
+ if (rsdt)
+ rsdp->rsdt_address = nomap_to_sysmem(rsdt);
+
+ if (xsdt)
+ rsdp->xsdt_address = nomap_to_sysmem(xsdt);
+
+ rsdp->length = sizeof(struct acpi_rsdp);
+ rsdp->revision = ACPI_RSDP_REV_ACPI_2_0;
+
+ /* Calculate checksums */
+ rsdp->checksum = table_compute_checksum(rsdp, 20);
+ rsdp->ext_checksum = table_compute_checksum(rsdp,
+ sizeof(struct acpi_rsdp));
+}
+
#ifndef CONFIG_X86
static int evt_write_acpi_tables(void)
{
@@ -264,9 +297,9 @@ static int evt_write_acpi_tables(void)
void *ptr;
/* Reserve 64K for ACPI tables, aligned to a 4K boundary */
- ptr = memalign(SZ_4K, SZ_64K);
+ ptr = bloblist_add(BLOBLISTT_ACPI_TABLES, SZ_64K, 12);
if (!ptr)
- return -ENOMEM;
+ return -ENOBUFS;
addr = map_to_sysmem(ptr);
/* Generate ACPI tables */
diff --git a/drivers/misc/qfw_smbios.c b/drivers/misc/qfw_smbios.c
index c3e8c310d00..93c4a80286b 100644
--- a/drivers/misc/qfw_smbios.c
+++ b/drivers/misc/qfw_smbios.c
@@ -5,6 +5,7 @@
#define LOG_CATEGORY UCLASS_QFW
+#include <bloblist.h>
#include <efi_loader.h>
#include <errno.h>
#include <log.h>
@@ -15,6 +16,7 @@
#include <tables_csum.h>
#include <linux/sizes.h>
#include <asm/global_data.h>
+#include <linux/err.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -105,11 +107,10 @@ out:
/**
* qfw_write_smbios_tables() - copy SMBIOS tables from QEMU
*
- * @addr: target buffer
- * @size: size of target buffer
+ * @addr: address of target buffer
* Return: 0 for success, -ve on error
*/
-static int qfw_write_smbios_tables(u8 *addr, uint32_t size)
+ulong write_smbios_table(ulong addr)
{
int ret;
struct udevice *dev;
@@ -143,16 +144,13 @@ static int qfw_write_smbios_tables(u8 *addr, uint32_t size)
table = qfw_load_smbios_table(dev, &table_size,
"etc/smbios/smbios-tables");
- if (table_size + sizeof(struct smbios3_entry) > size) {
- free(table);
- return -ENOMEM;
- }
- memcpy(addr, table, table_size);
+ memcpy((void *)addr, table, table_size);
free(table);
- return 0;
+ return addr + table_size;
}
+#ifndef CONFIG_X86
/**
* qfw_evt_write_smbios_tables() - event handler for copying QEMU SMBIOS tables
*
@@ -160,9 +158,9 @@ static int qfw_write_smbios_tables(u8 *addr, uint32_t size)
*/
static int qfw_evt_write_smbios_tables(void)
{
- phys_addr_t addr;
+ ulong addr, end;
void *ptr;
- int ret;
+
/*
* TODO:
* This size is currently hard coded in lib/efi_loader/efi_smbios.c.
@@ -170,22 +168,21 @@ static int qfw_evt_write_smbios_tables(void)
*/
uint32_t size = SZ_4K;
- /* Reserve 64K for SMBIOS tables, aligned to a 4K boundary */
- ptr = memalign(SZ_4K, size);
- if (!ptr) {
- log_err("Out of memory\n");
- return -ENOMEM;
- }
+ log_debug("qfw_evt_write_smbios_tables bloblist\n");
+ /* Reserve 4K for SMBIOS tables, aligned to a 4K boundary */
+ ptr = bloblist_add(BLOBLISTT_SMBIOS_TABLES, size, 12);
+ if (!ptr)
+ return log_msg_ret("bloblist", -ENOBUFS);
+
addr = map_to_sysmem(ptr);
/* Generate SMBIOS tables */
- ret = qfw_write_smbios_tables(ptr, size);
- if (ret) {
- if (CONFIG_IS_ENABLED(GENERATE_SMBIOS_TABLE)) {
- log_info("Falling back to U-Boot generated SMBIOS tables\n");
- write_smbios_table(addr);
- }
+ end = write_smbios_table(addr);
+ if (IS_ERR_VALUE(end)) {
+ log_warning("SMBIOS: Failed to write (err=%dE)\n", (int)end);
} else {
+ if (end - addr > size)
+ return -ENOMEM;
log_debug("SMBIOS tables copied from QEMU\n");
}
@@ -193,5 +190,5 @@ static int qfw_evt_write_smbios_tables(void)
return 0;
}
-
EVENT_SPY_SIMPLE(EVT_LAST_STAGE_INIT, qfw_evt_write_smbios_tables);
+#endif /* !X86 */
diff --git a/lib/Kconfig b/lib/Kconfig
index 56ffdfa1839..3a1aaf08c70 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -1005,6 +1005,7 @@ menu "System tables"
config BLOBLIST_TABLES
bool "Put tables in a bloblist"
depends on BLOBLIST
+ default y if X86
default y if (ARM && EFI_LOADER && GENERATE_ACPI_TABLE)
default n
help
diff --git a/lib/Makefile b/lib/Makefile
index dbcfa87ebd6..95aafe0c182 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -41,7 +41,12 @@ obj-$(CONFIG_ERRNO_STR) += errno_str.o
obj-$(CONFIG_FIT) += fdtdec_common.o
obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o
+
+# With QEMU the SMBIOS tables come from there, not from U-Boot
+ifndef CONFIG_QFW_SMBIOS
obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
+endif
+
obj-$(CONFIG_SMBIOS_PARSER) += smbios-parser.o
obj-$(CONFIG_IMAGE_SPARSE) += image-sparse.o
obj-y += initcall.o
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 22/33] emulation: Use bloblist to hold tables
2025-01-06 14:32 ` [PATCH v2 22/33] emulation: Use bloblist to hold tables Simon Glass
@ 2025-01-09 22:28 ` Tom Rini
0 siblings, 0 replies; 60+ messages in thread
From: Tom Rini @ 2025-01-09 22:28 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Anand Moon, Bin Meng, Heinrich Schuchardt,
Ilias Apalodimas, Jerome Forissier, Masahisa Kojima, Michal Simek,
Patrick Delaunay, Patrick Rudolph, Raymond Mao, Sughosh Ganu,
Tuomas Tynkkynen
[-- Attachment #1: Type: text/plain, Size: 1287 bytes --]
On Mon, Jan 06, 2025 at 07:32:18AM -0700, Simon Glass wrote:
> QEMU can have its own internal ACPI and SMBIOS tables. At present U-Boot
> copies out the SMBIOS tables but points directly to the ACPI ones.
>
> The ACPI tables are not aligned on a 4KB boundary, which means that UPL
> cannot use them directly, since it uses a reserved-memory node for the
> tables and that it assumed (by EDK2) to be 4KB-aligned.
>
> On x86, QEMU completely takes over the generation of these tables, thus
> making it difficult to use any common code.
>
> Adjust the logic to fit within the existing table-generation code. Use a
> bloblist always and ensure that the ACPI tables is placed in an aligned
> region. Set a size of 8K for QEMU. This does not actually put all the
> tables in one place, for QEMU, since it currently adds a pointer to the
> tables in QFW.
>
> On ARM, enable bloblist so that SMBIOS tables can be added to the
> bloblist.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
Commits that are "big external project is doing things wrong, we need to
work around it" are at best worrisome to me. If re-thinking what you're
doing here isn't right, re-wording what you're doing to not be negative
about other projects is what's needed then.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 23/33] x86: Create more space for SPL with qemu-x86_64
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (21 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 22/33] emulation: Use bloblist to hold tables Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 24/33] pci: video: Set up the pixel-format field Simon Glass
` (11 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Bin Meng, Tom Rini
The space here is quite tight and there is plenty of room in the ROM.
Move SPL earlier to allow for expansion.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
configs/qemu-x86_64_defconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configs/qemu-x86_64_defconfig b/configs/qemu-x86_64_defconfig
index 3c0fc7a34fe..39375c76227 100644
--- a/configs/qemu-x86_64_defconfig
+++ b/configs/qemu-x86_64_defconfig
@@ -7,7 +7,7 @@ CONFIG_MAX_CPUS=2
CONFIG_SPL_DM_SPI=y
CONFIG_DEFAULT_DEVICE_TREE="qemu-x86_i440fx"
CONFIG_SPL_SYS_MALLOC_F_LEN=0x2000
-CONFIG_SPL_TEXT_BASE=0xfffd4000
+CONFIG_SPL_TEXT_BASE=0xfffd0000
CONFIG_DEBUG_UART_BASE=0x3f8
CONFIG_DEBUG_UART_CLOCK=1843200
CONFIG_X86_RUN_64BIT=y
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 24/33] pci: video: Set up the pixel-format field
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (22 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 23/33] x86: Create more space for SPL with qemu-x86_64 Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 25/33] x86: Show an error if video fails Simon Glass
` (10 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Alexander Graf, Alper Nebi Yasak, Anatolij Gustschin,
Devarsh Thakkar, Heiko Schocher, Miquel Raynal, Nikhil M Jain,
Peter Robinson, Tom Rini
Add this information to the handoff structure so that it is available to
U-Boot proper. Update bochs and the video handoff.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
drivers/pci/pci_rom.c | 3 +++
drivers/video/bochs.c | 1 +
drivers/video/video-uclass.c | 1 +
include/video.h | 2 ++
4 files changed, 7 insertions(+)
diff --git a/drivers/pci/pci_rom.c b/drivers/pci/pci_rom.c
index 2753df275ca..3697ad00be2 100644
--- a/drivers/pci/pci_rom.c
+++ b/drivers/pci/pci_rom.c
@@ -347,6 +347,7 @@ int vesa_setup_video_priv(struct vesa_mode_info *vesa, u64 fb,
case 32:
case 24:
uc_priv->bpix = VIDEO_BPP32;
+ uc_priv->format = VIDEO_X8B8G8R8;
break;
case 16:
uc_priv->bpix = VIDEO_BPP16;
@@ -392,6 +393,7 @@ int vesa_setup_video(struct udevice *dev, int (*int15_handler)(void))
uc_priv->ysize = ho->ysize;
uc_priv->line_length = ho->line_length;
uc_priv->bpix = ho->bpix;
+ uc_priv->format = ho->format;
} else {
bootstage_start(BOOTSTAGE_ID_ACCUM_LCD, "vesa display");
ret = dm_pci_run_vga_bios(dev, int15_handler,
@@ -438,6 +440,7 @@ int vesa_setup_video(struct udevice *dev, int (*int15_handler)(void))
ho->ysize = uc_priv->ysize;
ho->line_length = uc_priv->line_length;
ho->bpix = uc_priv->bpix;
+ ho->format = uc_priv->format;
}
return 0;
diff --git a/drivers/video/bochs.c b/drivers/video/bochs.c
index 00e673a4db0..c34bc23f274 100644
--- a/drivers/video/bochs.c
+++ b/drivers/video/bochs.c
@@ -64,6 +64,7 @@ static int bochs_init_fb(struct udevice *dev)
uc_priv->xsize = xsize;
uc_priv->ysize = ysize;
uc_priv->bpix = VIDEO_BPP32;
+ uc_priv->format = VIDEO_X8B8G8R8;
/* setup video mode */
bochs_write(mmio, INDEX_ENABLE, 0);
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index 407fb9fbe20..1e385f12f8e 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -616,6 +616,7 @@ static int video_post_probe(struct udevice *dev)
ho->ysize = priv->ysize;
ho->line_length = priv->line_length;
ho->bpix = priv->bpix;
+ ho->format = priv->format;
}
if (IS_ENABLED(CONFIG_VIDEO_COPY) && plat->copy_base)
diff --git a/include/video.h b/include/video.h
index 0d7b0d27e25..2fe2f73a865 100644
--- a/include/video.h
+++ b/include/video.h
@@ -161,6 +161,7 @@ struct video_ops {
* set by the driver, but if not, the uclass will set it after
* probing
* @bpix: Encoded bits per pixel (enum video_log2_bpp)
+ * @format: Video format (enum video_format)
*/
struct video_handoff {
u64 fb;
@@ -169,6 +170,7 @@ struct video_handoff {
u16 ysize;
u32 line_length;
u8 bpix;
+ u8 format;
};
/** enum colour_idx - the 16 colors supported by consoles */
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 25/33] x86: Show an error if video fails
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (23 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 24/33] pci: video: Set up the pixel-format field Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 26/33] x86: Support jumping to a UPL image Simon Glass
` (9 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Bin Meng, Tom Rini
If video is enabled we expect it to work. Avoid silent failure by adding
a panic if things go wrong.
Expand the SPL malloc-area for qemu-x86_64 to avoid a panic.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
arch/x86/lib/spl.c | 9 +++++++--
configs/qemu-x86_64_defconfig | 2 +-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
index f761fbc8bc3..aad748532d0 100644
--- a/arch/x86/lib/spl.c
+++ b/arch/x86/lib/spl.c
@@ -300,9 +300,14 @@ void spl_board_init(void)
if (CONFIG_IS_ENABLED(VIDEO)) {
struct udevice *dev;
+ int ret;
/* Set up PCI video in SPL if required */
- uclass_first_device_err(UCLASS_PCI, &dev);
- uclass_first_device_err(UCLASS_VIDEO, &dev);
+ ret = uclass_first_device_err(UCLASS_PCI, &dev);
+ if (ret)
+ panic("Failed to set up PCI");
+ ret = uclass_first_device_err(UCLASS_VIDEO, &dev);
+ if (ret)
+ panic("Failed to set up video");
}
}
diff --git a/configs/qemu-x86_64_defconfig b/configs/qemu-x86_64_defconfig
index 39375c76227..2771a4b9e19 100644
--- a/configs/qemu-x86_64_defconfig
+++ b/configs/qemu-x86_64_defconfig
@@ -6,7 +6,7 @@ CONFIG_ENV_SIZE=0x40000
CONFIG_MAX_CPUS=2
CONFIG_SPL_DM_SPI=y
CONFIG_DEFAULT_DEVICE_TREE="qemu-x86_i440fx"
-CONFIG_SPL_SYS_MALLOC_F_LEN=0x2000
+CONFIG_SPL_SYS_MALLOC_F_LEN=0x3000
CONFIG_SPL_TEXT_BASE=0xfffd0000
CONFIG_DEBUG_UART_BASE=0x3f8
CONFIG_DEBUG_UART_CLOCK=1843200
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 26/33] x86: Support jumping to a UPL image
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (24 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 25/33] x86: Show an error if video fails Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 27/33] x86: Enable UPL handoff for SPL Simon Glass
` (8 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Bin Meng, Heinrich Schuchardt, Janne Grunau,
Matthew Garrett, Sughosh Ganu, Tom Rini
Add a function to allow x86 boards to jump to a UPL images. Currently
only 32-bit entry is supported.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
arch/x86/lib/bootm.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c
index bbd8ee7870f..e7a98bf1590 100644
--- a/arch/x86/lib/bootm.c
+++ b/arch/x86/lib/bootm.c
@@ -283,3 +283,14 @@ int do_bootm_linux(int flag, struct bootm_info *bmi)
return boot_jump_linux(images);
}
+
+int arch_upl_jump(ulong entry, const struct abuf *buf)
+{
+ typedef EFIAPI void (*h_func)(void *hoff);
+ h_func func;
+
+ func = (h_func)(ulong)entry;
+ func(buf->data);
+
+ return -EFAULT;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 27/33] x86: Enable UPL handoff for SPL
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (25 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 26/33] x86: Support jumping to a UPL image Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 28/33] x86: Move tables to use SZ macros Simon Glass
` (7 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Bin Meng, Tom Rini
Add the GD_FLG_UPL so that a UPL-handoff is created.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
arch/x86/lib/spl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
index aad748532d0..7a033505101 100644
--- a/arch/x86/lib/spl.c
+++ b/arch/x86/lib/spl.c
@@ -298,6 +298,9 @@ void spl_board_init(void)
if (IS_ENABLED(CONFIG_QEMU))
qemu_chipset_init();
+ if (CONFIG_IS_ENABLED(UPL_OUT))
+ gd->flags |= GD_FLG_UPL;
+
if (CONFIG_IS_ENABLED(VIDEO)) {
struct udevice *dev;
int ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 28/33] x86: Move tables to use SZ macros
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (26 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 27/33] x86: Enable UPL handoff for SPL Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 29/33] x86: Align the SMBIOS table to a 4K boundary Simon Glass
` (6 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Heinrich Schuchardt, Bin Meng, Tom Rini
Update the tables to use linux/sizes rather than open-coped values.
Signed-off-by: Simon Glass <sjg@chromium.org>
Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
Changes in v2:
- Add new patch to move x86 tables to use SZ macros
arch/x86/lib/tables.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
index 5fc7dc75377..9e4f21ae5ad 100644
--- a/arch/x86/lib/tables.c
+++ b/arch/x86/lib/tables.c
@@ -16,6 +16,7 @@
#include <asm/tables.h>
#include <asm/coreboot_tables.h>
#include <linux/log2.h>
+#include <linux/sizes.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -59,10 +60,10 @@ static struct table_info table_list[] = {
* that the calculation of gd->table_end works properly
*/
#ifdef CONFIG_GENERATE_ACPI_TABLE
- { "acpi", write_acpi_tables, BLOBLISTT_ACPI_TABLES, 0x10000, 0x1000},
+ { "acpi", write_acpi_tables, BLOBLISTT_ACPI_TABLES, SZ_64K, SZ_4K},
#endif
#ifdef CONFIG_GENERATE_SMBIOS_TABLE
- { "smbios", write_smbios_table, BLOBLISTT_SMBIOS_TABLES, 0x1000, 0x100},
+ { "smbios", write_smbios_table, BLOBLISTT_SMBIOS_TABLES, SZ_4K, SZ_256},
#endif
};
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 29/33] x86: Align the SMBIOS table to a 4K boundary
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (27 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 28/33] x86: Move tables to use SZ macros Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 30/33] x86: emulation: Enable bloblist Simon Glass
` (5 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Bin Meng, Heinrich Schuchardt, Tom Rini
This isn't strictly needed, but with UPL we use the reserved-memory
nodes to indicate where the SMBIOS table is. Tianocore requires 4KB
alignment on these regions, so it is easier to adjust the alignment
to match.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Use 4K instead of 4KB
arch/x86/lib/tables.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
index 9e4f21ae5ad..44fe80c5224 100644
--- a/arch/x86/lib/tables.c
+++ b/arch/x86/lib/tables.c
@@ -63,7 +63,11 @@ static struct table_info table_list[] = {
{ "acpi", write_acpi_tables, BLOBLISTT_ACPI_TABLES, SZ_64K, SZ_4K},
#endif
#ifdef CONFIG_GENERATE_SMBIOS_TABLE
- { "smbios", write_smbios_table, BLOBLISTT_SMBIOS_TABLES, SZ_4K, SZ_256},
+ /*
+ * align this to a 4K boundary, since UPL adds a reserved-memory node
+ * for it
+ */
+ { "smbios", write_smbios_table, BLOBLISTT_SMBIOS_TABLES, SZ_4K, SZ_4K},
#endif
};
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 30/33] x86: emulation: Enable bloblist
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (28 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 29/33] x86: Align the SMBIOS table to a 4K boundary Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 31/33] dm: core: Clarify behaviour of ofnode_name_eq() Simon Glass
` (4 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Bin Meng, Tom Rini
Add bloblist support so that tables can be generated and placed in a
bloblist, then passed to a payload using UPL
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add new patch to enable bloblist on x86
configs/qemu-x86_64_defconfig | 2 ++
configs/qemu-x86_defconfig | 2 ++
2 files changed, 4 insertions(+)
diff --git a/configs/qemu-x86_64_defconfig b/configs/qemu-x86_64_defconfig
index 2771a4b9e19..792ba06c2b8 100644
--- a/configs/qemu-x86_64_defconfig
+++ b/configs/qemu-x86_64_defconfig
@@ -1,6 +1,7 @@
CONFIG_X86=y
CONFIG_TEXT_BASE=0x1110000
CONFIG_SYS_MALLOC_F_LEN=0x1000
+CONFIG_BLOBLIST_SIZE_RELOC=0x20000
CONFIG_NR_DRAM_BANKS=8
CONFIG_ENV_SIZE=0x40000
CONFIG_MAX_CPUS=2
@@ -33,6 +34,7 @@ CONFIG_LOGF_FUNC=y
CONFIG_SPL_LOG=y
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_PCI_INIT_R=y
+CONFIG_BLOBLIST=y
CONFIG_BLOBLIST_FIXED=y
CONFIG_BLOBLIST_ADDR=0x10000
CONFIG_SPL_NO_BSS_LIMIT=y
diff --git a/configs/qemu-x86_defconfig b/configs/qemu-x86_defconfig
index 947d15cd727..82b6a9e14a3 100644
--- a/configs/qemu-x86_defconfig
+++ b/configs/qemu-x86_defconfig
@@ -1,6 +1,7 @@
CONFIG_X86=y
CONFIG_TEXT_BASE=0xFFF00000
CONFIG_SYS_MALLOC_F_LEN=0x1000
+CONFIG_BLOBLIST_SIZE_RELOC=0x20000
CONFIG_NR_DRAM_BANKS=8
CONFIG_ENV_SIZE=0x40000
CONFIG_MAX_CPUS=2
@@ -23,6 +24,7 @@ CONFIG_LOG=y
CONFIG_LOGF_FUNC=y
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_PCI_INIT_R=y
+CONFIG_BLOBLIST=y
CONFIG_CMD_CPU=y
CONFIG_CMD_BOOTEFI_SELFTEST=y
CONFIG_CMD_NVEDIT_EFI=y
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 31/33] dm: core: Clarify behaviour of ofnode_name_eq()
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (29 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 30/33] x86: emulation: Enable bloblist Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 32/33] dm: core: Provide ofnode_name_eq_unit() to accept a unit address Simon Glass
` (3 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Christian Marangi, Heinrich Schuchardt, Janne Grunau,
Marek Vasut, Pavel Herrmann, Rasmus Villemoes, Sughosh Ganu,
Tom Rini
This function is somewhat ambiguous, so expand the comments and add a
test for the undefined behaviour.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
include/dm/ofnode.h | 7 +++----
test/dm/core.c | 2 ++
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 024ae4cd289..6d6a6fef8ef 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -386,11 +386,10 @@ static inline oftree oftree_from_np(struct device_node *root)
void oftree_dispose(oftree tree);
/**
- * ofnode_name_eq() - Check if the node name is equivalent to a given name
- * ignoring the unit address
+ * ofnode_name_eq() - Check a node name ignoring its unit address
*
- * @node: valid node reference that has to be compared
- * @name: name that has to be compared with the node name
+ * @node: valid node to compared, which may have a unit address
+ * @name: name (without unit address) to compare with the node name
* Return: true if matches, false if it doesn't match
*/
bool ofnode_name_eq(ofnode node, const char *name);
diff --git a/test/dm/core.c b/test/dm/core.c
index c59ffc6f611..d40916ef588 100644
--- a/test/dm/core.c
+++ b/test/dm/core.c
@@ -186,6 +186,8 @@ static int dm_test_compare_node_name(struct unit_test_state *uts)
ut_assert(ofnode_valid(node));
ut_assert(ofnode_name_eq(node, "mmio-bus"));
+ ut_assert(!ofnode_name_eq(node, "mmio-bus@0"));
+
return 0;
}
DM_TEST(dm_test_compare_node_name, UTF_SCAN_PDATA);
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 32/33] dm: core: Provide ofnode_name_eq_unit() to accept a unit address
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (30 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 31/33] dm: core: Clarify behaviour of ofnode_name_eq() Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 14:32 ` [PATCH v2 33/33] dm: core: Provide ofnode_find_subnode_unit() Simon Glass
` (2 subsequent siblings)
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Christian Marangi, Heinrich Schuchardt,
Ilias Apalodimas, Janne Grunau, Jonas Karlman, Marek Vasut,
Pavel Herrmann, Quentin Schulz, Rasmus Villemoes, Sughosh Ganu,
Tom Rini
When a unit-address is provided, use it to match against the node
name.
Since this increases code size, put it into a separate function.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Split this into its own function
drivers/core/ofnode.c | 23 +++++++++++++++++++++++
include/dm/ofnode.h | 14 ++++++++++++++
test/dm/core.c | 18 ++++++++++++++++++
3 files changed, 55 insertions(+)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 5b8be218d3b..529bc132f7d 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -309,6 +309,29 @@ bool ofnode_name_eq(ofnode node, const char *name)
return (strlen(name) == len) && !strncmp(node_name, name, len);
}
+bool ofnode_name_eq_unit(ofnode node, const char *name)
+{
+ const char *node_name, *p;
+ int len;
+
+ assert(ofnode_valid(node));
+
+ node_name = ofnode_get_name(node);
+
+ /* check the whole name */
+ if (!strcmp(node_name, name))
+ return true;
+
+ /* if @name has no unit address, try the node name without it */
+ len = strlen(name);
+ p = strchr(node_name, '@');
+ if (p && !strchr(name, '@') && len == p - node_name &&
+ !strncmp(node_name, name, len))
+ return true;
+
+ return false;
+}
+
int ofnode_read_u8(ofnode node, const char *propname, u8 *outp)
{
const u8 *cell;
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 6d6a6fef8ef..3bb7d3e05e1 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -394,6 +394,20 @@ void oftree_dispose(oftree tree);
*/
bool ofnode_name_eq(ofnode node, const char *name);
+/**
+ * ofnode_name_eq_unit() - Check a node name ignoring its unit address
+ *
+ * This is separate from ofnode_name_eq() to avoid code-size increase for
+ * boards which don't need this function
+ *
+ * @node: valid node to compared, which may have a unit address
+ * @name: name to compare with the node name. If this contains a unit
+ * address, it is matched, otherwise the unit address is ignored
+ * when searching for matches
+ * Return: true if matches, false if it doesn't match
+ */
+bool ofnode_name_eq_unit(ofnode node, const char *name);
+
/**
* ofnode_read_u8() - Read a 8-bit integer from a property
*
diff --git a/test/dm/core.c b/test/dm/core.c
index d40916ef588..959b834576f 100644
--- a/test/dm/core.c
+++ b/test/dm/core.c
@@ -192,6 +192,24 @@ static int dm_test_compare_node_name(struct unit_test_state *uts)
}
DM_TEST(dm_test_compare_node_name, UTF_SCAN_PDATA);
+/* compare node names ignoring the unit address */
+static int dm_test_compare_node_name_unit(struct unit_test_state *uts)
+{
+ ofnode node;
+
+ node = ofnode_path("/mmio-bus@0");
+ ut_assert(ofnode_valid(node));
+ ut_assert(ofnode_name_eq_unit(node, "mmio-bus"));
+
+ ut_assert(ofnode_name_eq_unit(node, "mmio-bus@0"));
+ ut_assert(!ofnode_name_eq_unit(node, "mmio-bus@1"));
+ ut_assert(!ofnode_name_eq_unit(node, "mmio-bu"));
+ ut_assert(!ofnode_name_eq_unit(node, "mmio-buss@0"));
+
+ return 0;
+}
+DM_TEST(dm_test_compare_node_name_unit, UTF_SCAN_PDATA);
+
/* Test that binding with uclass plat setting occurs correctly */
static int dm_test_autobind_uclass_pdata_valid(struct unit_test_state *uts)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 33/33] dm: core: Provide ofnode_find_subnode_unit()
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (31 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 32/33] dm: core: Provide ofnode_name_eq_unit() to accept a unit address Simon Glass
@ 2025-01-06 14:32 ` Simon Glass
2025-01-06 21:42 ` [PATCH v2 00/33] upl: Prerequite patches for updated spec Tom Rini
2025-01-08 18:37 ` Tom Rini
34 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-06 14:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Christian Marangi, Jonas Karlman, Marek Vasut,
Pavel Herrmann, Quentin Schulz, Tom Rini
The ofnode_find_subnode() function currently processes things two
different ways, so the treatment of unit addresses differs depending on
whether OF_LIVE is enabled or not.
Add a new version which uses the ofnode API and add a test to check that
unit addresses can be matched correctly. Leave the old function in place
for the !OF_LIVE case, to avoid a code-size increase, e.g. on
firefly-rk3288
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Split prerequisite patches into their own series
drivers/core/ofnode.c | 29 ++++++++++++++++++++++-------
include/dm/ofnode.h | 12 ++++++++++++
test/dm/ofnode.c | 19 +++++++++++++++++++
3 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 529bc132f7d..c9b03b0f925 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -599,14 +599,9 @@ ofnode ofnode_find_subnode(ofnode node, const char *subnode_name)
log_debug("%s: %s: ", __func__, subnode_name);
if (ofnode_is_np(node)) {
- struct device_node *np = ofnode_to_np(node);
-
- for (np = np->child; np; np = np->sibling) {
- if (!strcmp(subnode_name, np->name))
- break;
- }
- subnode = np_to_ofnode(np);
+ subnode = ofnode_find_subnode_unit(node, subnode_name);
} else {
+ /* special case to avoid code-size increase */
int ooffset = fdt_subnode_offset(ofnode_to_fdt(node),
ofnode_to_offset(node), subnode_name);
subnode = noffset_to_ofnode(node, ooffset);
@@ -617,6 +612,26 @@ ofnode ofnode_find_subnode(ofnode node, const char *subnode_name)
return subnode;
}
+ofnode ofnode_find_subnode_unit(ofnode node, const char *subnode_name)
+{
+ ofnode subnode, found = ofnode_null();
+
+ assert(ofnode_valid(node));
+ log_debug("%s: ", subnode_name);
+
+ ofnode_for_each_subnode(subnode, node) {
+ if (ofnode_name_eq_unit(subnode, subnode_name)) {
+ found = subnode;
+ break;
+ }
+ }
+
+ log_debug("%s\n", ofnode_valid(found) ?
+ ofnode_get_name(found) : "<none>");
+
+ return found;
+}
+
int ofnode_read_u32_array(ofnode node, const char *propname,
u32 *out_values, size_t sz)
{
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 3bb7d3e05e1..74ee6fb4ee2 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -607,6 +607,18 @@ bool ofnode_read_bool(ofnode node, const char *propname);
*/
ofnode ofnode_find_subnode(ofnode node, const char *subnode_name);
+/**
+ * ofnode_find_subnode_unit() - find a named subnode of a parent node
+ *
+ * @node: valid reference to parent node
+ * @subnode_name: name of subnode to find, including any unit address. If the
+ * unit address is omitted, any subnode which matches the name (excluding
+ * any unit address) is returned
+ * Return: reference to subnode (which can be invalid if there is no such
+ * subnode)
+ */
+ofnode ofnode_find_subnode_unit(ofnode node, const char *subnode_name);
+
#if CONFIG_IS_ENABLED(DM_INLINE_OFNODE)
#include <asm/global_data.h>
diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c
index ce996567c3c..521359c9bd7 100644
--- a/test/dm/ofnode.c
+++ b/test/dm/ofnode.c
@@ -1201,6 +1201,25 @@ static int dm_test_ofnode_find_subnode(struct unit_test_state *uts)
}
DM_TEST(dm_test_ofnode_find_subnode, UTF_SCAN_FDT);
+/* check ofnode_find_subnode() with unit addresses */
+static int dm_test_ofnode_find_subnode_unit(struct unit_test_state *uts)
+{
+ ofnode node, subnode;
+
+ node = ofnode_path("/some-bus");
+ ut_assert(ofnode_valid(node));
+ subnode = ofnode_find_subnode_unit(node, "c-test@5");
+ ut_assert(ofnode_valid(subnode));
+ ut_asserteq_str("c-test@5", ofnode_get_name(subnode));
+
+ subnode = ofnode_find_subnode_unit(node, "c-test");
+ ut_assert(ofnode_valid(subnode));
+ ut_asserteq_str("c-test@5", ofnode_get_name(subnode));
+
+ return 0;
+}
+DM_TEST(dm_test_ofnode_find_subnode_unit, UTF_SCAN_FDT);
+
/* test ofnode_find_subnode() on the 'other' tree */
static int dm_test_ofnode_find_subnode_ot(struct unit_test_state *uts)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 00/33] upl: Prerequite patches for updated spec
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (32 preceding siblings ...)
2025-01-06 14:32 ` [PATCH v2 33/33] dm: core: Provide ofnode_find_subnode_unit() Simon Glass
@ 2025-01-06 21:42 ` Tom Rini
2025-01-08 18:37 ` Tom Rini
34 siblings, 0 replies; 60+ messages in thread
From: Tom Rini @ 2025-01-06 21:42 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Ahelenia Ziemiańska, Alex Shumsky,
Alexander Dahl, Alexander Graf, Alexander Kochetkov,
Alexander Sverdlin, Alper Nebi Yasak, Anand Moon,
Anatolij Gustschin, Andy Shevchenko, Bin Meng, Caleb Connolly,
Chen-Yu Tsai, Christian Marangi, Devarsh Thakkar,
Dmitry Gerasimov, Dmitry Rokosov, Dragan Simic, Heiko Schocher,
Heinrich Schuchardt, Hou Zhiqiang, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Jagan Teki, Jan Kiszka, Janne Grunau,
Jerome Forissier, Jerry Van Baren, Jiaxun Yang, Joe Hershberger,
Jonas Karlman, Joshua Watt, Julien Masson, Kever Yang,
Laurent Pinchart, Leo Yu-Chi Liang, Levi Yun, Lukasz Majewski,
Marek Mojík, Marek Vasut, Marek Vasut, Masahisa Kojima,
Matthew Garrett, Mattijs Korpershoek, Maxim Moskalets,
Michael Walle, Michal Simek, Miquel Raynal, Nikhil M Jain,
Patrick Delaunay, Patrick Rudolph, Paul-Erwan Rio, Pavel Herrmann,
Peter Korsgaard, Peter Robinson, Philip Oberfichtner,
Quentin Schulz, Rasmus Villemoes, Raymond Mao, Roman Stratiienko,
Stefan Roese, Stefano Babic, Sughosh Ganu, Sumit Garg,
This contributor prefers not to receive mails, Tuomas Tynkkynen
[-- Attachment #1: Type: text/plain, Size: 929 bytes --]
On Mon, Jan 06, 2025 at 07:31:56AM -0700, Simon Glass wrote:
> The current UPL spec[1] has been tidied up and improved over the last
> year, since U-Boot's original UPL support was written.
>
> This series includes some prerequisite patches needed for the real UPL
> patches. It is split from [2]
>
> [1] git@github.com:UniversalPayload/spec.git commit 3f1450d
> [2] https://patchwork.ozlabs.org/project/uboot/list/?series=438574&state=*
>
> Changes in v2:
> - Fix 'Emable' typo
> - Enable the command in cmd/Kconfig instead
> - Fix unbalanced {}
> - Add new patch to move x86 tables to use SZ macros
> - Use 4K instead of 4KB
> - Add new patch to enable bloblist on x86
> - Split this into its own function
> - Split prerequisite patches into their own series
One thing I don't see in here is how to pass a BLOBLIST_ALLOC from one
stage of U-Boot to another. Or did I overlook something?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v2 00/33] upl: Prerequite patches for updated spec
2025-01-06 14:31 [PATCH v2 00/33] upl: Prerequite patches for updated spec Simon Glass
` (33 preceding siblings ...)
2025-01-06 21:42 ` [PATCH v2 00/33] upl: Prerequite patches for updated spec Tom Rini
@ 2025-01-08 18:37 ` Tom Rini
2025-01-09 12:37 ` Simon Glass
34 siblings, 1 reply; 60+ messages in thread
From: Tom Rini @ 2025-01-08 18:37 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Ahelenia Ziemiańska, Alex Shumsky,
Alexander Dahl, Alexander Graf, Alexander Kochetkov,
Alexander Sverdlin, Alper Nebi Yasak, Anand Moon,
Anatolij Gustschin, Andy Shevchenko, Bin Meng, Caleb Connolly,
Chen-Yu Tsai, Christian Marangi, Devarsh Thakkar,
Dmitry Gerasimov, Dmitry Rokosov, Dragan Simic, Heiko Schocher,
Heinrich Schuchardt, Hou Zhiqiang, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Jagan Teki, Jan Kiszka, Janne Grunau,
Jerome Forissier, Jerry Van Baren, Jiaxun Yang, Joe Hershberger,
Jonas Karlman, Joshua Watt, Julien Masson, Kever Yang,
Laurent Pinchart, Leo Yu-Chi Liang, Levi Yun, Lukasz Majewski,
Marek Mojík, Marek Vasut, Marek Vasut, Masahisa Kojima,
Matthew Garrett, Mattijs Korpershoek, Maxim Moskalets,
Michael Walle, Michal Simek, Miquel Raynal, Nikhil M Jain,
Patrick Delaunay, Patrick Rudolph, Paul-Erwan Rio, Pavel Herrmann,
Peter Korsgaard, Peter Robinson, Philip Oberfichtner,
Quentin Schulz, Rasmus Villemoes, Raymond Mao, Roman Stratiienko,
Stefan Roese, Stefano Babic, Sughosh Ganu, Sumit Garg,
This contributor prefers not to receive mails, Tuomas Tynkkynen
[-- Attachment #1: Type: text/plain, Size: 2770 bytes --]
On Mon, Jan 06, 2025 at 07:31:56AM -0700, Simon Glass wrote:
> The current UPL spec[1] has been tidied up and improved over the last
> year, since U-Boot's original UPL support was written.
>
> This series includes some prerequisite patches needed for the real UPL
> patches. It is split from [2]
>
> [1] git@github.com:UniversalPayload/spec.git commit 3f1450d
> [2] https://patchwork.ozlabs.org/project/uboot/list/?series=438574&state=*
>
> Changes in v2:
> - Fix 'Emable' typo
> - Enable the command in cmd/Kconfig instead
> - Fix unbalanced {}
> - Add new patch to move x86 tables to use SZ macros
> - Use 4K instead of 4KB
> - Add new patch to enable bloblist on x86
> - Split this into its own function
> - Split prerequisite patches into their own series
>
> Raymond Mao (1):
> bloblist: add api to get blob with size
>
> Simon Glass (32):
> bloblist: Drop BSD license
> bloblist: Make BLOBLIST_ALLOC the default
> abuf: Provide a way to get the buffer address
> abuf: Allow use in host tools
> abuf: Provide a constant buffer
> cpu: Provide a way to get the physical-address size
> serial: Support info() method in ns16550 xPL with UPL
> mkimage: Update map_to_sysmem() to match its prototype
> x86: Enable meminfo command
> x86: Show the timestamp counter with bdinfo
> ofnode: Use 4K for a default tree-size
> ofnode: Indicate when out of space in a few places
> ofnode: Update of_add_subnode() to indicate name is alloced
> boot: Rename fit_image_get_data()
> boot: Rename fit_image_get_data_and_size()
> boot: Update fit_image_get_emb_data to use abuf
> boot: Use fit_image_get_data() to get data
> boot: Update fit_image_get_data() to use abuf
> test: Fix inpected typo in upl test
> emulation: fdt: Relax condition for OF_HAS_PRIOR_STAGE
> emulation: Use bloblist to hold tables
> x86: Create more space for SPL with qemu-x86_64
> pci: video: Set up the pixel-format field
> x86: Show an error if video fails
> x86: Support jumping to a UPL image
> x86: Enable UPL handoff for SPL
> x86: Move tables to use SZ macros
> x86: Align the SMBIOS table to a 4K boundary
> x86: emulation: Enable bloblist
> dm: core: Clarify behaviour of ofnode_name_eq()
> dm: core: Provide ofnode_name_eq_unit() to accept a unit address
> dm: core: Provide ofnode_find_subnode_unit()
Setting aside the bloblist parts of this. The x86 parts (and pci: video)
are likely fine. I've commented about abuf now. I'm not keen, but I'm
not nak'ing all of the rename stuff. All of the dm stuff is still you,
and it doesn't look like there's anything behavior changing there, so
that's fine. The rest of the QEMU related stuff needs a think.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v2 00/33] upl: Prerequite patches for updated spec
2025-01-08 18:37 ` Tom Rini
@ 2025-01-09 12:37 ` Simon Glass
0 siblings, 0 replies; 60+ messages in thread
From: Simon Glass @ 2025-01-09 12:37 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Ahelenia Ziemiańska, Alex Shumsky,
Alexander Dahl, Alexander Graf, Alexander Kochetkov,
Alexander Sverdlin, Alper Nebi Yasak, Anand Moon,
Anatolij Gustschin, Andy Shevchenko, Bin Meng, Caleb Connolly,
Chen-Yu Tsai, Christian Marangi, Devarsh Thakkar,
Dmitry Gerasimov, Dmitry Rokosov, Dragan Simic, Heiko Schocher,
Heinrich Schuchardt, Hou Zhiqiang, Hugo Cornelis, Igor Opaniuk,
Ilias Apalodimas, Jagan Teki, Jan Kiszka, Janne Grunau,
Jerome Forissier, Jerry Van Baren, Jiaxun Yang, Joe Hershberger,
Jonas Karlman, Joshua Watt, Julien Masson, Kever Yang,
Laurent Pinchart, Leo Yu-Chi Liang, Levi Yun, Lukasz Majewski,
Marek Mojík, Marek Vasut, Marek Vasut, Masahisa Kojima,
Matthew Garrett, Mattijs Korpershoek, Maxim Moskalets,
Michael Walle, Michal Simek, Miquel Raynal, Nikhil M Jain,
Patrick Delaunay, Patrick Rudolph, Paul-Erwan Rio, Pavel Herrmann,
Peter Korsgaard, Peter Robinson, Philip Oberfichtner,
Quentin Schulz, Rasmus Villemoes, Raymond Mao, Roman Stratiienko,
Stefan Roese, Stefano Babic, Sughosh Ganu, Sumit Garg,
This contributor prefers not to receive mails, Tuomas Tynkkynen
Hi Tom,
On Wed, 8 Jan 2025 at 11:37, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Jan 06, 2025 at 07:31:56AM -0700, Simon Glass wrote:
>
> > The current UPL spec[1] has been tidied up and improved over the last
> > year, since U-Boot's original UPL support was written.
> >
> > This series includes some prerequisite patches needed for the real UPL
> > patches. It is split from [2]
> >
> > [1] git@github.com:UniversalPayload/spec.git commit 3f1450d
> > [2] https://patchwork.ozlabs.org/project/uboot/list/?series=438574&state=*
> >
> > Changes in v2:
> > - Fix 'Emable' typo
> > - Enable the command in cmd/Kconfig instead
> > - Fix unbalanced {}
> > - Add new patch to move x86 tables to use SZ macros
> > - Use 4K instead of 4KB
> > - Add new patch to enable bloblist on x86
> > - Split this into its own function
> > - Split prerequisite patches into their own series
> >
> > Raymond Mao (1):
> > bloblist: add api to get blob with size
> >
> > Simon Glass (32):
> > bloblist: Drop BSD license
> > bloblist: Make BLOBLIST_ALLOC the default
> > abuf: Provide a way to get the buffer address
> > abuf: Allow use in host tools
> > abuf: Provide a constant buffer
> > cpu: Provide a way to get the physical-address size
> > serial: Support info() method in ns16550 xPL with UPL
> > mkimage: Update map_to_sysmem() to match its prototype
> > x86: Enable meminfo command
> > x86: Show the timestamp counter with bdinfo
> > ofnode: Use 4K for a default tree-size
> > ofnode: Indicate when out of space in a few places
> > ofnode: Update of_add_subnode() to indicate name is alloced
> > boot: Rename fit_image_get_data()
> > boot: Rename fit_image_get_data_and_size()
> > boot: Update fit_image_get_emb_data to use abuf
> > boot: Use fit_image_get_data() to get data
> > boot: Update fit_image_get_data() to use abuf
> > test: Fix inpected typo in upl test
> > emulation: fdt: Relax condition for OF_HAS_PRIOR_STAGE
> > emulation: Use bloblist to hold tables
> > x86: Create more space for SPL with qemu-x86_64
> > pci: video: Set up the pixel-format field
> > x86: Show an error if video fails
> > x86: Support jumping to a UPL image
> > x86: Enable UPL handoff for SPL
> > x86: Move tables to use SZ macros
> > x86: Align the SMBIOS table to a 4K boundary
> > x86: emulation: Enable bloblist
> > dm: core: Clarify behaviour of ofnode_name_eq()
> > dm: core: Provide ofnode_name_eq_unit() to accept a unit address
> > dm: core: Provide ofnode_find_subnode_unit()
>
> Setting aside the bloblist parts of this. The x86 parts (and pci: video)
> are likely fine. I've commented about abuf now. I'm not keen, but I'm
> not nak'ing all of the rename stuff. All of the dm stuff is still you,
> and it doesn't look like there's anything behavior changing there, so
> that's fine. The rest of the QEMU related stuff needs a think.
OK, will wait to hear.
Regards,
Simon
^ permalink raw reply [flat|nested] 60+ messages in thread