Re: [PATCH v3 16/18] rockchip: Avoid #ifdefs in RK3399 SPL
Hi Simon,
On 2024/6/21 22:57, Simon Glass wrote:
Hi Jonas,
On Fri, 21 Jun 2024 at 00:45, Jonas Karlman wrote:
Hi Simon,
On 2024-06-21 01:06, Simon Glass wrote:
The code here is confusing due to large blocks which are #ifdefed out.
Add a function phase_sdram_init() which returns whether SDRAM init
should happen in the current phase, using that as needed to control the
code flow.
This increases code size by about 500 bytes in SPL when the cache is on,
since it must call the rather large rockchip_sdram_size() function.
Signed-off-by: Simon Glass
---
Changes in v3:
- Split out the refactoring into a separate patch
- Drop the non-dcache optimisation, since the cache should normally be on
drivers/ram/rockchip/sdram_rk3399.c | 47 -
1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/drivers/ram/rockchip/sdram_rk3399.c
b/drivers/ram/rockchip/sdram_rk3399.c
index 3c4e20f4e80..2f37dd712e7 100644
--- a/drivers/ram/rockchip/sdram_rk3399.c
+++ b/drivers/ram/rockchip/sdram_rk3399.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -63,8 +64,6 @@ struct chan_info {
};
struct dram_info {
-#if defined(CONFIG_TPL_BUILD) || \
- (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
u32 pwrup_srefresh_exit[2];
struct chan_info chan[2];
struct clk ddr_clk;
@@ -75,7 +74,6 @@ struct dram_info {
struct rk3399_pmusgrf_regs *pmusgrf;
struct rk3399_ddr_cic_regs *cic;
const struct sdram_rk3399_ops *ops;
-#endif
struct ram_info info;
struct rk3399_pmugrf_regs *pmugrf;
};
@@ -92,9 +90,6 @@ struct sdram_rk3399_ops {
struct rk3399_sdram_params *params);
};
-#if defined(CONFIG_TPL_BUILD) || \
- (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
-
struct rockchip_dmc_plat {
#if CONFIG_IS_ENABLED(OF_PLATDATA)
struct dtd_rockchip_rk3399_dmc dtplat;
@@ -191,6 +186,17 @@ struct io_setting {
},
};
+/**
+ * phase_sdram_init() - Check if this is the phase where SDRAM init happens
+ *
+ * Returns: true to do SDRAM init in this phase, false to not
+ */
+static bool phase_sdram_init(void)
+{
+ return spl_phase() == PHASE_TPL ||
+ (!IS_ENABLED(CONFIG_TPL) && !spl_in_proper());
+}
+
static struct io_setting *
lpddr4_get_io_settings(const struct rk3399_sdram_params *params, u32 mr5)
{
@@ -3024,7 +3030,7 @@ static int rk3399_dmc_of_to_plat(struct udevice *dev)
struct rockchip_dmc_plat *plat = dev_get_plat(dev);
int ret;
- if (!CONFIG_IS_ENABLED(OF_REAL))
+ if (!CONFIG_IS_ENABLED(OF_REAL) || !phase_sdram_init())
return 0;
ret = dev_read_u32_array(dev, "rockchip,sdram-params",
@@ -3138,22 +3144,24 @@ static int rk3399_dmc_init(struct udevice *dev)
return 0;
}
-#endif
static int rk3399_dmc_probe(struct udevice *dev)
{
struct dram_info *priv = dev_get_priv(dev);
-#if defined(CONFIG_TPL_BUILD) || \
- (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
- if (rk3399_dmc_init(dev))
- return 0;
-#endif
- priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
- debug("%s: pmugrf = %p\n", __func__, priv->pmugrf);
- priv->info.base = CFG_SYS_SDRAM_BASE;
- priv->info.size =
- rockchip_sdram_size((phys_addr_t)>pmugrf->os_reg2);
+ if (phase_sdram_init()) {
+ if (rk3399_dmc_init(dev))
+ return 0;
+ } else {
+ priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
+ debug("%s: pmugrf = %p\n", __func__, priv->pmugrf);
+ }
+
+ if (!CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
This check should be dropped, this driver intent is to initialize dram
in first phase (normally TPL), and report the size to any other phase.
This whole patch can be dropped :-) Here I am just trying to avoid
code-size growth when the cache is off. But yes, it is not needed.
The main need for phase_sdram_init() and disable of DCACHE can probably
be avoided if bob/kevin can move to use separate TPL and SPL instead of
doing both dram init and everything else in SPL.
My best guess is that enabling of caches in SPL cause issue for bob and
kevin because they only use SPL not TPL+SPL like (if I am not mistaken)
all other Rockchip arm64 targets.
Using SPL-only was not something I tested when caches was enabled in SPL.
Maybe bob/kevin can be changed to also use TPL+SPL similar to all other
RK3399 boards?
How U-Boot works on these chromebooks is still a mystery to me.
When coreboot is involved it would only load U-Boot proper and SPL or
TPL+SPL would never be involved at all?
If I understand correctly SPL is only involved for bare metal boot, if
this is the case then using TPL + back-to-brom to load SPL should
probably work fine?, and would align all RK3399 boards to work in a
similar way and reduce the need for special care/handling in the code.
USB gadget device (USB_ETHER) with ChipIdea device controller (imx)
Greetings,
Is anyone successfully using USB gadget device (USB_ETHER) on any imx
that has the ChipIdea device controller? (imx6/imx7/imx8mm):
I've never been able to get this to work as when
usb_setup_ehci_gadget() is called from ci_udc's
usb_gadget_register_driver() it first removes the usb controller (and
its children) before reprobing it and this causes a hang.
Here's an example with v2024.07-rc4 on an imx8mm-venice-gw73xx-0x
(imx8mm_venice_defconfig) after enabling CONFIG_USB_ETHER:
u-boot=> net list
eth0 : ethernet@30be 00:d0:12:ba:f8:cc active
u-boot=> bind usb 0 usb_ether # bind usb_ether to first usb controller
u-boot=> net list
eth0 : ethernet@30be 00:d0:12:ba:f8:cc active
eth2 : usb_ether 00:00:00:00:00:00
u-boot => setenv eth2addr 00:d0:12:ba:f8:cd
u-boot=> net list
eth0 : ethernet@30be 00:d0:12:ba:f8:cc active
eth2 : usb_ether 00:d0:12:ba:f8:cd
u-boot=> setenv ethact usb_ether; setenv ipaddr 10.0.0.2; setenv
netmask 255.255.255.0; ping 10.0.0.1
^^^ board hangs, USB host connected to board does not enumerate a CDC
ether device
Unbinding ethernet via 'unbind ethernet 0' first does not appear to
make a difference.
The following patch resolves this however I don't think this is the
right fix as that was apparently added for a reason in commit
fbeceb260232a ("dm: usb: Allow setting up a USB controller as a
device/gadget"):
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index a1cd0ad2d669..38111fda8af9 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -403,6 +403,7 @@ int usb_setup_ehci_gadget(struct ehci_ctrl **ctlrp)
ret = uclass_find_first_device(UCLASS_USB, );
if (ret)
return ret;
+#if 0
ret = device_remove(dev, DM_REMOVE_NORMAL);
if (ret)
return ret;
@@ -412,6 +413,7 @@ int usb_setup_ehci_gadget(struct ehci_ctrl **ctlrp)
ret = device_probe(dev);
if (ret)
return ret;
+#endif
*ctlrp = dev_get_priv(dev);
return 0;
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index a1cd0ad2d669..38111fda8af9 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -403,6 +403,7 @@ int usb_setup_ehci_gadget(struct ehci_ctrl **ctlrp)
ret = uclass_find_first_device(UCLASS_USB, );
if (ret)
return ret;
+#if 0
ret = device_remove(dev, DM_REMOVE_NORMAL);
if (ret)
return ret;
@@ -412,6 +413,7 @@ int usb_setup_ehci_gadget(struct ehci_ctrl **ctlrp)
ret = device_probe(dev);
if (ret)
return ret;
+#endif
*ctlrp = dev_get_priv(dev);
return 0;
Adding debugging to device_remove the hang occurs when removing
'usb_ether.bootdev' and I'm not quite sure where this device even
comes from:
usb_setup_ehci_gadget removing usb@32e4
device_remove usb@32e4 flags=0x1
device_remove usb_ether flags=0x1
device_remove usb_ether.bootdev flags=0x1
I don't understand why usb_setup_ehci_gadget() is removing the device
in the first place just to go and probe it again.
This is not a regression... I've never seen this work since I first
tried it back in Feb 2022 [1]
Any ideas?
Best Regards,
Tim
[1]
https://lore.kernel.org/all/CAJ+vNU1xH-J-0RV09OuM1e+hsRV=hxvh2os6pfcwy5d+0st...@mail.gmail.com/
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Fri, Jun 21, 2024 at 01:38:07PM -0600, Simon Glass wrote: > Hi Tom, > > On Fri, 21 Jun 2024 at 13:19, Tom Rini wrote: > > > > On Fri, Jun 21, 2024 at 11:55:42AM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Fri, 21 Jun 2024 at 10:05, Tom Rini wrote: > > [snip] > > > > Yes, I very much do not like guessing about 3 numbers instead of > > > > guessing about 1 number and using the standard mechanism we already > > > > have. Please use BOARD_SIZE_LIMIT as this is the standard mechanism to > > > > enforce size limits on U-Boot itself. > > > > > > If it were that easy I would have sent a patch :-) > > > > > > Here is the map for this board: > > > > > > ImagePosOffset Size Name > > > 0080 rom > > > ff80 ff80 1000 intel-descriptor > > > ff801000 ff801000 001ff000 intel-me > > > ffef ffef 000999f0 u-boot-with-ucode-ptr > > > fff899f0 fff899f0 5554 u-boot-dtb-with-ucode > > > fff8ef50 fff8ef50 u-boot-ucode > > > fff8ef50 fff8ef50 0571 fdtmap > > > fff9 fff9 0001 intel-vga > > > fffa fffa 0002fc94 intel-mrc > > > fffcfc94 fffcfc94 private-files > > > f800 f800 0070 x86-start16 > > > fff0 fff0 0005 x86-reset16 > > > fff8 fff8 0008 image-header > > > > > > What limit should I set on what? > > > > Is this a trick question? > > $ printf %d\\n $(( 0xfff9 - 0xffef)) > > 655360 > > > > Of course since we're less than that today, you can reduce it by > > whatever other magic numbers I'm not seeing but are part of your assumed > > sizes. > > That limit is on u-boot-nodtb.bin. Even with a size (for that file) of > 634816 it doesn't fit. I need to calculate a size based on the size of > the dtb and the microcode...which of course can change. Yes, and you're able to assume some size for them, which is what you put in the dts file? > > > - the U-Boot is the thing you are wanting to limit > > > - the dtb has microcode added > > > - the ucode is empty in this case > > > - the fdtmap is variable in size > > > > > > So this all seems a bit backwards. The actual limit is that > > > (u-boot-with-ucode-ptr + u-boot-dtb-with-ucode + u-boot-ucode + > > > fdtmap) fits in the space available. Note that some boards don't have > > > intel-vga or intel-mrc. > > > > > > With the other patch I sent I can have a sensible limit for all x86 > > > boards. > > Did you miss the comments above? No, I saw them. They're similar constraints to other systems. > > > > > And you can set the same sensible limit with the existing mechanism with > > the bonus of it not making x86 different from the rest? > > I understand that it is possible to set a limit for u-boot-nodtb.bin > but that is not accurate nor sufficient in the presence of blobs. The > solution belongs in Binman. Your series puts reasonable estimates on the size of the blobs and then will give a failure such as: binman: Node '/binman/rom/intel-vga': Offset 0xfff9 (4294508544) overlaps with previous entry '/binman/rom/fdtmap' ending at 0xfff902e1 (4294509281) Which is not as nice as (I just threw in a limit): u-boot-nodtb.bin exceeds file size limit: limit: 0x927c0 bytes actual: 0x9a810 bytes excess: 0x8050 bytes make[1]: *** [/home/trini/work/u-boot/u-boot/Makefile:1359: u-boot-nodtb.bin] Error 1 And tells us how much we need to get back size wise. Aside from when using the actual blobs (and in which case a real error will be shown when trying to use them), it's always about making an estimate on the part of the system that we control. -- Tom signature.asc Description: PGP signature
Re: [PATCH 3/5] buildman: Support building within a Python venv
On Fri, Jun 21, 2024 at 01:39:46PM -0600, Simon Glass wrote:
> Hi Tom,
>
> On Fri, 21 Jun 2024 at 13:26, Tom Rini wrote:
> >
> > On Fri, Jun 21, 2024 at 12:19:12PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Fri, 21 Jun 2024 at 09:22, Tom Rini wrote:
> > > >
> > > > On Fri, Jun 21, 2024 at 08:57:50AM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Thu, 20 Jun 2024 at 17:30, Tom Rini wrote:
> > > > > >
> > > > > > On Thu, Jun 20, 2024 at 05:05:30PM -0600, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Thu, 20 Jun 2024 at 08:32, Tom Rini wrote:
> > > > > > > >
> > > > > > > > On Thu, Jun 20, 2024 at 07:19:35AM -0600, Simon Glass wrote:
> > > > > > > >
> > > > > > > > > The Python virtualenv tool sets up a few things in the
> > > > > > > > > envronment,
> > > > > > > > > putting its path first in the PATH environment variable and
> > > > > > > > > setting up
> > > > > > > > > a sys.prefix different from the sys.base_prefix value.
> > > > > > > > >
> > > > > > > > > At present buildman puts the toolchain path first in PATH so
> > > > > > > > > that it can
> > > > > > > > > be found easily during the build. For sandbox this causes
> > > > > > > > > problems since
> > > > > > > > > /usr/bin/gcc (for example) results in '/usr/bin' being
> > > > > > > > > prepended to the
> > > > > > > > > PATH variable. As a result, the venv is partially disabled.
> > > > > > > > >
> > > > > > > > > The result is that sandbox builds within a venv ignore the
> > > > > > > > > venv, e.g.
> > > > > > > > > when looking for packages.
> > > > > > > > >
> > > > > > > > > Correct this by detecting the venv and adding the toolchain
> > > > > > > > > path after
> > > > > > > > > the venv path.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass
> > > > > > > >
> > > > > > > > Why are we using PATH at all in this case? Shouldn't we just be
> > > > > > > > setting
> > > > > > > > CROSS_COMPILE=/full/path/to/the/prefix ?
> > > > > > >
> > > > > > > This is the -p option to buildman. The original commit was:
> > > > > > >
> > > > > > > commit bb1501f2c22c979961b735db775605cccedd98f6
> > > > > > > Author: Simon Glass
> > > > > > > Date: Mon Dec 1 17:34:00 2014 -0700
> > > > > > >
> > > > > > > buildman: Add an option to use the full tool chain path
> > > > > > >
> > > > > > > In some cases there may be multiple toolchains with the same
> > > > > > > name in the
> > > > > > > path. Provide an option to use the full path in the
> > > > > > > CROSS_COMPILE
> > > > > > > environment variable.
> > > > > > >
> > > > > > > Note: Wolfgang mentioned that this is dangerous since in some
> > > > > > > cases there
> > > > > > > may be other tools on the path that are needed. So this is
> > > > > > > set up as an
> > > > > > > option, not the default. I will need test confirmation (i.e.
> > > > > > > that this
> > > > > > > commit fixes a real problem) before merging it.
> > > > > > >
> > > > > > > As to why we don't always do this, well that is back in the mists
> > > > > > > of
> > > > > > > time, 10 years ago.
> > > > > > >
> > > > > > > BTW, this is raising a point ("let's change the behaviour")
> > > > > > > separate
> > > > > > > from the goal of this commit, which is to fix a problem with venv,
> > > > > > > albeit that if we made -p the only option, then we could
> > > > > > > potentially
> > > > > > > drop all PATH changes. Perhaps toolchains are built differently
> > > > > > > now,
> > > > > > > such that they always invoke their tools using the same prefix and
> > > > > > > dir?
> > > > > >
> > > > > > Wait, I'm confused. buildman internally updates its own PATH to
> > > > > > avoid
> > > > > > calling CROSS_COMPILE with the full path due to a concern about
> > > > > > toolchain bugs?
> > > > >
> > > > > Not its own PATH: the one it passes to U-Boot's 'make'.
> > > >
> > > > OK, but the point stands.
> > > >
> > > > > I'm not sure why, actually. It is such a long time ago that I don't
> > > > > remember.
> > > > >
> > > > > I see:
> > > > >
> > > > > ~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-ld
> > > >
> > > > Yes, prefixed version that's allowed to be called by users.
> > > >
> > > > > and
> > > > >
> > > > > ~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/arm-linux-gnueabi/bin/ld
> > > >
> > > > Internal usage, here be dragons and all that.
> > > >
> > > > > but interestingly there is no gcc in the latter directory, which there
> > > > > was in 4.6 (and presumably for some time after).
> > > > >
> > > > > Certainly for sandbox there is no prefix, so we cannot add it in that
> > > > > case, and sandbox is actually the arch used to run these tests.
> > > >
> > > > CROSS_COMPILE is empty for sandbox, yes.
> > > >
> > > > > What are you suggesting we change about this patch?
> > > >
> > > > That it's going about things backwards? If you're setting CROSS_COMPILE
> > > >
[PATCH 11/11] buildman: Add a way to limit the number of buildmans
Buildman uses all available CPUs by default, so running more than one or
two concurrent processes is not normally useful.
However in some CI cases we want to be able to run several jobs at once
to save time. For example, in a lab situation we may want to run a test
on 20 boards at a time, since only the build step actually takes much
CPU.
Add an option which allows such a limit. When buildman starts up, it
waits until the number of running processes goes below the limit, then
claims a spot in the list. The list is maintained with a temporary file.
Note that the temp file is user-specific, since it is hard to create a
locked temporary file which can be accessed by any user. In most cases,
only one user is running jobs on a machine, so this should not matter.
Signed-off-by: Simon Glass
---
tools/buildman/buildman.rst| 5 ++
tools/buildman/cmdline.py | 2 +
tools/buildman/control.py | 140 -
tools/buildman/pyproject.toml | 6 +-
tools/buildman/test.py | 121
tools/u_boot_pylib/terminal.py | 7 +-
6 files changed, 277 insertions(+), 4 deletions(-)
diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst
index bd0482af5f7..b8ff3bf1ab2 100644
--- a/tools/buildman/buildman.rst
+++ b/tools/buildman/buildman.rst
@@ -1286,6 +1286,11 @@ then buildman hangs. Failing to handle any eventuality
is a bug in buildman and
should be reported. But you can use -T0 to disable threading and hopefully
figure out the root cause of the build failure.
+For situations where buildman is invoked from multiple running processes, it is
+sometimes useful to have buildman wait until the others have finished. Use the
+--process-limit option for this: --process-limit 1 will allow only one buildman
+to process jobs at a time.
+
Build summary
-
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 8dc5a8787b5..544a391a464 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -129,6 +129,8 @@ def add_after_m(parser):
default=False, help="Use an O= (output) directory per board rather
than per thread")
parser.add_argument('--print-arch', action='store_true',
default=False, help="Print the architecture for a board (ARCH=)")
+parser.add_argument('--process-limit', type=int,
+ default=0, help='Limit to number of buildmans running at once')
parser.add_argument('-r', '--reproducible-builds', action='store_true',
help='Set SOURCE_DATE_EPOCH=0 to suuport a reproducible build')
parser.add_argument('-R', '--regen-board-list', type=str,
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index f2dd87814c3..464835c5be5 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -7,10 +7,13 @@
This holds the main control logic for buildman, when not running tests.
"""
+import getpass
import multiprocessing
import os
import shutil
import sys
+import tempfile
+import time
from buildman import boards
from buildman import bsettings
@@ -21,10 +24,23 @@ from patman import gitutil
from patman import patchstream
from u_boot_pylib import command
from u_boot_pylib import terminal
-from u_boot_pylib.terminal import tprint
+from u_boot_pylib import tools
+from u_boot_pylib.terminal import print_clear, tprint
TEST_BUILDER = None
+# Space-separated list of buildman process IDs currently running jobs
+RUNNING_FNAME = f'buildmanq.{getpass.getuser()}'
+
+# Lock file for access to RUNNING_FILE
+LOCK_FNAME = f'{RUNNING_FNAME}.lock'
+
+# Wait time for access to lock (seconds)
+LOCK_WAIT_S = 10
+
+# Wait time to start running
+RUN_WAIT_S = 300
+
def get_plural(count):
"""Returns a plural 's' if count is not 1"""
return 's' if count != 1 else ''
@@ -578,6 +594,125 @@ def calc_adjust_cfg(adjust_cfg, reproducible_builds):
return adjust_cfg
+def read_procs(tmpdir=tempfile.gettempdir()):
+"""Read the list of running buildman processes
+
+If the list is corrupted, returns an empty list
+
+Args:
+tmpdir (str): Temporary directory to use (for testing only)
+"""
+running_fname = os.path.join(tmpdir, RUNNING_FNAME)
+procs = []
+if os.path.exists(running_fname):
+items = tools.read_file(running_fname, binary=False).split()
+try:
+procs = [int(x) for x in items]
+except ValueError: # Handle invalid format
+pass
+return procs
+
+
+def check_pid(pid):
+"""Check for existence of a unix PID
+
+
https://stackoverflow.com/questions/568271/how-to-check-if-there-exists-a-process-with-a-given-pid-in-python
+
+Args:
+pid (int): PID to check
+
+Returns:
+True if it exists, else False
+"""
+try:
+os.kill(pid, 0)
+except OSError:
+return False
+else:
+return True
+
+
+def write_procs(procs, tmpdir=tempfile.gettempdir()):
+"""Write the list of
[PATCH 09/11] buildman: Add a flag to force mrproper on failure
When a file is removed by a commit (e.g. include/common.h yay!) it can cause incremental build failures since one of the dependency files from a previous build may mention the file. Add an option to run 'make mrproper' automatically when a build fails. This can be used to automatically resolve the problem, without always adding the large overhead of 'make mrproper' to every build. Signed-off-by: Simon Glass --- tools/buildman/builder.py | 18 ++ tools/buildman/builderthread.py | 6 -- tools/buildman/buildman.rst | 3 ++- tools/buildman/cmdline.py | 4 +++- tools/buildman/control.py | 1 + 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index f35175b4598..c4384f53e8d 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -256,14 +256,14 @@ class Builder: def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1, no_subdirs=False, full_path=False, verbose_build=False, - mrproper=False, per_board_out_dir=False, - config_only=False, squash_config_y=False, - warnings_as_errors=False, work_in_output=False, - test_thread_exceptions=False, adjust_cfg=None, - allow_missing=False, no_lto=False, reproducible_builds=False, - force_build=False, force_build_failures=False, - force_reconfig=False, in_tree=False, - force_config_on_failure=False, make_func=None): + mrproper=False, fallback_mrproper=False, + per_board_out_dir=False, config_only=False, + squash_config_y=False, warnings_as_errors=False, + work_in_output=False, test_thread_exceptions=False, + adjust_cfg=None, allow_missing=False, no_lto=False, + reproducible_builds=False, force_build=False, + force_build_failures=False, force_reconfig=False, + in_tree=False, force_config_on_failure=False, make_func=None): """Create a new Builder object Args: @@ -283,6 +283,7 @@ class Builder: PATH verbose_build: Run build with V=1 and don't use 'make -s' mrproper: Always run 'make mrproper' when configuring +fallback_mrproper: Run 'make mrproper' and retry on build failure per_board_out_dir: Build in a separate persistent directory per board rather than a thread-specific directory config_only: Only configure each build, don't build it @@ -352,6 +353,7 @@ class Builder: self.force_reconfig = force_reconfig self.in_tree = in_tree self.force_config_on_failure = force_config_on_failure +self.fallback_mrproper = fallback_mrproper if not self.squash_config_y: self.config_filenames += EXTRA_CONFIG_FILENAMES diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index c0b1067e3f7..bbe2f6f0d24 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -705,8 +705,10 @@ class BuilderThread(threading.Thread): # with a reconfig. if self.builder.force_config_on_failure: result, request_config = self.run_commit(commit_upto, -brd, work_dir, True, self.mrproper, False, True, -False, job.work_in_output, job.adjust_cfg) +brd, work_dir, True, +self.mrproper or self.builder.fallback_mrproper, +False, True, False, job.work_in_output, +job.adjust_cfg) did_config = True if not self.builder.force_reconfig: do_config = request_config diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst index aae2477b5c3..bd0482af5f7 100644 --- a/tools/buildman/buildman.rst +++ b/tools/buildman/buildman.rst @@ -995,7 +995,8 @@ By default, buildman doesn't execute 'make mrproper' prior to building the first commit for each board. This reduces the amount of work 'make' does, and hence speeds up the build. To force use of 'make mrproper', use -the -m flag. This flag will slow down any buildman invocation, since it increases the amount -of work done on any build. +of work done on any build. An alternative is to use the --fallback-mrproper +flag, which retries the build with 'make mrproper' only after a build failure. One possible application of buildman is as part of a continual edit, build, edit, build, ... cycle; repeatedly applying buildman to the same change or diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 03211bd5aa5..8dc5a8787b5 100644 ---
[PATCH 10/11] buildman: Retry the build for current source
Buildman retries a failed build when processing a branch, but does not do this when building current source. It is useful to do this retry in both cases, so add the logic for it. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 8 1 file changed, 8 insertions(+) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index bbe2f6f0d24..55658487abf 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -755,6 +755,14 @@ class BuilderThread(threading.Thread): self.mrproper, self.builder.config_only, True, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) +failed = result.return_code or result.stderr +if failed and not self.mrproper: +result, request_config = self.run_commit(None, brd, work_dir, +True, self.builder.fallback_mrproper, +self.builder.config_only, True, +self.builder.force_build_failures, +job.work_in_output, job.adjust_cfg) + result.commit_upto = 0 self._write_result(result, job.keep_outputs, job.work_in_output) self._send_result(result) -- 2.34.1
[PATCH 07/11] buildman: Make mrproper an argument to run_commit()
Pass this in so the caller can change it independently of the member variable. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index ff63f9397e6..0a7ff2e083e 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -448,9 +448,9 @@ class BuilderThread(threading.Thread): result.cmd_list = cmd_list return result, do_config -def run_commit(self, commit_upto, brd, work_dir, do_config, config_only, - force_build, force_build_failures, work_in_output, - adjust_cfg): +def run_commit(self, commit_upto, brd, work_dir, do_config, mrproper, + config_only, force_build, force_build_failures, + work_in_output, adjust_cfg): """Build a particular commit. If the build is already done, and we are not forcing a build, we skip @@ -461,6 +461,7 @@ class BuilderThread(threading.Thread): brd (Board): Board to build work_dir (str): Directory to which the source will be checked out do_config (bool): True to run a make _defconfig on the source +mrproper (bool): True to run mrproper first config_only (bool): Only configure the source, do not build it force_build (bool): Force a build even if one was previously done force_build_failures (bool): Force a bulid if the previous result @@ -501,7 +502,7 @@ class BuilderThread(threading.Thread): if self.toolchain: commit = self._checkout(commit_upto, work_dir) result, do_config = self._config_and_build( -commit_upto, brd, work_dir, do_config, self.mrproper, +commit_upto, brd, work_dir, do_config, mrproper, config_only, adjust_cfg, commit, out_dir, out_rel_dir, result) result.already_done = False @@ -692,7 +693,8 @@ class BuilderThread(threading.Thread): force_build = False for commit_upto in range(0, len(job.commits), job.step): result, request_config = self.run_commit(commit_upto, brd, -work_dir, do_config, self.builder.config_only, +work_dir, do_config, self.mrproper, +self.builder.config_only, force_build or self.builder.force_build, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) @@ -703,8 +705,8 @@ class BuilderThread(threading.Thread): # with a reconfig. if self.builder.force_config_on_failure: result, request_config = self.run_commit(commit_upto, -brd, work_dir, True, False, True, False, -job.work_in_output, job.adjust_cfg) +brd, work_dir, True, self.mrproper, False, True, +False, job.work_in_output, job.adjust_cfg) did_config = True if not self.builder.force_reconfig: do_config = request_config @@ -748,7 +750,7 @@ class BuilderThread(threading.Thread): else: # Just build the currently checked-out build result, request_config = self.run_commit(None, brd, work_dir, True, -self.builder.config_only, True, +self.mrproper, self.builder.config_only, True, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) result.commit_upto = 0 -- 2.34.1
[PATCH 08/11] buildman: Avoid rebuilding when --mrproper is used
When this flag is enabled, 'make mrproper' is always used when reconfiguring, so there is no point in doing it again. Update this. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 0a7ff2e083e..c0b1067e3f7 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -700,7 +700,7 @@ class BuilderThread(threading.Thread): job.work_in_output, job.adjust_cfg) failed = result.return_code or result.stderr did_config = do_config -if failed and not do_config: +if failed and not do_config and not self.mrproper: # If our incremental build failed, try building again # with a reconfig. if self.builder.force_config_on_failure: -- 2.34.1
[PATCH 06/11] buildman: Make mrproper an argument to _config_and_build()
Pass this in so the caller can change it independently of the member variable. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 5d4426bf0d1..ff63f9397e6 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -381,7 +381,7 @@ class BuilderThread(threading.Thread): commit = 'current' return commit -def _config_and_build(self, commit_upto, brd, work_dir, do_config, +def _config_and_build(self, commit_upto, brd, work_dir, do_config, mrproper, config_only, adjust_cfg, commit, out_dir, out_rel_dir, result): """Do the build, configuring first if necessary @@ -391,6 +391,7 @@ class BuilderThread(threading.Thread): brd (Board): Board to create arguments for work_dir (str): Directory to which the source will be checked out do_config (bool): True to run a make _defconfig on the source +mrproper (bool): True to run mrproper first config_only (bool): Only configure the source, do not build it adjust_cfg (list of str): See the cfgutil module and run_commit() commit (Commit): Commit only being built @@ -421,7 +422,7 @@ class BuilderThread(threading.Thread): if do_config or adjust_cfg: result = self._reconfigure( commit, brd, cwd, args, env, config_args, config_out, cmd_list, -self.mrproper) +mrproper) do_config = False # No need to configure next time if adjust_cfg: cfgutil.adjust_cfg_file(cfg_file, adjust_cfg) @@ -500,8 +501,9 @@ class BuilderThread(threading.Thread): if self.toolchain: commit = self._checkout(commit_upto, work_dir) result, do_config = self._config_and_build( -commit_upto, brd, work_dir, do_config, config_only, -adjust_cfg, commit, out_dir, out_rel_dir, result) +commit_upto, brd, work_dir, do_config, self.mrproper, +config_only, adjust_cfg, commit, out_dir, out_rel_dir, +result) result.already_done = False result.toolchain = self.toolchain -- 2.34.1
[PATCH 04/11] binman: Update the entrydocs header
Reduce the length of the underline for this header, to match the
heading itself.
Signed-off-by: Simon Glass
---
tools/binman/entry.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 42e0b7b9145..2ed65800d22 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -812,7 +812,7 @@ class Entry(object):
as missing
"""
print('''Binman Entry Documentation
-===
+==
This file describes the entry types supported by binman. These entry types can
be placed in an image one by one to build up a final firmware image. It is
--
2.34.1
[PATCH 01/11] binman: efi: Correct entry docs
Somehow the class documentation has got out of sync with the generated
entries.rst file. Regenerating it causes errors, so correct these and
regenerate the entries.rst file.
Signed-off-by: Simon Glass
Fixes: 809f28e7213 ("binman: capsule: Use dumped capsule header...")
---
tools/binman/entries.rst| 58 -
tools/binman/etype/efi_capsule.py | 40 -
tools/binman/etype/efi_empty_capsule.py | 22 +-
3 files changed, 61 insertions(+), 59 deletions(-)
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index 254afe76074..dc42a9cb97d 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -470,11 +470,11 @@ updating the EC on startup via software sync.
.. _etype_efi_capsule:
-Entry: capsule: Entry for generating EFI Capsule files
---
+Entry: efi-capsule: Generate EFI capsules
+-
-The parameters needed for generation of the capsules can be provided
-as properties in the entry.
+The parameters needed for generation of the capsules can
+be provided as properties in the entry.
Properties / Entry arguments:
- image-index: Unique number for identifying corresponding
@@ -495,9 +495,9 @@ Properties / Entry arguments:
file. Mandatory property for generating signed capsules.
- oem-flags - OEM flags to be passed through capsule header.
-Since this is a subclass of Entry_section, all properties of the parent
-class also apply here. Except for the properties stated as mandatory, the
-rest of the properties are optional.
+Since this is a subclass of Entry_section, all properties of the parent
+class also apply here. Except for the properties stated as mandatory, the
+rest of the properties are optional.
For more details on the description of the capsule format, and the capsule
update functionality, refer Section 8.5 and Chapter 23 in the `UEFI
@@ -510,17 +510,17 @@ provided as a subnode of the capsule entry.
A typical capsule entry node would then look something like this::
capsule {
-type = "efi-capsule";
-image-index = <0x1>;
-/* Image GUID for testing capsule update */
-image-guid = SANDBOX_UBOOT_IMAGE_GUID;
-hardware-instance = <0x0>;
-private-key = "path/to/the/private/key";
-public-key-cert = "path/to/the/public-key-cert";
-oem-flags = <0x8000>;
+type = "efi-capsule";
+image-index = <0x1>;
+/* Image GUID for testing capsule update */
+image-guid = SANDBOX_UBOOT_IMAGE_GUID;
+hardware-instance = <0x0>;
+private-key = "path/to/the/private/key";
+public-key-cert = "path/to/the/public-key-cert";
+oem-flags = <0x8000>;
-u-boot {
-};
+u-boot {
+};
};
In the above example, the capsule payload is the U-Boot image. The
@@ -534,8 +534,8 @@ payload using the blob-ext subnode.
.. _etype_efi_empty_capsule:
-Entry: efi-empty-capsule: Entry for generating EFI Empty Capsule files
---
+Entry: efi-empty-capsule: Generate EFI empty capsules
+-
The parameters needed for generation of the empty capsules can
be provided as properties in the entry.
@@ -551,22 +551,22 @@ update functionality, refer Section 8.5 and Chapter 23 in
the `UEFI
specification`_. For more information on the empty capsule, refer the
sections 2.3.2 and 2.3.3 in the `Dependable Boot specification`_.
-A typical accept empty capsule entry node would then look something
-like this::
+A typical accept empty capsule entry node would then look something like
+this::
empty-capsule {
-type = "efi-empty-capsule";
-/* GUID of the image being accepted */
-image-type-id = SANDBOX_UBOOT_IMAGE_GUID;
-capsule-type = "accept";
+type = "efi-empty-capsule";
+/* GUID of image being accepted */
+image-type-id = SANDBOX_UBOOT_IMAGE_GUID;
+capsule-type = "accept";
};
-A typical revert empty capsule entry node would then look something
-like this::
+A typical revert empty capsule entry node would then look something like
+this::
empty-capsule {
-type = "efi-empty-capsule";
-capsule-type = "revert";
+type = "efi-empty-capsule";
+capsule-type = "revert";
};
The empty capsules do not have any input payload image.
diff --git a/tools/binman/etype/efi_capsule.py
b/tools/binman/etype/efi_capsule.py
index e3203717822..751f654bf31 100644
--- a/tools/binman/etype/efi_capsule.py
+++ b/tools/binman/etype/efi_capsule.py
@@ -36,23 +36,23 @@ class Entry_efi_capsule(Entry_section):
be provided as properties in the entry.
Properties / Entry arguments:
-- image-index:
[PATCH 05/11] buildman: Make mrproper an argument to _reconfigure()
Pass this in so the caller can change it independently of the member variable. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index a8599c0bb2a..5d4426bf0d1 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -240,7 +240,7 @@ class BuilderThread(threading.Thread): return args, cwd, src_dir def _reconfigure(self, commit, brd, cwd, args, env, config_args, config_out, - cmd_list): + cmd_list, mrproper): """Reconfigure the build Args: @@ -251,11 +251,12 @@ class BuilderThread(threading.Thread): env (dict): Environment strings config_args (list of str): defconfig arg for this board cmd_list (list of str): List to add the commands to, for logging +mrproper (bool): True to run mrproper first Returns: CommandResult object """ -if self.mrproper: +if mrproper: result = self.make(commit, brd, 'mrproper', cwd, 'mrproper', *args, env=env) config_out.write(result.combined) @@ -419,7 +420,8 @@ class BuilderThread(threading.Thread): cmd_list = [] if do_config or adjust_cfg: result = self._reconfigure( -commit, brd, cwd, args, env, config_args, config_out, cmd_list) +commit, brd, cwd, args, env, config_args, config_out, cmd_list, +self.mrproper) do_config = False # No need to configure next time if adjust_cfg: cfgutil.adjust_cfg_file(cfg_file, adjust_cfg) -- 2.34.1
[PATCH 03/11] binman: ti: Regenerate entry docs
Correct formatting errors in the documentation.
Regenerate the entries.rst file to include this recent addition.
Signed-off-by: Simon Glass
---
tools/binman/entries.rst| 35 +
tools/binman/etype/ti_secure.py | 45 +
2 files changed, 58 insertions(+), 22 deletions(-)
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index 1b9b868e33f..bdda1ef2855 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -1951,6 +1951,12 @@ Properties / Entry arguments:
- content: List of phandles to entries to sign
- keyfile: Filename of file containing key to sign binary with
- sha: Hash function to be used for signing
+- auth-in-place: This is an integer field that contains two pieces
+ of information:
+
+- Lower Byte - Remains 0x02 as per our use case
+ ( 0x02: Move the authenticated binary back to the header )
+- Upper Byte - The Host ID of the core owning the firewall
Output files:
- input. - input file passed to openssl
@@ -1959,6 +1965,35 @@ Output files:
- cert. - output file generated by openssl (which is
used as the entry contents)
+Depending on auth-in-place information in the inputs, we read the
+firewall nodes that describe the configurations of firewall that TIFS
+will be doing after reading the certificate.
+
+The syntax of the firewall nodes are as such::
+
+firewall-257-0 {
+id = <257>; /* The ID of the firewall being configured */
+region = <0>; /* Region number to configure */
+
+control = /* The control register */
+<(FWCTRL_EN | FWCTRL_LOCK | FWCTRL_BG | FWCTRL_CACHE)>;
+
+permissions = /* The permission registers */
+<((FWPRIVID_ALL << FWPRIVID_SHIFT) |
+FWPERM_SECURE_PRIV_RWCD |
+FWPERM_SECURE_USER_RWCD |
+FWPERM_NON_SECURE_PRIV_RWCD |
+FWPERM_NON_SECURE_USER_RWCD)>;
+
+/* More defines can be found in k3-security.h */
+
+start_address =/* The Start Address of the firewall */
+<0x0 0x0>;
+end_address = /* The End Address of the firewall */
+<0xff 0x>;
+};
+
+
openssl signs the provided data, using the TI templated config file and
writes the signature in this entry. This allows verification that the
data is genuine.
diff --git a/tools/binman/etype/ti_secure.py b/tools/binman/etype/ti_secure.py
index 704dcf8a381..420ee263e4f 100644
--- a/tools/binman/etype/ti_secure.py
+++ b/tools/binman/etype/ti_secure.py
@@ -53,10 +53,11 @@ class Entry_ti_secure(Entry_x509_cert):
- keyfile: Filename of file containing key to sign binary with
- sha: Hash function to be used for signing
- auth-in-place: This is an integer field that contains two pieces
- of information
-Lower Byte - Remains 0x02 as per our use case
-( 0x02: Move the authenticated binary back to the header )
-Upper Byte - The Host ID of the core owning the firewall
+ of information:
+
+- Lower Byte - Remains 0x02 as per our use case
+ ( 0x02: Move the authenticated binary back to the header )
+- Upper Byte - The Host ID of the core owning the firewall
Output files:
- input. - input file passed to openssl
@@ -69,29 +70,29 @@ class Entry_ti_secure(Entry_x509_cert):
firewall nodes that describe the configurations of firewall that TIFS
will be doing after reading the certificate.
-The syntax of the firewall nodes are as such:
+The syntax of the firewall nodes are as such::
-firewall-257-0 {
-id = <257>; /* The ID of the firewall being configured */
-region = <0>; /* Region number to configure */
+firewall-257-0 {
+id = <257>; /* The ID of the firewall being configured */
+region = <0>; /* Region number to configure */
-control = /* The control register */
-<(FWCTRL_EN | FWCTRL_LOCK | FWCTRL_BG | FWCTRL_CACHE)>;
+control = /* The control register */
+<(FWCTRL_EN | FWCTRL_LOCK | FWCTRL_BG | FWCTRL_CACHE)>;
-permissions = /* The permission registers */
-<((FWPRIVID_ALL << FWPRIVID_SHIFT) |
-FWPERM_SECURE_PRIV_RWCD |
-FWPERM_SECURE_USER_RWCD |
-FWPERM_NON_SECURE_PRIV_RWCD |
-FWPERM_NON_SECURE_USER_RWCD)>;
+permissions = /* The permission registers */
+<((FWPRIVID_ALL << FWPRIVID_SHIFT) |
+FWPERM_SECURE_PRIV_RWCD |
+FWPERM_SECURE_USER_RWCD |
+FWPERM_NON_SECURE_PRIV_RWCD |
+
[PATCH 02/11] binman: Regenerate nxp docs
Regenerate the entries.rst file to include this recent addition.
Note that more docs are needed here, to actually describe the entry
type.
Note also that the entry type needs Binman tests added.
Signed-off-by: Simon Glass
---
tools/binman/entries.rst | 22 ++
1 file changed, 22 insertions(+)
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index dc42a9cb97d..1b9b868e33f 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -1521,6 +1521,28 @@ byte.
+.. _etype_nxp_imx8mcst:
+
+Entry: nxp-imx8mcst: NXP i.MX8M CST .cfg file generator and cst invoker
+---
+
+Properties / Entry arguments:
+- nxp,loader-address - loader address (SPL text base)
+
+
+
+.. _etype_nxp_imx8mimage:
+
+Entry: nxp-imx8mimage: NXP i.MX8M imx8mimage .cfg file generator and mkimage
invoker
+
+
+Properties / Entry arguments:
+- nxp,boot-from - device to boot from (e.g. 'sd')
+- nxp,loader-address - loader address (SPL text base)
+- nxp,rom-version - BootROM version ('2' for i.MX8M Nano and Plus)
+
+
+
.. _etype_opensbi:
Entry: opensbi: RISC-V OpenSBI fw_dynamic blob
--
2.34.1
[PATCH 00/11] Tools updates for Labgrid
This series includes patches to update buildman to support Labgrid better and some minor binman fixes. Simon Glass (11): binman: efi: Correct entry docs binman: Regenerate nxp docs binman: ti: Regenerate entry docs binman: Update the entrydocs header buildman: Make mrproper an argument to _reconfigure() buildman: Make mrproper an argument to _config_and_build() buildman: Make mrproper an argument to run_commit() buildman: Avoid rebuilding when --mrproper is used buildman: Add a flag to force mrproper on failure buildman: Retry the build for current source buildman: Add a way to limit the number of buildmans tools/binman/entries.rst| 115 ++- tools/binman/entry.py | 2 +- tools/binman/etype/efi_capsule.py | 40 +++ tools/binman/etype/efi_empty_capsule.py | 22 ++-- tools/binman/etype/ti_secure.py | 45 tools/buildman/builder.py | 18 +-- tools/buildman/builderthread.py | 44 +--- tools/buildman/buildman.rst | 8 +- tools/buildman/cmdline.py | 6 +- tools/buildman/control.py | 141 +++- tools/buildman/pyproject.toml | 6 +- tools/buildman/test.py | 121 tools/u_boot_pylib/terminal.py | 7 +- 13 files changed, 465 insertions(+), 110 deletions(-) -- 2.34.1
Re: [PATCH v2 00/43] labgrid: Provide an integration with Labgrid
Hi Tom,
OK will do.
One I have already split out ("Bug-fixes for a few boards") so I'll
split out a few more.
REgards,
Simon
On Fri, 21 Jun 2024 at 14:07, Tom Rini wrote:
>
> On Fri, Jun 21, 2024 at 01:51:21PM -0600, Simon Glass wrote:
>
> > Labgrid provides access to a hardware lab in an automated way. It is
> > possible to boot U-Boot on boards in the lab without physically touching
> > them. It relies on relays, USB UARTs and SD muxes, among other things.
> >
> > By way of background, about 4 years ago I wrong a thing called Labman[1]
> > which allowed my lab of about 30 devices to be operated remotely, using
> > tbot for the console and build integration. While it worked OK and I
> > used it for many bisects, I didn't take it any further.
> >
> > It turns out that there was already an existing program, called Labgrid,
> > which I did not know about at time (thank you Tom for telling me). It is
> > more rounded than Labman and has a number of advantages:
> >
> > - does not need udev rules, mostly
> > - has several existing users who rely on it
> > - supports multiple machines exporting their devices
> >
> > It lacks a 'lab check' feature and a few other things, but these can be
> > remedied.
> >
> > On and off over the past several weeks I have been experimenting with
> > Labgrid. I have managed to create an initial U-Boot integration (this
> > series) by adding various features to Labgrid[2] and the U-Boot test
> > hooks.
> >
> > I hope that this might inspire others to set up boards and run tests
> > automatically, rather than relying on infrequent, manual test. Perhaps
> > it may even be possible to have a number of labs available.
> >
> > Included in the integration are a number of simple scripts which make it
> > easy to connect to boards and run tests:
> >
> > ub-int
> > Build and boot on a target, starting an interactive session
> >
> > ub-cli
> > Build and boot on a target, ensure U-Boot starts and provide an
> > interactive
> > session from there
> >
> > ub-smoke
> > Smoke test U-Boot to check that it boots to a prompt on a target
> >
> > ub-bisect
> > Bisect a git tree to locate a failure on a particular target
> >
> > ub-pyt
> > Run U-Boot pytests on a target
> >
> > Some of these help to provide the same tbot[4] workflow which I have
> > relied on for several years, albeit much simpler versions.
> >
> > The goal here is to create some sort of script which can collect
> > patches from the mailing list, apply them and test them on a selection
> > of boards. I suspect that script already exists, so please let me know
> > what you suggest.
> >
> > I hope you find this interesting and take a look!
>
> Can you please split this up in to 3-4 series so that they can be
> reviewed and merged? Thanks.
>
> --
> Tom
[PATCH] doc: board: ti: Add capsule documentation for TI K3 devices
Added introductory documentation about capsule support for TI devices, including links to more detailed information. Also added a note in the build secction that points to the host package dependency docs. This patch is followup from a request in the series introducing capsule update for TI boards. https://lore.kernel.org/r/20240618145058.552eapp5iiz772ej@hardcore Signed-off-by: Jonathan Humphreys --- doc/board/ti/k3.rst | 29 + 1 file changed, 29 insertions(+) diff --git a/doc/board/ti/k3.rst b/doc/board/ti/k3.rst index 76375473185..67b066a07d3 100644 --- a/doc/board/ti/k3.rst +++ b/doc/board/ti/k3.rst @@ -204,6 +204,11 @@ online Build Procedure --- +.. note :: + + Make sure you have installed all necessary host package dependencies + before proceeding. See :ref:`build/gcc:Building with GCC`. + Depending on the specifics of your device, you will need three or more binaries to boot your SoC. @@ -388,6 +393,30 @@ wakeup and main domain and to boot to the U-Boot prompt | `tispl.bin` for HS devices or `tispl.bin_unsigned` for GP devices | `u-boot.img` for HS devices or `u-boot.img_unsigned` for GP devices +Capsules + + +Most K3 boards have support for UEFI capsule update via capsule-on-disk +functionality. Check the ``CONFIG_EFI_CAPSULE_ON_DISK`` config option for +the board under question to verify. If configured, capsules for each of the +binaries above are automatically generated as part of the binary's build. +They are named `-capsule.bin`. For example, the capsule for +`u-boot.img` would be called `uboot-capsule.bin`. + +See :ref:`uefi_capsule_update_ref` for more information on U-Boot's support +for capsule update and how they are applied. + +Each board defines the capsules generated, including where those capsules +are applied. See the ``update_info`` definition for a board, typically +found at `board/ti//evm.c`. For example, `board/ti/am62x/evm.c`. +Usually, if the board has OSPI flash, the capsules will be applied there, +else the boot partition of the eMMC device. + +Once applied, the board will have U-Boot binaries in on-board non-volatile +storage. To start booting from that storage, set the bootmode pins +accordingly. Future updates can be performed by using the capsules +generated from the corresponding U-Boot builds. + FIT signature signing - -- 2.34.1
Re: [PATCH v2 00/43] labgrid: Provide an integration with Labgrid
On Fri, Jun 21, 2024 at 01:51:21PM -0600, Simon Glass wrote: > Labgrid provides access to a hardware lab in an automated way. It is > possible to boot U-Boot on boards in the lab without physically touching > them. It relies on relays, USB UARTs and SD muxes, among other things. > > By way of background, about 4 years ago I wrong a thing called Labman[1] > which allowed my lab of about 30 devices to be operated remotely, using > tbot for the console and build integration. While it worked OK and I > used it for many bisects, I didn't take it any further. > > It turns out that there was already an existing program, called Labgrid, > which I did not know about at time (thank you Tom for telling me). It is > more rounded than Labman and has a number of advantages: > > - does not need udev rules, mostly > - has several existing users who rely on it > - supports multiple machines exporting their devices > > It lacks a 'lab check' feature and a few other things, but these can be > remedied. > > On and off over the past several weeks I have been experimenting with > Labgrid. I have managed to create an initial U-Boot integration (this > series) by adding various features to Labgrid[2] and the U-Boot test > hooks. > > I hope that this might inspire others to set up boards and run tests > automatically, rather than relying on infrequent, manual test. Perhaps > it may even be possible to have a number of labs available. > > Included in the integration are a number of simple scripts which make it > easy to connect to boards and run tests: > > ub-int > Build and boot on a target, starting an interactive session > > ub-cli > Build and boot on a target, ensure U-Boot starts and provide an > interactive > session from there > > ub-smoke > Smoke test U-Boot to check that it boots to a prompt on a target > > ub-bisect > Bisect a git tree to locate a failure on a particular target > > ub-pyt > Run U-Boot pytests on a target > > Some of these help to provide the same tbot[4] workflow which I have > relied on for several years, albeit much simpler versions. > > The goal here is to create some sort of script which can collect > patches from the mailing list, apply them and test them on a selection > of boards. I suspect that script already exists, so please let me know > what you suggest. > > I hope you find this interesting and take a look! Can you please split this up in to 3-4 series so that they can be reviewed and merged? Thanks. -- Tom signature.asc Description: PGP signature
[PATCH v2 43/43] CI: Allow running tests on sjg lab
Add a way to run tests on a real hardware lab. This is in the very early
experimental stages. There are only 23 boards and 3 of those are broken!
(bob, ff3399, samus). A fourth fails due to problems with the TPM tests.
To try this, assuming you have gitlab access, set SJG_LAB=1, e.g.:
git push -o ci.variable="SJG_LAB=1" dm HEAD:try
Signed-off-by: Simon Glass
---
Changes in v2:
- Avoid running a docker image for skipped lab tests
.gitlab-ci.yml | 153 +
1 file changed, 153 insertions(+)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 165f765a833..75c18a0f2f7 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -17,6 +17,7 @@ stages:
- testsuites
- test.py
- world build
+ - sjg-lab
.buildman_and_testpy_template: _and_testpy_dfn
stage: test.py
@@ -482,3 +483,155 @@ coreboot test.py:
TEST_PY_TEST_SPEC: "not sleep"
TEST_PY_ID: "--id qemu"
<<: *buildman_and_testpy_dfn
+
+.lab_template: _dfn
+ stage: sjg-lab
+ rules:
+- if: $SJG_LAB == "1"
+ when: always
+- when: manual
+ tags: [ 'lab' ]
+ script:
+- if [[ -z "${SJG_LAB}" ]]; then
+exit 0;
+ fi
+# Environment:
+# SRC - source tree
+# OUT - output directory for builds
+- export SRC="$(pwd)"
+- export OUT="${SRC}/build/${BOARD}"
+- export PATH=$PATH:~/bin
+- export PATH=$PATH:/vid/software/devel/ubtest/u-boot-test-hooks/bin
+
+# Load it on the device
+- ret=0
+- echo "role ${ROLE}"
+- export strategy="-s uboot -e off"
+# export verbose="-v"
+- ${SRC}/test/py/test.py --role ${ROLE} --build-dir "${OUT}"
+--capture=tee-sys -k "not bootstd"|| ret=$?
+- U_BOOT_BOARD_IDENTITY="${ROLE}" u-boot-test-release || true
+- if [[ $ret -ne 0 ]]; then
+exit $ret;
+ fi
+ artifacts:
+when: always
+paths:
+ - "build/${BOARD}/test-log.html"
+ - "build/${BOARD}/multiplexed_log.css"
+expire_in: 1 week
+
+rpi3:
+ variables:
+ROLE: rpi3
+ <<: *lab_dfn
+
+opi_pc:
+ variables:
+ROLE: opi_pc
+ <<: *lab_dfn
+
+pcduino3_nano:
+ variables:
+ROLE: pcduino3_nano
+ <<: *lab_dfn
+
+samus:
+ variables:
+ROLE: samus
+ <<: *lab_dfn
+
+link:
+ variables:
+ROLE: link
+ <<: *lab_dfn
+
+jerry:
+ variables:
+ROLE: jerry
+ <<: *lab_dfn
+
+minnowmax:
+ variables:
+ROLE: minnowmax
+ <<: *lab_dfn
+
+opi_pc2:
+ variables:
+ROLE: opi_pc2
+ <<: *lab_dfn
+
+bpi:
+ variables:
+ROLE: bpi
+ <<: *lab_dfn
+
+rpi2:
+ variables:
+ROLE: rpi2
+ <<: *lab_dfn
+
+bob:
+ variables:
+ROLE: bob
+ <<: *lab_dfn
+
+ff3399:
+ variables:
+ROLE: ff3399
+ <<: *lab_dfn
+
+coral:
+ variables:
+ROLE: coral
+ <<: *lab_dfn
+
+rpi3z:
+ variables:
+ROLE: rpi3z
+ <<: *lab_dfn
+
+bbb:
+ variables:
+ROLE: bbb
+ <<: *lab_dfn
+
+kevin:
+ variables:
+ROLE: kevin
+ <<: *lab_dfn
+
+pine64:
+ variables:
+ROLE: pine64
+ <<: *lab_dfn
+
+c4:
+ variables:
+ROLE: c4
+ <<: *lab_dfn
+
+rpi4:
+ variables:
+ROLE: rpi4
+ <<: *lab_dfn
+
+rpi0:
+ variables:
+ROLE: rpi0
+ <<: *lab_dfn
+
+snow:
+ variables:
+ROLE: snow
+ <<: *lab_dfn
+
+pcduino3:
+ variables:
+ROLE: pcduino3
+ <<: *lab_dfn
+
+nyan-big:
+ variables:
+ROLE: nyan-big
+ <<: *lab_dfn
--
2.34.1
[PATCH v2 42/43] Update u-boot.cfg to include CFG also
Some configuration is now in variables with a CFG_ prefix. Add these to
the .cfg file so that we can see everything in one place. Sort the
options so they are easier to find and compare.
Signed-off-by: Simon Glass
---
Changes in v2:
- Add new patch to update u-boot.cfg with CFG_... options
scripts/Makefile.autoconf | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
index b42f9b525fe..65ff11ea508 100644
--- a/scripts/Makefile.autoconf
+++ b/scripts/Makefile.autoconf
@@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@
quiet_cmd_u_boot_cfg = CFG $@
cmd_u_boot_cfg = \
$(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
- grep 'define CONFIG_' $@.tmp | \
+ egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
sed '/define CONFIG_IS_ENABLED(/d;/define
CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
rm $@.tmp; \
} || { \
--
2.34.1
[PATCH v2 41/43] test: Add a section for closing the connection
This can take a while and involve multiple steps (e.g. turning the board
back off). Add a section for it and show the output.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/u_boot_console_base.py | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index 1b00821c5e9..95096fc123c 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -157,7 +157,10 @@ class ConsoleBase(object):
"""
if self.p:
-self.p.close()
+self.log.start_section('Stopping U-Boot')
+close_type = self.p.close()
+self.log.info(f'Close type: {close_type}')
+self.log.end_section('Stopping U-Boot')
self.logstream.close()
def set_lab_mode(self):
--
2.34.1
[PATCH v2 40/43] test: Try to shut down the lab console gracefully
Send the Labgrid quit characters to ask it to exit gracefully. This typically allows it to power off the board being used. If that doesn't work, try the less graceful approach. Signed-off-by: Simon Glass --- (no changes since v1) test/py/u_boot_spawn.py | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index c0ff0813554..ec1fa465047 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -16,6 +16,9 @@ import termios import time import traceback +# Character to send (twice) to exit the terminal +EXIT_CHAR = 0x1d# FS (Ctrl + ]) + class Timeout(Exception): """An exception sub-class that indicates that a timeout occurred.""" @@ -304,15 +307,25 @@ class Spawn: None. Returns: -Nothing. +str: Type of closure completed """ +self.send(chr(EXIT_CHAR) * 2) +# Wait about 10 seconds for Labgrid to close and power off the board +for _ in range(100): +if not self.isalive(): +return 'normal' +time.sleep(0.1) + +# That didn't work, so try closing the PTY os.close(self.fd) for _ in range(100): if not self.isalive(): -break +return 'break' time.sleep(0.1) +return 'timeout' + def get_expect_output(self): """Return the output read by expect() -- 2.34.1
[PATCH v2 39/43] test: Avoid double echo when starting up
There is a very annoying bug at present where the terminal echos part of the first command sent to the board. This happens because the terminal is still set to echo for a period until Labgrid starts up and can change this. Fix this by disabling echo (and other terminal features) as soon as the spawn happens. Signed-off-by: Simon Glass --- Changes in v2: - Only disable echo if a terminal is detected test/py/u_boot_spawn.py | 13 + 1 file changed, 13 insertions(+) diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 62eb4118731..c0ff0813554 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -12,6 +12,7 @@ import pytest import signal import select import sys +import termios import time import traceback @@ -117,11 +118,23 @@ class Spawn: finally: os._exit(255) +old = None try: +if os.isatty(sys.stdout.fileno()): +new = termios.tcgetattr(self.fd) +old = new +new[3] = new[3] & ~(termios.ICANON | termios.ISIG) +new[3] = new[3] & ~termios.ECHO +new[6][termios.VMIN] = 0 +new[6][termios.VTIME] = 0 +termios.tcsetattr(self.fd, termios.TCSANOW, new) + self.poll = select.poll() self.poll.register(self.fd, select.POLLIN | select.POLLPRI | select.POLLERR | select.POLLHUP | select.POLLNVAL) except: +if old: +termios.tcsetattr(self.fd, termios.TCSANOW, old) self.close() raise -- 2.34.1
[PATCH v2 38/43] test: Fix mulptiplex_log typo
Fix a typo in a comment. Signed-off-by: Simon Glass --- (no changes since v1) test/py/u_boot_console_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index eda48cd35f7..1b00821c5e9 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -97,7 +97,7 @@ class ConsoleBase(object): Can only usefully be called by sub-classes. Args: -log: A mulptiplex_log.Logfile object, to which the U-Boot output +log: A multiplexed_log.Logfile object, to which the U-Boot output will be logged. config: A configuration data structure, as built by conftest.py. max_fifo_fill: The maximum number of characters to send to U-Boot -- 2.34.1
[PATCH v2 37/43] test: Improve handling of sending commands
We expect commands to be echoed and this should happen quite quickly,
since U-Boot is sitting at the prompt waiting for a command.
Reduce the timeout for this situation. Try to produce a more useful
error message when something goes wrong. Also handle the case where the
connection has gone away since the last command was issued.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/u_boot_console_base.py | 35 --
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index cfde57649f1..eda48cd35f7 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -31,6 +31,7 @@ PAT_RE = 1
# Timeout before expecting the console to be ready (in milliseconds)
TIMEOUT_MS = 3 # Standard timeout
+TIMEOUT_CMD_MS = 1 # Command-echo timeout
# Timeout for board preparation in lab mode. This needs to be enough to build
# U-Boot, write it to the board and then boot the board. Since this process is
@@ -274,22 +275,28 @@ class ConsoleBase(object):
try:
self.at_prompt = False
+if not self.p:
+raise BootFail(
+f"Lab failure: Connection lost when sending command
'{cmd}'")
+
if send_nl:
cmd += '\n'
-while cmd:
-# Limit max outstanding data, so UART FIFOs don't overflow
-chunk = cmd[:self.max_fifo_fill]
-cmd = cmd[self.max_fifo_fill:]
-self.p.send(chunk)
-if not wait_for_echo:
-continue
-chunk = re.escape(chunk)
-chunk = chunk.replace('\\\n', '[\r\n]')
-m = self.p.expect([chunk] + self.bad_patterns)
-if m != 0:
-self.at_prompt = False
-raise BootFail('Bad pattern found on console: ' +
-self.bad_pattern_ids[m - 1])
+rem = cmd # Remaining to be sent
+with self.temporary_timeout(TIMEOUT_CMD_MS):
+while rem:
+# Limit max outstanding data, so UART FIFOs don't overflow
+chunk = rem[:self.max_fifo_fill]
+rem = rem[self.max_fifo_fill:]
+self.p.send(chunk)
+if not wait_for_echo:
+continue
+chunk = re.escape(chunk)
+chunk = chunk.replace('\\\n', '[\r\n]')
+m = self.p.expect([chunk] + self.bad_patterns)
+if m != 0:
+self.at_prompt = False
+raise BootFail(f"Failed to get echo on console (cmd
'{cmd}':rem '{rem}'): " +
+self.bad_pattern_ids[m - 1])
if not wait_for_prompt:
return
if wait_for_reboot:
--
2.34.1
[PATCH v2 36/43] test: Introduce lab mode
There is quite a bit of code in pytest to try to start up U-Boot on a
board, with timeouts, expects, etc.
This is tedious to maintain and is peripheral to the test system's
purpose. It seems better to put this logic in the lab itself, where is
can provide such support.
With Labgrid we can use the UbootStrategy class to get the board into a
useful state, however it needs to do it. Then it can report to pytest
by writing a suitable string along with the U-Boot version it detected.
Add support for detecting 'lab mode' and simply assume that all is well
in that case. Collect the version string when Labgrid says it is ready.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/u_boot_console_base.py | 68 ++
1 file changed, 53 insertions(+), 15 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index dbf54eadd0f..cfde57649f1 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -23,13 +23,21 @@ pattern_stop_autoboot_prompt = re.compile('Hit any key to
stop autoboot: ')
pattern_unknown_command = re.compile('Unknown command \'.*\' - try \'help\'')
pattern_error_notification = re.compile('## Error: ')
pattern_error_please_reset = re.compile('### ERROR ### Please RESET the board
###')
-pattern_ready_prompt = re.compile('U-Boot is ready')
+pattern_ready_prompt = re.compile('{lab ready in (.*)s: (.*)}')
+pattern_lab_mode = re.compile('{lab mode.*}')
PAT_ID = 0
PAT_RE = 1
# Timeout before expecting the console to be ready (in milliseconds)
-TIMEOUT_MS = 3
+TIMEOUT_MS = 3 # Standard timeout
+
+# Timeout for board preparation in lab mode. This needs to be enough to build
+# U-Boot, write it to the board and then boot the board. Since this process is
+# under the control of another program (e.g. Labgrid), it will failure sooner
+# if something goes way. So use a very long timeout here to cover all possible
+# situations.
+TIMEOUT_PREPARE_MS = 3 * 60 * 1000
bad_pattern_defs = (
('spl_signon', pattern_u_boot_spl_signon),
@@ -117,6 +125,7 @@ class ConsoleBase(object):
self.at_prompt = False
self.at_prompt_logevt = None
+self.lab_mode = False
def get_spawn(self):
# This is not called, ssubclass must define this.
@@ -150,40 +159,69 @@ class ConsoleBase(object):
self.p.close()
self.logstream.close()
+def set_lab_mode(self):
+"""Select lab mode
+
+This tells us that we will get a 'lab ready' message when the board is
+ready for use. We don't need to look for signon messages.
+"""
+self.log.info(f'test.py: Lab mode is active')
+self.p.timeout = TIMEOUT_PREPARE_MS
+self.lab_mode = True
+
def wait_for_boot_prompt(self, loop_num = 1):
"""Wait for the boot up until command prompt. This is for internal use
only.
"""
try:
+self.log.info('Waiting for U-Boot to be ready')
bcfg = self.config.buildconfig
config_spl_serial = bcfg.get('config_spl_serial', 'n') == 'y'
env_spl_skipped = self.config.env.get('env__spl_skipped', False)
env_spl_banner_times =
self.config.env.get('env__spl_banner_times', 1)
-while loop_num > 0:
+while not self.lab_mode and loop_num > 0:
loop_num -= 1
while config_spl_serial and not env_spl_skipped and
env_spl_banner_times > 0:
-m = self.p.expect([pattern_u_boot_spl_signon] +
- self.bad_patterns)
-if m != 0:
+m = self.p.expect([pattern_u_boot_spl_signon,
+ pattern_lab_mode] + self.bad_patterns)
+if m == 1:
+self.set_lab_mode()
+break
+elif m != 0:
raise BootFail('Bad pattern found on SPL console: ' +
-self.bad_pattern_ids[m - 1])
+ self.bad_pattern_ids[m - 1])
env_spl_banner_times -= 1
-m = self.p.expect([pattern_u_boot_main_signon] +
self.bad_patterns)
-if m != 0:
-raise BootFail('Bad pattern found on console: ' +
-self.bad_pattern_ids[m - 1])
-self.u_boot_version_string = self.p.after
+if not self.lab_mode:
+m = self.p.expect([pattern_u_boot_main_signon,
+ pattern_lab_mode] + self.bad_patterns)
+if m == 1:
+self.set_lab_mode()
+elif m != 0:
+raise BootFail('Bad pattern found on console: ' +
+ self.bad_pattern_ids[m - 1])
+
[PATCH v2 35/43] test: Tidy up remaining exceptions
Use the new handle_exception() function from ConsoleBase also.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/u_boot_console_base.py | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index b656a1f38cd..dbf54eadd0f 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -14,7 +14,7 @@ import pytest
import re
import sys
import u_boot_spawn
-from u_boot_spawn import BootFail, Timeout, Unexpected
+from u_boot_spawn import BootFail, Timeout, Unexpected, handle_exception
# Regexes for text we expect U-Boot to send to the console.
pattern_u_boot_spl_signon = re.compile('(U-Boot SPL
\\d{4}\\.\\d{2}[^\r\n]*\\))')
@@ -268,12 +268,12 @@ class ConsoleBase(object):
# indentation.
return self.p.before.strip('\r\n')
except Timeout as exc:
-self.log.error(str(exc))
-self.cleanup_spawn()
+handle_exception(self.config, self, self.log, exc, 'Lab failure',
+ True)
raise
-except BootFail as ex:
-self.log.error(str(ex))
-self.cleanup_spawn()
+except BootFail as exc:
+handle_exception(self.config, self, self.log, exc, 'Boot fail',
+ True, self.get_spawn_output())
raise
finally:
self.log.timestamp()
--
2.34.1
[PATCH v2 34/43] test: Detect dead connections
When the connection to a board dies, assume it is dead forever until
some user action is taken. Skip all remaining tests. This avoids CI
runs taking an hour, with hundreds of 30-second timeouts all to no
avail.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/conftest.py | 19 +--
test/py/u_boot_spawn.py | 38 ++
2 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/test/py/conftest.py b/test/py/conftest.py
index 5de8d7b0e23..42af1abd72e 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -25,6 +25,7 @@ import re
from _pytest.runner import runtestprotocol
import subprocess
import sys
+from u_boot_spawn import BootFail, Timeout, Unexpected, handle_exception
# Globals: The HTML log file, and the connection to the U-Boot console.
log = None
@@ -280,6 +281,7 @@ def pytest_configure(config):
ubconfig.gdbserver = gdbserver
ubconfig.no_prompt_wait = config.getoption('no_prompt_wait')
ubconfig.dtb = build_dir + '/arch/sandbox/dts/test.dtb'
+ubconfig.connection_ok = True
env_vars = (
'board_type',
@@ -446,8 +448,21 @@ def u_boot_console(request):
Returns:
The fixture value.
"""
-
-console.ensure_spawned()
+if not ubconfig.connection_ok:
+pytest.skip('Cannot get target connection')
+return None
+try:
+console.ensure_spawned()
+except OSError as err:
+handle_exception(ubconfig, console, log, err, 'Lab failure', True)
+except Timeout as err:
+handle_exception(ubconfig, console, log, err, 'Lab timeout', True)
+except BootFail as err:
+handle_exception(ubconfig, console, log, err, 'Boot fail', True,
+ console.get_spawn_output())
+except Unexpected:
+handle_exception(ubconfig, console, log, err, 'Unexpected test output',
+ False)
return console
anchors = {}
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
index 57ea845ad4c..62eb4118731 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -8,6 +8,7 @@ Logic to spawn a sub-process and interact with its stdio.
import os
import re
import pty
+import pytest
import signal
import select
import sys
@@ -28,6 +29,43 @@ class BootFail(Exception):
class Unexpected(Exception):
"""An exception sub-class that indicates that unexpected test was seen."""
+
+def handle_exception(ubconfig, console, log, err, name, fatal, output=''):
+"""Handle an exception from the console
+
+Exceptions can occur when there is unexpected output or due to the board
+crashing or hanging. Some exceptions are likely fatal, where retrying will
+just chew up time to no available. In those cases it is best to cause
+further tests be skipped.
+
+Args:
+ubconfig (ArbitraryAttributeContainer): ubconfig object
+log (Logfile): Place to log errors
+console (ConsoleBase): Console to clean up, if fatal
+err (Exception): Exception which was thrown
+name (str): Name of problem, to log
+fatal (bool): True to abort all tests
+output (str): Extra output to report on boot failure. This can show the
+ target's console output as it tried to boot
+"""
+msg = f'{name}: '
+if fatal:
+msg += 'Marking connection bad - no other tests will run'
+else:
+msg += 'Assuming that lab is healthy'
+print(msg)
+log.error(msg)
+log.error(f'Error: {err}')
+
+if output:
+msg += f'; output {output}'
+
+if fatal:
+ubconfig.connection_ok = False
+console.cleanup_spawn()
+pytest.exit(msg)
+
+
class Spawn:
"""Represents the stdio of a freshly created sub-process. Commands may be
sent to the process, and responses waited for.
--
2.34.1
[PATCH v2 33/43] test: Separate out the exception handling
The tests currently catch a very board Exception in each case. This is
thrown even in the event of a coding error.
We want to handle exceptions differently depending on their severity,
so that we can avoid hour-long delays waiting for a board that is
clearly broken.
As a first step, create some new exception types, separating out those
which are simply an unexpected result from executed a command, from
those which indicate some kind of hardware failure.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/u_boot_console_base.py | 26 ++
test/py/u_boot_spawn.py| 11 +++
2 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index a61eec31148..b656a1f38cd 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -14,6 +14,7 @@ import pytest
import re
import sys
import u_boot_spawn
+from u_boot_spawn import BootFail, Timeout, Unexpected
# Regexes for text we expect U-Boot to send to the console.
pattern_u_boot_spl_signon = re.compile('(U-Boot SPL
\\d{4}\\.\\d{2}[^\r\n]*\\))')
@@ -164,13 +165,13 @@ class ConsoleBase(object):
m = self.p.expect([pattern_u_boot_spl_signon] +
self.bad_patterns)
if m != 0:
-raise Exception('Bad pattern found on SPL console: ' +
+raise BootFail('Bad pattern found on SPL console: ' +
self.bad_pattern_ids[m - 1])
env_spl_banner_times -= 1
m = self.p.expect([pattern_u_boot_main_signon] +
self.bad_patterns)
if m != 0:
-raise Exception('Bad pattern found on console: ' +
+raise BootFail('Bad pattern found on console: ' +
self.bad_pattern_ids[m - 1])
self.u_boot_version_string = self.p.after
while True:
@@ -181,13 +182,9 @@ class ConsoleBase(object):
if m == 2:
self.p.send(' ')
continue
-raise Exception('Bad pattern found on console: ' +
+raise BootFail('Bad pattern found on console: ' +
self.bad_pattern_ids[m - 3])
-except Exception as ex:
-self.log.error(str(ex))
-self.cleanup_spawn()
-raise
finally:
self.log.timestamp()
@@ -253,7 +250,7 @@ class ConsoleBase(object):
m = self.p.expect([chunk] + self.bad_patterns)
if m != 0:
self.at_prompt = False
-raise Exception('Bad pattern found on console: ' +
+raise BootFail('Bad pattern found on console: ' +
self.bad_pattern_ids[m - 1])
if not wait_for_prompt:
return
@@ -263,14 +260,18 @@ class ConsoleBase(object):
m = self.p.expect([self.prompt_compiled] + self.bad_patterns)
if m != 0:
self.at_prompt = False
-raise Exception('Bad pattern found on console: ' +
+raise BootFail('Missing prompt on console: ' +
self.bad_pattern_ids[m - 1])
self.at_prompt = True
self.at_prompt_logevt = self.logstream.logfile.cur_evt
# Only strip \r\n; space/TAB might be significant if testing
# indentation.
return self.p.before.strip('\r\n')
-except Exception as ex:
+except Timeout as exc:
+self.log.error(str(exc))
+self.cleanup_spawn()
+raise
+except BootFail as ex:
self.log.error(str(ex))
self.cleanup_spawn()
raise
@@ -329,8 +330,9 @@ class ConsoleBase(object):
text = re.escape(text)
m = self.p.expect([text] + self.bad_patterns)
if m != 0:
-raise Exception('Bad pattern found on console: ' +
-self.bad_pattern_ids[m - 1])
+raise Unexpected(
+"Unexpected pattern found on console (exp '{text}': " +
+self.bad_pattern_ids[m - 1])
def drain_console(self):
"""Read from and log the U-Boot console for a short time.
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
index cb0d8f702ba..57ea845ad4c 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -17,6 +17,17 @@ import traceback
class Timeout(Exception):
"""An exception sub-class that indicates that a timeout occurred."""
+class BootFail(Exception):
+"""An exception sub-class that indicates that a boot failure occurred.
+
+This is used when a bad pattern is seen when waiting for the boot prompt.
+It is regarded as fatal, to
[PATCH v2 32/43] test: Move the receive code into a function
There is quite a bit of code to deal with receiving data from the target
so move it into its own receive() function.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/u_boot_spawn.py | 39 +++
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
index 81a09a9d639..cb0d8f702ba 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -139,6 +139,32 @@ class Spawn:
os.write(self.fd, data.encode(errors='replace'))
+def receive(self, num_bytes):
+"""Receive data from the sub-process's stdin.
+
+Args:
+num_bytes (int): Maximum number of bytes to read
+
+Returns:
+str: The data received
+
+Raises:
+ValueError if U-Boot died
+"""
+try:
+c = os.read(self.fd, num_bytes).decode(errors='replace')
+except OSError as err:
+# With sandbox, try to detect when U-Boot exits when it
+# shouldn't and explain why. This is much more friendly than
+# just dying with an I/O error
+if self.decode_signal and err.errno == 5: # I/O error
+alive, _, info = self.checkalive()
+if alive:
+raise err
+raise ValueError('U-Boot exited with %s' % info)
+raise
+return c
+
def expect(self, patterns):
"""Wait for the sub-process to emit specific data.
@@ -195,18 +221,7 @@ class Spawn:
events = self.poll.poll(poll_maxwait)
if not events:
raise Timeout()
-try:
-c = os.read(self.fd, 1024).decode(errors='replace')
-except OSError as err:
-# With sandbox, try to detect when U-Boot exits when it
-# shouldn't and explain why. This is much more friendly
than
-# just dying with an I/O error
-if self.decode_signal and err.errno == 5: # I/O error
-alive, _, info = self.checkalive()
-if alive:
-raise err
-raise ValueError('U-Boot exited with %s' % info)
-raise
+c = self.receive(1024)
if self.logfile_read:
self.logfile_read.write(c)
self.buf += c
--
2.34.1
[PATCH v2 31/43] test: Introduce the concept of a role
In Labgrid there is the concept of a 'role', which is similar to the
U-Boot board ID in U-Boot's pytest subsystem.
The role indicates both the target and information about the U-Boot
build to use. It can also provide any amount of other configuration.
The information is obtained using the 'labgrid-client query' operation.
Make use of this in tests, so that only the role is required in gitlab
and other situations. The board type and other things can be queried
as needed.
Use a new 'u-boot-test-getrole' script to obtain the requested
information.
With this it is possible to run lab tests in gitlab with just a single
'ROLE' variable for each board.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/conftest.py | 31 +++
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/test/py/conftest.py b/test/py/conftest.py
index 6547c6922c6..5de8d7b0e23 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -23,6 +23,7 @@ from pathlib import Path
import pytest
import re
from _pytest.runner import runtestprotocol
+import subprocess
import sys
# Globals: The HTML log file, and the connection to the U-Boot console.
@@ -79,6 +80,7 @@ def pytest_addoption(parser):
parser.addoption('--gdbserver', default=None,
help='Run sandbox under gdbserver. The argument is the channel '+
'over which gdbserver should communicate, e.g. localhost:1234')
+parser.addoption('--role', help='U-Boot board role (for Labgrid)')
parser.addoption('--no-prompt-wait', default=False, action='store_true',
help="Assume that U-Boot is ready and don't wait for a prompt")
@@ -130,12 +132,33 @@ def get_details(config):
str: Build directory
str: Source directory
"""
-board_type = config.getoption('board_type')
-board_identity = config.getoption('board_identity')
+role = config.getoption('role')
build_dir = config.getoption('build_dir')
+if role:
+board_identity = role
+cmd = ['u-boot-test-getrole', role, '--configure']
+env = os.environ.copy()
+if build_dir:
+env['U_BOOT_BUILD_DIR'] = build_dir
+proc = subprocess.run(cmd, capture_output=True, encoding='utf-8',
+ env=env)
+if proc.returncode:
+raise ValueError(proc.stderr)
+print('conftest: lab:', proc.stdout)
+vals = {}
+for line in proc.stdout.splitlines():
+item, value = line.split(' ', maxsplit=1)
+k = item.split(':')[-1]
+vals[k] = value
+print('conftest: lab info:', vals)
+board_type, default_build_dir, source_dir = (vals['board'],
+vals['build_dir'], vals['source_dir'])
+else:
+board_type = config.getoption('board_type')
+board_identity = config.getoption('board_identity')
-source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR))
-default_build_dir = source_dir + '/build-' + board_type
+source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR))
+default_build_dir = source_dir + '/build-' + board_type
if not build_dir:
build_dir = default_build_dir
--
2.34.1
[PATCH v2 30/43] test: Create a common function to get the config
The settings are decoded in two places. Combine them into a new
function, before (in a future patch) expanding the number of items.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/conftest.py | 41 -
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/test/py/conftest.py b/test/py/conftest.py
index ca66b9d9e61..6547c6922c6 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -117,14 +117,36 @@ def run_build(config, source_dir, build_dir, board_type,
log):
runner.close()
log.status_pass('OK')
-def pytest_xdist_setupnodes(config, specs):
-"""Clear out any 'done' file from a previous build"""
-global build_done_file
-build_dir = config.getoption('build_dir')
+def get_details(config):
+"""Obtain salient details about the board and directories to use
+
+Args:
+config (pytest.Config): pytest configuration
+
+Returns:
+tuple:
+str: Board type (U-Boot build name)
+str: Identity for the lab board
+str: Build directory
+str: Source directory
+"""
board_type = config.getoption('board_type')
+board_identity = config.getoption('board_identity')
+build_dir = config.getoption('build_dir')
+
source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR))
+default_build_dir = source_dir + '/build-' + board_type
if not build_dir:
-build_dir = source_dir + '/build-' + board_type
+build_dir = default_build_dir
+
+return board_type, board_identity, build_dir, source_dir
+
+def pytest_xdist_setupnodes(config, specs):
+"""Clear out any 'done' file from a previous build"""
+global build_done_file
+
+build_dir = get_details(config)[2]
+
build_done_file = Path(build_dir) / 'build.done'
if build_done_file.exists():
os.remove(build_done_file)
@@ -163,17 +185,10 @@ def pytest_configure(config):
global console
global ubconfig
-source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR))
+board_type, board_identity, build_dir, source_dir = get_details(config)
-board_type = config.getoption('board_type')
board_type_filename = board_type.replace('-', '_')
-
-board_identity = config.getoption('board_identity')
board_identity_filename = board_identity.replace('-', '_')
-
-build_dir = config.getoption('build_dir')
-if not build_dir:
-build_dir = source_dir + '/build-' + board_type
mkdir_p(build_dir)
result_dir = config.getoption('result_dir')
--
2.34.1
[PATCH v2 29/43] test: Check help output
The current test doesn't check anything about the output. If a bug
results in junk before the output, this is not currently detected.
Add a check for the first line being the one expected.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/tests/test_help.py | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/test/py/tests/test_help.py b/test/py/tests/test_help.py
index 153133cf28f..2325ff69229 100644
--- a/test/py/tests/test_help.py
+++ b/test/py/tests/test_help.py
@@ -7,7 +7,11 @@ import pytest
def test_help(u_boot_console):
"""Test that the "help" command can be executed."""
-u_boot_console.run_command('help')
+lines = u_boot_console.run_command('help')
+if u_boot_console.config.buildconfig.get('config_cmd_2048', 'n') == 'y':
+assert lines.splitlines()[0] == "2048 - The 2048 game"
+else:
+assert lines.splitlines()[0] == "? - alias for 'help'"
@pytest.mark.boardspec('sandbox')
def test_help_no_devicetree(u_boot_console):
--
2.34.1
[PATCH v2 28/43] test: dm: Show failing driver name
When a driver is not registered properly it is not clear which one it
is. Adjust test_dm_compat() to show this.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/tests/test_dm.py | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/test/py/tests/test_dm.py b/test/py/tests/test_dm.py
index 68d4ea12235..be94971e455 100644
--- a/test/py/tests/test_dm.py
+++ b/test/py/tests/test_dm.py
@@ -13,8 +13,11 @@ def test_dm_compat(u_boot_console):
for line in response[:-1].split('\n')[2:])
response = u_boot_console.run_command('dm compat')
+bad_drivers = set()
for driver in drivers:
-assert driver in response
+if not driver in response:
+bad_drivers.add(driver)
+assert not bad_drivers
# check sorting - output looks something like this:
# testacpi 0 [ ] testacpi_drv |-- acpi-test
--
2.34.1
[PATCH v2 27/43] test: Avoid failing skipped tests
When a test returns -EAGAIN this should not be considered a failure.
Fix what seems to be a problem case, where the pytests see a failure
when a test has merely been skipped.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/test-main.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/test/test-main.c b/test/test-main.c
index 3fa6f6e32ec..cda1a186390 100644
--- a/test/test-main.c
+++ b/test/test-main.c
@@ -448,7 +448,7 @@ static int ut_run_test(struct unit_test_state *uts, struct
unit_test *test,
static int ut_run_test_live_flat(struct unit_test_state *uts,
struct unit_test *test)
{
- int runs;
+ int runs, ret;
if ((test->flags & UT_TESTF_OTHER_FDT) && !IS_ENABLED(CONFIG_SANDBOX))
return skip_test(uts);
@@ -458,8 +458,11 @@ static int ut_run_test_live_flat(struct unit_test_state
*uts,
if (CONFIG_IS_ENABLED(OF_LIVE)) {
if (!(test->flags & UT_TESTF_FLAT_TREE)) {
uts->of_live = true;
- ut_assertok(ut_run_test(uts, test, test->name));
- runs++;
+ ret = ut_run_test(uts, test, test->name);
+ if (ret != -EAGAIN) {
+ ut_assertok(ret);
+ runs++;
+ }
}
}
@@ -483,8 +486,11 @@ static int ut_run_test_live_flat(struct unit_test_state
*uts,
(!runs || ut_test_run_on_flattree(test)) &&
!(gd->flags & GD_FLG_FDT_CHANGED)) {
uts->of_live = false;
- ut_assertok(ut_run_test(uts, test, test->name));
- runs++;
+ ret = ut_run_test(uts, test, test->name);
+ if (ret != -EAGAIN) {
+ ut_assertok(ret);
+ runs++;
+ }
}
return 0;
--
2.34.1
[PATCH v2 26/43] test: Decode exceptions only with sandbox
When a real board fails we don't want to decode the exception. Reserve
that behaviour for sandbox. Also avoid raising a new exception on
failure - just re-raise the existing one.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/u_boot_console_sandbox.py | 2 +-
test/py/u_boot_spawn.py | 10 ++
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/test/py/u_boot_console_sandbox.py
b/test/py/u_boot_console_sandbox.py
index 27c6db8d719..7bc44c78b8b 100644
--- a/test/py/u_boot_console_sandbox.py
+++ b/test/py/u_boot_console_sandbox.py
@@ -58,7 +58,7 @@ class ConsoleSandbox(ConsoleBase):
if self.use_dtb:
cmd += ['-d', self.config.dtb]
cmd += self.sandbox_flags
-return Spawn(cmd, cwd=self.config.source_dir)
+return Spawn(cmd, cwd=self.config.source_dir, decode_signal=True)
def restart_uboot_with_flags(self, flags, expect_reset=False,
use_dtb=True):
"""Run U-Boot with the given command-line flags
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
index 7421da49aa9..81a09a9d639 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -25,18 +25,20 @@ class Spawn:
output: accumulated output from expect()
"""
-def __init__(self, args, cwd=None):
+def __init__(self, args, cwd=None, decode_signal=False):
"""Spawn (fork/exec) the sub-process.
Args:
args: array of processs arguments. argv[0] is the command to
execute.
cwd: the directory to run the process in, or None for no change.
+decode_signal (bool): True to indicate the exception number when
+something goes wrong
Returns:
Nothing.
"""
-
+self.decode_signal = decode_signal
self.waited = False
self.exit_code = 0
self.exit_info = ''
@@ -199,12 +201,12 @@ class Spawn:
# With sandbox, try to detect when U-Boot exits when it
# shouldn't and explain why. This is much more friendly
than
# just dying with an I/O error
-if err.errno == 5: # Input/output error
+if self.decode_signal and err.errno == 5: # I/O error
alive, _, info = self.checkalive()
if alive:
raise err
raise ValueError('U-Boot exited with %s' % info)
-raise err
+raise
if self.logfile_read:
self.logfile_read.write(c)
self.buf += c
--
2.34.1
[PATCH v2 25/43] test: Allow connecting to a running board
Sometimes we know that the board is already running the right software,
so provide an option to allow running of tests directly, without first
resetting the board.
This saves time when re-running a test where only the Python code is
changing.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/conftest.py | 3 +++
test/py/u_boot_console_base.py| 14 ++
test/py/u_boot_console_exec_attach.py | 21 -
3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/test/py/conftest.py b/test/py/conftest.py
index fc9dd3a83f8..ca66b9d9e61 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -79,6 +79,8 @@ def pytest_addoption(parser):
parser.addoption('--gdbserver', default=None,
help='Run sandbox under gdbserver. The argument is the channel '+
'over which gdbserver should communicate, e.g. localhost:1234')
+parser.addoption('--no-prompt-wait', default=False, action='store_true',
+help="Assume that U-Boot is ready and don't wait for a prompt")
def run_build(config, source_dir, build_dir, board_type, log):
"""run_build: Build U-Boot
@@ -238,6 +240,7 @@ def pytest_configure(config):
ubconfig.board_type = board_type
ubconfig.board_identity = board_identity
ubconfig.gdbserver = gdbserver
+ubconfig.no_prompt_wait = config.getoption('no_prompt_wait')
ubconfig.dtb = build_dir + '/arch/sandbox/dts/test.dtb'
env_vars = (
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index e4f86f6af5b..a61eec31148 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -413,11 +413,17 @@ class ConsoleBase(object):
if not self.config.gdbserver:
self.p.timeout = TIMEOUT_MS
self.p.logfile_read = self.logstream
-if expect_reset:
-loop_num = 2
+if self.config.no_prompt_wait:
+# Send an empty command to set up the 'expect' logic. This has
+# the side effect of ensuring that there was no partial command
+# line entered
+self.run_command(' ')
else:
-loop_num = 1
-self.wait_for_boot_prompt(loop_num = loop_num)
+if expect_reset:
+loop_num = 2
+else:
+loop_num = 1
+self.wait_for_boot_prompt(loop_num = loop_num)
self.at_prompt = True
self.at_prompt_logevt = self.logstream.logfile.cur_evt
except Exception as ex:
diff --git a/test/py/u_boot_console_exec_attach.py
b/test/py/u_boot_console_exec_attach.py
index 5f4916b7da2..42fc15197b9 100644
--- a/test/py/u_boot_console_exec_attach.py
+++ b/test/py/u_boot_console_exec_attach.py
@@ -59,15 +59,18 @@ class ConsoleExecAttach(ConsoleBase):
args = [self.config.board_type, self.config.board_identity]
s = Spawn(['u-boot-test-console'] + args)
-try:
-self.log.action('Resetting board')
-cmd = ['u-boot-test-reset'] + args
-runner = self.log.get_runner(cmd[0], sys.stdout)
-runner.run(cmd)
-runner.close()
-except:
-s.close()
-raise
+if self.config.no_prompt_wait:
+self.log.action('Connecting to board without reset')
+else:
+try:
+self.log.action('Resetting board')
+cmd = ['u-boot-test-reset'] + args
+runner = self.log.get_runner(cmd[0], sys.stdout)
+runner.run(cmd)
+runner.close()
+except:
+s.close()
+raise
return s
--
2.34.1
[PATCH v2 24/43] log: Allow tests to pass with CONFIG_LOGF_FUNC_PAD set
This setting pads out the function names. Adjust the test to handle
this, since some boards use it.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/tests/test_log.py | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/test/py/tests/test_log.py b/test/py/tests/test_log.py
index 140dcb9aa2b..79808674bbe 100644
--- a/test/py/tests/test_log.py
+++ b/test/py/tests/test_log.py
@@ -27,13 +27,16 @@ def test_log_format(u_boot_console):
cons = u_boot_console
with cons.log.section('format'):
-run_with_format('all', 'NOTICE.arch,file.c:123-func() msg')
+pad =
int(u_boot_console.config.buildconfig.get('config_logf_func_pad'))
+padding = ' ' * (pad - len('func'))
+
+run_with_format('all', f'NOTICE.arch,file.c:123-{padding}func() msg')
output = cons.run_command('log format')
assert output == 'Log format: clFLfm'
-run_with_format('fm', 'func() msg')
-run_with_format('clfm', 'NOTICE.arch,func() msg')
-run_with_format('FLfm', 'file.c:123-func() msg')
+run_with_format('fm', f'{padding}func() msg')
+run_with_format('clfm', f'NOTICE.arch,{padding}func() msg')
+run_with_format('FLfm', f'file.c:123-{padding}func() msg')
run_with_format('lm', 'NOTICE. msg')
run_with_format('m', 'msg')
--
2.34.1
[PATCH v2 23/43] test: Release board after tests complete
When a board is finished with, the lab may want to power it off, or
perform some other function. Add a new script which is called when tests
are complete.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/u_boot_console_exec_attach.py | 10 ++
1 file changed, 10 insertions(+)
diff --git a/test/py/u_boot_console_exec_attach.py
b/test/py/u_boot_console_exec_attach.py
index 8dd8cc1230c..5f4916b7da2 100644
--- a/test/py/u_boot_console_exec_attach.py
+++ b/test/py/u_boot_console_exec_attach.py
@@ -70,3 +70,13 @@ class ConsoleExecAttach(ConsoleBase):
raise
return s
+
+def close(self):
+super().close()
+
+self.log.action('Releasing board')
+args = [self.config.board_type, self.config.board_identity]
+cmd = ['u-boot-test-release'] + args
+runner = self.log.get_runner(cmd[0], sys.stdout)
+runner.run(cmd)
+runner.close()
--
2.34.1
[PATCH v2 22/43] test: Pass stderr to stdout
Some tests may output things to stderr. Ensure that this output is not
dropped, by redirecting it to stdout
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/u_boot_spawn.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
index 7c48d96210e..7421da49aa9 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -10,6 +10,7 @@ import re
import pty
import signal
import select
+import sys
import time
import traceback
@@ -57,6 +58,7 @@ class Spawn:
signal.signal(signal.SIGHUP, signal.SIG_DFL)
if cwd:
os.chdir(cwd)
+sys.stderr = sys.stdout
os.execvp(args[0], args)
except:
print('CHILD EXECEPTION:')
--
2.34.1
[PATCH v2 21/43] test: Use a constant for the test timeout
Declare a constant rather than open-coding the same value twice.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/u_boot_console_base.py | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index e230caf37e1..e4f86f6af5b 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -27,6 +27,9 @@ pattern_ready_prompt = re.compile('U-Boot is ready')
PAT_ID = 0
PAT_RE = 1
+# Timeout before expecting the console to be ready (in milliseconds)
+TIMEOUT_MS = 3
+
bad_pattern_defs = (
('spl_signon', pattern_u_boot_spl_signon),
('main_signon', pattern_u_boot_main_signon),
@@ -397,7 +400,7 @@ class ConsoleBase(object):
# Reset the console timeout value as some tests may change
# its default value during the execution
if not self.config.gdbserver:
-self.p.timeout = 3
+self.p.timeout = TIMEOUT_MS
return
try:
self.log.start_section('Starting U-Boot')
@@ -408,7 +411,7 @@ class ConsoleBase(object):
# future, possibly per-test to be optimal. This works for 'help'
# on board 'seaboard'.
if not self.config.gdbserver:
-self.p.timeout = 3
+self.p.timeout = TIMEOUT_MS
self.p.logfile_read = self.logstream
if expect_reset:
loop_num = 2
--
2.34.1
[PATCH v2 20/43] test: Make bootstd init run only on sandbox
Tests for standard boot need disks to be set up, which can only be done
on sandbox, since adjusting disks on real hardware is not currently
supported. Mark the init function as sandbox-only.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/tests/test_ut.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index c169c835e38..58205066ec8 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -470,6 +470,7 @@ def test_ut_dm_init(u_boot_console):
fh.write(data)
@pytest.mark.buildconfigspec('cmd_bootflow')
+@pytest.mark.buildconfigspec('sandbox')
def test_ut_dm_init_bootstd(u_boot_console):
"""Initialise data for bootflow tests"""
--
2.34.1
[PATCH v2 19/43] test: Allow signaling that U-Boot is ready
When Labgrid is used, it can get U-Boot ready for running tests. It
prints a message when it has done so.
Add logic to detect this message and accept it.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/u_boot_console_base.py | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index 3e01be11029..e230caf37e1 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -22,6 +22,7 @@ pattern_stop_autoboot_prompt = re.compile('Hit any key to
stop autoboot: ')
pattern_unknown_command = re.compile('Unknown command \'.*\' - try \'help\'')
pattern_error_notification = re.compile('## Error: ')
pattern_error_please_reset = re.compile('### ERROR ### Please RESET the board
###')
+pattern_ready_prompt = re.compile('U-Boot is ready')
PAT_ID = 0
PAT_RE = 1
@@ -170,15 +171,15 @@ class ConsoleBase(object):
self.bad_pattern_ids[m - 1])
self.u_boot_version_string = self.p.after
while True:
-m = self.p.expect([self.prompt_compiled,
+m = self.p.expect([self.prompt_compiled, pattern_ready_prompt,
pattern_stop_autoboot_prompt] + self.bad_patterns)
-if m == 0:
+if m == 0 or m == 1:
break
-if m == 1:
+if m == 2:
self.p.send(' ')
continue
raise Exception('Bad pattern found on console: ' +
-self.bad_pattern_ids[m - 2])
+self.bad_pattern_ids[m - 3])
except Exception as ex:
self.log.error(str(ex))
--
2.34.1
[PATCH v2 18/43] meson: Correct driver declaration for meson_axg_gpio
This should use the driver macros so that the driver appears in the
linker list. Fix this.
Fixes: 8587839f19d ("pinctrl: meson: add axg support")
Reviewed-by: Neil Armstrong
Signed-off-by: Simon Glass
---
(no changes since v1)
drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c | 2 +-
drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 ++--
drivers/pinctrl/meson/pinctrl-meson-axg.h | 2 +-
drivers/pinctrl/meson/pinctrl-meson-g12a.c| 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c
b/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c
index 52c726cf038..15ebd574ef1 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c
@@ -179,7 +179,7 @@ static const struct dm_gpio_ops meson_axg_gpio_ops = {
.direction_output = meson_gpio_direction_output,
};
-const struct driver meson_axg_gpio_driver = {
+U_BOOT_DRIVER(meson_axg_gpio) = {
.name = "meson-axg-gpio",
.id = UCLASS_GPIO,
.probe = meson_gpio_probe,
diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.c
b/drivers/pinctrl/meson/pinctrl-meson-axg.c
index 94e09cd3f8a..ed3f92b2d75 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-axg.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-axg.c
@@ -939,7 +939,7 @@ struct meson_pinctrl_data meson_axg_periphs_pinctrl_data = {
.num_groups = ARRAY_SIZE(meson_axg_periphs_groups),
.num_funcs = ARRAY_SIZE(meson_axg_periphs_functions),
.num_banks = ARRAY_SIZE(meson_axg_periphs_banks),
- .gpio_driver= _axg_gpio_driver,
+ .gpio_driver= DM_DRIVER_REF(meson_axg_gpio),
.pmx_data = _axg_periphs_pmx_banks_data,
};
@@ -953,7 +953,7 @@ struct meson_pinctrl_data meson_axg_aobus_pinctrl_data = {
.num_groups = ARRAY_SIZE(meson_axg_aobus_groups),
.num_funcs = ARRAY_SIZE(meson_axg_aobus_functions),
.num_banks = ARRAY_SIZE(meson_axg_aobus_banks),
- .gpio_driver= _axg_gpio_driver,
+ .gpio_driver= DM_DRIVER_REF(meson_axg_gpio),
.pmx_data = _axg_aobus_pmx_banks_data,
};
diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.h
b/drivers/pinctrl/meson/pinctrl-meson-axg.h
index c8d2b3af036..a6581bab500 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-axg.h
+++ b/drivers/pinctrl/meson/pinctrl-meson-axg.h
@@ -61,6 +61,6 @@ struct meson_pmx_axg_data {
}
extern const struct pinctrl_ops meson_axg_pinctrl_ops;
-extern const struct driver meson_axg_gpio_driver;
+extern U_BOOT_DRIVER(meson_axg_gpio);
#endif /* __PINCTRL_MESON_AXG_H__ */
diff --git a/drivers/pinctrl/meson/pinctrl-meson-g12a.c
b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
index 24f47f82558..67114df6824 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-g12a.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
@@ -1253,7 +1253,7 @@ static struct meson_pinctrl_data
meson_g12a_periphs_pinctrl_data = {
.num_groups = ARRAY_SIZE(meson_g12a_periphs_groups),
.num_funcs = ARRAY_SIZE(meson_g12a_periphs_functions),
.num_banks = ARRAY_SIZE(meson_g12a_periphs_banks),
- .gpio_driver= _axg_gpio_driver,
+ .gpio_driver= DM_DRIVER_REF(meson_axg_gpio),
.pmx_data = _g12a_periphs_pmx_banks_data,
};
@@ -1267,7 +1267,7 @@ static struct meson_pinctrl_data
meson_g12a_aobus_pinctrl_data = {
.num_groups = ARRAY_SIZE(meson_g12a_aobus_groups),
.num_funcs = ARRAY_SIZE(meson_g12a_aobus_functions),
.num_banks = ARRAY_SIZE(meson_g12a_aobus_banks),
- .gpio_driver= _axg_gpio_driver,
+ .gpio_driver= DM_DRIVER_REF(meson_axg_gpio),
.pmx_data = _g12a_aobus_pmx_banks_data,
};
--
2.34.1
[PATCH v2 17/43] google: Disable TPMv2 on most Chromebooks
This feature is not present on older Chromebooks, so disable the setting. Signed-off-by: Simon Glass Reviewed-by: Ilias Apalodimas --- (no changes since v1) configs/chromebook_link64_defconfig| 1 + configs/chromebook_link_defconfig | 1 + configs/chromebook_samus_defconfig | 1 + configs/chromebook_samus_tpl_defconfig | 1 + configs/nyan-big_defconfig | 4 +--- configs/snow_defconfig | 1 + 6 files changed, 6 insertions(+), 3 deletions(-) diff --git a/configs/chromebook_link64_defconfig b/configs/chromebook_link64_defconfig index 7cf23b29e46..9583f87bf0f 100644 --- a/configs/chromebook_link64_defconfig +++ b/configs/chromebook_link64_defconfig @@ -80,6 +80,7 @@ CONFIG_SYS_NS16550=y CONFIG_SYS_NS16550_PORT_MAPPED=y CONFIG_SPI=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig index a9f91dd9b26..75a7a488a92 100644 --- a/configs/chromebook_link_defconfig +++ b/configs/chromebook_link_defconfig @@ -70,6 +70,7 @@ CONFIG_SYS_NS16550_PORT_MAPPED=y CONFIG_SOUND=y CONFIG_SPI=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_VIDEO_COPY=y diff --git a/configs/chromebook_samus_defconfig b/configs/chromebook_samus_defconfig index 40cc449b9b3..8cdad8d2344 100644 --- a/configs/chromebook_samus_defconfig +++ b/configs/chromebook_samus_defconfig @@ -74,6 +74,7 @@ CONFIG_SOUND_I8254=y CONFIG_SOUND_RT5677=y CONFIG_SPI=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_VIDEO_COPY=y diff --git a/configs/chromebook_samus_tpl_defconfig b/configs/chromebook_samus_tpl_defconfig index 3e7298f16af..1be57560f89 100644 --- a/configs/chromebook_samus_tpl_defconfig +++ b/configs/chromebook_samus_tpl_defconfig @@ -96,6 +96,7 @@ CONFIG_SOUND_RT5677=y CONFIG_SPI=y CONFIG_TPL_SYSRESET=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y diff --git a/configs/nyan-big_defconfig b/configs/nyan-big_defconfig index 4dec710cf8d..78fb7580da7 100644 --- a/configs/nyan-big_defconfig +++ b/configs/nyan-big_defconfig @@ -11,8 +11,6 @@ CONFIG_DEFAULT_DEVICE_TREE="tegra124-nyan-big" CONFIG_SPL_TEXT_BASE=0x80108000 CONFIG_SPL_STACK=0x800c CONFIG_BOOTSTAGE_STASH_ADDR=0x8300 -CONFIG_DEBUG_UART_BASE=0x70006000 -CONFIG_DEBUG_UART_CLOCK=40800 CONFIG_TEGRA124=y CONFIG_TARGET_NYAN_BIG=y CONFIG_TEGRA_GPU=y @@ -75,7 +73,6 @@ CONFIG_DM_REGULATOR=y CONFIG_REGULATOR_AS3722=y CONFIG_DM_REGULATOR_FIXED=y CONFIG_PWM_TEGRA=y -CONFIG_DEBUG_UART_SHIFT=2 CONFIG_SYS_NS16550=y CONFIG_SOUND=y CONFIG_I2S=y @@ -83,6 +80,7 @@ CONFIG_I2S_TEGRA=y CONFIG_SOUND_MAX98090=y CONFIG_TEGRA114_SPI=y CONFIG_TPM_TIS_INFINEON=y +# CONFIG_TPM_V2 is not set CONFIG_USB=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_TEGRA=y diff --git a/configs/snow_defconfig b/configs/snow_defconfig index 3a617c6cf40..2c0757194bd 100644 --- a/configs/snow_defconfig +++ b/configs/snow_defconfig @@ -88,6 +88,7 @@ CONFIG_SOUND_MAX98095=y CONFIG_SOUND_WM8994=y CONFIG_EXYNOS_SPI=y CONFIG_TPM_TIS_INFINEON=y +# CONFIG_TPM_V2 is not set CONFIG_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y -- 2.34.1
[PATCH v2 16/43] sunxi: Mark scp as optional
This binary does not prevent the system from booting. Mark it optional
so that U-Boot can be built without it.
Signed-off-by: Simon Glass
---
(no changes since v1)
arch/arm/dts/sunxi-u-boot.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi
index 0909a67883e..e1a9a7f5d4c 100644
--- a/arch/arm/dts/sunxi-u-boot.dtsi
+++ b/arch/arm/dts/sunxi-u-boot.dtsi
@@ -90,6 +90,7 @@
scp {
filename = "scp.bin";
missing-msg = "scp-sunxi";
+ optional;
};
};
#endif
--
2.34.1
[PATCH v2 15/43] am33xx: Provide a function to set up the debug UART
Since commit 0dba45864b2a ("arm: Init the debug UART") the debug UART is
set up in _main() before early_system_init() is called.
Add a suitable board_debug_uart_init() function to set up the UART in
SPL.
Signed-off-by: Simon Glass
---
(no changes since v1)
arch/arm/mach-omap2/am33xx/board.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/am33xx/board.c
b/arch/arm/mach-omap2/am33xx/board.c
index 78c1e965c9f..84a60dedd72 100644
--- a/arch/arm/mach-omap2/am33xx/board.c
+++ b/arch/arm/mach-omap2/am33xx/board.c
@@ -490,9 +490,6 @@ void early_system_init(void)
*/
save_omap_boot_params();
#endif
-#ifdef CONFIG_DEBUG_UART_OMAP
- debug_uart_init();
-#endif
#ifdef CONFIG_SPL_BUILD
spl_early_init();
@@ -533,3 +530,18 @@ static int am33xx_dm_post_init(void)
return 0;
}
EVENT_SPY_SIMPLE(EVT_DM_POST_INIT_F, am33xx_dm_post_init);
+
+#ifdef CONFIG_DEBUG_UART_BOARD_INIT
+void board_debug_uart_init(void)
+{
+ if (u_boot_first_phase()) {
+ hw_data_init();
+ set_uart_mux_conf();
+ setup_early_clocks();
+ uart_soft_reset();
+
+ /* avoid uart gibberish by allowing the clocks to settle */
+ mdelay(50);
+ }
+}
+#endif
--
2.34.1
[PATCH v2 14/43] initcall: Correct use of relocation offset
The relocation offset can change in some initcall sequences. Handle
this and make sure it is used for all debugging statements in
init_run_list()
Update the trace test to match.
Signed-off-by: Simon Glass
Reviewed-by: Caleb Connolly
---
(no changes since v1)
lib/initcall.c | 6 --
test/py/tests/test_trace.py | 4 ++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/initcall.c b/lib/initcall.c
index c8e2b0f6a38..2686b9aed5c 100644
--- a/lib/initcall.c
+++ b/lib/initcall.c
@@ -49,13 +49,14 @@ static int initcall_is_event(init_fnc_t func)
*/
int initcall_run_list(const init_fnc_t init_sequence[])
{
- ulong reloc_ofs = calc_reloc_ofs();
+ ulong reloc_ofs;
const init_fnc_t *ptr;
enum event_t type;
init_fnc_t func;
int ret = 0;
for (ptr = init_sequence; func = *ptr, func; ptr++) {
+ reloc_ofs = calc_reloc_ofs();
type = initcall_is_event(func);
if (type) {
@@ -84,7 +85,8 @@ int initcall_run_list(const init_fnc_t init_sequence[])
sprintf(buf, "event %d/%s", type,
event_type_name(type));
} else {
- sprintf(buf, "call %p", func);
+ sprintf(buf, "call %p",
+ (char *)func - reloc_ofs);
}
printf("initcall failed at %s (err=%dE)\n", buf, ret);
diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py
index f41d4cf71f0..ec1e624722c 100644
--- a/test/py/tests/test_trace.py
+++ b/test/py/tests/test_trace.py
@@ -175,7 +175,7 @@ def check_funcgraph(cons, fname, proftool, map_fname,
trace_dat):
# Then look for this:
# u-boot-1 0. 282.101375: funcgraph_exit: 0.006 us |
}
# Then check for this:
-# u-boot-1 0. 282.101375: funcgraph_entry:0.000 us |
initcall_is_event();
+# u-boot-1 0. 282.101375: funcgraph_entry:0.000 us |
calc_reloc_ofs();
expected_indent = None
found_start = False
@@ -199,7 +199,7 @@ def check_funcgraph(cons, fname, proftool, map_fname,
trace_dat):
# The next function after initf_bootstage() exits should be
# initcall_is_event()
-assert upto == 'initcall_is_event()'
+assert upto == 'calc_reloc_ofs()'
# Now look for initf_dm() and dm_timer_init() so we can check the bootstage
# time
--
2.34.1
[PATCH v2 13/43] dm: core: Enhance comments on bind_drivers_pass()
This part of driver model is a little subtle, so add some more comments
to promote better understanding.
Signed-off-by: Simon Glass
---
(no changes since v1)
drivers/core/lists.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index 2839a9b7371..a84e6a98cb1 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -8,6 +8,7 @@
#define LOG_CATEGORY LOGC_DM
+#include
#include
#include
#include
@@ -50,6 +51,21 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id)
return NULL;
}
+/**
+ * bind_drivers_pass() - Perform a pass of driver binding
+ *
+ * Work through the driver_info records binding a driver for each one. If the
+ * binding fails, continue binding others, but return the error.
+ *
+ * For OF_PLATDATA we must bind parent devices before their children. So only
+ * children of bound parents are bound on each call to this function. When a
+ * child is left unbound, -EAGAIN is returned, indicating that this function
+ * should be called again
+ *
+ * @parent: Parent device to use when binding each child device
+ * Return: 0 if OK, -EAGAIN if unbound children exist, -ENOENT if there is no
+ * driver for one of the devices, other -ve on other error
+ */
static int bind_drivers_pass(struct udevice *parent, bool pre_reloc_only)
{
struct driver_info *info =
--
2.34.1
[PATCH v2 12/43] buildman: Add a way to limit the number of buildmans
Buildman uses all available CPUs by default, so running more than one or
two concurrent processes is not normally useful.
However in some CI cases we want to be able to run several jobs at once
to save time. For example, in a lab situation we may want to run a test
on 20 boards at a time, since only the build step actually takes much
CPU.
Add an option which allows such a limit. When buildman starts up, it
waits until the number of running processes goes below the limit, then
claims a spot in the list. The list is maintained with a temporary file.
Note that the temp file is user-specific, since it is hard to create a
locked temporary file which can be accessed by any user. In most cases,
only one user is running jobs on a machine, so this should not matter.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/buildman.rst| 5 ++
tools/buildman/cmdline.py | 2 +
tools/buildman/control.py | 140 -
tools/buildman/pyproject.toml | 6 +-
tools/buildman/test.py | 121
tools/u_boot_pylib/terminal.py | 7 +-
6 files changed, 277 insertions(+), 4 deletions(-)
diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst
index bd0482af5f7..b8ff3bf1ab2 100644
--- a/tools/buildman/buildman.rst
+++ b/tools/buildman/buildman.rst
@@ -1286,6 +1286,11 @@ then buildman hangs. Failing to handle any eventuality
is a bug in buildman and
should be reported. But you can use -T0 to disable threading and hopefully
figure out the root cause of the build failure.
+For situations where buildman is invoked from multiple running processes, it is
+sometimes useful to have buildman wait until the others have finished. Use the
+--process-limit option for this: --process-limit 1 will allow only one buildman
+to process jobs at a time.
+
Build summary
-
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 8dc5a8787b5..544a391a464 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -129,6 +129,8 @@ def add_after_m(parser):
default=False, help="Use an O= (output) directory per board rather
than per thread")
parser.add_argument('--print-arch', action='store_true',
default=False, help="Print the architecture for a board (ARCH=)")
+parser.add_argument('--process-limit', type=int,
+ default=0, help='Limit to number of buildmans running at once')
parser.add_argument('-r', '--reproducible-builds', action='store_true',
help='Set SOURCE_DATE_EPOCH=0 to suuport a reproducible build')
parser.add_argument('-R', '--regen-board-list', type=str,
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index f2dd87814c3..464835c5be5 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -7,10 +7,13 @@
This holds the main control logic for buildman, when not running tests.
"""
+import getpass
import multiprocessing
import os
import shutil
import sys
+import tempfile
+import time
from buildman import boards
from buildman import bsettings
@@ -21,10 +24,23 @@ from patman import gitutil
from patman import patchstream
from u_boot_pylib import command
from u_boot_pylib import terminal
-from u_boot_pylib.terminal import tprint
+from u_boot_pylib import tools
+from u_boot_pylib.terminal import print_clear, tprint
TEST_BUILDER = None
+# Space-separated list of buildman process IDs currently running jobs
+RUNNING_FNAME = f'buildmanq.{getpass.getuser()}'
+
+# Lock file for access to RUNNING_FILE
+LOCK_FNAME = f'{RUNNING_FNAME}.lock'
+
+# Wait time for access to lock (seconds)
+LOCK_WAIT_S = 10
+
+# Wait time to start running
+RUN_WAIT_S = 300
+
def get_plural(count):
"""Returns a plural 's' if count is not 1"""
return 's' if count != 1 else ''
@@ -578,6 +594,125 @@ def calc_adjust_cfg(adjust_cfg, reproducible_builds):
return adjust_cfg
+def read_procs(tmpdir=tempfile.gettempdir()):
+"""Read the list of running buildman processes
+
+If the list is corrupted, returns an empty list
+
+Args:
+tmpdir (str): Temporary directory to use (for testing only)
+"""
+running_fname = os.path.join(tmpdir, RUNNING_FNAME)
+procs = []
+if os.path.exists(running_fname):
+items = tools.read_file(running_fname, binary=False).split()
+try:
+procs = [int(x) for x in items]
+except ValueError: # Handle invalid format
+pass
+return procs
+
+
+def check_pid(pid):
+"""Check for existence of a unix PID
+
+
https://stackoverflow.com/questions/568271/how-to-check-if-there-exists-a-process-with-a-given-pid-in-python
+
+Args:
+pid (int): PID to check
+
+Returns:
+True if it exists, else False
+"""
+try:
+os.kill(pid, 0)
+except OSError:
+return False
+else:
+return True
+
+
+def write_procs(procs, tmpdir=tempfile.gettempdir()):
+
[PATCH v2 11/43] buildman: Retry the build for current source
Buildman retries a failed build when processing a branch, but does not do this when building current source. It is useful to do this retry in both cases, so add the logic for it. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builderthread.py | 8 1 file changed, 8 insertions(+) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index bbe2f6f0d24..55658487abf 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -755,6 +755,14 @@ class BuilderThread(threading.Thread): self.mrproper, self.builder.config_only, True, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) +failed = result.return_code or result.stderr +if failed and not self.mrproper: +result, request_config = self.run_commit(None, brd, work_dir, +True, self.builder.fallback_mrproper, +self.builder.config_only, True, +self.builder.force_build_failures, +job.work_in_output, job.adjust_cfg) + result.commit_upto = 0 self._write_result(result, job.keep_outputs, job.work_in_output) self._send_result(result) -- 2.34.1
[PATCH v2 10/43] buildman: Add a flag to force mrproper on failure
When a file is removed by a commit (e.g. include/common.h yay!) it can cause incremental build failures since one of the dependency files from a previous build may mention the file. Add an option to run 'make mrproper' automatically when a build fails. This can be used to automatically resolve the problem, without always adding the large overhead of 'make mrproper' to every build. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builder.py | 18 ++ tools/buildman/builderthread.py | 6 -- tools/buildman/buildman.rst | 3 ++- tools/buildman/cmdline.py | 4 +++- tools/buildman/control.py | 1 + 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index f35175b4598..c4384f53e8d 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -256,14 +256,14 @@ class Builder: def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1, no_subdirs=False, full_path=False, verbose_build=False, - mrproper=False, per_board_out_dir=False, - config_only=False, squash_config_y=False, - warnings_as_errors=False, work_in_output=False, - test_thread_exceptions=False, adjust_cfg=None, - allow_missing=False, no_lto=False, reproducible_builds=False, - force_build=False, force_build_failures=False, - force_reconfig=False, in_tree=False, - force_config_on_failure=False, make_func=None): + mrproper=False, fallback_mrproper=False, + per_board_out_dir=False, config_only=False, + squash_config_y=False, warnings_as_errors=False, + work_in_output=False, test_thread_exceptions=False, + adjust_cfg=None, allow_missing=False, no_lto=False, + reproducible_builds=False, force_build=False, + force_build_failures=False, force_reconfig=False, + in_tree=False, force_config_on_failure=False, make_func=None): """Create a new Builder object Args: @@ -283,6 +283,7 @@ class Builder: PATH verbose_build: Run build with V=1 and don't use 'make -s' mrproper: Always run 'make mrproper' when configuring +fallback_mrproper: Run 'make mrproper' and retry on build failure per_board_out_dir: Build in a separate persistent directory per board rather than a thread-specific directory config_only: Only configure each build, don't build it @@ -352,6 +353,7 @@ class Builder: self.force_reconfig = force_reconfig self.in_tree = in_tree self.force_config_on_failure = force_config_on_failure +self.fallback_mrproper = fallback_mrproper if not self.squash_config_y: self.config_filenames += EXTRA_CONFIG_FILENAMES diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index c0b1067e3f7..bbe2f6f0d24 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -705,8 +705,10 @@ class BuilderThread(threading.Thread): # with a reconfig. if self.builder.force_config_on_failure: result, request_config = self.run_commit(commit_upto, -brd, work_dir, True, self.mrproper, False, True, -False, job.work_in_output, job.adjust_cfg) +brd, work_dir, True, +self.mrproper or self.builder.fallback_mrproper, +False, True, False, job.work_in_output, +job.adjust_cfg) did_config = True if not self.builder.force_reconfig: do_config = request_config diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst index aae2477b5c3..bd0482af5f7 100644 --- a/tools/buildman/buildman.rst +++ b/tools/buildman/buildman.rst @@ -995,7 +995,8 @@ By default, buildman doesn't execute 'make mrproper' prior to building the first commit for each board. This reduces the amount of work 'make' does, and hence speeds up the build. To force use of 'make mrproper', use -the -m flag. This flag will slow down any buildman invocation, since it increases the amount -of work done on any build. +of work done on any build. An alternative is to use the --fallback-mrproper +flag, which retries the build with 'make mrproper' only after a build failure. One possible application of buildman is as part of a continual edit, build, edit, build, ... cycle; repeatedly applying buildman to the same change or diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index
[PATCH v2 09/43] buildman: Avoid rebuilding when --mrproper is used
When this flag is enabled, 'make mrproper' is always used when reconfiguring, so there is no point in doing it again. Update this. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builderthread.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 0a7ff2e083e..c0b1067e3f7 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -700,7 +700,7 @@ class BuilderThread(threading.Thread): job.work_in_output, job.adjust_cfg) failed = result.return_code or result.stderr did_config = do_config -if failed and not do_config: +if failed and not do_config and not self.mrproper: # If our incremental build failed, try building again # with a reconfig. if self.builder.force_config_on_failure: -- 2.34.1
[PATCH v2 08/43] buildman: Make mrproper an argument to run_commit()
Pass this in so the caller can change it independently of the member variable. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builderthread.py | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index ff63f9397e6..0a7ff2e083e 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -448,9 +448,9 @@ class BuilderThread(threading.Thread): result.cmd_list = cmd_list return result, do_config -def run_commit(self, commit_upto, brd, work_dir, do_config, config_only, - force_build, force_build_failures, work_in_output, - adjust_cfg): +def run_commit(self, commit_upto, brd, work_dir, do_config, mrproper, + config_only, force_build, force_build_failures, + work_in_output, adjust_cfg): """Build a particular commit. If the build is already done, and we are not forcing a build, we skip @@ -461,6 +461,7 @@ class BuilderThread(threading.Thread): brd (Board): Board to build work_dir (str): Directory to which the source will be checked out do_config (bool): True to run a make _defconfig on the source +mrproper (bool): True to run mrproper first config_only (bool): Only configure the source, do not build it force_build (bool): Force a build even if one was previously done force_build_failures (bool): Force a bulid if the previous result @@ -501,7 +502,7 @@ class BuilderThread(threading.Thread): if self.toolchain: commit = self._checkout(commit_upto, work_dir) result, do_config = self._config_and_build( -commit_upto, brd, work_dir, do_config, self.mrproper, +commit_upto, brd, work_dir, do_config, mrproper, config_only, adjust_cfg, commit, out_dir, out_rel_dir, result) result.already_done = False @@ -692,7 +693,8 @@ class BuilderThread(threading.Thread): force_build = False for commit_upto in range(0, len(job.commits), job.step): result, request_config = self.run_commit(commit_upto, brd, -work_dir, do_config, self.builder.config_only, +work_dir, do_config, self.mrproper, +self.builder.config_only, force_build or self.builder.force_build, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) @@ -703,8 +705,8 @@ class BuilderThread(threading.Thread): # with a reconfig. if self.builder.force_config_on_failure: result, request_config = self.run_commit(commit_upto, -brd, work_dir, True, False, True, False, -job.work_in_output, job.adjust_cfg) +brd, work_dir, True, self.mrproper, False, True, +False, job.work_in_output, job.adjust_cfg) did_config = True if not self.builder.force_reconfig: do_config = request_config @@ -748,7 +750,7 @@ class BuilderThread(threading.Thread): else: # Just build the currently checked-out build result, request_config = self.run_commit(None, brd, work_dir, True, -self.builder.config_only, True, +self.mrproper, self.builder.config_only, True, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) result.commit_upto = 0 -- 2.34.1
[PATCH v2 07/43] buildman: Make mrproper an argument to _config_and_build()
Pass this in so the caller can change it independently of the member variable. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builderthread.py | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 5d4426bf0d1..ff63f9397e6 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -381,7 +381,7 @@ class BuilderThread(threading.Thread): commit = 'current' return commit -def _config_and_build(self, commit_upto, brd, work_dir, do_config, +def _config_and_build(self, commit_upto, brd, work_dir, do_config, mrproper, config_only, adjust_cfg, commit, out_dir, out_rel_dir, result): """Do the build, configuring first if necessary @@ -391,6 +391,7 @@ class BuilderThread(threading.Thread): brd (Board): Board to create arguments for work_dir (str): Directory to which the source will be checked out do_config (bool): True to run a make _defconfig on the source +mrproper (bool): True to run mrproper first config_only (bool): Only configure the source, do not build it adjust_cfg (list of str): See the cfgutil module and run_commit() commit (Commit): Commit only being built @@ -421,7 +422,7 @@ class BuilderThread(threading.Thread): if do_config or adjust_cfg: result = self._reconfigure( commit, brd, cwd, args, env, config_args, config_out, cmd_list, -self.mrproper) +mrproper) do_config = False # No need to configure next time if adjust_cfg: cfgutil.adjust_cfg_file(cfg_file, adjust_cfg) @@ -500,8 +501,9 @@ class BuilderThread(threading.Thread): if self.toolchain: commit = self._checkout(commit_upto, work_dir) result, do_config = self._config_and_build( -commit_upto, brd, work_dir, do_config, config_only, -adjust_cfg, commit, out_dir, out_rel_dir, result) +commit_upto, brd, work_dir, do_config, self.mrproper, +config_only, adjust_cfg, commit, out_dir, out_rel_dir, +result) result.already_done = False result.toolchain = self.toolchain -- 2.34.1
[PATCH v2 06/43] buildman: Make mrproper an argument to _reconfigure()
Pass this in so the caller can change it independently of the member variable. Signed-off-by: Simon Glass --- (no changes since v1) tools/buildman/builderthread.py | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index a8599c0bb2a..5d4426bf0d1 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -240,7 +240,7 @@ class BuilderThread(threading.Thread): return args, cwd, src_dir def _reconfigure(self, commit, brd, cwd, args, env, config_args, config_out, - cmd_list): + cmd_list, mrproper): """Reconfigure the build Args: @@ -251,11 +251,12 @@ class BuilderThread(threading.Thread): env (dict): Environment strings config_args (list of str): defconfig arg for this board cmd_list (list of str): List to add the commands to, for logging +mrproper (bool): True to run mrproper first Returns: CommandResult object """ -if self.mrproper: +if mrproper: result = self.make(commit, brd, 'mrproper', cwd, 'mrproper', *args, env=env) config_out.write(result.combined) @@ -419,7 +420,8 @@ class BuilderThread(threading.Thread): cmd_list = [] if do_config or adjust_cfg: result = self._reconfigure( -commit, brd, cwd, args, env, config_args, config_out, cmd_list) +commit, brd, cwd, args, env, config_args, config_out, cmd_list, +self.mrproper) do_config = False # No need to configure next time if adjust_cfg: cfgutil.adjust_cfg_file(cfg_file, adjust_cfg) -- 2.34.1
[PATCH v2 05/43] binman: Update the entrydocs header
Reduce the length of the underline for this header, to match the
heading itself.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/binman/entry.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 42e0b7b9145..2ed65800d22 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -812,7 +812,7 @@ class Entry(object):
as missing
"""
print('''Binman Entry Documentation
-===
+==
This file describes the entry types supported by binman. These entry types can
be placed in an image one by one to build up a final firmware image. It is
--
2.34.1
[PATCH v2 04/43] binman: ti: Regenerate entry docs
Correct formatting errors in the documentation.
Regenerate the entries.rst file to include this recent addition.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/binman/entries.rst| 35 +
tools/binman/etype/ti_secure.py | 45 +
2 files changed, 58 insertions(+), 22 deletions(-)
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index ecccf77d112..f59c0341840 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -1958,6 +1958,12 @@ Properties / Entry arguments:
- content: List of phandles to entries to sign
- keyfile: Filename of file containing key to sign binary with
- sha: Hash function to be used for signing
+- auth-in-place: This is an integer field that contains two pieces
+ of information:
+
+- Lower Byte - Remains 0x02 as per our use case
+ ( 0x02: Move the authenticated binary back to the header )
+- Upper Byte - The Host ID of the core owning the firewall
Output files:
- input. - input file passed to openssl
@@ -1966,6 +1972,35 @@ Output files:
- cert. - output file generated by openssl (which is
used as the entry contents)
+Depending on auth-in-place information in the inputs, we read the
+firewall nodes that describe the configurations of firewall that TIFS
+will be doing after reading the certificate.
+
+The syntax of the firewall nodes are as such::
+
+firewall-257-0 {
+id = <257>; /* The ID of the firewall being configured */
+region = <0>; /* Region number to configure */
+
+control = /* The control register */
+<(FWCTRL_EN | FWCTRL_LOCK | FWCTRL_BG | FWCTRL_CACHE)>;
+
+permissions = /* The permission registers */
+<((FWPRIVID_ALL << FWPRIVID_SHIFT) |
+FWPERM_SECURE_PRIV_RWCD |
+FWPERM_SECURE_USER_RWCD |
+FWPERM_NON_SECURE_PRIV_RWCD |
+FWPERM_NON_SECURE_USER_RWCD)>;
+
+/* More defines can be found in k3-security.h */
+
+start_address =/* The Start Address of the firewall */
+<0x0 0x0>;
+end_address = /* The End Address of the firewall */
+<0xff 0x>;
+};
+
+
openssl signs the provided data, using the TI templated config file and
writes the signature in this entry. This allows verification that the
data is genuine.
diff --git a/tools/binman/etype/ti_secure.py b/tools/binman/etype/ti_secure.py
index 704dcf8a381..420ee263e4f 100644
--- a/tools/binman/etype/ti_secure.py
+++ b/tools/binman/etype/ti_secure.py
@@ -53,10 +53,11 @@ class Entry_ti_secure(Entry_x509_cert):
- keyfile: Filename of file containing key to sign binary with
- sha: Hash function to be used for signing
- auth-in-place: This is an integer field that contains two pieces
- of information
-Lower Byte - Remains 0x02 as per our use case
-( 0x02: Move the authenticated binary back to the header )
-Upper Byte - The Host ID of the core owning the firewall
+ of information:
+
+- Lower Byte - Remains 0x02 as per our use case
+ ( 0x02: Move the authenticated binary back to the header )
+- Upper Byte - The Host ID of the core owning the firewall
Output files:
- input. - input file passed to openssl
@@ -69,29 +70,29 @@ class Entry_ti_secure(Entry_x509_cert):
firewall nodes that describe the configurations of firewall that TIFS
will be doing after reading the certificate.
-The syntax of the firewall nodes are as such:
+The syntax of the firewall nodes are as such::
-firewall-257-0 {
-id = <257>; /* The ID of the firewall being configured */
-region = <0>; /* Region number to configure */
+firewall-257-0 {
+id = <257>; /* The ID of the firewall being configured */
+region = <0>; /* Region number to configure */
-control = /* The control register */
-<(FWCTRL_EN | FWCTRL_LOCK | FWCTRL_BG | FWCTRL_CACHE)>;
+control = /* The control register */
+<(FWCTRL_EN | FWCTRL_LOCK | FWCTRL_BG | FWCTRL_CACHE)>;
-permissions = /* The permission registers */
-<((FWPRIVID_ALL << FWPRIVID_SHIFT) |
-FWPERM_SECURE_PRIV_RWCD |
-FWPERM_SECURE_USER_RWCD |
-FWPERM_NON_SECURE_PRIV_RWCD |
-FWPERM_NON_SECURE_USER_RWCD)>;
+permissions = /* The permission registers */
+<((FWPRIVID_ALL << FWPRIVID_SHIFT) |
+FWPERM_SECURE_PRIV_RWCD |
+FWPERM_SECURE_USER_RWCD |
+
[PATCH v2 02/43] binman: efi: Correct entry docs
Somehow the class documentation has got out of sync with the generated
entries.rst file. Regenerating it causes errors, so correct these and
regenerate the entries.rst file.
Signed-off-by: Simon Glass
Fixes: 809f28e7213 ("binman: capsule: Use dumped capsule header...")
---
(no changes since v1)
tools/binman/entries.rst| 58 -
tools/binman/etype/efi_capsule.py | 40 -
tools/binman/etype/efi_empty_capsule.py | 22 +-
3 files changed, 61 insertions(+), 59 deletions(-)
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index 86a3c02b485..531a92e2e0c 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -477,11 +477,11 @@ updating the EC on startup via software sync.
.. _etype_efi_capsule:
-Entry: capsule: Entry for generating EFI Capsule files
---
+Entry: efi-capsule: Generate EFI capsules
+-
-The parameters needed for generation of the capsules can be provided
-as properties in the entry.
+The parameters needed for generation of the capsules can
+be provided as properties in the entry.
Properties / Entry arguments:
- image-index: Unique number for identifying corresponding
@@ -502,9 +502,9 @@ Properties / Entry arguments:
file. Mandatory property for generating signed capsules.
- oem-flags - OEM flags to be passed through capsule header.
-Since this is a subclass of Entry_section, all properties of the parent
-class also apply here. Except for the properties stated as mandatory, the
-rest of the properties are optional.
+Since this is a subclass of Entry_section, all properties of the parent
+class also apply here. Except for the properties stated as mandatory, the
+rest of the properties are optional.
For more details on the description of the capsule format, and the capsule
update functionality, refer Section 8.5 and Chapter 23 in the `UEFI
@@ -517,17 +517,17 @@ provided as a subnode of the capsule entry.
A typical capsule entry node would then look something like this::
capsule {
-type = "efi-capsule";
-image-index = <0x1>;
-/* Image GUID for testing capsule update */
-image-guid = SANDBOX_UBOOT_IMAGE_GUID;
-hardware-instance = <0x0>;
-private-key = "path/to/the/private/key";
-public-key-cert = "path/to/the/public-key-cert";
-oem-flags = <0x8000>;
+type = "efi-capsule";
+image-index = <0x1>;
+/* Image GUID for testing capsule update */
+image-guid = SANDBOX_UBOOT_IMAGE_GUID;
+hardware-instance = <0x0>;
+private-key = "path/to/the/private/key";
+public-key-cert = "path/to/the/public-key-cert";
+oem-flags = <0x8000>;
-u-boot {
-};
+u-boot {
+};
};
In the above example, the capsule payload is the U-Boot image. The
@@ -541,8 +541,8 @@ payload using the blob-ext subnode.
.. _etype_efi_empty_capsule:
-Entry: efi-empty-capsule: Entry for generating EFI Empty Capsule files
---
+Entry: efi-empty-capsule: Generate EFI empty capsules
+-
The parameters needed for generation of the empty capsules can
be provided as properties in the entry.
@@ -558,22 +558,22 @@ update functionality, refer Section 8.5 and Chapter 23 in
the `UEFI
specification`_. For more information on the empty capsule, refer the
sections 2.3.2 and 2.3.3 in the `Dependable Boot specification`_.
-A typical accept empty capsule entry node would then look something
-like this::
+A typical accept empty capsule entry node would then look something like
+this::
empty-capsule {
-type = "efi-empty-capsule";
-/* GUID of the image being accepted */
-image-type-id = SANDBOX_UBOOT_IMAGE_GUID;
-capsule-type = "accept";
+type = "efi-empty-capsule";
+/* GUID of image being accepted */
+image-type-id = SANDBOX_UBOOT_IMAGE_GUID;
+capsule-type = "accept";
};
-A typical revert empty capsule entry node would then look something
-like this::
+A typical revert empty capsule entry node would then look something like
+this::
empty-capsule {
-type = "efi-empty-capsule";
-capsule-type = "revert";
+type = "efi-empty-capsule";
+capsule-type = "revert";
};
The empty capsules do not have any input payload image.
diff --git a/tools/binman/etype/efi_capsule.py
b/tools/binman/etype/efi_capsule.py
index e3203717822..751f654bf31 100644
--- a/tools/binman/etype/efi_capsule.py
+++ b/tools/binman/etype/efi_capsule.py
@@ -36,23 +36,23 @@ class Entry_efi_capsule(Entry_section):
be provided as properties in the entry.
Properties / Entry arguments:
[PATCH v2 03/43] binman: Regenerate nxp docs
Regenerate the entries.rst file to include this recent addition.
Note that more docs are needed here, to actually describe the entry
type.
Note also that the entry type needs Binman tests added.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/binman/entries.rst | 22 ++
1 file changed, 22 insertions(+)
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index 531a92e2e0c..ecccf77d112 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -1528,6 +1528,28 @@ byte.
+.. _etype_nxp_imx8mcst:
+
+Entry: nxp-imx8mcst: NXP i.MX8M CST .cfg file generator and cst invoker
+---
+
+Properties / Entry arguments:
+- nxp,loader-address - loader address (SPL text base)
+
+
+
+.. _etype_nxp_imx8mimage:
+
+Entry: nxp-imx8mimage: NXP i.MX8M imx8mimage .cfg file generator and mkimage
invoker
+
+
+Properties / Entry arguments:
+- nxp,boot-from - device to boot from (e.g. 'sd')
+- nxp,loader-address - loader address (SPL text base)
+- nxp,rom-version - BootROM version ('2' for i.MX8M Nano and Plus)
+
+
+
.. _etype_opensbi:
Entry: opensbi: RISC-V OpenSBI fw_dynamic blob
--
2.34.1
[PATCH v2 01/43] trace: Update test to tolerate different trace-cmd version
Some versions of trace-cmd (or some machines?) show one less dot in the
CPU list.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/py/tests/test_trace.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py
index 7c5696ce747..f41d4cf71f0 100644
--- a/test/py/tests/test_trace.py
+++ b/test/py/tests/test_trace.py
@@ -12,7 +12,7 @@ import u_boot_utils as util
TMPDIR = '/tmp/test_trace'
# Decode a function-graph line
-RE_LINE = re.compile(r'.*0\.\.\.\.\. \s*([0-9.]*):
func.*[|](\s*)(\S.*)?([{};])$')
+RE_LINE = re.compile(r'.*0\.\.\.\.\.? \s*([0-9.]*):
func.*[|](\s*)(\S.*)?([{};])$')
def collect_trace(cons):
--
2.34.1
[PATCH v2 00/43] labgrid: Provide an integration with Labgrid
Labgrid provides access to a hardware lab in an automated way. It is possible to boot U-Boot on boards in the lab without physically touching them. It relies on relays, USB UARTs and SD muxes, among other things. By way of background, about 4 years ago I wrong a thing called Labman[1] which allowed my lab of about 30 devices to be operated remotely, using tbot for the console and build integration. While it worked OK and I used it for many bisects, I didn't take it any further. It turns out that there was already an existing program, called Labgrid, which I did not know about at time (thank you Tom for telling me). It is more rounded than Labman and has a number of advantages: - does not need udev rules, mostly - has several existing users who rely on it - supports multiple machines exporting their devices It lacks a 'lab check' feature and a few other things, but these can be remedied. On and off over the past several weeks I have been experimenting with Labgrid. I have managed to create an initial U-Boot integration (this series) by adding various features to Labgrid[2] and the U-Boot test hooks. I hope that this might inspire others to set up boards and run tests automatically, rather than relying on infrequent, manual test. Perhaps it may even be possible to have a number of labs available. Included in the integration are a number of simple scripts which make it easy to connect to boards and run tests: ub-int Build and boot on a target, starting an interactive session ub-cli Build and boot on a target, ensure U-Boot starts and provide an interactive session from there ub-smoke Smoke test U-Boot to check that it boots to a prompt on a target ub-bisect Bisect a git tree to locate a failure on a particular target ub-pyt Run U-Boot pytests on a target Some of these help to provide the same tbot[4] workflow which I have relied on for several years, albeit much simpler versions. The goal here is to create some sort of script which can collect patches from the mailing list, apply them and test them on a selection of boards. I suspect that script already exists, so please let me know what you suggest. I hope you find this interesting and take a look! [1] https://github.com/sjg20/u-boot/tree/lab6a [2] https://github.com/labgrid-project/labgrid/pull/1411 [3] https://github.com/sjg20/uboot-test-hooks/tree/labgrid [4] https://tbot.tools/index.html Changes in v2: - Only disable echo if a terminal is detected - Add new patch to update u-boot.cfg with CFG_... options - Avoid running a docker image for skipped lab tests Simon Glass (43): trace: Update test to tolerate different trace-cmd version binman: efi: Correct entry docs binman: Regenerate nxp docs binman: ti: Regenerate entry docs binman: Update the entrydocs header buildman: Make mrproper an argument to _reconfigure() buildman: Make mrproper an argument to _config_and_build() buildman: Make mrproper an argument to run_commit() buildman: Avoid rebuilding when --mrproper is used buildman: Add a flag to force mrproper on failure buildman: Retry the build for current source buildman: Add a way to limit the number of buildmans dm: core: Enhance comments on bind_drivers_pass() initcall: Correct use of relocation offset am33xx: Provide a function to set up the debug UART sunxi: Mark scp as optional google: Disable TPMv2 on most Chromebooks meson: Correct driver declaration for meson_axg_gpio test: Allow signaling that U-Boot is ready test: Make bootstd init run only on sandbox test: Use a constant for the test timeout test: Pass stderr to stdout test: Release board after tests complete log: Allow tests to pass with CONFIG_LOGF_FUNC_PAD set test: Allow connecting to a running board test: Decode exceptions only with sandbox test: Avoid failing skipped tests test: dm: Show failing driver name test: Check help output test: Create a common function to get the config test: Introduce the concept of a role test: Move the receive code into a function test: Separate out the exception handling test: Detect dead connections test: Tidy up remaining exceptions test: Introduce lab mode test: Improve handling of sending commands test: Fix mulptiplex_log typo test: Avoid double echo when starting up test: Try to shut down the lab console gracefully test: Add a section for closing the connection Update u-boot.cfg to include CFG also CI: Allow running tests on sjg lab .gitlab-ci.yml| 153 + arch/arm/dts/sunxi-u-boot.dtsi| 1 + arch/arm/mach-omap2/am33xx/board.c| 18 +- configs/chromebook_link64_defconfig | 1 + configs/chromebook_link_defconfig | 1 + configs/chromebook_samus_defconfig| 1 + configs/chromebook_samus_tpl_defconfig| 1 + configs/nyan-big_defconfig| 4 +- configs/snow_defconfig
Re: [PATCH 3/5] buildman: Support building within a Python venv
Hi Tom,
On Fri, 21 Jun 2024 at 13:26, Tom Rini wrote:
>
> On Fri, Jun 21, 2024 at 12:19:12PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 21 Jun 2024 at 09:22, Tom Rini wrote:
> > >
> > > On Fri, Jun 21, 2024 at 08:57:50AM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 20 Jun 2024 at 17:30, Tom Rini wrote:
> > > > >
> > > > > On Thu, Jun 20, 2024 at 05:05:30PM -0600, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Thu, 20 Jun 2024 at 08:32, Tom Rini wrote:
> > > > > > >
> > > > > > > On Thu, Jun 20, 2024 at 07:19:35AM -0600, Simon Glass wrote:
> > > > > > >
> > > > > > > > The Python virtualenv tool sets up a few things in the
> > > > > > > > envronment,
> > > > > > > > putting its path first in the PATH environment variable and
> > > > > > > > setting up
> > > > > > > > a sys.prefix different from the sys.base_prefix value.
> > > > > > > >
> > > > > > > > At present buildman puts the toolchain path first in PATH so
> > > > > > > > that it can
> > > > > > > > be found easily during the build. For sandbox this causes
> > > > > > > > problems since
> > > > > > > > /usr/bin/gcc (for example) results in '/usr/bin' being
> > > > > > > > prepended to the
> > > > > > > > PATH variable. As a result, the venv is partially disabled.
> > > > > > > >
> > > > > > > > The result is that sandbox builds within a venv ignore the
> > > > > > > > venv, e.g.
> > > > > > > > when looking for packages.
> > > > > > > >
> > > > > > > > Correct this by detecting the venv and adding the toolchain
> > > > > > > > path after
> > > > > > > > the venv path.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass
> > > > > > >
> > > > > > > Why are we using PATH at all in this case? Shouldn't we just be
> > > > > > > setting
> > > > > > > CROSS_COMPILE=/full/path/to/the/prefix ?
> > > > > >
> > > > > > This is the -p option to buildman. The original commit was:
> > > > > >
> > > > > > commit bb1501f2c22c979961b735db775605cccedd98f6
> > > > > > Author: Simon Glass
> > > > > > Date: Mon Dec 1 17:34:00 2014 -0700
> > > > > >
> > > > > > buildman: Add an option to use the full tool chain path
> > > > > >
> > > > > > In some cases there may be multiple toolchains with the same
> > > > > > name in the
> > > > > > path. Provide an option to use the full path in the
> > > > > > CROSS_COMPILE
> > > > > > environment variable.
> > > > > >
> > > > > > Note: Wolfgang mentioned that this is dangerous since in some
> > > > > > cases there
> > > > > > may be other tools on the path that are needed. So this is set
> > > > > > up as an
> > > > > > option, not the default. I will need test confirmation (i.e.
> > > > > > that this
> > > > > > commit fixes a real problem) before merging it.
> > > > > >
> > > > > > As to why we don't always do this, well that is back in the mists of
> > > > > > time, 10 years ago.
> > > > > >
> > > > > > BTW, this is raising a point ("let's change the behaviour") separate
> > > > > > from the goal of this commit, which is to fix a problem with venv,
> > > > > > albeit that if we made -p the only option, then we could potentially
> > > > > > drop all PATH changes. Perhaps toolchains are built differently now,
> > > > > > such that they always invoke their tools using the same prefix and
> > > > > > dir?
> > > > >
> > > > > Wait, I'm confused. buildman internally updates its own PATH to avoid
> > > > > calling CROSS_COMPILE with the full path due to a concern about
> > > > > toolchain bugs?
> > > >
> > > > Not its own PATH: the one it passes to U-Boot's 'make'.
> > >
> > > OK, but the point stands.
> > >
> > > > I'm not sure why, actually. It is such a long time ago that I don't
> > > > remember.
> > > >
> > > > I see:
> > > >
> > > > ~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-ld
> > >
> > > Yes, prefixed version that's allowed to be called by users.
> > >
> > > > and
> > > >
> > > > ~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/arm-linux-gnueabi/bin/ld
> > >
> > > Internal usage, here be dragons and all that.
> > >
> > > > but interestingly there is no gcc in the latter directory, which there
> > > > was in 4.6 (and presumably for some time after).
> > > >
> > > > Certainly for sandbox there is no prefix, so we cannot add it in that
> > > > case, and sandbox is actually the arch used to run these tests.
> > >
> > > CROSS_COMPILE is empty for sandbox, yes.
> > >
> > > > What are you suggesting we change about this patch?
> > >
> > > That it's going about things backwards? If you're setting CROSS_COMPILE
> > > _then_ it should be the full path that it already knows otherwise if not
> > > setting CROSS_COMPILE then also not modifying PATH.
> >
> > That is what the code does, yes. It either adds the toolchain path to
> > CROSS_COMPILE or to PATH, not both. The 'full_path' argument controls
> > which one it uses.
>
> I'm saying it should always use the full path to
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Tom, On Fri, 21 Jun 2024 at 13:19, Tom Rini wrote: > > On Fri, Jun 21, 2024 at 11:55:42AM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Fri, 21 Jun 2024 at 10:05, Tom Rini wrote: > [snip] > > > Yes, I very much do not like guessing about 3 numbers instead of > > > guessing about 1 number and using the standard mechanism we already > > > have. Please use BOARD_SIZE_LIMIT as this is the standard mechanism to > > > enforce size limits on U-Boot itself. > > > > If it were that easy I would have sent a patch :-) > > > > Here is the map for this board: > > > > ImagePosOffset Size Name > > 0080 rom > > ff80 ff80 1000 intel-descriptor > > ff801000 ff801000 001ff000 intel-me > > ffef ffef 000999f0 u-boot-with-ucode-ptr > > fff899f0 fff899f0 5554 u-boot-dtb-with-ucode > > fff8ef50 fff8ef50 u-boot-ucode > > fff8ef50 fff8ef50 0571 fdtmap > > fff9 fff9 0001 intel-vga > > fffa fffa 0002fc94 intel-mrc > > fffcfc94 fffcfc94 private-files > > f800 f800 0070 x86-start16 > > fff0 fff0 0005 x86-reset16 > > fff8 fff8 0008 image-header > > > > What limit should I set on what? > > Is this a trick question? > $ printf %d\\n $(( 0xfff9 - 0xffef)) > 655360 > > Of course since we're less than that today, you can reduce it by > whatever other magic numbers I'm not seeing but are part of your assumed > sizes. That limit is on u-boot-nodtb.bin. Even with a size (for that file) of 634816 it doesn't fit. I need to calculate a size based on the size of the dtb and the microcode...which of course can change. > > > - the U-Boot is the thing you are wanting to limit > > - the dtb has microcode added > > - the ucode is empty in this case > > - the fdtmap is variable in size > > > > So this all seems a bit backwards. The actual limit is that > > (u-boot-with-ucode-ptr + u-boot-dtb-with-ucode + u-boot-ucode + > > fdtmap) fits in the space available. Note that some boards don't have > > intel-vga or intel-mrc. > > > > With the other patch I sent I can have a sensible limit for all x86 boards. Did you miss the comments above? > > And you can set the same sensible limit with the existing mechanism with > the bonus of it not making x86 different from the rest? I understand that it is possible to set a limit for u-boot-nodtb.bin but that is not accurate nor sufficient in the presence of blobs. The solution belongs in Binman. Regards, Simon
Re: [PATCH 3/5] buildman: Support building within a Python venv
On Fri, Jun 21, 2024 at 12:19:12PM -0600, Simon Glass wrote:
> Hi Tom,
>
> On Fri, 21 Jun 2024 at 09:22, Tom Rini wrote:
> >
> > On Fri, Jun 21, 2024 at 08:57:50AM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 20 Jun 2024 at 17:30, Tom Rini wrote:
> > > >
> > > > On Thu, Jun 20, 2024 at 05:05:30PM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Thu, 20 Jun 2024 at 08:32, Tom Rini wrote:
> > > > > >
> > > > > > On Thu, Jun 20, 2024 at 07:19:35AM -0600, Simon Glass wrote:
> > > > > >
> > > > > > > The Python virtualenv tool sets up a few things in the envronment,
> > > > > > > putting its path first in the PATH environment variable and
> > > > > > > setting up
> > > > > > > a sys.prefix different from the sys.base_prefix value.
> > > > > > >
> > > > > > > At present buildman puts the toolchain path first in PATH so that
> > > > > > > it can
> > > > > > > be found easily during the build. For sandbox this causes
> > > > > > > problems since
> > > > > > > /usr/bin/gcc (for example) results in '/usr/bin' being prepended
> > > > > > > to the
> > > > > > > PATH variable. As a result, the venv is partially disabled.
> > > > > > >
> > > > > > > The result is that sandbox builds within a venv ignore the venv,
> > > > > > > e.g.
> > > > > > > when looking for packages.
> > > > > > >
> > > > > > > Correct this by detecting the venv and adding the toolchain path
> > > > > > > after
> > > > > > > the venv path.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass
> > > > > >
> > > > > > Why are we using PATH at all in this case? Shouldn't we just be
> > > > > > setting
> > > > > > CROSS_COMPILE=/full/path/to/the/prefix ?
> > > > >
> > > > > This is the -p option to buildman. The original commit was:
> > > > >
> > > > > commit bb1501f2c22c979961b735db775605cccedd98f6
> > > > > Author: Simon Glass
> > > > > Date: Mon Dec 1 17:34:00 2014 -0700
> > > > >
> > > > > buildman: Add an option to use the full tool chain path
> > > > >
> > > > > In some cases there may be multiple toolchains with the same name
> > > > > in the
> > > > > path. Provide an option to use the full path in the CROSS_COMPILE
> > > > > environment variable.
> > > > >
> > > > > Note: Wolfgang mentioned that this is dangerous since in some
> > > > > cases there
> > > > > may be other tools on the path that are needed. So this is set up
> > > > > as an
> > > > > option, not the default. I will need test confirmation (i.e. that
> > > > > this
> > > > > commit fixes a real problem) before merging it.
> > > > >
> > > > > As to why we don't always do this, well that is back in the mists of
> > > > > time, 10 years ago.
> > > > >
> > > > > BTW, this is raising a point ("let's change the behaviour") separate
> > > > > from the goal of this commit, which is to fix a problem with venv,
> > > > > albeit that if we made -p the only option, then we could potentially
> > > > > drop all PATH changes. Perhaps toolchains are built differently now,
> > > > > such that they always invoke their tools using the same prefix and
> > > > > dir?
> > > >
> > > > Wait, I'm confused. buildman internally updates its own PATH to avoid
> > > > calling CROSS_COMPILE with the full path due to a concern about
> > > > toolchain bugs?
> > >
> > > Not its own PATH: the one it passes to U-Boot's 'make'.
> >
> > OK, but the point stands.
> >
> > > I'm not sure why, actually. It is such a long time ago that I don't
> > > remember.
> > >
> > > I see:
> > >
> > > ~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-ld
> >
> > Yes, prefixed version that's allowed to be called by users.
> >
> > > and
> > >
> > > ~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/arm-linux-gnueabi/bin/ld
> >
> > Internal usage, here be dragons and all that.
> >
> > > but interestingly there is no gcc in the latter directory, which there
> > > was in 4.6 (and presumably for some time after).
> > >
> > > Certainly for sandbox there is no prefix, so we cannot add it in that
> > > case, and sandbox is actually the arch used to run these tests.
> >
> > CROSS_COMPILE is empty for sandbox, yes.
> >
> > > What are you suggesting we change about this patch?
> >
> > That it's going about things backwards? If you're setting CROSS_COMPILE
> > _then_ it should be the full path that it already knows otherwise if not
> > setting CROSS_COMPILE then also not modifying PATH.
>
> That is what the code does, yes. It either adds the toolchain path to
> CROSS_COMPILE or to PATH, not both. The 'full_path' argument controls
> which one it uses.
I'm saying it should always use the full path to CROSS_COMPILE and never
modify PATH.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Fri, Jun 21, 2024 at 11:55:42AM -0600, Simon Glass wrote: > Hi Tom, > > On Fri, 21 Jun 2024 at 10:05, Tom Rini wrote: [snip] > > Yes, I very much do not like guessing about 3 numbers instead of > > guessing about 1 number and using the standard mechanism we already > > have. Please use BOARD_SIZE_LIMIT as this is the standard mechanism to > > enforce size limits on U-Boot itself. > > If it were that easy I would have sent a patch :-) > > Here is the map for this board: > > ImagePosOffset Size Name > 0080 rom > ff80 ff80 1000 intel-descriptor > ff801000 ff801000 001ff000 intel-me > ffef ffef 000999f0 u-boot-with-ucode-ptr > fff899f0 fff899f0 5554 u-boot-dtb-with-ucode > fff8ef50 fff8ef50 u-boot-ucode > fff8ef50 fff8ef50 0571 fdtmap > fff9 fff9 0001 intel-vga > fffa fffa 0002fc94 intel-mrc > fffcfc94 fffcfc94 private-files > f800 f800 0070 x86-start16 > fff0 fff0 0005 x86-reset16 > fff8 fff8 0008 image-header > > What limit should I set on what? Is this a trick question? $ printf %d\\n $(( 0xfff9 - 0xffef)) 655360 Of course since we're less than that today, you can reduce it by whatever other magic numbers I'm not seeing but are part of your assumed sizes. > - the U-Boot is the thing you are wanting to limit > - the dtb has microcode added > - the ucode is empty in this case > - the fdtmap is variable in size > > So this all seems a bit backwards. The actual limit is that > (u-boot-with-ucode-ptr + u-boot-dtb-with-ucode + u-boot-ucode + > fdtmap) fits in the space available. Note that some boards don't have > intel-vga or intel-mrc. > > With the other patch I sent I can have a sensible limit for all x86 boards. And you can set the same sensible limit with the existing mechanism with the bonus of it not making x86 different from the rest? -- Tom signature.asc Description: PGP signature
Re: [PATCH v4 00/14] Introduce the lwIP network stack
Hi Tim and Jerome, On Fri, Jun 21, 2024 at 1:08 PM Tim Harvey wrote: > I tried your to-upstream/v5-wip branch > (042bea36eb9731079a3d7afffe3774d79e06ac5d) and it behaves the same. Do > you have something else to try/test? Yes, when I tested older versions from Maxim I could never get lwIP to work with i.MX. Jerome, Please try to run the lwIP series on any i.MX board, if possible. Thanks
Re: [PATCH 3/5] buildman: Support building within a Python venv
Hi Tom,
On Fri, 21 Jun 2024 at 09:22, Tom Rini wrote:
>
> On Fri, Jun 21, 2024 at 08:57:50AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 20 Jun 2024 at 17:30, Tom Rini wrote:
> > >
> > > On Thu, Jun 20, 2024 at 05:05:30PM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 20 Jun 2024 at 08:32, Tom Rini wrote:
> > > > >
> > > > > On Thu, Jun 20, 2024 at 07:19:35AM -0600, Simon Glass wrote:
> > > > >
> > > > > > The Python virtualenv tool sets up a few things in the envronment,
> > > > > > putting its path first in the PATH environment variable and setting
> > > > > > up
> > > > > > a sys.prefix different from the sys.base_prefix value.
> > > > > >
> > > > > > At present buildman puts the toolchain path first in PATH so that
> > > > > > it can
> > > > > > be found easily during the build. For sandbox this causes problems
> > > > > > since
> > > > > > /usr/bin/gcc (for example) results in '/usr/bin' being prepended to
> > > > > > the
> > > > > > PATH variable. As a result, the venv is partially disabled.
> > > > > >
> > > > > > The result is that sandbox builds within a venv ignore the venv,
> > > > > > e.g.
> > > > > > when looking for packages.
> > > > > >
> > > > > > Correct this by detecting the venv and adding the toolchain path
> > > > > > after
> > > > > > the venv path.
> > > > > >
> > > > > > Signed-off-by: Simon Glass
> > > > >
> > > > > Why are we using PATH at all in this case? Shouldn't we just be
> > > > > setting
> > > > > CROSS_COMPILE=/full/path/to/the/prefix ?
> > > >
> > > > This is the -p option to buildman. The original commit was:
> > > >
> > > > commit bb1501f2c22c979961b735db775605cccedd98f6
> > > > Author: Simon Glass
> > > > Date: Mon Dec 1 17:34:00 2014 -0700
> > > >
> > > > buildman: Add an option to use the full tool chain path
> > > >
> > > > In some cases there may be multiple toolchains with the same name
> > > > in the
> > > > path. Provide an option to use the full path in the CROSS_COMPILE
> > > > environment variable.
> > > >
> > > > Note: Wolfgang mentioned that this is dangerous since in some cases
> > > > there
> > > > may be other tools on the path that are needed. So this is set up
> > > > as an
> > > > option, not the default. I will need test confirmation (i.e. that
> > > > this
> > > > commit fixes a real problem) before merging it.
> > > >
> > > > As to why we don't always do this, well that is back in the mists of
> > > > time, 10 years ago.
> > > >
> > > > BTW, this is raising a point ("let's change the behaviour") separate
> > > > from the goal of this commit, which is to fix a problem with venv,
> > > > albeit that if we made -p the only option, then we could potentially
> > > > drop all PATH changes. Perhaps toolchains are built differently now,
> > > > such that they always invoke their tools using the same prefix and
> > > > dir?
> > >
> > > Wait, I'm confused. buildman internally updates its own PATH to avoid
> > > calling CROSS_COMPILE with the full path due to a concern about
> > > toolchain bugs?
> >
> > Not its own PATH: the one it passes to U-Boot's 'make'.
>
> OK, but the point stands.
>
> > I'm not sure why, actually. It is such a long time ago that I don't
> > remember.
> >
> > I see:
> >
> > ~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-ld
>
> Yes, prefixed version that's allowed to be called by users.
>
> > and
> >
> > ~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/arm-linux-gnueabi/bin/ld
>
> Internal usage, here be dragons and all that.
>
> > but interestingly there is no gcc in the latter directory, which there
> > was in 4.6 (and presumably for some time after).
> >
> > Certainly for sandbox there is no prefix, so we cannot add it in that
> > case, and sandbox is actually the arch used to run these tests.
>
> CROSS_COMPILE is empty for sandbox, yes.
>
> > What are you suggesting we change about this patch?
>
> That it's going about things backwards? If you're setting CROSS_COMPILE
> _then_ it should be the full path that it already knows otherwise if not
> setting CROSS_COMPILE then also not modifying PATH.
That is what the code does, yes. It either adds the toolchain path to
CROSS_COMPILE or to PATH, not both. The 'full_path' argument controls
which one it uses.
Regards,
Simon
Re: [PATCH 5/9] fdt: Correct condition for bloblist existing
Hi Raymond,
On Fri, 21 Jun 2024 at 10:40, Raymond Mao wrote:
>
> Hi Simon,
>
> On Fri, 21 Jun 2024 at 10:58, Simon Glass wrote:
>>
>> Hi Ilias,
>>
>> On Wed, 19 Jun 2024 at 06:41, Ilias Apalodimas
>> wrote:
>> >
>> > Hi Simon,
>> >
>> > On Wed, Jun 12, 2024 at 02:24:31PM -0600, Simon Glass wrote:
>> > > Hi Ilias,
>> > >
>> > > On Wed, 12 Jun 2024 at 00:02, Ilias Apalodimas
>> > > wrote:
>> > > >
>> > > > [...]
>> > > >
>> > > > > > > >> ---
>> > > > > > > >>
>> > > > > > > >> lib/fdtdec.c | 12 ++--
>> > > > > > > >> 1 file changed, 10 insertions(+), 2 deletions(-)
>> > > > > > > >>
>> > > > > > > >> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>> > > > > > > >> index b2c59ab3818..b141244e3b9 100644
>> > > > > > > >> --- a/lib/fdtdec.c
>> > > > > > > >> +++ b/lib/fdtdec.c
>> > > > > > > >> @@ -1669,8 +1669,16 @@ int fdtdec_setup(void)
>> > > > > > > >> {
>> > > > > > > >> int ret = -ENOENT;
>> > > > > > > >>
>> > > > > > > >> - /* If allowing a bloblist, check that first */
>> > > > > > > >> - if (CONFIG_IS_ENABLED(BLOBLIST)) {
>> > > > > > > >> + /*
>> > > > > > > >> +* If allowing a bloblist, check that first. This
>> > > > > > > >> would be better
>> > > > > > > >> +* handled with an OF_BLOBLIST Kconfig, but that
>> > > > > > > >> caused far too much
>> > > > > > > >> +* argument, so add a hack here, used e.g. by
>> > > > > > > >> chromebook_coral
>> > > > > > > >
>> > > > > > > > I am a bit confused by this comment - It means you will not
>> > > > > > > > use OF_BLOBLIST,
>> > > > > > > > but actually you are using it below. Is it a typo?
>> > > > > > >
>> > > > > > > Basically it would be cleaner to have a separate, phase-specific
>> > > > > > > Kconfig control as to whether the DT can come from the bloblist
>> > > > > > > (I
>> > > > > > > can't remember the Kconfig name I suggested, nor the patch as it
>> > > > > > > was
>> > > > > > > last year sometime). But for now I am adding this hack to get a
>> > > > > > > few
>> > > > > > > boards working again.
>> > > > > >
>> > > > > > I am a bit confused.
>> > > > > > First of all the comment is innapropriate. We went through a
>> > > > > > lengthy
>> > > > > > discussion on BLOBLIST/OF_BLOSLIST etc and, even Tom chimed in
>> > > > > > and we
>> > > > > > made up our minds. Why are you adding this comment now? Why do code
>> > > > > > comments have to illustrate your personal opinion -- which was
>> > > > > > rejected?
>> > > > >
>> > > > > I'm sorry for the tone of the comment. I am not trying to offend
>> > > > > anyone here and I'm happy to change the language.
>> > > >
>> > > > Yes please a comment explaining why that piece of code is there is far
>> > > > more intuitive.
>> > >
>> > > OK, once we have agreed the below I can do that.
>> > >
>> > > >
>> > > > > As I probably
>> > > > > mentioned at the time, my accepted patch breaks my workflow and
>> > > > > several boards. I hope you can understand how frustrating that sort
>> > > > > of
>> > > > > thing can be.
>> > > >
>> > > > Yes, I do and I am fine with a short-term patch that fixes issues, as
>> > > > long as its not too intrusive. There is a very thin line between quick
>> > > > and dirty fixes to spaghetti unreadable code. But we should have
>> > > > comments and/or commit messages indicating that this needs a proper
>> > > > fix
>> > >
>> > > I spent a lot of time explaining this last time.
>> > >
>> > > >
>> > > > > Also, now that I have my lab back up and running and I
>> > > > > would like these boards to work again on mainline!
>> > > > >
>> > > > > >
>> > > > > > Grepping for OF_BLOBLIST, I can't find any matches, so is the
>> > > > > > above if a typo?
>> > > > >
>> > > > > Remember, it was a patch you rejected :-)
>> > > >
>> > > > I don't maintain any of that. I only gave some feedback along the
>> > > > lines of "bloblist was designed to be auto-discoverable, I don't see
>> > > > how adding an explicit Kconfig helps". IIRC we eventually followed
>> > > > what Tom suggested.
>> > >
>> > > I'm not trying to point the finger here. So far the boards are broken
>> > > in mainline...I'm just trying to fix that,
>> > >
>> > > >
>> > > > In any case, the amount of bike-shedding in the topic is too much. Do
>> > > > you mind explaining the problem in your workflow again? Perhaps we can
>> > > > find a solution that is integrated in bloblist_maybe_init() instead of
>> > > > injecting ifs on when a bloblist should or should not be searched for
>> > > > all over the place.
>> > >
>> > > TPL (or SPL) sets up a bloblist with bits of info in it, but no DT
>> > > (which is in memory-mapped SPI flash)
>> > > U-Boot proper starts up, it wants to get the bloblist but not hang if
>> > > the bloblist doesn't have a DT
>> >
>> > Sure, that's reasonable and IIRC that's the design we agreed a while back.
>> > Looking at the code, if the DT isn't found in a bloblist and the feature is
>> > enabled, we don't crash. We just print a
Re: [PATCH v4 00/14] Introduce the lwIP network stack
On 6/21/24 18:08, Tim Harvey wrote: > On Fri, Jun 21, 2024 at 5:59 AM Jerome Forissier > wrote: >> >> >> >> On 6/20/24 19:10, Tim Harvey wrote: >>> On Mon, Jun 17, 2024 at 8:33 AM Jerome Forissier >>> wrote: This is a rework of a patch series by Maxim Uvarov: "net/lwip: add lwip library for the network stack" [1]. The goal is to introduce the lwIP TCP/IP stack [2] [3] as an alternative to the current implementation in net/, selectable with Kconfig, and ultimately keep only lwIP if possible. Some reasons for doing so are: - Make the support of HTTPS in the wget command easier. Javier T. (CC'd) has some additional lwIP and Mbed TLS patches to do so. With that it becomes possible to fetch and launch a distro installer such as Debian etc. using a secure, authenticated connection directly from the U-Boot shell. Several use cases: * Authentication: prevent MITM attack (third party replacing the binary with a different one) * Confidentiality: prevent third parties from grabbing a copy of the image as it is being downloaded * Allow connection to servers that do not support plain HTTP anymore (this is becoming more and more common on the Internet these days) - Possibly benefit from additional features implemented in lwIP - Less code to maintain in U-Boot Prior to applying this series, the lwIP stack needs to be added as a Git subtree with the following command: $ git subtree add --squash --prefix lib/lwip/lwip https://git.savannah.gnu.org/git/lwip.git STABLE-2_2_0_RELEASE The first patch renames some enums in order to avoid a conflict when a later patch enables the lwIP library. The second patch introduces a new Kconfig symbol: NET_LWIP, which selects the lwIP implementation instead of the current one (NET). Contrary to the approach chosen by Maxim in [1], NET_LWIP and NET cannot be enabled simultaneously. The rationale is we want to start from a clean state and not pull potentially duplicated functionality from both stacks. Note however that a few files are still built in net/, they are the ones related to ethernet device management and the ethernet bootflow. The third patch splits the net.h header into net-legacy.h, net-common.h, net-lwip.h, leaving net.h as a simple wrapper. The fourth patch introduces the Makefile to build lwIP when NET_LWIP is enabled. The subsequent patches implement various network-oriented commands and features: dhcp, dns, ping, tftpboot, wget. A number of features are currently incompatible with NET_LWIP: SANDBOX, DFU_TFTP, FASTBOOT, SPL_NET. All make assumptions on how the network stack is implemented and/or pull sybols that are not trivially exported from lwIP. Some interface rework may be needed. Due to the above and in order to provide some level of testing, a new QEMU configuration is introduced (qemu_arm64_lwip_defconfig) which is the same as qemu_arm64_defconfig but with NET_LWIP and CMD_*_LWIP enabled. Tests are added to test/py/tests/test_net.py for that configuration. >>> >>> Hi Jermone, >>> >>> I've given this a spin on an imx8mm-venice-gw73xx-0x via >>> imx8mm_venice_defconfig and I can't get DHCP to work (didn't work in >>> v3 either): >>> >>> $ diff defconfig configs/imx8mm_venice_defconfig >>> 68,69c68,71 >>> < CONFIG_CMD_DNS_LWIP=y >>> < CONFIG_CMD_WGET_LWIP=y >>> --- CONFIG_CMD_DHCP6=y CONFIG_CMD_TFTPPUT=y CONFIG_SYS_DISABLE_AUTOLOAD=y CONFIG_CMD_WGET=y >>> 88,90c90,94 >>> < CONFIG_NET_LWIP=y >>> < CONFIG_LWIP_DEBUG=y >>> < CONFIG_LWIP_ASSERT=y >>> --- CONFIG_NET_RANDOM_ETHADDR=y CONFIG_IP_DEFRAG=y CONFIG_TFTP_BLOCKSIZE=4096 CONFIG_PROT_TCP_SACK=y CONFIG_IPV6=y >>> >>> Target: >>> u-boot=> net list >>> eth0 : ethernet@30be 00:d0:12:b5:f8:41 active >>> u-boot=> dhcp || echo fail >>> eth0: ethernet@30be 00:d0:12:b5:f8:41 active >>> netif_set_ipaddr: netif address being changed >>> netif: added interface 0 addr 192.168.1.1 netmask 0.0.0.0 gw 0.0.0.0 >>> etharp_request: sending ARP request. >>> etharp_raw: sending raw ARP packet. >>> ethernet_output: sending packet 7df00bf0 >>> dhcp_start(netif=7df2af40) 0dhcp_start(): mallocing new DHCP client >>> dhcp_start(): allocated dhcp >>> dhcp_start(): starting DHCP configuration >>> dhcp_discover() >>> transaction id xid(42021) >>> dhcp_discover: making request >>> dhcp_discover: sendto(DISCOVER, IP_ADDR_BROADCAST, >>> LWIP_IANA_PORT_DHCP_SERVER) >>> ip4_output_if: 0IP header: >>> +---+ >>> | 4 | 5 | 0x00 | 336 | (v, hl, tos, len) >>> +---+ >>> |0 |000| 0 | (id, flags, offset) >>> +---+ >>> | 255 | 17 |0xba9d | (ttl, proto, chksum) >>>
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Tom, On Fri, 21 Jun 2024 at 10:05, Tom Rini wrote: > > On Fri, Jun 21, 2024 at 08:57:51AM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Thu, 20 Jun 2024 at 17:19, Tom Rini wrote: > > > > > > On Thu, Jun 20, 2024 at 05:05:29PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Wed, 19 Jun 2024 at 09:32, Tom Rini wrote: > > > > > > > > > > On Tue, Jun 18, 2024 at 09:03:37PM -0600, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Tue, 18 Jun 2024 at 08:15, Tom Rini wrote: > > > > > > > > > > > > > > On Tue, Jun 18, 2024 at 06:43:51AM -0600, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Mon, 17 Jun 2024 at 11:16, Tom Rini > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Heinrich > > > > > > > > > > > > > > > > > > > > > > resending the reply, I accidentally sent half of the > > > > > > > > > > > message... > > > > > > > > > > > > > > > > > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > > > > > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > > > > > > > > > wrote: > > > > > > > > > > > > >> > > > > > > > > > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > > > > > > > > > >>> Hi Simon, > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass > > > > > > > > > > > > >>> wrote: > > > > > > > > > > > > > > > > > > > > > > > > It does not make sense to enable all SHA > > > > > > > > > > > > algorithms unless they are > > > > > > > > > > > > needed. It bloats the code and in this case, > > > > > > > > > > > > causes chromebook_link to > > > > > > > > > > > > fail to build. That board does use the TPM, but > > > > > > > > > > > > not with measured boot, > > > > > > > > > > > > nor EFI. > > > > > > > > > > > > > > > > > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these > > > > > > > > > > > > options, we just need to > > > > > > > > > > > > add them to MEASURED_BOOT as well. > > > > > > > > > > > > > > > > > > > > > > > > Note that the original commit combines refactoring > > > > > > > > > > > > and new features, > > > > > > > > > > > > which makes it hard to see what is going on. > > > > > > > > > > > > > > > > > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > > > > > > > > > - Consider MEASURED_BOOT too > > > > > > > > > > > > > > > > > > > > > > > > boot/Kconfig | 4 > > > > > > > > > > > > lib/Kconfig | 4 > > > > > > > > > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > > > > > > > > index 6f3096c15a6..b061891e109 100644 > > > > > > > > > > > > --- a/boot/Kconfig > > > > > > > > > > > > +++ b/boot/Kconfig > > > > > > > > > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > > > > > > > > > config MEASURED_BOOT > > > > > > > > > > > > bool "Measure boot images and > > > > > > > > > > > > configuration when booting without EFI" > > > > > > > > > > > > depends on HASH && TPM_V2 > > > > > > > > > > > > + select SHA1 > > > > > > > > > > > > + select SHA256 > > > > > > > > > > > > + select SHA384 > > > > > > > > > > > > + select SHA512 > > > > > > > > > > > > help > > > > > > > > > > > > This option enables measurement of the > > > > > > > > > > > > boot process when booting > > > > > > > > > > > > without UEFI . Measurement involves > > > > > > > > > > > > creating cryptographic hashes > > > > > > > > > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > > > > > > > > > index 189e6eb31aa..568892fce44 100644 > > > > > > > > > > > > --- a/lib/Kconfig > > > > > > > > > > > > +++ b/lib/Kconfig > > > > > > > > > > > > @@ -438,10 +438,6 @@ config TPM > > > > > > > > > > > > bool "Trusted Platform Module (TPM) > > > > > > > > > > > > Support" > > > > > > > > > > > > depends on DM > > > > > > > > > > > > imply DM_RNG > > > > > > > > > > > > - select SHA1 > > > > > > > > > > > > - select SHA256 > >
Re: [PATCH v4 00/14] Introduce the lwIP network stack
Hi Simon,
On 6/21/24 16:57, Simon Glass wrote:
> Hi Jerome,
>
> On Mon, 17 Jun 2024 at 09:33, Jerome Forissier
> wrote:
>>
>> This is a rework of a patch series by Maxim Uvarov: "net/lwip: add lwip
>> library for the network stack" [1]. The goal is to introduce the lwIP TCP/IP
>> stack [2] [3] as an alternative to the current implementation in net/,
>> selectable with Kconfig, and ultimately keep only lwIP if possible. Some
>> reasons for doing so are:
>> - Make the support of HTTPS in the wget command easier. Javier T. (CC'd)
>> has some additional lwIP and Mbed TLS patches to do so. With that it
>> becomes possible to fetch and launch a distro installer such as Debian
>> etc. using a secure, authenticated connection directly from the U-Boot
>> shell. Several use cases:
>> * Authentication: prevent MITM attack (third party replacing the
>> binary with a different one)
>> * Confidentiality: prevent third parties from grabbing a copy of the
>> image as it is being downloaded
>> * Allow connection to servers that do not support plain HTTP anymore
>> (this is becoming more and more common on the Internet these days)
>> - Possibly benefit from additional features implemented in lwIP
>> - Less code to maintain in U-Boot
>>
>> Prior to applying this series, the lwIP stack needs to be added as a
>> Git subtree with the following command:
>>
>> $ git subtree add --squash --prefix lib/lwip/lwip
>> https://git.savannah.gnu.org/git/lwip.git STABLE-2_2_0_RELEASE
>>
>> The first patch renames some enums in order to avoid a conflict when a
>> later patch enables the lwIP library.
>>
>> The second patch introduces a new Kconfig symbol: NET_LWIP, which selects
>> the lwIP implementation instead of the current one (NET). Contrary to the
>> approach chosen by Maxim in [1], NET_LWIP and NET cannot be enabled
>> simultaneously. The rationale is we want to start from a clean state and
>> not pull potentially duplicated functionality from both stacks. Note
>> however that a few files are still built in net/, they are the ones
>> related to ethernet device management and the ethernet bootflow.
>>
>> The third patch splits the net.h header into net-legacy.h, net-common.h,
>> net-lwip.h, leaving net.h as a simple wrapper.
>>
>> The fourth patch introduces the Makefile to build lwIP when NET_LWIP is
>> enabled.
>>
>> The subsequent patches implement various network-oriented commands and
>> features: dhcp, dns, ping, tftpboot, wget.
>>
>> A number of features are currently incompatible with NET_LWIP: SANDBOX,
>> DFU_TFTP, FASTBOOT, SPL_NET. All make assumptions on how the network
>> stack is implemented and/or pull sybols that are not trivially exported
>> from lwIP. Some interface rework may be needed.
>>
>> Due to the above and in order to provide some level of testing, a new QEMU
>> configuration is introduced (qemu_arm64_lwip_defconfig) which is the same
>> as qemu_arm64_defconfig but with NET_LWIP and CMD_*_LWIP enabled.
>> Tests are added to test/py/tests/test_net.py for that configuration.
>>
>> Changes in v4:
>>
>> - Fixed the DHCP algorithm which was missing a sys_timeout() call in
>> the "fine timer" callback. This should close the issue that Tom R.
>> reported with his Raspberry Pi 3 (it does fix it on mine).
>> - The DHCP exchange timeout is increased from 2 to 10 seconds
>> - The DHCP exchange can be interrupted with Ctrl-C.
>> - "net: introduce alternative implementation as net-lwip/": rework
>> dependencies. A few symbols have 'depends on !NET_LWIP' and in addition
>> 'NET_LWIP depends on !SANDBOX'. Sandbox, DSA and fastboot are
>> unsupported, because they are deeply welded to the current stack.
>> - All network commands (dns, ping, tftp and wget):
>> * Get rid of global variables (Ilias A.)
>> * Use printf() rather than log_info()
>> - "net-lwip: add ping command": use packet count instead of
>> timeout, fix code style (Ilias A.)
>> - Add "net: split cmd/net.c into cmd/net.c and cmd/net-common.c"
>> extracted from the wget patch (Ilias A.).
>> - Add "net: split include/net.h into net{,-common,-legacy,-lwip}.h"
>> (Ilias A.)
>> - Add "flash: prefix error codes with FL_" which is required to
>> avoid name clashes when splitting net.h
>> - Reworked the initialization of the lwIP stack. One and only
>> one network interface (struct netif) is added for the duration
>> of the command that uses that interface. That's commit "net-lwip:
>> add DHCP support and dhcp commmand".
>> - Drop "test: dm: dsa, eth: disable tests when CONFIG_NET_LWIP=y",
>> not needed now that NET_LWIP depend on !SANDBOX.
>> - qemu_arm64_lwip_defconfig now enables CMD_DNS and CMD_WGET (so
>> that all the supported network commands are available).
>>
>> Changes in v3:
>>
>> - Make NET_LWIP a Kconfig choice in patch "net: introduce alternative
>> implementation as net-lwip/" (Tom R.)
>> - Drop the patch introducing lwIP as a Git subtree and document the git
>> command in the cover letter instead (Tom R.)
>> - "net-lwip: add TFTP support
Re: [PATCH v3 10/18] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Ilias,
On Fri, 21 Jun 2024 at 09:52, Ilias Apalodimas
wrote:
>
> Hi Simon,
>
> On Fri, 21 Jun 2024 at 17:57, Simon Glass wrote:
> >
> > Hi Ilias,
> >
> > On Thu, 20 Jun 2024 at 23:49, Ilias Apalodimas
> > wrote:
> > >
> > > On Fri, 21 Jun 2024 at 08:32, Ilias Apalodimas
> > > wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Fri, 21 Jun 2024 at 02:06, Simon Glass wrote:
> > > > >
> > > > > It does not make sense to enable all SHA algorithms unless they are
> > > > > needed. It bloats the code and in this case, causes chromebook_link to
> > > > > fail to build. That board does use the TPM, but not with measured
> > > > > boot,
> > > > > nor EFI.
> > > > >
> > > > > Since EFI_TCG2_PROTOCOL already selects these options, we just need to
> > > > > add them to MEASURED_BOOT as well.
> > > > >
> > > > > Note that the original commit combines refactoring and new features,
> > > > > which makes it hard to see what is going on.
> > > > >
> > > > > Fixes: 97707f12fda tpm: Support boot measurements
> > > > > Reviewed-by: Heinrich Schuchardt
> > > > > Signed-off-by: Simon Glass
> > > > > ---
> > > > >
> > > > > (no changes since v2)
> > > >
> > > > There was a discussion in the previous version, bout enabling these on
> > > > CMD_TPM as they are required.
> >
> > For reference [1]
> >
> > >
> > > Or switch this to an imply instead so you can disable it ?
> >
> > The thing is, we need to be able to easily disable unwanted features.
> > My reading of that thread is that U-Boot doesn't use SHA384/512 from
> > the 'tpm2 pcr_extend' command today.
>
> It does, it was mentioned on the thread you referenced. commit
> 89aa8463cdf39 ("tpm-v2: allow algorithm name to be configured for
> pcr_read and pcr_extend")
>
> > Even if it did, I would think it
> > better to allow the supported algorithms to be controlled. At worst, I
> > would expect a separate option to disable that subcommand which would
> > remove the implies.
>
> To control the algos you need to compile, you need to control the
> algorithms the TPM supports. Otherwise, you are leaving security
> holes. We don't configure the supported PCR banks today iirc.
>
> If we need the fix for those platforms for the release so badly, why
> don't we just disable the TPM? It's going to be useless and
> potentially dangerous without SHA support. Any idea why those
> platforms use it for?
>
> >
> > Some tpm code recently added puts a table and some functions in
> > tpm-v2.h - could someone fix that? tpm-common.c would be a better
> > place.
>
> Do you mean commit 954b95e77ef0a8? If that's the one, I don't mind
> moving it there.
> Those values though are described in [0], which is primarily for v2.0.
> Nothing prevents you from using it in v1.2 but all of the values apart
> from SHA1 are useless. Also, we don't support measured boot with 1.2
> anyway so I didn't see the point back then.
>
> > Also we already have a list of hash algorithms in U-Boot
> > (common/hash.c) so should use that, >and respect the fact that certain
> > algos may not be supported.
>
> That seems tailored for a very specific reason. Do you want to fill it
> in with EFI internal definitions?
> We could adjust some lookup functions which seems common.
>
> >
> > If there is a requirement that (for a certain situation / protocol) we
> > need to support everything, then let's have a Kconfig for that.
>
> We do. The difference here is that the *TPM at runtime* dictates what
> you need. So you can't blindly disable stuff.
>
> > It is
> > what my patch was trying to work around, perhaps. For boards which
> > don't care about extending TPM in this way, or don't use the more
> > expensive algorithms, it avoids lots of code bloat.
>
> Ok, I disagree here. That's potentially dangerous. Because someone,
> might need the TPM, easily misconfigure it, and end up with broken
> measurements.
>
> >
> > One of the principles of U-Boot is to allow configurability in these
> > situations. It shouldn't be too hard to have the best of both worlds
> > (supports all reasonable cases by default / allow unwanted cases to be
> > dropped).
>
> It's not. We can have the TPM configure the supported algorithms once it
> starts.
> IMHO that's the proper way to do it. Everything else is just papering
> over the problem -- which even worse no one will care to fix.
>
> >
> > So I'll await your reply on the above before figuring out where to go
> > with this. For this release I'd just like to get the board working,
> > but I understand that the TPM stuff is very-much in flux, perhaps in
> > -next.
>
> This is a very complex problem. Even if we configure the TPMs banks
> based on the supported algoriths, a previous stage loader might have
> enabled more. So you end up breaking that in that case...
>
> Let me see if I can configure the TPM on boot
I just don't understand any of this very well.
My request is, please can you find a way to allow the TPM to be
enabled for a board that doesn't use the pcr_extend subcommand with
Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
On Fri, 21 Jun 2024, 19:06 Vincent Stehlé, wrote:
> On Fri, Jun 21, 2024 at 01:00:51PM +0200, Heinrich Schuchardt wrote:
> (..)
> > The current specification is in RFC 9562, 4.1, "Variant field"
> >
> > "The variant field consists of a variable number of the most significant
> > bits of octet 8 of the UUID.
> >
> > ...
> >
> > Specifically for UUIDs in this document, bits 64 and 65 of the UUID
> > (bits 0 and 1 of octet 8) MUST be set to 1 and 0 as specified in row 2
> > of Table 1."
> >
> > This reference to byte 8 does not depend on endianness.
>
> Hi Heinrich,
>
> Agreed, variant is not concerned by the endianness.
>
> >
> > U-Boot's include/uuid.h has:
> >
> > /* This is structure is in big-endian */
> > struct uuid {
> >
> > The field time_hi_and_version needs to be stored in big-endian fashion.
>
> Thanks! I thought this structure was used to hold either a big-endian UUID
> or a
> little-endian GUID, but now you have convinced me.
>
> This confirms that the generation of the dynamic GUID is missing something:
>
> gen_uuid_v5(,
> (struct uuid *)_array[i].image_type_id,
> compatible, strlen(compatible),
> fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
> - sizeof(uint16_t),
> NULL);
>
> It is not possible to cast the little-endian efi_guid_t .image_type_id as
> the
> big-endian struct uuid output of gen_uuid_v5() like this; we need to
> convert the
> three time fields from big to little endianness.
>
I'm inclined to disagree, the comment above struct uuid in include/uuid.h
state clearly that the format in memory is always big endian, but that a
GUID when printed has some fields converted to little endian.
> >
> >
> > tools/genguid uses UUID_STR_FORMAT_GUID which prints low-endian which is
> > typical in the EFI context but not understood by 'uuid -d'. Maybe we
> > should add a parameter for the output format.
>
> My understanding is that there is a single universal string format for both
> UUIDs and GUIDs, which uuid -d understands, and which has no notion of
> endianness. (Only the structures in memory have an endianness.)
> This means we do not need an output format parameter.
>
> genguid is calling the new gen_uuid_v5() function, which outputs a
> big-endian
> struct uuid. Therefore, genguid should print it with 'STD format, not
> 'GUID.
>
> Best regards,
> Vincent.
>
> >
> > Best regards
> >
> > Heinrich
>
Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs
On Fri, Jun 21, 2024 at 01:00:51PM +0200, Heinrich Schuchardt wrote:
(..)
> The current specification is in RFC 9562, 4.1, "Variant field"
>
> "The variant field consists of a variable number of the most significant
> bits of octet 8 of the UUID.
>
> ...
>
> Specifically for UUIDs in this document, bits 64 and 65 of the UUID
> (bits 0 and 1 of octet 8) MUST be set to 1 and 0 as specified in row 2
> of Table 1."
>
> This reference to byte 8 does not depend on endianness.
Hi Heinrich,
Agreed, variant is not concerned by the endianness.
>
> U-Boot's include/uuid.h has:
>
> /* This is structure is in big-endian */
> struct uuid {
>
> The field time_hi_and_version needs to be stored in big-endian fashion.
Thanks! I thought this structure was used to hold either a big-endian UUID or a
little-endian GUID, but now you have convinced me.
This confirms that the generation of the dynamic GUID is missing something:
gen_uuid_v5(,
(struct uuid *)_array[i].image_type_id,
compatible, strlen(compatible),
fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
- sizeof(uint16_t),
NULL);
It is not possible to cast the little-endian efi_guid_t .image_type_id as the
big-endian struct uuid output of gen_uuid_v5() like this; we need to convert the
three time fields from big to little endianness.
>
>
> tools/genguid uses UUID_STR_FORMAT_GUID which prints low-endian which is
> typical in the EFI context but not understood by 'uuid -d'. Maybe we
> should add a parameter for the output format.
My understanding is that there is a single universal string format for both
UUIDs and GUIDs, which uuid -d understands, and which has no notion of
endianness. (Only the structures in memory have an endianness.)
This means we do not need an output format parameter.
genguid is calling the new gen_uuid_v5() function, which outputs a big-endian
struct uuid. Therefore, genguid should print it with 'STD format, not 'GUID.
Best regards,
Vincent.
>
> Best regards
>
> Heinrich
Re: [PATCH 5/9] fdt: Correct condition for bloblist existing
Hi Simon,
On Fri, 21 Jun 2024 at 10:58, Simon Glass wrote:
> Hi Ilias,
>
> On Wed, 19 Jun 2024 at 06:41, Ilias Apalodimas
> wrote:
> >
> > Hi Simon,
> >
> > On Wed, Jun 12, 2024 at 02:24:31PM -0600, Simon Glass wrote:
> > > Hi Ilias,
> > >
> > > On Wed, 12 Jun 2024 at 00:02, Ilias Apalodimas
> > > wrote:
> > > >
> > > > [...]
> > > >
> > > > > > > >> ---
> > > > > > > >>
> > > > > > > >> lib/fdtdec.c | 12 ++--
> > > > > > > >> 1 file changed, 10 insertions(+), 2 deletions(-)
> > > > > > > >>
> > > > > > > >> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> > > > > > > >> index b2c59ab3818..b141244e3b9 100644
> > > > > > > >> --- a/lib/fdtdec.c
> > > > > > > >> +++ b/lib/fdtdec.c
> > > > > > > >> @@ -1669,8 +1669,16 @@ int fdtdec_setup(void)
> > > > > > > >> {
> > > > > > > >> int ret = -ENOENT;
> > > > > > > >>
> > > > > > > >> - /* If allowing a bloblist, check that first */
> > > > > > > >> - if (CONFIG_IS_ENABLED(BLOBLIST)) {
> > > > > > > >> + /*
> > > > > > > >> +* If allowing a bloblist, check that first. This
> would be better
> > > > > > > >> +* handled with an OF_BLOBLIST Kconfig, but that
> caused far too much
> > > > > > > >> +* argument, so add a hack here, used e.g. by
> chromebook_coral
> > > > > > > >
> > > > > > > > I am a bit confused by this comment - It means you will not
> use OF_BLOBLIST,
> > > > > > > > but actually you are using it below. Is it a typo?
> > > > > > >
> > > > > > > Basically it would be cleaner to have a separate,
> phase-specific
> > > > > > > Kconfig control as to whether the DT can come from the
> bloblist (I
> > > > > > > can't remember the Kconfig name I suggested, nor the patch as
> it was
> > > > > > > last year sometime). But for now I am adding this hack to get
> a few
> > > > > > > boards working again.
> > > > > >
> > > > > > I am a bit confused.
> > > > > > First of all the comment is innapropriate. We went through a
> lengthy
> > > > > > discussion on BLOBLIST/OF_BLOSLIST etc and, even Tom chimed in
> and we
> > > > > > made up our minds. Why are you adding this comment now? Why do
> code
> > > > > > comments have to illustrate your personal opinion -- which was
> > > > > > rejected?
> > > > >
> > > > > I'm sorry for the tone of the comment. I am not trying to offend
> > > > > anyone here and I'm happy to change the language.
> > > >
> > > > Yes please a comment explaining why that piece of code is there is
> far
> > > > more intuitive.
> > >
> > > OK, once we have agreed the below I can do that.
> > >
> > > >
> > > > > As I probably
> > > > > mentioned at the time, my accepted patch breaks my workflow and
> > > > > several boards. I hope you can understand how frustrating that
> sort of
> > > > > thing can be.
> > > >
> > > > Yes, I do and I am fine with a short-term patch that fixes issues, as
> > > > long as its not too intrusive. There is a very thin line between
> quick
> > > > and dirty fixes to spaghetti unreadable code. But we should have
> > > > comments and/or commit messages indicating that this needs a proper
> > > > fix
> > >
> > > I spent a lot of time explaining this last time.
> > >
> > > >
> > > > > Also, now that I have my lab back up and running and I
> > > > > would like these boards to work again on mainline!
> > > > >
> > > > > >
> > > > > > Grepping for OF_BLOBLIST, I can't find any matches, so is the
> above if a typo?
> > > > >
> > > > > Remember, it was a patch you rejected :-)
> > > >
> > > > I don't maintain any of that. I only gave some feedback along the
> > > > lines of "bloblist was designed to be auto-discoverable, I don't see
> > > > how adding an explicit Kconfig helps". IIRC we eventually followed
> > > > what Tom suggested.
> > >
> > > I'm not trying to point the finger here. So far the boards are broken
> > > in mainline...I'm just trying to fix that,
> > >
> > > >
> > > > In any case, the amount of bike-shedding in the topic is too much. Do
> > > > you mind explaining the problem in your workflow again? Perhaps we
> can
> > > > find a solution that is integrated in bloblist_maybe_init() instead
> of
> > > > injecting ifs on when a bloblist should or should not be searched for
> > > > all over the place.
> > >
> > > TPL (or SPL) sets up a bloblist with bits of info in it, but no DT
> > > (which is in memory-mapped SPI flash)
> > > U-Boot proper starts up, it wants to get the bloblist but not hang if
> > > the bloblist doesn't have a DT
> >
> > Sure, that's reasonable and IIRC that's the design we agreed a while
> back.
> > Looking at the code, if the DT isn't found in a bloblist and the feature
> is
> > enabled, we don't crash. We just print a debug message and continue to
> find
> > the DT as we used. Why do we have to skip the call to
> > bloblist_maybe_init()?
>
> Because at this point, if there is no bloblist, then it needs to be
> created. But creating it this early may fail, e.g. since there is no
> malloc(). The intent of this code is to locate an
Re: [PATCH v4 00/14] Introduce the lwIP network stack
On Fri, Jun 21, 2024 at 5:59 AM Jerome Forissier wrote: > > > > On 6/20/24 19:10, Tim Harvey wrote: > > On Mon, Jun 17, 2024 at 8:33 AM Jerome Forissier > > wrote: > >> > >> This is a rework of a patch series by Maxim Uvarov: "net/lwip: add lwip > >> library for the network stack" [1]. The goal is to introduce the lwIP > >> TCP/IP > >> stack [2] [3] as an alternative to the current implementation in net/, > >> selectable with Kconfig, and ultimately keep only lwIP if possible. Some > >> reasons for doing so are: > >> - Make the support of HTTPS in the wget command easier. Javier T. (CC'd) > >> has some additional lwIP and Mbed TLS patches to do so. With that it > >> becomes possible to fetch and launch a distro installer such as Debian > >> etc. using a secure, authenticated connection directly from the U-Boot > >> shell. Several use cases: > >> * Authentication: prevent MITM attack (third party replacing the > >> binary with a different one) > >> * Confidentiality: prevent third parties from grabbing a copy of the > >> image as it is being downloaded > >> * Allow connection to servers that do not support plain HTTP anymore > >> (this is becoming more and more common on the Internet these days) > >> - Possibly benefit from additional features implemented in lwIP > >> - Less code to maintain in U-Boot > >> > >> Prior to applying this series, the lwIP stack needs to be added as a > >> Git subtree with the following command: > >> > >> $ git subtree add --squash --prefix lib/lwip/lwip > >> https://git.savannah.gnu.org/git/lwip.git STABLE-2_2_0_RELEASE > >> > >> The first patch renames some enums in order to avoid a conflict when a > >> later patch enables the lwIP library. > >> > >> The second patch introduces a new Kconfig symbol: NET_LWIP, which selects > >> the lwIP implementation instead of the current one (NET). Contrary to the > >> approach chosen by Maxim in [1], NET_LWIP and NET cannot be enabled > >> simultaneously. The rationale is we want to start from a clean state and > >> not pull potentially duplicated functionality from both stacks. Note > >> however that a few files are still built in net/, they are the ones > >> related to ethernet device management and the ethernet bootflow. > >> > >> The third patch splits the net.h header into net-legacy.h, net-common.h, > >> net-lwip.h, leaving net.h as a simple wrapper. > >> > >> The fourth patch introduces the Makefile to build lwIP when NET_LWIP is > >> enabled. > >> > >> The subsequent patches implement various network-oriented commands and > >> features: dhcp, dns, ping, tftpboot, wget. > >> > >> A number of features are currently incompatible with NET_LWIP: SANDBOX, > >> DFU_TFTP, FASTBOOT, SPL_NET. All make assumptions on how the network > >> stack is implemented and/or pull sybols that are not trivially exported > >> from lwIP. Some interface rework may be needed. > >> > >> Due to the above and in order to provide some level of testing, a new QEMU > >> configuration is introduced (qemu_arm64_lwip_defconfig) which is the same > >> as qemu_arm64_defconfig but with NET_LWIP and CMD_*_LWIP enabled. > >> Tests are added to test/py/tests/test_net.py for that configuration. > >> > > > > Hi Jermone, > > > > I've given this a spin on an imx8mm-venice-gw73xx-0x via > > imx8mm_venice_defconfig and I can't get DHCP to work (didn't work in > > v3 either): > > > > $ diff defconfig configs/imx8mm_venice_defconfig > > 68,69c68,71 > > < CONFIG_CMD_DNS_LWIP=y > > < CONFIG_CMD_WGET_LWIP=y > > --- > >> CONFIG_CMD_DHCP6=y > >> CONFIG_CMD_TFTPPUT=y > >> CONFIG_SYS_DISABLE_AUTOLOAD=y > >> CONFIG_CMD_WGET=y > > 88,90c90,94 > > < CONFIG_NET_LWIP=y > > < CONFIG_LWIP_DEBUG=y > > < CONFIG_LWIP_ASSERT=y > > --- > >> CONFIG_NET_RANDOM_ETHADDR=y > >> CONFIG_IP_DEFRAG=y > >> CONFIG_TFTP_BLOCKSIZE=4096 > >> CONFIG_PROT_TCP_SACK=y > >> CONFIG_IPV6=y > > > > Target: > > u-boot=> net list > > eth0 : ethernet@30be 00:d0:12:b5:f8:41 active > > u-boot=> dhcp || echo fail > > eth0: ethernet@30be 00:d0:12:b5:f8:41 active > > netif_set_ipaddr: netif address being changed > > netif: added interface 0 addr 192.168.1.1 netmask 0.0.0.0 gw 0.0.0.0 > > etharp_request: sending ARP request. > > etharp_raw: sending raw ARP packet. > > ethernet_output: sending packet 7df00bf0 > > dhcp_start(netif=7df2af40) 0dhcp_start(): mallocing new DHCP client > > dhcp_start(): allocated dhcp > > dhcp_start(): starting DHCP configuration > > dhcp_discover() > > transaction id xid(42021) > > dhcp_discover: making request > > dhcp_discover: sendto(DISCOVER, IP_ADDR_BROADCAST, > > LWIP_IANA_PORT_DHCP_SERVER) > > ip4_output_if: 0IP header: > > +---+ > > | 4 | 5 | 0x00 | 336 | (v, hl, tos, len) > > +---+ > > |0 |000| 0 | (id, flags, offset) > > +---+ > > | 255 | 17 |0xba9d | (ttl, proto, chksum) > > +---+ > > |0 |0
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Fri, Jun 21, 2024 at 08:57:51AM -0600, Simon Glass wrote: > Hi Tom, > > On Thu, 20 Jun 2024 at 17:19, Tom Rini wrote: > > > > On Thu, Jun 20, 2024 at 05:05:29PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Wed, 19 Jun 2024 at 09:32, Tom Rini wrote: > > > > > > > > On Tue, Jun 18, 2024 at 09:03:37PM -0600, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Tue, 18 Jun 2024 at 08:15, Tom Rini wrote: > > > > > > > > > > > > On Tue, Jun 18, 2024 at 06:43:51AM -0600, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Mon, 17 Jun 2024 at 11:16, Tom Rini wrote: > > > > > > > > > > > > > > > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi Heinrich > > > > > > > > > > > > > > > > > > > > resending the reply, I accidentally sent half of the > > > > > > > > > > message... > > > > > > > > > > > > > > > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > > > > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > > > > > > > > wrote: > > > > > > > > > > > >> > > > > > > > > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > > > > > > > > >>> Hi Simon, > > > > > > > > > > > >>> > > > > > > > > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass > > > > > > > > > > > >>> wrote: > > > > > > > > > > > > > > > > > > > > > > It does not make sense to enable all SHA algorithms > > > > > > > > > > > unless they are > > > > > > > > > > > needed. It bloats the code and in this case, causes > > > > > > > > > > > chromebook_link to > > > > > > > > > > > fail to build. That board does use the TPM, but not > > > > > > > > > > > with measured boot, > > > > > > > > > > > nor EFI. > > > > > > > > > > > > > > > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these > > > > > > > > > > > options, we just need to > > > > > > > > > > > add them to MEASURED_BOOT as well. > > > > > > > > > > > > > > > > > > > > > > Note that the original commit combines refactoring > > > > > > > > > > > and new features, > > > > > > > > > > > which makes it hard to see what is going on. > > > > > > > > > > > > > > > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > > > > > > > > - Consider MEASURED_BOOT too > > > > > > > > > > > > > > > > > > > > > > boot/Kconfig | 4 > > > > > > > > > > > lib/Kconfig | 4 > > > > > > > > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > > > > > > > index 6f3096c15a6..b061891e109 100644 > > > > > > > > > > > --- a/boot/Kconfig > > > > > > > > > > > +++ b/boot/Kconfig > > > > > > > > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > > > > > > > > config MEASURED_BOOT > > > > > > > > > > > bool "Measure boot images and > > > > > > > > > > > configuration when booting without EFI" > > > > > > > > > > > depends on HASH && TPM_V2 > > > > > > > > > > > + select SHA1 > > > > > > > > > > > + select SHA256 > > > > > > > > > > > + select SHA384 > > > > > > > > > > > + select SHA512 > > > > > > > > > > > help > > > > > > > > > > > This option enables measurement of the > > > > > > > > > > > boot process when booting > > > > > > > > > > > without UEFI . Measurement involves > > > > > > > > > > > creating cryptographic hashes > > > > > > > > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > > > > > > > > index 189e6eb31aa..568892fce44 100644 > > > > > > > > > > > --- a/lib/Kconfig > > > > > > > > > > > +++ b/lib/Kconfig > > > > > > > > > > > @@ -438,10 +438,6 @@ config TPM > > > > > > > > > > > bool "Trusted Platform Module (TPM) > > > > > > > > > > > Support" > > > > > > > > > > > depends on DM > > > > > > > > > > > imply DM_RNG > > > > > > > > > > > - select SHA1 > > > > > > > > > > > - select SHA256 > > > > > > > > > > > - select SHA384 > > > > > > > > > > > - select SHA512 > > > > > > > > > > > >>> > > > > > > > > > > > >>> I am not sure this is the right way to deal with your > > > > > > > > > > > >>> problem. > > > > > > > > > > > >>> The TPM main
Re: [PATCH v3 10/18] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Simon,
On Fri, 21 Jun 2024 at 17:57, Simon Glass wrote:
>
> Hi Ilias,
>
> On Thu, 20 Jun 2024 at 23:49, Ilias Apalodimas
> wrote:
> >
> > On Fri, 21 Jun 2024 at 08:32, Ilias Apalodimas
> > wrote:
> > >
> > > Hi Simon,
> > >
> > > On Fri, 21 Jun 2024 at 02:06, Simon Glass wrote:
> > > >
> > > > It does not make sense to enable all SHA algorithms unless they are
> > > > needed. It bloats the code and in this case, causes chromebook_link to
> > > > fail to build. That board does use the TPM, but not with measured boot,
> > > > nor EFI.
> > > >
> > > > Since EFI_TCG2_PROTOCOL already selects these options, we just need to
> > > > add them to MEASURED_BOOT as well.
> > > >
> > > > Note that the original commit combines refactoring and new features,
> > > > which makes it hard to see what is going on.
> > > >
> > > > Fixes: 97707f12fda tpm: Support boot measurements
> > > > Reviewed-by: Heinrich Schuchardt
> > > > Signed-off-by: Simon Glass
> > > > ---
> > > >
> > > > (no changes since v2)
> > >
> > > There was a discussion in the previous version, bout enabling these on
> > > CMD_TPM as they are required.
>
> For reference [1]
>
> >
> > Or switch this to an imply instead so you can disable it ?
>
> The thing is, we need to be able to easily disable unwanted features.
> My reading of that thread is that U-Boot doesn't use SHA384/512 from
> the 'tpm2 pcr_extend' command today.
It does, it was mentioned on the thread you referenced. commit
89aa8463cdf39 ("tpm-v2: allow algorithm name to be configured for
pcr_read and pcr_extend")
> Even if it did, I would think it
> better to allow the supported algorithms to be controlled. At worst, I
> would expect a separate option to disable that subcommand which would
> remove the implies.
To control the algos you need to compile, you need to control the
algorithms the TPM supports. Otherwise, you are leaving security
holes. We don't configure the supported PCR banks today iirc.
If we need the fix for those platforms for the release so badly, why
don't we just disable the TPM? It's going to be useless and
potentially dangerous without SHA support. Any idea why those
platforms use it for?
>
> Some tpm code recently added puts a table and some functions in
> tpm-v2.h - could someone fix that? tpm-common.c would be a better
> place.
Do you mean commit 954b95e77ef0a8? If that's the one, I don't mind
moving it there.
Those values though are described in [0], which is primarily for v2.0.
Nothing prevents you from using it in v1.2 but all of the values apart
from SHA1 are useless. Also, we don't support measured boot with 1.2
anyway so I didn't see the point back then.
> Also we already have a list of hash algorithms in U-Boot
> (common/hash.c) so should use that, >and respect the fact that certain
> algos may not be supported.
That seems tailored for a very specific reason. Do you want to fill it
in with EFI internal definitions?
We could adjust some lookup functions which seems common.
>
> If there is a requirement that (for a certain situation / protocol) we
> need to support everything, then let's have a Kconfig for that.
We do. The difference here is that the *TPM at runtime* dictates what
you need. So you can't blindly disable stuff.
> It is
> what my patch was trying to work around, perhaps. For boards which
> don't care about extending TPM in this way, or don't use the more
> expensive algorithms, it avoids lots of code bloat.
Ok, I disagree here. That's potentially dangerous. Because someone,
might need the TPM, easily misconfigure it, and end up with broken
measurements.
>
> One of the principles of U-Boot is to allow configurability in these
> situations. It shouldn't be too hard to have the best of both worlds
> (supports all reasonable cases by default / allow unwanted cases to be
> dropped).
It's not. We can have the TPM configure the supported algorithms once it starts.
IMHO that's the proper way to do it. Everything else is just papering
over the problem -- which even worse no one will care to fix.
>
> So I'll await your reply on the above before figuring out where to go
> with this. For this release I'd just like to get the board working,
> but I understand that the TPM stuff is very-much in flux, perhaps in
> -next.
This is a very complex problem. Even if we configure the TPMs banks
based on the supported algoriths, a previous stage loader might have
enabled more. So you end up breaking that in that case...
Let me see if I can configure the TPM on boot
Cheers
/Ilias
>
> Regards,
> Simon
>
> [1]
> https://patchwork.ozlabs.org/project/uboot/patch/20240610145920.3302001-3-...@chromium.org/
[0]
https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf
-- search for EFI_TCG2_BOOT_HASH_ALG_SHA512
Re: Request for hosting a boot-firmware repository in u-boot git (denx and GitHub)
On June 21, 2024 thus sayeth Simon Glass: > Hi Peter, > > On Fri, 21 Jun 2024 at 01:35, Peter Robinson wrote: > > > > On Fri, 21 Jun 2024 at 00:05, Simon Glass wrote: > > > > > > Hi Nishanth, > > > > > > On Thu, 20 Jun 2024 at 15:35, Nishanth Menon wrote: > > > > > > > > Hi Team, > > > > > > > > We have briefly discussed this topic on IRC[1]. I would like to > > > > propose a new boot-firmware repository similar to the Linux-firmware > > > > repository under the aegis of u-boot hosting. > > > > > > > > In addition to TI, it looks like some NXP[2] and Rockchip[3] > > > > platforms seem to require additional closed-source/open-source > > > > binaries to have a complete bootable image. Distribution rights and > > > > locations of these binaries are challenging, and there needs to be a > > > > standard for how and where they are hosted for end users. > > > > > > > > Further, looking ahead to future architectures: > > > > * IP firmware: More and more IP vendors are embedding their own > > > > "specialized controllers" and require firmware for the operation > > > > (similar to Rockchip's DDR controller, I guess), > > > > * boot stage firmware: Additional stages of the boot process involve > > > > vendor intermediate firmware, such as power configuration. > > > > * Security enclave binaries: While I see a few folks trying to have an > > > > open-source s/w architecture, many PKA and PQC systems still require > > > > prop binaries for IP reasons. > > > > > > > > NOTE: I am not judging any company(including TI) for reasons why some > > > > firmware is proprietary, but I hate to have the end users and other > > > > system (distro) maintainers have to deal with hell trying to make the > > > > life of end users easy to live with. > > > > > > > > In the case of TI's K3 architecture devices, we have two binary blobs > > > > that are critical for the boot process. > > > > > > > > 1. TIFS Firmware / DMSC firmware[4]—This is the security enclave > > > > firmware. It is often encrypted, and sources are not public (due to > > > > various business/regulatory reasons). > > > > 2. DM Firmware[5] - There is a source in public in some cases and > > > > binary only in others - essentially limited function binary to be > > > > put up in the device management uC. In cases where the source is > > > > available, the build procedure is, in my personal opinion, pretty > > > > arcane, and even though in theory it is practical, in practice, not > > > > friendly - efforts are going to simplify it, even probably integrate > > > > it with a more opensource ecosystem, but that is talking "look at the > > > > tea leaves" stuff. > > > > 3. Low Power Management (LPM) binaries: tifs stub: another encrypted > > > > binary that gives the tifs system context restore logic before > > > > retrieving tifs firmware and a corresponding DM restoration binary. > > > > > > > > All told, this is not unlike the situation that necessitated the > > > > creation of a Linux firmware repository. > > > > > > > > Options that I see: > > > > > > > > 1. Let the status quo be - SoC vendors maintain random locations and > > > > random rules to maintain boot firmware. > > > > 2. Ask Linux-firmware to host the binaries in a single canonical > > > > location > > > > 3. Host a boot-firmware repository - u-boot repo may be the more > > > > logical location. > > > > > > > > * (1) isn't the correct answer. > > > > > > > > * (2) Though I haven't seen any policy from the Linux-firmware > > > > community mandating anything of the form, the binaries we are talking > > > > of may not belong to Linux-firmware as they aren't strictly speaking > > > > something Linux kernel will load (since the bootloader has that > > > > responsibility), and in some cases may not even directly talk to > > > > (security enclave or DDR firmware stuff). I am adding Josh to this > > > > mail to see if he has any opinions on the topic (but keeping > > > > from cross posting on linux-firmware list, unless folks feel it is > > > > OK). > > > > > > > > On (3): > > > > Proposal: > > > > > > > > * Create a boot firmware repository in Denx and/or GitHub (if > > > > financials are a hurdle, I hope we can solve it as a community). > > > > * Limit binaries only to those consumed part of the u-boot scope. > > > > > > > > * Limit binaries only to those that do not have an opensource project > > > > (Trusted Firmware-A/M, OP-TEE, etc..) or depend entirely on vendor > > > > source or are binary only in nature (subject to licensing terms below) > > > > * Limit binaries to some pre-established size to prevent repository > > > > explosion - say, 512Kib? > > > > * Follow the same rules of integration and licensing guidelines as > > > > Linux-firmware[6]. > > > > * Similar rules as Linux-firmware guidelines of ABI backward and > > > > forward compatibility. > > > > * Set a workflow update flow and a compatibility requirements document > > > > > > > > If
Re: [PATCH] imx8mp-venice-gw702x: Drop EQos clock workaround
On Fri, Jun 21, 2024 at 7:24 AM Fabio Estevam wrote:
>
> Hi Tim,
>
> On Wed, Jun 19, 2024 at 10:30 PM Tim Harvey wrote:
> >
> > The assigned-clock no longer have to be dropped, the clock are now
> > defined in clk-imx8mp.c and used by DWMAC driver to configure the
> > DWMAC clock. Drop the workarounds from U-Boot specific DT extras.
> >
> > Having the clocks dropped causes the EQoS to be non-functional.
> >
> > See commit c7ea9612df0f ("arm64: dts: imx8mp: Drop EQoS clock
> > workaround").
> >
> > Fixes: 48c6f9777cee ("board: gateworks: venice: add imx8mp-gw7905-2x
> > support")
> > Signed-off-by: Tim Harvey
>
> Is this targetted for master or next?
Hi Fabio,
It doesn't make much difference to me. This is a fix for a board model
that was renamed from gw7905 to gw7500 anyway which is a change in
dts/upstream I'm going to have to wait for anyway so you can leave it
for next if you want. I actually have about 8 patches in my repo to
dts/upstream from linux 6.9 and upcoming linux 6.10 as dts/upstream
was last merged from v6.8.
The set of 5 or so patches I sent in the last few days are all fixes
of sorts and they are all isolated to board/gateworks/venice so don't
affect anyone else.
Best Regards,
Tim
Re: [PATCH] imx: hab: Make imx_hab_is_enabled dependent on FIELD_RETURN
On 6/21/24 3:06 PM, Paul Geurts wrote: The decision on whether HAB is enabled is solely based on the SEC_CONFIG fuse. The HAB FIELD_RETURN feature is able to permanently disable HAB on a CPU, after which it is able to boot unsigned firmware. U-Boot however does not take into account the FIELD_RETURN mode, and refuses to boot unsigned software when the feature is enabled. Also take the FIELD_RETURN fuse into account when deciding whether HAB is enabled. When The FIELD_RETURN fuse is blown, HAB is not enabled. Tested on i.MX8M Mini, i.MX8M Plus, i.MX8M Nano and i.MX6ULL The purpose of the field return fuse is to unlock a system when it is returned to factory, right ? Can the system be re-locked afterward too ?
Re: [PATCH v5] cmd: bootm: add ELF file support
On 6/21/24 16:16, Максим Москалец wrote:
пт, 21 июн. 2024 г. в 15:51, Heinrich Schuchardt :
On 21.06.24 13:42, Maxim Moskalets wrote:
Some operating systems (e.g. seL4) and embedded applications are ELF
images. It is convenient to use FIT-images to implement trusted boot.
Added "elf" image type for booting using bootm command.
Signed-off-by: Maxim Moskalets
I guess a test could be added to test/py/tests/test_zynqmp_rpu.py which
already uses the bootelf command?
Is it necessary to use specific hardware to run this test?
In Gitlab CI xilinx_zynq_virt_defconfig is used with QEMU parameters
from
https://source.denx.de/u-boot/u-boot-test-hooks/-/blob/master/bin/travis-ci/conf.xilinx_zynq_virt_qemu:
qemu-system-arm \
-machine xilinx-zynq-a9 \
-display none \
-m 1G \
-nographic \
-serial /dev/null \
-serial mon:stdio \
-monitor null \
-device loader,file=u-boot-dtb.bin,addr=0x400,cpu-num=0
See for instance this build:
https://source.denx.de/u-boot/custodians/u-boot-efi/-/jobs/852062
Best regards
Heinrich
Heinrich
---
v5:
used func from lib/elf.c to avoid dependency form CLI
---
boot/bootm_os.c | 18 ++
boot/image-fit.c | 3 ++-
boot/image.c | 5 -
cmd/Kconfig | 7 +++
include/image.h | 1 +
5 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/boot/bootm_os.c b/boot/bootm_os.c
index 15297ddb53..6a6621706f 100644
--- a/boot/bootm_os.c
+++ b/boot/bootm_os.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -394,6 +395,20 @@ static int do_bootm_qnxelf(int flag, struct
bootm_info *bmi)
}
#endif
+#if defined(CONFIG_BOOTM_ELF)
+static int do_bootm_elf(int flag, struct bootm_info *bmi)
+{
+ Bootelf_flags flags = { .autostart = 1 };
+
+ if (flag != BOOTM_STATE_OS_GO)
+ return 0;
+
+ bootelf(bmi->images->ep, flags, 0, NULL);
+
+ return 1;
+}
+#endif
+
#ifdef CONFIG_INTEGRITY
static int do_bootm_integrity(int flag, struct bootm_info *bmi)
{
@@ -535,6 +550,9 @@ static boot_os_fn *boot_os[] = {
#ifdef CONFIG_BOOTM_EFI
[IH_OS_EFI] = do_bootm_efi,
#endif
+#if defined(CONFIG_BOOTM_ELF)
+ [IH_OS_ELF] = do_bootm_elf,
+#endif
};
/* Allow for arch specific config before we boot */
diff --git a/boot/image-fit.c b/boot/image-fit.c
index f6464bcf62..9253f81fff 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -2175,7 +2175,8 @@ int fit_image_load(struct bootm_headers *images,
ulong addr,
fit_image_check_os(fit, noffset, IH_OS_TEE) ||
fit_image_check_os(fit, noffset, IH_OS_OPENRTOS) ||
fit_image_check_os(fit, noffset, IH_OS_EFI) ||
- fit_image_check_os(fit, noffset, IH_OS_VXWORKS);
+ fit_image_check_os(fit, noffset, IH_OS_VXWORKS) ||
+ fit_image_check_os(fit, noffset, IH_OS_ELF);
/*
* If either of the checks fail, we should report an error, but
diff --git a/boot/image.c b/boot/image.c
index fc774d605d..abac254e02 100644
--- a/boot/image.c
+++ b/boot/image.c
@@ -130,7 +130,10 @@ static const table_entry_t uimage_os[] = {
{ IH_OS_OPENRTOS, "openrtos", "OpenRTOS", },
#endif
{ IH_OS_OPENSBI, "opensbi", "RISC-V OpenSBI", },
- { IH_OS_EFI, "efi", "EFI Firmware" },
+ { IH_OS_EFI, "efi", "EFI Firmware" },
+#ifdef CONFIG_BOOTM_ELF
+ { IH_OS_ELF, "elf", "ELF Image" },
+#endif
{ -1, "", "", },
};
diff --git a/cmd/Kconfig b/cmd/Kconfig
index a2dee34689..613baa106e 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -321,6 +321,13 @@ config BOOTM_EFI
help
Support booting UEFI FIT images via the bootm command.
+config BOOTM_ELF
+ bool "Support booting ELF images"
+ depends on CMD_BOOTM && LIB_ELF
+ default n
+ help
+ Support booting ELF images via the bootm command.
+
config CMD_BOOTZ
bool "bootz"
help
diff --git a/include/image.h b/include/image.h
index c5b288f62b..9daaee15cd 100644
--- a/include/image.h
+++ b/include/image.h
@@ -100,6 +100,7 @@ enum {
IH_OS_TEE, /* Trusted Execution Environment */
IH_OS_OPENSBI, /* RISC-V OpenSBI */
IH_OS_EFI, /* EFI Firmware (e.g. GRUB2) */
+ IH_OS_ELF, /* ELF Image (e.g. seL4) */
IH_OS_COUNT,
};
Re: [PATCH v2 2/6] buildman: Add python3-pycryptodome
On Fri, Jun 21, 2024 at 07:14:18AM -0600, Simon Glass wrote: > This is used by some Binman entry types, so add it to allow more tests > to pass. > > Signed-off-by: Simon Glass Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/6] buildman: Add python3-coverage
On Fri, Jun 21, 2024 at 07:14:17AM -0600, Simon Glass wrote: > Add this package so we can run code-coverage tests for Binman. > > Signed-off-by: Simon Glass Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH 3/5] buildman: Support building within a Python venv
On Fri, Jun 21, 2024 at 08:57:50AM -0600, Simon Glass wrote:
> Hi Tom,
>
> On Thu, 20 Jun 2024 at 17:30, Tom Rini wrote:
> >
> > On Thu, Jun 20, 2024 at 05:05:30PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 20 Jun 2024 at 08:32, Tom Rini wrote:
> > > >
> > > > On Thu, Jun 20, 2024 at 07:19:35AM -0600, Simon Glass wrote:
> > > >
> > > > > The Python virtualenv tool sets up a few things in the envronment,
> > > > > putting its path first in the PATH environment variable and setting up
> > > > > a sys.prefix different from the sys.base_prefix value.
> > > > >
> > > > > At present buildman puts the toolchain path first in PATH so that it
> > > > > can
> > > > > be found easily during the build. For sandbox this causes problems
> > > > > since
> > > > > /usr/bin/gcc (for example) results in '/usr/bin' being prepended to
> > > > > the
> > > > > PATH variable. As a result, the venv is partially disabled.
> > > > >
> > > > > The result is that sandbox builds within a venv ignore the venv, e.g.
> > > > > when looking for packages.
> > > > >
> > > > > Correct this by detecting the venv and adding the toolchain path after
> > > > > the venv path.
> > > > >
> > > > > Signed-off-by: Simon Glass
> > > >
> > > > Why are we using PATH at all in this case? Shouldn't we just be setting
> > > > CROSS_COMPILE=/full/path/to/the/prefix ?
> > >
> > > This is the -p option to buildman. The original commit was:
> > >
> > > commit bb1501f2c22c979961b735db775605cccedd98f6
> > > Author: Simon Glass
> > > Date: Mon Dec 1 17:34:00 2014 -0700
> > >
> > > buildman: Add an option to use the full tool chain path
> > >
> > > In some cases there may be multiple toolchains with the same name in
> > > the
> > > path. Provide an option to use the full path in the CROSS_COMPILE
> > > environment variable.
> > >
> > > Note: Wolfgang mentioned that this is dangerous since in some cases
> > > there
> > > may be other tools on the path that are needed. So this is set up as
> > > an
> > > option, not the default. I will need test confirmation (i.e. that this
> > > commit fixes a real problem) before merging it.
> > >
> > > As to why we don't always do this, well that is back in the mists of
> > > time, 10 years ago.
> > >
> > > BTW, this is raising a point ("let's change the behaviour") separate
> > > from the goal of this commit, which is to fix a problem with venv,
> > > albeit that if we made -p the only option, then we could potentially
> > > drop all PATH changes. Perhaps toolchains are built differently now,
> > > such that they always invoke their tools using the same prefix and
> > > dir?
> >
> > Wait, I'm confused. buildman internally updates its own PATH to avoid
> > calling CROSS_COMPILE with the full path due to a concern about
> > toolchain bugs?
>
> Not its own PATH: the one it passes to U-Boot's 'make'.
OK, but the point stands.
> I'm not sure why, actually. It is such a long time ago that I don't remember.
>
> I see:
>
> ~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-ld
Yes, prefixed version that's allowed to be called by users.
> and
>
> ~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/arm-linux-gnueabi/bin/ld
Internal usage, here be dragons and all that.
> but interestingly there is no gcc in the latter directory, which there
> was in 4.6 (and presumably for some time after).
>
> Certainly for sandbox there is no prefix, so we cannot add it in that
> case, and sandbox is actually the arch used to run these tests.
CROSS_COMPILE is empty for sandbox, yes.
> What are you suggesting we change about this patch?
That it's going about things backwards? If you're setting CROSS_COMPILE
_then_ it should be the full path that it already knows otherwise if not
setting CROSS_COMPILE then also not modifying PATH.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v3 16/18] rockchip: Avoid #ifdefs in RK3399 SPL
Hi Jonas,
On Fri, 21 Jun 2024 at 00:45, Jonas Karlman wrote:
>
> Hi Simon,
>
> On 2024-06-21 01:06, Simon Glass wrote:
> > The code here is confusing due to large blocks which are #ifdefed out.
> > Add a function phase_sdram_init() which returns whether SDRAM init
> > should happen in the current phase, using that as needed to control the
> > code flow.
> >
> > This increases code size by about 500 bytes in SPL when the cache is on,
> > since it must call the rather large rockchip_sdram_size() function.
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > Changes in v3:
> > - Split out the refactoring into a separate patch
> > - Drop the non-dcache optimisation, since the cache should normally be on
> >
> > drivers/ram/rockchip/sdram_rk3399.c | 47 -
> > 1 file changed, 26 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/ram/rockchip/sdram_rk3399.c
> > b/drivers/ram/rockchip/sdram_rk3399.c
> > index 3c4e20f4e80..2f37dd712e7 100644
> > --- a/drivers/ram/rockchip/sdram_rk3399.c
> > +++ b/drivers/ram/rockchip/sdram_rk3399.c
> > @@ -13,6 +13,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -63,8 +64,6 @@ struct chan_info {
> > };
> >
> > struct dram_info {
> > -#if defined(CONFIG_TPL_BUILD) || \
> > - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
> > u32 pwrup_srefresh_exit[2];
> > struct chan_info chan[2];
> > struct clk ddr_clk;
> > @@ -75,7 +74,6 @@ struct dram_info {
> > struct rk3399_pmusgrf_regs *pmusgrf;
> > struct rk3399_ddr_cic_regs *cic;
> > const struct sdram_rk3399_ops *ops;
> > -#endif
> > struct ram_info info;
> > struct rk3399_pmugrf_regs *pmugrf;
> > };
> > @@ -92,9 +90,6 @@ struct sdram_rk3399_ops {
> > struct rk3399_sdram_params *params);
> > };
> >
> > -#if defined(CONFIG_TPL_BUILD) || \
> > - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
> > -
> > struct rockchip_dmc_plat {
> > #if CONFIG_IS_ENABLED(OF_PLATDATA)
> > struct dtd_rockchip_rk3399_dmc dtplat;
> > @@ -191,6 +186,17 @@ struct io_setting {
> > },
> > };
> >
> > +/**
> > + * phase_sdram_init() - Check if this is the phase where SDRAM init happens
> > + *
> > + * Returns: true to do SDRAM init in this phase, false to not
> > + */
> > +static bool phase_sdram_init(void)
> > +{
> > + return spl_phase() == PHASE_TPL ||
> > + (!IS_ENABLED(CONFIG_TPL) && !spl_in_proper());
> > +}
> > +
> > static struct io_setting *
> > lpddr4_get_io_settings(const struct rk3399_sdram_params *params, u32 mr5)
> > {
> > @@ -3024,7 +3030,7 @@ static int rk3399_dmc_of_to_plat(struct udevice *dev)
> > struct rockchip_dmc_plat *plat = dev_get_plat(dev);
> > int ret;
> >
> > - if (!CONFIG_IS_ENABLED(OF_REAL))
> > + if (!CONFIG_IS_ENABLED(OF_REAL) || !phase_sdram_init())
> > return 0;
> >
> > ret = dev_read_u32_array(dev, "rockchip,sdram-params",
> > @@ -3138,22 +3144,24 @@ static int rk3399_dmc_init(struct udevice *dev)
> >
> > return 0;
> > }
> > -#endif
> >
> > static int rk3399_dmc_probe(struct udevice *dev)
> > {
> > struct dram_info *priv = dev_get_priv(dev);
> >
> > -#if defined(CONFIG_TPL_BUILD) || \
> > - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
> > - if (rk3399_dmc_init(dev))
> > - return 0;
> > -#endif
> > - priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
> > - debug("%s: pmugrf = %p\n", __func__, priv->pmugrf);
> > - priv->info.base = CFG_SYS_SDRAM_BASE;
> > - priv->info.size =
> > - rockchip_sdram_size((phys_addr_t)>pmugrf->os_reg2);
> > + if (phase_sdram_init()) {
> > + if (rk3399_dmc_init(dev))
> > + return 0;
> > + } else {
> > + priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
> > + debug("%s: pmugrf = %p\n", __func__, priv->pmugrf);
> > + }
> > +
> > + if (!CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
>
> This check should be dropped, this driver intent is to initialize dram
> in first phase (normally TPL), and report the size to any other phase.
This whole patch can be dropped :-) Here I am just trying to avoid
code-size growth when the cache is off. But yes, it is not needed.
>
> The main need for phase_sdram_init() and disable of DCACHE can probably
> be avoided if bob/kevin can move to use separate TPL and SPL instead of
> doing both dram init and everything else in SPL.
>
> My best guess is that enabling of caches in SPL cause issue for bob and
> kevin because they only use SPL not TPL+SPL like (if I am not mistaken)
> all other Rockchip arm64 targets.
>
> Using SPL-only was not something I tested when caches was enabled in SPL.
>
> Maybe bob/kevin can be changed to also use TPL+SPL similar to all other
> RK3399 boards?
>
> How U-Boot works on these chromebooks is still a mystery to me.
>
Re: [PATCH v4 00/14] Introduce the lwIP network stack
Hi Jerome,
On Mon, 17 Jun 2024 at 09:33, Jerome Forissier
wrote:
>
> This is a rework of a patch series by Maxim Uvarov: "net/lwip: add lwip
> library for the network stack" [1]. The goal is to introduce the lwIP TCP/IP
> stack [2] [3] as an alternative to the current implementation in net/,
> selectable with Kconfig, and ultimately keep only lwIP if possible. Some
> reasons for doing so are:
> - Make the support of HTTPS in the wget command easier. Javier T. (CC'd)
> has some additional lwIP and Mbed TLS patches to do so. With that it
> becomes possible to fetch and launch a distro installer such as Debian
> etc. using a secure, authenticated connection directly from the U-Boot
> shell. Several use cases:
> * Authentication: prevent MITM attack (third party replacing the
> binary with a different one)
> * Confidentiality: prevent third parties from grabbing a copy of the
> image as it is being downloaded
> * Allow connection to servers that do not support plain HTTP anymore
> (this is becoming more and more common on the Internet these days)
> - Possibly benefit from additional features implemented in lwIP
> - Less code to maintain in U-Boot
>
> Prior to applying this series, the lwIP stack needs to be added as a
> Git subtree with the following command:
>
> $ git subtree add --squash --prefix lib/lwip/lwip
> https://git.savannah.gnu.org/git/lwip.git STABLE-2_2_0_RELEASE
>
> The first patch renames some enums in order to avoid a conflict when a
> later patch enables the lwIP library.
>
> The second patch introduces a new Kconfig symbol: NET_LWIP, which selects
> the lwIP implementation instead of the current one (NET). Contrary to the
> approach chosen by Maxim in [1], NET_LWIP and NET cannot be enabled
> simultaneously. The rationale is we want to start from a clean state and
> not pull potentially duplicated functionality from both stacks. Note
> however that a few files are still built in net/, they are the ones
> related to ethernet device management and the ethernet bootflow.
>
> The third patch splits the net.h header into net-legacy.h, net-common.h,
> net-lwip.h, leaving net.h as a simple wrapper.
>
> The fourth patch introduces the Makefile to build lwIP when NET_LWIP is
> enabled.
>
> The subsequent patches implement various network-oriented commands and
> features: dhcp, dns, ping, tftpboot, wget.
>
> A number of features are currently incompatible with NET_LWIP: SANDBOX,
> DFU_TFTP, FASTBOOT, SPL_NET. All make assumptions on how the network
> stack is implemented and/or pull sybols that are not trivially exported
> from lwIP. Some interface rework may be needed.
>
> Due to the above and in order to provide some level of testing, a new QEMU
> configuration is introduced (qemu_arm64_lwip_defconfig) which is the same
> as qemu_arm64_defconfig but with NET_LWIP and CMD_*_LWIP enabled.
> Tests are added to test/py/tests/test_net.py for that configuration.
>
> Changes in v4:
>
> - Fixed the DHCP algorithm which was missing a sys_timeout() call in
> the "fine timer" callback. This should close the issue that Tom R.
> reported with his Raspberry Pi 3 (it does fix it on mine).
> - The DHCP exchange timeout is increased from 2 to 10 seconds
> - The DHCP exchange can be interrupted with Ctrl-C.
> - "net: introduce alternative implementation as net-lwip/": rework
> dependencies. A few symbols have 'depends on !NET_LWIP' and in addition
> 'NET_LWIP depends on !SANDBOX'. Sandbox, DSA and fastboot are
> unsupported, because they are deeply welded to the current stack.
> - All network commands (dns, ping, tftp and wget):
> * Get rid of global variables (Ilias A.)
> * Use printf() rather than log_info()
> - "net-lwip: add ping command": use packet count instead of
> timeout, fix code style (Ilias A.)
> - Add "net: split cmd/net.c into cmd/net.c and cmd/net-common.c"
> extracted from the wget patch (Ilias A.).
> - Add "net: split include/net.h into net{,-common,-legacy,-lwip}.h"
> (Ilias A.)
> - Add "flash: prefix error codes with FL_" which is required to
> avoid name clashes when splitting net.h
> - Reworked the initialization of the lwIP stack. One and only
> one network interface (struct netif) is added for the duration
> of the command that uses that interface. That's commit "net-lwip:
> add DHCP support and dhcp commmand".
> - Drop "test: dm: dsa, eth: disable tests when CONFIG_NET_LWIP=y",
> not needed now that NET_LWIP depend on !SANDBOX.
> - qemu_arm64_lwip_defconfig now enables CMD_DNS and CMD_WGET (so
> that all the supported network commands are available).
>
> Changes in v3:
>
> - Make NET_LWIP a Kconfig choice in patch "net: introduce alternative
> implementation as net-lwip/" (Tom R.)
> - Drop the patch introducing lwIP as a Git subtree and document the git
> command in the cover letter instead (Tom R.)
> - "net-lwip: add TFTP support and tftpboot command": use the same
> "Bytes transferred =" message as in the legacy implementation (Tom R.,
> Maxim U.)
> - Drop "test/py: net: add
Re: Request for hosting a boot-firmware repository in u-boot git (denx and GitHub)
Hi Peter, On Fri, 21 Jun 2024 at 01:35, Peter Robinson wrote: > > On Fri, 21 Jun 2024 at 00:05, Simon Glass wrote: > > > > Hi Nishanth, > > > > On Thu, 20 Jun 2024 at 15:35, Nishanth Menon wrote: > > > > > > Hi Team, > > > > > > We have briefly discussed this topic on IRC[1]. I would like to > > > propose a new boot-firmware repository similar to the Linux-firmware > > > repository under the aegis of u-boot hosting. > > > > > > In addition to TI, it looks like some NXP[2] and Rockchip[3] > > > platforms seem to require additional closed-source/open-source > > > binaries to have a complete bootable image. Distribution rights and > > > locations of these binaries are challenging, and there needs to be a > > > standard for how and where they are hosted for end users. > > > > > > Further, looking ahead to future architectures: > > > * IP firmware: More and more IP vendors are embedding their own > > > "specialized controllers" and require firmware for the operation > > > (similar to Rockchip's DDR controller, I guess), > > > * boot stage firmware: Additional stages of the boot process involve > > > vendor intermediate firmware, such as power configuration. > > > * Security enclave binaries: While I see a few folks trying to have an > > > open-source s/w architecture, many PKA and PQC systems still require > > > prop binaries for IP reasons. > > > > > > NOTE: I am not judging any company(including TI) for reasons why some > > > firmware is proprietary, but I hate to have the end users and other > > > system (distro) maintainers have to deal with hell trying to make the > > > life of end users easy to live with. > > > > > > In the case of TI's K3 architecture devices, we have two binary blobs > > > that are critical for the boot process. > > > > > > 1. TIFS Firmware / DMSC firmware[4]—This is the security enclave > > > firmware. It is often encrypted, and sources are not public (due to > > > various business/regulatory reasons). > > > 2. DM Firmware[5] - There is a source in public in some cases and > > > binary only in others - essentially limited function binary to be > > > put up in the device management uC. In cases where the source is > > > available, the build procedure is, in my personal opinion, pretty > > > arcane, and even though in theory it is practical, in practice, not > > > friendly - efforts are going to simplify it, even probably integrate > > > it with a more opensource ecosystem, but that is talking "look at the > > > tea leaves" stuff. > > > 3. Low Power Management (LPM) binaries: tifs stub: another encrypted > > > binary that gives the tifs system context restore logic before > > > retrieving tifs firmware and a corresponding DM restoration binary. > > > > > > All told, this is not unlike the situation that necessitated the > > > creation of a Linux firmware repository. > > > > > > Options that I see: > > > > > > 1. Let the status quo be - SoC vendors maintain random locations and > > > random rules to maintain boot firmware. > > > 2. Ask Linux-firmware to host the binaries in a single canonical > > > location > > > 3. Host a boot-firmware repository - u-boot repo may be the more > > > logical location. > > > > > > * (1) isn't the correct answer. > > > > > > * (2) Though I haven't seen any policy from the Linux-firmware > > > community mandating anything of the form, the binaries we are talking > > > of may not belong to Linux-firmware as they aren't strictly speaking > > > something Linux kernel will load (since the bootloader has that > > > responsibility), and in some cases may not even directly talk to > > > (security enclave or DDR firmware stuff). I am adding Josh to this > > > mail to see if he has any opinions on the topic (but keeping > > > from cross posting on linux-firmware list, unless folks feel it is > > > OK). > > > > > > On (3): > > > Proposal: > > > > > > * Create a boot firmware repository in Denx and/or GitHub (if > > > financials are a hurdle, I hope we can solve it as a community). > > > * Limit binaries only to those consumed part of the u-boot scope. > > > > > > * Limit binaries only to those that do not have an opensource project > > > (Trusted Firmware-A/M, OP-TEE, etc..) or depend entirely on vendor > > > source or are binary only in nature (subject to licensing terms below) > > > * Limit binaries to some pre-established size to prevent repository > > > explosion - say, 512Kib? > > > * Follow the same rules of integration and licensing guidelines as > > > Linux-firmware[6]. > > > * Similar rules as Linux-firmware guidelines of ABI backward and > > > forward compatibility. > > > * Set a workflow update flow and a compatibility requirements document > > > > > > If we agree to have boot firmware under the stewardship of u-boot, we > > > should also set other rules, which is excellent to discuss. > > > > > > Thoughts? > > > > I suggest: > > > > 4) Add a 'binman blob' subcommand which can fetch
Re: [PATCH v3 07/18] binman: Keep the efi_capsule input file
Hi Sughosh,
On Fri, 21 Jun 2024 at 00:14, Sughosh Ganu wrote:
>
> On Fri, 21 Jun 2024 at 04:36, Simon Glass wrote:
> >
> > There is no need to remove input files. It makes it harder to diagnose
> > failures. Keep the payload file.
> >
> > There is no test for this condition, but one could be added.
> >
> > Signed-off-by: Simon Glass
> > Fixes: b617611b27a ("binman: capsule: Add support for generating...")
> > ---
>
> Acked-by: Sughosh Ganu
>
> Although I don't think this warrants a Fixes tag. It is not fixing any
> issue as such.
I'm OK to drop it if you like. Binman is designed to keep its
intermittent files in the output dir. We have talked about having a
separate dir for 'intermediate' files to avoid cluttering the output
dir, but have not done this yet.
Regards,
Simon
>
> -sughosh
>
> >
> > (no changes since v1)
> >
> > tools/binman/etype/efi_capsule.py | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/tools/binman/etype/efi_capsule.py
> > b/tools/binman/etype/efi_capsule.py
> > index 47da5da324b..03e55cbc4d9 100644
> > --- a/tools/binman/etype/efi_capsule.py
> > +++ b/tools/binman/etype/efi_capsule.py
> > @@ -148,7 +148,6 @@ class Entry_efi_capsule(Entry_section):
> > self.fw_version,
> > self.oem_flags)
> > if ret is not None:
> > -os.remove(payload)
> > return tools.read_file(capsule_fname)
> > else:
> > # Bintool is missing; just use the input data as the output
> > --
> > 2.34.1
> >
Re: [PATCH 2/2] doc: develop: testing: Fix reference to test writing section
On Fri, 21 Jun 2024 at 01:47, Alexander Dahl wrote:
>
> Fixes: fc3283314539 ("doc: Explain briefly how to write new tests")
> Signed-off-by: Alexander Dahl
> ---
> doc/develop/testing.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass
Please do always add a commit message
>
> diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst
> index 056bdfd2b69..9114d11ad26 100644
> --- a/doc/develop/testing.rst
> +++ b/doc/develop/testing.rst
> @@ -123,7 +123,7 @@ or is covered sparingly. So here are some suggestions:
>is much easier to add onto a test - writing a new large test can seem
>daunting to most contributors.
>
> -See doc:`tests_writing` for how to write tests.
> +See :doc:`tests_writing` for how to write tests.
>
>
> Future work
> --
> 2.39.2
>
Re: [PATCH 1/2] doc: develop: testing: Fix broken reference to pytest suite help
On Fri, 21 Jun 2024 at 01:47, Alexander Dahl wrote:
>
> This should have been adapted directly with or after
> v2021.01-693-gca6583d4e08 ("doc: move test/README to HTML
> documentation") or v2021.01-694-g0157619d5c8 ("doc: move
> test/py/README.md to HTML documentation") already.
>
> Signed-off-by: Alexander Dahl
> ---
> doc/develop/testing.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Simon Glass
Re: [PATCH 5/9] fdt: Correct condition for bloblist existing
Hi Ilias,
On Wed, 19 Jun 2024 at 06:41, Ilias Apalodimas
wrote:
>
> Hi Simon,
>
> On Wed, Jun 12, 2024 at 02:24:31PM -0600, Simon Glass wrote:
> > Hi Ilias,
> >
> > On Wed, 12 Jun 2024 at 00:02, Ilias Apalodimas
> > wrote:
> > >
> > > [...]
> > >
> > > > > > >> ---
> > > > > > >>
> > > > > > >> lib/fdtdec.c | 12 ++--
> > > > > > >> 1 file changed, 10 insertions(+), 2 deletions(-)
> > > > > > >>
> > > > > > >> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> > > > > > >> index b2c59ab3818..b141244e3b9 100644
> > > > > > >> --- a/lib/fdtdec.c
> > > > > > >> +++ b/lib/fdtdec.c
> > > > > > >> @@ -1669,8 +1669,16 @@ int fdtdec_setup(void)
> > > > > > >> {
> > > > > > >> int ret = -ENOENT;
> > > > > > >>
> > > > > > >> - /* If allowing a bloblist, check that first */
> > > > > > >> - if (CONFIG_IS_ENABLED(BLOBLIST)) {
> > > > > > >> + /*
> > > > > > >> +* If allowing a bloblist, check that first. This would
> > > > > > >> be better
> > > > > > >> +* handled with an OF_BLOBLIST Kconfig, but that caused
> > > > > > >> far too much
> > > > > > >> +* argument, so add a hack here, used e.g. by
> > > > > > >> chromebook_coral
> > > > > > >
> > > > > > > I am a bit confused by this comment - It means you will not use
> > > > > > > OF_BLOBLIST,
> > > > > > > but actually you are using it below. Is it a typo?
> > > > > >
> > > > > > Basically it would be cleaner to have a separate, phase-specific
> > > > > > Kconfig control as to whether the DT can come from the bloblist (I
> > > > > > can't remember the Kconfig name I suggested, nor the patch as it was
> > > > > > last year sometime). But for now I am adding this hack to get a few
> > > > > > boards working again.
> > > > >
> > > > > I am a bit confused.
> > > > > First of all the comment is innapropriate. We went through a lengthy
> > > > > discussion on BLOBLIST/OF_BLOSLIST etc and, even Tom chimed in and we
> > > > > made up our minds. Why are you adding this comment now? Why do code
> > > > > comments have to illustrate your personal opinion -- which was
> > > > > rejected?
> > > >
> > > > I'm sorry for the tone of the comment. I am not trying to offend
> > > > anyone here and I'm happy to change the language.
> > >
> > > Yes please a comment explaining why that piece of code is there is far
> > > more intuitive.
> >
> > OK, once we have agreed the below I can do that.
> >
> > >
> > > > As I probably
> > > > mentioned at the time, my accepted patch breaks my workflow and
> > > > several boards. I hope you can understand how frustrating that sort of
> > > > thing can be.
> > >
> > > Yes, I do and I am fine with a short-term patch that fixes issues, as
> > > long as its not too intrusive. There is a very thin line between quick
> > > and dirty fixes to spaghetti unreadable code. But we should have
> > > comments and/or commit messages indicating that this needs a proper
> > > fix
> >
> > I spent a lot of time explaining this last time.
> >
> > >
> > > > Also, now that I have my lab back up and running and I
> > > > would like these boards to work again on mainline!
> > > >
> > > > >
> > > > > Grepping for OF_BLOBLIST, I can't find any matches, so is the above
> > > > > if a typo?
> > > >
> > > > Remember, it was a patch you rejected :-)
> > >
> > > I don't maintain any of that. I only gave some feedback along the
> > > lines of "bloblist was designed to be auto-discoverable, I don't see
> > > how adding an explicit Kconfig helps". IIRC we eventually followed
> > > what Tom suggested.
> >
> > I'm not trying to point the finger here. So far the boards are broken
> > in mainline...I'm just trying to fix that,
> >
> > >
> > > In any case, the amount of bike-shedding in the topic is too much. Do
> > > you mind explaining the problem in your workflow again? Perhaps we can
> > > find a solution that is integrated in bloblist_maybe_init() instead of
> > > injecting ifs on when a bloblist should or should not be searched for
> > > all over the place.
> >
> > TPL (or SPL) sets up a bloblist with bits of info in it, but no DT
> > (which is in memory-mapped SPI flash)
> > U-Boot proper starts up, it wants to get the bloblist but not hang if
> > the bloblist doesn't have a DT
>
> Sure, that's reasonable and IIRC that's the design we agreed a while back.
> Looking at the code, if the DT isn't found in a bloblist and the feature is
> enabled, we don't crash. We just print a debug message and continue to find
> the DT as we used. Why do we have to skip the call to
> bloblist_maybe_init()?
Because at this point, if there is no bloblist, then it needs to be
created. But creating it this early may fail, e.g. since there is no
malloc(). The intent of this code is to locate an FDT from an existing
bloblist. There is no point in creating a new bloblist here, since it
obviously won't have an FDT in it.
Regards,
Simon
Re: [PATCH] RFC: Add a tag for the world builds
Hi Tom, On Thu, 20 Jun 2024 at 17:15, Tom Rini wrote: > > On Thu, Jun 20, 2024 at 05:05:26PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Thu, 20 Jun 2024 at 08:39, Tom Rini wrote: > > > > > > On Thu, Jun 20, 2024 at 07:33:46AM -0600, Simon Glass wrote: > > > > > > > Currently the world builds run on all runners, including faster and > > > > slower ones. > > > > > > > > The difference can be quite dramatic, with some builders 4x as fast as > > > > others, resulting in just one world build taking between 20 minutes and > > > > an hour and 20 minutes. > > > > > > > > Add a tag so that we can select which builders run these CPU-intensive > > > > jobs. > > > > > > > > With this tag we can also increase CPU utilisation by running multiple > > > > QEMU tests in parallel. Currently these tests leave most machines fairly > > > > idle, since we cannot run more than one world build on a machine. > > > > > > > > Signed-off-by: Simon Glass > > > > > > This conflicts I think with Jiaxun's desire to make our GitLab job > > > runnable on the public runners too, and where we'll end up with 10 world > > > build jobs ala Azure. > > > > It probably doesn't actually conflict, although I am not sure if one > > can add a tag to jobs that run on public runners. > > I mean conceptually at least as it will likely be slower to build the > world as 10 jobs than as 4 jobs. Yes I've noticed that the per-board build time reduces as the number of boards increases. For example moa varies between about 0.53 builds per second (for ~100 boards), to 0.59 (for ~400 or ~600). But there are 64 threads, so we need at least a few hundred to get to a steady state. Regards, Simon > > -- > Tom
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Tom, On Thu, 20 Jun 2024 at 17:19, Tom Rini wrote: > > On Thu, Jun 20, 2024 at 05:05:29PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Wed, 19 Jun 2024 at 09:32, Tom Rini wrote: > > > > > > On Tue, Jun 18, 2024 at 09:03:37PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Tue, 18 Jun 2024 at 08:15, Tom Rini wrote: > > > > > > > > > > On Tue, Jun 18, 2024 at 06:43:51AM -0600, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Mon, 17 Jun 2024 at 11:16, Tom Rini wrote: > > > > > > > > > > > > > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hi Heinrich > > > > > > > > > > > > > > > > > > resending the reply, I accidentally sent half of the > > > > > > > > > message... > > > > > > > > > > > > > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > > > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > > > > > > > wrote: > > > > > > > > > > >> > > > > > > > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > > > > > > > >>> Hi Simon, > > > > > > > > > > >>> > > > > > > > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass > > > > > > > > > > >>> wrote: > > > > > > > > > > > > > > > > > > > > It does not make sense to enable all SHA algorithms > > > > > > > > > > unless they are > > > > > > > > > > needed. It bloats the code and in this case, causes > > > > > > > > > > chromebook_link to > > > > > > > > > > fail to build. That board does use the TPM, but not > > > > > > > > > > with measured boot, > > > > > > > > > > nor EFI. > > > > > > > > > > > > > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these options, > > > > > > > > > > we just need to > > > > > > > > > > add them to MEASURED_BOOT as well. > > > > > > > > > > > > > > > > > > > > Note that the original commit combines refactoring and > > > > > > > > > > new features, > > > > > > > > > > which makes it hard to see what is going on. > > > > > > > > > > > > > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > > > > > > > - Consider MEASURED_BOOT too > > > > > > > > > > > > > > > > > > > > boot/Kconfig | 4 > > > > > > > > > > lib/Kconfig | 4 > > > > > > > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > > > > > > index 6f3096c15a6..b061891e109 100644 > > > > > > > > > > --- a/boot/Kconfig > > > > > > > > > > +++ b/boot/Kconfig > > > > > > > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > > > > > > > config MEASURED_BOOT > > > > > > > > > > bool "Measure boot images and configuration > > > > > > > > > > when booting without EFI" > > > > > > > > > > depends on HASH && TPM_V2 > > > > > > > > > > + select SHA1 > > > > > > > > > > + select SHA256 > > > > > > > > > > + select SHA384 > > > > > > > > > > + select SHA512 > > > > > > > > > > help > > > > > > > > > > This option enables measurement of the > > > > > > > > > > boot process when booting > > > > > > > > > > without UEFI . Measurement involves > > > > > > > > > > creating cryptographic hashes > > > > > > > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > > > > > > > index 189e6eb31aa..568892fce44 100644 > > > > > > > > > > --- a/lib/Kconfig > > > > > > > > > > +++ b/lib/Kconfig > > > > > > > > > > @@ -438,10 +438,6 @@ config TPM > > > > > > > > > > bool "Trusted Platform Module (TPM) Support" > > > > > > > > > > depends on DM > > > > > > > > > > imply DM_RNG > > > > > > > > > > - select SHA1 > > > > > > > > > > - select SHA256 > > > > > > > > > > - select SHA384 > > > > > > > > > > - select SHA512 > > > > > > > > > > >>> > > > > > > > > > > >>> I am not sure this is the right way to deal with your > > > > > > > > > > >>> problem. > > > > > > > > > > >>> The TPM main functionality is to measure and extend > > > > > > > > > > >>> PCRs, so sha > > > > > > > > > > >>> is really required. To make things even worse, you > > > > > > > > > > >>> don't know the PCR > > > > > > > > > > >>> banks that are enabled beforehand. This is a runtime > > > > > > > > > >
Re: [PATCH 3/5] buildman: Support building within a Python venv
Hi Tom,
On Thu, 20 Jun 2024 at 17:30, Tom Rini wrote:
>
> On Thu, Jun 20, 2024 at 05:05:30PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 20 Jun 2024 at 08:32, Tom Rini wrote:
> > >
> > > On Thu, Jun 20, 2024 at 07:19:35AM -0600, Simon Glass wrote:
> > >
> > > > The Python virtualenv tool sets up a few things in the envronment,
> > > > putting its path first in the PATH environment variable and setting up
> > > > a sys.prefix different from the sys.base_prefix value.
> > > >
> > > > At present buildman puts the toolchain path first in PATH so that it can
> > > > be found easily during the build. For sandbox this causes problems since
> > > > /usr/bin/gcc (for example) results in '/usr/bin' being prepended to the
> > > > PATH variable. As a result, the venv is partially disabled.
> > > >
> > > > The result is that sandbox builds within a venv ignore the venv, e.g.
> > > > when looking for packages.
> > > >
> > > > Correct this by detecting the venv and adding the toolchain path after
> > > > the venv path.
> > > >
> > > > Signed-off-by: Simon Glass
> > >
> > > Why are we using PATH at all in this case? Shouldn't we just be setting
> > > CROSS_COMPILE=/full/path/to/the/prefix ?
> >
> > This is the -p option to buildman. The original commit was:
> >
> > commit bb1501f2c22c979961b735db775605cccedd98f6
> > Author: Simon Glass
> > Date: Mon Dec 1 17:34:00 2014 -0700
> >
> > buildman: Add an option to use the full tool chain path
> >
> > In some cases there may be multiple toolchains with the same name in the
> > path. Provide an option to use the full path in the CROSS_COMPILE
> > environment variable.
> >
> > Note: Wolfgang mentioned that this is dangerous since in some cases
> > there
> > may be other tools on the path that are needed. So this is set up as an
> > option, not the default. I will need test confirmation (i.e. that this
> > commit fixes a real problem) before merging it.
> >
> > As to why we don't always do this, well that is back in the mists of
> > time, 10 years ago.
> >
> > BTW, this is raising a point ("let's change the behaviour") separate
> > from the goal of this commit, which is to fix a problem with venv,
> > albeit that if we made -p the only option, then we could potentially
> > drop all PATH changes. Perhaps toolchains are built differently now,
> > such that they always invoke their tools using the same prefix and
> > dir?
>
> Wait, I'm confused. buildman internally updates its own PATH to avoid
> calling CROSS_COMPILE with the full path due to a concern about
> toolchain bugs?
Not its own PATH: the one it passes to U-Boot's 'make'.
I'm not sure why, actually. It is such a long time ago that I don't remember.
I see:
~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-ld
and
~/.buildman-toolchains/gcc-13.2.0-nolibc/arm-linux-gnueabi/arm-linux-gnueabi/bin/ld
but interestingly there is no gcc in the latter directory, which there
was in 4.6 (and presumably for some time after).
Certainly for sandbox there is no prefix, so we cannot add it in that
case, and sandbox is actually the arch used to run these tests.
What are you suggesting we change about this patch?
Regards,
Simon
Re: [PATCH v3 10/18] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Ilias, On Thu, 20 Jun 2024 at 23:49, Ilias Apalodimas wrote: > > On Fri, 21 Jun 2024 at 08:32, Ilias Apalodimas > wrote: > > > > Hi Simon, > > > > On Fri, 21 Jun 2024 at 02:06, Simon Glass wrote: > > > > > > It does not make sense to enable all SHA algorithms unless they are > > > needed. It bloats the code and in this case, causes chromebook_link to > > > fail to build. That board does use the TPM, but not with measured boot, > > > nor EFI. > > > > > > Since EFI_TCG2_PROTOCOL already selects these options, we just need to > > > add them to MEASURED_BOOT as well. > > > > > > Note that the original commit combines refactoring and new features, > > > which makes it hard to see what is going on. > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > Reviewed-by: Heinrich Schuchardt > > > Signed-off-by: Simon Glass > > > --- > > > > > > (no changes since v2) > > > > There was a discussion in the previous version, bout enabling these on > > CMD_TPM as they are required. For reference [1] > > Or switch this to an imply instead so you can disable it ? The thing is, we need to be able to easily disable unwanted features. My reading of that thread is that U-Boot doesn't use SHA384/512 from the 'tpm2 pcr_extend' command today. Even if it did, I would think it better to allow the supported algorithms to be controlled. At worst, I would expect a separate option to disable that subcommand which would remove the implies. Some tpm code recently added puts a table and some functions in tpm-v2.h - could someone fix that? tpm-common.c would be a better place. Also we already have a list of hash algorithms in U-Boot (common/hash.c) so should use that, and respect the fact that certain algos may not be supported. If there is a requirement that (for a certain situation / protocol) we need to support everything, then let's have a Kconfig for that. It is what my patch was trying to work around, perhaps. For boards which don't care about extending TPM in this way, or don't use the more expensive algorithms, it avoids lots of code bloat. One of the principles of U-Boot is to allow configurability in these situations. It shouldn't be too hard to have the best of both worlds (supports all reasonable cases by default / allow unwanted cases to be dropped). So I'll await your reply on the above before figuring out where to go with this. For this release I'd just like to get the board working, but I understand that the TPM stuff is very-much in flux, perhaps in -next. Regards, Simon [1] https://patchwork.ozlabs.org/project/uboot/patch/20240610145920.3302001-3-...@chromium.org/
Re: Request for hosting a boot-firmware repository in u-boot git (denx and GitHub)
On June 20, 2024 thus sayeth Peter Robinson: > Hi Nishanth, > > Thanks for starting this conversation. > > > We have briefly discussed this topic on IRC[1]. I would like to > > propose a new boot-firmware repository similar to the Linux-firmware > > repository under the aegis of u-boot hosting. > > > > In addition to TI, it looks like some NXP[2] and Rockchip[3] > > platforms seem to require additional closed-source/open-source > > binaries to have a complete bootable image. Distribution rights and > > locations of these binaries are challenging, and there needs to be a > > standard for how and where they are hosted for end users. > > Yes, it's been painful for some time, and often the distribution is > unclear because often the binaries are copies around between SBC > makers often without the other bits such as licenses etc so it's a > case of "who knows ¯\_(ツ)_/¯" even if it's the same for all devices of > a particular SoC. > > Having a process around licensing and redistribution requirements is useful. > > > Further, looking ahead to future architectures: > > * IP firmware: More and more IP vendors are embedding their own > > "specialized controllers" and require firmware for the operation > > (similar to Rockchip's DDR controller, I guess), > > * boot stage firmware: Additional stages of the boot process involve > > vendor intermediate firmware, such as power configuration. > > * Security enclave binaries: While I see a few folks trying to have an > > open-source s/w architecture, many PKA and PQC systems still require > > prop binaries for IP reasons. > > > > NOTE: I am not judging any company(including TI) for reasons why some > > firmware is proprietary, but I hate to have the end users and other > > system (distro) maintainers have to deal with hell trying to make the > > life of end users easy to live with. > > > > In the case of TI's K3 architecture devices, we have two binary blobs > > that are critical for the boot process. > > > > 1. TIFS Firmware / DMSC firmware[4]—This is the security enclave > > firmware. It is often encrypted, and sources are not public (due to > > various business/regulatory reasons). > > 2. DM Firmware[5] - There is a source in public in some cases and > > binary only in others - essentially limited function binary to be > > put up in the device management uC. In cases where the source is > > available, the build procedure is, in my personal opinion, pretty > > arcane, and even though in theory it is practical, in practice, not > > friendly - efforts are going to simplify it, even probably integrate > > it with a more opensource ecosystem, but that is talking "look at the > > tea leaves" stuff. > > 3. Low Power Management (LPM) binaries: tifs stub: another encrypted > > binary that gives the tifs system context restore logic before > > retrieving tifs firmware and a corresponding DM restoration binary. > > > > All told, this is not unlike the situation that necessitated the > > creation of a Linux firmware repository. > > > > Options that I see: > > > > 1. Let the status quo be - SoC vendors maintain random locations and > > random rules to maintain boot firmware. > > 2. Ask Linux-firmware to host the binaries in a single canonical > > location > > I don't believe this makes sense. linux-firmware is designed for > firmware that is loaded by linunx drivers, these are not, they're > generally used a part of early boot firmware prior to the linux kernel > even booting, and could in fact be any other OS. As it is > linux-firmware is 2Gb+ in size, as a maintainer of that package in > Fedora having to determine which of 1000s of firmware is used by linux > or something else or if we need to ship it is hard enough already > without throwing a whole raft of other FW into the mix. I think a > separate repo is the right way to go here please. > > Agree! While consolidating all of these binaries together under denx.de or linux-firmware is needed, it doesn't make much sense to put this in a linux-firmware like repo. Unlike linux-firmware we don't need all of these binaries on the board (another side-effect of putting these u-boot binaries in linux-firmware), only the tiniest of subsets will be used for a specific board build. My hope was we could use some type of distribution mirroring system like most distributions use for their binaries today with some central routing for all the build machines consuming these blobs. This has the benefit of allowing vendors to help out with hosting and bandwidth if that's a problem and gives denx.de the ability to secure the software supply chain. Using a system like this does add to the complexity of an already complex setup/build system for u-boot. But we can do much better than a signed git repo ~Bryan > > 3. Host a boot-firmware repository - u-boot repo may be the more > > logical location. > > > > * (1) isn't the correct answer. > > > > * (2) Though I haven't seen any policy from
Re: [PATCH] imx8mp-venice-gw702x: Drop EQos clock workaround
Hi Tim,
On Wed, Jun 19, 2024 at 10:30 PM Tim Harvey wrote:
>
> The assigned-clock no longer have to be dropped, the clock are now
> defined in clk-imx8mp.c and used by DWMAC driver to configure the
> DWMAC clock. Drop the workarounds from U-Boot specific DT extras.
>
> Having the clocks dropped causes the EQoS to be non-functional.
>
> See commit c7ea9612df0f ("arm64: dts: imx8mp: Drop EQoS clock
> workaround").
>
> Fixes: 48c6f9777cee ("board: gateworks: venice: add imx8mp-gw7905-2x support")
> Signed-off-by: Tim Harvey
Is this targetted for master or next?
