diff mbox series

[U-Boot,v4,7/9] cmd: efitool: add memmap command

Message ID 20190115025522.12060-8-takahiro.akashi@linaro.org
State Superseded
Delegated to: Alexander Graf
Headers show
Series cmd: add efitool for efi environment | expand

Commit Message

AKASHI Takahiro Jan. 15, 2019, 2:55 a.m. UTC 
"memmap" command prints uefi-specific memory map information.
=> efi memmap
Type             Start            End              Attributes

Comments

Heinrich Schuchardt Jan. 15, 2019, 4:26 a.m. UTC | #1 
On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> "memmap" command prints uefi-specific memory map information.
> => efi memmap
> Type             Start            End              Attributes
> ================ ================ ================ ==========
> CONVENTIONAL     0000000040000000-000000007de27000 WB
> RUNTIME DATA     000000007de27000-000000007de28000 WB|RT
> RESERVED         000000007de28000-000000007de2a000 WB
> RUNTIME DATA     000000007de2a000-000000007de2b000 WB|RT
> RESERVED         000000007de2b000-000000007de2c000 WB
> RUNTIME DATA     000000007de2c000-000000007de2d000 WB|RT
> LOADER DATA      000000007de2d000-000000007ff37000 WB
> RUNTIME CODE     000000007ff37000-000000007ff38000 WB|RT
> LOADER DATA      000000007ff38000-0000000080000000 WB
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  cmd/efitool.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/cmd/efitool.c b/cmd/efitool.c
> index e77273fc6e1e..f06718ea580d 100644
> --- a/cmd/efitool.c
> +++ b/cmd/efitool.c
> @@ -540,6 +540,78 @@ static int do_efi_show_images(int argc, char * const argv[])
>  	return CMD_RET_SUCCESS;
>  }
>  
> +static const char * const efi_mem_type_string[] = {
> +	[EFI_RESERVED_MEMORY_TYPE] = "RESERVED",
> +	[EFI_LOADER_CODE] = "LOADER CODE",
> +	[EFI_LOADER_DATA] = "LOADER DATA",
> +	[EFI_BOOT_SERVICES_CODE] = "BOOT CODE",
> +	[EFI_BOOT_SERVICES_DATA] = "BOOT DATA",
> +	[EFI_RUNTIME_SERVICES_CODE] = "RUNTIME CODE",
> +	[EFI_RUNTIME_SERVICES_DATA] = "RUNTIME DATA",
> +	[EFI_CONVENTIONAL_MEMORY] = "CONVENTIONAL",
> +	[EFI_UNUSABLE_MEMORY] = "UNUSABLE MEM",
> +	[EFI_ACPI_RECLAIM_MEMORY] = "ACPI RECLAIM MEM",
> +	[EFI_ACPI_MEMORY_NVS] = "ACPI NVS",
> +	[EFI_MMAP_IO] = "IO",
> +	[EFI_MMAP_IO_PORT] = "IO PORT",
> +	[EFI_PAL_CODE] = "PAL",
> +};
> +
> +#define EFI_MEM_ATTR(attribute, bit, string) \
> +	if ((attribute) & (bit)) {	\
> +		if (sep)		\
> +			putc('|');	\
> +		sep = 1;		\
> +		printf(string);		\
> +	}
> +
> +static int do_efi_show_memmap(int argc, char * const argv[])
> +{
> +	struct efi_mem_desc *memmap = NULL, *map;
> +	efi_uintn_t map_size = 0;
> +	int i, sep;
> +	efi_status_t ret;
> +
> +	ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);

ret = EFI_SUCCCESS will only occur if the memory has not been setup yet.

So shouldn't we error out if ret != EFI_BUFFER_TOO_SMALL ?

> +	if (ret == EFI_BUFFER_TOO_SMALL) {
> +		memmap = malloc(map_size);
> +		if (!memmap)
> +			return CMD_RET_FAILURE;
> +		ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
> +	}
> +	if (ret != EFI_SUCCESS) {
> +		free(memmap);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	printf("Type             Start            End              Attributes\n");
> +	printf("================ ================ ================ ==========\n");
> +	for (i = 0, map = memmap; i < map_size / sizeof(*map); map++, i++) {
> +		sep = 0;
> +		printf("%-16s %016llx-%016llx ",
> +		       efi_mem_type_string[map->type],
> +		       map->physical_start,
> +		       map->physical_start + map->num_pages * EFI_PAGE_SIZE);
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UC, "UC");
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WC, "WC");
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WT, "WT");
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WB, "WB");
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UCE, "UCE");
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WP, "WP");
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RP, "RP");
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_XP, "WP");
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_NV, "NV");
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_MORE_RELIABLE, "REL");
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RO, "RO");
> +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RUNTIME, "RT");

Please, put that logic into a separate function which returns a const
char *. Then we need only a single printf().

I suggest to use an array for the mapping:

struct memory_attribute_name {
	u64 attribute;
	const char *short_name;
};

struct memory_attribute_name memory_attribute_names[] = {
	{EFI_MEMORY_UC, "UC"},
	...
};

For looping you can use ARRAY_SIZE(memory_attribute_names).

> +		putc('\n');
> +	}
> +
> +	free(memmap);
> +
> +	return CMD_RET_SUCCESS;
> +}
> +
>  static int do_efi_boot_add(int argc, char * const argv[])
>  {

Please, refer to doc/README.commands for the standard way of adding
sub-commands.

Best regards

Heinrich

>  	int id;
> @@ -955,6 +1027,8 @@ static int do_efitool(cmd_tbl_t *cmdtp, int flag,
>  		return do_efi_show_handles(argc, argv);
>  	else if (!strcmp(command, "images"))
>  		return do_efi_show_images(argc, argv);
> +	else if (!strcmp(command, "memmap"))
> +		return do_efi_show_memmap(argc, argv);
>  	else
>  		return CMD_RET_USAGE;
>  }
> @@ -987,7 +1061,9 @@ static char efitool_help_text[] =
>  	"efitool dh\n"
>  	"  - show uefi handles\n"
>  	"efitool images\n"
> -	"  - show loaded images\n";
> +	"  - show loaded images\n"
> +	"efitool memmap\n"
> +	"  - show uefi memory map\n";
>  #endif
>  
>  U_BOOT_CMD(
>
AKASHI Takahiro Jan. 17, 2019, 7:03 a.m. UTC | #2 
On Tue, Jan 15, 2019 at 05:26:42AM +0100, Heinrich Schuchardt wrote:
> On 1/15/19 3:55 AM, AKASHI Takahiro wrote:
> > "memmap" command prints uefi-specific memory map information.
> > => efi memmap
> > Type             Start            End              Attributes
> > ================ ================ ================ ==========
> > CONVENTIONAL     0000000040000000-000000007de27000 WB
> > RUNTIME DATA     000000007de27000-000000007de28000 WB|RT
> > RESERVED         000000007de28000-000000007de2a000 WB
> > RUNTIME DATA     000000007de2a000-000000007de2b000 WB|RT
> > RESERVED         000000007de2b000-000000007de2c000 WB
> > RUNTIME DATA     000000007de2c000-000000007de2d000 WB|RT
> > LOADER DATA      000000007de2d000-000000007ff37000 WB
> > RUNTIME CODE     000000007ff37000-000000007ff38000 WB|RT
> > LOADER DATA      000000007ff38000-0000000080000000 WB
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  cmd/efitool.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 77 insertions(+), 1 deletion(-)
> > 
> > diff --git a/cmd/efitool.c b/cmd/efitool.c
> > index e77273fc6e1e..f06718ea580d 100644
> > --- a/cmd/efitool.c
> > +++ b/cmd/efitool.c
> > @@ -540,6 +540,78 @@ static int do_efi_show_images(int argc, char * const argv[])
> >  	return CMD_RET_SUCCESS;
> >  }
> >  
> > +static const char * const efi_mem_type_string[] = {
> > +	[EFI_RESERVED_MEMORY_TYPE] = "RESERVED",
> > +	[EFI_LOADER_CODE] = "LOADER CODE",
> > +	[EFI_LOADER_DATA] = "LOADER DATA",
> > +	[EFI_BOOT_SERVICES_CODE] = "BOOT CODE",
> > +	[EFI_BOOT_SERVICES_DATA] = "BOOT DATA",
> > +	[EFI_RUNTIME_SERVICES_CODE] = "RUNTIME CODE",
> > +	[EFI_RUNTIME_SERVICES_DATA] = "RUNTIME DATA",
> > +	[EFI_CONVENTIONAL_MEMORY] = "CONVENTIONAL",
> > +	[EFI_UNUSABLE_MEMORY] = "UNUSABLE MEM",
> > +	[EFI_ACPI_RECLAIM_MEMORY] = "ACPI RECLAIM MEM",
> > +	[EFI_ACPI_MEMORY_NVS] = "ACPI NVS",
> > +	[EFI_MMAP_IO] = "IO",
> > +	[EFI_MMAP_IO_PORT] = "IO PORT",
> > +	[EFI_PAL_CODE] = "PAL",
> > +};
> > +
> > +#define EFI_MEM_ATTR(attribute, bit, string) \
> > +	if ((attribute) & (bit)) {	\
> > +		if (sep)		\
> > +			putc('|');	\
> > +		sep = 1;		\
> > +		printf(string);		\
> > +	}
> > +
> > +static int do_efi_show_memmap(int argc, char * const argv[])
> > +{
> > +	struct efi_mem_desc *memmap = NULL, *map;
> > +	efi_uintn_t map_size = 0;
> > +	int i, sep;
> > +	efi_status_t ret;
> > +
> > +	ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
> 
> ret = EFI_SUCCCESS will only occur if the memory has not been setup yet.

I don't think so.

> So shouldn't we error out if ret != EFI_BUFFER_TOO_SMALL ?
> 
> > +	if (ret == EFI_BUFFER_TOO_SMALL) {
> > +		memmap = malloc(map_size);
> > +		if (!memmap)
> > +			return CMD_RET_FAILURE;
> > +		ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
> > +	}
> > +	if (ret != EFI_SUCCESS) {
> > +		free(memmap);
> > +		return CMD_RET_FAILURE;
> > +	}
> > +
> > +	printf("Type             Start            End              Attributes\n");
> > +	printf("================ ================ ================ ==========\n");
> > +	for (i = 0, map = memmap; i < map_size / sizeof(*map); map++, i++) {
> > +		sep = 0;
> > +		printf("%-16s %016llx-%016llx ",
> > +		       efi_mem_type_string[map->type],
> > +		       map->physical_start,
> > +		       map->physical_start + map->num_pages * EFI_PAGE_SIZE);
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UC, "UC");
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WC, "WC");
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WT, "WT");
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WB, "WB");
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UCE, "UCE");
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WP, "WP");
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RP, "RP");
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_XP, "WP");
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_NV, "NV");
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_MORE_RELIABLE, "REL");
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RO, "RO");
> > +		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RUNTIME, "RT");
> 
> Please, put that logic into a separate function which returns a const
> char *. Then we need only a single printf().

I don't think it's worth generating a one-time string only for this
specific use.
If 'printf' is a matter, I will simply replace it to puts() here.

> I suggest to use an array for the mapping:
> 
> struct memory_attribute_name {
> 	u64 attribute;
> 	const char *short_name;
> };
> 
> struct memory_attribute_name memory_attribute_names[] = {
> 	{EFI_MEMORY_UC, "UC"},
> 	...
> };
>
> For looping you can use ARRAY_SIZE(memory_attribute_names).
> 
> > +		putc('\n');
> > +	}
> > +
> > +	free(memmap);
> > +
> > +	return CMD_RET_SUCCESS;
> > +}
> > +
> >  static int do_efi_boot_add(int argc, char * const argv[])
> >  {
> 
> Please, refer to doc/README.commands for the standard way of adding
> sub-commands.

OK

Thanks,
-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >  	int id;
> > @@ -955,6 +1027,8 @@ static int do_efitool(cmd_tbl_t *cmdtp, int flag,
> >  		return do_efi_show_handles(argc, argv);
> >  	else if (!strcmp(command, "images"))
> >  		return do_efi_show_images(argc, argv);
> > +	else if (!strcmp(command, "memmap"))
> > +		return do_efi_show_memmap(argc, argv);
> >  	else
> >  		return CMD_RET_USAGE;
> >  }
> > @@ -987,7 +1061,9 @@ static char efitool_help_text[] =
> >  	"efitool dh\n"
> >  	"  - show uefi handles\n"
> >  	"efitool images\n"
> > -	"  - show loaded images\n";
> > +	"  - show loaded images\n"
> > +	"efitool memmap\n"
> > +	"  - show uefi memory map\n";
> >  #endif
> >  
> >  U_BOOT_CMD(
> > 
>
diff mbox series

Patch

================ ================ ================ ==========
CONVENTIONAL     0000000040000000-000000007de27000 WB
RUNTIME DATA     000000007de27000-000000007de28000 WB|RT
RESERVED         000000007de28000-000000007de2a000 WB
RUNTIME DATA     000000007de2a000-000000007de2b000 WB|RT
RESERVED         000000007de2b000-000000007de2c000 WB
RUNTIME DATA     000000007de2c000-000000007de2d000 WB|RT
LOADER DATA      000000007de2d000-000000007ff37000 WB
RUNTIME CODE     000000007ff37000-000000007ff38000 WB|RT
LOADER DATA      000000007ff38000-0000000080000000 WB

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/efitool.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 77 insertions(+), 1 deletion(-)

diff --git a/cmd/efitool.c b/cmd/efitool.c
index e77273fc6e1e..f06718ea580d 100644
--- a/cmd/efitool.c
+++ b/cmd/efitool.c
@@ -540,6 +540,78 @@  static int do_efi_show_images(int argc, char * const argv[])
 	return CMD_RET_SUCCESS;
 }
 
+static const char * const efi_mem_type_string[] = {
+	[EFI_RESERVED_MEMORY_TYPE] = "RESERVED",
+	[EFI_LOADER_CODE] = "LOADER CODE",
+	[EFI_LOADER_DATA] = "LOADER DATA",
+	[EFI_BOOT_SERVICES_CODE] = "BOOT CODE",
+	[EFI_BOOT_SERVICES_DATA] = "BOOT DATA",
+	[EFI_RUNTIME_SERVICES_CODE] = "RUNTIME CODE",
+	[EFI_RUNTIME_SERVICES_DATA] = "RUNTIME DATA",
+	[EFI_CONVENTIONAL_MEMORY] = "CONVENTIONAL",
+	[EFI_UNUSABLE_MEMORY] = "UNUSABLE MEM",
+	[EFI_ACPI_RECLAIM_MEMORY] = "ACPI RECLAIM MEM",
+	[EFI_ACPI_MEMORY_NVS] = "ACPI NVS",
+	[EFI_MMAP_IO] = "IO",
+	[EFI_MMAP_IO_PORT] = "IO PORT",
+	[EFI_PAL_CODE] = "PAL",
+};
+
+#define EFI_MEM_ATTR(attribute, bit, string) \
+	if ((attribute) & (bit)) {	\
+		if (sep)		\
+			putc('|');	\
+		sep = 1;		\
+		printf(string);		\
+	}
+
+static int do_efi_show_memmap(int argc, char * const argv[])
+{
+	struct efi_mem_desc *memmap = NULL, *map;
+	efi_uintn_t map_size = 0;
+	int i, sep;
+	efi_status_t ret;
+
+	ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
+	if (ret == EFI_BUFFER_TOO_SMALL) {
+		memmap = malloc(map_size);
+		if (!memmap)
+			return CMD_RET_FAILURE;
+		ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
+	}
+	if (ret != EFI_SUCCESS) {
+		free(memmap);
+		return CMD_RET_FAILURE;
+	}
+
+	printf("Type             Start            End              Attributes\n");
+	printf("================ ================ ================ ==========\n");
+	for (i = 0, map = memmap; i < map_size / sizeof(*map); map++, i++) {
+		sep = 0;
+		printf("%-16s %016llx-%016llx ",
+		       efi_mem_type_string[map->type],
+		       map->physical_start,
+		       map->physical_start + map->num_pages * EFI_PAGE_SIZE);
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UC, "UC");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WC, "WC");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WT, "WT");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WB, "WB");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_UCE, "UCE");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_WP, "WP");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RP, "RP");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_XP, "WP");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_NV, "NV");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_MORE_RELIABLE, "REL");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RO, "RO");
+		EFI_MEM_ATTR(map->attribute, EFI_MEMORY_RUNTIME, "RT");
+		putc('\n');
+	}
+
+	free(memmap);
+
+	return CMD_RET_SUCCESS;
+}
+
 static int do_efi_boot_add(int argc, char * const argv[])
 {
 	int id;
@@ -955,6 +1027,8 @@  static int do_efitool(cmd_tbl_t *cmdtp, int flag,
 		return do_efi_show_handles(argc, argv);
 	else if (!strcmp(command, "images"))
 		return do_efi_show_images(argc, argv);
+	else if (!strcmp(command, "memmap"))
+		return do_efi_show_memmap(argc, argv);
 	else
 		return CMD_RET_USAGE;
 }
@@ -987,7 +1061,9 @@  static char efitool_help_text[] =
 	"efitool dh\n"
 	"  - show uefi handles\n"
 	"efitool images\n"
-	"  - show loaded images\n";
+	"  - show loaded images\n"
+	"efitool memmap\n"
+	"  - show uefi memory map\n";
 #endif
 
 U_BOOT_CMD(