Re: [PATCH v1 5/5] fastboot: add oem console command support

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Svyatoslav,

Thank you for your patch.

On mar., nov. 07, 2023 at 14:42, Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> From: Ion Agorria <ion@agorria.com>
>
> "oem console" serves to read console record buffer.
>
> Signed-off-by: Ion Agorria <ion@agorria.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  doc/android/fastboot.rst      |  1 +
>  drivers/fastboot/Kconfig      |  7 +++++++
>  drivers/fastboot/fb_command.c | 39 +++++++++++++++++++++++++++++++++++
>  include/fastboot.h            |  1 +
>  4 files changed, 48 insertions(+)
>
> diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
> index 1ad8a897c8..05d8f77759 100644
> --- a/doc/android/fastboot.rst
> +++ b/doc/android/fastboot.rst
> @@ -29,6 +29,7 @@ The following OEM commands are supported (if enabled):
>    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
> +- ``oem console`` - this dumps U-Boot console record buffer
>  
>  Support for both eMMC and NAND devices is included.
>  
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index 837c6f1180..58b08120a4 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_CMD_OEM_CONSOLE
> +	bool "Enable the 'oem console' command"
> +	depends on CONSOLE_RECORD
> +	help
> +	  Add support for the "oem console" command to input and read console
> +	  record buffer.
> +
>  endif # FASTBOOT
>  
>  endmenu
> diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
> index 6f621df074..acf5971108 100644
> --- a/drivers/fastboot/fb_command.c
> +++ b/drivers/fastboot/fb_command.c
> @@ -41,6 +41,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_console(char *, char *);
>  static void run_ucmd(char *, char *);
>  static void run_acmd(char *, char *);
>  
> @@ -108,6 +109,10 @@ static const struct {
>  		.command = "oem run",
>  		.dispatch = CONFIG_IS_ENABLED(FASTBOOT_OEM_RUN, (run_ucmd), (NULL))
>  	},
> +	[FASTBOOT_COMMAND_OEM_CONSOLE] = {
> +		.command = "oem console",
> +		.dispatch = CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_CONSOLE, (oem_console), (NULL))
> +	},
>  	[FASTBOOT_COMMAND_UCMD] = {
>  		.command = "UCmd",
>  		.dispatch = CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (run_ucmd), (NULL))
> @@ -159,6 +164,23 @@ void fastboot_multiresponse(int cmd, char *response)
>  	case FASTBOOT_COMMAND_GETVAR:
>  		fastboot_getvar_all(response);
>  		break;
> +#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_CONSOLE)

Checkpatch also complains about this.

Can we rewrite this using if (IS_ENABLED(CONFIG...)) please ?


> +	case FASTBOOT_COMMAND_OEM_CONSOLE:
> +		char buf[FASTBOOT_RESPONSE_LEN] = { 0 };
> +
> +		if (console_record_isempty()) {
> +			console_record_reset();
> +			fastboot_okay(NULL, response);
> +		} else {
> +			int ret = console_record_readline(buf, sizeof(buf) - 5);
> +
> +			if (ret < 0)
> +				fastboot_fail("Error reading console", response);
> +			else
> +				fastboot_response("INFO", response, "%s", buf);
> +		}
> +		break;
> +#endif
>  	default:
>  		fastboot_fail("Unknown multiresponse command", response);
>  		break;
> @@ -503,3 +525,20 @@ static void __maybe_unused oem_bootbus(char *cmd_parameter, char *response)
>  	else
>  		fastboot_okay(NULL, response);
>  }
> +
> +/**
> + * oem_console() - Execute the OEM console command
> + *
> + * @cmd_parameter: Pointer to command parameter
> + * @response: Pointer to fastboot response buffer
> + */
> +static void __maybe_unused oem_console(char *cmd_parameter, char *response)
> +{
> +	if (cmd_parameter)
> +		console_in_puts(cmd_parameter);
> +
> +	if (console_record_isempty())
> +		fastboot_fail("Empty console", response);
> +	else
> +		fastboot_response("MORE", response, NULL);

MORE -> TEXT

> +}
> diff --git a/include/fastboot.h b/include/fastboot.h
> index d1a2b74b2f..23d26fb4be 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_CONSOLE,
>  	FASTBOOT_COMMAND_ACMD,
>  	FASTBOOT_COMMAND_UCMD,
>  	FASTBOOT_COMMAND_COUNT
> -- 
> 2.40.1