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.
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 ?
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. 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 ?
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
- 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.