On 6/20/23 11:17, Xavier Drudis Ferran wrote:
El Mon, Jun 19, 2023 at 11:49:18PM +0200, Marek Vasut deia:
On 6/19/23 12:12, Xavier Drudis Ferran wrote:
It seems the email addresses are being constantly corrupted in each email.
This time the ML address is wrong and missing an e at the end. There is some
e@ nonexistent address which I have to keep removing.
Yes, that's my fault. I'm sorry. I apologize to you and others. I
resent my mail with the proper address. Please just look for my mail
with the wrong address and delete it from your mail archive to prevent
further such problems. You can reply to the other mail I sent (June
19th), because it has the same content, just with an added
apology. Sorry again.
That's fine
When DISTRO_DEFAULTS is not set, the default environment has
bootcmd=bootflow
That is not right, on $randomboard I picked the bootcmd is something else.
And how default is your default environment for your $randomboard ?
Default, see:
$ git grep CONFIG_BOOTCOMMAND configs/
Almost half the configs/* redefine CONFIG_BOOTCOMMAND (524/1268)
When DISTRO_DEFAULTS is not set, that makes BOOTSTD_BOOTCOMMAND default to y
and the default for BOOTCMD is not "run distro_boootcmd", but "bootflow scan"
or (for sandbox) "bootflow scan -lb". When there's bootcmd at all.
But this is only the default for the default environment. It can be
overriden and the Kconfig is not exactly simple. An extract:
next branch:
arch/Arm/Kconfig:
config ARCH_ROCKCHIP
[...]
imply BOOTSTD_DEFAULTS
[...]
cmd/Kconfig:
config CMD_BOOTFLOW
bool "bootflow"
depends on BOOTSTD
default y
[...]
config CMD_BOOTFLOW_FULL
bool "bootflow - extract subcommands"
depends on BOOTSTD_FULL
default y
[...]
boot/Kconfig:
config BOOT_DEFAULTS
bool # Common defaults for standard boot and distroboot
imply USE_BOOTCOMMAND
[...]
config BOOTSTD
bool "Standard boot support"
default y
[...]
config BOOTSTD_FULL
bool "Enhanced features for standard boot"
default y if SANDBOX
[...]
if BOOTSTD
[...]
config BOOTSTD_DEFAULTS
bool "Select some common defaults for standard boot"
depends on BOOTSTD
imply USE_BOOTCOMMAND
select BOOT_DEFAULTS
select BOOTMETH_DISTRO
[...]
config BOOTSTD_BOOTCOMMAND
bool "Use bootstd to boot"
default y if !DISTRO_DEFAULTS
[...]
[...]
endif
[...]
config DISTRO_DEFAULTS
bool "Select defaults suitable for booting general purpose Linux
distributions"
select BOOT_DEFAULTS
[...]
config BOOTCOMMAND
string "bootcmd value"
depends on USE_BOOTCOMMAND && !USE_DEFAULT_ENV_FILE
default "bootflow scan -lb" if BOOTSTD_DEFAULTS && CMD_BOOTFLOW_FULL
default "bootflow scan" if BOOTSTD_DEFAULTS && !CMD_BOOTFLOW_FULL
default "run distro_bootcmd" if !BOOTSTD_BOOTCOMMAND && DISTRO_DEFAULTS
Does this happen if you set empty bootcmd ('=> setenv bootcmd 'echo hello'
for example), then 'saveenv' , then 'reset' , then drop into U-Boot shell
and run 'usb reset ; usb info' too ?
I haven't tested it. If bootflow scan is not run it might not happen.
So, what is the minimal test case ?
I have been asking for that for a while.
Someone has to hang the UCLASS_BOOTDEV on the usb mass storage device,
for it to fail. But as far as I know the idea is to make bootflow the
default in more and more cases. You'll always be able to avoid it
running in your board by setting your own environment at runtime or
changing the configuration, yes, but what's the point ?
I thought that failing one scenario was enough to fix things. When
one finds a bug it tries to help others to reproduce it. When others
help the bug finder to run other scenarios that don't have the bug,
what's that useful for ?
Note that it won't fail if the boot succeeds, because then you won't
have a shell to run usb info/tree. It won't fail if usb is not in
boot_targets. It won't fail if there's no mass storage device
connected to usb when bootflow scan is run...
But I still think the failing case is worth fixing. Someone may be
wondering why bootflow fails, run usb info and find a reset, when
setting up a new board, or trying to boot from the wrong usb stick
after the system partition has been corrupted, or whatever. It's not
something that breaks any board in production, but it's not something
to leave forever broken. In theory a null pointer dereference might be
used by some attacker, but in this case I don't really see any useful
attack, maybe it's my lack of imagination. So I'm not claiming it's a
severe bug. It's just a normal bug that needs fixing when possible.
Or are you trying to hint that the solution shouldn't be changing
cmd/usb.c but cleaning up the UCLASS_BOOTDEVs after bootflow scan
somehow ?
I would really like a minimal test case. Empty your env and figure out
the commands which need to be executed to trigger this. Without any
interference from other commands/scripting/...
Or I should change the commit message because the point is not so much
what's the default environment or the default default environment, but
simply that bootflow scan is run with an usb mass storage device
connected and no boot content present in any of the boot_targets
media, and then usb tree/info is run ?
If it's just that you can't reproduce it, can you try to ?:
- set up a board with no boot media (I tested like this but it might
not be needed),
- put usb in boot_targets (if you put only usb there you may not need
the previous step):
setenv boot_targets usb
Here you assume distro bootcommand or some such . Can we remove that
assumption ? (I think yes, and we should)
- plug a non-booting usb mass storage device to an usb port,
- run usb reset in case you already had usb initialized at boot, or
skip this if usb is not initialized yet. If in doubt, run usb reset.
- run bootflow scan
- run usb info
It should list some devices, but give you a reset just after listing the
usb mass storage device without my patch, and it should just list all
usb devices and go back to the prompt with my patch.
Does it crash if you empty your env and run simply
=> usb reset ; bootflow scan ; usb info
?
