[PATCH v1] fastboot: introduce 'oem board' subcommand

[PATCH v1] fastboot: introduce 'oem board' subcommand

[Alexey Romanov]

Currently, fastboot protocol in U-Boot has no opportunity
to execute vendor custom code with verifed boot. 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 <avromanov@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
-- 
2.30.1

Re: [PATCH v1] fastboot: introduce 'oem board' subcommand

[Sean Anderson]

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?

--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 <avromanov@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

Re: [PATCH v1] fastboot: introduce 'oem board' subcommand

[Alexey Romanov]

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.

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.

> 
> --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 <avromanov@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

Re: [PATCH v1] fastboot: introduce 'oem board' subcommand

[Sean Anderson]

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.

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

--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 <avromanov@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
>> 
> 

Re: [PATCH v1] fastboot: introduce 'oem board' subcommand

[Alexey Romanov]

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.

> 
> > 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(...)

  ...

> 
> --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 <avromanov@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

Re: [PATCH v1] fastboot: introduce 'oem board' subcommand

[Alexey Romanov]

Sorry, my e-mail client is lagging and I sent two replies.

On Wed, Jan 10, 2024 at 08:03:39AM +0000, Alexey Romanov 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.
> 
> > 
> > > 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(...)
> 
>   ...
> 
> > 
> > --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 <avromanov@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

Re: [PATCH v1] fastboot: introduce 'oem board' subcommand

[Mattijs Korpershoek]

Hi Alexey, Sean,

On mer., janv. 10, 2024 at 08:03, Alexey Romanov <avromanov@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?

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 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 <avromanov@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

Re: [PATCH v1] fastboot: introduce 'oem board' subcommand

[Alexey Romanov]

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 <avromanov@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-avromanov@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 <avromanov@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

Re: [PATCH v1] fastboot: introduce 'oem board' subcommand

[Alexey Romanov]

On Thu, Jan 11, 2024 at 10:14:45AM +0000, Alexey Romanov wrote:
> 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 <avromanov@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)
> > 

I think we need to abstract ourselves from my specific case.
OEM commands, logically, should be define at the board level (not at
the command or scripts level). Currently, U-Boot doesn't provide such an
opportunity; OEM cannot run custom code defined at board level with a verified boot.

This is the main idea of my patch.

source command is not suitable in all cases, in addition, it forces you
you to store the necessary (for firmware flashing) *.itb files at your
host.

> > >
> > >> 
> > >> > 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-avromanov@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 <avromanov@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

-- 
Thank you,
Alexey

Re: [PATCH v1] fastboot: introduce 'oem board' subcommand

[Sean Anderson]

On 1/10/24 03:03, Alexey Romanov 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?

Well, you do this however you would enable fastboot normally. So usually
you run fastboot if you main boot fails or if the user holds down a
button or something.  Same thing here.

> Yes, it seems like it will work. But the code for complex
> fastboot scripts will be complex and difficult to read.

If you write a (regular) command to call meson_bootloader_write_bl2 you
can do it in a script with imxtract. Although the latter command does
not check signatures. I have a patch for that, but I still need to write
tests for it.

> I think my version looks more elegant and simple.
> 
>> 
>> > 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(...)
> 

I'm a little curious why you do this via a custom command and not mkimage.

Anyway, I think fastboot already has a lot of features (especially for
something which is often used with secure boot enabled) so I think it is
good to try and use existing features if possible. TBH I would prefer to
promote the use of signed scripts to do this sort of thing, but I agree
that it can be a bit clunky. That said, this is something which can be
generated automatically as part of a build.

For v2 can you include the board command you want to run as a separate
patch? I think it would have sped up the discussion if there had been a
concrete example.

--Sean

>> 
>> --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 <avromanov@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
>> >> 
>> > 
>> 
>