diff mbox series

[4/6] cmd: efidebug: Add support for querying UEFI variable storage

Message ID 20200506191246.237790-5-ilias.apalodimas@linaro.org
State Superseded
Headers show
Series EFI variable support via OP-TEE | expand

Commit Message

Ilias Apalodimas May 6, 2020, 7:12 p.m. UTC
With the previous patches that use OP-TEE and StandAloneMM for UEFI
variable storage we've added functionality for efi_query_variable_info.
So let's add the relevant command to efidebug and retrieve information
about the container used to store UEFI variables

Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
---
 cmd/efidebug.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

Comments

Heinrich Schuchardt May 9, 2020, 9:58 a.m. UTC | #1
On 5/6/20 9:12 PM, Ilias Apalodimas wrote:
> With the previous patches that use OP-TEE and StandAloneMM for UEFI
> variable storage we've added functionality for efi_query_variable_info.
> So let's add the relevant command to efidebug and retrieve information
> about the container used to store UEFI variables
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
>  cmd/efidebug.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index d8a76d78a388..17e36ef76d69 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -1160,6 +1160,45 @@ static int do_efi_test(cmd_tbl_t *cmdtp, int flag,
>  	return cp->cmd(cmdtp, flag, argc, argv);
>  }
>
> +/**
> + * do_efi_query_info() - QueryVariableInfo EFI service
> + *
> + * @cmdtp:	Command table
> + * @flag:	Command flag
> + * @argc:	Number of arguments
> + * @argv:	Argument array
> + * Return:	CMD_RET_SUCCESS on success,
> + *		CMD_RET_USAGE or CMD_RET_FAILURE on failure
> + *
> + * Implement efidebug "test" sub-command.
> + */
> +
> +static int do_efi_query_info(cmd_tbl_t *cmdtp, int flag,
> +			     int argc, char * const argv[])
> +{
> +	efi_status_t ret;
> +	u32 attr = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +			EFI_VARIABLE_RUNTIME_ACCESS |
> +			EFI_VARIABLE_NON_VOLATILE;
> +	u64 max_variable_storage_size;
> +	u64 remain_variable_storage_size;
> +	u64 max_variable_size;
> +
> +	ret = EFI_CALL(efi_query_variable_info(attr,
> +					       &max_variable_storage_size,
> +					       &remain_variable_storage_size,
> +					       &max_variable_size));
> +	if (ret != EFI_SUCCESS)
> +		return CMD_RET_FAILURE;
> +
> +	printf("%.*s ====================\n", EFI_HANDLE_WIDTH, sep);

We are not printing handles. Please remove the line.

> +	printf("Max storage size %llu\n", max_variable_storage_size);
> +	printf("Remaining storage size %llu\n", remain_variable_storage_size);
> +	printf("Max variable size %llu\n", max_variable_size);
> +
> +	return CMD_RET_SUCCESS;
> +}
> +
>  static cmd_tbl_t cmd_efidebug_sub[] = {
>  	U_BOOT_CMD_MKENT(boot, CONFIG_SYS_MAXARGS, 1, do_efi_boot_opt, "", ""),
>  	U_BOOT_CMD_MKENT(devices, CONFIG_SYS_MAXARGS, 1, do_efi_show_devices,
> @@ -1176,6 +1215,8 @@ static cmd_tbl_t cmd_efidebug_sub[] = {
>  			 "", ""),
>  	U_BOOT_CMD_MKENT(test, CONFIG_SYS_MAXARGS, 1, do_efi_test,
>  			 "", ""),
> +	U_BOOT_CMD_MKENT(query, CONFIG_SYS_MAXARGS, 1, do_efi_query_info,
> +			 "", ""),
>  };
>
>  /**
> @@ -1247,7 +1288,9 @@ static char efidebug_help_text[] =
>  	"efidebug tables\n"
>  	"  - show UEFI configuration tables\n"
>  	"efidebug test bootmgr\n"
> -	"  - run simple bootmgr for test\n";
> +	"  - run simple bootmgr for test\n"
> +	"efidebug query\n"
> +	"  - show information of the container used to store UEFI variables\n";

This text does not make it clear that we will get size information. How
about:

"show size of UEFI variables store\n"

Best regards

Heinrich

>  #endif
>
>  U_BOOT_CMD(
>
Ilias Apalodimas May 11, 2020, 8:49 a.m. UTC | #2
On Sat, May 09, 2020 at 11:58:17AM +0200, Heinrich Schuchardt wrote:
> On 5/6/20 9:12 PM, Ilias Apalodimas wrote:
> > +

[...]

> > +	printf("%.*s ====================\n", EFI_HANDLE_WIDTH, sep);
> 
> We are not printing handles. Please remove the line.
> 

Ok

> > +	printf("Max storage size %llu\n", max_variable_storage_size);
> > +	printf("Remaining storage size %llu\n", remain_variable_storage_size);
> > +	printf("Max variable size %llu\n", max_variable_size);
> > +
> > +	return CMD_RET_SUCCESS;
> > +}
> > +
> >  static cmd_tbl_t cmd_efidebug_sub[] = {
> >  	U_BOOT_CMD_MKENT(boot, CONFIG_SYS_MAXARGS, 1, do_efi_boot_opt, "", ""),
> >  	U_BOOT_CMD_MKENT(devices, CONFIG_SYS_MAXARGS, 1, do_efi_show_devices,
> > @@ -1176,6 +1215,8 @@ static cmd_tbl_t cmd_efidebug_sub[] = {
> >  			 "", ""),
> >  	U_BOOT_CMD_MKENT(test, CONFIG_SYS_MAXARGS, 1, do_efi_test,
> >  			 "", ""),
> > +	U_BOOT_CMD_MKENT(query, CONFIG_SYS_MAXARGS, 1, do_efi_query_info,
> > +			 "", ""),
> >  };
> >
> >  /**
> > @@ -1247,7 +1288,9 @@ static char efidebug_help_text[] =
> >  	"efidebug tables\n"
> >  	"  - show UEFI configuration tables\n"
> >  	"efidebug test bootmgr\n"
> > -	"  - run simple bootmgr for test\n";
> > +	"  - run simple bootmgr for test\n"
> > +	"efidebug query\n"
> > +	"  - show information of the container used to store UEFI variables\n";
> 
> This text does not make it clear that we will get size information. How
> about:
> 
> "show size of UEFI variables store\n"

Well the text was a c/p from here [1], but I agree I'll change it to what you
propose.


[1] https://edk2-docs.gitbook.io/edk-ii-uefi-driver-writer-s-guide/5_uefi_services/readme.2/526_queryvariableinfo


Regards
/Ilias
> 
> Best regards
> 
> Heinrich
> 
> >  #endif
> >
> >  U_BOOT_CMD(
> >
>
diff mbox series

Patch

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index d8a76d78a388..17e36ef76d69 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -1160,6 +1160,45 @@  static int do_efi_test(cmd_tbl_t *cmdtp, int flag,
 	return cp->cmd(cmdtp, flag, argc, argv);
 }
 
+/**
+ * do_efi_query_info() - QueryVariableInfo EFI service
+ *
+ * @cmdtp:	Command table
+ * @flag:	Command flag
+ * @argc:	Number of arguments
+ * @argv:	Argument array
+ * Return:	CMD_RET_SUCCESS on success,
+ *		CMD_RET_USAGE or CMD_RET_FAILURE on failure
+ *
+ * Implement efidebug "test" sub-command.
+ */
+
+static int do_efi_query_info(cmd_tbl_t *cmdtp, int flag,
+			     int argc, char * const argv[])
+{
+	efi_status_t ret;
+	u32 attr = EFI_VARIABLE_BOOTSERVICE_ACCESS |
+			EFI_VARIABLE_RUNTIME_ACCESS |
+			EFI_VARIABLE_NON_VOLATILE;
+	u64 max_variable_storage_size;
+	u64 remain_variable_storage_size;
+	u64 max_variable_size;
+
+	ret = EFI_CALL(efi_query_variable_info(attr,
+					       &max_variable_storage_size,
+					       &remain_variable_storage_size,
+					       &max_variable_size));
+	if (ret != EFI_SUCCESS)
+		return CMD_RET_FAILURE;
+
+	printf("%.*s ====================\n", EFI_HANDLE_WIDTH, sep);
+	printf("Max storage size %llu\n", max_variable_storage_size);
+	printf("Remaining storage size %llu\n", remain_variable_storage_size);
+	printf("Max variable size %llu\n", max_variable_size);
+
+	return CMD_RET_SUCCESS;
+}
+
 static cmd_tbl_t cmd_efidebug_sub[] = {
 	U_BOOT_CMD_MKENT(boot, CONFIG_SYS_MAXARGS, 1, do_efi_boot_opt, "", ""),
 	U_BOOT_CMD_MKENT(devices, CONFIG_SYS_MAXARGS, 1, do_efi_show_devices,
@@ -1176,6 +1215,8 @@  static cmd_tbl_t cmd_efidebug_sub[] = {
 			 "", ""),
 	U_BOOT_CMD_MKENT(test, CONFIG_SYS_MAXARGS, 1, do_efi_test,
 			 "", ""),
+	U_BOOT_CMD_MKENT(query, CONFIG_SYS_MAXARGS, 1, do_efi_query_info,
+			 "", ""),
 };
 
 /**
@@ -1247,7 +1288,9 @@  static char efidebug_help_text[] =
 	"efidebug tables\n"
 	"  - show UEFI configuration tables\n"
 	"efidebug test bootmgr\n"
-	"  - run simple bootmgr for test\n";
+	"  - run simple bootmgr for test\n"
+	"efidebug query\n"
+	"  - show information of the container used to store UEFI variables\n";
 #endif
 
 U_BOOT_CMD(