* [U-Boot] [PATCH v2 01/14] efi_loader: allow device == NULL in efi_dp_from_name()
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-11-05 9:06 ` [U-Boot] [PATCH v2 02/14] efi_loader: bootmgr: add load option helper functions AKASHI Takahiro
` (12 subsequent siblings)
13 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
This is a preparatory patch for use in efi_serialize_load_option()
as a load option's file_path should have both a device path and
a file path.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
lib/efi_loader/efi_device_path.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index cdf7c7be8c5c..d94982314a3e 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -955,7 +955,7 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
char filename[32] = { 0 }; /* dp->str is u16[32] long */
char *s;
- if (!device || (path && !file))
+ if (path && !file)
return EFI_INVALID_PARAMETER;
is_net = !strcmp(dev, "Net");
@@ -965,10 +965,12 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
if (part < 0)
return EFI_INVALID_PARAMETER;
- *device = efi_dp_from_part(desc, part);
+ if (device)
+ *device = efi_dp_from_part(desc, part);
} else {
#ifdef CONFIG_NET
- *device = efi_dp_from_eth();
+ if (device)
+ *device = efi_dp_from_eth();
#endif
}
@@ -985,7 +987,8 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
s = filename;
while ((s = strchr(s, '/')))
*s++ = '\\';
- *file = efi_dp_from_file(NULL, 0, filename);
+ *file = efi_dp_from_file(((!is_net && device) ? desc : NULL),
+ part, filename);
return EFI_SUCCESS;
}
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 02/14] efi_loader: bootmgr: add load option helper functions
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
2018-11-05 9:06 ` [U-Boot] [PATCH v2 01/14] efi_loader: allow device == NULL in efi_dp_from_name() AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-11-05 9:06 ` [U-Boot] [PATCH v2 03/14] efi_loader: bootmgr: allow for running a given load option AKASHI Takahiro
` (11 subsequent siblings)
13 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
In this patch, helper functions for an load option variable (BootXXXX)
are added:
* efi_deserialize_load_option(): parse a string into load_option data
(renamed from parse_load_option and exported)
* efi_serialize_load_option(): convert load_option data into a string
Those functions will be used to implement efishell command.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
include/efi_loader.h | 23 +++++++++
lib/efi_loader/efi_bootmgr.c | 93 +++++++++++++++++++++++-------------
2 files changed, 83 insertions(+), 33 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 5c2485bd03e4..d83de906fbce 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -517,6 +517,29 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_guid_t *vendor,
u32 attributes, efi_uintn_t data_size,
void *data);
+/*
+ * See section 3.1.3 in the v2.7 UEFI spec for more details on
+ * the layout of EFI_LOAD_OPTION. In short it is:
+ *
+ * typedef struct _EFI_LOAD_OPTION {
+ * UINT32 Attributes;
+ * UINT16 FilePathListLength;
+ * // CHAR16 Description[]; <-- variable length, NULL terminated
+ * // EFI_DEVICE_PATH_PROTOCOL FilePathList[];
+ * <-- FilePathListLength bytes
+ * // UINT8 OptionalData[];
+ * } EFI_LOAD_OPTION;
+ */
+struct efi_load_option {
+ u32 attributes;
+ u16 file_path_length;
+ u16 *label;
+ struct efi_device_path *file_path;
+ u8 *optional_data;
+};
+
+void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
+unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
void *efi_bootmgr_load(struct efi_device_path **device_path,
struct efi_device_path **file_path);
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 2aae12e15456..a095df3f540b 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -9,6 +9,7 @@
#include <charset.h>
#include <malloc.h>
#include <efi_loader.h>
+#include <asm/unaligned.h>
static const struct efi_boot_services *bs;
static const struct efi_runtime_services *rs;
@@ -30,42 +31,68 @@ static const struct efi_runtime_services *rs;
*/
-/*
- * See section 3.1.3 in the v2.7 UEFI spec for more details on
- * the layout of EFI_LOAD_OPTION. In short it is:
- *
- * typedef struct _EFI_LOAD_OPTION {
- * UINT32 Attributes;
- * UINT16 FilePathListLength;
- * // CHAR16 Description[]; <-- variable length, NULL terminated
- * // EFI_DEVICE_PATH_PROTOCOL FilePathList[]; <-- FilePathListLength bytes
- * // UINT8 OptionalData[];
- * } EFI_LOAD_OPTION;
- */
-struct load_option {
- u32 attributes;
- u16 file_path_length;
- u16 *label;
- struct efi_device_path *file_path;
- u8 *optional_data;
-};
-
-/* parse an EFI_LOAD_OPTION, as described above */
-static void parse_load_option(struct load_option *lo, void *ptr)
+/* Parse serialized data and transform it into efi_load_option structure */
+void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data)
{
- lo->attributes = *(u32 *)ptr;
- ptr += sizeof(u32);
+ lo->attributes = get_unaligned_le32(data);
+ data += sizeof(u32);
+
+ lo->file_path_length = get_unaligned_le16(data);
+ data += sizeof(u16);
- lo->file_path_length = *(u16 *)ptr;
- ptr += sizeof(u16);
+ /* FIXME */
+ lo->label = (u16 *)data;
+ data += (u16_strlen(lo->label) + 1) * sizeof(u16);
- lo->label = ptr;
- ptr += (u16_strlen(lo->label) + 1) * 2;
+ /* FIXME */
+ lo->file_path = (struct efi_device_path *)data;
+ data += lo->file_path_length;
- lo->file_path = ptr;
- ptr += lo->file_path_length;
+ lo->optional_data = data;
+}
- lo->optional_data = ptr;
+/*
+ * Serialize efi_load_option structure into byte stream for BootXXXX.
+ * Return a size of allocated data.
+ */
+unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data)
+{
+ unsigned long label_len, option_len;
+ unsigned long size;
+ u8 *p;
+
+ label_len = (u16_strlen(lo->label) + 1) * sizeof(u16);
+ option_len = strlen((char *)lo->optional_data);
+
+ /* total size */
+ size = sizeof(lo->attributes);
+ size += sizeof(lo->file_path_length);
+ size += label_len;
+ size += lo->file_path_length;
+ size += option_len + 1;
+ p = malloc(size);
+ if (!p)
+ return 0;
+
+ /* copy data */
+ *data = p;
+ memcpy(p, &lo->attributes, sizeof(lo->attributes));
+ p += sizeof(lo->attributes);
+
+ memcpy(p, &lo->file_path_length, sizeof(lo->file_path_length));
+ p += sizeof(lo->file_path_length);
+
+ memcpy(p, lo->label, label_len);
+ p += label_len;
+
+ memcpy(p, lo->file_path, lo->file_path_length);
+ p += lo->file_path_length;
+
+ memcpy(p, lo->optional_data, option_len);
+ p += option_len;
+ *(char *)p = '\0';
+
+ return size;
}
/* free() the result */
@@ -100,7 +127,7 @@ static void *get_var(u16 *name, const efi_guid_t *vendor,
static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,
struct efi_device_path **file_path)
{
- struct load_option lo;
+ struct efi_load_option lo;
u16 varname[] = L"Boot0000";
u16 hexmap[] = L"0123456789ABCDEF";
void *load_option, *image = NULL;
@@ -115,7 +142,7 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,
if (!load_option)
return NULL;
- parse_load_option(&lo, load_option);
+ efi_deserialize_load_option(&lo, load_option);
if (lo.attributes & LOAD_OPTION_ACTIVE) {
efi_status_t ret;
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 03/14] efi_loader: bootmgr: allow for running a given load option
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
2018-11-05 9:06 ` [U-Boot] [PATCH v2 01/14] efi_loader: allow device == NULL in efi_dp_from_name() AKASHI Takahiro
2018-11-05 9:06 ` [U-Boot] [PATCH v2 02/14] efi_loader: bootmgr: add load option helper functions AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-12-02 23:22 ` Alexander Graf
2018-11-05 9:06 ` [U-Boot] [PATCH v2 04/14] cmd: add efishell command AKASHI Takahiro
` (10 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
With an extra argument, efi_bootmgr_load() can now load an efi binary
based on a "BootXXXX" variable specified.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
include/efi_loader.h | 3 ++-
lib/efi_loader/efi_bootmgr.c | 8 +++++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index d83de906fbce..ce0f420b5004 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -540,7 +540,8 @@ struct efi_load_option {
void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
-void *efi_bootmgr_load(struct efi_device_path **device_path,
+void *efi_bootmgr_load(int boot_id,
+ struct efi_device_path **device_path,
struct efi_device_path **file_path);
#else /* CONFIG_IS_ENABLED(EFI_LOADER) */
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index a095df3f540b..74eb832a8c92 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -170,7 +170,8 @@ error:
* available load-options, finding and returning the first one that can
* be loaded successfully.
*/
-void *efi_bootmgr_load(struct efi_device_path **device_path,
+void *efi_bootmgr_load(int boot_id,
+ struct efi_device_path **device_path,
struct efi_device_path **file_path)
{
uint16_t *bootorder;
@@ -183,6 +184,11 @@ void *efi_bootmgr_load(struct efi_device_path **device_path,
bs = systab.boottime;
rs = systab.runtime;
+ if (boot_id != -1) {
+ image = try_load_entry(boot_id, device_path, file_path);
+ goto error;
+ }
+
bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size);
if (!bootorder)
goto error;
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 03/14] efi_loader: bootmgr: allow for running a given load option
2018-11-05 9:06 ` [U-Boot] [PATCH v2 03/14] efi_loader: bootmgr: allow for running a given load option AKASHI Takahiro
@ 2018-12-02 23:22 ` Alexander Graf
2018-12-03 3:20 ` AKASHI Takahiro
0 siblings, 1 reply; 39+ messages in thread
From: Alexander Graf @ 2018-12-02 23:22 UTC (permalink / raw)
To: u-boot
On 05.11.18 10:06, AKASHI Takahiro wrote:
> With an extra argument, efi_bootmgr_load() can now load an efi binary
> based on a "BootXXXX" variable specified.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
I don't see you changing the caller, so this hunk won't compile on its own?
Please make sure that every single step in your patch set compiles.
> ---
> include/efi_loader.h | 3 ++-
> lib/efi_loader/efi_bootmgr.c | 8 +++++++-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index d83de906fbce..ce0f420b5004 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -540,7 +540,8 @@ struct efi_load_option {
>
> void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
> unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
> -void *efi_bootmgr_load(struct efi_device_path **device_path,
> +void *efi_bootmgr_load(int boot_id,
> + struct efi_device_path **device_path,
> struct efi_device_path **file_path);
>
> #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index a095df3f540b..74eb832a8c92 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -170,7 +170,8 @@ error:
> * available load-options, finding and returning the first one that can
> * be loaded successfully.
> */
> -void *efi_bootmgr_load(struct efi_device_path **device_path,
> +void *efi_bootmgr_load(int boot_id,
> + struct efi_device_path **device_path,
> struct efi_device_path **file_path)
> {
> uint16_t *bootorder;
> @@ -183,6 +184,11 @@ void *efi_bootmgr_load(struct efi_device_path **device_path,
> bs = systab.boottime;
> rs = systab.runtime;
>
> + if (boot_id != -1) {
> + image = try_load_entry(boot_id, device_path, file_path);
> + goto error;
I think at this point it might be better to split the function, no?
You're not actually reusing any code from efi_bootmgr_load with the 2
instantiation types (explicit boot id, loop through boot order).
Alex
> + }
> +
> bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size);
> if (!bootorder)
> goto error;
>
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 03/14] efi_loader: bootmgr: allow for running a given load option
2018-12-02 23:22 ` Alexander Graf
@ 2018-12-03 3:20 ` AKASHI Takahiro
2018-12-03 13:54 ` Alexander Graf
0 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-12-03 3:20 UTC (permalink / raw)
To: u-boot
On Mon, Dec 03, 2018 at 12:22:37AM +0100, Alexander Graf wrote:
>
>
> On 05.11.18 10:06, AKASHI Takahiro wrote:
> > With an extra argument, efi_bootmgr_load() can now load an efi binary
> > based on a "BootXXXX" variable specified.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>
> I don't see you changing the caller, so this hunk won't compile on its own?
You're correct.
> Please make sure that every single step in your patch set compiles.
I will try to double-check in the future.
> > ---
> > include/efi_loader.h | 3 ++-
> > lib/efi_loader/efi_bootmgr.c | 8 +++++++-
> > 2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index d83de906fbce..ce0f420b5004 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -540,7 +540,8 @@ struct efi_load_option {
> >
> > void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
> > unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
> > -void *efi_bootmgr_load(struct efi_device_path **device_path,
> > +void *efi_bootmgr_load(int boot_id,
> > + struct efi_device_path **device_path,
> > struct efi_device_path **file_path);
> >
> > #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > index a095df3f540b..74eb832a8c92 100644
> > --- a/lib/efi_loader/efi_bootmgr.c
> > +++ b/lib/efi_loader/efi_bootmgr.c
> > @@ -170,7 +170,8 @@ error:
> > * available load-options, finding and returning the first one that can
> > * be loaded successfully.
> > */
> > -void *efi_bootmgr_load(struct efi_device_path **device_path,
> > +void *efi_bootmgr_load(int boot_id,
> > + struct efi_device_path **device_path,
> > struct efi_device_path **file_path)
> > {
> > uint16_t *bootorder;
> > @@ -183,6 +184,11 @@ void *efi_bootmgr_load(struct efi_device_path **device_path,
> > bs = systab.boottime;
> > rs = systab.runtime;
> >
> > + if (boot_id != -1) {
> > + image = try_load_entry(boot_id, device_path, file_path);
> > + goto error;
>
> I think at this point it might be better to split the function, no?
It's one way to fix, but I want simply to fix the issue by
modifying the caller, do_bootefi_bootmgr_exec() because
> You're not actually reusing any code from efi_bootmgr_load with the 2
> instantiation types (explicit boot id, loop through boot order).
I will add "BootNext" support to efi_bootmgr_load() so that it will be
a single point of loading. See my relevant patch:
https://lists.denx.de/pipermail/u-boot/2018-November/349281.html
Thanks,
-Takahiro Akashi
>
> Alex
>
> > + }
> > +
> > bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size);
> > if (!bootorder)
> > goto error;
> >
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 03/14] efi_loader: bootmgr: allow for running a given load option
2018-12-03 3:20 ` AKASHI Takahiro
@ 2018-12-03 13:54 ` Alexander Graf
0 siblings, 0 replies; 39+ messages in thread
From: Alexander Graf @ 2018-12-03 13:54 UTC (permalink / raw)
To: u-boot
On 03.12.18 04:20, AKASHI Takahiro wrote:
> On Mon, Dec 03, 2018 at 12:22:37AM +0100, Alexander Graf wrote:
>>
>>
>> On 05.11.18 10:06, AKASHI Takahiro wrote:
>>> With an extra argument, efi_bootmgr_load() can now load an efi binary
>>> based on a "BootXXXX" variable specified.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>
>> I don't see you changing the caller, so this hunk won't compile on its own?
>
> You're correct.
>
>> Please make sure that every single step in your patch set compiles.
>
> I will try to double-check in the future.
>
>>> ---
>>> include/efi_loader.h | 3 ++-
>>> lib/efi_loader/efi_bootmgr.c | 8 +++++++-
>>> 2 files changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>> index d83de906fbce..ce0f420b5004 100644
>>> --- a/include/efi_loader.h
>>> +++ b/include/efi_loader.h
>>> @@ -540,7 +540,8 @@ struct efi_load_option {
>>>
>>> void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
>>> unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
>>> -void *efi_bootmgr_load(struct efi_device_path **device_path,
>>> +void *efi_bootmgr_load(int boot_id,
>>> + struct efi_device_path **device_path,
>>> struct efi_device_path **file_path);
>>>
>>> #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
>>> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>>> index a095df3f540b..74eb832a8c92 100644
>>> --- a/lib/efi_loader/efi_bootmgr.c
>>> +++ b/lib/efi_loader/efi_bootmgr.c
>>> @@ -170,7 +170,8 @@ error:
>>> * available load-options, finding and returning the first one that can
>>> * be loaded successfully.
>>> */
>>> -void *efi_bootmgr_load(struct efi_device_path **device_path,
>>> +void *efi_bootmgr_load(int boot_id,
>>> + struct efi_device_path **device_path,
>>> struct efi_device_path **file_path)
>>> {
>>> uint16_t *bootorder;
>>> @@ -183,6 +184,11 @@ void *efi_bootmgr_load(struct efi_device_path **device_path,
>>> bs = systab.boottime;
>>> rs = systab.runtime;
>>>
>>> + if (boot_id != -1) {
>>> + image = try_load_entry(boot_id, device_path, file_path);
>>> + goto error;
>>
>> I think at this point it might be better to split the function, no?
>
> It's one way to fix, but I want simply to fix the issue by
> modifying the caller, do_bootefi_bootmgr_exec() because
>
>> You're not actually reusing any code from efi_bootmgr_load with the 2
>> instantiation types (explicit boot id, loop through boot order).
>
> I will add "BootNext" support to efi_bootmgr_load() so that it will be
> a single point of loading. See my relevant patch:
> https://lists.denx.de/pipermail/u-boot/2018-November/349281.html
I don't fully understand. I would expect BootNext to only take effect
when *not* booting an explicit ID? So that would fit quite nicely into a
function based split.
Alex
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 04/14] cmd: add efishell command
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
` (2 preceding siblings ...)
2018-11-05 9:06 ` [U-Boot] [PATCH v2 03/14] efi_loader: bootmgr: allow for running a given load option AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-12-02 23:42 ` Alexander Graf
2018-11-05 9:06 ` [U-Boot] [PATCH v2 05/14] cmd: efishell: add devices command AKASHI Takahiro
` (9 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
Currently, there is no easy way to add or modify UEFI variables.
In particular, bootmgr supports BootOrder/BootXXXX variables, it is
quite hard to define them as u-boot variables because they are represented
in a complicated and encoded format.
The new command, efishell, helps address these issues and give us
more friendly interfaces:
* efishell boot add: add BootXXXX variable
* efishell boot rm: remove BootXXXX variable
* efishell boot dump: display all BootXXXX variables
* efishell boot order: set/display a boot order (BootOrder)
* efishell setvar: set an UEFI variable (with limited functionality)
* efishell dumpvar: display all UEFI variables
As the name suggests, this command basically provides a subset fo UEFI
shell commands with simplified functionality.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/Makefile | 2 +-
cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
cmd/efishell.h | 5 +
3 files changed, 679 insertions(+), 1 deletion(-)
create mode 100644 cmd/efishell.c
create mode 100644 cmd/efishell.h
diff --git a/cmd/Makefile b/cmd/Makefile
index d9cdaf6064b8..bd531d505a8e 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -24,7 +24,7 @@ obj-$(CONFIG_CMD_BINOP) += binop.o
obj-$(CONFIG_CMD_BLOCK_CACHE) += blkcache.o
obj-$(CONFIG_CMD_BMP) += bmp.o
obj-$(CONFIG_CMD_BOOTCOUNT) += bootcount.o
-obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o
+obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o efishell.o
obj-$(CONFIG_CMD_BOOTMENU) += bootmenu.o
obj-$(CONFIG_CMD_BOOTSTAGE) += bootstage.o
obj-$(CONFIG_CMD_BOOTZ) += bootz.o
diff --git a/cmd/efishell.c b/cmd/efishell.c
new file mode 100644
index 000000000000..abc8216c7bd6
--- /dev/null
+++ b/cmd/efishell.c
@@ -0,0 +1,673 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * EFI Shell-like command
+ *
+ * Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
+ */
+
+#include <charset.h>
+#include <common.h>
+#include <command.h>
+#include <efi_loader.h>
+#include <environment.h>
+#include <errno.h>
+#include <exports.h>
+#include <hexdump.h>
+#include <malloc.h>
+#include <search.h>
+#include <linux/ctype.h>
+#include <asm/global_data.h>
+#include "efishell.h"
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static void dump_var_data(char *data, unsigned long len)
+{
+ char *start, *end, *p;
+ unsigned long pos, count;
+ char hex[3], line[9];
+ int i;
+
+ end = data + len;
+ for (start = data, pos = 0; start < end; start += count, pos += count) {
+ count = end - start;
+ if (count > 16)
+ count = 16;
+
+ /* count should be multiple of two */
+ printf("%08lx: ", pos);
+
+ /* in hex format */
+ p = start;
+ for (i = 0; i < count / 2; p += 2, i++)
+ printf(" %c%c", *p, *(p + 1));
+ for (; i < 8; i++)
+ printf(" ");
+
+ /* in character format */
+ p = start;
+ hex[2] = '\0';
+ for (i = 0; i < count / 2; i++) {
+ hex[0] = *p++;
+ hex[1] = *p++;
+ line[i] = simple_strtoul(hex, 0, 16);
+ if (line[i] < 0x20 || line[i] > 0x7f)
+ line[i] = '.';
+ }
+ line[i] = '\0';
+ printf(" %s\n", line);
+ }
+}
+
+/*
+ * From efi_variable.c,
+ *
+ * Mapping between EFI variables and u-boot variables:
+ *
+ * efi_$guid_$varname = {attributes}(type)value
+ */
+static int do_efi_dump_var(int argc, char * const argv[])
+{
+ char regex[256];
+ char * const regexlist[] = {regex};
+ char *res = NULL, *start, *end;
+ int len;
+
+ if (argc > 2)
+ return CMD_RET_USAGE;
+
+ if (argc == 2)
+ snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_%s", argv[1]);
+ else
+ snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
+ debug("%s:%d grep uefi var %s\n", __func__, __LINE__, regex);
+
+ len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
+ &res, 0, 1, regexlist);
+
+ if (len < 0)
+ return CMD_RET_FAILURE;
+
+ if (len > 0) {
+ end = res;
+ while (true) {
+ /* variable name */
+ start = strchr(end, '_');
+ if (!start)
+ break;
+ start = strchr(++start, '_');
+ if (!start)
+ break;
+ end = strchr(++start, '=');
+ if (!end)
+ break;
+ printf("%.*s:", (int)(end - start), start);
+ end++;
+
+ /* value */
+ start = end;
+ end = strstr(start, "(blob)");
+ if (!end) {
+ putc('\n');
+ break;
+ }
+ end += 6;
+ printf(" %.*s\n", (int)(end - start), start);
+
+ start = end;
+ end = strchr(start, '\n');
+ if (!end)
+ break;
+ dump_var_data(start, end - start);
+ }
+ free(res);
+
+ if (len < 2 && argc == 2)
+ printf("%s: not found\n", argv[1]);
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
+static int append_value(char **bufp, unsigned long *sizep, char *data)
+{
+ char *tmp_buf = NULL, *new_buf = NULL, *value;
+ unsigned long len = 0;
+
+ if (!strncmp(data, "=0x", 2)) { /* hexadecimal number */
+ union {
+ u8 u8;
+ u16 u16;
+ u32 u32;
+ u64 u64;
+ } tmp_data;
+ unsigned long hex_value;
+ void *hex_ptr;
+
+ data += 3;
+ len = strlen(data);
+ if ((len & 0x1)) /* not multiple of two */
+ return -1;
+
+ len /= 2;
+ if (len > 8)
+ return -1;
+ else if (len > 4)
+ len = 8;
+ else if (len > 2)
+ len = 4;
+
+ /* convert hex hexadecimal number */
+ if (strict_strtoul(data, 16, &hex_value) < 0)
+ return -1;
+
+ tmp_buf = malloc(len);
+ if (!tmp_buf)
+ return -1;
+
+ if (len == 1) {
+ tmp_data.u8 = hex_value;
+ hex_ptr = &tmp_data.u8;
+ } else if (len == 2) {
+ tmp_data.u16 = hex_value;
+ hex_ptr = &tmp_data.u16;
+ } else if (len == 4) {
+ tmp_data.u32 = hex_value;
+ hex_ptr = &tmp_data.u32;
+ } else {
+ tmp_data.u64 = hex_value;
+ hex_ptr = &tmp_data.u64;
+ }
+ memcpy(tmp_buf, hex_ptr, len);
+ value = tmp_buf;
+
+ } else if (!strncmp(data, "=H", 2)) { /* hexadecimal-byte array */
+ data += 2;
+ len = strlen(data);
+ if (len & 0x1) /* not multiple of two */
+ return -1;
+
+ len /= 2;
+ tmp_buf = malloc(len);
+ if (!tmp_buf)
+ return -1;
+
+ if (hex2bin((u8 *)tmp_buf, data, len) < 0)
+ return -1;
+
+ value = tmp_buf;
+ } else { /* string */
+ if (!strncmp(data, "=\"", 2) || !strncmp(data, "=S\"", 3)) {
+ if (data[1] == '"')
+ data += 2;
+ else
+ data += 3;
+ value = data;
+ len = strlen(data) - 1;
+ if (data[len] != '"')
+ return -1;
+ } else {
+ value = data;
+ len = strlen(data);
+ }
+ }
+
+ new_buf = realloc(*bufp, *sizep + len);
+ if (!new_buf)
+ goto out;
+
+ memcpy(new_buf + *sizep, value, len);
+ *bufp = new_buf;
+ *sizep += len;
+
+out:
+ free(tmp_buf);
+
+ return 0;
+}
+
+static int do_efi_set_var(int argc, char * const argv[])
+{
+ char *var_name, *value = NULL;
+ unsigned long size = 0;
+ u16 *var_name16, *p;
+ efi_guid_t guid;
+ efi_status_t ret;
+
+ if (argc == 1)
+ return CMD_RET_SUCCESS;
+
+ var_name = argv[1];
+ if (argc == 2) {
+ /* remove */
+ value = NULL;
+ size = 0;
+ } else { /* set */
+ argc -= 2;
+ argv += 2;
+
+ for ( ; argc > 0; argc--, argv++)
+ if (append_value(&value, &size, argv[0]) < 0) {
+ ret = CMD_RET_FAILURE;
+ goto out;
+ }
+ }
+
+ var_name16 = malloc((strlen(var_name) + 1) * 2);
+ p = var_name16;
+ utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
+
+ guid = efi_global_variable_guid;
+ ret = efi_set_variable(var_name16, &guid,
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS, size, value);
+ ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+ return ret;
+}
+
+static int do_efi_boot_add(int argc, char * const argv[])
+{
+ int id;
+ char *endp;
+ char var_name[9];
+ u16 var_name16[9], *p;
+ efi_guid_t guid;
+ size_t label_len, label_len16;
+ u16 *label;
+ struct efi_device_path *device_path = NULL, *file_path = NULL;
+ struct efi_load_option lo;
+ void *data = NULL;
+ unsigned long size;
+ int ret;
+
+ if (argc < 6 || argc > 7)
+ return CMD_RET_USAGE;
+
+ id = (int)simple_strtoul(argv[1], &endp, 0);
+ if (*endp != '\0' || id > 0xffff)
+ return CMD_RET_FAILURE;
+
+ sprintf(var_name, "Boot%04X", id);
+ p = var_name16;
+ utf8_utf16_strncpy(&p, var_name, 9);
+
+ guid = efi_global_variable_guid;
+
+ /* attributes */
+ lo.attributes = 0x1; /* always ACTIVE */
+
+ /* label */
+ label_len = strlen(argv[2]);
+ label_len16 = utf8_utf16_strnlen(argv[2], label_len);
+ label = malloc((label_len16 + 1) * sizeof(u16));
+ if (!label)
+ return CMD_RET_FAILURE;
+ lo.label = label; /* label will be changed below */
+ utf8_utf16_strncpy(&label, argv[2], label_len);
+
+ /* file path */
+ ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
+ &file_path);
+ if (ret != EFI_SUCCESS) {
+ ret = CMD_RET_FAILURE;
+ goto out;
+ }
+ lo.file_path = file_path;
+ lo.file_path_length = efi_dp_size(file_path)
+ + sizeof(struct efi_device_path); /* for END */
+
+ /* optional data */
+ lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
+
+ size = efi_serialize_load_option(&lo, (u8 **)&data);
+ if (!size) {
+ ret = CMD_RET_FAILURE;
+ goto out;
+ }
+
+ ret = efi_set_variable(var_name16, &guid,
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS, size, data);
+ ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+ free(data);
+#if 0 /* FIXME */
+ free(device_path);
+ free(file_path);
+#endif
+ free(lo.label);
+ return ret;
+}
+
+static int do_efi_boot_rm(int argc, char * const argv[])
+{
+ efi_guid_t guid;
+ int id, i;
+ char *endp;
+ char var_name[9];
+ u16 var_name16[9];
+ efi_status_t ret;
+
+ if (argc == 1)
+ return CMD_RET_USAGE;
+
+ guid = efi_global_variable_guid;
+ for (i = 1; i < argc; i++, argv++) {
+ id = (int)simple_strtoul(argv[1], &endp, 0);
+ if (*endp != '\0' || id > 0xffff)
+ return CMD_RET_FAILURE;
+
+ sprintf(var_name, "Boot%04X", id);
+ utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
+
+ ret = efi_set_variable(var_name16, &guid,
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);
+ if (ret) {
+ printf("cannot remove Boot%04X", id);
+ return CMD_RET_FAILURE;
+ }
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
+static void show_efi_boot_opt_data(int id, void *data)
+{
+ struct efi_load_option lo;
+ char *label, *p;
+ size_t label_len16, label_len;
+ u16 *dp_str;
+
+ efi_deserialize_load_option(&lo, data);
+
+ label_len16 = u16_strlen(lo.label);
+ label_len = utf16_utf8_strnlen(lo.label, label_len16);
+ label = malloc(label_len + 1);
+ if (!label)
+ return;
+ p = label;
+ utf16_utf8_strncpy(&p, lo.label, label_len16);
+
+ printf("Boot%04X:\n", id);
+ printf("\tattributes: %c%c%c (0x%08x)\n",
+ /* ACTIVE */
+ lo.attributes & 0x1 ? 'A' : '-',
+ /* FORCE RECONNECT */
+ lo.attributes & 0x2 ? 'R' : '-',
+ /* HIDDEN */
+ lo.attributes & 0x8 ? 'H' : '-',
+ lo.attributes);
+ printf("\tlabel: %s\n", label);
+
+ dp_str = efi_dp_str(lo.file_path);
+ printf("\tfile_path: %ls\n", dp_str);
+ efi_free_pool(dp_str);
+
+ printf("\tdata: %s\n", lo.optional_data);
+
+ free(label);
+}
+
+static void show_efi_boot_opt(int id)
+{
+ char var_name[9];
+ u16 var_name16[9], *p;
+ efi_guid_t guid;
+ void *data = NULL;
+ unsigned long size;
+ int ret;
+
+ sprintf(var_name, "Boot%04X", id);
+ p = var_name16;
+ utf8_utf16_strncpy(&p, var_name, 9);
+ guid = efi_global_variable_guid;
+
+ size = 0;
+ ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
+ if (ret == (int)EFI_BUFFER_TOO_SMALL) {
+ data = malloc(size);
+ ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
+ }
+ if (ret == EFI_SUCCESS) {
+ show_efi_boot_opt_data(id, data);
+ free(data);
+ } else if (ret == EFI_NOT_FOUND) {
+ printf("Boot%04X: not found\n", id);
+ }
+}
+
+static int do_efi_boot_dump(int argc, char * const argv[])
+{
+ char regex[256];
+ char * const regexlist[] = {regex};
+ char *variables = NULL, *boot, *value;
+ int len;
+ int id;
+
+ if (argc > 1)
+ return CMD_RET_USAGE;
+
+ snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+");
+
+ len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
+ &variables, 0, 1, regexlist);
+
+ if (!len)
+ return CMD_RET_SUCCESS;
+
+ if (len < 0)
+ return CMD_RET_FAILURE;
+
+ boot = variables;
+ while (*boot) {
+ value = strstr(boot, "Boot") + 4;
+ id = (int)simple_strtoul(value, NULL, 16);
+ show_efi_boot_opt(id);
+ boot = strchr(boot, '\n');
+ if (!*boot)
+ break;
+ boot++;
+ }
+ free(variables);
+
+ return CMD_RET_SUCCESS;
+}
+
+static int show_efi_boot_order(void)
+{
+ efi_guid_t guid;
+ u16 *bootorder = NULL;
+ unsigned long size;
+ int num, i;
+ char var_name[9];
+ u16 var_name16[9], *p16;
+ void *data;
+ struct efi_load_option lo;
+ char *label, *p;
+ size_t label_len16, label_len;
+ efi_status_t ret = CMD_RET_SUCCESS;
+
+ guid = efi_global_variable_guid;
+ size = 0;
+ ret = efi_get_variable(L"BootOrder", &guid, NULL, &size, NULL);
+ if (ret == EFI_BUFFER_TOO_SMALL) {
+ bootorder = malloc(size);
+ ret = efi_get_variable(L"BootOrder", &guid, NULL, &size,
+ bootorder);
+ }
+ if (ret != EFI_SUCCESS) {
+ printf("BootOrder not defined\n");
+ return CMD_RET_SUCCESS;
+ }
+
+ num = size / sizeof(u16);
+ for (i = 0; i < num; i++) {
+ sprintf(var_name, "Boot%04X", bootorder[i]);
+ p16 = var_name16;
+ utf8_utf16_strncpy(&p16, var_name, 9);
+
+ size = 0;
+ ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
+ if (ret != EFI_BUFFER_TOO_SMALL) {
+ printf("%2d: Boot%04X: (not defined)\n",
+ i + 1, bootorder[i]);
+ continue;
+ }
+
+ data = malloc(size);
+ if (!data) {
+ ret = CMD_RET_FAILURE;
+ goto out;
+ }
+ ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
+ if (ret != EFI_SUCCESS) {
+ free(data);
+ ret = CMD_RET_FAILURE;
+ goto out;
+ }
+
+ efi_deserialize_load_option(&lo, data);
+
+ label_len16 = u16_strlen(lo.label);
+ label_len = utf16_utf8_strnlen(lo.label, label_len16);
+ label = malloc(label_len + 1);
+ if (!label) {
+ free(data);
+ ret = CMD_RET_FAILURE;
+ goto out;
+ }
+ p = label;
+ utf16_utf8_strncpy(&p, lo.label, label_len16);
+ printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
+ free(label);
+
+ free(data);
+ }
+out:
+ free(bootorder);
+
+ return ret;
+}
+
+static int do_efi_boot_order(int argc, char * const argv[])
+{
+ u16 *bootorder = NULL;
+ unsigned long size;
+ int id, i;
+ char *endp;
+ efi_guid_t guid;
+ efi_status_t ret;
+
+ if (argc == 1)
+ return show_efi_boot_order();
+
+ argc--;
+ argv++;
+
+ size = argc * sizeof(u16);
+ bootorder = malloc(size);
+ if (!bootorder)
+ return CMD_RET_FAILURE;
+
+ for (i = 0; i < argc; i++) {
+ id = (int)simple_strtoul(argv[i], &endp, 0);
+ if (*endp != '\0' || id > 0xffff) {
+ printf("invalid value: %s\n", argv[i]);
+ ret = CMD_RET_FAILURE;
+ goto out;
+ }
+
+ bootorder[i] = (u16)id;
+ }
+
+ guid = efi_global_variable_guid;
+ ret = efi_set_variable(L"BootOrder", &guid,
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);
+ ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
+out:
+ free(bootorder);
+
+ return ret;
+}
+
+static int do_efi_boot_opt(int argc, char * const argv[])
+{
+ char *sub_command;
+
+ if (argc < 2)
+ return CMD_RET_USAGE;
+
+ argc--; argv++;
+ sub_command = argv[0];
+
+ if (!strcmp(sub_command, "add"))
+ return do_efi_boot_add(argc, argv);
+ else if (!strcmp(sub_command, "rm"))
+ return do_efi_boot_rm(argc, argv);
+ else if (!strcmp(sub_command, "dump"))
+ return do_efi_boot_dump(argc, argv);
+ else if (!strcmp(sub_command, "order"))
+ return do_efi_boot_order(argc, argv);
+ else
+ return CMD_RET_USAGE;
+}
+
+/* Interpreter command to configure EFI environment */
+static int do_efishell(cmd_tbl_t *cmdtp, int flag,
+ int argc, char * const argv[])
+{
+ char *command;
+ efi_status_t r;
+
+ if (argc < 2)
+ return CMD_RET_USAGE;
+
+ argc--; argv++;
+ command = argv[0];
+
+ /* Initialize EFI drivers */
+ r = efi_init_obj_list();
+ if (r != EFI_SUCCESS) {
+ printf("Error: Cannot set up EFI drivers, r = %lu\n",
+ r & ~EFI_ERROR_MASK);
+ return CMD_RET_FAILURE;
+ }
+
+ if (!strcmp(command, "boot"))
+ return do_efi_boot_opt(argc, argv);
+ else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore"))
+ return do_efi_dump_var(argc, argv);
+ else if (!strcmp(command, "setvar"))
+ return do_efi_set_var(argc, argv);
+ else
+ return CMD_RET_USAGE;
+}
+
+#ifdef CONFIG_SYS_LONGHELP
+static char efishell_help_text[] =
+ " - EFI Shell-like interface to configure EFI environment\n"
+ "\n"
+ "efishell boot add <bootid> <label> <interface> <device>[:<part>] <file path> <option>\n"
+ " - set uefi's BOOT variable\n"
+ "efishell boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
+ " - set/display uefi boot order\n"
+ "efishell boot dump\n"
+ " - display all uefi's BOOT variables\n"
+ "efishell boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
+ " - set/display uefi boot order\n"
+ "\n"
+ "efishell dumpvar [<name>]\n"
+ " - get uefi variable's value\n"
+ "efishell setvar <name> [<value>]\n"
+ " - set/delete uefi variable's value\n"
+ " <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n";
+#endif
+
+U_BOOT_CMD(
+ efishell, 10, 0, do_efishell,
+ "Configure EFI environment",
+ efishell_help_text
+);
diff --git a/cmd/efishell.h b/cmd/efishell.h
new file mode 100644
index 000000000000..6f70b402b26b
--- /dev/null
+++ b/cmd/efishell.h
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <efi.h>
+
+efi_status_t efi_init_obj_list(void);
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 04/14] cmd: add efishell command
2018-11-05 9:06 ` [U-Boot] [PATCH v2 04/14] cmd: add efishell command AKASHI Takahiro
@ 2018-12-02 23:42 ` Alexander Graf
2018-12-03 6:42 ` AKASHI Takahiro
0 siblings, 1 reply; 39+ messages in thread
From: Alexander Graf @ 2018-12-02 23:42 UTC (permalink / raw)
To: u-boot
On 05.11.18 10:06, AKASHI Takahiro wrote:
> Currently, there is no easy way to add or modify UEFI variables.
> In particular, bootmgr supports BootOrder/BootXXXX variables, it is
> quite hard to define them as u-boot variables because they are represented
> in a complicated and encoded format.
>
> The new command, efishell, helps address these issues and give us
> more friendly interfaces:
> * efishell boot add: add BootXXXX variable
> * efishell boot rm: remove BootXXXX variable
> * efishell boot dump: display all BootXXXX variables
> * efishell boot order: set/display a boot order (BootOrder)
> * efishell setvar: set an UEFI variable (with limited functionality)
> * efishell dumpvar: display all UEFI variables
>
> As the name suggests, this command basically provides a subset fo UEFI
> shell commands with simplified functionality.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/Makefile | 2 +-
> cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
> cmd/efishell.h | 5 +
> 3 files changed, 679 insertions(+), 1 deletion(-)
> create mode 100644 cmd/efishell.c
> create mode 100644 cmd/efishell.h
>
> diff --git a/cmd/Makefile b/cmd/Makefile
> index d9cdaf6064b8..bd531d505a8e 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -24,7 +24,7 @@ obj-$(CONFIG_CMD_BINOP) += binop.o
> obj-$(CONFIG_CMD_BLOCK_CACHE) += blkcache.o
> obj-$(CONFIG_CMD_BMP) += bmp.o
> obj-$(CONFIG_CMD_BOOTCOUNT) += bootcount.o
> -obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o
> +obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o efishell.o
Please create a separate command line option for efishell and make it
disabled by default.
I think it's a really really useful debugging aid, but in a normal
environment people should only need to run efi binaries (plus bootmgr)
and modify efi variables, no?
> obj-$(CONFIG_CMD_BOOTMENU) += bootmenu.o
> obj-$(CONFIG_CMD_BOOTSTAGE) += bootstage.o
> obj-$(CONFIG_CMD_BOOTZ) += bootz.o
> diff --git a/cmd/efishell.c b/cmd/efishell.c
> new file mode 100644
> index 000000000000..abc8216c7bd6
> --- /dev/null
> +++ b/cmd/efishell.c
> @@ -0,0 +1,673 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * EFI Shell-like command
> + *
> + * Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
> + */
> +
> +#include <charset.h>
> +#include <common.h>
> +#include <command.h>
> +#include <efi_loader.h>
> +#include <environment.h>
> +#include <errno.h>
> +#include <exports.h>
> +#include <hexdump.h>
> +#include <malloc.h>
> +#include <search.h>
> +#include <linux/ctype.h>
> +#include <asm/global_data.h>
> +#include "efishell.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
Where in this file do you need gd?
> +
> +static void dump_var_data(char *data, unsigned long len)
> +{
> + char *start, *end, *p;
> + unsigned long pos, count;
> + char hex[3], line[9];
> + int i;
> +
> + end = data + len;
> + for (start = data, pos = 0; start < end; start += count, pos += count) {
> + count = end - start;
> + if (count > 16)
> + count = 16;
> +
> + /* count should be multiple of two */
> + printf("%08lx: ", pos);
> +
> + /* in hex format */
> + p = start;
> + for (i = 0; i < count / 2; p += 2, i++)
> + printf(" %c%c", *p, *(p + 1));
> + for (; i < 8; i++)
> + printf(" ");
> +
> + /* in character format */
> + p = start;
> + hex[2] = '\0';
> + for (i = 0; i < count / 2; i++) {
> + hex[0] = *p++;
> + hex[1] = *p++;
> + line[i] = simple_strtoul(hex, 0, 16);
> + if (line[i] < 0x20 || line[i] > 0x7f)
> + line[i] = '.';
> + }
> + line[i] = '\0';
> + printf(" %s\n", line);
> + }
> +}
> +
> +/*
> + * From efi_variable.c,
> + *
> + * Mapping between EFI variables and u-boot variables:
> + *
> + * efi_$guid_$varname = {attributes}(type)value
> + */
> +static int do_efi_dump_var(int argc, char * const argv[])
> +{
> + char regex[256];
> + char * const regexlist[] = {regex};
> + char *res = NULL, *start, *end;
> + int len;
> +
> + if (argc > 2)
> + return CMD_RET_USAGE;
> +
> + if (argc == 2)
> + snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_%s", argv[1]);
> + else
> + snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
> + debug("%s:%d grep uefi var %s\n", __func__, __LINE__, regex);
> +
> + len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
> + &res, 0, 1, regexlist);
> +
> + if (len < 0)
> + return CMD_RET_FAILURE;
> +
> + if (len > 0) {
> + end = res;
> + while (true) {
> + /* variable name */
> + start = strchr(end, '_');
> + if (!start)
> + break;
> + start = strchr(++start, '_');
> + if (!start)
> + break;
> + end = strchr(++start, '=');
> + if (!end)
> + break;
> + printf("%.*s:", (int)(end - start), start);
> + end++;
> +
> + /* value */
> + start = end;
> + end = strstr(start, "(blob)");
> + if (!end) {
> + putc('\n');
> + break;
> + }
> + end += 6;
> + printf(" %.*s\n", (int)(end - start), start);
> +
> + start = end;
> + end = strchr(start, '\n');
> + if (!end)
> + break;
> + dump_var_data(start, end - start);
> + }
> + free(res);
> +
> + if (len < 2 && argc == 2)
> + printf("%s: not found\n", argv[1]);
> + }
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int append_value(char **bufp, unsigned long *sizep, char *data)
> +{
> + char *tmp_buf = NULL, *new_buf = NULL, *value;
> + unsigned long len = 0;
> +
> + if (!strncmp(data, "=0x", 2)) { /* hexadecimal number */
> + union {
> + u8 u8;
> + u16 u16;
> + u32 u32;
> + u64 u64;
> + } tmp_data;
> + unsigned long hex_value;
> + void *hex_ptr;
> +
> + data += 3;
> + len = strlen(data);
> + if ((len & 0x1)) /* not multiple of two */
> + return -1;
> +
> + len /= 2;
> + if (len > 8)
> + return -1;
> + else if (len > 4)
> + len = 8;
> + else if (len > 2)
> + len = 4;
> +
> + /* convert hex hexadecimal number */
> + if (strict_strtoul(data, 16, &hex_value) < 0)
> + return -1;
> +
> + tmp_buf = malloc(len);
> + if (!tmp_buf)
> + return -1;
> +
> + if (len == 1) {
> + tmp_data.u8 = hex_value;
> + hex_ptr = &tmp_data.u8;
> + } else if (len == 2) {
> + tmp_data.u16 = hex_value;
> + hex_ptr = &tmp_data.u16;
> + } else if (len == 4) {
> + tmp_data.u32 = hex_value;
> + hex_ptr = &tmp_data.u32;
> + } else {
> + tmp_data.u64 = hex_value;
> + hex_ptr = &tmp_data.u64;
> + }
> + memcpy(tmp_buf, hex_ptr, len);
> + value = tmp_buf;
> +
> + } else if (!strncmp(data, "=H", 2)) { /* hexadecimal-byte array */
> + data += 2;
> + len = strlen(data);
> + if (len & 0x1) /* not multiple of two */
> + return -1;
> +
> + len /= 2;
> + tmp_buf = malloc(len);
> + if (!tmp_buf)
> + return -1;
> +
> + if (hex2bin((u8 *)tmp_buf, data, len) < 0)
> + return -1;
> +
> + value = tmp_buf;
> + } else { /* string */
> + if (!strncmp(data, "=\"", 2) || !strncmp(data, "=S\"", 3)) {
> + if (data[1] == '"')
> + data += 2;
> + else
> + data += 3;
> + value = data;
> + len = strlen(data) - 1;
> + if (data[len] != '"')
> + return -1;
> + } else {
> + value = data;
> + len = strlen(data);
> + }
> + }
> +
> + new_buf = realloc(*bufp, *sizep + len);
> + if (!new_buf)
> + goto out;
> +
> + memcpy(new_buf + *sizep, value, len);
> + *bufp = new_buf;
> + *sizep += len;
> +
> +out:
> + free(tmp_buf);
> +
> + return 0;
> +}
> +
> +static int do_efi_set_var(int argc, char * const argv[])
> +{
> + char *var_name, *value = NULL;
> + unsigned long size = 0;
> + u16 *var_name16, *p;
> + efi_guid_t guid;
> + efi_status_t ret;
> +
> + if (argc == 1)
> + return CMD_RET_SUCCESS;
> +
> + var_name = argv[1];
> + if (argc == 2) {
> + /* remove */
> + value = NULL;
> + size = 0;
> + } else { /* set */
> + argc -= 2;
> + argv += 2;
> +
> + for ( ; argc > 0; argc--, argv++)
> + if (append_value(&value, &size, argv[0]) < 0) {
> + ret = CMD_RET_FAILURE;
> + goto out;
> + }
> + }
> +
> + var_name16 = malloc((strlen(var_name) + 1) * 2);
> + p = var_name16;
> + utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
> +
> + guid = efi_global_variable_guid;
> + ret = efi_set_variable(var_name16, &guid,
> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS, size, value);
> + ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> +out:
> + return ret;
> +}
> +
> +static int do_efi_boot_add(int argc, char * const argv[])
> +{
> + int id;
> + char *endp;
> + char var_name[9];
> + u16 var_name16[9], *p;
> + efi_guid_t guid;
> + size_t label_len, label_len16;
> + u16 *label;
> + struct efi_device_path *device_path = NULL, *file_path = NULL;
> + struct efi_load_option lo;
> + void *data = NULL;
> + unsigned long size;
> + int ret;
> +
> + if (argc < 6 || argc > 7)
> + return CMD_RET_USAGE;
> +
> + id = (int)simple_strtoul(argv[1], &endp, 0);
> + if (*endp != '\0' || id > 0xffff)
> + return CMD_RET_FAILURE;
> +
> + sprintf(var_name, "Boot%04X", id);
> + p = var_name16;
> + utf8_utf16_strncpy(&p, var_name, 9);
> +
> + guid = efi_global_variable_guid;
> +
> + /* attributes */
> + lo.attributes = 0x1; /* always ACTIVE */
> +
> + /* label */
> + label_len = strlen(argv[2]);
> + label_len16 = utf8_utf16_strnlen(argv[2], label_len);
> + label = malloc((label_len16 + 1) * sizeof(u16));
> + if (!label)
> + return CMD_RET_FAILURE;
> + lo.label = label; /* label will be changed below */
> + utf8_utf16_strncpy(&label, argv[2], label_len);
> +
> + /* file path */
> + ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
> + &file_path);
> + if (ret != EFI_SUCCESS) {
> + ret = CMD_RET_FAILURE;
> + goto out;
> + }
> + lo.file_path = file_path;
> + lo.file_path_length = efi_dp_size(file_path)
> + + sizeof(struct efi_device_path); /* for END */
> +
> + /* optional data */
> + lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
> +
> + size = efi_serialize_load_option(&lo, (u8 **)&data);
> + if (!size) {
> + ret = CMD_RET_FAILURE;
> + goto out;
> + }
> +
> + ret = efi_set_variable(var_name16, &guid,
> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS, size, data);
> + ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> +out:
> + free(data);
> +#if 0 /* FIXME */
Eh, what? :)
> + free(device_path);
> + free(file_path);
> +#endif
> + free(lo.label);
> + return ret;
> +}
> +
> +static int do_efi_boot_rm(int argc, char * const argv[])
> +{
> + efi_guid_t guid;
> + int id, i;
> + char *endp;
> + char var_name[9];
> + u16 var_name16[9];
> + efi_status_t ret;
> +
> + if (argc == 1)
> + return CMD_RET_USAGE;
> +
> + guid = efi_global_variable_guid;
> + for (i = 1; i < argc; i++, argv++) {
> + id = (int)simple_strtoul(argv[1], &endp, 0);
> + if (*endp != '\0' || id > 0xffff)
> + return CMD_RET_FAILURE;
> +
> + sprintf(var_name, "Boot%04X", id);
> + utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
> +
> + ret = efi_set_variable(var_name16, &guid,
> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);
> + if (ret) {
> + printf("cannot remove Boot%04X", id);
> + return CMD_RET_FAILURE;
> + }
> + }
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static void show_efi_boot_opt_data(int id, void *data)
> +{
> + struct efi_load_option lo;
> + char *label, *p;
> + size_t label_len16, label_len;
> + u16 *dp_str;
> +
> + efi_deserialize_load_option(&lo, data);
> +
> + label_len16 = u16_strlen(lo.label);
> + label_len = utf16_utf8_strnlen(lo.label, label_len16);
> + label = malloc(label_len + 1);
> + if (!label)
> + return;
> + p = label;
> + utf16_utf8_strncpy(&p, lo.label, label_len16);
> +
> + printf("Boot%04X:\n", id);
> + printf("\tattributes: %c%c%c (0x%08x)\n",
> + /* ACTIVE */
> + lo.attributes & 0x1 ? 'A' : '-',
> + /* FORCE RECONNECT */
> + lo.attributes & 0x2 ? 'R' : '-',
> + /* HIDDEN */
> + lo.attributes & 0x8 ? 'H' : '-',
> + lo.attributes);
> + printf("\tlabel: %s\n", label);
> +
> + dp_str = efi_dp_str(lo.file_path);
> + printf("\tfile_path: %ls\n", dp_str);
> + efi_free_pool(dp_str);
> +
> + printf("\tdata: %s\n", lo.optional_data);
> +
> + free(label);
> +}
> +
> +static void show_efi_boot_opt(int id)
> +{
> + char var_name[9];
> + u16 var_name16[9], *p;
> + efi_guid_t guid;
> + void *data = NULL;
> + unsigned long size;
> + int ret;
> +
> + sprintf(var_name, "Boot%04X", id);
> + p = var_name16;
> + utf8_utf16_strncpy(&p, var_name, 9);
> + guid = efi_global_variable_guid;
> +
> + size = 0;
> + ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
> + if (ret == (int)EFI_BUFFER_TOO_SMALL) {
> + data = malloc(size);
> + ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
> + }
> + if (ret == EFI_SUCCESS) {
> + show_efi_boot_opt_data(id, data);
> + free(data);
> + } else if (ret == EFI_NOT_FOUND) {
> + printf("Boot%04X: not found\n", id);
Missing free?
> + }
> +}
> +
> +static int do_efi_boot_dump(int argc, char * const argv[])
> +{
> + char regex[256];
> + char * const regexlist[] = {regex};
> + char *variables = NULL, *boot, *value;
> + int len;
> + int id;
> +
> + if (argc > 1)
> + return CMD_RET_USAGE;
> +
> + snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+");
> +
> + len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
> + &variables, 0, 1, regexlist);
> +
> + if (!len)
> + return CMD_RET_SUCCESS;
> +
> + if (len < 0)
> + return CMD_RET_FAILURE;
> +
> + boot = variables;
> + while (*boot) {
> + value = strstr(boot, "Boot") + 4;
> + id = (int)simple_strtoul(value, NULL, 16);
> + show_efi_boot_opt(id);
> + boot = strchr(boot, '\n');
> + if (!*boot)
> + break;
> + boot++;
> + }
> + free(variables);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int show_efi_boot_order(void)
> +{
> + efi_guid_t guid;
> + u16 *bootorder = NULL;
> + unsigned long size;
> + int num, i;
> + char var_name[9];
> + u16 var_name16[9], *p16;
> + void *data;
> + struct efi_load_option lo;
> + char *label, *p;
> + size_t label_len16, label_len;
> + efi_status_t ret = CMD_RET_SUCCESS;
> +
> + guid = efi_global_variable_guid;
> + size = 0;
> + ret = efi_get_variable(L"BootOrder", &guid, NULL, &size, NULL);
> + if (ret == EFI_BUFFER_TOO_SMALL) {
> + bootorder = malloc(size);
> + ret = efi_get_variable(L"BootOrder", &guid, NULL, &size,
> + bootorder);
> + }
> + if (ret != EFI_SUCCESS) {
> + printf("BootOrder not defined\n");
Missing free(bootorder)
> + return CMD_RET_SUCCESS;
> + }
> +
> + num = size / sizeof(u16);
> + for (i = 0; i < num; i++) {
> + sprintf(var_name, "Boot%04X", bootorder[i]);
> + p16 = var_name16;
> + utf8_utf16_strncpy(&p16, var_name, 9);
> +
> + size = 0;
> + ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
> + if (ret != EFI_BUFFER_TOO_SMALL) {
> + printf("%2d: Boot%04X: (not defined)\n",
> + i + 1, bootorder[i]);
> + continue;
> + }
> +
> + data = malloc(size);
> + if (!data) {
> + ret = CMD_RET_FAILURE;
> + goto out;
> + }
> + ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
> + if (ret != EFI_SUCCESS) {
> + free(data);
> + ret = CMD_RET_FAILURE;
> + goto out;
Probably better to free(data) at out:, no?
> + }
> +
> + efi_deserialize_load_option(&lo, data);
> +
> + label_len16 = u16_strlen(lo.label);
> + label_len = utf16_utf8_strnlen(lo.label, label_len16);
> + label = malloc(label_len + 1);
> + if (!label) {
> + free(data);
> + ret = CMD_RET_FAILURE;
> + goto out;
> + }
> + p = label;
> + utf16_utf8_strncpy(&p, lo.label, label_len16);
> + printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
> + free(label);
> +
> + free(data);
> + }
> +out:
> + free(bootorder);
> +
> + return ret;
> +}
> +
> +static int do_efi_boot_order(int argc, char * const argv[])
> +{
> + u16 *bootorder = NULL;
> + unsigned long size;
> + int id, i;
> + char *endp;
> + efi_guid_t guid;
> + efi_status_t ret;
> +
> + if (argc == 1)
> + return show_efi_boot_order();
> +
> + argc--;
> + argv++;
> +
> + size = argc * sizeof(u16);
> + bootorder = malloc(size);
> + if (!bootorder)
> + return CMD_RET_FAILURE;
> +
> + for (i = 0; i < argc; i++) {
> + id = (int)simple_strtoul(argv[i], &endp, 0);
> + if (*endp != '\0' || id > 0xffff) {
> + printf("invalid value: %s\n", argv[i]);
> + ret = CMD_RET_FAILURE;
> + goto out;
> + }
> +
> + bootorder[i] = (u16)id;
> + }
> +
> + guid = efi_global_variable_guid;
> + ret = efi_set_variable(L"BootOrder", &guid,
> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> + EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);
> + ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> +out:
> + free(bootorder);
> +
> + return ret;
> +}
> +
> +static int do_efi_boot_opt(int argc, char * const argv[])
> +{
> + char *sub_command;
> +
> + if (argc < 2)
> + return CMD_RET_USAGE;
> +
> + argc--; argv++;
> + sub_command = argv[0];
> +
> + if (!strcmp(sub_command, "add"))
> + return do_efi_boot_add(argc, argv);
> + else if (!strcmp(sub_command, "rm"))
> + return do_efi_boot_rm(argc, argv);
> + else if (!strcmp(sub_command, "dump"))
> + return do_efi_boot_dump(argc, argv);
> + else if (!strcmp(sub_command, "order"))
> + return do_efi_boot_order(argc, argv);
> + else
> + return CMD_RET_USAGE;
> +}
> +
> +/* Interpreter command to configure EFI environment */
> +static int do_efishell(cmd_tbl_t *cmdtp, int flag,
> + int argc, char * const argv[])
> +{
> + char *command;
> + efi_status_t r;
> +
> + if (argc < 2)
> + return CMD_RET_USAGE;
> +
> + argc--; argv++;
> + command = argv[0];
> +
> + /* Initialize EFI drivers */
> + r = efi_init_obj_list();
> + if (r != EFI_SUCCESS) {
> + printf("Error: Cannot set up EFI drivers, r = %lu\n",
> + r & ~EFI_ERROR_MASK);
> + return CMD_RET_FAILURE;
> + }
> +
> + if (!strcmp(command, "boot"))
> + return do_efi_boot_opt(argc, argv);
> + else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore"))
> + return do_efi_dump_var(argc, argv);
> + else if (!strcmp(command, "setvar"))
> + return do_efi_set_var(argc, argv);
> + else
> + return CMD_RET_USAGE;
> +}
> +
> +#ifdef CONFIG_SYS_LONGHELP
> +static char efishell_help_text[] =
> + " - EFI Shell-like interface to configure EFI environment\n"
> + "\n"
> + "efishell boot add <bootid> <label> <interface> <device>[:<part>] <file path> <option>\n"
> + " - set uefi's BOOT variable\n"
> + "efishell boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
> + " - set/display uefi boot order\n"
> + "efishell boot dump\n"
> + " - display all uefi's BOOT variables\n"
> + "efishell boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
> + " - set/display uefi boot order\n"
> + "\n"
> + "efishell dumpvar [<name>]\n"
> + " - get uefi variable's value\n"
> + "efishell setvar <name> [<value>]\n"
> + " - set/delete uefi variable's value\n"
> + " <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n";
> +#endif
> +
> +U_BOOT_CMD(
> + efishell, 10, 0, do_efishell,
> + "Configure EFI environment",
> + efishell_help_text
> +);
> diff --git a/cmd/efishell.h b/cmd/efishell.h
> new file mode 100644
> index 000000000000..6f70b402b26b
> --- /dev/null
> +++ b/cmd/efishell.h
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <efi.h>
> +
> +efi_status_t efi_init_obj_list(void);
Why isn't this in efi_loader.h? That's the subsystem that exports it, no?
Alex
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 04/14] cmd: add efishell command
2018-12-02 23:42 ` Alexander Graf
@ 2018-12-03 6:42 ` AKASHI Takahiro
2018-12-03 14:01 ` Alexander Graf
0 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-12-03 6:42 UTC (permalink / raw)
To: u-boot
On Mon, Dec 03, 2018 at 12:42:43AM +0100, Alexander Graf wrote:
>
>
> On 05.11.18 10:06, AKASHI Takahiro wrote:
> > Currently, there is no easy way to add or modify UEFI variables.
> > In particular, bootmgr supports BootOrder/BootXXXX variables, it is
> > quite hard to define them as u-boot variables because they are represented
> > in a complicated and encoded format.
> >
> > The new command, efishell, helps address these issues and give us
> > more friendly interfaces:
> > * efishell boot add: add BootXXXX variable
> > * efishell boot rm: remove BootXXXX variable
> > * efishell boot dump: display all BootXXXX variables
> > * efishell boot order: set/display a boot order (BootOrder)
> > * efishell setvar: set an UEFI variable (with limited functionality)
> > * efishell dumpvar: display all UEFI variables
> >
> > As the name suggests, this command basically provides a subset fo UEFI
> > shell commands with simplified functionality.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > cmd/Makefile | 2 +-
> > cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
> > cmd/efishell.h | 5 +
> > 3 files changed, 679 insertions(+), 1 deletion(-)
> > create mode 100644 cmd/efishell.c
> > create mode 100644 cmd/efishell.h
> >
> > diff --git a/cmd/Makefile b/cmd/Makefile
> > index d9cdaf6064b8..bd531d505a8e 100644
> > --- a/cmd/Makefile
> > +++ b/cmd/Makefile
> > @@ -24,7 +24,7 @@ obj-$(CONFIG_CMD_BINOP) += binop.o
> > obj-$(CONFIG_CMD_BLOCK_CACHE) += blkcache.o
> > obj-$(CONFIG_CMD_BMP) += bmp.o
> > obj-$(CONFIG_CMD_BOOTCOUNT) += bootcount.o
> > -obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o
> > +obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o efishell.o
>
> Please create a separate command line option for efishell and make it
> disabled by default.
OK.
> I think it's a really really useful debugging aid, but in a normal
> environment people should only need to run efi binaries (plus bootmgr)
> and modify efi variables, no?
The ultimate goal would be to provide this command as a separate
application. In this case, however, it won't much different from
uefi shell itself :)
>
>
> > obj-$(CONFIG_CMD_BOOTMENU) += bootmenu.o
> > obj-$(CONFIG_CMD_BOOTSTAGE) += bootstage.o
> > obj-$(CONFIG_CMD_BOOTZ) += bootz.o
> > diff --git a/cmd/efishell.c b/cmd/efishell.c
> > new file mode 100644
> > index 000000000000..abc8216c7bd6
> > --- /dev/null
> > +++ b/cmd/efishell.c
> > @@ -0,0 +1,673 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * EFI Shell-like command
> > + *
> > + * Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
> > + */
> > +
> > +#include <charset.h>
> > +#include <common.h>
> > +#include <command.h>
> > +#include <efi_loader.h>
> > +#include <environment.h>
> > +#include <errno.h>
> > +#include <exports.h>
> > +#include <hexdump.h>
> > +#include <malloc.h>
> > +#include <search.h>
> > +#include <linux/ctype.h>
> > +#include <asm/global_data.h>
> > +#include "efishell.h"
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
>
> Where in this file do you need gd?
OK.
# I thought this should always be declared in any file by default.
> > +
> > +static void dump_var_data(char *data, unsigned long len)
> > +{
> > + char *start, *end, *p;
> > + unsigned long pos, count;
> > + char hex[3], line[9];
> > + int i;
> > +
> > + end = data + len;
> > + for (start = data, pos = 0; start < end; start += count, pos += count) {
> > + count = end - start;
> > + if (count > 16)
> > + count = 16;
> > +
> > + /* count should be multiple of two */
> > + printf("%08lx: ", pos);
> > +
> > + /* in hex format */
> > + p = start;
> > + for (i = 0; i < count / 2; p += 2, i++)
> > + printf(" %c%c", *p, *(p + 1));
> > + for (; i < 8; i++)
> > + printf(" ");
> > +
> > + /* in character format */
> > + p = start;
> > + hex[2] = '\0';
> > + for (i = 0; i < count / 2; i++) {
> > + hex[0] = *p++;
> > + hex[1] = *p++;
> > + line[i] = simple_strtoul(hex, 0, 16);
> > + if (line[i] < 0x20 || line[i] > 0x7f)
> > + line[i] = '.';
> > + }
> > + line[i] = '\0';
> > + printf(" %s\n", line);
> > + }
> > +}
> > +
> > +/*
> > + * From efi_variable.c,
> > + *
> > + * Mapping between EFI variables and u-boot variables:
> > + *
> > + * efi_$guid_$varname = {attributes}(type)value
> > + */
> > +static int do_efi_dump_var(int argc, char * const argv[])
> > +{
> > + char regex[256];
> > + char * const regexlist[] = {regex};
> > + char *res = NULL, *start, *end;
> > + int len;
> > +
> > + if (argc > 2)
> > + return CMD_RET_USAGE;
> > +
> > + if (argc == 2)
> > + snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_%s", argv[1]);
> > + else
> > + snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
> > + debug("%s:%d grep uefi var %s\n", __func__, __LINE__, regex);
> > +
> > + len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
> > + &res, 0, 1, regexlist);
> > +
> > + if (len < 0)
> > + return CMD_RET_FAILURE;
> > +
> > + if (len > 0) {
> > + end = res;
> > + while (true) {
> > + /* variable name */
> > + start = strchr(end, '_');
> > + if (!start)
> > + break;
> > + start = strchr(++start, '_');
> > + if (!start)
> > + break;
> > + end = strchr(++start, '=');
> > + if (!end)
> > + break;
> > + printf("%.*s:", (int)(end - start), start);
> > + end++;
> > +
> > + /* value */
> > + start = end;
> > + end = strstr(start, "(blob)");
> > + if (!end) {
> > + putc('\n');
> > + break;
> > + }
> > + end += 6;
> > + printf(" %.*s\n", (int)(end - start), start);
> > +
> > + start = end;
> > + end = strchr(start, '\n');
> > + if (!end)
> > + break;
> > + dump_var_data(start, end - start);
> > + }
> > + free(res);
> > +
> > + if (len < 2 && argc == 2)
> > + printf("%s: not found\n", argv[1]);
> > + }
> > +
> > + return CMD_RET_SUCCESS;
> > +}
> > +
> > +static int append_value(char **bufp, unsigned long *sizep, char *data)
> > +{
> > + char *tmp_buf = NULL, *new_buf = NULL, *value;
> > + unsigned long len = 0;
> > +
> > + if (!strncmp(data, "=0x", 2)) { /* hexadecimal number */
> > + union {
> > + u8 u8;
> > + u16 u16;
> > + u32 u32;
> > + u64 u64;
> > + } tmp_data;
> > + unsigned long hex_value;
> > + void *hex_ptr;
> > +
> > + data += 3;
> > + len = strlen(data);
> > + if ((len & 0x1)) /* not multiple of two */
> > + return -1;
> > +
> > + len /= 2;
> > + if (len > 8)
> > + return -1;
> > + else if (len > 4)
> > + len = 8;
> > + else if (len > 2)
> > + len = 4;
> > +
> > + /* convert hex hexadecimal number */
> > + if (strict_strtoul(data, 16, &hex_value) < 0)
> > + return -1;
> > +
> > + tmp_buf = malloc(len);
> > + if (!tmp_buf)
> > + return -1;
> > +
> > + if (len == 1) {
> > + tmp_data.u8 = hex_value;
> > + hex_ptr = &tmp_data.u8;
> > + } else if (len == 2) {
> > + tmp_data.u16 = hex_value;
> > + hex_ptr = &tmp_data.u16;
> > + } else if (len == 4) {
> > + tmp_data.u32 = hex_value;
> > + hex_ptr = &tmp_data.u32;
> > + } else {
> > + tmp_data.u64 = hex_value;
> > + hex_ptr = &tmp_data.u64;
> > + }
> > + memcpy(tmp_buf, hex_ptr, len);
> > + value = tmp_buf;
> > +
> > + } else if (!strncmp(data, "=H", 2)) { /* hexadecimal-byte array */
> > + data += 2;
> > + len = strlen(data);
> > + if (len & 0x1) /* not multiple of two */
> > + return -1;
> > +
> > + len /= 2;
> > + tmp_buf = malloc(len);
> > + if (!tmp_buf)
> > + return -1;
> > +
> > + if (hex2bin((u8 *)tmp_buf, data, len) < 0)
> > + return -1;
> > +
> > + value = tmp_buf;
> > + } else { /* string */
> > + if (!strncmp(data, "=\"", 2) || !strncmp(data, "=S\"", 3)) {
> > + if (data[1] == '"')
> > + data += 2;
> > + else
> > + data += 3;
> > + value = data;
> > + len = strlen(data) - 1;
> > + if (data[len] != '"')
> > + return -1;
> > + } else {
> > + value = data;
> > + len = strlen(data);
> > + }
> > + }
> > +
> > + new_buf = realloc(*bufp, *sizep + len);
> > + if (!new_buf)
> > + goto out;
> > +
> > + memcpy(new_buf + *sizep, value, len);
> > + *bufp = new_buf;
> > + *sizep += len;
> > +
> > +out:
> > + free(tmp_buf);
> > +
> > + return 0;
> > +}
> > +
> > +static int do_efi_set_var(int argc, char * const argv[])
> > +{
> > + char *var_name, *value = NULL;
> > + unsigned long size = 0;
> > + u16 *var_name16, *p;
> > + efi_guid_t guid;
> > + efi_status_t ret;
> > +
> > + if (argc == 1)
> > + return CMD_RET_SUCCESS;
> > +
> > + var_name = argv[1];
> > + if (argc == 2) {
> > + /* remove */
> > + value = NULL;
> > + size = 0;
> > + } else { /* set */
> > + argc -= 2;
> > + argv += 2;
> > +
> > + for ( ; argc > 0; argc--, argv++)
> > + if (append_value(&value, &size, argv[0]) < 0) {
> > + ret = CMD_RET_FAILURE;
> > + goto out;
> > + }
> > + }
> > +
> > + var_name16 = malloc((strlen(var_name) + 1) * 2);
> > + p = var_name16;
> > + utf8_utf16_strncpy(&p, var_name, strlen(var_name) + 1);
> > +
> > + guid = efi_global_variable_guid;
> > + ret = efi_set_variable(var_name16, &guid,
> > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS, size, value);
> > + ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> > +out:
> > + return ret;
> > +}
> > +
> > +static int do_efi_boot_add(int argc, char * const argv[])
> > +{
> > + int id;
> > + char *endp;
> > + char var_name[9];
> > + u16 var_name16[9], *p;
> > + efi_guid_t guid;
> > + size_t label_len, label_len16;
> > + u16 *label;
> > + struct efi_device_path *device_path = NULL, *file_path = NULL;
> > + struct efi_load_option lo;
> > + void *data = NULL;
> > + unsigned long size;
> > + int ret;
> > +
> > + if (argc < 6 || argc > 7)
> > + return CMD_RET_USAGE;
> > +
> > + id = (int)simple_strtoul(argv[1], &endp, 0);
> > + if (*endp != '\0' || id > 0xffff)
> > + return CMD_RET_FAILURE;
> > +
> > + sprintf(var_name, "Boot%04X", id);
> > + p = var_name16;
> > + utf8_utf16_strncpy(&p, var_name, 9);
> > +
> > + guid = efi_global_variable_guid;
> > +
> > + /* attributes */
> > + lo.attributes = 0x1; /* always ACTIVE */
> > +
> > + /* label */
> > + label_len = strlen(argv[2]);
> > + label_len16 = utf8_utf16_strnlen(argv[2], label_len);
> > + label = malloc((label_len16 + 1) * sizeof(u16));
> > + if (!label)
> > + return CMD_RET_FAILURE;
> > + lo.label = label; /* label will be changed below */
> > + utf8_utf16_strncpy(&label, argv[2], label_len);
> > +
> > + /* file path */
> > + ret = efi_dp_from_name(argv[3], argv[4], argv[5], &device_path,
> > + &file_path);
> > + if (ret != EFI_SUCCESS) {
> > + ret = CMD_RET_FAILURE;
> > + goto out;
> > + }
> > + lo.file_path = file_path;
> > + lo.file_path_length = efi_dp_size(file_path)
> > + + sizeof(struct efi_device_path); /* for END */
> > +
> > + /* optional data */
> > + lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
> > +
> > + size = efi_serialize_load_option(&lo, (u8 **)&data);
> > + if (!size) {
> > + ret = CMD_RET_FAILURE;
> > + goto out;
> > + }
> > +
> > + ret = efi_set_variable(var_name16, &guid,
> > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS, size, data);
> > + ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> > +out:
> > + free(data);
> > +#if 0 /* FIXME */
>
> Eh, what? :)
Well, in the past, I saw u-boot hang-out if free()'s were enabled.
Now I found that we must free data with efi_free_pool() instead of free().
> > + free(device_path);
> > + free(file_path);
> > +#endif
> > + free(lo.label);
> > + return ret;
> > +}
> > +
> > +static int do_efi_boot_rm(int argc, char * const argv[])
> > +{
> > + efi_guid_t guid;
> > + int id, i;
> > + char *endp;
> > + char var_name[9];
> > + u16 var_name16[9];
> > + efi_status_t ret;
> > +
> > + if (argc == 1)
> > + return CMD_RET_USAGE;
> > +
> > + guid = efi_global_variable_guid;
> > + for (i = 1; i < argc; i++, argv++) {
> > + id = (int)simple_strtoul(argv[1], &endp, 0);
> > + if (*endp != '\0' || id > 0xffff)
> > + return CMD_RET_FAILURE;
> > +
> > + sprintf(var_name, "Boot%04X", id);
> > + utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
> > +
> > + ret = efi_set_variable(var_name16, &guid,
> > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL);
> > + if (ret) {
> > + printf("cannot remove Boot%04X", id);
> > + return CMD_RET_FAILURE;
> > + }
> > + }
> > +
> > + return CMD_RET_SUCCESS;
> > +}
> > +
> > +static void show_efi_boot_opt_data(int id, void *data)
> > +{
> > + struct efi_load_option lo;
> > + char *label, *p;
> > + size_t label_len16, label_len;
> > + u16 *dp_str;
> > +
> > + efi_deserialize_load_option(&lo, data);
> > +
> > + label_len16 = u16_strlen(lo.label);
> > + label_len = utf16_utf8_strnlen(lo.label, label_len16);
> > + label = malloc(label_len + 1);
> > + if (!label)
> > + return;
> > + p = label;
> > + utf16_utf8_strncpy(&p, lo.label, label_len16);
> > +
> > + printf("Boot%04X:\n", id);
> > + printf("\tattributes: %c%c%c (0x%08x)\n",
> > + /* ACTIVE */
> > + lo.attributes & 0x1 ? 'A' : '-',
> > + /* FORCE RECONNECT */
> > + lo.attributes & 0x2 ? 'R' : '-',
> > + /* HIDDEN */
> > + lo.attributes & 0x8 ? 'H' : '-',
> > + lo.attributes);
> > + printf("\tlabel: %s\n", label);
> > +
> > + dp_str = efi_dp_str(lo.file_path);
> > + printf("\tfile_path: %ls\n", dp_str);
> > + efi_free_pool(dp_str);
> > +
> > + printf("\tdata: %s\n", lo.optional_data);
> > +
> > + free(label);
> > +}
> > +
> > +static void show_efi_boot_opt(int id)
> > +{
> > + char var_name[9];
> > + u16 var_name16[9], *p;
> > + efi_guid_t guid;
> > + void *data = NULL;
> > + unsigned long size;
> > + int ret;
> > +
> > + sprintf(var_name, "Boot%04X", id);
> > + p = var_name16;
> > + utf8_utf16_strncpy(&p, var_name, 9);
> > + guid = efi_global_variable_guid;
> > +
> > + size = 0;
> > + ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
> > + if (ret == (int)EFI_BUFFER_TOO_SMALL) {
> > + data = malloc(size);
> > + ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
> > + }
> > + if (ret == EFI_SUCCESS) {
> > + show_efi_boot_opt_data(id, data);
> > + free(data);
> > + } else if (ret == EFI_NOT_FOUND) {
> > + printf("Boot%04X: not found\n", id);
>
> Missing free?
Fix it.
> > + }
> > +}
> > +
> > +static int do_efi_boot_dump(int argc, char * const argv[])
> > +{
> > + char regex[256];
> > + char * const regexlist[] = {regex};
> > + char *variables = NULL, *boot, *value;
> > + int len;
> > + int id;
> > +
> > + if (argc > 1)
> > + return CMD_RET_USAGE;
> > +
> > + snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+");
> > +
> > + len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY,
> > + &variables, 0, 1, regexlist);
> > +
> > + if (!len)
> > + return CMD_RET_SUCCESS;
> > +
> > + if (len < 0)
> > + return CMD_RET_FAILURE;
> > +
> > + boot = variables;
> > + while (*boot) {
> > + value = strstr(boot, "Boot") + 4;
> > + id = (int)simple_strtoul(value, NULL, 16);
> > + show_efi_boot_opt(id);
> > + boot = strchr(boot, '\n');
> > + if (!*boot)
> > + break;
> > + boot++;
> > + }
> > + free(variables);
> > +
> > + return CMD_RET_SUCCESS;
> > +}
> > +
> > +static int show_efi_boot_order(void)
> > +{
> > + efi_guid_t guid;
> > + u16 *bootorder = NULL;
> > + unsigned long size;
> > + int num, i;
> > + char var_name[9];
> > + u16 var_name16[9], *p16;
> > + void *data;
> > + struct efi_load_option lo;
> > + char *label, *p;
> > + size_t label_len16, label_len;
> > + efi_status_t ret = CMD_RET_SUCCESS;
> > +
> > + guid = efi_global_variable_guid;
> > + size = 0;
> > + ret = efi_get_variable(L"BootOrder", &guid, NULL, &size, NULL);
> > + if (ret == EFI_BUFFER_TOO_SMALL) {
> > + bootorder = malloc(size);
> > + ret = efi_get_variable(L"BootOrder", &guid, NULL, &size,
> > + bootorder);
> > + }
> > + if (ret != EFI_SUCCESS) {
> > + printf("BootOrder not defined\n");
>
> Missing free(bootorder)
OK. In addition, I will modify the code to check for EFI_NOT_FOUND.
> > + return CMD_RET_SUCCESS;
> > + }
> > +
> > + num = size / sizeof(u16);
> > + for (i = 0; i < num; i++) {
> > + sprintf(var_name, "Boot%04X", bootorder[i]);
> > + p16 = var_name16;
> > + utf8_utf16_strncpy(&p16, var_name, 9);
> > +
> > + size = 0;
> > + ret = efi_get_variable(var_name16, &guid, NULL, &size, NULL);
> > + if (ret != EFI_BUFFER_TOO_SMALL) {
> > + printf("%2d: Boot%04X: (not defined)\n",
> > + i + 1, bootorder[i]);
> > + continue;
> > + }
> > +
> > + data = malloc(size);
> > + if (!data) {
> > + ret = CMD_RET_FAILURE;
> > + goto out;
> > + }
> > + ret = efi_get_variable(var_name16, &guid, NULL, &size, data);
> > + if (ret != EFI_SUCCESS) {
> > + free(data);
> > + ret = CMD_RET_FAILURE;
> > + goto out;
>
> Probably better to free(data) at out:, no?
It's possible, but 'data' is only allocated and used repeatedly in a loop,
so freeing it within a loop would look to be a good manner.
> > + }
> > +
> > + efi_deserialize_load_option(&lo, data);
> > +
> > + label_len16 = u16_strlen(lo.label);
> > + label_len = utf16_utf8_strnlen(lo.label, label_len16);
> > + label = malloc(label_len + 1);
> > + if (!label) {
> > + free(data);
> > + ret = CMD_RET_FAILURE;
> > + goto out;
> > + }
> > + p = label;
> > + utf16_utf8_strncpy(&p, lo.label, label_len16);
> > + printf("%2d: Boot%04X: %s\n", i + 1, bootorder[i], label);
> > + free(label);
> > +
> > + free(data);
> > + }
> > +out:
> > + free(bootorder);
> > +
> > + return ret;
> > +}
> > +
> > +static int do_efi_boot_order(int argc, char * const argv[])
> > +{
> > + u16 *bootorder = NULL;
> > + unsigned long size;
> > + int id, i;
> > + char *endp;
> > + efi_guid_t guid;
> > + efi_status_t ret;
> > +
> > + if (argc == 1)
> > + return show_efi_boot_order();
> > +
> > + argc--;
> > + argv++;
> > +
> > + size = argc * sizeof(u16);
> > + bootorder = malloc(size);
> > + if (!bootorder)
> > + return CMD_RET_FAILURE;
> > +
> > + for (i = 0; i < argc; i++) {
> > + id = (int)simple_strtoul(argv[i], &endp, 0);
> > + if (*endp != '\0' || id > 0xffff) {
> > + printf("invalid value: %s\n", argv[i]);
> > + ret = CMD_RET_FAILURE;
> > + goto out;
> > + }
> > +
> > + bootorder[i] = (u16)id;
> > + }
> > +
> > + guid = efi_global_variable_guid;
> > + ret = efi_set_variable(L"BootOrder", &guid,
> > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS, size, bootorder);
> > + ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
> > +out:
> > + free(bootorder);
> > +
> > + return ret;
> > +}
> > +
> > +static int do_efi_boot_opt(int argc, char * const argv[])
> > +{
> > + char *sub_command;
> > +
> > + if (argc < 2)
> > + return CMD_RET_USAGE;
> > +
> > + argc--; argv++;
> > + sub_command = argv[0];
> > +
> > + if (!strcmp(sub_command, "add"))
> > + return do_efi_boot_add(argc, argv);
> > + else if (!strcmp(sub_command, "rm"))
> > + return do_efi_boot_rm(argc, argv);
> > + else if (!strcmp(sub_command, "dump"))
> > + return do_efi_boot_dump(argc, argv);
> > + else if (!strcmp(sub_command, "order"))
> > + return do_efi_boot_order(argc, argv);
> > + else
> > + return CMD_RET_USAGE;
> > +}
> > +
> > +/* Interpreter command to configure EFI environment */
> > +static int do_efishell(cmd_tbl_t *cmdtp, int flag,
> > + int argc, char * const argv[])
> > +{
> > + char *command;
> > + efi_status_t r;
> > +
> > + if (argc < 2)
> > + return CMD_RET_USAGE;
> > +
> > + argc--; argv++;
> > + command = argv[0];
> > +
> > + /* Initialize EFI drivers */
> > + r = efi_init_obj_list();
> > + if (r != EFI_SUCCESS) {
> > + printf("Error: Cannot set up EFI drivers, r = %lu\n",
> > + r & ~EFI_ERROR_MASK);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + if (!strcmp(command, "boot"))
> > + return do_efi_boot_opt(argc, argv);
> > + else if (!strcmp(command, "dumpvar") || !strcmp(command, "dmpstore"))
> > + return do_efi_dump_var(argc, argv);
> > + else if (!strcmp(command, "setvar"))
> > + return do_efi_set_var(argc, argv);
> > + else
> > + return CMD_RET_USAGE;
> > +}
> > +
> > +#ifdef CONFIG_SYS_LONGHELP
> > +static char efishell_help_text[] =
> > + " - EFI Shell-like interface to configure EFI environment\n"
> > + "\n"
> > + "efishell boot add <bootid> <label> <interface> <device>[:<part>] <file path> <option>\n"
> > + " - set uefi's BOOT variable\n"
> > + "efishell boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n"
> > + " - set/display uefi boot order\n"
> > + "efishell boot dump\n"
> > + " - display all uefi's BOOT variables\n"
> > + "efishell boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
> > + " - set/display uefi boot order\n"
> > + "\n"
> > + "efishell dumpvar [<name>]\n"
> > + " - get uefi variable's value\n"
> > + "efishell setvar <name> [<value>]\n"
> > + " - set/delete uefi variable's value\n"
> > + " <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n";
> > +#endif
> > +
> > +U_BOOT_CMD(
> > + efishell, 10, 0, do_efishell,
> > + "Configure EFI environment",
> > + efishell_help_text
> > +);
> > diff --git a/cmd/efishell.h b/cmd/efishell.h
> > new file mode 100644
> > index 000000000000..6f70b402b26b
> > --- /dev/null
> > +++ b/cmd/efishell.h
> > @@ -0,0 +1,5 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +#include <efi.h>
> > +
> > +efi_status_t efi_init_obj_list(void);
>
> Why isn't this in efi_loader.h? That's the subsystem that exports it, no?
Agree. Moreover, I think of refactoring the code and moving efi_init_obj_list()
to a new file, lib/efi_loader/efi_setup.c, so that we can add more
features directly as part of the initialization code.
Features may include USB removable disk support for which I already posted
a patch set[1] and yet-coming capsule-on-disk support.
Actually, I have this refactoring patch in my local dev branch.
May I submit it as a separate one?
[1] https://lists.denx.de/pipermail/u-boot/2018-November/347493.html
Thanks,
-Takahiro Akashi
>
> Alex
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 04/14] cmd: add efishell command
2018-12-03 6:42 ` AKASHI Takahiro
@ 2018-12-03 14:01 ` Alexander Graf
0 siblings, 0 replies; 39+ messages in thread
From: Alexander Graf @ 2018-12-03 14:01 UTC (permalink / raw)
To: u-boot
On 03.12.18 07:42, AKASHI Takahiro wrote:
> On Mon, Dec 03, 2018 at 12:42:43AM +0100, Alexander Graf wrote:
>>
>>
>> On 05.11.18 10:06, AKASHI Takahiro wrote:
>>> Currently, there is no easy way to add or modify UEFI variables.
>>> In particular, bootmgr supports BootOrder/BootXXXX variables, it is
>>> quite hard to define them as u-boot variables because they are represented
>>> in a complicated and encoded format.
>>>
>>> The new command, efishell, helps address these issues and give us
>>> more friendly interfaces:
>>> * efishell boot add: add BootXXXX variable
>>> * efishell boot rm: remove BootXXXX variable
>>> * efishell boot dump: display all BootXXXX variables
>>> * efishell boot order: set/display a boot order (BootOrder)
>>> * efishell setvar: set an UEFI variable (with limited functionality)
>>> * efishell dumpvar: display all UEFI variables
>>>
>>> As the name suggests, this command basically provides a subset fo UEFI
>>> shell commands with simplified functionality.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>> ---
>>> cmd/Makefile | 2 +-
>>> cmd/efishell.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++
>>> cmd/efishell.h | 5 +
>>> 3 files changed, 679 insertions(+), 1 deletion(-)
>>> create mode 100644 cmd/efishell.c
>>> create mode 100644 cmd/efishell.h
>>>
>>> diff --git a/cmd/Makefile b/cmd/Makefile
>>> index d9cdaf6064b8..bd531d505a8e 100644
>>> --- a/cmd/Makefile
>>> +++ b/cmd/Makefile
>>> @@ -24,7 +24,7 @@ obj-$(CONFIG_CMD_BINOP) += binop.o
>>> obj-$(CONFIG_CMD_BLOCK_CACHE) += blkcache.o
>>> obj-$(CONFIG_CMD_BMP) += bmp.o
>>> obj-$(CONFIG_CMD_BOOTCOUNT) += bootcount.o
>>> -obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o
>>> +obj-$(CONFIG_CMD_BOOTEFI) += bootefi.o efishell.o
>>
>> Please create a separate command line option for efishell and make it
>> disabled by default.
>
> OK.
>
>> I think it's a really really useful debugging aid, but in a normal
>> environment people should only need to run efi binaries (plus bootmgr)
>> and modify efi variables, no?
>
> The ultimate goal would be to provide this command as a separate
> application. In this case, however, it won't much different from
> uefi shell itself :)
>
>>
>>
>>> obj-$(CONFIG_CMD_BOOTMENU) += bootmenu.o
>>> obj-$(CONFIG_CMD_BOOTSTAGE) += bootstage.o
>>> obj-$(CONFIG_CMD_BOOTZ) += bootz.o
>>> diff --git a/cmd/efishell.c b/cmd/efishell.c
>>> new file mode 100644
>>> index 000000000000..abc8216c7bd6
>>> --- /dev/null
>>> +++ b/cmd/efishell.c
>>> @@ -0,0 +1,673 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * EFI Shell-like command
>>> + *
>>> + * Copyright (c) 2018 AKASHI Takahiro, Linaro Limited
>>> + */
>>> +
>>> +#include <charset.h>
>>> +#include <common.h>
>>> +#include <command.h>
>>> +#include <efi_loader.h>
>>> +#include <environment.h>
>>> +#include <errno.h>
>>> +#include <exports.h>
>>> +#include <hexdump.h>
>>> +#include <malloc.h>
>>> +#include <search.h>
>>> +#include <linux/ctype.h>
>>> +#include <asm/global_data.h>
>>> +#include "efishell.h"
>>> +
>>> +DECLARE_GLOBAL_DATA_PTR;
>>
>> Where in this file do you need gd?
>
> OK.
> # I thought this should always be declared in any file by default.
You only need to declare it when you need gd :). Most efi_loader code
should be able to live just fine without.
[...]
>>> + /* optional data */
>>> + lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]);
>>> +
>>> + size = efi_serialize_load_option(&lo, (u8 **)&data);
>>> + if (!size) {
>>> + ret = CMD_RET_FAILURE;
>>> + goto out;
>>> + }
>>> +
>>> + ret = efi_set_variable(var_name16, &guid,
>>> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
>>> + EFI_VARIABLE_RUNTIME_ACCESS, size, data);
>>> + ret = (ret == EFI_SUCCESS ? CMD_RET_SUCCESS : CMD_RET_FAILURE);
>>> +out:
>>> + free(data);
>>> +#if 0 /* FIXME */
>>
>> Eh, what? :)
>
> Well, in the past, I saw u-boot hang-out if free()'s were enabled.
> Now I found that we must free data with efi_free_pool() instead of free().
It depends on how data was allocated, yes.
>
>>> + free(device_path);
>>> + free(file_path);
>>> +#endif
>>> + free(lo.label);
>>> + return ret;
>>> +}
>>> +
[...]
>>> diff --git a/cmd/efishell.h b/cmd/efishell.h
>>> new file mode 100644
>>> index 000000000000..6f70b402b26b
>>> --- /dev/null
>>> +++ b/cmd/efishell.h
>>> @@ -0,0 +1,5 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +
>>> +#include <efi.h>
>>> +
>>> +efi_status_t efi_init_obj_list(void);
>>
>> Why isn't this in efi_loader.h? That's the subsystem that exports it, no?
>
> Agree. Moreover, I think of refactoring the code and moving efi_init_obj_list()
> to a new file, lib/efi_loader/efi_setup.c, so that we can add more
> features directly as part of the initialization code.
Well, as Heinrich already indicated, the long term goal would be to have
UEFI handles implicitly generated for DM devices. So we wouldn't need to
maintain our own copy of the already existing object model.
Alex
> Features may include USB removable disk support for which I already posted
> a patch set[1] and yet-coming capsule-on-disk support.
>
> Actually, I have this refactoring patch in my local dev branch.
> May I submit it as a separate one?
>
> [1] https://lists.denx.de/pipermail/u-boot/2018-November/347493.html
>
> Thanks,
> -Takahiro Akashi
>
>
>>
>> Alex
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 05/14] cmd: efishell: add devices command
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
` (3 preceding siblings ...)
2018-11-05 9:06 ` [U-Boot] [PATCH v2 04/14] cmd: add efishell command AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-12-02 23:46 ` Alexander Graf
2018-11-05 9:06 ` [U-Boot] [PATCH v2 06/14] cmd: efishell: add drivers command AKASHI Takahiro
` (8 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
"devices" command prints all the uefi variables on the system.
=> efishell devices
Device Name
============================================
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
HD(2,MBR,0x086246ba,0x40800,0x3f800)
/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
HD(1,MBR,0x086246ba,0x800,0x40000)
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 87 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c
index abc8216c7bd6..f4fa3fdf28a7 100644
--- a/cmd/efishell.c
+++ b/cmd/efishell.c
@@ -21,6 +21,8 @@
DECLARE_GLOBAL_DATA_PTR;
+static const struct efi_boot_services *bs;
+
static void dump_var_data(char *data, unsigned long len)
{
char *start, *end, *p;
@@ -266,6 +268,84 @@ out:
return ret;
}
+static efi_handle_t *efi_get_handles_by_proto(efi_guid_t *guid)
+{
+ efi_handle_t *handles = NULL;
+ efi_uintn_t size = 0;
+ efi_status_t ret;
+
+ if (guid) {
+ ret = bs->locate_handle(BY_PROTOCOL, guid, NULL, &size,
+ handles);
+ if (ret == EFI_BUFFER_TOO_SMALL) {
+ handles = calloc(1, size);
+ if (!handles)
+ return NULL;
+
+ ret = bs->locate_handle(BY_PROTOCOL, guid, NULL, &size,
+ handles);
+ }
+ if (ret != EFI_SUCCESS) {
+ free(handles);
+ return NULL;
+ }
+ } else {
+ ret = bs->locate_handle(ALL_HANDLES, NULL, NULL, &size, NULL);
+ if (ret == EFI_BUFFER_TOO_SMALL) {
+ handles = calloc(1, size);
+ if (!handles)
+ return NULL;
+
+ ret = bs->locate_handle(ALL_HANDLES, NULL, NULL, &size,
+ handles);
+ }
+ if (ret != EFI_SUCCESS) {
+ free(handles);
+ return NULL;
+ }
+ }
+
+ return handles;
+}
+
+static int efi_get_device_handle_info(efi_handle_t handle, u16 **name)
+{
+ struct efi_device_path *dp;
+ efi_status_t ret;
+
+ ret = bs->open_protocol(handle, &efi_guid_device_path,
+ (void **)&dp, NULL /* FIXME */, NULL,
+ EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+ if (ret == EFI_SUCCESS) {
+ *name = efi_dp_str(dp);
+ return 0;
+ } else {
+ return -1;
+ }
+}
+
+static int do_efi_show_devices(int argc, char * const argv[])
+{
+ efi_handle_t *handles = NULL, *handle;
+ u16 *devname;
+
+ handles = efi_get_handles_by_proto(NULL);
+ if (!handles)
+ return CMD_RET_SUCCESS;
+
+ printf("Device Name\n");
+ printf("============================================\n");
+ for (handle = handles; *handle; handle++) {
+ if (!efi_get_device_handle_info(*handle, &devname)) {
+ printf("%ls\n", devname);
+ efi_free_pool(devname);
+ }
+ handle++;
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
static int do_efi_boot_add(int argc, char * const argv[])
{
int id;
@@ -625,6 +705,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag,
if (argc < 2)
return CMD_RET_USAGE;
+ bs = systab.boottime;
+
argc--; argv++;
command = argv[0];
@@ -642,6 +724,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag,
return do_efi_dump_var(argc, argv);
else if (!strcmp(command, "setvar"))
return do_efi_set_var(argc, argv);
+ else if (!strcmp(command, "devices"))
+ return do_efi_show_devices(argc, argv);
else
return CMD_RET_USAGE;
}
@@ -663,7 +747,9 @@ static char efishell_help_text[] =
" - get uefi variable's value\n"
"efishell setvar <name> [<value>]\n"
" - set/delete uefi variable's value\n"
- " <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n";
+ " <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n"
+ "efishell devices\n"
+ " - show uefi devices\n";
#endif
U_BOOT_CMD(
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 05/14] cmd: efishell: add devices command
2018-11-05 9:06 ` [U-Boot] [PATCH v2 05/14] cmd: efishell: add devices command AKASHI Takahiro
@ 2018-12-02 23:46 ` Alexander Graf
2018-12-03 7:02 ` AKASHI Takahiro
0 siblings, 1 reply; 39+ messages in thread
From: Alexander Graf @ 2018-12-02 23:46 UTC (permalink / raw)
To: u-boot
On 05.11.18 10:06, AKASHI Takahiro wrote:
> "devices" command prints all the uefi variables on the system.
> => efishell devices
> Device Name
> ============================================
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
> HD(2,MBR,0x086246ba,0x40800,0x3f800)
> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
> HD(1,MBR,0x086246ba,0x800,0x40000)
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 87 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/efishell.c b/cmd/efishell.c
> index abc8216c7bd6..f4fa3fdf28a7 100644
> --- a/cmd/efishell.c
> +++ b/cmd/efishell.c
> @@ -21,6 +21,8 @@
>
> DECLARE_GLOBAL_DATA_PTR;
>
> +static const struct efi_boot_services *bs;
Why do you need a local copy of this?
Alex
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 05/14] cmd: efishell: add devices command
2018-12-02 23:46 ` Alexander Graf
@ 2018-12-03 7:02 ` AKASHI Takahiro
2018-12-23 3:11 ` Alexander Graf
0 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-12-03 7:02 UTC (permalink / raw)
To: u-boot
On Mon, Dec 03, 2018 at 12:46:20AM +0100, Alexander Graf wrote:
>
>
> On 05.11.18 10:06, AKASHI Takahiro wrote:
> > "devices" command prints all the uefi variables on the system.
> > => efishell devices
> > Device Name
> > ============================================
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
> > HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
> > HD(1,MBR,0x086246ba,0x800,0x40000)
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 87 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/efishell.c b/cmd/efishell.c
> > index abc8216c7bd6..f4fa3fdf28a7 100644
> > --- a/cmd/efishell.c
> > +++ b/cmd/efishell.c
> > @@ -21,6 +21,8 @@
> >
> > DECLARE_GLOBAL_DATA_PTR;
> >
> > +static const struct efi_boot_services *bs;
>
> Why do you need a local copy of this?
Good point. It's because I followed the way boot manager does :)
I think that it would be good to do so since either boot manager or
efishell should ultimately be an independent efi application
in its nature.
What do you think?
FYI, one of the reasons why efishell cannot be an application
is that we lack an runtime service interface of GetNextVariableName()
which can be used to enumerate variables in dumpvar sub-command.
I also have a patch for adding GetNextVariableName() in my local dev branch.
I intend to post this patch along with capsule-on-disk support,
but I may be able to submit it separately if you like.
Thanks,
-Takahiro Akashi
>
> Alex
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 05/14] cmd: efishell: add devices command
2018-12-03 7:02 ` AKASHI Takahiro
@ 2018-12-23 3:11 ` Alexander Graf
2018-12-25 12:00 ` AKASHI Takahiro
0 siblings, 1 reply; 39+ messages in thread
From: Alexander Graf @ 2018-12-23 3:11 UTC (permalink / raw)
To: u-boot
On 03.12.18 08:02, AKASHI Takahiro wrote:
> On Mon, Dec 03, 2018 at 12:46:20AM +0100, Alexander Graf wrote:
>>
>>
>> On 05.11.18 10:06, AKASHI Takahiro wrote:
>>> "devices" command prints all the uefi variables on the system.
>>> => efishell devices
>>> Device Name
>>> ============================================
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
>>> HD(2,MBR,0x086246ba,0x40800,0x3f800)
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
>>> HD(1,MBR,0x086246ba,0x800,0x40000)
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>> ---
>>> cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 87 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/cmd/efishell.c b/cmd/efishell.c
>>> index abc8216c7bd6..f4fa3fdf28a7 100644
>>> --- a/cmd/efishell.c
>>> +++ b/cmd/efishell.c
>>> @@ -21,6 +21,8 @@
>>>
>>> DECLARE_GLOBAL_DATA_PTR;
>>>
>>> +static const struct efi_boot_services *bs;
>>
>> Why do you need a local copy of this?
>
> Good point. It's because I followed the way boot manager does :)
>
> I think that it would be good to do so since either boot manager or
> efishell should ultimately be an independent efi application
> in its nature.
>
> What do you think?
As mentioned in the other email thread, I think that we should
definitely evaluate to add the edk2 shell as built-in option.
That way for the "full-fledged" shell experience, your built-in code is
not needed. But I still believe it would be useful for quick and
built-in debugging.
Alex
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 05/14] cmd: efishell: add devices command
2018-12-23 3:11 ` Alexander Graf
@ 2018-12-25 12:00 ` AKASHI Takahiro
2018-12-26 8:00 ` Alexander Graf
0 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-12-25 12:00 UTC (permalink / raw)
To: u-boot
On Sun, Dec 23, 2018 at 04:11:14AM +0100, Alexander Graf wrote:
>
>
> On 03.12.18 08:02, AKASHI Takahiro wrote:
> > On Mon, Dec 03, 2018 at 12:46:20AM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 05.11.18 10:06, AKASHI Takahiro wrote:
> >>> "devices" command prints all the uefi variables on the system.
> >>> => efishell devices
> >>> Device Name
> >>> ============================================
> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
> >>> HD(2,MBR,0x086246ba,0x40800,0x3f800)
> >>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
> >>> HD(1,MBR,0x086246ba,0x800,0x40000)
> >>>
> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>> ---
> >>> cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >>> 1 file changed, 87 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/cmd/efishell.c b/cmd/efishell.c
> >>> index abc8216c7bd6..f4fa3fdf28a7 100644
> >>> --- a/cmd/efishell.c
> >>> +++ b/cmd/efishell.c
> >>> @@ -21,6 +21,8 @@
> >>>
> >>> DECLARE_GLOBAL_DATA_PTR;
> >>>
> >>> +static const struct efi_boot_services *bs;
> >>
> >> Why do you need a local copy of this?
> >
> > Good point. It's because I followed the way boot manager does :)
> >
> > I think that it would be good to do so since either boot manager or
> > efishell should ultimately be an independent efi application
> > in its nature.
> >
> > What do you think?
Back to your original comment: Why do you need a local copy of this?
Do you think we should use systab.boottime,runtime instead?
> As mentioned in the other email thread, I think that we should
> definitely evaluate to add the edk2 shell as built-in option.
Do you expect that I will add a new config, say, CONFIG_EFI_SHELL_PATH?
-Takahiro Akashi
> That way for the "full-fledged" shell experience, your built-in code is
> not needed. But I still believe it would be useful for quick and
> built-in debugging.
>
>
> Alex
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 05/14] cmd: efishell: add devices command
2018-12-25 12:00 ` AKASHI Takahiro
@ 2018-12-26 8:00 ` Alexander Graf
2019-01-07 8:22 ` AKASHI Takahiro
0 siblings, 1 reply; 39+ messages in thread
From: Alexander Graf @ 2018-12-26 8:00 UTC (permalink / raw)
To: u-boot
On 25.12.18 13:00, AKASHI Takahiro wrote:
> On Sun, Dec 23, 2018 at 04:11:14AM +0100, Alexander Graf wrote:
>>
>>
>> On 03.12.18 08:02, AKASHI Takahiro wrote:
>>> On Mon, Dec 03, 2018 at 12:46:20AM +0100, Alexander Graf wrote:
>>>>
>>>>
>>>> On 05.11.18 10:06, AKASHI Takahiro wrote:
>>>>> "devices" command prints all the uefi variables on the system.
>>>>> => efishell devices
>>>>> Device Name
>>>>> ============================================
>>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
>>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
>>>>> HD(2,MBR,0x086246ba,0x40800,0x3f800)
>>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
>>>>> HD(1,MBR,0x086246ba,0x800,0x40000)
>>>>>
>>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>>>> ---
>>>>> cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>> 1 file changed, 87 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/cmd/efishell.c b/cmd/efishell.c
>>>>> index abc8216c7bd6..f4fa3fdf28a7 100644
>>>>> --- a/cmd/efishell.c
>>>>> +++ b/cmd/efishell.c
>>>>> @@ -21,6 +21,8 @@
>>>>>
>>>>> DECLARE_GLOBAL_DATA_PTR;
>>>>>
>>>>> +static const struct efi_boot_services *bs;
>>>>
>>>> Why do you need a local copy of this?
>>>
>>> Good point. It's because I followed the way boot manager does :)
>>>
>>> I think that it would be good to do so since either boot manager or
>>> efishell should ultimately be an independent efi application
>>> in its nature.
>>>
>>> What do you think?
>
> Back to your original comment: Why do you need a local copy of this?
>
> Do you think we should use systab.boottime,runtime instead?
Sure, that's one thing. Or you could make it a local variable. Or you
could even do a #define in the file. But that static const here will
most likely waste space in .bss and does not take into account when
someone patches the systab.
>> As mentioned in the other email thread, I think that we should
>> definitely evaluate to add the edk2 shell as built-in option.
>
> Do you expect that I will add a new config, say, CONFIG_EFI_SHELL_PATH?
Doesn't have to be you, but it might be useful to have something like
that, yes.
CONFIG_CMD_BOOTEFI_SHELL=y
CONFIG_CMD_BOOTEFI_SHELL_PATH=shell.efi
which then would work the same as CONFIG_CMD_BOOTEFI_HELLO, but simply
execute a precompiled version of the UEFI shell. IIRC the UEFI shell
also supports passing arguments via the command line (load_options ->
"bootargs" variable).
So with that you should be able to run
U-Boot# setenv bootargs memmap; bootefi shell
and get a list of the UEFI memory map.
Alex
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 05/14] cmd: efishell: add devices command
2018-12-26 8:00 ` Alexander Graf
@ 2019-01-07 8:22 ` AKASHI Takahiro
0 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2019-01-07 8:22 UTC (permalink / raw)
To: u-boot
On Wed, Dec 26, 2018 at 09:00:42AM +0100, Alexander Graf wrote:
>
>
> On 25.12.18 13:00, AKASHI Takahiro wrote:
> > On Sun, Dec 23, 2018 at 04:11:14AM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 03.12.18 08:02, AKASHI Takahiro wrote:
> >>> On Mon, Dec 03, 2018 at 12:46:20AM +0100, Alexander Graf wrote:
> >>>>
> >>>>
> >>>> On 05.11.18 10:06, AKASHI Takahiro wrote:
> >>>>> "devices" command prints all the uefi variables on the system.
> >>>>> => efishell devices
> >>>>> Device Name
> >>>>> ============================================
> >>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> >>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> >>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
> >>>>> HD(2,MBR,0x086246ba,0x40800,0x3f800)
> >>>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/\
> >>>>> HD(1,MBR,0x086246ba,0x800,0x40000)
> >>>>>
> >>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>>>> ---
> >>>>> cmd/efishell.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >>>>> 1 file changed, 87 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/cmd/efishell.c b/cmd/efishell.c
> >>>>> index abc8216c7bd6..f4fa3fdf28a7 100644
> >>>>> --- a/cmd/efishell.c
> >>>>> +++ b/cmd/efishell.c
> >>>>> @@ -21,6 +21,8 @@
> >>>>>
> >>>>> DECLARE_GLOBAL_DATA_PTR;
> >>>>>
> >>>>> +static const struct efi_boot_services *bs;
> >>>>
> >>>> Why do you need a local copy of this?
> >>>
> >>> Good point. It's because I followed the way boot manager does :)
> >>>
> >>> I think that it would be good to do so since either boot manager or
> >>> efishell should ultimately be an independent efi application
> >>> in its nature.
> >>>
> >>> What do you think?
> >
> > Back to your original comment: Why do you need a local copy of this?
> >
> > Do you think we should use systab.boottime,runtime instead?
>
> Sure, that's one thing. Or you could make it a local variable. Or you
> could even do a #define in the file. But that static const here will
> most likely waste space in .bss and does not take into account when
> someone patches the systab.
OK, I will add a macro definition.
-Takahiro Akashi
> >> As mentioned in the other email thread, I think that we should
> >> definitely evaluate to add the edk2 shell as built-in option.
> >
> > Do you expect that I will add a new config, say, CONFIG_EFI_SHELL_PATH?
>
> Doesn't have to be you, but it might be useful to have something like
> that, yes.
>
> CONFIG_CMD_BOOTEFI_SHELL=y
> CONFIG_CMD_BOOTEFI_SHELL_PATH=shell.efi
>
> which then would work the same as CONFIG_CMD_BOOTEFI_HELLO, but simply
> execute a precompiled version of the UEFI shell. IIRC the UEFI shell
> also supports passing arguments via the command line (load_options ->
> "bootargs" variable).
>
> So with that you should be able to run
>
> U-Boot# setenv bootargs memmap; bootefi shell
>
> and get a list of the UEFI memory map.
>
>
> Alex
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 06/14] cmd: efishell: add drivers command
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
` (4 preceding siblings ...)
2018-11-05 9:06 ` [U-Boot] [PATCH v2 05/14] cmd: efishell: add devices command AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-11-05 9:06 ` [U-Boot] [PATCH v2 07/14] cmd: efishell: add images command AKASHI Takahiro
` (7 subsequent siblings)
13 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
"drivers" command prints all the uefi drivers on the system.
=> efi drivers
Driver Name Image Path
============================================
(unknown) <NULL>+(not found)
guid: 18a031ab-b443-4d1a-a5c0-0c09261e9f71
Currently, no useful information can be printed.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/efishell.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c
index f4fa3fdf28a7..2f54f7803ac5 100644
--- a/cmd/efishell.c
+++ b/cmd/efishell.c
@@ -346,6 +346,95 @@ static int do_efi_show_devices(int argc, char * const argv[])
return CMD_RET_SUCCESS;
}
+static int efi_get_driver_handle_info(efi_handle_t handle, u16 **name,
+ u16 **devname, u16 **filename)
+{
+ struct efi_driver_binding_protocol *binding;
+ struct efi_loaded_image *image;
+ efi_status_t ret;
+
+ ret = bs->open_protocol(handle, &efi_guid_driver_binding_protocol,
+ (void **)&binding, NULL, NULL,
+ EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+ if (ret != EFI_SUCCESS)
+ return -1;
+
+ ret = bs->open_protocol(binding->image_handle, &efi_guid_loaded_image,
+ (void **)&image, NULL /* FIXME */, NULL,
+ EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+ if (ret != EFI_SUCCESS)
+ goto e_out;
+
+ /*
+ * TODO:
+ * handle image->device_handle,
+ * use append_device_path()
+ */
+ *devname = NULL;
+ *filename = efi_dp_str(image->file_path);
+
+ return 0;
+
+e_out:
+ *devname = NULL;
+ *filename = L"(not found)";
+
+ return 0;
+}
+
+static int do_efi_show_drivers(int argc, char * const argv[])
+{
+ efi_handle_t *handles = NULL, *handle;
+ efi_uintn_t size = 0;
+ u16 *drvname, *devname, *filename;
+ efi_status_t ret;
+ int i;
+
+ ret = bs->locate_handle(BY_PROTOCOL, &efi_guid_driver_binding_protocol,
+ NULL, &size, NULL);
+ if (ret == EFI_BUFFER_TOO_SMALL) {
+ handles = calloc(1, size);
+ if (!handles)
+ return CMD_RET_FAILURE;
+
+ ret = bs->locate_handle(BY_PROTOCOL,
+ &efi_guid_driver_binding_protocol,
+ NULL, &size, handles);
+ }
+ if (ret != EFI_SUCCESS) {
+ free(handles);
+ return CMD_RET_FAILURE;
+ }
+
+ printf("Driver Name Image Path\n");
+ printf("============================================\n");
+ handle = handles;
+ for (i = 0; i < size / sizeof(*handle); i++) {
+ if (!efi_get_driver_handle_info(*handle, &drvname, &devname,
+ &filename)) {
+ printf("%-16s%ls+%ls\n",
+ "(unknown)", devname, filename);
+ efi_free_pool(devname);
+ efi_free_pool(filename);
+
+ /* TODO: no other info */
+ struct efi_object *efiobj;
+ struct list_head *lhandle;
+ struct efi_handler *protocol;
+
+ efiobj = efi_search_obj(*handle);
+ list_for_each(lhandle, &efiobj->protocols) {
+ protocol = list_entry(lhandle,
+ struct efi_handler, link);
+ printf(" guid: %pUl\n", protocol->guid);
+ }
+ }
+ handle++;
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
static int do_efi_boot_add(int argc, char * const argv[])
{
int id;
@@ -726,6 +815,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag,
return do_efi_set_var(argc, argv);
else if (!strcmp(command, "devices"))
return do_efi_show_devices(argc, argv);
+ else if (!strcmp(command, "drivers"))
+ return do_efi_show_drivers(argc, argv);
else
return CMD_RET_USAGE;
}
@@ -749,7 +840,9 @@ static char efishell_help_text[] =
" - set/delete uefi variable's value\n"
" <value> may be \"=\"...\"\", \"=0x...\" (set) or \"=\" (delete)\n"
"efishell devices\n"
- " - show uefi devices\n";
+ " - show uefi devices\n"
+ "efishell drivers\n"
+ " - show uefi drivers\n";
#endif
U_BOOT_CMD(
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 07/14] cmd: efishell: add images command
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
` (5 preceding siblings ...)
2018-11-05 9:06 ` [U-Boot] [PATCH v2 06/14] cmd: efishell: add drivers command AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-11-05 9:06 ` [U-Boot] [PATCH v2 08/14] cmd: efishell: add memmap command AKASHI Takahiro
` (6 subsequent siblings)
13 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
"images" command prints loaded images-related information.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/efishell.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c
index 2f54f7803ac5..b5050d63fa22 100644
--- a/cmd/efishell.c
+++ b/cmd/efishell.c
@@ -435,6 +435,13 @@ static int do_efi_show_drivers(int argc, char * const argv[])
return CMD_RET_SUCCESS;
}
+static int do_efi_show_images(int argc, char * const argv[])
+{
+ efi_print_image_infos(NULL);
+
+ return CMD_RET_SUCCESS;
+}
+
static int do_efi_boot_add(int argc, char * const argv[])
{
int id;
@@ -817,6 +824,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag,
return do_efi_show_devices(argc, argv);
else if (!strcmp(command, "drivers"))
return do_efi_show_drivers(argc, argv);
+ else if (!strcmp(command, "images"))
+ return do_efi_show_images(argc, argv);
else
return CMD_RET_USAGE;
}
@@ -842,7 +851,9 @@ static char efishell_help_text[] =
"efishell devices\n"
" - show uefi devices\n"
"efishell drivers\n"
- " - show uefi drivers\n";
+ " - show uefi drivers\n"
+ "efishell images\n"
+ " - show loaded images\n";
#endif
U_BOOT_CMD(
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 08/14] cmd: efishell: add memmap command
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
` (6 preceding siblings ...)
2018-11-05 9:06 ` [U-Boot] [PATCH v2 07/14] cmd: efishell: add images command AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-12-02 23:48 ` Alexander Graf
2018-11-05 9:06 ` [U-Boot] [PATCH v2 09/14] cmd: efishell: add dh command AKASHI Takahiro
` (5 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
"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/efishell.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 77 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c
index b5050d63fa22..3e1118a407ff 100644
--- a/cmd/efishell.c
+++ b/cmd/efishell.c
@@ -442,6 +442,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 *map = NULL;
+ efi_uintn_t map_size = 0;
+ int i, sep;
+ efi_status_t ret;
+
+ ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
+ if (ret == EFI_BUFFER_TOO_SMALL) {
+ map = malloc(map_size);
+ if (!map)
+ return CMD_RET_FAILURE;
+ ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
+ }
+ if (ret != EFI_SUCCESS)
+ return CMD_RET_FAILURE;
+
+ ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
+ if (ret != EFI_SUCCESS)
+ return CMD_RET_FAILURE;
+
+ printf("Type Start End Attributes\n");
+ printf("================ ================ ================ ==========\n");
+ for (i = 0; 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');
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
static int do_efi_boot_add(int argc, char * const argv[])
{
int id;
@@ -826,6 +898,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag,
return do_efi_show_drivers(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;
}
@@ -853,7 +927,9 @@ static char efishell_help_text[] =
"efishell drivers\n"
" - show uefi drivers\n"
"efishell images\n"
- " - show loaded images\n";
+ " - show loaded images\n"
+ "efishell memmap\n"
+ " - show uefi memory map\n";
#endif
U_BOOT_CMD(
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 08/14] cmd: efishell: add memmap command
2018-11-05 9:06 ` [U-Boot] [PATCH v2 08/14] cmd: efishell: add memmap command AKASHI Takahiro
@ 2018-12-02 23:48 ` Alexander Graf
2018-12-03 7:10 ` AKASHI Takahiro
0 siblings, 1 reply; 39+ messages in thread
From: Alexander Graf @ 2018-12-02 23:48 UTC (permalink / raw)
To: u-boot
On 05.11.18 10:06, 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/efishell.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/efishell.c b/cmd/efishell.c
> index b5050d63fa22..3e1118a407ff 100644
> --- a/cmd/efishell.c
> +++ b/cmd/efishell.c
> @@ -442,6 +442,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 *map = NULL;
> + efi_uintn_t map_size = 0;
> + int i, sep;
> + efi_status_t ret;
> +
> + ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
> + if (ret == EFI_BUFFER_TOO_SMALL) {
> + map = malloc(map_size);
Where is the free() to this malloc()?
Alex
> + if (!map)
> + return CMD_RET_FAILURE;
> + ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
> + }
> + if (ret != EFI_SUCCESS)
> + return CMD_RET_FAILURE;
> +
> + ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
> + if (ret != EFI_SUCCESS)
> + return CMD_RET_FAILURE;
> +
> + printf("Type Start End Attributes\n");
> + printf("================ ================ ================ ==========\n");
> + for (i = 0; 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');
> + }
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> static int do_efi_boot_add(int argc, char * const argv[])
> {
> int id;
> @@ -826,6 +898,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag,
> return do_efi_show_drivers(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;
> }
> @@ -853,7 +927,9 @@ static char efishell_help_text[] =
> "efishell drivers\n"
> " - show uefi drivers\n"
> "efishell images\n"
> - " - show loaded images\n";
> + " - show loaded images\n"
> + "efishell memmap\n"
> + " - show uefi memory map\n";
> #endif
>
> U_BOOT_CMD(
>
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 08/14] cmd: efishell: add memmap command
2018-12-02 23:48 ` Alexander Graf
@ 2018-12-03 7:10 ` AKASHI Takahiro
0 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2018-12-03 7:10 UTC (permalink / raw)
To: u-boot
On Mon, Dec 03, 2018 at 12:48:28AM +0100, Alexander Graf wrote:
>
>
> On 05.11.18 10:06, 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/efishell.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 77 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/efishell.c b/cmd/efishell.c
> > index b5050d63fa22..3e1118a407ff 100644
> > --- a/cmd/efishell.c
> > +++ b/cmd/efishell.c
> > @@ -442,6 +442,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 *map = NULL;
> > + efi_uintn_t map_size = 0;
> > + int i, sep;
> > + efi_status_t ret;
> > +
> > + ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
> > + if (ret == EFI_BUFFER_TOO_SMALL) {
> > + map = malloc(map_size);
>
> Where is the free() to this malloc()?
OK, fix it.
Thanks,
-Takahiro Akashi
>
> Alex
>
> > + if (!map)
> > + return CMD_RET_FAILURE;
> > + ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
> > + }
> > + if (ret != EFI_SUCCESS)
> > + return CMD_RET_FAILURE;
> > +
> > + ret = efi_get_memory_map(&map_size, map, NULL, NULL, NULL);
> > + if (ret != EFI_SUCCESS)
> > + return CMD_RET_FAILURE;
> > +
> > + printf("Type Start End Attributes\n");
> > + printf("================ ================ ================ ==========\n");
> > + for (i = 0; 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');
> > + }
> > +
> > + return CMD_RET_SUCCESS;
> > +}
> > +
> > static int do_efi_boot_add(int argc, char * const argv[])
> > {
> > int id;
> > @@ -826,6 +898,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag,
> > return do_efi_show_drivers(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;
> > }
> > @@ -853,7 +927,9 @@ static char efishell_help_text[] =
> > "efishell drivers\n"
> > " - show uefi drivers\n"
> > "efishell images\n"
> > - " - show loaded images\n";
> > + " - show loaded images\n"
> > + "efishell memmap\n"
> > + " - show uefi memory map\n";
> > #endif
> >
> > U_BOOT_CMD(
> >
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 09/14] cmd: efishell: add dh command
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
` (7 preceding siblings ...)
2018-11-05 9:06 ` [U-Boot] [PATCH v2 08/14] cmd: efishell: add memmap command AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-11-05 9:06 ` [U-Boot] [PATCH v2 10/14] cmd: bootefi: carve out fdt parameter handling AKASHI Takahiro
` (4 subsequent siblings)
13 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
"dh" command prints all the uefi handles used in the system.
=> efishell dh
(T.B.D.)
0: (protocol info not available)
1: (protocol info not available)
2: (protocol info not available)
3: (protocol info not available)
4: (protocol info not available)
5: (protocol info not available)
6: (protocol info not available)
7: (protocol info not available)
8: (protocol info not available)
9: (protocol info not available)
10: (protocol info not available)
11: (protocol info not available)
12: (protocol info not available)
13: (protocol info not available)
14: (protocol info not available)
15: (protocol info not available)
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/efishell.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c
index 3e1118a407ff..bd2b99e74079 100644
--- a/cmd/efishell.c
+++ b/cmd/efishell.c
@@ -514,6 +514,33 @@ static int do_efi_show_memmap(int argc, char * const argv[])
return CMD_RET_SUCCESS;
}
+static char *efi_get_proto_info(efi_handle_t handle)
+{
+ return strdup("(protocol info not available)");
+}
+
+static int do_efi_show_handles(int argc, char * const argv[])
+{
+ efi_handle_t *handles = NULL, *handle;
+ char *info;
+ int i;
+
+ handles = efi_get_handles_by_proto(NULL);
+ if (!handles)
+ return CMD_RET_SUCCESS;
+
+ for (handle = handles, i = 0; *handle; handle++, i++) {
+ /* TODO: depends on protocols */
+ info = efi_get_proto_info(*handle);
+ printf("%d: %s\n", i, info ?: "");
+ free(info);
+ }
+
+ free(handles);
+
+ return CMD_RET_SUCCESS;
+}
+
static int do_efi_boot_add(int argc, char * const argv[])
{
int id;
@@ -900,6 +927,8 @@ static int do_efishell(cmd_tbl_t *cmdtp, int flag,
return do_efi_show_images(argc, argv);
else if (!strcmp(command, "memmap"))
return do_efi_show_memmap(argc, argv);
+ else if (!strcmp(command, "dh"))
+ return do_efi_show_handles(argc, argv);
else
return CMD_RET_USAGE;
}
@@ -929,7 +958,9 @@ static char efishell_help_text[] =
"efishell images\n"
" - show loaded images\n"
"efishell memmap\n"
- " - show uefi memory map\n";
+ " - show uefi memory map\n"
+ "efishell dh\n"
+ " - show uefi handles\n";
#endif
U_BOOT_CMD(
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 10/14] cmd: bootefi: carve out fdt parameter handling
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
` (8 preceding siblings ...)
2018-11-05 9:06 ` [U-Boot] [PATCH v2 09/14] cmd: efishell: add dh command AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-12-02 23:50 ` Alexander Graf
2018-11-05 9:06 ` [U-Boot] [PATCH v2 11/14] cmd: bootefi: run an EFI application of a specific load option AKASHI Takahiro
` (3 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
The current way how command parameters, particularly "fdt addr," are
handled makes it a bit complicated to add a subcommand-specific parameter.
So just refactor the code and extract efi_handle_fdt().
This commit is a preparatory change for enhancing bootmgr sub-command.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/bootefi.c | 51 +++++++++++++++++++++++++++++++++-----------------
cmd/efishell.h | 1 +
2 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 5d61819f1f5c..3aedf5a09f93 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -356,6 +356,31 @@ static efi_status_t efi_install_fdt(ulong fdt_addr)
return ret;
}
+int efi_handle_fdt(char *fdt_opt)
+{
+ unsigned long fdt_addr;
+ efi_status_t r;
+
+ if (fdt_opt) {
+ fdt_addr = simple_strtoul(fdt_opt, NULL, 16);
+ if (!fdt_addr && *fdt_opt != '0')
+ return CMD_RET_USAGE;
+
+ /* Install device tree */
+ r = efi_install_fdt(fdt_addr);
+ if (r != EFI_SUCCESS) {
+ printf("ERROR: failed to install device tree\n");
+ return CMD_RET_FAILURE;
+ }
+ } else {
+ /* Remove device tree. EFI_NOT_FOUND can be ignored here */
+ efi_install_configuration_table(&efi_guid_fdt, NULL);
+ printf("WARNING: booting without device tree\n");
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
/**
* do_bootefi_exec() - execute EFI binary
*
@@ -511,7 +536,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
unsigned long addr;
char *saddr;
efi_status_t r;
- unsigned long fdt_addr;
/* Allow unaligned memory access */
allow_unaligned();
@@ -527,21 +551,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
if (argc < 2)
return CMD_RET_USAGE;
- if (argc > 2) {
- fdt_addr = simple_strtoul(argv[2], NULL, 16);
- if (!fdt_addr && *argv[2] != '0')
- return CMD_RET_USAGE;
- /* Install device tree */
- r = efi_install_fdt(fdt_addr);
- if (r != EFI_SUCCESS) {
- printf("ERROR: failed to install device tree\n");
- return CMD_RET_FAILURE;
- }
- } else {
- /* Remove device tree. EFI_NOT_FOUND can be ignored here */
- efi_install_configuration_table(&efi_guid_fdt, NULL);
- printf("WARNING: booting without device tree\n");
- }
#ifdef CONFIG_CMD_BOOTEFI_HELLO
if (!strcmp(argv[1], "hello")) {
ulong size = __efi_helloworld_end - __efi_helloworld_begin;
@@ -559,6 +568,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
struct efi_loaded_image_obj *image_obj;
struct efi_loaded_image *loaded_image_info;
+ if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
+ return CMD_RET_FAILURE;
+
/* Construct a dummy device path. */
bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
(uintptr_t)&efi_selftest,
@@ -583,7 +595,10 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
} else
#endif
if (!strcmp(argv[1], "bootmgr")) {
- return do_bootefi_bootmgr_exec();
+ if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
+ return CMD_RET_FAILURE;
+
+ return do_bootefi_bootmgr_exec(boot_id);
} else {
saddr = argv[1];
@@ -592,6 +607,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
if (!addr && *saddr != '0')
return CMD_RET_USAGE;
+ if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
+ return CMD_RET_FAILURE;
}
printf("## Starting EFI application at %08lx ...\n", addr);
diff --git a/cmd/efishell.h b/cmd/efishell.h
index 6f70b402b26b..1e5764a8a38d 100644
--- a/cmd/efishell.h
+++ b/cmd/efishell.h
@@ -3,3 +3,4 @@
#include <efi.h>
efi_status_t efi_init_obj_list(void);
+int efi_handle_fdt(char *fdt_opt);
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 10/14] cmd: bootefi: carve out fdt parameter handling
2018-11-05 9:06 ` [U-Boot] [PATCH v2 10/14] cmd: bootefi: carve out fdt parameter handling AKASHI Takahiro
@ 2018-12-02 23:50 ` Alexander Graf
2018-12-03 7:33 ` AKASHI Takahiro
0 siblings, 1 reply; 39+ messages in thread
From: Alexander Graf @ 2018-12-02 23:50 UTC (permalink / raw)
To: u-boot
On 05.11.18 10:06, AKASHI Takahiro wrote:
> The current way how command parameters, particularly "fdt addr," are
> handled makes it a bit complicated to add a subcommand-specific parameter.
> So just refactor the code and extract efi_handle_fdt().
>
> This commit is a preparatory change for enhancing bootmgr sub-command.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/bootefi.c | 51 +++++++++++++++++++++++++++++++++-----------------
> cmd/efishell.h | 1 +
> 2 files changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 5d61819f1f5c..3aedf5a09f93 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -356,6 +356,31 @@ static efi_status_t efi_install_fdt(ulong fdt_addr)
> return ret;
> }
>
> +int efi_handle_fdt(char *fdt_opt)
> +{
> + unsigned long fdt_addr;
> + efi_status_t r;
> +
> + if (fdt_opt) {
> + fdt_addr = simple_strtoul(fdt_opt, NULL, 16);
> + if (!fdt_addr && *fdt_opt != '0')
> + return CMD_RET_USAGE;
> +
> + /* Install device tree */
> + r = efi_install_fdt(fdt_addr);
> + if (r != EFI_SUCCESS) {
> + printf("ERROR: failed to install device tree\n");
> + return CMD_RET_FAILURE;
> + }
> + } else {
> + /* Remove device tree. EFI_NOT_FOUND can be ignored here */
> + efi_install_configuration_table(&efi_guid_fdt, NULL);
> + printf("WARNING: booting without device tree\n");
> + }
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> /**
> * do_bootefi_exec() - execute EFI binary
> *
> @@ -511,7 +536,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> unsigned long addr;
> char *saddr;
> efi_status_t r;
> - unsigned long fdt_addr;
>
> /* Allow unaligned memory access */
> allow_unaligned();
> @@ -527,21 +551,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> if (argc < 2)
> return CMD_RET_USAGE;
>
> - if (argc > 2) {
> - fdt_addr = simple_strtoul(argv[2], NULL, 16);
> - if (!fdt_addr && *argv[2] != '0')
> - return CMD_RET_USAGE;
> - /* Install device tree */
> - r = efi_install_fdt(fdt_addr);
> - if (r != EFI_SUCCESS) {
> - printf("ERROR: failed to install device tree\n");
> - return CMD_RET_FAILURE;
> - }
> - } else {
> - /* Remove device tree. EFI_NOT_FOUND can be ignored here */
> - efi_install_configuration_table(&efi_guid_fdt, NULL);
> - printf("WARNING: booting without device tree\n");
> - }
> #ifdef CONFIG_CMD_BOOTEFI_HELLO
> if (!strcmp(argv[1], "hello")) {
> ulong size = __efi_helloworld_end - __efi_helloworld_begin;
> @@ -559,6 +568,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> struct efi_loaded_image_obj *image_obj;
> struct efi_loaded_image *loaded_image_info;
>
> + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
> + return CMD_RET_FAILURE;
> +
> /* Construct a dummy device path. */
> bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
> (uintptr_t)&efi_selftest,
> @@ -583,7 +595,10 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> } else
> #endif
> if (!strcmp(argv[1], "bootmgr")) {
> - return do_bootefi_bootmgr_exec();
> + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
> + return CMD_RET_FAILURE;
> +
> + return do_bootefi_bootmgr_exec(boot_id);
> } else {
> saddr = argv[1];
>
> @@ -592,6 +607,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> if (!addr && *saddr != '0')
> return CMD_RET_USAGE;
>
> + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
> + return CMD_RET_FAILURE;
> }
>
> printf("## Starting EFI application at %08lx ...\n", addr);
> diff --git a/cmd/efishell.h b/cmd/efishell.h
> index 6f70b402b26b..1e5764a8a38d 100644
> --- a/cmd/efishell.h
> +++ b/cmd/efishell.h
> @@ -3,3 +3,4 @@
> #include <efi.h>
>
> efi_status_t efi_init_obj_list(void);
> +int efi_handle_fdt(char *fdt_opt);
This should also go into efi_loader.h, as that's the subsystem that
exports the function.
Alex
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 10/14] cmd: bootefi: carve out fdt parameter handling
2018-12-02 23:50 ` Alexander Graf
@ 2018-12-03 7:33 ` AKASHI Takahiro
2018-12-23 3:11 ` Alexander Graf
0 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-12-03 7:33 UTC (permalink / raw)
To: u-boot
On Mon, Dec 03, 2018 at 12:50:04AM +0100, Alexander Graf wrote:
>
>
> On 05.11.18 10:06, AKASHI Takahiro wrote:
> > The current way how command parameters, particularly "fdt addr," are
> > handled makes it a bit complicated to add a subcommand-specific parameter.
> > So just refactor the code and extract efi_handle_fdt().
> >
> > This commit is a preparatory change for enhancing bootmgr sub-command.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > cmd/bootefi.c | 51 +++++++++++++++++++++++++++++++++-----------------
> > cmd/efishell.h | 1 +
> > 2 files changed, 35 insertions(+), 17 deletions(-)
> >
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index 5d61819f1f5c..3aedf5a09f93 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -356,6 +356,31 @@ static efi_status_t efi_install_fdt(ulong fdt_addr)
> > return ret;
> > }
> >
> > +int efi_handle_fdt(char *fdt_opt)
> > +{
> > + unsigned long fdt_addr;
> > + efi_status_t r;
> > +
> > + if (fdt_opt) {
> > + fdt_addr = simple_strtoul(fdt_opt, NULL, 16);
> > + if (!fdt_addr && *fdt_opt != '0')
> > + return CMD_RET_USAGE;
> > +
> > + /* Install device tree */
> > + r = efi_install_fdt(fdt_addr);
> > + if (r != EFI_SUCCESS) {
> > + printf("ERROR: failed to install device tree\n");
> > + return CMD_RET_FAILURE;
> > + }
> > + } else {
> > + /* Remove device tree. EFI_NOT_FOUND can be ignored here */
> > + efi_install_configuration_table(&efi_guid_fdt, NULL);
> > + printf("WARNING: booting without device tree\n");
> > + }
> > +
> > + return CMD_RET_SUCCESS;
> > +}
> > +
> > /**
> > * do_bootefi_exec() - execute EFI binary
> > *
> > @@ -511,7 +536,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > unsigned long addr;
> > char *saddr;
> > efi_status_t r;
> > - unsigned long fdt_addr;
> >
> > /* Allow unaligned memory access */
> > allow_unaligned();
> > @@ -527,21 +551,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > if (argc < 2)
> > return CMD_RET_USAGE;
> >
> > - if (argc > 2) {
> > - fdt_addr = simple_strtoul(argv[2], NULL, 16);
> > - if (!fdt_addr && *argv[2] != '0')
> > - return CMD_RET_USAGE;
> > - /* Install device tree */
> > - r = efi_install_fdt(fdt_addr);
> > - if (r != EFI_SUCCESS) {
> > - printf("ERROR: failed to install device tree\n");
> > - return CMD_RET_FAILURE;
> > - }
> > - } else {
> > - /* Remove device tree. EFI_NOT_FOUND can be ignored here */
> > - efi_install_configuration_table(&efi_guid_fdt, NULL);
> > - printf("WARNING: booting without device tree\n");
> > - }
> > #ifdef CONFIG_CMD_BOOTEFI_HELLO
> > if (!strcmp(argv[1], "hello")) {
> > ulong size = __efi_helloworld_end - __efi_helloworld_begin;
> > @@ -559,6 +568,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > struct efi_loaded_image_obj *image_obj;
> > struct efi_loaded_image *loaded_image_info;
> >
> > + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
> > + return CMD_RET_FAILURE;
> > +
> > /* Construct a dummy device path. */
> > bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
> > (uintptr_t)&efi_selftest,
> > @@ -583,7 +595,10 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > } else
> > #endif
> > if (!strcmp(argv[1], "bootmgr")) {
> > - return do_bootefi_bootmgr_exec();
> > + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
> > + return CMD_RET_FAILURE;
> > +
> > + return do_bootefi_bootmgr_exec(boot_id);
> > } else {
> > saddr = argv[1];
> >
> > @@ -592,6 +607,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > if (!addr && *saddr != '0')
> > return CMD_RET_USAGE;
> >
> > + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
> > + return CMD_RET_FAILURE;
> > }
> >
> > printf("## Starting EFI application at %08lx ...\n", addr);
> > diff --git a/cmd/efishell.h b/cmd/efishell.h
> > index 6f70b402b26b..1e5764a8a38d 100644
> > --- a/cmd/efishell.h
> > +++ b/cmd/efishell.h
> > @@ -3,3 +3,4 @@
> > #include <efi.h>
> >
> > efi_status_t efi_init_obj_list(void);
> > +int efi_handle_fdt(char *fdt_opt);
>
> This should also go into efi_loader.h, as that's the subsystem that
> exports the function.
I hesitate to agree with you here as this function is provided as part
of bootefi in the first place. Run command is just a variant of
bootefi for efi binary. So it won't be seen as part of efi_loader function.
# In this sense, efishell.h should be renamed to bootefi.h?
Thanks,
-Takahiro Akashi
>
> Alex
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 10/14] cmd: bootefi: carve out fdt parameter handling
2018-12-03 7:33 ` AKASHI Takahiro
@ 2018-12-23 3:11 ` Alexander Graf
2018-12-25 12:05 ` AKASHI Takahiro
0 siblings, 1 reply; 39+ messages in thread
From: Alexander Graf @ 2018-12-23 3:11 UTC (permalink / raw)
To: u-boot
On 03.12.18 08:33, AKASHI Takahiro wrote:
> On Mon, Dec 03, 2018 at 12:50:04AM +0100, Alexander Graf wrote:
>>
>>
>> On 05.11.18 10:06, AKASHI Takahiro wrote:
>>> The current way how command parameters, particularly "fdt addr," are
>>> handled makes it a bit complicated to add a subcommand-specific parameter.
>>> So just refactor the code and extract efi_handle_fdt().
>>>
>>> This commit is a preparatory change for enhancing bootmgr sub-command.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>> ---
>>> cmd/bootefi.c | 51 +++++++++++++++++++++++++++++++++-----------------
>>> cmd/efishell.h | 1 +
>>> 2 files changed, 35 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>> index 5d61819f1f5c..3aedf5a09f93 100644
>>> --- a/cmd/bootefi.c
>>> +++ b/cmd/bootefi.c
>>> @@ -356,6 +356,31 @@ static efi_status_t efi_install_fdt(ulong fdt_addr)
>>> return ret;
>>> }
>>>
>>> +int efi_handle_fdt(char *fdt_opt)
>>> +{
>>> + unsigned long fdt_addr;
>>> + efi_status_t r;
>>> +
>>> + if (fdt_opt) {
>>> + fdt_addr = simple_strtoul(fdt_opt, NULL, 16);
>>> + if (!fdt_addr && *fdt_opt != '0')
>>> + return CMD_RET_USAGE;
>>> +
>>> + /* Install device tree */
>>> + r = efi_install_fdt(fdt_addr);
>>> + if (r != EFI_SUCCESS) {
>>> + printf("ERROR: failed to install device tree\n");
>>> + return CMD_RET_FAILURE;
>>> + }
>>> + } else {
>>> + /* Remove device tree. EFI_NOT_FOUND can be ignored here */
>>> + efi_install_configuration_table(&efi_guid_fdt, NULL);
>>> + printf("WARNING: booting without device tree\n");
>>> + }
>>> +
>>> + return CMD_RET_SUCCESS;
>>> +}
>>> +
>>> /**
>>> * do_bootefi_exec() - execute EFI binary
>>> *
>>> @@ -511,7 +536,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>> unsigned long addr;
>>> char *saddr;
>>> efi_status_t r;
>>> - unsigned long fdt_addr;
>>>
>>> /* Allow unaligned memory access */
>>> allow_unaligned();
>>> @@ -527,21 +551,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>> if (argc < 2)
>>> return CMD_RET_USAGE;
>>>
>>> - if (argc > 2) {
>>> - fdt_addr = simple_strtoul(argv[2], NULL, 16);
>>> - if (!fdt_addr && *argv[2] != '0')
>>> - return CMD_RET_USAGE;
>>> - /* Install device tree */
>>> - r = efi_install_fdt(fdt_addr);
>>> - if (r != EFI_SUCCESS) {
>>> - printf("ERROR: failed to install device tree\n");
>>> - return CMD_RET_FAILURE;
>>> - }
>>> - } else {
>>> - /* Remove device tree. EFI_NOT_FOUND can be ignored here */
>>> - efi_install_configuration_table(&efi_guid_fdt, NULL);
>>> - printf("WARNING: booting without device tree\n");
>>> - }
>>> #ifdef CONFIG_CMD_BOOTEFI_HELLO
>>> if (!strcmp(argv[1], "hello")) {
>>> ulong size = __efi_helloworld_end - __efi_helloworld_begin;
>>> @@ -559,6 +568,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>> struct efi_loaded_image_obj *image_obj;
>>> struct efi_loaded_image *loaded_image_info;
>>>
>>> + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
>>> + return CMD_RET_FAILURE;
>>> +
>>> /* Construct a dummy device path. */
>>> bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
>>> (uintptr_t)&efi_selftest,
>>> @@ -583,7 +595,10 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>> } else
>>> #endif
>>> if (!strcmp(argv[1], "bootmgr")) {
>>> - return do_bootefi_bootmgr_exec();
>>> + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
>>> + return CMD_RET_FAILURE;
>>> +
>>> + return do_bootefi_bootmgr_exec(boot_id);
>>> } else {
>>> saddr = argv[1];
>>>
>>> @@ -592,6 +607,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>> if (!addr && *saddr != '0')
>>> return CMD_RET_USAGE;
>>>
>>> + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
>>> + return CMD_RET_FAILURE;
>>> }
>>>
>>> printf("## Starting EFI application at %08lx ...\n", addr);
>>> diff --git a/cmd/efishell.h b/cmd/efishell.h
>>> index 6f70b402b26b..1e5764a8a38d 100644
>>> --- a/cmd/efishell.h
>>> +++ b/cmd/efishell.h
>>> @@ -3,3 +3,4 @@
>>> #include <efi.h>
>>>
>>> efi_status_t efi_init_obj_list(void);
>>> +int efi_handle_fdt(char *fdt_opt);
>>
>> This should also go into efi_loader.h, as that's the subsystem that
>> exports the function.
>
> I hesitate to agree with you here as this function is provided as part
> of bootefi in the first place. Run command is just a variant of
> bootefi for efi binary. So it won't be seen as part of efi_loader function.
>
> # In this sense, efishell.h should be renamed to bootefi.h?
Feel free to introduce a bootefi.h that contains all exports from
bootefi.c, yes :).
Alex
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 10/14] cmd: bootefi: carve out fdt parameter handling
2018-12-23 3:11 ` Alexander Graf
@ 2018-12-25 12:05 ` AKASHI Takahiro
0 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2018-12-25 12:05 UTC (permalink / raw)
To: u-boot
On Sun, Dec 23, 2018 at 04:11:59AM +0100, Alexander Graf wrote:
>
>
> On 03.12.18 08:33, AKASHI Takahiro wrote:
> > On Mon, Dec 03, 2018 at 12:50:04AM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 05.11.18 10:06, AKASHI Takahiro wrote:
> >>> The current way how command parameters, particularly "fdt addr," are
> >>> handled makes it a bit complicated to add a subcommand-specific parameter.
> >>> So just refactor the code and extract efi_handle_fdt().
> >>>
> >>> This commit is a preparatory change for enhancing bootmgr sub-command.
> >>>
> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>> ---
> >>> cmd/bootefi.c | 51 +++++++++++++++++++++++++++++++++-----------------
> >>> cmd/efishell.h | 1 +
> >>> 2 files changed, 35 insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> >>> index 5d61819f1f5c..3aedf5a09f93 100644
> >>> --- a/cmd/bootefi.c
> >>> +++ b/cmd/bootefi.c
> >>> @@ -356,6 +356,31 @@ static efi_status_t efi_install_fdt(ulong fdt_addr)
> >>> return ret;
> >>> }
> >>>
> >>> +int efi_handle_fdt(char *fdt_opt)
> >>> +{
> >>> + unsigned long fdt_addr;
> >>> + efi_status_t r;
> >>> +
> >>> + if (fdt_opt) {
> >>> + fdt_addr = simple_strtoul(fdt_opt, NULL, 16);
> >>> + if (!fdt_addr && *fdt_opt != '0')
> >>> + return CMD_RET_USAGE;
> >>> +
> >>> + /* Install device tree */
> >>> + r = efi_install_fdt(fdt_addr);
> >>> + if (r != EFI_SUCCESS) {
> >>> + printf("ERROR: failed to install device tree\n");
> >>> + return CMD_RET_FAILURE;
> >>> + }
> >>> + } else {
> >>> + /* Remove device tree. EFI_NOT_FOUND can be ignored here */
> >>> + efi_install_configuration_table(&efi_guid_fdt, NULL);
> >>> + printf("WARNING: booting without device tree\n");
> >>> + }
> >>> +
> >>> + return CMD_RET_SUCCESS;
> >>> +}
> >>> +
> >>> /**
> >>> * do_bootefi_exec() - execute EFI binary
> >>> *
> >>> @@ -511,7 +536,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >>> unsigned long addr;
> >>> char *saddr;
> >>> efi_status_t r;
> >>> - unsigned long fdt_addr;
> >>>
> >>> /* Allow unaligned memory access */
> >>> allow_unaligned();
> >>> @@ -527,21 +551,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >>> if (argc < 2)
> >>> return CMD_RET_USAGE;
> >>>
> >>> - if (argc > 2) {
> >>> - fdt_addr = simple_strtoul(argv[2], NULL, 16);
> >>> - if (!fdt_addr && *argv[2] != '0')
> >>> - return CMD_RET_USAGE;
> >>> - /* Install device tree */
> >>> - r = efi_install_fdt(fdt_addr);
> >>> - if (r != EFI_SUCCESS) {
> >>> - printf("ERROR: failed to install device tree\n");
> >>> - return CMD_RET_FAILURE;
> >>> - }
> >>> - } else {
> >>> - /* Remove device tree. EFI_NOT_FOUND can be ignored here */
> >>> - efi_install_configuration_table(&efi_guid_fdt, NULL);
> >>> - printf("WARNING: booting without device tree\n");
> >>> - }
> >>> #ifdef CONFIG_CMD_BOOTEFI_HELLO
> >>> if (!strcmp(argv[1], "hello")) {
> >>> ulong size = __efi_helloworld_end - __efi_helloworld_begin;
> >>> @@ -559,6 +568,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >>> struct efi_loaded_image_obj *image_obj;
> >>> struct efi_loaded_image *loaded_image_info;
> >>>
> >>> + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
> >>> + return CMD_RET_FAILURE;
> >>> +
> >>> /* Construct a dummy device path. */
> >>> bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
> >>> (uintptr_t)&efi_selftest,
> >>> @@ -583,7 +595,10 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >>> } else
> >>> #endif
> >>> if (!strcmp(argv[1], "bootmgr")) {
> >>> - return do_bootefi_bootmgr_exec();
> >>> + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
> >>> + return CMD_RET_FAILURE;
> >>> +
> >>> + return do_bootefi_bootmgr_exec(boot_id);
> >>> } else {
> >>> saddr = argv[1];
> >>>
> >>> @@ -592,6 +607,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >>> if (!addr && *saddr != '0')
> >>> return CMD_RET_USAGE;
> >>>
> >>> + if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
> >>> + return CMD_RET_FAILURE;
> >>> }
> >>>
> >>> printf("## Starting EFI application at %08lx ...\n", addr);
> >>> diff --git a/cmd/efishell.h b/cmd/efishell.h
> >>> index 6f70b402b26b..1e5764a8a38d 100644
> >>> --- a/cmd/efishell.h
> >>> +++ b/cmd/efishell.h
> >>> @@ -3,3 +3,4 @@
> >>> #include <efi.h>
> >>>
> >>> efi_status_t efi_init_obj_list(void);
> >>> +int efi_handle_fdt(char *fdt_opt);
> >>
> >> This should also go into efi_loader.h, as that's the subsystem that
> >> exports the function.
> >
> > I hesitate to agree with you here as this function is provided as part
> > of bootefi in the first place. Run command is just a variant of
> > bootefi for efi binary. So it won't be seen as part of efi_loader function.
> >
> > # In this sense, efishell.h should be renamed to bootefi.h?
>
> Feel free to introduce a bootefi.h that contains all exports from
> bootefi.c, yes :).
OK, but due to your comment on a new do_bootefi_run() function,
we may no longer need to export efi_handle_fdt() (at least for now).
That's good.
-Takahiro Akashi
>
> Alex
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 11/14] cmd: bootefi: run an EFI application of a specific load option
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
` (9 preceding siblings ...)
2018-11-05 9:06 ` [U-Boot] [PATCH v2 10/14] cmd: bootefi: carve out fdt parameter handling AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-11-05 9:06 ` [U-Boot] [PATCH v2 12/14] cmd: run: add "-e" option to run an EFI application AKASHI Takahiro
` (2 subsequent siblings)
13 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
With this patch applied, we will be able to selectively execute
an EFI application by specifying a load option, say "1" for Boot0001,
"2" for Boot0002 and so on.
=> bootefi bootmgr <fdt addr> 1, or
bootefi bootmgr - 1
Please note that BootXXXX need not be included in "BootOrder".
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/bootefi.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 3aedf5a09f93..7580008f691b 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -509,13 +509,13 @@ exit:
return ret;
}
-static int do_bootefi_bootmgr_exec(void)
+static int do_bootefi_bootmgr_exec(int boot_id)
{
struct efi_device_path *device_path, *file_path;
void *addr;
efi_status_t r;
- addr = efi_bootmgr_load(&device_path, &file_path);
+ addr = efi_bootmgr_load(boot_id, &device_path, &file_path);
if (!addr)
return 1;
@@ -595,8 +595,20 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
} else
#endif
if (!strcmp(argv[1], "bootmgr")) {
- if (efi_handle_fdt(argc > 2 ? argv[2] : NULL))
- return CMD_RET_FAILURE;
+ char *endp;
+ int boot_id = -1;
+
+ if (argc > 2)
+ if (efi_handle_fdt((argv[2][0] == '-') ?
+ NULL : argv[2]))
+ return CMD_RET_FAILURE;
+
+ if (argc > 3) {
+ boot_id = (int)simple_strtoul(argv[3], &endp, 0);
+ if ((argv[3] + strlen(argv[3]) != endp) ||
+ boot_id > 0xffff)
+ return CMD_RET_USAGE;
+ }
return do_bootefi_bootmgr_exec(boot_id);
} else {
@@ -639,7 +651,7 @@ static char bootefi_help_text[] =
" Use environment variable efi_selftest to select a single test.\n"
" Use 'setenv efi_selftest list' to enumerate all tests.\n"
#endif
- "bootefi bootmgr [fdt addr]\n"
+ "bootefi bootmgr [<fdt addr>|'-' [<boot id>]]\n"
" - load and boot EFI payload based on BootOrder/BootXXXX variables.\n"
"\n"
" If specified, the device tree located at <fdt address> gets\n"
@@ -647,7 +659,7 @@ static char bootefi_help_text[] =
#endif
U_BOOT_CMD(
- bootefi, 3, 0, do_bootefi,
+ bootefi, 5, 0, do_bootefi,
"Boots an EFI payload from memory",
bootefi_help_text
);
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 12/14] cmd: run: add "-e" option to run an EFI application
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
` (10 preceding siblings ...)
2018-11-05 9:06 ` [U-Boot] [PATCH v2 11/14] cmd: bootefi: run an EFI application of a specific load option AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-12-02 23:53 ` Alexander Graf
2018-11-05 9:06 ` [U-Boot] [PATCH v2 13/14] cmd: efishell: export uefi variable helper functions AKASHI Takahiro
2018-11-05 9:06 ` [U-Boot] [PATCH v2 14/14] cmd: env: add "-e" option for handling UEFI variables AKASHI Takahiro
13 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
"run -e" allows for executing EFI application with a specific "BootXXXX"
variable. If no "BootXXXX" is specified or "BootOrder" is specified,
it tries to run an EFI application specified in the order of "bootOrder."
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/bootefi.c | 2 +-
cmd/nvedit.c | 5 +++++
common/cli.c | 30 ++++++++++++++++++++++++++++++
include/command.h | 3 +++
4 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 7580008f691b..3cefb4d0ecaa 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -509,7 +509,7 @@ exit:
return ret;
}
-static int do_bootefi_bootmgr_exec(int boot_id)
+int do_bootefi_bootmgr_exec(int boot_id)
{
struct efi_device_path *device_path, *file_path;
void *addr;
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index de16c72c23f2..c0facabfc4fe 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -1343,8 +1343,13 @@ U_BOOT_CMD(
U_BOOT_CMD_COMPLETE(
run, CONFIG_SYS_MAXARGS, 1, do_run,
"run commands in an environment variable",
+#if defined(CONFIG_CMD_BOOTEFI)
+ "var -e [BootXXXX]\n"
+ " - load and run UEFI app based on 'BootXXXX' UEFI variable",
+#else
"var [...]\n"
" - run the commands in the environment variable(s) 'var'",
+#endif
var_complete
);
#endif
diff --git a/common/cli.c b/common/cli.c
index 51b8d5f85cbb..e4ec35b5eb00 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -14,6 +14,7 @@
#include <console.h>
#include <fdtdec.h>
#include <malloc.h>
+#include "../cmd/efishell.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -125,6 +126,35 @@ int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
if (argc < 2)
return CMD_RET_USAGE;
+#ifdef CONFIG_CMD_BOOTEFI
+ if (!strcmp(argv[1], "-e")) {
+ int boot_id = -1;
+ char *endp;
+
+ if (argc == 3) {
+ if (!strcmp(argv[2], "BootOrder")) {
+ /* boot_id = -1 */;
+ } else if (!strncmp(argv[2], "Boot", 4)) {
+ boot_id = (int)simple_strtoul(&argv[2][4],
+ &endp, 0);
+ if ((argv[2] + strlen(argv[2]) != endp) ||
+ boot_id > 0xffff)
+ return CMD_RET_USAGE;
+ } else {
+ return CMD_RET_USAGE;
+ }
+ }
+
+ if (efi_init_obj_list())
+ return CMD_RET_FAILURE;
+
+ if (efi_handle_fdt(NULL))
+ return CMD_RET_FAILURE;
+
+ return do_bootefi_bootmgr_exec(boot_id);
+ }
+#endif
+
for (i = 1; i < argc; ++i) {
char *arg;
diff --git a/include/command.h b/include/command.h
index 5b1577f3b477..5bb675122cce 100644
--- a/include/command.h
+++ b/include/command.h
@@ -48,6 +48,9 @@ typedef struct cmd_tbl_s cmd_tbl_t;
#if defined(CONFIG_CMD_RUN)
extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
#endif
+#if defined(CONFIG_CMD_BOOTEFI)
+int do_bootefi_bootmgr_exec(int boot_id);
+#endif
/* common/command.c */
int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp, int
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 12/14] cmd: run: add "-e" option to run an EFI application
2018-11-05 9:06 ` [U-Boot] [PATCH v2 12/14] cmd: run: add "-e" option to run an EFI application AKASHI Takahiro
@ 2018-12-02 23:53 ` Alexander Graf
2018-12-03 7:57 ` AKASHI Takahiro
0 siblings, 1 reply; 39+ messages in thread
From: Alexander Graf @ 2018-12-02 23:53 UTC (permalink / raw)
To: u-boot
On 05.11.18 10:06, AKASHI Takahiro wrote:
> "run -e" allows for executing EFI application with a specific "BootXXXX"
> variable. If no "BootXXXX" is specified or "BootOrder" is specified,
> it tries to run an EFI application specified in the order of "bootOrder."
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/bootefi.c | 2 +-
> cmd/nvedit.c | 5 +++++
> common/cli.c | 30 ++++++++++++++++++++++++++++++
> include/command.h | 3 +++
> 4 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 7580008f691b..3cefb4d0ecaa 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -509,7 +509,7 @@ exit:
> return ret;
> }
>
> -static int do_bootefi_bootmgr_exec(int boot_id)
> +int do_bootefi_bootmgr_exec(int boot_id)
> {
> struct efi_device_path *device_path, *file_path;
> void *addr;
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index de16c72c23f2..c0facabfc4fe 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -1343,8 +1343,13 @@ U_BOOT_CMD(
> U_BOOT_CMD_COMPLETE(
> run, CONFIG_SYS_MAXARGS, 1, do_run,
> "run commands in an environment variable",
> +#if defined(CONFIG_CMD_BOOTEFI)
> + "var -e [BootXXXX]\n"
> + " - load and run UEFI app based on 'BootXXXX' UEFI variable",
> +#else
> "var [...]\n"
> " - run the commands in the environment variable(s) 'var'",
> +#endif
> var_complete
> );
> #endif
> diff --git a/common/cli.c b/common/cli.c
> index 51b8d5f85cbb..e4ec35b5eb00 100644
> --- a/common/cli.c
> +++ b/common/cli.c
> @@ -14,6 +14,7 @@
> #include <console.h>
> #include <fdtdec.h>
> #include <malloc.h>
> +#include "../cmd/efishell.h"
I guess this already shows that something is going wrong ;).
Please move the depending functions into lib/efi_loader. That way we can
have the efishell command be optional, but keep this logic around
regardless.
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -125,6 +126,35 @@ int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> if (argc < 2)
> return CMD_RET_USAGE;
>
> +#ifdef CONFIG_CMD_BOOTEFI
> + if (!strcmp(argv[1], "-e")) {
> + int boot_id = -1;
> + char *endp;
> +
> + if (argc == 3) {
> + if (!strcmp(argv[2], "BootOrder")) {
> + /* boot_id = -1 */;
Just write that in code. The compiler will optimize it out.
> + } else if (!strncmp(argv[2], "Boot", 4)) {
> + boot_id = (int)simple_strtoul(&argv[2][4],
> + &endp, 0);
> + if ((argv[2] + strlen(argv[2]) != endp) ||
> + boot_id > 0xffff)
> + return CMD_RET_USAGE;
> + } else {
> + return CMD_RET_USAGE;
> + }
> + }
> +
> + if (efi_init_obj_list())
> + return CMD_RET_FAILURE;
> +
> + if (efi_handle_fdt(NULL))
> + return CMD_RET_FAILURE;
> +
> + return do_bootefi_bootmgr_exec(boot_id);
> + }
> +#endif
> +
> for (i = 1; i < argc; ++i) {
> char *arg;
>
> diff --git a/include/command.h b/include/command.h
> index 5b1577f3b477..5bb675122cce 100644
> --- a/include/command.h
> +++ b/include/command.h
> @@ -48,6 +48,9 @@ typedef struct cmd_tbl_s cmd_tbl_t;
> #if defined(CONFIG_CMD_RUN)
> extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> #endif
> +#if defined(CONFIG_CMD_BOOTEFI)
> +int do_bootefi_bootmgr_exec(int boot_id);
> +#endif
This needs to be in efi_loader.h which then needs to get included from
cli.c.
Alex
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 12/14] cmd: run: add "-e" option to run an EFI application
2018-12-02 23:53 ` Alexander Graf
@ 2018-12-03 7:57 ` AKASHI Takahiro
0 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2018-12-03 7:57 UTC (permalink / raw)
To: u-boot
On Mon, Dec 03, 2018 at 12:53:41AM +0100, Alexander Graf wrote:
>
>
> On 05.11.18 10:06, AKASHI Takahiro wrote:
> > "run -e" allows for executing EFI application with a specific "BootXXXX"
> > variable. If no "BootXXXX" is specified or "BootOrder" is specified,
> > it tries to run an EFI application specified in the order of "bootOrder."
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > cmd/bootefi.c | 2 +-
> > cmd/nvedit.c | 5 +++++
> > common/cli.c | 30 ++++++++++++++++++++++++++++++
> > include/command.h | 3 +++
> > 4 files changed, 39 insertions(+), 1 deletion(-)
> >
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index 7580008f691b..3cefb4d0ecaa 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -509,7 +509,7 @@ exit:
> > return ret;
> > }
> >
> > -static int do_bootefi_bootmgr_exec(int boot_id)
> > +int do_bootefi_bootmgr_exec(int boot_id)
> > {
> > struct efi_device_path *device_path, *file_path;
> > void *addr;
> > diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> > index de16c72c23f2..c0facabfc4fe 100644
> > --- a/cmd/nvedit.c
> > +++ b/cmd/nvedit.c
> > @@ -1343,8 +1343,13 @@ U_BOOT_CMD(
> > U_BOOT_CMD_COMPLETE(
> > run, CONFIG_SYS_MAXARGS, 1, do_run,
> > "run commands in an environment variable",
> > +#if defined(CONFIG_CMD_BOOTEFI)
> > + "var -e [BootXXXX]\n"
> > + " - load and run UEFI app based on 'BootXXXX' UEFI variable",
> > +#else
> > "var [...]\n"
> > " - run the commands in the environment variable(s) 'var'",
> > +#endif
> > var_complete
> > );
> > #endif
> > diff --git a/common/cli.c b/common/cli.c
> > index 51b8d5f85cbb..e4ec35b5eb00 100644
> > --- a/common/cli.c
> > +++ b/common/cli.c
> > @@ -14,6 +14,7 @@
> > #include <console.h>
> > #include <fdtdec.h>
> > #include <malloc.h>
> > +#include "../cmd/efishell.h"
>
> I guess this already shows that something is going wrong ;).
efishell.h here is included solely for efi_init_obj_list() definition
and so
> Please move the depending functions into lib/efi_loader. That way we can
> have the efishell command be optional, but keep this logic around
> regardless.
I can assume that you agree to this function being moved to efi_setup.c.
> >
> > DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -125,6 +126,35 @@ int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > if (argc < 2)
> > return CMD_RET_USAGE;
> >
> > +#ifdef CONFIG_CMD_BOOTEFI
> > + if (!strcmp(argv[1], "-e")) {
> > + int boot_id = -1;
> > + char *endp;
> > +
> > + if (argc == 3) {
> > + if (!strcmp(argv[2], "BootOrder")) {
> > + /* boot_id = -1 */;
>
> Just write that in code. The compiler will optimize it out.
OK.
> > + } else if (!strncmp(argv[2], "Boot", 4)) {
> > + boot_id = (int)simple_strtoul(&argv[2][4],
> > + &endp, 0);
> > + if ((argv[2] + strlen(argv[2]) != endp) ||
> > + boot_id > 0xffff)
> > + return CMD_RET_USAGE;
> > + } else {
> > + return CMD_RET_USAGE;
> > + }
> > + }
> > +
> > + if (efi_init_obj_list())
> > + return CMD_RET_FAILURE;
> > +
> > + if (efi_handle_fdt(NULL))
> > + return CMD_RET_FAILURE;
> > +
> > + return do_bootefi_bootmgr_exec(boot_id);
> > + }
> > +#endif
> > +
> > for (i = 1; i < argc; ++i) {
> > char *arg;
> >
> > diff --git a/include/command.h b/include/command.h
> > index 5b1577f3b477..5bb675122cce 100644
> > --- a/include/command.h
> > +++ b/include/command.h
> > @@ -48,6 +48,9 @@ typedef struct cmd_tbl_s cmd_tbl_t;
> > #if defined(CONFIG_CMD_RUN)
> > extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> > #endif
> > +#if defined(CONFIG_CMD_BOOTEFI)
> > +int do_bootefi_bootmgr_exec(int boot_id);
> > +#endif
>
> This needs to be in efi_loader.h which then needs to get included from
> cli.c.
After your comment above, you expect that do_bootefi_bootmgr_exec(),
hence do_bootefi_exec() (and efi_handle_fdt()?) as well, be also moved
to lib/efi_loader/(efi_bootmgr.c)?
Thanks,
-Takahiro Akashi
>
> Alex
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 13/14] cmd: efishell: export uefi variable helper functions
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
` (11 preceding siblings ...)
2018-11-05 9:06 ` [U-Boot] [PATCH v2 12/14] cmd: run: add "-e" option to run an EFI application AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
2018-12-02 23:54 ` Alexander Graf
2018-11-05 9:06 ` [U-Boot] [PATCH v2 14/14] cmd: env: add "-e" option for handling UEFI variables AKASHI Takahiro
13 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
Those function will be used for integration with 'env' command
so as to handle uefi variables.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/efishell.c | 4 ++--
include/command.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/cmd/efishell.c b/cmd/efishell.c
index bd2b99e74079..8122b842dd76 100644
--- a/cmd/efishell.c
+++ b/cmd/efishell.c
@@ -68,7 +68,7 @@ static void dump_var_data(char *data, unsigned long len)
*
* efi_$guid_$varname = {attributes}(type)value
*/
-static int do_efi_dump_var(int argc, char * const argv[])
+int do_efi_dump_var(int argc, char * const argv[])
{
char regex[256];
char * const regexlist[] = {regex};
@@ -228,7 +228,7 @@ out:
return 0;
}
-static int do_efi_set_var(int argc, char * const argv[])
+int do_efi_set_var(int argc, char * const argv[])
{
char *var_name, *value = NULL;
unsigned long size = 0;
diff --git a/include/command.h b/include/command.h
index 5bb675122cce..2ce8b53f74c8 100644
--- a/include/command.h
+++ b/include/command.h
@@ -50,6 +50,8 @@ extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
#endif
#if defined(CONFIG_CMD_BOOTEFI)
int do_bootefi_bootmgr_exec(int boot_id);
+int do_efi_dump_var(int argc, char * const argv[]);
+int do_efi_set_var(int argc, char * const argv[]);
#endif
/* common/command.c */
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 13/14] cmd: efishell: export uefi variable helper functions
2018-11-05 9:06 ` [U-Boot] [PATCH v2 13/14] cmd: efishell: export uefi variable helper functions AKASHI Takahiro
@ 2018-12-02 23:54 ` Alexander Graf
2018-12-03 8:08 ` AKASHI Takahiro
0 siblings, 1 reply; 39+ messages in thread
From: Alexander Graf @ 2018-12-02 23:54 UTC (permalink / raw)
To: u-boot
On 05.11.18 10:06, AKASHI Takahiro wrote:
> Those function will be used for integration with 'env' command
> so as to handle uefi variables.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/efishell.c | 4 ++--
> include/command.h | 2 ++
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/efishell.c b/cmd/efishell.c
> index bd2b99e74079..8122b842dd76 100644
> --- a/cmd/efishell.c
> +++ b/cmd/efishell.c
> @@ -68,7 +68,7 @@ static void dump_var_data(char *data, unsigned long len)
> *
> * efi_$guid_$varname = {attributes}(type)value
> */
> -static int do_efi_dump_var(int argc, char * const argv[])
> +int do_efi_dump_var(int argc, char * const argv[])
> {
> char regex[256];
> char * const regexlist[] = {regex};
> @@ -228,7 +228,7 @@ out:
> return 0;
> }
>
> -static int do_efi_set_var(int argc, char * const argv[])
> +int do_efi_set_var(int argc, char * const argv[])
> {
> char *var_name, *value = NULL;
> unsigned long size = 0;
> diff --git a/include/command.h b/include/command.h
> index 5bb675122cce..2ce8b53f74c8 100644
> --- a/include/command.h
> +++ b/include/command.h
> @@ -50,6 +50,8 @@ extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> #endif
> #if defined(CONFIG_CMD_BOOTEFI)
> int do_bootefi_bootmgr_exec(int boot_id);
> +int do_efi_dump_var(int argc, char * const argv[]);
> +int do_efi_set_var(int argc, char * const argv[]);
I guess you're seeing a pattern to my comments by now. This needs to go
into efi_loader.h. The respective functions need to go into lib/efi_loader.
Alex
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 13/14] cmd: efishell: export uefi variable helper functions
2018-12-02 23:54 ` Alexander Graf
@ 2018-12-03 8:08 ` AKASHI Takahiro
2018-12-23 3:13 ` Alexander Graf
0 siblings, 1 reply; 39+ messages in thread
From: AKASHI Takahiro @ 2018-12-03 8:08 UTC (permalink / raw)
To: u-boot
On Mon, Dec 03, 2018 at 12:54:44AM +0100, Alexander Graf wrote:
>
>
> On 05.11.18 10:06, AKASHI Takahiro wrote:
> > Those function will be used for integration with 'env' command
> > so as to handle uefi variables.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > cmd/efishell.c | 4 ++--
> > include/command.h | 2 ++
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/cmd/efishell.c b/cmd/efishell.c
> > index bd2b99e74079..8122b842dd76 100644
> > --- a/cmd/efishell.c
> > +++ b/cmd/efishell.c
> > @@ -68,7 +68,7 @@ static void dump_var_data(char *data, unsigned long len)
> > *
> > * efi_$guid_$varname = {attributes}(type)value
> > */
> > -static int do_efi_dump_var(int argc, char * const argv[])
> > +int do_efi_dump_var(int argc, char * const argv[])
> > {
> > char regex[256];
> > char * const regexlist[] = {regex};
> > @@ -228,7 +228,7 @@ out:
> > return 0;
> > }
> >
> > -static int do_efi_set_var(int argc, char * const argv[])
> > +int do_efi_set_var(int argc, char * const argv[])
> > {
> > char *var_name, *value = NULL;
> > unsigned long size = 0;
> > diff --git a/include/command.h b/include/command.h
> > index 5bb675122cce..2ce8b53f74c8 100644
> > --- a/include/command.h
> > +++ b/include/command.h
> > @@ -50,6 +50,8 @@ extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> > #endif
> > #if defined(CONFIG_CMD_BOOTEFI)
> > int do_bootefi_bootmgr_exec(int boot_id);
> > +int do_efi_dump_var(int argc, char * const argv[]);
> > +int do_efi_set_var(int argc, char * const argv[]);
>
> I guess you're seeing a pattern to my comments by now.
Definitely, but
> This needs to go
> into efi_loader.h. The respective functions need to go into lib/efi_loader.
Those two functions are dedicated for command interfaces, and in my opinion,
it is hard for them to be seen as part of efi_loader functionality.
Thanks,
-Takahiro Akashi
>
>
> Alex
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 13/14] cmd: efishell: export uefi variable helper functions
2018-12-03 8:08 ` AKASHI Takahiro
@ 2018-12-23 3:13 ` Alexander Graf
2018-12-25 12:14 ` AKASHI Takahiro
0 siblings, 1 reply; 39+ messages in thread
From: Alexander Graf @ 2018-12-23 3:13 UTC (permalink / raw)
To: u-boot
On 03.12.18 09:08, AKASHI Takahiro wrote:
> On Mon, Dec 03, 2018 at 12:54:44AM +0100, Alexander Graf wrote:
>>
>>
>> On 05.11.18 10:06, AKASHI Takahiro wrote:
>>> Those function will be used for integration with 'env' command
>>> so as to handle uefi variables.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>> ---
>>> cmd/efishell.c | 4 ++--
>>> include/command.h | 2 ++
>>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/cmd/efishell.c b/cmd/efishell.c
>>> index bd2b99e74079..8122b842dd76 100644
>>> --- a/cmd/efishell.c
>>> +++ b/cmd/efishell.c
>>> @@ -68,7 +68,7 @@ static void dump_var_data(char *data, unsigned long len)
>>> *
>>> * efi_$guid_$varname = {attributes}(type)value
>>> */
>>> -static int do_efi_dump_var(int argc, char * const argv[])
>>> +int do_efi_dump_var(int argc, char * const argv[])
>>> {
>>> char regex[256];
>>> char * const regexlist[] = {regex};
>>> @@ -228,7 +228,7 @@ out:
>>> return 0;
>>> }
>>>
>>> -static int do_efi_set_var(int argc, char * const argv[])
>>> +int do_efi_set_var(int argc, char * const argv[])
>>> {
>>> char *var_name, *value = NULL;
>>> unsigned long size = 0;
>>> diff --git a/include/command.h b/include/command.h
>>> index 5bb675122cce..2ce8b53f74c8 100644
>>> --- a/include/command.h
>>> +++ b/include/command.h
>>> @@ -50,6 +50,8 @@ extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>>> #endif
>>> #if defined(CONFIG_CMD_BOOTEFI)
>>> int do_bootefi_bootmgr_exec(int boot_id);
>>> +int do_efi_dump_var(int argc, char * const argv[]);
>>> +int do_efi_set_var(int argc, char * const argv[]);
>>
>> I guess you're seeing a pattern to my comments by now.
>
> Definitely, but
>
>> This needs to go
>> into efi_loader.h. The respective functions need to go into lib/efi_loader.
>
> Those two functions are dedicated for command interfaces, and in my opinion,
> it is hard for them to be seen as part of efi_loader functionality.
Same comment here then :). Feel free to introduce a new header file for
bootefi bits, but I'm not a big fan of a giant command.h that just
exposes all possible command handlers in the world.
Alex
^ permalink raw reply [flat|nested] 39+ messages in thread* [U-Boot] [PATCH v2 13/14] cmd: efishell: export uefi variable helper functions
2018-12-23 3:13 ` Alexander Graf
@ 2018-12-25 12:14 ` AKASHI Takahiro
0 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2018-12-25 12:14 UTC (permalink / raw)
To: u-boot
On Sun, Dec 23, 2018 at 04:13:57AM +0100, Alexander Graf wrote:
>
>
> On 03.12.18 09:08, AKASHI Takahiro wrote:
> > On Mon, Dec 03, 2018 at 12:54:44AM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 05.11.18 10:06, AKASHI Takahiro wrote:
> >>> Those function will be used for integration with 'env' command
> >>> so as to handle uefi variables.
> >>>
> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>> ---
> >>> cmd/efishell.c | 4 ++--
> >>> include/command.h | 2 ++
> >>> 2 files changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/cmd/efishell.c b/cmd/efishell.c
> >>> index bd2b99e74079..8122b842dd76 100644
> >>> --- a/cmd/efishell.c
> >>> +++ b/cmd/efishell.c
> >>> @@ -68,7 +68,7 @@ static void dump_var_data(char *data, unsigned long len)
> >>> *
> >>> * efi_$guid_$varname = {attributes}(type)value
> >>> */
> >>> -static int do_efi_dump_var(int argc, char * const argv[])
> >>> +int do_efi_dump_var(int argc, char * const argv[])
> >>> {
> >>> char regex[256];
> >>> char * const regexlist[] = {regex};
> >>> @@ -228,7 +228,7 @@ out:
> >>> return 0;
> >>> }
> >>>
> >>> -static int do_efi_set_var(int argc, char * const argv[])
> >>> +int do_efi_set_var(int argc, char * const argv[])
> >>> {
> >>> char *var_name, *value = NULL;
> >>> unsigned long size = 0;
> >>> diff --git a/include/command.h b/include/command.h
> >>> index 5bb675122cce..2ce8b53f74c8 100644
> >>> --- a/include/command.h
> >>> +++ b/include/command.h
> >>> @@ -50,6 +50,8 @@ extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> >>> #endif
> >>> #if defined(CONFIG_CMD_BOOTEFI)
> >>> int do_bootefi_bootmgr_exec(int boot_id);
> >>> +int do_efi_dump_var(int argc, char * const argv[]);
> >>> +int do_efi_set_var(int argc, char * const argv[]);
> >>
> >> I guess you're seeing a pattern to my comments by now.
> >
> > Definitely, but
> >
> >> This needs to go
> >> into efi_loader.h. The respective functions need to go into lib/efi_loader.
> >
> > Those two functions are dedicated for command interfaces, and in my opinion,
> > it is hard for them to be seen as part of efi_loader functionality.
>
> Same comment here then :). Feel free to introduce a new header file for
> bootefi bits, but I'm not a big fan of a giant command.h that just
> exposes all possible command handlers in the world.
I've assumed that all the do_xxx() stuff should go into command.h.
I will leave them in command.h for now.
-Takahiro Akashi
>
> Alex
^ permalink raw reply [flat|nested] 39+ messages in thread
* [U-Boot] [PATCH v2 14/14] cmd: env: add "-e" option for handling UEFI variables
2018-11-05 9:06 [U-Boot] [PATCH v2 00/14] efi: make efi and bootmgr more usable AKASHI Takahiro
` (12 preceding siblings ...)
2018-11-05 9:06 ` [U-Boot] [PATCH v2 13/14] cmd: efishell: export uefi variable helper functions AKASHI Takahiro
@ 2018-11-05 9:06 ` AKASHI Takahiro
13 siblings, 0 replies; 39+ messages in thread
From: AKASHI Takahiro @ 2018-11-05 9:06 UTC (permalink / raw)
To: u-boot
"env [print|set] -e" allows for handling uefi variables without
knowing details about mapping to corresponding u-boot variables.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/nvedit.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index c0facabfc4fe..1925483c0285 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -36,6 +36,7 @@
#include <linux/stddef.h>
#include <asm/byteorder.h>
#include <asm/io.h>
+#include "efishell.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -119,6 +120,25 @@ static int do_env_print(cmd_tbl_t *cmdtp, int flag, int argc,
int rcode = 0;
int env_flag = H_HIDE_DOT;
+#if defined(CONFIG_CMD_BOOTEFI)
+ if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') {
+ efi_status_t r;
+
+ argc--;
+ argv++;
+
+ /* Initialize EFI drivers */
+ r = efi_init_obj_list();
+ if (r != EFI_SUCCESS) {
+ printf("Error: Cannot set up EFI drivers, r = %lu\n",
+ r & ~EFI_ERROR_MASK);
+ return CMD_RET_FAILURE;
+ }
+
+ return do_efi_dump_var(argc, argv);
+ }
+#endif
+
if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'a') {
argc--;
argv++;
@@ -216,6 +236,26 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
ENTRY e, *ep;
debug("Initial value for argc=%d\n", argc);
+
+#if defined(CONFIG_CMD_BOOTEFI)
+ if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e') {
+ efi_status_t r;
+
+ argc--;
+ argv++;
+
+ /* Initialize EFI drivers */
+ r = efi_init_obj_list();
+ if (r != EFI_SUCCESS) {
+ printf("Error: Cannot set up EFI drivers, r = %lu\n",
+ r & ~EFI_ERROR_MASK);
+ return CMD_RET_FAILURE;
+ }
+
+ return do_efi_set_var(argc, argv);
+ }
+#endif
+
while (argc > 1 && **(argv + 1) == '-') {
char *arg = *++argv;
@@ -1262,15 +1302,23 @@ static char env_help_text[] =
#if defined(CONFIG_CMD_IMPORTENV)
"env import [-d] [-t [-r] | -b | -c] addr [size] [var ...] - import environment\n"
#endif
+#if defined(CONFIG_CMD_BOOTEFI)
+ "env print [-a | -e [name] | name ...] - print environment\n"
+#else
"env print [-a | name ...] - print environment\n"
+#endif
#if defined(CONFIG_CMD_RUN)
"env run var [...] - run commands in an environment variable\n"
#endif
#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
"env save - save environment\n"
#endif
+#if defined(CONFIG_CMD_BOOTEFI)
+ "env set [-e | -f] name [arg ...]\n";
+#else
"env set [-f] name [arg ...]\n";
#endif
+#endif
U_BOOT_CMD(
env, CONFIG_SYS_MAXARGS, 1, do_env,
@@ -1295,6 +1343,10 @@ U_BOOT_CMD_COMPLETE(
printenv, CONFIG_SYS_MAXARGS, 1, do_env_print,
"print environment variables",
"[-a]\n - print [all] values of all environment variables\n"
+#if defined(CONFIG_CMD_BOOTEFI)
+ "printenv -e [<name>]\n"
+ " - print UEFI variable 'name' or all the variables\n"
+#endif
"printenv name ...\n"
" - print value of environment variable 'name'",
var_complete
@@ -1322,7 +1374,11 @@ U_BOOT_CMD_COMPLETE(
U_BOOT_CMD_COMPLETE(
setenv, CONFIG_SYS_MAXARGS, 0, do_env_set,
"set environment variables",
- "[-f] name value ...\n"
+#if defined(CONFIG_CMD_BOOTEFI)
+ "-e <name> [<value>]\n"
+ " - set UEFI variable 'name' to 'value' ...'\n"
+#endif
+ "setenv [-f] name value ...\n"
" - [forcibly] set environment variable 'name' to 'value ...'\n"
"setenv [-f] name\n"
" - [forcibly] delete environment variable 'name'",
--
2.19.0
^ permalink raw reply related [flat|nested] 39+ messages in thread