[PATCH v2] fastboot: Add OEM run command

[PATCH v2] fastboot: Add OEM run command

[Sean Anderson]

This adds the UUU UCmd functionality as an OEM command. While the
fastboot tool allows sending arbitrary commands as long as they are
prefixed with "oem". This allows running generic U-Boot commands over
fastboot without UUU, which is especially useful when not using USB.
This is really the route we should have gone in the first place when
adding these commands.

While we're here, clean up the Kconfig a bit.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

Changes in v2:
- Document usage
- Keep enum in order

 doc/android/fastboot.rst      | 15 +++++++++++++++
 drivers/fastboot/Kconfig      | 10 +++++-----
 drivers/fastboot/fb_command.c |  4 ++++
 include/fastboot.h            |  1 +
 4 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
index 7611f07038..7f343d5dfc 100644
--- a/doc/android/fastboot.rst
+++ b/doc/android/fastboot.rst
@@ -28,6 +28,7 @@ The following OEM commands are supported (if enabled):
 - ``oem partconf`` - this executes ``mmc partconf %x <arg> 0`` to configure eMMC
   with <arg> = boot_ack boot_partition
 - ``oem bootbus``  - this executes ``mmc bootbus %x %s`` to configure eMMC
+- ``oem run`` - this executes an arbitrary U-Boot command
 
 Support for both eMMC and NAND devices is included.
 
@@ -127,6 +128,19 @@ Boot command
 When executing the fastboot ``boot`` command, if ``fastboot_bootcmd`` is set
 then that will be executed in place of ``bootm <CONFIG_FASTBOOT_BUF_ADDR>``.
 
+Running Shell Commands
+----------------------
+
+Normally, arbitrary U-Boot command execution is not enabled. This is so
+fastboot can be used to update systems using verified boot. However, such
+functionality can be useful for production or when verified boot is not in use.
+Enable ``CONFIG_FASTBOOT_UUU_SUPPORT`` to use this functionality. This will
+enable the ``UCmd`` and ``ACmd`` commands for use with UUU [3]_. It also
+enables the ``oem run`` command, which can be used with the fastboot client.
+For example, to print "Hello world", run::
+
+    $ fastboot oem run:echo Hello world
+
 Partition Names
 ---------------
 
@@ -232,3 +246,4 @@ References
 
 .. [1] :doc:`fastboot-protocol`
 .. [2] https://developer.android.com/studio/releases/platform-tools
+.. [3] https://github.com/NXPmicro/mfgtools
diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
index b97c67bf60..8f2d52cb8a 100644
--- a/drivers/fastboot/Kconfig
+++ b/drivers/fastboot/Kconfig
@@ -80,12 +80,12 @@ config FASTBOOT_FLASH
 	  this to enable the "fastboot flash" command.
 
 config FASTBOOT_UUU_SUPPORT
-	bool "Enable FASTBOOT i.MX UUU special command"
+	bool "Enable running arbitrary commands from FASTBOOT"
 	help
-	  The fastboot protocol includes "UCmd" and "ACmd" command.
-	  Be aware that you provide full access to any U-Boot command,
-	  including working with memory and may open a huge backdoor,
-	  when enabling this option.
+	  This extends the fastboot protocol with the "UCmd" and "ACmd"
+	  commands, as well as the "oem run" command.  These commands provide
+	  full access to any U-Boot command, including working with memory.
+	  This may open a huge backdoor if you are using verified boot.
 
 choice
 	prompt "Flash provider for FASTBOOT"
diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
index 98eccc3455..1732406c18 100644
--- a/drivers/fastboot/fb_command.c
+++ b/drivers/fastboot/fb_command.c
@@ -123,6 +123,10 @@ static const struct {
 	},
 #endif
 #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
+	[FASTBOOT_COMMAND_OEM_RUN] = {
+		.command = "oem run",
+		.dispatch = run_ucmd,
+	},
 	[FASTBOOT_COMMAND_UCMD] = {
 		.command = "UCmd",
 		.dispatch = run_ucmd,
diff --git a/include/fastboot.h b/include/fastboot.h
index 57daaf1298..8b6b4b934a 100644
--- a/include/fastboot.h
+++ b/include/fastboot.h
@@ -45,6 +45,7 @@ enum {
 	FASTBOOT_COMMAND_OEM_BOOTBUS,
 #endif
 #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
+	FASTBOOT_COMMAND_OEM_RUN,
 	FASTBOOT_COMMAND_ACMD,
 	FASTBOOT_COMMAND_UCMD,
 #endif
-- 
2.35.1.1320.gc452695387.dirty

Re: [PATCH v2] fastboot: Add OEM run command

[Marek Vasut]

On 12/2/22 22:03, Sean Anderson wrote:

[...]

> +Running Shell Commands
> +----------------------
> +
> +Normally, arbitrary U-Boot command execution is not enabled. This is so
> +fastboot can be used to update systems using verified boot. However, such
> +functionality can be useful for production or when verified boot is not in use.
> +Enable ``CONFIG_FASTBOOT_UUU_SUPPORT`` to use this functionality. This will
> +enable the ``UCmd`` and ``ACmd`` commands for use with UUU [3]_. It also
> +enables the ``oem run`` command, which can be used with the fastboot client.
> +For example, to print "Hello world", run::
> +
> +    $ fastboot oem run:echo Hello world
> +

Does this also support more complex shell constructs, e.g. run:"echo 
hello ; echo world" ? Or how does one represent the ; in that command ?

Does variable expansion and access work as it should ?

[...]

> diff --git a/include/fastboot.h b/include/fastboot.h
> index 57daaf1298..8b6b4b934a 100644
> --- a/include/fastboot.h
> +++ b/include/fastboot.h
> @@ -45,6 +45,7 @@ enum {
>   	FASTBOOT_COMMAND_OEM_BOOTBUS,
>   #endif
>   #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
> +	FASTBOOT_COMMAND_OEM_RUN,
>   	FASTBOOT_COMMAND_ACMD,

That OEM_RUN entry should be here, no ?

>   	FASTBOOT_COMMAND_UCMD,

[...]

Re: [PATCH v2] fastboot: Add OEM run command

[Sean Anderson]

On 12/2/22 16:22, Marek Vasut wrote:
> On 12/2/22 22:03, Sean Anderson wrote:
> 
> [...]
> 
>> +Running Shell Commands
>> +----------------------
>> +
>> +Normally, arbitrary U-Boot command execution is not enabled. This is so
>> +fastboot can be used to update systems using verified boot. However, such
>> +functionality can be useful for production or when verified boot is not in use.
>> +Enable ``CONFIG_FASTBOOT_UUU_SUPPORT`` to use this functionality. This will
>> +enable the ``UCmd`` and ``ACmd`` commands for use with UUU [3]_. It also
>> +enables the ``oem run`` command, which can be used with the fastboot client.
>> +For example, to print "Hello world", run::
>> +
>> +    $ fastboot oem run:echo Hello world
>> +
> 
> Does this also support more complex shell constructs, e.g. run:"echo hello ; echo world" ? Or how does one represent the ; in that command ?

Yes. 

> Does variable expansion and access work as it should ?

Yes. But it has to be escaped properly (for your shell).

Under the hood this just calls into run_command, and fastboot effectively
does ' '.join(argv) for oem commands.

TBH I haven't dug into this too much, as I just need to run some basic commands
for my intended use case.

--Sean

> 
> [...]
> 
>> diff --git a/include/fastboot.h b/include/fastboot.h
>> index 57daaf1298..8b6b4b934a 100644
>> --- a/include/fastboot.h
>> +++ b/include/fastboot.h
>> @@ -45,6 +45,7 @@ enum {
>>       FASTBOOT_COMMAND_OEM_BOOTBUS,
>>   #endif
>>   #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>> +    FASTBOOT_COMMAND_OEM_RUN,
>>       FASTBOOT_COMMAND_ACMD,
> 
> That OEM_RUN entry should be here, no ?
> 
>>       FASTBOOT_COMMAND_UCMD,
> 
> [...]

Re: [PATCH v2] fastboot: Add OEM run command

[Marek Vasut]

On 12/2/22 22:34, Sean Anderson wrote:
> On 12/2/22 16:22, Marek Vasut wrote:
>> On 12/2/22 22:03, Sean Anderson wrote:
>>
>> [...]
>>
>>> +Running Shell Commands
>>> +----------------------
>>> +
>>> +Normally, arbitrary U-Boot command execution is not enabled. This is so
>>> +fastboot can be used to update systems using verified boot. However, such
>>> +functionality can be useful for production or when verified boot is not in use.
>>> +Enable ``CONFIG_FASTBOOT_UUU_SUPPORT`` to use this functionality. This will
>>> +enable the ``UCmd`` and ``ACmd`` commands for use with UUU [3]_. It also
>>> +enables the ``oem run`` command, which can be used with the fastboot client.
>>> +For example, to print "Hello world", run::
>>> +
>>> +    $ fastboot oem run:echo Hello world
>>> +
>>
>> Does this also support more complex shell constructs, e.g. run:"echo hello ; echo world" ? Or how does one represent the ; in that command ?
> 
> Yes.
> 
>> Does variable expansion and access work as it should ?
> 
> Yes. But it has to be escaped properly (for your shell).
> 
> Under the hood this just calls into run_command, and fastboot effectively
> does ' '.join(argv) for oem commands.
> 
> TBH I haven't dug into this too much, as I just need to run some basic commands
> for my intended use case.

It would be nice to have an example of how to run

fastboot oem run:echo Hello ; echo world

or

fastboot oem run:echo Hello && echo world

or

fastboot oem run:setenv Hello world ; echo $Hello

that should cover all the cases.

Re: [PATCH v2] fastboot: Add OEM run command

[Patrick DELAUNAY]

Hi,

On 12/2/22 22:03, Sean Anderson wrote:
> This adds the UUU UCmd functionality as an OEM command. While the
> fastboot tool allows sending arbitrary commands as long as they are
> prefixed with "oem". This allows running generic U-Boot commands over
> fastboot without UUU, which is especially useful when not using USB.
> This is really the route we should have gone in the first place when
> adding these commands.
>
> While we're here, clean up the Kconfig a bit.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
> Changes in v2:
> - Document usage
> - Keep enum in order
>
>   doc/android/fastboot.rst      | 15 +++++++++++++++
>   drivers/fastboot/Kconfig      | 10 +++++-----
>   drivers/fastboot/fb_command.c |  4 ++++
>   include/fastboot.h            |  1 +
>   4 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
> index 7611f07038..7f343d5dfc 100644
> --- a/doc/android/fastboot.rst
> +++ b/doc/android/fastboot.rst
> @@ -28,6 +28,7 @@ The following OEM commands are supported (if enabled):
>   - ``oem partconf`` - this executes ``mmc partconf %x <arg> 0`` to configure eMMC
>     with <arg> = boot_ack boot_partition
>   - ``oem bootbus``  - this executes ``mmc bootbus %x %s`` to configure eMMC
> +- ``oem run`` - this executes an arbitrary U-Boot command
>   
>   Support for both eMMC and NAND devices is included.
>   
> @@ -127,6 +128,19 @@ Boot command
>   When executing the fastboot ``boot`` command, if ``fastboot_bootcmd`` is set
>   then that will be executed in place of ``bootm <CONFIG_FASTBOOT_BUF_ADDR>``.
>   
> +Running Shell Commands
> +----------------------
> +
> +Normally, arbitrary U-Boot command execution is not enabled. This is so
> +fastboot can be used to update systems using verified boot. However, such
> +functionality can be useful for production or when verified boot is not in use.
> +Enable ``CONFIG_FASTBOOT_UUU_SUPPORT`` to use this functionality. This will
> +enable the ``UCmd`` and ``ACmd`` commands for use with UUU [3]_. It also
> +enables the ``oem run`` command, which can be used with the fastboot client.
> +For example, to print "Hello world", run::
> +
> +    $ fastboot oem run:echo Hello world
> +
>   Partition Names
>   ---------------
>   
> @@ -232,3 +246,4 @@ References
>   
>   .. [1] :doc:`fastboot-protocol`
>   .. [2] https://developer.android.com/studio/releases/platform-tools
> +.. [3] https://github.com/NXPmicro/mfgtools
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index b97c67bf60..8f2d52cb8a 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -80,12 +80,12 @@ config FASTBOOT_FLASH
>   	  this to enable the "fastboot flash" command.
>   
>   config FASTBOOT_UUU_SUPPORT
> -	bool "Enable FASTBOOT i.MX UUU special command"
> +	bool "Enable running arbitrary commands from FASTBOOT"
>   	help
> -	  The fastboot protocol includes "UCmd" and "ACmd" command.
> -	  Be aware that you provide full access to any U-Boot command,
> -	  including working with memory and may open a huge backdoor,
> -	  when enabling this option.
> +	  This extends the fastboot protocol with the "UCmd" and "ACmd"
> +	  commands, as well as the "oem run" command.  These commands provide
> +	  full access to any U-Boot command, including working with memory.
> +	  This may open a huge backdoor if you are using verified boot.
>   

why the support of "oem run" generic support is include in the IMX config CONFIG_FASTBOOT_UUU_SUPPORT

"UCmd" and "Acmd" are imx additions in fastboot standard for UUU support

https://android.googlesource.com/platform/system/core/+/2ec418a4c98f6e8f95395456e1ad4c2956cac007/fastboot/fastboot_protocol.txt

but "oem run" can be see as generic U-Boot 'oem' command to execute 
U-boot command.


for me I see 2 separate configs to handle other platform than IMX ones (with UUU support)


config FASTBOOT_CMD_OEM_RUN
	bool "Enable fastboot oem run command"

config FASTBOOT_UUU_SUPPORT
	bool "Enable FASTBOOT i.MX UUU special command"
	select FASTBOOT_CMD_OEM_RUN


PS: FASTBOOT_UUU_SUPPORT could depends on CONFIG_ARCH_IMX*


>   choice
>   	prompt "Flash provider for FASTBOOT"
> diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
> index 98eccc3455..1732406c18 100644
> --- a/drivers/fastboot/fb_command.c
> +++ b/drivers/fastboot/fb_command.c
> @@ -123,6 +123,10 @@ static const struct {
>   	},
>   #endif
>   #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
> +	[FASTBOOT_COMMAND_OEM_RUN] = {
> +		.command = "oem run",
> +		.dispatch = run_ucmd,
> +	},


  #if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_RUN)
	[FASTBOOT_COMMAND_OEM_RUN] = {
		.command = "oem run",
		.dispatch = run_ucmd,
	},

#endif

#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)

	[FASTBOOT_COMMAND_UCMD] = {
  		.command = "UCmd",
  		.dispatch = run_ucmd,


That allows other platform to enable  FASTBOOT_CMD_OEM_RUN but not 
FASTBOOT_UUU_SUPPORT.


>   	[FASTBOOT_COMMAND_UCMD] = {
>   		.command = "UCmd",
>   		.dispatch = run_ucmd,
> diff --git a/include/fastboot.h b/include/fastboot.h
> index 57daaf1298..8b6b4b934a 100644
> --- a/include/fastboot.h
> +++ b/include/fastboot.h
> @@ -45,6 +45,7 @@ enum {
>   	FASTBOOT_COMMAND_OEM_BOOTBUS,
>   #endif
>   #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
> +	FASTBOOT_COMMAND_OEM_RUN,
>   	FASTBOOT_COMMAND_ACMD,
>   	FASTBOOT_COMMAND_UCMD,
>   #endif

+#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_RUN)

+	FASTBOOT_COMMAND_OEM_RUN,
+#endif


and also some stuff to compile the fucntion run_ucmd()


PS: we have many #if CONFIG is the fastboot source code

       they can be replaced by CONFIG_IS_ENABLED macro and 
__maybe_unused in .h and .c  ?

       and that avoids the compilation issue => let the linker remove 
the used functions

for example

enum {

	FASTBOOT_COMMAND_GETVAR = 0,
	FASTBOOT_COMMAND_DOWNLOAD,
	CONFIG_IS_ENABLED(FASTBOOT_FLASH, (FASTBOOT_COMMAND_FLASH,))
	CONFIG_IS_ENABLED(FASTBOOT_FLASH, (FASTBOOT_COMMAND_ERASE,))
	FASTBOOT_COMMAND_BOOT,
	FASTBOOT_COMMAND_CONTINUE,
	FASTBOOT_COMMAND_REBOOT,
	FASTBOOT_COMMAND_REBOOT_BOOTLOADER,
	FASTBOOT_COMMAND_REBOOT_FASTBOOTD,
	FASTBOOT_COMMAND_REBOOT_RECOVERY,
	FASTBOOT_COMMAND_SET_ACTIVE,
	CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT, (FASTBOOT_COMMAND_OEM_FORMAT,))
	CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF, (FASTBOOT_COMMAND_OEM_PARTCONF,))
	CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS, (FASTBOOT_COMMAND_OEM_BOOTBUS,))
	CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (FASTBOOT_COMMAND_ACMD,))
	CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (FASTBOOT_COMMAND_UCMD,))
	FASTBOOT_COMMAND_COUNT
};

...

ommands[FASTBOOT_COMMAND_COUNT] = {
[FASTBOOT_COMMAND_GETVAR] = {
.command = "getvar",
.dispatch = getvar
},
[FASTBOOT_COMMAND_DOWNLOAD] = {
.command = "download",
.dispatch = download
},
CONFIG_IS_ENABLED(FASTBOOT_FLASH, (
[FASTBOOT_COMMAND_FLASH] = {
.command = "flash",
.dispatch = flash
},
[FASTBOOT_COMMAND_ERASE] = {
.command = "erase",
.dispatch = erase
},
))
....
staticvoid__maybe_unused erase(char*cmd_parameter, char*response)
{
if(CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
fastboot_mmc_erase(cmd_parameter, response);
if(CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
fastboot_nand_erase(cmd_parameter, response);
}


regards

Patrick

Re: [PATCH v2] fastboot: Add OEM run command

[Sean Anderson]

On 12/5/22 14:04, Patrick DELAUNAY wrote:
> Hi,
> 
> On 12/2/22 22:03, Sean Anderson wrote:
>> This adds the UUU UCmd functionality as an OEM command. While the
>> fastboot tool allows sending arbitrary commands as long as they are
>> prefixed with "oem". This allows running generic U-Boot commands over
>> fastboot without UUU, which is especially useful when not using USB.
>> This is really the route we should have gone in the first place when
>> adding these commands.
>>
>> While we're here, clean up the Kconfig a bit.
>>
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>> ---
>>
>> Changes in v2:
>> - Document usage
>> - Keep enum in order
>>
>>   doc/android/fastboot.rst      | 15 +++++++++++++++
>>   drivers/fastboot/Kconfig      | 10 +++++-----
>>   drivers/fastboot/fb_command.c |  4 ++++
>>   include/fastboot.h            |  1 +
>>   4 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
>> index 7611f07038..7f343d5dfc 100644
>> --- a/doc/android/fastboot.rst
>> +++ b/doc/android/fastboot.rst
>> @@ -28,6 +28,7 @@ The following OEM commands are supported (if enabled):
>>   - ``oem partconf`` - this executes ``mmc partconf %x <arg> 0`` to configure eMMC
>>     with <arg> = boot_ack boot_partition
>>   - ``oem bootbus``  - this executes ``mmc bootbus %x %s`` to configure eMMC
>> +- ``oem run`` - this executes an arbitrary U-Boot command
>>     Support for both eMMC and NAND devices is included.
>>   @@ -127,6 +128,19 @@ Boot command
>>   When executing the fastboot ``boot`` command, if ``fastboot_bootcmd`` is set
>>   then that will be executed in place of ``bootm <CONFIG_FASTBOOT_BUF_ADDR>``.
>>   +Running Shell Commands
>> +----------------------
>> +
>> +Normally, arbitrary U-Boot command execution is not enabled. This is so
>> +fastboot can be used to update systems using verified boot. However, such
>> +functionality can be useful for production or when verified boot is not in use.
>> +Enable ``CONFIG_FASTBOOT_UUU_SUPPORT`` to use this functionality. This will
>> +enable the ``UCmd`` and ``ACmd`` commands for use with UUU [3]_. It also
>> +enables the ``oem run`` command, which can be used with the fastboot client.
>> +For example, to print "Hello world", run::
>> +
>> +    $ fastboot oem run:echo Hello world
>> +
>>   Partition Names
>>   ---------------
>>   @@ -232,3 +246,4 @@ References
>>     .. [1] :doc:`fastboot-protocol`
>>   .. [2] https://developer.android.com/studio/releases/platform-tools
>> +.. [3] https://github.com/NXPmicro/mfgtools
>> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
>> index b97c67bf60..8f2d52cb8a 100644
>> --- a/drivers/fastboot/Kconfig
>> +++ b/drivers/fastboot/Kconfig
>> @@ -80,12 +80,12 @@ config FASTBOOT_FLASH
>>         this to enable the "fastboot flash" command.
>>     config FASTBOOT_UUU_SUPPORT
>> -    bool "Enable FASTBOOT i.MX UUU special command"
>> +    bool "Enable running arbitrary commands from FASTBOOT"
>>       help
>> -      The fastboot protocol includes "UCmd" and "ACmd" command.
>> -      Be aware that you provide full access to any U-Boot command,
>> -      including working with memory and may open a huge backdoor,
>> -      when enabling this option.
>> +      This extends the fastboot protocol with the "UCmd" and "ACmd"
>> +      commands, as well as the "oem run" command.  These commands provide
>> +      full access to any U-Boot command, including working with memory.
>> +      This may open a huge backdoor if you are using verified boot.
>>   
> 
> why the support of "oem run" generic support is include in the IMX config CONFIG_FASTBOOT_UUU_SUPPORT
>

Because they are effectively the same command, but named differently.
oem run is just an alias for UCmd which can be used by the fastboot
Linux command without modification.

> "UCmd" and "Acmd" are imx additions in fastboot standard for UUU support
> 
> https://android.googlesource.com/platform/system/core/+/2ec418a4c98f6e8f95395456e1ad4c2956cac007/fastboot/fastboot_protocol.txt

These commands are absent from that standard (and thus are non-standard).

> but "oem run" can be see as generic U-Boot 'oem' command to execute U-boot command.

All of these commands are effectively non-standard, although "oem" is a
common prefix.

> for me I see 2 separate configs to handle other platform than IMX ones (with UUU support)
> 
> 
> config FASTBOOT_CMD_OEM_RUN
>     bool "Enable fastboot oem run command"
> 
> config FASTBOOT_UUU_SUPPORT
>     bool "Enable FASTBOOT i.MX UUU special command"
>     select FASTBOOT_CMD_OEM_RUN
> 
> 
> PS: FASTBOOT_UUU_SUPPORT could depends on CONFIG_ARCH_IMX*

Maybe we can have something like

config FASTBOOT_ARBITRARY_EXECUTION

which UUU_SUPPORT could depend on.

>>   choice
>>       prompt "Flash provider for FASTBOOT"
>> diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
>> index 98eccc3455..1732406c18 100644
>> --- a/drivers/fastboot/fb_command.c
>> +++ b/drivers/fastboot/fb_command.c
>> @@ -123,6 +123,10 @@ static const struct {
>>       },
>>   #endif
>>   #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>> +    [FASTBOOT_COMMAND_OEM_RUN] = {
>> +        .command = "oem run",
>> +        .dispatch = run_ucmd,
>> +    },
> 
> 
>  #if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_RUN)
>     [FASTBOOT_COMMAND_OEM_RUN] = {
>         .command = "oem run",
>         .dispatch = run_ucmd,
>     },
> 
> #endif
> 
> #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
> 
>     [FASTBOOT_COMMAND_UCMD] = {
>          .command = "UCmd",
>          .dispatch = run_ucmd,
> 
> 
> That allows other platform to enable  FASTBOOT_CMD_OEM_RUN but not FASTBOOT_UUU_SUPPORT.
> 
> 
>>       [FASTBOOT_COMMAND_UCMD] = {
>>           .command = "UCmd",
>>           .dispatch = run_ucmd,
>> diff --git a/include/fastboot.h b/include/fastboot.h
>> index 57daaf1298..8b6b4b934a 100644
>> --- a/include/fastboot.h
>> +++ b/include/fastboot.h
>> @@ -45,6 +45,7 @@ enum {
>>       FASTBOOT_COMMAND_OEM_BOOTBUS,
>>   #endif
>>   #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>> +    FASTBOOT_COMMAND_OEM_RUN,
>>       FASTBOOT_COMMAND_ACMD,
>>       FASTBOOT_COMMAND_UCMD,
>>   #endif
> 
> +#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_RUN)
> 
> +    FASTBOOT_COMMAND_OEM_RUN,
> +#endif
> 
> 
> and also some stuff to compile the fucntion run_ucmd()
> 
> 
> PS: we have many #if CONFIG is the fastboot source code
> 
>       they can be replaced by CONFIG_IS_ENABLED macro and __maybe_unused in .h and .c  ?
> 
>       and that avoids the compilation issue => let the linker remove the used functions
> 
> for example
> 
> enum {
> 
>     FASTBOOT_COMMAND_GETVAR = 0,
>     FASTBOOT_COMMAND_DOWNLOAD,
>     CONFIG_IS_ENABLED(FASTBOOT_FLASH, (FASTBOOT_COMMAND_FLASH,))
>     CONFIG_IS_ENABLED(FASTBOOT_FLASH, (FASTBOOT_COMMAND_ERASE,))
>     FASTBOOT_COMMAND_BOOT,
>     FASTBOOT_COMMAND_CONTINUE,
>     FASTBOOT_COMMAND_REBOOT,
>     FASTBOOT_COMMAND_REBOOT_BOOTLOADER,
>     FASTBOOT_COMMAND_REBOOT_FASTBOOTD,
>     FASTBOOT_COMMAND_REBOOT_RECOVERY,
>     FASTBOOT_COMMAND_SET_ACTIVE,
>     CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT, (FASTBOOT_COMMAND_OEM_FORMAT,))
>     CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF, (FASTBOOT_COMMAND_OEM_PARTCONF,))
>     CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS, (FASTBOOT_COMMAND_OEM_BOOTBUS,))
>     CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (FASTBOOT_COMMAND_ACMD,))
>     CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (FASTBOOT_COMMAND_UCMD,))
>     FASTBOOT_COMMAND_COUNT
> };
> 
> ...
> 
> ommands[FASTBOOT_COMMAND_COUNT] = {
> [FASTBOOT_COMMAND_GETVAR] = {
> .command = "getvar",
> .dispatch = getvar
> },
> [FASTBOOT_COMMAND_DOWNLOAD] = {
> .command = "download",
> .dispatch = download
> },
> CONFIG_IS_ENABLED(FASTBOOT_FLASH, (
> [FASTBOOT_COMMAND_FLASH] = {
> .command = "flash",
> .dispatch = flash
> },
> [FASTBOOT_COMMAND_ERASE] = {
> .command = "erase",
> .dispatch = erase
> },
> ))
> ....
> staticvoid__maybe_unused erase(char*cmd_parameter, char*response)
> {
> if(CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
> fastboot_mmc_erase(cmd_parameter, response);
> if(CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
> fastboot_nand_erase(cmd_parameter, response);
> }
> 

Yes, that would probably be a good cleanup idea. However, I'd like to
keep this patch's scope focused.

--Sean

Re: [PATCH v2] fastboot: Add OEM run command

[Patrick DELAUNAY]

Hi,

On 12/5/22 20:15, Sean Anderson wrote:
> On 12/5/22 14:04, Patrick DELAUNAY wrote:
>> Hi,
>>
>> On 12/2/22 22:03, Sean Anderson wrote:
>>> This adds the UUU UCmd functionality as an OEM command. While the
>>> fastboot tool allows sending arbitrary commands as long as they are
>>> prefixed with "oem". This allows running generic U-Boot commands over
>>> fastboot without UUU, which is especially useful when not using USB.
>>> This is really the route we should have gone in the first place when
>>> adding these commands.
>>>
>>> While we're here, clean up the Kconfig a bit.
>>>
>>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>>> ---
>>>
>>> Changes in v2:
>>> - Document usage
>>> - Keep enum in order
>>>
>>>    doc/android/fastboot.rst      | 15 +++++++++++++++
>>>    drivers/fastboot/Kconfig      | 10 +++++-----
>>>    drivers/fastboot/fb_command.c |  4 ++++
>>>    include/fastboot.h            |  1 +
>>>    4 files changed, 25 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
>>> index 7611f07038..7f343d5dfc 100644
>>> --- a/doc/android/fastboot.rst
>>> +++ b/doc/android/fastboot.rst
>>> @@ -28,6 +28,7 @@ The following OEM commands are supported (if enabled):
>>>    - ``oem partconf`` - this executes ``mmc partconf %x <arg> 0`` to configure eMMC
>>>      with <arg> = boot_ack boot_partition
>>>    - ``oem bootbus``  - this executes ``mmc bootbus %x %s`` to configure eMMC
>>> +- ``oem run`` - this executes an arbitrary U-Boot command
>>>      Support for both eMMC and NAND devices is included.
>>>    @@ -127,6 +128,19 @@ Boot command
>>>    When executing the fastboot ``boot`` command, if ``fastboot_bootcmd`` is set
>>>    then that will be executed in place of ``bootm <CONFIG_FASTBOOT_BUF_ADDR>``.
>>>    +Running Shell Commands
>>> +----------------------
>>> +
>>> +Normally, arbitrary U-Boot command execution is not enabled. This is so
>>> +fastboot can be used to update systems using verified boot. However, such
>>> +functionality can be useful for production or when verified boot is not in use.
>>> +Enable ``CONFIG_FASTBOOT_UUU_SUPPORT`` to use this functionality. This will
>>> +enable the ``UCmd`` and ``ACmd`` commands for use with UUU [3]_. It also
>>> +enables the ``oem run`` command, which can be used with the fastboot client.
>>> +For example, to print "Hello world", run::
>>> +
>>> +    $ fastboot oem run:echo Hello world
>>> +
>>>    Partition Names
>>>    ---------------
>>>    @@ -232,3 +246,4 @@ References
>>>      .. [1] :doc:`fastboot-protocol`
>>>    .. [2] https://developer.android.com/studio/releases/platform-tools
>>> +.. [3] https://github.com/NXPmicro/mfgtools
>>> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
>>> index b97c67bf60..8f2d52cb8a 100644
>>> --- a/drivers/fastboot/Kconfig
>>> +++ b/drivers/fastboot/Kconfig
>>> @@ -80,12 +80,12 @@ config FASTBOOT_FLASH
>>>          this to enable the "fastboot flash" command.
>>>      config FASTBOOT_UUU_SUPPORT
>>> -    bool "Enable FASTBOOT i.MX UUU special command"
>>> +    bool "Enable running arbitrary commands from FASTBOOT"
>>>        help
>>> -      The fastboot protocol includes "UCmd" and "ACmd" command.
>>> -      Be aware that you provide full access to any U-Boot command,
>>> -      including working with memory and may open a huge backdoor,
>>> -      when enabling this option.
>>> +      This extends the fastboot protocol with the "UCmd" and "ACmd"
>>> +      commands, as well as the "oem run" command.  These commands provide
>>> +      full access to any U-Boot command, including working with memory.
>>> +      This may open a huge backdoor if you are using verified boot.
>>>    
>> why the support of "oem run" generic support is include in the IMX config CONFIG_FASTBOOT_UUU_SUPPORT
>>
> Because they are effectively the same command, but named differently.
> oem run is just an alias for UCmd which can be used by the fastboot
> Linux command without modification.
>
>> "UCmd" and "Acmd" are imx additions in fastboot standard for UUU support
>>
>> https://android.googlesource.com/platform/system/core/+/2ec418a4c98f6e8f95395456e1ad4c2956cac007/fastboot/fastboot_protocol.txt
> These commands are absent from that standard (and thus are non-standard).


Yes, they are 'non-standard', but the command prefixed by 'oem' are 
allowed by the fastboot specification.

So all the "oem" commands can be used with the official Android tool...

$> fastboot --help

....

   oem <parameter1> ... <parameterN>        Executes oem specific command.
....

=> they are not reserved to IMX platforms


the added command UCmd and Acmd can be only used with UUU imx tools.



>
>> but "oem run" can be see as generic U-Boot 'oem' command to execute U-boot command.
> All of these commands are effectively non-standard, although "oem" is a
> common prefix.
>
>> for me I see 2 separate configs to handle other platform than IMX ones (with UUU support)
>>
>>
>> config FASTBOOT_CMD_OEM_RUN
>>      bool "Enable fastboot oem run command"
>>
>> config FASTBOOT_UUU_SUPPORT
>>      bool "Enable FASTBOOT i.MX UUU special command"
>>      select FASTBOOT_CMD_OEM_RUN
>>
>>
>> PS: FASTBOOT_UUU_SUPPORT could depends on CONFIG_ARCH_IMX*
> Maybe we can have something like
>
> config FASTBOOT_ARBITRARY_EXECUTION
>
> which UUU_SUPPORT could depend on.


whatever the Kconfig logic, the "fastboot oem run" should be enabled 
independently of imx/UUU support.


>
>>>    choice
>>>        prompt "Flash provider for FASTBOOT"
>>> diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
>>> index 98eccc3455..1732406c18 100644
>>> --- a/drivers/fastboot/fb_command.c
>>> +++ b/drivers/fastboot/fb_command.c
>>> @@ -123,6 +123,10 @@ static const struct {
>>>        },
>>>    #endif
>>>    #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>>> +    [FASTBOOT_COMMAND_OEM_RUN] = {
>>> +        .command = "oem run",
>>> +        .dispatch = run_ucmd,
>>> +    },
>>
>>   #if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_RUN)
>>      [FASTBOOT_COMMAND_OEM_RUN] = {
>>          .command = "oem run",
>>          .dispatch = run_ucmd,
>>      },
>>
>> #endif
>>
>> #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>>
>>      [FASTBOOT_COMMAND_UCMD] = {
>>           .command = "UCmd",
>>           .dispatch = run_ucmd,
>>
>>
>> That allows other platform to enable  FASTBOOT_CMD_OEM_RUN but not FASTBOOT_UUU_SUPPORT.
>>
>>
>>>        [FASTBOOT_COMMAND_UCMD] = {
>>>            .command = "UCmd",
>>>            .dispatch = run_ucmd,
>>> diff --git a/include/fastboot.h b/include/fastboot.h
>>> index 57daaf1298..8b6b4b934a 100644
>>> --- a/include/fastboot.h
>>> +++ b/include/fastboot.h
>>> @@ -45,6 +45,7 @@ enum {
>>>        FASTBOOT_COMMAND_OEM_BOOTBUS,
>>>    #endif
>>>    #if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>>> +    FASTBOOT_COMMAND_OEM_RUN,
>>>        FASTBOOT_COMMAND_ACMD,
>>>        FASTBOOT_COMMAND_UCMD,
>>>    #endif
>> +#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_RUN)
>>
>> +    FASTBOOT_COMMAND_OEM_RUN,
>> +#endif
>>
>>
>> and also some stuff to compile the fucntion run_ucmd()
>>
>>
>> PS: we have many #if CONFIG is the fastboot source code
>>
>>        they can be replaced by CONFIG_IS_ENABLED macro and __maybe_unused in .h and .c  ?
>>
>>        and that avoids the compilation issue => let the linker remove the used functions
>>
>> for example
>>
>> enum {
>>
>>      FASTBOOT_COMMAND_GETVAR = 0,
>>      FASTBOOT_COMMAND_DOWNLOAD,
>>      CONFIG_IS_ENABLED(FASTBOOT_FLASH, (FASTBOOT_COMMAND_FLASH,))
>>      CONFIG_IS_ENABLED(FASTBOOT_FLASH, (FASTBOOT_COMMAND_ERASE,))
>>      FASTBOOT_COMMAND_BOOT,
>>      FASTBOOT_COMMAND_CONTINUE,
>>      FASTBOOT_COMMAND_REBOOT,
>>      FASTBOOT_COMMAND_REBOOT_BOOTLOADER,
>>      FASTBOOT_COMMAND_REBOOT_FASTBOOTD,
>>      FASTBOOT_COMMAND_REBOOT_RECOVERY,
>>      FASTBOOT_COMMAND_SET_ACTIVE,
>>      CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT, (FASTBOOT_COMMAND_OEM_FORMAT,))
>>      CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF, (FASTBOOT_COMMAND_OEM_PARTCONF,))
>>      CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS, (FASTBOOT_COMMAND_OEM_BOOTBUS,))
>>      CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (FASTBOOT_COMMAND_ACMD,))
>>      CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (FASTBOOT_COMMAND_UCMD,))
>>      FASTBOOT_COMMAND_COUNT
>> };
>>
>> ...
>>
>> ommands[FASTBOOT_COMMAND_COUNT] = {
>> [FASTBOOT_COMMAND_GETVAR] = {
>> .command = "getvar",
>> .dispatch = getvar
>> },
>> [FASTBOOT_COMMAND_DOWNLOAD] = {
>> .command = "download",
>> .dispatch = download
>> },
>> CONFIG_IS_ENABLED(FASTBOOT_FLASH, (
>> [FASTBOOT_COMMAND_FLASH] = {
>> .command = "flash",
>> .dispatch = flash
>> },
>> [FASTBOOT_COMMAND_ERASE] = {
>> .command = "erase",
>> .dispatch = erase
>> },
>> ))
>> ....
>> staticvoid__maybe_unused erase(char*cmd_parameter, char*response)
>> {
>> if(CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
>> fastboot_mmc_erase(cmd_parameter, response);
>> if(CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
>> fastboot_nand_erase(cmd_parameter, response);
>> }
>>
> Yes, that would probably be a good cleanup idea. However, I'd like to
> keep this patch's scope focused.


Ok, I will try to propose something in parallel:

https://source.denx.de/u-boot/custodians/u-boot-stm/-/tree/fastboot

the compilation in CI is started...

>
> --Sean


Patrick

Re: [PATCH v2] fastboot: Add OEM run command

[Patrick DELAUNAY]

Hi,

On 12/6/22 11:23, Patrick DELAUNAY wrote:
> Hi,
>
> On 12/5/22 20:15, Sean Anderson wrote:
>> On 12/5/22 14:04, Patrick DELAUNAY wrote:
>>> Hi,
>>>
>>> On 12/2/22 22:03, Sean Anderson wrote:
>>>> This adds the UUU UCmd functionality as an OEM command. While the
>>>> fastboot tool allows sending arbitrary commands as long as they are
>>>> prefixed with "oem". This allows running generic U-Boot commands over
>>>> fastboot without UUU, which is especially useful when not using USB.
>>>> This is really the route we should have gone in the first place when
>>>> adding these commands.
>>>>
>>>> While we're here, clean up the Kconfig a bit.
>>>>
>>>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - Document usage
>>>> - Keep enum in order
>>>>
>>>>    doc/android/fastboot.rst      | 15 +++++++++++++++
>>>>    drivers/fastboot/Kconfig      | 10 +++++-----
>>>>    drivers/fastboot/fb_command.c |  4 ++++
>>>>    include/fastboot.h            |  1 +
>>>>    4 files changed, 25 insertions(+), 5 deletions(-)
>>>>
....


>>>> PS: we have many #if CONFIG is the fastboot source code
>>>>
>>>>        they can be replaced by CONFIG_IS_ENABLED macro and 
>>>> __maybe_unused in .h and .c  ?
>>>>
>>>>        and that avoids the compilation issue => let the linker 
>>>> remove the used functions
>>>>
>>>> for example
>>>>
>>>> enum {
>>>>
>>>>      FASTBOOT_COMMAND_GETVAR = 0,
>>>>      FASTBOOT_COMMAND_DOWNLOAD,
>>>>      CONFIG_IS_ENABLED(FASTBOOT_FLASH, (FASTBOOT_COMMAND_FLASH,))
>>>>      CONFIG_IS_ENABLED(FASTBOOT_FLASH, (FASTBOOT_COMMAND_ERASE,))
>>>>      FASTBOOT_COMMAND_BOOT,
>>>>      FASTBOOT_COMMAND_CONTINUE,
>>>>      FASTBOOT_COMMAND_REBOOT,
>>>>      FASTBOOT_COMMAND_REBOOT_BOOTLOADER,
>>>>      FASTBOOT_COMMAND_REBOOT_FASTBOOTD,
>>>>      FASTBOOT_COMMAND_REBOOT_RECOVERY,
>>>>      FASTBOOT_COMMAND_SET_ACTIVE,
>>>>      CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT, 
>>>> (FASTBOOT_COMMAND_OEM_FORMAT,))
>>>>      CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF, 
>>>> (FASTBOOT_COMMAND_OEM_PARTCONF,))
>>>>      CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS, 
>>>> (FASTBOOT_COMMAND_OEM_BOOTBUS,))
>>>>      CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (FASTBOOT_COMMAND_ACMD,))
>>>>      CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (FASTBOOT_COMMAND_UCMD,))
>>>>      FASTBOOT_COMMAND_COUNT
>>>> };
>>>>
>>>> ...
>>>>
>>>> ommands[FASTBOOT_COMMAND_COUNT] = {
>>>> [FASTBOOT_COMMAND_GETVAR] = {
>>>> .command = "getvar",
>>>> .dispatch = getvar
>>>> },
>>>> [FASTBOOT_COMMAND_DOWNLOAD] = {
>>>> .command = "download",
>>>> .dispatch = download
>>>> },
>>>> CONFIG_IS_ENABLED(FASTBOOT_FLASH, (
>>>> [FASTBOOT_COMMAND_FLASH] = {
>>>> .command = "flash",
>>>> .dispatch = flash
>>>> },
>>>> [FASTBOOT_COMMAND_ERASE] = {
>>>> .command = "erase",
>>>> .dispatch = erase
>>>> },
>>>> ))
>>>> ....
>>>> staticvoid__maybe_unused erase(char*cmd_parameter, char*response)
>>>> {
>>>> if(CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
>>>> fastboot_mmc_erase(cmd_parameter, response);
>>>> if(CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
>>>> fastboot_nand_erase(cmd_parameter, response);
>>>> }
>>>>
>> Yes, that would probably be a good cleanup idea. However, I'd like to
>> keep this patch's scope focused.
>
>
> Ok, I will try to propose something in parallel:
>
> https://source.denx.de/u-boot/custodians/u-boot-stm/-/tree/fastboot
>
> the compilation in CI is started...


For information, patch sent:

http://patchwork.ozlabs.org/project/uboot/list/?series=332837

"fastboot: remove #ifdef CONFIG when it is possible"


and CI build is OK =>

https://source.denx.de/u-boot/custodians/u-boot-stm/-/pipelines/14377


>
>>
>> --Sean
>
>
> Patrick
>

regards

Patrick