[U-Boot] [PATCH 16/16] efi: sandbox: Add a simple 'bootefi test' command

[Simon Glass]

Hi Heinrich,

On 18 September 2017 at 05:02, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 09/18/2017 12:59 AM, Simon Glass wrote:
>> This jumps to test code which can call directly into the EFI support. It
>> does not need a separate image so it is easy to write tests with it.
>>
>> For now the test just outputs a message. To try it:
>>
>> ./sandbox/u-boot -c "bootefi test"
>> U-Boot 2017.09-00204-g696c9855fe (Sep 17 2017 - 16:43:53 -0600)
>>
>> DRAM:  128 MiB
>> MMC:
>> Using default environment
>>
>> In:    serial
>> Out:   serial
>> Err:   serial
>> SCSI:  Net:   No ethernet found.
>> IDE:   Bus 0: not available
>> Found 0 disks
>> Hello, world!
>> Test passed
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>>  cmd/bootefi.c             | 18 ++++++++++++++++++
>>  configs/sandbox_defconfig |  1 +
>>  include/efi_loader.h      |  3 +++
>>  lib/efi_loader/Kconfig    | 10 ++++++++++
>>  lib/efi_loader/Makefile   |  1 +
>>  lib/efi_loader/efi_test.c | 17 +++++++++++++++++
>>  6 files changed, 50 insertions(+)
>>  create mode 100644 lib/efi_loader/efi_test.c
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index ee07733e3e..f499103d23 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -323,6 +323,24 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>               memcpy((char *)addr, __efi_hello_world_begin, size);
>>       } else
>>  #endif
>> +     if (IS_ENABLED(CONFIG_BOOTEFI_TEST) && !strcmp(argv[1], "test")) {
>> +             int ret;
>> +
>> +             /* Initialize and populate EFI object list */
>> +             if (efi_init_obj_list())
>> +                     return CMD_RET_FAILURE;
>> +
>> +             loaded_image_info.device_handle = bootefi_device_path;
>> +             loaded_image_info.file_path = bootefi_image_path;
>> +             ret = efi_test(&loaded_image_info, &systab);
>> +             if (ret) {
>> +                     printf("Test failed: err=%d\n", ret);
>> +                     return CMD_RET_FAILURE;
>> +             }
>> +             printf("Test passed\n");
>> +
>> +             return 0;
>> +     }
>>  #ifdef CONFIG_CMD_BOOTEFI_SELFTEST
>>       if (!strcmp(argv[1], "selftest")) {
>>               /*
>> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
>> index 72600afea8..ab63f639de 100644
>> --- a/configs/sandbox_defconfig
>> +++ b/configs/sandbox_defconfig
>> @@ -196,3 +196,4 @@ CONFIG_UT_TIME=y
>>  CONFIG_UT_DM=y
>>  CONFIG_UT_ENV=y
>>  CONFIG_UT_OVERLAY=y
>> +CONFIG_CMD_BOOTEFI_SELFTEST=y
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 79d2dad22c..43919137b0 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -263,6 +263,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
>>                                struct efi_system_table *systab);
>>  #endif
>>
>> +/* Perform EFI tests */
>> +int efi_test(efi_handle_t image_handle, struct efi_system_table *systab);
>> +
>>  #else /* defined(EFI_LOADER) && !defined(CONFIG_SPL_BUILD) */
>>
>>  /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */
>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>> index dee0a96a98..659b2b18f4 100644
>> --- a/lib/efi_loader/Kconfig
>> +++ b/lib/efi_loader/Kconfig
>> @@ -16,3 +16,13 @@ config EFI_LOADER_BOUNCE_BUFFER
>>         Some hardware does not support DMA to full 64bit addresses. For this
>>         hardware we can create a bounce buffer so that payloads don't have to
>>         worry about platform details.
>> +
>> +config BOOTEFI_TEST
>> +     bool "Provide a test for the EFI loader"
>> +     depends on EFI_LOADER && SANDBOX
>> +     default y
>> +     help
>> +       Provides a test of the EFI loader functionality accessed via the
>> +       command line ('bootefi test'). This runs within U-Boot so does not
>> +       need a separate EFI application to work. It aims to include coverage
>> +       of all EFI code which can be accessed within sandbox.
>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>> index 30bf343a36..69eb93518d 100644
>> --- a/lib/efi_loader/Makefile
>> +++ b/lib/efi_loader/Makefile
>> @@ -21,3 +21,4 @@ obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>>  obj-$(CONFIG_PARTITIONS) += efi_disk.o
>>  obj-$(CONFIG_NET) += efi_net.o
>>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
>> +obj-$(CONFIG_BOOTEFI_TEST) += efi_test.o
>> diff --git a/lib/efi_loader/efi_test.c b/lib/efi_loader/efi_test.c
>> new file mode 100644
>> index 0000000000..3fdce78b05
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_test.c
>> @@ -0,0 +1,17 @@
>> +/*
>> + * Copyright (c) 2017, Google Inc. All rights reserved.
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <efi_api.h>
>> +
>> +int efi_test(efi_handle_t image_handle, struct efi_system_table *systable)
>> +{
>> +     struct efi_simple_text_output_protocol *con_out = systable->con_out;
>> +
>> +     con_out->output_string(con_out, L"Hello, world!\n");
>> +
>> +     return 0;
>> +}
>>
>
> Hello Simon,
>
> why do we need this patch?
> You could just call bootefi selftest to demonstrate the same (after
> fixing the memory map initialization for the sandbox).

Yes, but I'd like to use sandbox for testing where possible.

>
> Could you, please, review the framework that I have setup for bootefi
> selftest. Does the design make sense to you?

Yes it looks good, now reviewed. I think we need both:

- sandbox test for ease of use (but will not provide 100% coverage,
e.g. I'm not sure we can implement exit boot services)
- selftest to actually test the API calls properly (should be able to
test nearly everything here)

Regards,
Simon