* [PATCH 01/14] dm: core: Avoid registering an invalid tree in oftree_ensure()
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:17 ` [PATCH 02/14] bootm: Change incorrect 'unsupported' error Simon Glass
` (12 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Marek Vasut, Pavel Herrmann
If the tree is not valid we should not register it. Update the function
to check this first.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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 6bdab6886b6..9a782e7b19f 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -51,7 +51,7 @@ static oftree oftree_ensure(void *fdt)
oftree tree;
int i;
- if (gd->flags & GD_FLG_RELOC) {
+ if (fdt && (gd->flags & GD_FLG_RELOC)) {
i = oftree_find(fdt);
if (i == -1) {
if (oftree_count == CONFIG_OFNODE_MULTI_TREE_MAX) {
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 02/14] bootm: Change incorrect 'unsupported' error
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
2022-09-09 15:17 ` [PATCH 01/14] dm: core: Avoid registering an invalid tree in oftree_ensure() Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:33 ` Heinrich Schuchardt
2022-09-09 15:17 ` [PATCH 03/14] bootm: Avoid returning error codes from command Simon Glass
` (11 subsequent siblings)
13 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Alexandru Gagniuc, Artem Lapkin, Heinrich Schuchardt,
Philippe Reynes, Sean Anderson
At present when bootm fails, it says:
subcommand not supported
and then prints help for the bootm command. This is not very useful, since
generally the error is related to something else, such as fixups failing.
It is quite confusing to see this in a test run.
Change the error and show the error code.
We could update the OS functions to return -ENOSYS when they do not
support the bootm subcommand. But this involves some thought since this is
arch-specific code and proper errno error codes are not always returned.
Also, with the code as is, all required subcommands are of course
supported - a problem would only come if someone added a new one or
removed support for one from an existing OS. Therefore it seems better to
leave that sort of effort for when our bootm tests are improved.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
boot/bootm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/boot/bootm.c b/boot/bootm.c
index f6713807fda..ed6b489c4b3 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -790,7 +790,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
/* Check for unsupported subcommand. */
if (ret) {
- puts("subcommand not supported\n");
+ printf("subcommand failed (err=%d)\n", ret);
return ret;
}
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 02/14] bootm: Change incorrect 'unsupported' error
2022-09-09 15:17 ` [PATCH 02/14] bootm: Change incorrect 'unsupported' error Simon Glass
@ 2022-09-09 15:33 ` Heinrich Schuchardt
2022-09-09 18:20 ` Simon Glass
0 siblings, 1 reply; 19+ messages in thread
From: Heinrich Schuchardt @ 2022-09-09 15:33 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List
Cc: Alexandru Gagniuc, Artem Lapkin, Philippe Reynes, Sean Anderson
Am 9. September 2022 17:17:49 MESZ schrieb Simon Glass <sjg@chromium.org>:
>At present when bootm fails, it says:
>
> subcommand not supported
>
>and then prints help for the bootm command. This is not very useful, since
>generally the error is related to something else, such as fixups failing.
>It is quite confusing to see this in a test run.
>
>Change the error and show the error code.
>
>We could update the OS functions to return -ENOSYS when they do not
>support the bootm subcommand. But this involves some thought since this is
>arch-specific code and proper errno error codes are not always returned.
>Also, with the code as is, all required subcommands are of course
>supported - a problem would only come if someone added a new one or
>removed support for one from an existing OS. Therefore it seems better to
>leave that sort of effort for when our bootm tests are improved.
>
>Signed-off-by: Simon Glass <sjg@chromium.org>
>---
>
> boot/bootm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/boot/bootm.c b/boot/bootm.c
>index f6713807fda..ed6b489c4b3 100644
>--- a/boot/bootm.c
>+++ b/boot/bootm.c
>@@ -790,7 +790,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
>
> /* Check for unsupported subcommand. */
> if (ret) {
>- puts("subcommand not supported\n");
>+ printf("subcommand failed (err=%d)\n", ret);
Return codes are only interpretable by developers. We have a function to convert errno to a string.
For the average user it would be helpful to know which (sub-)command failed especially if this boot command is executed in an automated way.
Best regards
Heinrich
> return ret;
> }
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 02/14] bootm: Change incorrect 'unsupported' error
2022-09-09 15:33 ` Heinrich Schuchardt
@ 2022-09-09 18:20 ` Simon Glass
2022-09-10 6:53 ` [PATCH 02/14] bood Heinrich Schuchardt
0 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2022-09-09 18:20 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: U-Boot Mailing List, Alexandru Gagniuc, Artem Lapkin,
Philippe Reynes, Sean Anderson
Hi Heinrich,
On Fri, 9 Sept 2022 at 09:33, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> Am 9. September 2022 17:17:49 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >At present when bootm fails, it says:
> >
> > subcommand not supported
> >
> >and then prints help for the bootm command. This is not very useful, since
> >generally the error is related to something else, such as fixups failing.
> >It is quite confusing to see this in a test run.
> >
> >Change the error and show the error code.
> >
> >We could update the OS functions to return -ENOSYS when they do not
> >support the bootm subcommand. But this involves some thought since this is
> >arch-specific code and proper errno error codes are not always returned.
> >Also, with the code as is, all required subcommands are of course
> >supported - a problem would only come if someone added a new one or
> >removed support for one from an existing OS. Therefore it seems better to
> >leave that sort of effort for when our bootm tests are improved.
> >
> >Signed-off-by: Simon Glass <sjg@chromium.org>
> >---
> >
> > boot/bootm.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/boot/bootm.c b/boot/bootm.c
> >index f6713807fda..ed6b489c4b3 100644
> >--- a/boot/bootm.c
> >+++ b/boot/bootm.c
> >@@ -790,7 +790,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
> >
> > /* Check for unsupported subcommand. */
> > if (ret) {
> >- puts("subcommand not supported\n");
> >+ printf("subcommand failed (err=%d)\n", ret);
>
> Return codes are only interpretable by developers. We have a function to convert errno to a string.
>
> For the average user it would be helpful to know which (sub-)command failed especially if this boot command is executed in an automated way.
I don't disagree, but:
1. The error strings add to code size, about 5KB or so
2. For devs the error number is much easier to use
3. For bug reports the error number is better too IMO
4. As per the commit message, we don't have a consistent way for
subcommands to report errors
So I think this patch is an improvement, in that it actually says what
is happening (rather than mostly saying something that is untrue) and
does not increase code size much.
I wonder if we should have a way to show an error number + string in printf()?
printf("subcommand failed (%pE)\n", ret);
I don't fully understand how we allow things after %p without
ambiguity...do you know?
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 02/14] bood
2022-09-09 18:20 ` Simon Glass
@ 2022-09-10 6:53 ` Heinrich Schuchardt
2022-09-12 13:34 ` Simon Glass
0 siblings, 1 reply; 19+ messages in thread
From: Heinrich Schuchardt @ 2022-09-10 6:53 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Alexandru Gagniuc, Artem Lapkin,
Philippe Reynes, Sean Anderson
On 9/9/22 20:20, Simon Glass wrote:
> Hi Heinrich,
>
> On Fri, 9 Sept 2022 at 09:33, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>>
>>
>> Am 9. September 2022 17:17:49 MESZ schrieb Simon Glass <sjg@chromium.org>:
>>> At present when bootm fails, it says:
>>>
>>> subcommand not supported
>>>
>>> and then prints help for the bootm command. This is not very useful, since
>>> generally the error is related to something else, such as fixups failing.
>>> It is quite confusing to see this in a test run.
>>>
>>> Change the error and show the error code.
>>>
>>> We could update the OS functions to return -ENOSYS when they do not
>>> support the bootm subcommand. But this involves some thought since this is
>>> arch-specific code and proper errno error codes are not always returned.
>>> Also, with the code as is, all required subcommands are of course
>>> supported - a problem would only come if someone added a new one or
>>> removed support for one from an existing OS. Therefore it seems better to
>>> leave that sort of effort for when our bootm tests are improved.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>> boot/bootm.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/boot/bootm.c b/boot/bootm.c
>>> index f6713807fda..ed6b489c4b3 100644
>>> --- a/boot/bootm.c
>>> +++ b/boot/bootm.c
>>> @@ -790,7 +790,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
>>>
>>> /* Check for unsupported subcommand. */
>>> if (ret) {
>>> - puts("subcommand not supported\n");
>>> + printf("subcommand failed (err=%d)\n", ret);
>>
>> Return codes are only interpretable by developers. We have a function to convert errno to a string.
>>
>> For the average user it would be helpful to know which (sub-)command failed especially if this boot command is executed in an automated way.
>
> I don't disagree, but:
> 1. The error strings add to code size, about 5KB or so
This is controlled by CONFIG_ERRNO_STR.
> 2. For devs the error number is much easier to use
> 3. For bug reports the error number is better too IMO
> 4. As per the commit message, we don't have a consistent way for
> subcommands to report errors
>
> So I think this patch is an improvement, in that it actually says what
> is happening (rather than mostly saying something that is untrue) and
> does not increase code size much.
>
> I wonder if we should have a way to show an error number + string in printf()?
>
> printf("subcommand failed (%pE)\n", ret);
%p is meant for pointers only. Using it for an integer will lead to a
build error:
format ‘%p’ expects argument of type ‘void *’,
but argument 2 has type ‘int’
For signed int we have the choice of:
'%d', '%i', '%o', or '%x'.
I suggest to use %iE.
%dE already occurs in existing code:
include/ansi.h:15
:#define ANSI_CURSOR_NEXTLINE "\e[%dE"
>
> I don't fully understand how we allow things after %p without
> ambiguity...do you know?
We rely on developers only wanting to print a pointer not using any
character with special meaning after %p. If you actually wanted to print
the letter 'D' directly after a pointer you would have to put it into a
separate string:
printf("%p""D", p);
In our existing code %i is succeeded by the following characters:
' ', '!', '"', ')', ',', '.', ':', '@', '\', ']', '0', 'a', 'g', 'n', 'o'.
So using 'E' is safe.
For %d succeeding characters are:
' ', '!', '"', '%', ''', '(', ')', '*', '+', ',', '-', '.', '/', ':',
';', '<', '=', '>', '?', '@', '[', '\', ']', '_', '}', '0', '1', '2',
'3', '4', '5', '7', 'a', 'A', 'b', 'B', 'C', 'd', 'D', 'e', 'E', 'f',
'F', 'G', 'H', 'i', 'k', 'K', 'm', 'M', 'n', 'o', 'p', 'r', 'R', 's',
'T', 'u', 'W', 'x'.
For %o:
' ', '"', ')', 'b', 'p', 'r'.
For %x:
'!', '"', '#', '%', ''', '(', ')', ',', '-', '.', '/', ':', ';', '>',
'[', '\', ']', '_', '}', '1', 'h', 'n'
Best regards
Heinrich
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 02/14] bood
2022-09-10 6:53 ` [PATCH 02/14] bood Heinrich Schuchardt
@ 2022-09-12 13:34 ` Simon Glass
0 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-12 13:34 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: U-Boot Mailing List, Alexandru Gagniuc, Artem Lapkin,
Philippe Reynes, Sean Anderson
Hi Heinrich,
On Sat, 10 Sept 2022 at 00:53, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 9/9/22 20:20, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 9 Sept 2022 at 09:33, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >>
> >>
> >> Am 9. September 2022 17:17:49 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >>> At present when bootm fails, it says:
> >>>
> >>> subcommand not supported
> >>>
> >>> and then prints help for the bootm command. This is not very useful, since
> >>> generally the error is related to something else, such as fixups failing.
> >>> It is quite confusing to see this in a test run.
> >>>
> >>> Change the error and show the error code.
> >>>
> >>> We could update the OS functions to return -ENOSYS when they do not
> >>> support the bootm subcommand. But this involves some thought since this is
> >>> arch-specific code and proper errno error codes are not always returned.
> >>> Also, with the code as is, all required subcommands are of course
> >>> supported - a problem would only come if someone added a new one or
> >>> removed support for one from an existing OS. Therefore it seems better to
> >>> leave that sort of effort for when our bootm tests are improved.
> >>>
> >>> Signed-off-by: Simon Glass <sjg@chromium.org>
> >>> ---
> >>>
> >>> boot/bootm.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/boot/bootm.c b/boot/bootm.c
> >>> index f6713807fda..ed6b489c4b3 100644
> >>> --- a/boot/bootm.c
> >>> +++ b/boot/bootm.c
> >>> @@ -790,7 +790,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
> >>>
> >>> /* Check for unsupported subcommand. */
> >>> if (ret) {
> >>> - puts("subcommand not supported\n");
> >>> + printf("subcommand failed (err=%d)\n", ret);
> >>
> >> Return codes are only interpretable by developers. We have a function to convert errno to a string.
> >>
> >> For the average user it would be helpful to know which (sub-)command failed especially if this boot command is executed in an automated way.
> >
> > I don't disagree, but:
> > 1. The error strings add to code size, about 5KB or so
>
> This is controlled by CONFIG_ERRNO_STR.
>
> > 2. For devs the error number is much easier to use
> > 3. For bug reports the error number is better too IMO
> > 4. As per the commit message, we don't have a consistent way for
> > subcommands to report errors
> >
> > So I think this patch is an improvement, in that it actually says what
> > is happening (rather than mostly saying something that is untrue) and
> > does not increase code size much.
> >
> > I wonder if we should have a way to show an error number + string in printf()?
> >
> > printf("subcommand failed (%pE)\n", ret);
>
> %p is meant for pointers only. Using it for an integer will lead to a
> build error:
>
> format ‘%p’ expects argument of type ‘void *’,
> but argument 2 has type ‘int’
Yes and that is the other piece of the puzzle - we cannot add random
chars between % and p since the C compiler complains.
>
> For signed int we have the choice of:
> '%d', '%i', '%o', or '%x'.
>
> I suggest to use %iE.
>
> %dE already occurs in existing code:
>
> include/ansi.h:15
> :#define ANSI_CURSOR_NEXTLINE "\e[%dE"
>
> >
> > I don't fully understand how we allow things after %p without
> > ambiguity...do you know?
>
> We rely on developers only wanting to print a pointer not using any
> character with special meaning after %p. If you actually wanted to print
> the letter 'D' directly after a pointer you would have to put it into a
> separate string:
>
> printf("%p""D", p);
>
> In our existing code %i is succeeded by the following characters:
> ' ', '!', '"', ')', ',', '.', ':', '@', '\', ']', '0', 'a', 'g', 'n', 'o'.
>
> So using 'E' is safe.
>
> For %d succeeding characters are:
> ' ', '!', '"', '%', ''', '(', ')', '*', '+', ',', '-', '.', '/', ':',
> ';', '<', '=', '>', '?', '@', '[', '\', ']', '_', '}', '0', '1', '2',
> '3', '4', '5', '7', 'a', 'A', 'b', 'B', 'C', 'd', 'D', 'e', 'E', 'f',
> 'F', 'G', 'H', 'i', 'k', 'K', 'm', 'M', 'n', 'o', 'p', 'r', 'R', 's',
> 'T', 'u', 'W', 'x'.
>
> For %o:
> ' ', '"', ')', 'b', 'p', 'r'.
>
> For %x:
> '!', '"', '#', '%', ''', '(', ')', ',', '-', '.', '/', ':', ';', '>',
> '[', '\', ']', '_', '}', '1', 'h', 'n'
I was assuming that %dE (the obvious choice since errors start with a
capital E) would be confusing / annoying, but I think you are right. I
will take a look.
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 03/14] bootm: Avoid returning error codes from command
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
2022-09-09 15:17 ` [PATCH 01/14] dm: core: Avoid registering an invalid tree in oftree_ensure() Simon Glass
2022-09-09 15:17 ` [PATCH 02/14] bootm: Change incorrect 'unsupported' error Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:17 ` [PATCH 04/14] bootm: Drop #ifdef from do_bootm() Simon Glass
` (10 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Patrick Delaunay, Philippe Reynes, Stefan Roese
Functions which implement commands must return a CMD_RET_... error code.
At present bootm can return a negative errno value in some cases, thus
causing strange behaviour such as trying to exit the shell and printing
usage information.
Fix this by returning the correct value.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
cmd/bootm.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/cmd/bootm.c b/cmd/bootm.c
index d764a27002d..f09b41c2c16 100644
--- a/cmd/bootm.c
+++ b/cmd/bootm.c
@@ -111,7 +111,7 @@ static int do_bootm_subcommand(struct cmd_tbl *cmdtp, int flag, int argc,
bootm_get_addr(argc, argv) + image_load_offset);
#endif
- return ret;
+ return ret ? CMD_RET_FAILURE : 0;
}
/*******************************************************************/
@@ -120,6 +120,8 @@ static int do_bootm_subcommand(struct cmd_tbl *cmdtp, int flag, int argc,
int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
{
+ int ret;
+
#ifdef CONFIG_NEEDS_MANUAL_RELOC
static int relocated = 0;
@@ -152,7 +154,7 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
return do_bootm_subcommand(cmdtp, flag, argc, argv);
}
- return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
+ ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOTHER |
BOOTM_STATE_LOADOS |
#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
@@ -163,6 +165,8 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
#endif
BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |
BOOTM_STATE_OS_GO, &images, 1);
+
+ return ret ? CMD_RET_FAILURE : 0;
}
int bootm_maybe_autostart(struct cmd_tbl *cmdtp, const char *cmd)
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 04/14] bootm: Drop #ifdef from do_bootm()
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
` (2 preceding siblings ...)
2022-09-09 15:17 ` [PATCH 03/14] bootm: Avoid returning error codes from command Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:17 ` [PATCH 05/14] boot: Correct handling of addresses in boot_relocate_fdt() Simon Glass
` (9 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Patrick Delaunay, Philippe Reynes, Stefan Roese
Drop the #ifdefs from this command by using a variable to hold the states
that should be executed.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
cmd/bootm.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/cmd/bootm.c b/cmd/bootm.c
index f09b41c2c16..37c2af96e08 100644
--- a/cmd/bootm.c
+++ b/cmd/bootm.c
@@ -120,6 +120,7 @@ static int do_bootm_subcommand(struct cmd_tbl *cmdtp, int flag, int argc,
int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
{
+ int states;
int ret;
#ifdef CONFIG_NEEDS_MANUAL_RELOC
@@ -154,17 +155,15 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
return do_bootm_subcommand(cmdtp, flag, argc, argv);
}
- ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
- BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOTHER |
- BOOTM_STATE_LOADOS |
-#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
- BOOTM_STATE_RAMDISK |
-#endif
-#if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
- BOOTM_STATE_OS_CMDLINE |
-#endif
+ states = BOOTM_STATE_START | BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD |
+ BOOTM_STATE_FINDOTHER | BOOTM_STATE_LOADOS |
BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |
- BOOTM_STATE_OS_GO, &images, 1);
+ BOOTM_STATE_OS_GO;
+ if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH))
+ states |= BOOTM_STATE_RAMDISK;
+ if (IS_ENABLED(CONFIG_PPC) || IS_ENABLED(CONFIG_MIPS))
+ states |= BOOTM_STATE_OS_CMDLINE;
+ ret = do_bootm_states(cmdtp, flag, argc, argv, states, &images, 1);
return ret ? CMD_RET_FAILURE : 0;
}
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 05/14] boot: Correct handling of addresses in boot_relocate_fdt()
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
` (3 preceding siblings ...)
2022-09-09 15:17 ` [PATCH 04/14] bootm: Drop #ifdef from do_bootm() Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:17 ` [PATCH 06/14] fs: Quieten down the filesystems more Simon Glass
` (8 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Alexandru Gagniuc, Artem Lapkin, Daniel Golle,
Heinrich Schuchardt, Marek Vasut
This code uses casts between addresses and pointers, so does not work with
sandbox. Update it so we can allow sandbox to do device tree fixups.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
boot/image-fdt.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index 884e089f2d8..f651940d9b4 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -186,24 +186,25 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size)
/* If fdt_high is set use it to select the relocation address */
fdt_high = env_get("fdt_high");
if (fdt_high) {
- void *desired_addr = (void *)hextoul(fdt_high, NULL);
+ ulong desired_addr = hextoul(fdt_high, NULL);
+ ulong addr;
- if (((ulong) desired_addr) == ~0UL) {
+ if (desired_addr == ~0UL) {
/* All ones means use fdt in place */
of_start = fdt_blob;
- lmb_reserve(lmb, (ulong)of_start, of_len);
+ lmb_reserve(lmb, map_to_sysmem(of_start), of_len);
disable_relocation = 1;
} else if (desired_addr) {
- of_start =
- (void *)(ulong) lmb_alloc_base(lmb, of_len, 0x1000,
- (ulong)desired_addr);
+ addr = lmb_alloc_base(lmb, of_len, 0x1000,
+ desired_addr);
+ of_start = map_sysmem(addr, of_len);
if (of_start == NULL) {
puts("Failed using fdt_high value for Device Tree");
goto error;
}
} else {
- of_start =
- (void *)(ulong) lmb_alloc(lmb, of_len, 0x1000);
+ addr = lmb_alloc(lmb, of_len, 0x1000);
+ of_start = map_sysmem(addr, of_len);
}
} else {
mapsize = env_get_bootm_mapsize();
@@ -224,9 +225,8 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size)
* At least part of this DRAM bank is usable, try
* using it for LMB allocation.
*/
- of_start =
- (void *)(ulong) lmb_alloc_base(lmb, of_len, 0x1000,
- start + usable);
+ of_start = map_sysmem((ulong)lmb_alloc_base(lmb,
+ of_len, 0x1000, start + usable), of_len);
/* Allocation succeeded, use this block. */
if (of_start != NULL)
break;
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 06/14] fs: Quieten down the filesystems more
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
` (4 preceding siblings ...)
2022-09-09 15:17 ` [PATCH 05/14] boot: Correct handling of addresses in boot_relocate_fdt() Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:17 ` [PATCH 07/14] fdt: Show a message when the working FDT changes Simon Glass
` (7 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Heinrich Schuchardt, Marek Behún,
Pali Rohár, Philippe Reynes, Qu Wenruo, Stefan Roese,
linux-btrfs
When looking for a filesystem on a partition we should do so quietly. At
present if the filesystem is very small (e.g. 512 bytes) we get a host of
messages.
Update these to only show when debugging.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
disk/part_efi.c | 15 +++++++--------
fs/btrfs/disk-io.c | 7 ++++---
fs/ext4/ext4_common.c | 2 +-
fs/fs_internal.c | 3 +--
4 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c
index 5090efd1192..911089f76dd 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -266,20 +266,19 @@ int part_get_info_efi(struct blk_desc *dev_desc, int part,
/* "part" argument must be at least 1 */
if (part < 1) {
- printf("%s: Invalid Argument(s)\n", __func__);
- return -1;
+ log_debug("Invalid Argument(s)\n");
+ return -EINVAL;
}
/* This function validates AND fills in the GPT header and PTE */
if (find_valid_gpt(dev_desc, gpt_head, &gpt_pte) != 1)
- return -1;
+ return -EINVAL;
if (part > le32_to_cpu(gpt_head->num_partition_entries) ||
!is_pte_valid(&gpt_pte[part - 1])) {
- debug("%s: *** ERROR: Invalid partition number %d ***\n",
- __func__, part);
+ log_debug("*** ERROR: Invalid partition number %d ***\n", part);
free(gpt_pte);
- return -1;
+ return -EPERM;
}
/* The 'lbaint_t' casting may limit the maximum disk size to 2 TB */
@@ -302,8 +301,8 @@ int part_get_info_efi(struct blk_desc *dev_desc, int part,
info->type_guid, UUID_STR_FORMAT_GUID);
#endif
- debug("%s: start 0x" LBAF ", size 0x" LBAF ", name %s\n", __func__,
- info->start, info->size, info->name);
+ log_debug("start 0x" LBAF ", size 0x" LBAF ", name %s\n", info->start,
+ info->size, info->name);
/* Remember to free pte */
free(gpt_pte);
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 8043abc1bd6..e2562877e0a 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0+
#include <common.h>
#include <fs_internal.h>
+#include <log.h>
#include <uuid.h>
#include <memalign.h>
#include "kernel-shared/btrfs_tree.h"
@@ -912,9 +913,9 @@ static int btrfs_scan_fs_devices(struct blk_desc *desc,
if (round_up(BTRFS_SUPER_INFO_SIZE + BTRFS_SUPER_INFO_OFFSET,
desc->blksz) > (part->size << desc->log2blksz)) {
- error("superblock end %u is larger than device size " LBAFU,
- BTRFS_SUPER_INFO_SIZE + BTRFS_SUPER_INFO_OFFSET,
- part->size << desc->log2blksz);
+ log_debug("superblock end %u is larger than device size " LBAFU,
+ BTRFS_SUPER_INFO_SIZE + BTRFS_SUPER_INFO_OFFSET,
+ part->size << desc->log2blksz);
return -EINVAL;
}
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index d49ba4a9954..1185cb2c046 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2415,7 +2415,7 @@ int ext4fs_mount(unsigned part_length)
return 1;
fail:
- printf("Failed to mount ext2 filesystem...\n");
+ log_debug("Failed to mount ext2 filesystem...\n");
fail_noerr:
free(data);
ext4fs_root = NULL;
diff --git a/fs/fs_internal.c b/fs/fs_internal.c
index ae1cb8584c7..111f91b355d 100644
--- a/fs/fs_internal.c
+++ b/fs/fs_internal.c
@@ -29,8 +29,7 @@ int fs_devread(struct blk_desc *blk, struct disk_partition *partition,
/* Check partition boundaries */
if ((sector + ((byte_offset + byte_len - 1) >> log2blksz))
>= partition->size) {
- log_err("%s read outside partition " LBAFU "\n", __func__,
- sector);
+ log_debug("read outside partition " LBAFU "\n", sector);
return 0;
}
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 07/14] fdt: Show a message when the working FDT changes
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
` (5 preceding siblings ...)
2022-09-09 15:17 ` [PATCH 06/14] fs: Quieten down the filesystems more Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:17 ` [PATCH 08/14] bootstd: Move VBE setup into a shared function Simon Glass
` (6 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Jerry Van Baren
The working FDT is the one which comes from the OS and is fixed up by
U-Boot. When the bootm command runs, it sets up the working FDT to be the
one it is about to pass to the OS, so that fixups can happen.
This seems like an important step, so add a message indicating that the
working FDT has changed. This is shown during the running of the bootm
command.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
cmd/fdt.c | 1 +
doc/usage/cmd/fdt.rst | 1 +
test/cmd/fdt.c | 11 ++++++++++-
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/cmd/fdt.c b/cmd/fdt.c
index 842e6cb634b..0afc7222c74 100644
--- a/cmd/fdt.c
+++ b/cmd/fdt.c
@@ -40,6 +40,7 @@ void set_working_fdt_addr(ulong addr)
{
void *buf;
+ printf("Working FDT set to %lx\n", addr);
buf = map_sysmem(addr, 0);
working_fdt = buf;
env_set_hex("fdtaddr", addr);
diff --git a/doc/usage/cmd/fdt.rst b/doc/usage/cmd/fdt.rst
index 07fed732e45..36b8230877c 100644
--- a/doc/usage/cmd/fdt.rst
+++ b/doc/usage/cmd/fdt.rst
@@ -60,6 +60,7 @@ The second word shows the size of the FDT. Now set the working FDT to that
address and expand it to 0xf000 in size::
=> fdt addr 10000 f000
+ Working FDT set to 10000
=> md 10000 4
00010000: edfe0dd0 00f00000 78000000 7c270000 ...........x..'|
diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
index 100a7ef5ebf..ba9eaa42c14 100644
--- a/test/cmd/fdt.c
+++ b/test/cmd/fdt.c
@@ -55,6 +55,7 @@ static int fdt_test_addr(struct unit_test_state *uts)
/* The working fdt is not set, so this should fail */
set_working_fdt_addr(0);
+ ut_assert_nextline("Working FDT set to 0");
ut_asserteq(CMD_RET_FAILURE, run_command("fdt addr", 0));
ut_assert_nextline("libfdt fdt_check_header(): FDT_ERR_BADMAGIC");
ut_assertok(ut_check_console_end(uts));
@@ -63,18 +64,22 @@ static int fdt_test_addr(struct unit_test_state *uts)
ut_assertok(make_test_fdt(uts, fdt, sizeof(fdt)));
addr = map_to_sysmem(fdt);
set_working_fdt_addr(addr);
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_assertok(run_command("fdt addr", 0));
ut_assert_nextline("Working fdt: %08lx", (ulong)map_to_sysmem(fdt));
ut_assertok(ut_check_console_end(uts));
/* Set the working FDT */
set_working_fdt_addr(0);
+ ut_assert_nextline("Working FDT set to 0");
ut_assertok(run_commandf("fdt addr %08x", addr));
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_asserteq(addr, map_to_sysmem(working_fdt));
ut_assertok(ut_check_console_end(uts));
set_working_fdt_addr(0);
+ ut_assert_nextline("Working FDT set to 0");
- /* Set the working FDT */
+ /* Set the control FDT */
fdt_blob = gd->fdt_blob;
gd->fdt_blob = NULL;
ret = run_commandf("fdt addr -c %08x", addr);
@@ -93,6 +98,7 @@ static int fdt_test_addr(struct unit_test_state *uts)
/* Test detecting an invalid FDT */
fdt[0] = 123;
set_working_fdt_addr(addr);
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_asserteq(1, run_commandf("fdt addr"));
ut_assert_nextline("libfdt fdt_check_header(): FDT_ERR_BADMAGIC");
ut_assertok(ut_check_console_end(uts));
@@ -115,16 +121,19 @@ static int fdt_test_resize(struct unit_test_state *uts)
/* Test setting and resizing the working FDT to a larger size */
ut_assertok(console_record_reset_enable());
ut_assertok(run_commandf("fdt addr %08x %x", addr, newsize));
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_assertok(ut_check_console_end(uts));
/* Try shrinking it */
ut_assertok(run_commandf("fdt addr %08x %x", addr, sizeof(fdt) / 4));
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_assert_nextline("New length %d < existing length %d, ignoring",
(int)sizeof(fdt) / 4, newsize);
ut_assertok(ut_check_console_end(uts));
/* ...quietly */
ut_assertok(run_commandf("fdt addr -q %08x %x", addr, sizeof(fdt) / 4));
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_assertok(ut_check_console_end(uts));
/* We cannot easily provoke errors in fdt_open_into(), so ignore that */
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 08/14] bootstd: Move VBE setup into a shared function
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
` (6 preceding siblings ...)
2022-09-09 15:17 ` [PATCH 07/14] fdt: Show a message when the working FDT changes Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:17 ` [PATCH 09/14] sandbox: Support FDT fixups Simon Glass
` (5 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass
This information needs to be set up by the bootstd tests as well. Move it
into a common function and ensure it is executed before any bootstd test
is run.
Make sure the 'images' parameter is set correctly for fixups.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
test/boot/bootstd_common.c | 48 ++++++++++++++++++++++++++++++++++++++
test/boot/bootstd_common.h | 16 +++++++++++++
test/boot/vbe_simple.c | 34 ++++-----------------------
3 files changed, 68 insertions(+), 30 deletions(-)
diff --git a/test/boot/bootstd_common.c b/test/boot/bootstd_common.c
index 05347d87106..59d46bef0c5 100644
--- a/test/boot/bootstd_common.c
+++ b/test/boot/bootstd_common.c
@@ -9,10 +9,46 @@
#include <common.h>
#include <bootstd.h>
#include <dm.h>
+#include <memalign.h>
+#include <mmc.h>
+#include <linux/log2.h>
#include <test/suites.h>
#include <test/ut.h>
+#include <u-boot/crc.h>
#include "bootstd_common.h"
+bool vbe_setup_done;
+
+/* set up MMC for VBE tests */
+int bootstd_setup_for_tests(void)
+{
+ ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN);
+ struct udevice *mmc;
+ struct blk_desc *desc;
+ int ret;
+
+ /* Set up the version string */
+ ret = uclass_get_device(UCLASS_MMC, 1, &mmc);
+ if (ret)
+ return log_msg_ret("mmc", -EIO);
+ desc = blk_get_by_device(mmc);
+
+ memset(buf, '\0', MMC_MAX_BLOCK_LEN);
+ strcpy(buf, TEST_VERSION);
+ if (blk_dwrite(desc, VERSION_START_BLK, 1, buf) != 1)
+ return log_msg_ret("wr1", -EIO);
+
+ /* Set up the nvdata */
+ memset(buf, '\0', MMC_MAX_BLOCK_LEN);
+ buf[1] = ilog2(0x40) << 4 | 1;
+ *(u32 *)(buf + 4) = TEST_VERNUM;
+ buf[0] = crc8(0, buf + 1, 0x3f);
+ if (blk_dwrite(desc, NVDATA_START_BLK, 1, buf) != 1)
+ return log_msg_ret("wr2", -EIO);
+
+ return 0;
+}
+
int bootstd_test_drop_bootdev_order(struct unit_test_state *uts)
{
struct bootstd_priv *priv;
@@ -30,6 +66,18 @@ int do_ut_bootstd(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
struct unit_test *tests = UNIT_TEST_SUITE_START(bootstd_test);
const int n_ents = UNIT_TEST_SUITE_COUNT(bootstd_test);
+ if (!vbe_setup_done) {
+ int ret;
+
+ ret = bootstd_setup_for_tests();
+ if (ret) {
+ printf("Failed to set up for bootstd tests (err=%d)\n",
+ ret);
+ return CMD_RET_FAILURE;
+ }
+ vbe_setup_done = true;
+ }
+
return cmd_ut_category("bootstd", "bootstd_test_",
tests, n_ents, argc, argv);
}
diff --git a/test/boot/bootstd_common.h b/test/boot/bootstd_common.h
index 676ef0a57f9..c5e0fd1ceab 100644
--- a/test/boot/bootstd_common.h
+++ b/test/boot/bootstd_common.h
@@ -9,10 +9,17 @@
#ifndef __bootstd_common_h
#define __bootstd_common_h
+#include <version_string.h>
+
/* Declare a new bootdev test */
#define BOOTSTD_TEST(_name, _flags) \
UNIT_TEST(_name, _flags, bootstd_test)
+#define NVDATA_START_BLK ((0x400 + 0x400) / MMC_MAX_BLOCK_LEN)
+#define VERSION_START_BLK ((0x400 + 0x800) / MMC_MAX_BLOCK_LEN)
+#define TEST_VERSION "U-Boot v2022.04-local2"
+#define TEST_VERNUM 0x00010002
+
struct unit_test_state;
/**
@@ -24,4 +31,13 @@ struct unit_test_state;
*/
int bootstd_test_drop_bootdev_order(struct unit_test_state *uts);
+/**
+ * bootstd_setup_for_tests() - Set up MMC data for VBE tests
+ *
+ * Some data is needed for VBE tests to work. This function sets that up.
+ *
+ * @return 0 if OK, -ve on error
+ */
+int bootstd_setup_for_tests(void);
+
#endif
diff --git a/test/boot/vbe_simple.c b/test/boot/vbe_simple.c
index 8acd777f4cd..faba9e8f90b 100644
--- a/test/boot/vbe_simple.c
+++ b/test/boot/vbe_simple.c
@@ -10,54 +10,27 @@
#include <bootmeth.h>
#include <dm.h>
#include <image.h>
-#include <memalign.h>
-#include <mmc.h>
#include <of_live.h>
#include <vbe.h>
-#include <version_string.h>
-#include <linux/log2.h>
#include <test/suites.h>
#include <test/ut.h>
-#include <u-boot/crc.h>
#include "bootstd_common.h"
-#define NVDATA_START_BLK ((0x400 + 0x400) / MMC_MAX_BLOCK_LEN)
-#define VERSION_START_BLK ((0x400 + 0x800) / MMC_MAX_BLOCK_LEN)
-#define TEST_VERSION "U-Boot v2022.04-local2"
-#define TEST_VERNUM 0x00010002
-
/* Basic test of reading nvdata and updating a fwupd node in the device tree */
static int vbe_simple_test_base(struct unit_test_state *uts)
{
- ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN);
const char *version, *bl_version;
struct event_ft_fixup fixup;
- struct udevice *dev, *mmc;
+ struct udevice *dev;
struct device_node *np;
- struct blk_desc *desc;
char fdt_buf[0x400];
char info[100];
int node_ofs;
ofnode node;
u32 vernum;
- /* Set up the version string */
- ut_assertok(uclass_get_device(UCLASS_MMC, 1, &mmc));
- desc = blk_get_by_device(mmc);
- ut_assertnonnull(desc);
-
- memset(buf, '\0', MMC_MAX_BLOCK_LEN);
- strcpy(buf, TEST_VERSION);
- if (blk_dwrite(desc, VERSION_START_BLK, 1, buf) != 1)
- return log_msg_ret("write", -EIO);
-
- /* Set up the nvdata */
- memset(buf, '\0', MMC_MAX_BLOCK_LEN);
- buf[1] = ilog2(0x40) << 4 | 1;
- *(u32 *)(buf + 4) = TEST_VERNUM;
- buf[0] = crc8(0, buf + 1, 0x3f);
- if (blk_dwrite(desc, NVDATA_START_BLK, 1, buf) != 1)
- return log_msg_ret("write", -EIO);
+ /* Set up the VBE info */
+ ut_assertok(bootstd_setup_for_tests());
/* Read the version back */
ut_assertok(vbe_find_by_any("firmware0", &dev));
@@ -90,6 +63,7 @@ static int vbe_simple_test_base(struct unit_test_state *uts)
*
* Two fix this we need image_setup_libfdt() is updated to use ofnode
*/
+ fixup.images = NULL;
ut_assertok(event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup)));
node = oftree_path(fixup.tree, "/chosen/fwupd/firmware0");
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 09/14] sandbox: Support FDT fixups
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
` (7 preceding siblings ...)
2022-09-09 15:17 ` [PATCH 08/14] bootstd: Move VBE setup into a shared function Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:17 ` [PATCH 10/14] boot: Pass the correct FDT to the EVT_FT_FIXUP event Simon Glass
` (4 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass
Add support for doing device tree fixups in sandbox. This allows us to
test that functionality in CI.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
arch/sandbox/lib/bootm.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/sandbox/lib/bootm.c b/arch/sandbox/lib/bootm.c
index c1742f94de7..28f4a746fb6 100644
--- a/arch/sandbox/lib/bootm.c
+++ b/arch/sandbox/lib/bootm.c
@@ -50,8 +50,25 @@ int bootz_setup(ulong image, ulong *start, ulong *end)
return ret;
}
+/* Subcommand: PREP */
+static int boot_prep_linux(struct bootm_headers *images)
+{
+ int ret;
+
+ if (CONFIG_IS_ENABLED(LMB)) {
+ ret = image_setup_linux(images);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
int do_bootm_linux(int flag, int argc, char *argv[], struct bootm_headers *images)
{
+ if (flag & BOOTM_STATE_OS_PREP)
+ return boot_prep_linux(images);
+
if (flag & (BOOTM_STATE_OS_GO | BOOTM_STATE_OS_FAKE_GO)) {
bootstage_mark(BOOTSTAGE_ID_RUN_OS);
printf("## Transferring control to Linux (at address %08lx)...\n",
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 10/14] boot: Pass the correct FDT to the EVT_FT_FIXUP event
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
` (8 preceding siblings ...)
2022-09-09 15:17 ` [PATCH 09/14] sandbox: Support FDT fixups Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:17 ` [PATCH 11/14] boot: Tidy up logging and naming in vbe_simple Simon Glass
` (3 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Alexandru Gagniuc, Daniel Golle, Heinrich Schuchardt,
Marek Vasut
Now that we support multiple device trees with the ofnode interface, we
can pass the correct FDT to this event. This allows the 'working' FDT to
be fixed up, as expected, so long as OFNODE_MULTI_TREE is enabled.
Also make sure we don't try to do this with livetree, which does not
support fixups yet.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
boot/image-fdt.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index f651940d9b4..b830a0ab418 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -665,15 +665,18 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob,
goto err;
}
}
- if (CONFIG_IS_ENABLED(EVENT)) {
+ if (!of_live_active() && CONFIG_IS_ENABLED(EVENT)) {
struct event_ft_fixup fixup;
- fixup.tree = oftree_default();
+ fixup.tree = oftree_from_fdt(blob);
fixup.images = images;
- ret = event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup));
- if (ret) {
- printf("ERROR: fdt fixup event failed: %d\n", ret);
- goto err;
+ if (oftree_valid(fixup.tree)) {
+ ret = event_notify(EVT_FT_FIXUP, &fixup, sizeof(fixup));
+ if (ret) {
+ printf("ERROR: fdt fixup event failed: %d\n",
+ ret);
+ goto err;
+ }
}
}
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 11/14] boot: Tidy up logging and naming in vbe_simple
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
` (9 preceding siblings ...)
2022-09-09 15:17 ` [PATCH 10/14] boot: Pass the correct FDT to the EVT_FT_FIXUP event Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:17 ` [PATCH 12/14] test: Move common FIT code into a separate fit_util file Simon Glass
` (2 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Heinrich Schuchardt
Make sure the log_msg_ret() values are unique so that the log trace is
unambiguous with LOG_ERROR_RETURN. Also avoid reusing the 'node' variable
for two different nodes in bootmeth_vbe_simple_ft_fixup(), since this is
confusing.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
boot/vbe_simple.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/boot/vbe_simple.c b/boot/vbe_simple.c
index 61b6322ebe2..076b650c25a 100644
--- a/boot/vbe_simple.c
+++ b/boot/vbe_simple.c
@@ -6,6 +6,8 @@
* Written by Simon Glass <sjg@chromium.org>
*/
+#define LOG_CATEGORY LOGC_BOOT
+
#include <common.h>
#include <log.h>
#include <memalign.h>
@@ -199,17 +201,17 @@ int vbe_simple_fixup_node(ofnode node, struct simple_state *state)
version = strdup(state->fw_version);
if (!version)
- return log_msg_ret("ver", -ENOMEM);
+ return log_msg_ret("dup", -ENOMEM);
ret = ofnode_write_string(node, "cur-version", version);
if (ret)
return log_msg_ret("ver", ret);
ret = ofnode_write_u32(node, "cur-vernum", state->fw_vernum);
if (ret)
- return log_msg_ret("ver", ret);
+ return log_msg_ret("num", ret);
ret = ofnode_write_string(node, "bootloader-version", version_string);
if (ret)
- return log_msg_ret("fix", ret);
+ return log_msg_ret("bl", ret);
return 0;
}
@@ -233,7 +235,7 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event)
*/
for (vbe_find_first_device(&dev); dev; vbe_find_next_device(&dev)) {
struct simple_state state;
- ofnode node;
+ ofnode node, subnode;
int ret;
if (strcmp("vbe_simple", dev->driver->name))
@@ -243,8 +245,8 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event)
node = oftree_path(tree, "/chosen/fwupd");
if (!ofnode_valid(node))
continue;
- node = ofnode_find_subnode(node, dev->name);
- if (!ofnode_valid(node))
+ subnode = ofnode_find_subnode(node, dev->name);
+ if (!ofnode_valid(subnode))
continue;
log_debug("Fixing up: %s\n", dev->name);
@@ -255,7 +257,7 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event)
if (ret)
return log_msg_ret("read", ret);
- ret = vbe_simple_fixup_node(node, &state);
+ ret = vbe_simple_fixup_node(subnode, &state);
if (ret)
return log_msg_ret("fix", ret);
}
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 12/14] test: Move common FIT code into a separate fit_util file
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
` (10 preceding siblings ...)
2022-09-09 15:17 ` [PATCH 11/14] boot: Tidy up logging and naming in vbe_simple Simon Glass
@ 2022-09-09 15:17 ` Simon Glass
2022-09-09 15:18 ` [PATCH 13/14] vbe: Add fixups for a basic set of OS requests Simon Glass
2022-09-09 15:18 ` [PATCH 14/14] vbe: Add a test for VBE device tree fixups Simon Glass
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:17 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass, Philippe Reynes
To avoid duplicating code, create a new fit_util module which provides
various utility functions for FIT. Move this code out from the existing
test_fit.py and refactor it with addition parameters.
Fix up pylint warnings in the conversion.
This involves no functional change.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
test/boot/bootstd_common.c | 21 ++++-----
test/py/tests/fit_util.py | 90 ++++++++++++++++++++++++++++++++++++++
test/py/tests/test_fit.py | 79 ++++-----------------------------
3 files changed, 110 insertions(+), 80 deletions(-)
create mode 100644 test/py/tests/fit_util.py
diff --git a/test/boot/bootstd_common.c b/test/boot/bootstd_common.c
index 59d46bef0c5..7a40836507a 100644
--- a/test/boot/bootstd_common.c
+++ b/test/boot/bootstd_common.c
@@ -17,6 +17,7 @@
#include <u-boot/crc.h>
#include "bootstd_common.h"
+/* tracks whether bootstd_setup_for_tests() has been run yet */
bool vbe_setup_done;
/* set up MMC for VBE tests */
@@ -27,6 +28,9 @@ int bootstd_setup_for_tests(void)
struct blk_desc *desc;
int ret;
+ if (vbe_setup_done)
+ return 0;
+
/* Set up the version string */
ret = uclass_get_device(UCLASS_MMC, 1, &mmc);
if (ret)
@@ -46,6 +50,8 @@ int bootstd_setup_for_tests(void)
if (blk_dwrite(desc, NVDATA_START_BLK, 1, buf) != 1)
return log_msg_ret("wr2", -EIO);
+ vbe_setup_done = true;
+
return 0;
}
@@ -65,17 +71,12 @@ int do_ut_bootstd(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
{
struct unit_test *tests = UNIT_TEST_SUITE_START(bootstd_test);
const int n_ents = UNIT_TEST_SUITE_COUNT(bootstd_test);
+ int ret;
- if (!vbe_setup_done) {
- int ret;
-
- ret = bootstd_setup_for_tests();
- if (ret) {
- printf("Failed to set up for bootstd tests (err=%d)\n",
- ret);
- return CMD_RET_FAILURE;
- }
- vbe_setup_done = true;
+ ret = bootstd_setup_for_tests();
+ if (ret) {
+ printf("Failed to set up for bootstd tests (err=%d)\n", ret);
+ return CMD_RET_FAILURE;
}
return cmd_ut_category("bootstd", "bootstd_test_",
diff --git a/test/py/tests/fit_util.py b/test/py/tests/fit_util.py
new file mode 100644
index 00000000000..fcec4c43c3c
--- /dev/null
+++ b/test/py/tests/fit_util.py
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2022 Google LLC
+
+"""Common utility functions for FIT tests"""
+
+import os
+
+import u_boot_utils as util
+
+def make_fname(cons, basename):
+ """Make a temporary filename
+
+ Args:
+ cons (ConsoleBase): u_boot_console to use
+ basename (str): Base name of file to create (within temporary directory)
+ Return:
+ Temporary filename
+ """
+
+ return os.path.join(cons.config.build_dir, basename)
+
+def make_its(cons, base_its, params, basename='test.its'):
+ """Make a sample .its file with parameters embedded
+
+ Args:
+ cons (ConsoleBase): u_boot_console to use
+ base_its (str): Template text for the .its file, typically containing
+ %() references
+ params (dict of str): Parameters to embed in the %() strings
+ basename (str): base name to write to (will be placed in the temp dir)
+ Returns:
+ str: Filename of .its file created
+ """
+ its = make_fname(cons, basename)
+ with open(its, 'w', encoding='utf-8') as outf:
+ print(base_its % params, file=outf)
+ return its
+
+def make_fit(cons, mkimage, base_its, params, basename='test.fit'):
+ """Make a sample .fit file ready for loading
+
+ This creates a .its script with the selected parameters and uses mkimage to
+ turn this into a .fit image.
+
+ Args:
+ cons (ConsoleBase): u_boot_console to use
+ mkimage (str): Filename of 'mkimage' utility
+ base_its (str): Template text for the .its file, typically containing
+ %() references
+ params (dict of str): Parameters to embed in the %() strings
+ basename (str): base name to write to (will be placed in the temp dir)
+ Return:
+ Filename of .fit file created
+ """
+ fit = make_fname(cons, basename)
+ its = make_its(cons, base_its, params)
+ util.run_and_log(cons, [mkimage, '-f', its, fit])
+ return fit
+
+def make_kernel(cons, basename, text):
+ """Make a sample kernel with test data
+
+ Args:
+ cons (ConsoleBase): u_boot_console to use
+ basename (str): base name to write to (will be placed in the temp dir)
+ text (str): Contents of the kernel file (will be repeated 100 times)
+ Returns:
+ str: Full path and filename of the kernel it created
+ """
+ fname = make_fname(cons, basename)
+ data = ''
+ for i in range(100):
+ data += f'this {text} {i} is unlikely to boot\n'
+ with open(fname, 'w', encoding='utf-8') as outf:
+ print(data, file=outf)
+ return fname
+
+def make_dtb(cons, base_fdt, basename):
+ """Make a sample .dts file and compile it to a .dtb
+
+ Returns:
+ cons (ConsoleBase): u_boot_console to use
+ Filename of .dtb file created
+ """
+ src = make_fname(cons, f'{basename}.dts')
+ dtb = make_fname(cons, f'{basename}.dtb')
+ with open(src, 'w', encoding='utf-8') as outf:
+ outf.write(base_fdt)
+ util.run_and_log(cons, ['dtc', src, '-O', 'dtb', '-o', dtb])
+ return dtb
diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py
index 5856960be23..f45848484eb 100755
--- a/test/py/tests/test_fit.py
+++ b/test/py/tests/test_fit.py
@@ -7,6 +7,7 @@ import os
import pytest
import struct
import u_boot_utils as util
+import fit_util
# Define a base ITS which we can adjust using % and a dictionary
base_its = '''
@@ -126,7 +127,6 @@ def test_fit(u_boot_console):
Return:
Temporary filename
"""
-
return os.path.join(cons.config.build_dir, leaf)
def filesize(fname):
@@ -150,67 +150,6 @@ def test_fit(u_boot_console):
with open(fname, 'rb') as fd:
return fd.read()
- def make_dtb():
- """Make a sample .dts file and compile it to a .dtb
-
- Returns:
- Filename of .dtb file created
- """
- src = make_fname('u-boot.dts')
- dtb = make_fname('u-boot.dtb')
- with open(src, 'w') as fd:
- fd.write(base_fdt)
- util.run_and_log(cons, ['dtc', src, '-O', 'dtb', '-o', dtb])
- return dtb
-
- def make_its(params):
- """Make a sample .its file with parameters embedded
-
- Args:
- params: Dictionary containing parameters to embed in the %() strings
- Returns:
- Filename of .its file created
- """
- its = make_fname('test.its')
- with open(its, 'w') as fd:
- print(base_its % params, file=fd)
- return its
-
- def make_fit(mkimage, params):
- """Make a sample .fit file ready for loading
-
- This creates a .its script with the selected parameters and uses mkimage to
- turn this into a .fit image.
-
- Args:
- mkimage: Filename of 'mkimage' utility
- params: Dictionary containing parameters to embed in the %() strings
- Return:
- Filename of .fit file created
- """
- fit = make_fname('test.fit')
- its = make_its(params)
- util.run_and_log(cons, [mkimage, '-f', its, fit])
- with open(make_fname('u-boot.dts'), 'w') as fd:
- fd.write(base_fdt)
- return fit
-
- def make_kernel(filename, text):
- """Make a sample kernel with test data
-
- Args:
- filename: the name of the file you want to create
- Returns:
- Full path and filename of the kernel it created
- """
- fname = make_fname(filename)
- data = ''
- for i in range(100):
- data += 'this %s %d is unlikely to boot\n' % (text, i)
- with open(fname, 'w') as fd:
- print(data, file=fd)
- return fname
-
def make_ramdisk(filename, text):
"""Make a sample ramdisk with test data
@@ -321,10 +260,10 @@ def test_fit(u_boot_console):
- run code coverage to make sure we are testing all the code
"""
# Set up invariant files
- control_dtb = make_dtb()
- kernel = make_kernel('test-kernel.bin', 'kernel')
+ control_dtb = fit_util.make_dtb(cons, base_fdt, 'u-boot')
+ kernel = fit_util.make_kernel(cons, 'test-kernel.bin', 'kernel')
ramdisk = make_ramdisk('test-ramdisk.bin', 'ramdisk')
- loadables1 = make_kernel('test-loadables1.bin', 'lenrek')
+ loadables1 = fit_util.make_kernel(cons, 'test-loadables1.bin', 'lenrek')
loadables2 = make_ramdisk('test-loadables2.bin', 'ksidmar')
kernel_out = make_fname('kernel-out.bin')
fdt = make_fname('u-boot.dtb')
@@ -372,7 +311,7 @@ def test_fit(u_boot_console):
}
# Make a basic FIT and a script to load it
- fit = make_fit(mkimage, params)
+ fit = fit_util.make_fit(cons, mkimage, base_its, params)
params['fit'] = fit
cmd = base_script % params
@@ -403,7 +342,7 @@ def test_fit(u_boot_console):
# Now a kernel and an FDT
with cons.log.section('Kernel + FDT load'):
params['fdt_load'] = 'load = <%#x>;' % params['fdt_addr']
- fit = make_fit(mkimage, params)
+ fit = fit_util.make_fit(cons, mkimage, base_its, params)
cons.restart_uboot()
output = cons.run_command_list(cmd.splitlines())
check_equal(kernel, kernel_out, 'Kernel not loaded')
@@ -415,7 +354,7 @@ def test_fit(u_boot_console):
with cons.log.section('Kernel + FDT + Ramdisk load'):
params['ramdisk_config'] = 'ramdisk = "ramdisk-1";'
params['ramdisk_load'] = 'load = <%#x>;' % params['ramdisk_addr']
- fit = make_fit(mkimage, params)
+ fit = fit_util.make_fit(cons, mkimage, base_its, params)
cons.restart_uboot()
output = cons.run_command_list(cmd.splitlines())
check_equal(ramdisk, ramdisk_out, 'Ramdisk not loaded')
@@ -427,7 +366,7 @@ def test_fit(u_boot_console):
params['loadables1_addr'])
params['loadables2_load'] = ('load = <%#x>;' %
params['loadables2_addr'])
- fit = make_fit(mkimage, params)
+ fit = fit_util.make_fit(cons, mkimage, base_its, params)
cons.restart_uboot()
output = cons.run_command_list(cmd.splitlines())
check_equal(loadables1, loadables1_out,
@@ -441,7 +380,7 @@ def test_fit(u_boot_console):
params['kernel'] = make_compressed(kernel)
params['fdt'] = make_compressed(fdt)
params['ramdisk'] = make_compressed(ramdisk)
- fit = make_fit(mkimage, params)
+ fit = fit_util.make_fit(cons, mkimage, base_its, params)
cons.restart_uboot()
output = cons.run_command_list(cmd.splitlines())
check_equal(kernel, kernel_out, 'Kernel not loaded')
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 13/14] vbe: Add fixups for a basic set of OS requests
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
` (11 preceding siblings ...)
2022-09-09 15:17 ` [PATCH 12/14] test: Move common FIT code into a separate fit_util file Simon Glass
@ 2022-09-09 15:18 ` Simon Glass
2022-09-09 15:18 ` [PATCH 14/14] vbe: Add a test for VBE device tree fixups Simon Glass
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:18 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Simon Glass, Artem Lapkin, John Keeping, Philippe Reynes,
Ramon Fried
As a starting point, add support for providing random data, if requested
by the OS. Also add ASLR, as a placeholder for now.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
boot/Makefile | 2 +-
boot/vbe_fixup.c | 233 +++++++++++++++++++++++++++++++
test/py/tests/test_event_dump.py | 1 +
3 files changed, 235 insertions(+), 1 deletion(-)
create mode 100644 boot/vbe_fixup.c
diff --git a/boot/Makefile b/boot/Makefile
index 67e335255f1..dd45d786f8c 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -47,5 +47,5 @@ ifdef CONFIG_SPL_BUILD
obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o
endif
-obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o
+obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o vbe_fixup.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o
diff --git a/boot/vbe_fixup.c b/boot/vbe_fixup.c
new file mode 100644
index 00000000000..8cdff6972b1
--- /dev/null
+++ b/boot/vbe_fixup.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Verified Boot for Embedded (VBE) device tree fixup functions
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#define LOG_CATEGORY LOGC_BOOT
+
+#include <common.h>
+#include <dm.h>
+#include <event.h>
+#include <image.h>
+#include <malloc.h>
+#include <rng.h>
+#include <dm/ofnode.h>
+
+#define VBE_PREFIX "vbe,"
+#define VBE_PREFIX_LEN (sizeof(VBE_PREFIX) - 1)
+#define VBE_ERR_STR_LEN 128
+#define VBE_MAX_RAND_SIZE 256
+
+struct vbe_result {
+ int errnum;
+ char err_str[VBE_ERR_STR_LEN];
+};
+
+typedef int (*vbe_req_func)(ofnode node, struct vbe_result *result);
+
+static int handle_random_req(ofnode node, int default_size,
+ struct vbe_result *result)
+{
+ char buf[VBE_MAX_RAND_SIZE];
+ struct udevice *dev;
+ u32 size;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_DM_RNG))
+ return -ENOTSUPP;
+
+ if (ofnode_read_u32(node, "vbe,size", &size)) {
+ if (!default_size) {
+ snprintf(result->err_str, VBE_ERR_STR_LEN,
+ "Missing vbe,size property");
+ return log_msg_ret("byt", -EINVAL);
+ }
+ size = default_size;
+ }
+ if (size > VBE_MAX_RAND_SIZE) {
+ snprintf(result->err_str, VBE_ERR_STR_LEN,
+ "vbe,size %#x exceeds max size %#x", size,
+ VBE_MAX_RAND_SIZE);
+ return log_msg_ret("siz", -E2BIG);
+ }
+ ret = uclass_first_device_check(UCLASS_RNG, &dev);
+ if (ret) {
+ snprintf(result->err_str, VBE_ERR_STR_LEN,
+ "Cannot find random-number device (err=%d)", ret);
+ return log_msg_ret("wr", ret);
+ }
+ ret = dm_rng_read(dev, buf, size);
+ if (ret) {
+ snprintf(result->err_str, VBE_ERR_STR_LEN,
+ "Failed to read random-number device (err=%d)", ret);
+ return log_msg_ret("rd", ret);
+ }
+ ret = ofnode_write_prop(node, "data", buf, size, true);
+ if (ret)
+ return log_msg_ret("wr", -EINVAL);
+
+ return 0;
+}
+
+static int vbe_req_random_seed(ofnode node, struct vbe_result *result)
+{
+ return handle_random_req(node, 0, result);
+}
+
+static int vbe_req_aslr_move(ofnode node, struct vbe_result *result)
+{
+ return -ENOTSUPP;
+}
+
+static int vbe_req_aslr_rand(ofnode node, struct vbe_result *result)
+{
+ return handle_random_req(node, 4, result);
+}
+
+static int vbe_req_efi_runtime_rand(ofnode node, struct vbe_result *result)
+{
+ return handle_random_req(node, 4, result);
+}
+
+static struct vbe_req {
+ const char *compat;
+ vbe_req_func func;
+} vbe_reqs[] = {
+ /* address space layout randomization - move the OS in memory */
+ { "aslr-move", vbe_req_aslr_move },
+
+ /* provide random data for address space layout randomization */
+ { "aslr-rand", vbe_req_aslr_rand },
+
+ /* provide random data for EFI-runtime-services address */
+ { "efi-runtime-rand", vbe_req_efi_runtime_rand },
+
+ /* generate random data bytes to see the OS's rand generator */
+ { "random-rand", vbe_req_random_seed },
+
+};
+
+static int vbe_process_request(ofnode node, struct vbe_result *result)
+{
+ const char *compat, *req_name;
+ int i;
+
+ compat = ofnode_read_string(node, "compatible");
+ if (!compat)
+ return 0;
+
+ if (strlen(compat) <= VBE_PREFIX_LEN ||
+ strncmp(compat, VBE_PREFIX, VBE_PREFIX_LEN))
+ return -EINVAL;
+
+ req_name = compat + VBE_PREFIX_LEN; /* drop "vbe," prefix */
+ for (i = 0; i < ARRAY_SIZE(vbe_reqs); i++) {
+ if (!strcmp(vbe_reqs[i].compat, req_name)) {
+ int ret;
+
+ ret = vbe_reqs[i].func(node, result);
+ if (ret)
+ return log_msg_ret("req", ret);
+ return 0;
+ }
+ }
+ snprintf(result->err_str, VBE_ERR_STR_LEN, "Unknown request: %s",
+ req_name);
+
+ return -ENOTSUPP;
+}
+
+/**
+ * bootmeth_vbe_ft_fixup() - Process VBE OS requests and do device tree fixups
+ *
+ * If there are no images provided, this does nothing and returns 0.
+ *
+ * @ctx: Context for event
+ * @event: Event to process
+ * @return 0 if OK, -ve on error
+ */
+static int bootmeth_vbe_ft_fixup(void *ctx, struct event *event)
+{
+ const struct event_ft_fixup *fixup = &event->data.ft_fixup;
+ const struct bootm_headers *images = fixup->images;
+ ofnode parent, dest_parent, root, node;
+ oftree fit;
+
+ if (!images || !images->fit_hdr_os)
+ return 0;
+
+ /* Get the image node with requests in it */
+ log_debug("fit=%p, noffset=%d\n", images->fit_hdr_os,
+ images->fit_noffset_os);
+ fit = oftree_from_fdt(images->fit_hdr_os);
+ root = oftree_root(fit);
+ if (of_live_active()) {
+ log_warning("Cannot fix up live tree\n");
+ return 0;
+ }
+ if (!ofnode_valid(root))
+ return log_msg_ret("rt", -EINVAL);
+ parent = noffset_to_ofnode(root, images->fit_noffset_os);
+ if (!ofnode_valid(parent))
+ return log_msg_ret("img", -EINVAL);
+ dest_parent = oftree_path(fixup->tree, "/chosen");
+ if (!ofnode_valid(dest_parent))
+ return log_msg_ret("dst", -EINVAL);
+
+ ofnode_for_each_subnode(node, parent) {
+ const char *name = ofnode_get_name(node);
+ struct vbe_result result;
+ ofnode dest;
+ int ret;
+
+ log_debug("copy subnode: %s\n", name);
+ ret = ofnode_add_subnode(dest_parent, name, &dest);
+ if (ret && ret != -EEXIST)
+ return log_msg_ret("add", ret);
+ ret = ofnode_copy_props(node, dest);
+ if (ret)
+ return log_msg_ret("cp", ret);
+
+ *result.err_str = '\0';
+ ret = vbe_process_request(dest, &result);
+ if (ret) {
+ result.errnum = ret;
+ log_err("Failed to process VBE request %s (err=%d)\n",
+ ofnode_get_name(dest), ret);
+ if (*result.err_str) {
+ char *msg = strdup(result.err_str);
+
+ if (!msg)
+ return log_msg_ret("msg", -ENOMEM);
+ ret = ofnode_write_string(dest, "vbe,error",
+ msg);
+ if (ret) {
+ free(msg);
+ return log_msg_ret("str", -ENOMEM);
+ }
+ }
+ if (result.errnum) {
+ ret = ofnode_write_u32(dest, "vbe,errnum",
+ result.errnum);
+ if (ret)
+ return log_msg_ret("num", -ENOMEM);
+ if (result.errnum != -ENOTSUPP)
+ return log_msg_ret("pro",
+ result.errnum);
+ if (result.errnum == -ENOTSUPP &&
+ ofnode_read_bool(dest, "vbe,required")) {
+ log_err("Cannot handle required request: %s\n",
+ ofnode_get_name(dest));
+ return log_msg_ret("req",
+ result.errnum);
+ }
+ }
+ }
+ }
+
+ return 0;
+}
+EVENT_SPY(EVT_FT_FIXUP, bootmeth_vbe_ft_fixup);
diff --git a/test/py/tests/test_event_dump.py b/test/py/tests/test_event_dump.py
index bc54149e8f2..e63c25df537 100644
--- a/test/py/tests/test_event_dump.py
+++ b/test/py/tests/test_event_dump.py
@@ -16,6 +16,7 @@ def test_event_dump(u_boot_console):
out = util.run_and_log(cons, ['scripts/event_dump.py', sandbox])
expect = '''.*Event type Id Source location
-------------------- ------------------------------ ------------------------------
+EVT_FT_FIXUP bootmeth_vbe_ft_fixup .*boot/vbe_fixup.c:.*
EVT_FT_FIXUP bootmeth_vbe_simple_ft_fixup .*boot/vbe_simple.c:.*
EVT_MISC_INIT_F sandbox_misc_init_f .*arch/sandbox/cpu/start.c:'''
assert re.match(expect, out, re.MULTILINE) is not None
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 14/14] vbe: Add a test for VBE device tree fixups
2022-09-09 15:17 [PATCH 00/14] vbe: Support device tree fixups for OS requests Simon Glass
` (12 preceding siblings ...)
2022-09-09 15:18 ` [PATCH 13/14] vbe: Add fixups for a basic set of OS requests Simon Glass
@ 2022-09-09 15:18 ` Simon Glass
13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2022-09-09 15:18 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Simon Glass
When a FIT includes some OS requests, U-Boot should process these and add
the requested info to corresponding subnodes of the /chosen node. Add a
pytest for this, which sets up the FIT, runs bootm and then uses a C
unit test to check that everything looks OK.
The test needs to run on sandbox_flattree since we don't support
device tree fixups on sandbox (live tree) yet. So enable BOOTMETH_VBE and
disable bootflow_system(), since EFI is not supported on
sandbox_flattree.
Add a link to the initial documentation.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
configs/sandbox_flattree_defconfig | 2 +-
doc/develop/vbe.rst | 3 +-
test/boot/Makefile | 1 +
test/boot/bootflow.c | 2 +
test/boot/vbe_fixup.c | 59 ++++++++++++++
test/py/tests/fit_util.py | 5 +-
test/py/tests/test_vbe.py | 123 +++++++++++++++++++++++++++++
7 files changed, 192 insertions(+), 3 deletions(-)
create mode 100644 test/boot/vbe_fixup.c
create mode 100644 test/py/tests/test_vbe.py
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index a8b439faa96..58fd4ad6395 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -11,7 +11,6 @@ CONFIG_DISTRO_DEFAULTS=y
CONFIG_FIT=y
CONFIG_FIT_SIGNATURE=y
CONFIG_FIT_VERBOSE=y
-# CONFIG_BOOTMETH_VBE is not set
CONFIG_BOOTSTAGE=y
CONFIG_BOOTSTAGE_REPORT=y
CONFIG_BOOTSTAGE_FDT=y
@@ -83,6 +82,7 @@ CONFIG_REGMAP=y
CONFIG_SYSCON=y
CONFIG_DEVRES=y
CONFIG_DEBUG_DEVRES=y
+CONFIG_OFNODE_MULTI_TREE=y
CONFIG_ADC=y
CONFIG_ADC_SANDBOX=y
CONFIG_SYS_SATA_MAX_DEVICE=2
diff --git a/doc/develop/vbe.rst b/doc/develop/vbe.rst
index 8f147fd9360..cca193c8fd4 100644
--- a/doc/develop/vbe.rst
+++ b/doc/develop/vbe.rst
@@ -19,8 +19,9 @@ listing methods and getting the status for a method.
For a detailed overview of VBE, see vbe-intro_. A fuller description of
bootflows is at vbe-bootflows_ and the firmware-update mechanism is described at
-vbe-fwupdate_.
+vbe-fwupdate_. VBE OS requests are described at vbe-osrequests_.
.. _vbe-intro: https://docs.google.com/document/d/e/2PACX-1vQjXLPWMIyVktaTMf8edHZYDrEvMYD_iNzIj1FgPmKF37fpglAC47Tt5cvPBC5fvTdoK-GA5Zv1wifo/pub
.. _vbe-bootflows: https://docs.google.com/document/d/e/2PACX-1vR0OzhuyRJQ8kdeOibS3xB1rVFy3J4M_QKTM5-3vPIBNcdvR0W8EXu9ymG-yWfqthzWoM4JUNhqwydN/pub
.. _vbe-fwupdate: https://docs.google.com/document/d/e/2PACX-1vTnlIL17vVbl6TVoTHWYMED0bme7oHHNk-g5VGxblbPiKIdGDALE1HKId8Go5f0g1eziLsv4h9bocbk/pub
+.. _vbe-osrequests: https://docs.google.com/document/d/e/2PACX-1vTHhxX7WSZe68i9rAkW-DHdx6koU-jxYHhamLhZn9GQ9QT2_epSBosMV1_r7yPHOXZccx71rF_t0PXL/pub
diff --git a/test/boot/Makefile b/test/boot/Makefile
index 9e9d5ae21f3..5bb3f889759 100644
--- a/test/boot/Makefile
+++ b/test/boot/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_BOOTSTD) += bootdev.o bootstd_common.o bootflow.o bootmeth.o
ifdef CONFIG_OF_LIVE
obj-$(CONFIG_BOOTMETH_VBE_SIMPLE) += vbe_simple.o
endif
+obj-$(CONFIG_BOOTMETH_VBE) += vbe_fixup.o
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 85305234e01..1e8ea754bcd 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -329,6 +329,8 @@ static int bootflow_system(struct unit_test_state *uts)
{
struct udevice *dev;
+ if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR))
+ return 0;
ut_assertok(uclass_get_device_by_name(UCLASS_BOOTMETH, "efi_mgr",
&dev));
sandbox_set_fake_efi_mgr_dev(dev, true);
diff --git a/test/boot/vbe_fixup.c b/test/boot/vbe_fixup.c
new file mode 100644
index 00000000000..1b488e25ab6
--- /dev/null
+++ b/test/boot/vbe_fixup.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Test for VBE device tree fix-ups
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <common.h>
+#include <dm/ofnode.h>
+#include <linux/libfdt.h>
+#include <test/test.h>
+#include <test/ut.h>
+#include "bootstd_common.h"
+
+/* Basic test of reading nvdata and updating a fwupd node in the device tree */
+static int vbe_test_fixup(struct unit_test_state *uts)
+{
+ ofnode chosen, node;
+ const char *data;
+ oftree tree;
+ int size;
+
+ /*
+ * This test works when called from test_vbe.py and it must use the
+ * flat tree, since device tree fix-ups do not yet support live tree.
+ */
+ if (!working_fdt)
+ return 0;
+
+ tree = oftree_from_fdt(working_fdt);
+ ut_assert(oftree_valid(tree));
+
+ chosen = oftree_path(tree, "/chosen");
+ ut_assert(ofnode_valid(chosen));
+
+ /* check the things set up for the FIT in test_vbe.py */
+ node = ofnode_find_subnode(chosen, "random");
+
+ /* ignore if this test is run on its own */
+ if (!ofnode_valid(node))
+ return 0;
+ data = ofnode_read_prop(node, "data", &size);
+ ut_asserteq(0x40, size);
+
+ node = ofnode_find_subnode(chosen, "aslr2");
+ ut_assert(ofnode_valid(node));
+ data = ofnode_read_prop(node, "data", &size);
+ ut_asserteq(4, size);
+
+ node = ofnode_find_subnode(chosen, "efi-runtime");
+ ut_assert(ofnode_valid(node));
+ data = ofnode_read_prop(node, "data", &size);
+ ut_asserteq(4, size);
+
+ return 0;
+}
+BOOTSTD_TEST(vbe_test_fixup,
+ UT_TESTF_DM | UT_TESTF_SCAN_FDT | UT_TESTF_FLAT_TREE);
diff --git a/test/py/tests/fit_util.py b/test/py/tests/fit_util.py
index fcec4c43c3c..79718d431a0 100644
--- a/test/py/tests/fit_util.py
+++ b/test/py/tests/fit_util.py
@@ -36,7 +36,7 @@ def make_its(cons, base_its, params, basename='test.its'):
print(base_its % params, file=outf)
return its
-def make_fit(cons, mkimage, base_its, params, basename='test.fit'):
+def make_fit(cons, mkimage, base_its, params, basename='test.fit', base_fdt=None):
"""Make a sample .fit file ready for loading
This creates a .its script with the selected parameters and uses mkimage to
@@ -55,6 +55,9 @@ def make_fit(cons, mkimage, base_its, params, basename='test.fit'):
fit = make_fname(cons, basename)
its = make_its(cons, base_its, params)
util.run_and_log(cons, [mkimage, '-f', its, fit])
+ if base_fdt:
+ with open(make_fname(cons, 'u-boot.dts'), 'w') as fd:
+ fd.write(base_fdt)
return fit
def make_kernel(cons, basename, text):
diff --git a/test/py/tests/test_vbe.py b/test/py/tests/test_vbe.py
new file mode 100644
index 00000000000..559c2918868
--- /dev/null
+++ b/test/py/tests/test_vbe.py
@@ -0,0 +1,123 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2022 Google LLC
+#
+# Test addition of VBE
+
+import pytest
+
+import fit_util
+
+# Define a base ITS which we can adjust using % and a dictionary
+base_its = '''
+/dts-v1/;
+
+/ {
+ description = "Example kernel";
+
+ images {
+ kernel-1 {
+ data = /incbin/("%(kernel)s");
+ type = "kernel";
+ arch = "sandbox";
+ os = "linux";
+ load = <0x40000>;
+ entry = <0x8>;
+ compression = "%(compression)s";
+
+ random {
+ compatible = "vbe,random-rand";
+ vbe,size = <0x40>;
+ vbe,required;
+ };
+ aslr1 {
+ compatible = "vbe,aslr-move";
+ vbe,align = <0x100000>;
+ };
+ aslr2 {
+ compatible = "vbe,aslr-rand";
+ };
+ efi-runtime {
+ compatible = "vbe,efi-runtime-rand";
+ };
+ wibble {
+ compatible = "vbe,wibble";
+ };
+ };
+
+ fdt-1 {
+ description = "snow";
+ data = /incbin/("%(fdt)s");
+ type = "flat_dt";
+ arch = "sandbox";
+ load = <%(fdt_addr)#x>;
+ compression = "%(compression)s";
+ };
+ };
+ configurations {
+ default = "conf-1";
+ conf-1 {
+ kernel = "kernel-1";
+ fdt = "fdt-1";
+ };
+ };
+};
+'''
+
+# Define a base FDT - currently we don't use anything in this
+base_fdt = '''
+/dts-v1/;
+
+/ {
+ chosen {
+ };
+};
+'''
+
+# This is the U-Boot script that is run for each test. First load the FIT,
+# then run the 'bootm' command, then run the unit test which checks that the
+# working tree has the required things filled in according to the OS requests
+# above (random, aslr2, etc.)
+base_script = '''
+host load hostfs 0 %(fit_addr)x %(fit)s
+fdt addr %(fit_addr)x
+bootm start %(fit_addr)x
+bootm loados
+bootm prep
+fdt addr
+fdt print
+ut bootstd vbe_test_fixup
+'''
+
+@pytest.mark.boardspec('sandbox_flattree')
+@pytest.mark.requiredtool('dtc')
+def test_vbe(u_boot_console):
+ cons = u_boot_console
+ kernel = fit_util.make_kernel(cons, 'vbe-kernel.bin', 'kernel')
+ fdt = fit_util.make_dtb(cons, base_fdt, 'vbe-fdt')
+ fdt_out = fit_util.make_fname(cons, 'fdt-out.dtb')
+
+ params = {
+ 'fit_addr' : 0x1000,
+
+ 'kernel' : kernel,
+
+ 'fdt' : fdt,
+ 'fdt_out' : fdt_out,
+ 'fdt_addr' : 0x80000,
+ 'fdt_size' : 0x1000,
+
+ 'compression' : 'none',
+ }
+ mkimage = cons.config.build_dir + '/tools/mkimage'
+ fit = fit_util.make_fit(cons, mkimage, base_its, params, 'test-vbe.fit',
+ base_fdt)
+ params['fit'] = fit
+ cmd = base_script % params
+
+ with cons.log.section('Kernel load'):
+ output = cons.run_command_list(cmd.splitlines())
+
+ # This is a little wonky since there are two tests running in CI. The final
+ # one is the 'ut bootstd' command above
+ failures = [line for line in output if 'Failures' in line]
+ assert len(failures) >= 1 and 'Failures: 0' in failures[-1]
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread