[PATCH v2] fastboot: Add OEM run command

Tue Dec 6 11:23:57 CET 2022

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