Re: [U-Boot] [PATCH 08/11] efi_selftest: allow to select a single test for exexution

2017-10-09 Thread Alexander Graf


On 09.10.17 12:54, Heinrich Schuchardt wrote:
> On 10/09/2017 08:57 AM, Alexander Graf wrote:
>>
>>
>> On 09.10.17 08:14, Heinrich Schuchardt wrote:
>>> On 10/09/2017 06:46 AM, Alexander Graf wrote:


 On 08.10.17 06:57, Heinrich Schuchardt wrote:
> The second argument of bootefi is passed as a configuration
> table to the selftest application. It is used to select
> a single test to be executed.
>
> Tests get an on_request property. If this property is set
> the tests are only executed if explicitly requested.
>
> A new command 'bootefi selftest list' is added that allows to list
> all tests.
>
> The invocation of efi_selftest is changed to reflect that
> bootefi selftest list will call the Exit bootservice.
>
> Signed-off-by: Heinrich Schuchardt 

 Wouldn't it make more sense to just pass "bootargs" to the EFI payload
 as command line arguments?

 We could then just

   U-Boot# setenv bootargs list
   U-Boot# bootefi selftest

 to list the available self tests. Same for selecting them.
>>>
>>> Why bootargs?
>>>
>>
>> Because bootargs is the variable that "bootm" pushes into a payload as
>> command line arguments.
>>

 That way we would also be able to directly load Linux as EFI binary and
 pass command line arguments to it without jumping through fdt patching
 hoops.
>>>
>>> Does the Linux EFI stub or grub.efi have a capability to receive the
>>> command line?
>>
>> Linux does:
>>
>>
>> https://github.com/torvalds/linux/blob/master/drivers/firmware/efi/libstub/efi-stub-helper.c#L773
>>
>> I don't think grub implements it today, but I don't see why it should.
>> Any UEFI application that expects to be executed from the UEFI Shell
>> certainly interprets the passed in command line.
>>
>>
>> Alex
>>
> 
> Thanks for pointing me at fields LoadOptionsSize and LoadOptions in the
> EFI_Loaded_Image_Protocol. Yes it makes sense to use this mechanism.
> 
> I guess efi_setup_loaded_image would be a good place to set the fields.

I'm not sure about that. efi_setup_loaded_image gets called from payload
context too. Imagine a bootefi on a UEFI Shell which then runs Linux.
That Linux binary should not see our bootargs arguments.

I think it really belongs in do_bootefi_exec.


Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 08/11] efi_selftest: allow to select a single test for exexution

2017-10-09 Thread Heinrich Schuchardt
On 10/09/2017 08:57 AM, Alexander Graf wrote:
> 
> 
> On 09.10.17 08:14, Heinrich Schuchardt wrote:
>> On 10/09/2017 06:46 AM, Alexander Graf wrote:
>>>
>>>
>>> On 08.10.17 06:57, Heinrich Schuchardt wrote:
 The second argument of bootefi is passed as a configuration
 table to the selftest application. It is used to select
 a single test to be executed.

 Tests get an on_request property. If this property is set
 the tests are only executed if explicitly requested.

 A new command 'bootefi selftest list' is added that allows to list
 all tests.

 The invocation of efi_selftest is changed to reflect that
 bootefi selftest list will call the Exit bootservice.

 Signed-off-by: Heinrich Schuchardt 
>>>
>>> Wouldn't it make more sense to just pass "bootargs" to the EFI payload
>>> as command line arguments?
>>>
>>> We could then just
>>>
>>>   U-Boot# setenv bootargs list
>>>   U-Boot# bootefi selftest
>>>
>>> to list the available self tests. Same for selecting them.
>>
>> Why bootargs?
>>
> 
> Because bootargs is the variable that "bootm" pushes into a payload as
> command line arguments.
> 
>>>
>>> That way we would also be able to directly load Linux as EFI binary and
>>> pass command line arguments to it without jumping through fdt patching
>>> hoops.
>>
>> Does the Linux EFI stub or grub.efi have a capability to receive the
>> command line?
> 
> Linux does:
> 
> 
> https://github.com/torvalds/linux/blob/master/drivers/firmware/efi/libstub/efi-stub-helper.c#L773
> 
> I don't think grub implements it today, but I don't see why it should.
> Any UEFI application that expects to be executed from the UEFI Shell
> certainly interprets the passed in command line.
> 
> 
> Alex
> 

Thanks for pointing me at fields LoadOptionsSize and LoadOptions in the
EFI_Loaded_Image_Protocol. Yes it makes sense to use this mechanism.

I guess efi_setup_loaded_image would be a good place to set the fields.

We will have to free the associated memory when unloading the image
(efi_unload_image).

We should call efi_unload_image from efi_exit when indicated by the
subsystem and exit code.

I will put these changes into separate patches.

Regards

Heinrich
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 08/11] efi_selftest: allow to select a single test for exexution

2017-10-08 Thread Alexander Graf


On 09.10.17 08:14, Heinrich Schuchardt wrote:
> On 10/09/2017 06:46 AM, Alexander Graf wrote:
>>
>>
>> On 08.10.17 06:57, Heinrich Schuchardt wrote:
>>> The second argument of bootefi is passed as a configuration
>>> table to the selftest application. It is used to select
>>> a single test to be executed.
>>>
>>> Tests get an on_request property. If this property is set
>>> the tests are only executed if explicitly requested.
>>>
>>> A new command 'bootefi selftest list' is added that allows to list
>>> all tests.
>>>
>>> The invocation of efi_selftest is changed to reflect that
>>> bootefi selftest list will call the Exit bootservice.
>>>
>>> Signed-off-by: Heinrich Schuchardt 
>>
>> Wouldn't it make more sense to just pass "bootargs" to the EFI payload
>> as command line arguments?
>>
>> We could then just
>>
>>   U-Boot# setenv bootargs list
>>   U-Boot# bootefi selftest
>>
>> to list the available self tests. Same for selecting them.
> 
> Why bootargs?
> 

Because bootargs is the variable that "bootm" pushes into a payload as
command line arguments.

>>
>> That way we would also be able to directly load Linux as EFI binary and
>> pass command line arguments to it without jumping through fdt patching
>> hoops.
> 
> Does the Linux EFI stub or grub.efi have a capability to receive the
> command line?

Linux does:


https://github.com/torvalds/linux/blob/master/drivers/firmware/efi/libstub/efi-stub-helper.c#L773

I don't think grub implements it today, but I don't see why it should.
Any UEFI application that expects to be executed from the UEFI Shell
certainly interprets the passed in command line.


Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 08/11] efi_selftest: allow to select a single test for exexution

2017-10-08 Thread Heinrich Schuchardt
On 10/09/2017 06:46 AM, Alexander Graf wrote:
> 
> 
> On 08.10.17 06:57, Heinrich Schuchardt wrote:
>> The second argument of bootefi is passed as a configuration
>> table to the selftest application. It is used to select
>> a single test to be executed.
>>
>> Tests get an on_request property. If this property is set
>> the tests are only executed if explicitly requested.
>>
>> A new command 'bootefi selftest list' is added that allows to list
>> all tests.
>>
>> The invocation of efi_selftest is changed to reflect that
>> bootefi selftest list will call the Exit bootservice.
>>
>> Signed-off-by: Heinrich Schuchardt 
> 
> Wouldn't it make more sense to just pass "bootargs" to the EFI payload
> as command line arguments?
> 
> We could then just
> 
>   U-Boot# setenv bootargs list
>   U-Boot# bootefi selftest
> 
> to list the available self tests. Same for selecting them.

Why bootargs?

> 
> That way we would also be able to directly load Linux as EFI binary and
> pass command line arguments to it without jumping through fdt patching
> hoops.

Does the Linux EFI stub or grub.efi have a capability to receive the
command line?

Regards

Heinrich
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 08/11] efi_selftest: allow to select a single test for exexution

2017-10-08 Thread Simon Glass
On 7 October 2017 at 22:57, Heinrich Schuchardt  wrote:
> The second argument of bootefi is passed as a configuration
> table to the selftest application. It is used to select
> a single test to be executed.
>
> Tests get an on_request property. If this property is set
> the tests are only executed if explicitly requested.
>
> A new command 'bootefi selftest list' is added that allows to list
> all tests.
>
> The invocation of efi_selftest is changed to reflect that
> bootefi selftest list will call the Exit bootservice.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  cmd/bootefi.c| 24 --
>  include/efi_selftest.h   | 18 
>  lib/efi_selftest/efi_selftest.c  | 88 
> ++--
>  lib/efi_selftest/efi_selftest_util.c |  9 
>  4 files changed, 132 insertions(+), 7 deletions(-)

Reviewed-by: Simon Glass 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 08/11] efi_selftest: allow to select a single test for exexution

2017-10-08 Thread Alexander Graf


On 08.10.17 06:57, Heinrich Schuchardt wrote:
> The second argument of bootefi is passed as a configuration
> table to the selftest application. It is used to select
> a single test to be executed.
> 
> Tests get an on_request property. If this property is set
> the tests are only executed if explicitly requested.
> 
> A new command 'bootefi selftest list' is added that allows to list
> all tests.
> 
> The invocation of efi_selftest is changed to reflect that
> bootefi selftest list will call the Exit bootservice.
> 
> Signed-off-by: Heinrich Schuchardt 

Wouldn't it make more sense to just pass "bootargs" to the EFI payload
as command line arguments?

We could then just

  U-Boot# setenv bootargs list
  U-Boot# bootefi selftest

to list the available self tests. Same for selecting them.

That way we would also be able to directly load Linux as EFI binary and
pass command line arguments to it without jumping through fdt patching
hoops.


Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH 08/11] efi_selftest: allow to select a single test for exexution

2017-10-07 Thread Heinrich Schuchardt
The second argument of bootefi is passed as a configuration
table to the selftest application. It is used to select
a single test to be executed.

Tests get an on_request property. If this property is set
the tests are only executed if explicitly requested.

A new command 'bootefi selftest list' is added that allows to list
all tests.

The invocation of efi_selftest is changed to reflect that
bootefi selftest list will call the Exit bootservice.

Signed-off-by: Heinrich Schuchardt 
---
 cmd/bootefi.c| 24 --
 include/efi_selftest.h   | 18 
 lib/efi_selftest/efi_selftest.c  | 88 ++--
 lib/efi_selftest/efi_selftest_util.c |  9 
 4 files changed, 132 insertions(+), 7 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 24958ada46..efc9e5d263 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -275,20 +276,34 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int 
argc, char * const argv[])
if (!strcmp(argv[1], "selftest")) {
struct efi_loaded_image loaded_image_info = {};
struct efi_object loaded_image_info_obj = {};
+   char *test = NULL;
 
efi_setup_loaded_image(&loaded_image_info,
   &loaded_image_info_obj,
-  bootefi_device_path, bootefi_image_path);
+  NULL, NULL);
/*
 * gd lives in a fixed register which may get clobbered while we
 * execute the payload. So save it here and restore it on every
 * callback entry
 */
efi_save_gd();
+   loaded_image_info.image_code_type = EFI_LOADER_CODE;
+   loaded_image_info.image_data_type = EFI_LOADER_DATA;
/* Initialize and populate EFI object list */
if (!efi_obj_list_initalized)
efi_init_obj_list();
-   return efi_selftest(&loaded_image_info, &systab);
+   /*
+* The second parameter specifies the test to be executed.
+* Transfer it as configuration table.
+*/
+   if (argc > 2)
+   test = argv[2];
+   efi_install_configuration_table(&efi_selftest_table_guid, test);
+   /* Execute the test */
+   r = efi_selftest(&loaded_image_info, &systab);
+   efi_restore_gd();
+   list_del(&loaded_image_info_obj.link);
+   return r;
} else
 #endif
if (!strcmp(argv[1], "bootmgr")) {
@@ -332,8 +347,11 @@ static char bootefi_help_text[] =
"  - boot a sample Hello World application stored within U-Boot\n"
 #endif
 #ifdef CONFIG_CMD_BOOTEFI_SELFTEST
-   "bootefi selftest\n"
+   "bootefi selftest [test name]\n"
"  - boot an EFI selftest application stored within U-Boot\n"
+   "If specified only a named test is executed.\n"
+   "bootefi selftest list\n"
+   "list available tests\n"
 #endif
"bootmgr [fdt addr]\n"
"  - load and boot EFI payload based on BootOrder/Boot variables.\n"
diff --git a/include/efi_selftest.h b/include/efi_selftest.h
index 7ec42a0406..ea0f180251 100644
--- a/include/efi_selftest.h
+++ b/include/efi_selftest.h
@@ -71,6 +71,16 @@ void efi_st_printf(const char *fmt, ...)
  */
 int efi_st_memcmp(const void *buf1, const void *buf2, size_t length);
 
+/*
+ * Compare strings.
+ * We cannot use lib/string.c due to different CFLAGS values.
+ *
+ * @buf1:  first buffer
+ * @buf2:  second buffer
+ * @return:0 if both buffers contain the same bytes
+ */
+int efi_st_strcmp(const char *buf1, const char *buf2);
+
 /*
  * Reads an Unicode character from the input device.
  *
@@ -88,6 +98,7 @@ u16 efi_st_get_key(void);
  * @setup: set up the unit test
  * @teardown:  tear down the unit test
  * @execute:   execute the unit test
+ * @on_request:test is only executed on request
  */
 struct efi_unit_test {
const char *name;
@@ -96,10 +107,17 @@ struct efi_unit_test {
 const struct efi_system_table *systable);
int (*execute)(void);
int (*teardown)(void);
+   bool on_request;
 };
 
 /* Declare a new EFI unit test */
 #define EFI_UNIT_TEST(__name)  \
ll_entry_declare(struct efi_unit_test, __name, efi_unit_test)
 
+#define EFI_SELFTEST_TABLE_GUID \
+   EFI_GUID(0xbc3ebe57, 0x09e5, 0xa59d, 0xdb, 0x87, \
+0xf5, 0x79, 0x61, 0x62, 0x06, 0xde)
+
+extern const efi_guid_t efi_selftest_table_guid;
+
 #endif /* _EFI_SELFTEST_H */
diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c
index 45d8d3d384..35a961aeb0 100644
--- a/lib/efi_selftest/efi_s