Re: [PATCH RFC] bootm: support printing bootm parameters determined at runtime to file

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Sascha,

On 10.01.24 16:18, Sascha Hauer wrote:
> On Wed, Jan 10, 2024 at 03:39:58PM +0100, Ahmad Fatoum wrote:
>> The barebox bootm command is often not called directly, but via
>> bootloader spec or FIT image boot handlers. For debugging, it can be
>> useful to reuse those boot handlers, but replace single artifacts, e.g.
>> using the barebox device tree instead of the bootloader-spec provided
>> device tree.
>>
>> To make this easier, have boot -v -d (verbose + dry run) write a boot
>> script that reproduces the cancelled boot to /env/boot/cancelled.
> 
> I like the idea very much. I also had this problem more than once.
> 
> I think an explicit option rather than "-v -d" would be better. With
> this there would be a natural way to document this behaviour in the
> boot help output.
> 
> /env/boot/cancelled would be saved in the environment with a saveenv
> which is likely not desirable. Maybe better put it somewhere in /tmp/?

Or have the new option take an argument, which would be the path to save
the file to?

> 
>> +	dprintf(fd, "global.bootm.verify=%s\n", verify);
>> +
>> +	dprintf(fd, "\n");
>> +
>> +	if (data->os_address != UIMAGE_SOME_ADDRESS)
>> +		dprintf(fd, "global.bootm.image.loadaddr=0x%08lx\n", data->os_address);
>> +	if (data->os_file)
>> +		dprintf(fd, "global.bootm.image='%s'\n", data->os_file);
>> +	if (data->oftree_file)
>> +		dprintf(fd, "global.bootm.oftree='%s'\n", data->oftree_file);
>> +#ifdef CONFIG_BOOTM_INITRD
>> +	if (data->initrd_file)
>> +		dprintf(fd, "global.bootm.initrd='%s'\n", data->initrd_file);
>> +
>> +	if (data->initrd_address != UIMAGE_INVALID_ADDRESS)
>> +		dprintf(fd, "global.bootm.initrd.loadaddr=0x%08lx\n", data->initrd_address);
>> +#endif
>> +	if (data->tee_file)
>> +		dprintf(fd, "global.bootm.tee_file='%s'\n", data->tee_file);
>> +
>> +	dprintf(fd, "\n");
>> +
>> +	bootargs = linux_bootargs_get();
>> +	if (bootargs)
>> +		dprintf(fd, "global linux.bootargs.dyn.concatenated='%s'\n", bootargs);
> 
> This will contain all of linux.bootargs.* which are then added again
> when we actually boot the script, so we end up having many bootargs
> twice. This is not very nice, I don't know though how/if we can improve
> that.

Yes. I could iterate over all linux.bootargs.* instead of adding a new variable.

Cheers,
Ahmad

> 
> Sascha
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |