Hi Mattijs, On Thu, Jan 11, 2024 at 10:50:26AM +0100, Mattijs Korpershoek wrote: > Hi Alexey, Sean, > > On mer., janv. 10, 2024 at 08:03, Alexey Romanov > <avroma...@salutedevices.com> wrote: > > > Hi, > > > > On Tue, Jan 09, 2024 at 10:45:46AM -0500, Sean Anderson wrote: > >> On 1/9/24 05:27, Alexey Romanov wrote: > >> > Hello Sean! > >> > > >> > Thanks for you reply. > >> > > >> > On Thu, Dec 28, 2023 at 11:45:04AM -0500, Sean Anderson wrote: > >> >> On 12/28/23 10:25, Alexey Romanov wrote: > >> >> > Currently, fastboot protocol in U-Boot has no opportunity > >> >> > to execute vendor custom code with verifed boot. > >> >> > >> >> Well, I would say the most conventional way to do this would be > >> >> something like > >> >> > >> >> => fastboot 0 > >> >> => source \# CONFIG_FASTBOOT_BUF_ADDR > >> >> > >> >> and on your host machine, > >> >> > >> >> $ fastboot stage my_script.itb > >> >> > >> >> where my_script.its looks like > >> >> > >> >> /dts-v1/; > >> >> > >> >> / { > >> >> description = "my script"; > >> >> #address-cells = <1>; > >> >> > >> >> images { > >> >> my-script { > >> >> data = /incbin/("my_script.scr"); > >> >> type = "script"; > >> >> arch = "arm64"; > >> >> compression = "none"; > >> >> hash-1 { > >> >> algo = "sha256"; > >> >> }; > >> >> }; > >> >> }; > >> >> > >> >> configurations { > >> >> default = "conf"; > >> >> conf { > >> >> description = "Load my script"; > >> >> script = "my-script"; > >> >> signature { > >> >> algo = "sha256,rsa2048"; > >> >> key-name-hint = "vboot"; > >> >> sign-images = "script"; > >> >> }; > >> >> }; > >> >> }; > >> >> }; > >> >> > >> >> This method is especially useful to pass complex parameters to your > >> >> command. > >> >> This method of course requires commit bcc85b96b5f ("cmd: source: Support > >> >> specifying config name"). > >> >> > >> >> Would it be possible to use the above method for your use case? > >> > > >> > This method sounds good for some cases, but we have encountered some > >> > problems that prevent us from applying it: > >> > > >> > 1. Looks like this method requires access to UART (for typing 'source' > >> > command). Open the UART is unsafe at devices with verified boot. > >> > >> Generally the idea is that you run source after you run fastboot. E.g. you > >> set > >> your altbootcmd (or whatever) to something like > >> > >> while true; fastboot 0; source \# || boot; done > >> > >> so you try to source whatever gets staged, and boot it otherwise. > > > > If I understand right, U-Boot always will try fastboot mode? > > Yes, it seems like it will work. But the code for complex > > fastboot scripts will be complex and difficult to read. > > > > I think my version looks more elegant and simple. > > I think your solution is elegant, but i'd like to make sure that the > problem cannot be solved otherwise (via environment changes or other things) > > > > >> > >> > 2. We use automation scripts to flash our devices using fastboot > >> > protocol. A typical example of flashing scripts (device is already in > >> > fastboot mode): > >> > > >> > $ fastboot erase bootloader > >> > $ fastboot stage bootloader.img > >> > $ fastboot oem board:write_bootloader > >> > > >> > $ fastboot reboot-bootloader > >> > > >> > $ fastboot erase super > >> > $ fastboot flash super super.bin > >> > > >> > $ fastboot reboot > >> > > >> > This method doesn't assume what something typing additional command in > >> > U-Boot shell, even if we have access to UART. > >> > >> I'm curious what you actual usage for this is? That is, what do you need > >> a custom command for. > > > > Our SoC vendor use custom scheme for flashing bootloader partition. > > So we can't just use fastbooot flash command - we have to use custom > > flashing logic. We also don't want to use hacks in generic fastboot > > code, for example something like this in _fb_nand_write(): > > > > ... > > > > if (!strcmp(part->name, "bootloader")) > > write_bootloader_custom_logic(...) > > else > > nand_write_skip_bad(...) > > Could you detail what "custom scheme" means?
We have a binary file that is uboot.bin combined with bl2.bin. This binary file must be written in a special way to the bootloader partition using helpers from following patchset: https://lore.kernel.org/all/20231228151421.82746-1-avroma...@salutedevices.com/ Our oem board implementation uses these functions with custom vendor flashing logic. Something like this: ... int write_bootloader(offset, partition_name) { int cpy_num = meson_bootloader_copy_num(partition_name); for (int i = 0; i < cpy_num; i++) if (!strcmp(partition_name, BOOT_BL2)) meson_bootloader_write_bl2(...); else nand_write_skip_bad(...); if (!strcmp(partition_name, BOOT_BL2)) meson_bootloader_write_info_pages(); } write_bootloader(offset, BOOT_BL2); write_bootloader(offset + BL2_SIZE, BOOT_TPL); ... > > Wouldn't using "fastboot_raw_partition_*" be appropriate for your use-case? > https://docs.u-boot.org/en/latest/android/fastboot.html#raw-partition-descriptors I'm not sure it will be possible to combine this with our implementation. > > I know the above is for mmc but maybe this can be extended for nand ? > > > > > ... > > > >> > >> --Sean > >> > >> >> > >> >> --Sean > >> >> > >> >> > This patch > >> >> > introduce new fastboot subcommand fastboot oem board:<cmd>, > >> >> > which allow to run custom oem_board function. > >> >> > = > >> >> > Default implementation is __weak. Vendor must redefine it in > >> >> > board/ folder with his own logic. > >> >> > > >> >> > For example, some vendors have their custom nand/emmc partition > >> >> > flashing or erasing. Here some typical command for such use cases: > >> >> > > >> >> > - flashing: > >> >> > > >> >> > $ fastboot stage bootloader.img > >> >> > $ fastboot oem board:write_bootloader > >> >> > > >> >> > - erasing: > >> >> > > >> >> > $ fastboot oem board:erase_env > >> >> > > >> >> > Signed-off-by: Alexey Romanov <avroma...@salutedevices.com> > >> >> > --- > >> >> > drivers/fastboot/Kconfig | 7 +++++++ > >> >> > drivers/fastboot/fb_command.c | 15 +++++++++++++++ > >> >> > include/fastboot.h | 1 + > >> >> > 3 files changed, 23 insertions(+) > >> >> > > >> >> > diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig > >> >> > index 3cfeea4837..4c955cabab 100644 > >> >> > --- a/drivers/fastboot/Kconfig > >> >> > +++ b/drivers/fastboot/Kconfig > >> >> > @@ -241,6 +241,13 @@ config FASTBOOT_OEM_RUN > >> >> > this feature if you are using verified boot, as it will allow > >> >> > an > >> >> > attacker to bypass any restrictions you have in place. > >> >> > > >> >> > +config FASTBOOT_OEM_BOARD > >> >> > + bool "Enable the 'oem board' command" > >> >> > + help > >> >> > + This extends the fastboot protocol with an "oem board" > >> >> > command. This > >> >> > + command allows running vendor custom code defined in board/ > >> >> > files. > >> >> > + Otherwise, it will do nothing and send fastboot fail. > >> >> > + > >> >> > endif # FASTBOOT > >> >> > > >> >> > endmenu > >> >> > diff --git a/drivers/fastboot/fb_command.c > >> >> > b/drivers/fastboot/fb_command.c > >> >> > index 71cfaec6e9..4d2b451f46 100644 > >> >> > --- a/drivers/fastboot/fb_command.c > >> >> > +++ b/drivers/fastboot/fb_command.c > >> >> > @@ -39,6 +39,7 @@ static void reboot_recovery(char *, char *); > >> >> > static void oem_format(char *, char *); > >> >> > static void oem_partconf(char *, char *); > >> >> > static void oem_bootbus(char *, char *); > >> >> > +static void oem_board(char *, char *); > >> >> > static void run_ucmd(char *, char *); > >> >> > static void run_acmd(char *, char *); > >> >> > > >> >> > @@ -106,6 +107,10 @@ static const struct { > >> >> > .command = "oem run", > >> >> > .dispatch = CONFIG_IS_ENABLED(FASTBOOT_OEM_RUN, > >> >> > (run_ucmd), (NULL)) > >> >> > }, > >> >> > + [FASTBOOT_COMMAND_OEM_BOARD] = { > >> >> > + .command = "oem board", > >> >> > + .dispatch = CONFIG_IS_ENABLED(FASTBOOT_OEM_BOARD, > >> >> > (oem_board), (NULL)) > >> >> > + }, > >> >> > [FASTBOOT_COMMAND_UCMD] = { > >> >> > .command = "UCmd", > >> >> > .dispatch = CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, > >> >> > (run_ucmd), (NULL)) > >> >> > @@ -489,3 +494,13 @@ static void __maybe_unused oem_bootbus(char > >> >> > *cmd_parameter, char *response) > >> >> > else > >> >> > fastboot_okay(NULL, response); > >> >> > } > >> >> > + > >> >> > +void __weak fastboot_oem_board(char *cmd_parameter, void *data, u32 > >> >> > size, char *response) > >> >> > +{ > >> >> > + fastboot_fail("oem board function not defined", response); > >> >> > +} > >> >> > + > >> >> > +static void __maybe_unused oem_board(char *cmd_parameter, char > >> >> > *response) > >> >> > +{ > >> >> > + fastboot_oem_board(cmd_parameter, fastboot_buf_addr, > >> >> > image_size, response); > >> >> > +} > >> >> > diff --git a/include/fastboot.h b/include/fastboot.h > >> >> > index 296451f89d..06c1f26b6c 100644 > >> >> > --- a/include/fastboot.h > >> >> > +++ b/include/fastboot.h > >> >> > @@ -37,6 +37,7 @@ enum { > >> >> > FASTBOOT_COMMAND_OEM_PARTCONF, > >> >> > FASTBOOT_COMMAND_OEM_BOOTBUS, > >> >> > FASTBOOT_COMMAND_OEM_RUN, > >> >> > + FASTBOOT_COMMAND_OEM_BOARD, > >> >> > FASTBOOT_COMMAND_ACMD, > >> >> > FASTBOOT_COMMAND_UCMD, > >> >> > FASTBOOT_COMMAND_COUNT > >> >> > >> > > >> > > > > -- > > Thank you, > > Alexey -- Thank you, Alexey