[kvm-unit-tests PATCH v1 0/3] x86 UEFI: pass envs and args

[kvm-unit-tests PATCH v1 0/3] x86 UEFI: pass envs and args

[Zixuan Wang]

Hello,

This patch series enables kvm-unit-tests to get envs and args under
UEFI. The host passes envs and args through files:

1. The host stores envs into ENVS.TXT and args into ARGS.TXT
2. The guest boots up and reads data from these files through UEFI file
operation services
3. The file data is passed to corresponding setup functions

As a result, several x86 test cases (e.g., kvmclock_test and vmexit)
can now get envs/args from the host [1], thus do not report FAIL when
running ./run-tests.sh.

An alternative approach for envs/args passing under UEFI is to use
QEMU's -append/-initrd options. However, this approach requires EFI
binaries to be passed through QEMU's -kernel option. While currently,
EFI binaries are loaded from a disk image. Changing this bootup process
may make kvm-unit-tests (under UEFI) unable to run on bare-metal [2].
On the other hand, passing envs/args through files should work on
bare-metal because UEFI's file operation services do not rely on QEMU's
functionalities, thus working on bare-metal.

The summary of this patch series:

Patch #1 pulls Linux kernel's UEFI definitions for file operations.

Patch #2 implements file read functions and envs setup functions.

Patch #3 implements the args setup functions.

Best regards,
Zixuan

[1] https://github.com/TheNetAdmin/KVM-Unit-Tests-dev-fork/issues/8
[2] https://lore.kernel.org/kvm/CAEDJ5ZQLm1rz+0a7MPPz3wMAoeTq2oH9z92sd0ZhCxEjWMkOpg@mail.gmail.com

Zixuan Wang (3):
  x86 UEFI: pull UEFI definitions for file operations
  x86 UEFI: read envs from file
  x86 UEFI: read args from file

 lib/efi.c       | 150 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/linux/efi.h |  82 +++++++++++++++++++++++++-
 x86/efi/run     |  36 +++++++++++-
 3 files changed, 265 insertions(+), 3 deletions(-)

-- 
2.35.1

[kvm-unit-tests PATCH v1 1/3] x86 UEFI: pull UEFI definitions for file operations

[Zixuan Wang]

From: Zixuan Wang <zxwang42@gmail.com>

Pull kernel's UEFI definitions for file operations. These definitions
will enable kvm-unit-tests to read files from the disk image, through
UEFI services.

Signed-off-by: Zixuan Wang <zxwang42@gmail.com>
---
 lib/linux/efi.h | 82 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/lib/linux/efi.h b/lib/linux/efi.h
index 455625a..a811bbd 100644
--- a/lib/linux/efi.h
+++ b/lib/linux/efi.h
@@ -1,5 +1,9 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-/* Relevant definitions from linux/efi.h. */
+/*
+ * Relevant definitions from
+ *   - linux/efi.h
+ *   - drivers/firmware/efi/libstub/efistub.h
+ */
 
 #ifndef __LINUX_UEFI_H
 #define __LINUX_UEFI_H
@@ -58,7 +62,10 @@ typedef guid_t efi_guid_t;
 	(b) & 0xff, ((b) >> 8) & 0xff,						\
 	(c) & 0xff, ((c) >> 8) & 0xff, d } }
 
-#define ACPI_TABLE_GUID EFI_GUID(0xeb9d2d30, 0x2d88, 0x11d3, 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
+#define ACPI_TABLE_GUID			EFI_GUID(0xeb9d2d30, 0x2d88, 0x11d3, 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
+#define LOADED_IMAGE_PROTOCOL_GUID	EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
+#define EFI_FILE_SYSTEM_GUID		EFI_GUID(0x964e5b22, 0x6459, 0x11d2, 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
+#define EFI_FILE_INFO_ID		EFI_GUID(0x09576e92, 0x6d3f, 0x11d2, 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
 
 typedef struct {
 	efi_guid_t guid;
@@ -416,6 +423,77 @@ struct efi_boot_memmap {
 	unsigned long           *buff_size;
 };
 
+#define __aligned_u64 u64 __attribute__((aligned(8)))
+typedef struct {
+	u32			revision;
+	efi_handle_t		parent_handle;
+	efi_system_table_t	*system_table;
+	efi_handle_t		device_handle;
+	void			*file_path;
+	void			*reserved;
+	u32			load_options_size;
+	void			*load_options;
+	void			*image_base;
+	__aligned_u64		image_size;
+	unsigned int		image_code_type;
+	unsigned int		image_data_type;
+	efi_status_t		(__efiapi *unload)(efi_handle_t image_handle);
+} efi_loaded_image_t;
+
+typedef struct {
+	u64			size;
+	u64			file_size;
+	u64			phys_size;
+	efi_time_t		create_time;
+	efi_time_t		last_access_time;
+	efi_time_t		modification_time;
+	__aligned_u64		attribute;
+	efi_char16_t		filename[];
+} efi_file_info_t;
+
+typedef struct efi_file_protocol efi_file_protocol_t;
+
+struct efi_file_protocol {
+	u64		revision;
+	efi_status_t	(__efiapi *open)	(efi_file_protocol_t *,
+						 efi_file_protocol_t **,
+						 efi_char16_t *, u64, u64);
+	efi_status_t	(__efiapi *close)	(efi_file_protocol_t *);
+	efi_status_t	(__efiapi *delete)	(efi_file_protocol_t *);
+	efi_status_t	(__efiapi *read)	(efi_file_protocol_t *,
+						 unsigned long *, void *);
+	efi_status_t	(__efiapi *write)	(efi_file_protocol_t *,
+						 unsigned long, void *);
+	efi_status_t	(__efiapi *get_position)(efi_file_protocol_t *, u64 *);
+	efi_status_t	(__efiapi *set_position)(efi_file_protocol_t *, u64);
+	efi_status_t	(__efiapi *get_info)	(efi_file_protocol_t *,
+						 efi_guid_t *, unsigned long *,
+						 void *);
+	efi_status_t	(__efiapi *set_info)	(efi_file_protocol_t *,
+						 efi_guid_t *, unsigned long,
+						 void *);
+	efi_status_t	(__efiapi *flush)	(efi_file_protocol_t *);
+};
+
+typedef struct efi_simple_file_system_protocol efi_simple_file_system_protocol_t;
+
+struct efi_simple_file_system_protocol {
+	u64	revision;
+	int	(__efiapi *open_volume)(efi_simple_file_system_protocol_t *,
+					efi_file_protocol_t **);
+};
+
+#define EFI_FILE_MODE_READ	0x0000000000000001
+#define EFI_FILE_MODE_WRITE	0x0000000000000002
+#define EFI_FILE_MODE_CREATE	0x8000000000000000
+
+#define MAX_FILENAME_SIZE	256
+
+struct finfo {
+	efi_file_info_t info;
+	efi_char16_t	filename[MAX_FILENAME_SIZE];
+};
+
 #define efi_bs_call(func, ...) efi_system_table->boottime->func(__VA_ARGS__)
 #define efi_rs_call(func, ...) efi_system_table->runtime->func(__VA_ARGS__)
 
-- 
2.35.1

[kvm-unit-tests PATCH v1 2/3] x86 UEFI: read envs from file

[Zixuan Wang]

From: Zixuan Wang <zxwang42@gmail.com>

This commit enables kvm-unit-tests to get envs from the host through
UEFI.

Previously, when kvm-unit-tests runs with Multiboot, it gets a kernel
binary through QEMU's '-kernel' argument and envs through the '-initrd'
argument.

But under UEFI, kvm-unit-tests loads an EFI binary from a disk image
instead of QEMU's '-kernel' argument. In this case, QEMU rejects the
'-initrd' argument without the '-kernel' argument being used, which
makes kvm-unit-tests unable to get an initrd through this argument.

This commit enables the host to pass the initrd through a file:
  1. The host runner script 'x86/efi/run' captures the '-initrd'
     argument and suppress it from QEMU arguments, so QEMU will not
     report the error
  2. The runner script reads the default initrd and the user-specified
     initrd to generate an 'ENVS.TXT' file for each EFI test case
  3. The kvm-unit-tests EFI set up process reads this 'ENVS.TXT' from
     the disk image and pass it to 'setup_env()'

Signed-off-by: Zixuan Wang <zxwang42@gmail.com>
---
 lib/efi.c   | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 x86/efi/run |  26 ++++++++++-
 2 files changed, 151 insertions(+), 1 deletion(-)

diff --git a/lib/efi.c b/lib/efi.c
index 64cc978..4ddc3a4 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -8,6 +8,7 @@
  */
 
 #include "efi.h"
+#include "argv.h"
 #include <libcflat.h>
 #include <asm/setup.h>
 
@@ -96,6 +97,121 @@ static void efi_exit(efi_status_t code)
 	efi_rs_call(reset_system, EFI_RESET_SHUTDOWN, code, 0, NULL);
 }
 
+static efi_status_t efi_get_volume(efi_handle_t handle,
+				   efi_file_protocol_t **volume)
+{
+	efi_guid_t loaded_image_protocol = LOADED_IMAGE_PROTOCOL_GUID;
+	efi_guid_t file_system_protocol = EFI_FILE_SYSTEM_GUID;
+	efi_loaded_image_t *loaded_image = NULL;
+	efi_simple_file_system_protocol_t *io;
+	efi_status_t status;
+
+	status = efi_bs_call(handle_protocol, handle, &loaded_image_protocol,
+			     (void **)&loaded_image);
+	if (status != EFI_SUCCESS) {
+		printf("ERROR: failed to handle loaded image");
+		goto efi_get_volume_error;
+	}
+
+	status = efi_bs_call(handle_protocol, loaded_image->device_handle,
+			     &file_system_protocol, (void **)&io);
+	if (status != EFI_SUCCESS) {
+		printf("ERROR: failed to handle file system protocol");
+		goto efi_get_volume_error;
+	}
+
+	status = io->open_volume(io, volume);
+	if (status != EFI_SUCCESS) {
+		printf("ERROR: failed to open volume");
+		goto efi_get_volume_error;
+	}
+
+	return EFI_SUCCESS;
+
+efi_get_volume_error:
+	printf(" error: 0x%lx\n", status);
+	return EFI_ABORTED;
+}
+
+static efi_status_t efi_read_file(efi_file_protocol_t *volume,
+				  efi_char16_t *file_name,
+				  unsigned long *file_size, char **file_data)
+{
+	efi_guid_t file_info_guid = EFI_FILE_INFO_ID;
+	efi_file_protocol_t *file_handle = NULL;
+	struct finfo file_info;
+	unsigned long file_info_size;
+	efi_status_t status;
+
+	status = volume->open(volume, &file_handle, file_name,
+			      EFI_FILE_MODE_READ, 0);
+	if (status != EFI_SUCCESS) {
+		printf("ERROR: failed to open file");
+		goto efi_read_file_error;
+	}
+
+	file_info_size = sizeof(file_info);
+	status = file_handle->get_info(file_handle, &file_info_guid,
+				       &file_info_size, &file_info);
+	if (status != EFI_SUCCESS) {
+		printf("ERROR: failed to get file info");
+		goto efi_read_file_error;
+	}
+
+	*file_size = file_info.info.file_size;
+	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, *file_size + 1,
+			     (void **)file_data);
+	if (status != EFI_SUCCESS) {
+		printf("ERROR: failed allocate buffer");
+		goto efi_read_file_error;
+	}
+
+	status = file_handle->read(file_handle, file_size, *file_data);
+	if (status != EFI_SUCCESS) {
+		printf("ERROR: failed to read file data");
+		goto efi_read_file_error;
+	}
+
+	status = file_handle->close(file_handle);
+	if (status != EFI_SUCCESS) {
+		printf("ERROR: failed to close file");
+		goto efi_read_file_error;
+	}
+
+	(*file_data)[*file_size] = '\0';
+
+	return EFI_SUCCESS;
+
+efi_read_file_error:
+	/*
+	 * TODO: Current printf does not support wide char (2nd byte of the each
+	 * wide char is always a '\0'), thus only the 1st character is printed.
+	 */
+	printf(" file: %ls, error: 0x%lx\n", file_name, status);
+	return EFI_ABORTED;
+}
+
+static efi_status_t efi_set_up_envs(efi_file_protocol_t *volume)
+{
+	efi_char16_t file_name[] = L"ENVS.TXT";
+	unsigned long file_size;
+	char *file_data = NULL;
+	efi_status_t status;
+
+	status = efi_read_file(volume, file_name, &file_size, &file_data);
+	if (status != EFI_SUCCESS) {
+		printf("Failed to read file\n");
+		goto efi_set_up_envs_error;
+	}
+
+	setup_env(file_data, (int)file_size);
+
+	return EFI_SUCCESS;
+
+efi_set_up_envs_error:
+	return EFI_ABORTED;
+}
+
 efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
 {
 	int ret;
@@ -103,12 +219,22 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
 	efi_bootinfo_t efi_bootinfo;
 
 	efi_system_table = sys_tab;
+	efi_file_protocol_t *volume = NULL;
 
 	/* Memory map struct values */
 	efi_memory_desc_t *map = NULL;
 	unsigned long map_size = 0, desc_size = 0, key = 0, buff_size = 0;
 	u32 desc_ver;
 
+	/* Open env and args files */
+	status = efi_get_volume(handle, &volume);
+	if (status != EFI_SUCCESS) {
+		printf("Failed to get volume\n");
+		goto efi_main_error;
+	}
+
+	efi_set_up_envs(volume);
+
 	/* Set up efi_bootinfo */
 	efi_bootinfo.mem_map.map = &map;
 	efi_bootinfo.mem_map.map_size = &map_size;
diff --git a/x86/efi/run b/x86/efi/run
index ac368a5..f4a5930 100755
--- a/x86/efi/run
+++ b/x86/efi/run
@@ -12,6 +12,7 @@ if [ ! -f config.mak ]; then
 	exit 2
 fi
 source config.mak
+source scripts/arch-run.bash
 
 : "${EFI_SRC:=$(realpath "$(dirname "$0")/../")}"
 : "${EFI_UEFI:=/usr/share/ovmf/OVMF.fd}"
@@ -43,6 +44,29 @@ fi
 mkdir -p "$EFI_CASE_DIR"
 cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_BINARY"
 
+# Capture -initrd
+efi_qemu_args=""
+efi_qemu_initrd=""
+while [[ -n "$1" ]]; do
+	case "$1" in
+		--initrd|-initrd)
+			shift 1
+			efi_qemu_initrd="$1"
+			shift 1
+		;;
+		*)
+			efi_qemu_args+=" $1"
+			shift 1
+		;;
+	esac
+done
+
+# Create ENVS file
+initrd_create
+KVM_UNIT_TESTS_EFI_ENV="$EFI_TEST/$EFI_CASE/ENVS.TXT"
+mv "$KVM_UNIT_TESTS_ENV" "$KVM_UNIT_TESTS_EFI_ENV"
+[[ -f "$efi_qemu_initrd" ]] && cat "$efi_qemu_initrd" >> "$KVM_UNIT_TESTS_EFI_ENV"
+
 # Run test case with 256MiB QEMU memory. QEMU default memory size is 128MiB.
 # After UEFI boot up and we call `LibMemoryMap()`, the largest consecutive
 # memory region is ~42MiB. Although this is sufficient for many test cases to
@@ -60,5 +84,5 @@ cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_BINARY"
 	-net none \
 	-nographic \
 	-m 256 \
-	"$@" \
+	${efi_qemu_args} \
 	-smp "$EFI_SMP"
-- 
2.35.1

[kvm-unit-tests PATCH v1 3/3] x86 UEFI: read args from file

[Zixuan Wang]

From: Zixuan Wang <zxwang42@gmail.com>

This commit enables kvm-unit-tests to get args from the host through
UEFI.

Previously, when compiled to run under UEFI, kvm-unit-tests could not
get args from the host through QEMU's '-append' argument. This is
because kvm-unit-tests (under UEFI) does not rely on QEMU's '-kernel'
argument, while without which, QEMU rejects the '-append' argument.

This commit enables the host to pass the args through a file. This
process is similar to the '-initrd' support introduced in the previous
commit.

Signed-off-by: Zixuan Wang <zxwang42@gmail.com>
---
 lib/efi.c   | 24 ++++++++++++++++++++++++
 x86/efi/run | 12 +++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/lib/efi.c b/lib/efi.c
index 4ddc3a4..dfde788 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -14,6 +14,7 @@
 
 /* From lib/argv.c */
 extern int __argc, __envc;
+extern const char *__args;
 extern char *__argv[100];
 extern char *__environ[200];
 
@@ -212,6 +213,28 @@ efi_set_up_envs_error:
 	return EFI_ABORTED;
 }
 
+static efi_status_t efi_set_up_args(efi_file_protocol_t *volume)
+{
+	efi_char16_t file_name[] = L"ARGS.TXT";
+	unsigned long file_size;
+	char *file_data = NULL;
+	efi_status_t status;
+
+	status = efi_read_file(volume, file_name, &file_size, &file_data);
+	if (status != EFI_SUCCESS) {
+		printf("Failed to read file\n");
+		goto efi_set_up_envs_error;
+	}
+
+	__args = file_data;
+	__setup_args();
+
+	return EFI_SUCCESS;
+
+efi_set_up_envs_error:
+	return EFI_ABORTED;
+}
+
 efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
 {
 	int ret;
@@ -234,6 +257,7 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
 	}
 
 	efi_set_up_envs(volume);
+	efi_set_up_args(volume);
 
 	/* Set up efi_bootinfo */
 	efi_bootinfo.mem_map.map = &map;
diff --git a/x86/efi/run b/x86/efi/run
index f4a5930..e02a6e1 100755
--- a/x86/efi/run
+++ b/x86/efi/run
@@ -44,9 +44,10 @@ fi
 mkdir -p "$EFI_CASE_DIR"
 cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_BINARY"
 
-# Capture -initrd
+# Capture -initrd and -append
 efi_qemu_args=""
 efi_qemu_initrd=""
+efi_qemu_append=""
 while [[ -n "$1" ]]; do
 	case "$1" in
 		--initrd|-initrd)
@@ -54,6 +55,11 @@ while [[ -n "$1" ]]; do
 			efi_qemu_initrd="$1"
 			shift 1
 		;;
+		--append|-append)
+			shift 1
+			efi_qemu_append="$1"
+			shift 1
+		;;
 		*)
 			efi_qemu_args+=" $1"
 			shift 1
@@ -67,6 +73,10 @@ KVM_UNIT_TESTS_EFI_ENV="$EFI_TEST/$EFI_CASE/ENVS.TXT"
 mv "$KVM_UNIT_TESTS_ENV" "$KVM_UNIT_TESTS_EFI_ENV"
 [[ -f "$efi_qemu_initrd" ]] && cat "$efi_qemu_initrd" >> "$KVM_UNIT_TESTS_EFI_ENV"
 
+# Create ARGS file
+KVM_UNIT_TESTS_EFI_ARG="$EFI_TEST/$EFI_CASE/ARGS.TXT"
+echo -n "$efi_qemu_append" > "$KVM_UNIT_TESTS_EFI_ARG"
+
 # Run test case with 256MiB QEMU memory. QEMU default memory size is 128MiB.
 # After UEFI boot up and we call `LibMemoryMap()`, the largest consecutive
 # memory region is ~42MiB. Although this is sufficient for many test cases to
-- 
2.35.1

Re: [kvm-unit-tests PATCH v1 0/3] x86 UEFI: pass envs and args

[Andrew Jones]

On Sun, Feb 20, 2022 at 02:42:31PM -0800, Zixuan Wang wrote:
> Hello,
> 
> This patch series enables kvm-unit-tests to get envs and args under
> UEFI. The host passes envs and args through files:
> 
> 1. The host stores envs into ENVS.TXT and args into ARGS.TXT

EFI already has support for an environment and EFI apps can accept args.
Why not find a way to convert kvm-unit-tests ENV and unit tests args
into the EFI system and then use that?

efi_setup_argv()[*] in my original PoC does that. It uses gnu-efi, but
it should be easy to strip away the gnu-efi stuff and go straight for
the underlining EFI functions.

[*] https://github.com/rhdrjones/kvm-unit-tests/commit/12a49a2e97b457e23af10bb25cd972362b379951#:~:text=static%20void%20efi_setup_argv(EFI_HANDLE%20Image%2C%20EFI_SYSTEM_TABLE%20*SysTab)

If you want to mimic efi_setup_argv(), then you'll also need 85baf398
("lib/argv: Allow environ to be primed") from that same branch.

EFI wrapper scripts for each unit test can be generated to pass the args
to the unit test EFI apps automatically. For the environment, the EFI
vars can be set as usual for the system. For QEMU, that means creating
a VARS.fd and then adding another flash device to the VM to exposes it.

Thanks,
drew


> 2. The guest boots up and reads data from these files through UEFI file
> operation services
> 3. The file data is passed to corresponding setup functions
> 
> As a result, several x86 test cases (e.g., kvmclock_test and vmexit)
> can now get envs/args from the host [1], thus do not report FAIL when
> running ./run-tests.sh.
> 
> An alternative approach for envs/args passing under UEFI is to use
> QEMU's -append/-initrd options. However, this approach requires EFI
> binaries to be passed through QEMU's -kernel option. While currently,
> EFI binaries are loaded from a disk image. Changing this bootup process
> may make kvm-unit-tests (under UEFI) unable to run on bare-metal [2].
> On the other hand, passing envs/args through files should work on
> bare-metal because UEFI's file operation services do not rely on QEMU's
> functionalities, thus working on bare-metal.
> 
> The summary of this patch series:
> 
> Patch #1 pulls Linux kernel's UEFI definitions for file operations.
> 
> Patch #2 implements file read functions and envs setup functions.
> 
> Patch #3 implements the args setup functions.
> 
> Best regards,
> Zixuan
> 
> [1] https://github.com/TheNetAdmin/KVM-Unit-Tests-dev-fork/issues/8
> [2] https://lore.kernel.org/kvm/CAEDJ5ZQLm1rz+0a7MPPz3wMAoeTq2oH9z92sd0ZhCxEjWMkOpg@mail.gmail.com
> 
> Zixuan Wang (3):
>   x86 UEFI: pull UEFI definitions for file operations
>   x86 UEFI: read envs from file
>   x86 UEFI: read args from file
> 
>  lib/efi.c       | 150 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/linux/efi.h |  82 +++++++++++++++++++++++++-
>  x86/efi/run     |  36 +++++++++++-
>  3 files changed, 265 insertions(+), 3 deletions(-)
> 
> -- 
> 2.35.1
> 

Re: [kvm-unit-tests PATCH v1 0/3] x86 UEFI: pass envs and args

[Andrew Jones]

On Mon, Feb 21, 2022 at 09:40:56AM +0100, Andrew Jones wrote:
> On Sun, Feb 20, 2022 at 02:42:31PM -0800, Zixuan Wang wrote:
> > Hello,
> > 
> > This patch series enables kvm-unit-tests to get envs and args under
> > UEFI. The host passes envs and args through files:
> > 
> > 1. The host stores envs into ENVS.TXT and args into ARGS.TXT
> 
> EFI already has support for an environment and EFI apps can accept args.
> Why not find a way to convert kvm-unit-tests ENV and unit tests args
> into the EFI system and then use that?
> 
> efi_setup_argv()[*] in my original PoC does that. It uses gnu-efi, but
> it should be easy to strip away the gnu-efi stuff and go straight for
> the underlining EFI functions.
> 
> [*] https://github.com/rhdrjones/kvm-unit-tests/commit/12a49a2e97b457e23af10bb25cd972362b379951#:~:text=static%20void%20efi_setup_argv(EFI_HANDLE%20Image%2C%20EFI_SYSTEM_TABLE%20*SysTab)
> 
> If you want to mimic efi_setup_argv(), then you'll also need 85baf398
> ("lib/argv: Allow environ to be primed") from that same branch.
> 
> EFI wrapper scripts for each unit test can be generated to pass the args
> to the unit test EFI apps automatically. For the environment, the EFI
> vars can be set as usual for the system. For QEMU, that means creating
> a VARS.fd and then adding another flash device to the VM to exposes it.

BTW, this tool from Gerd might be useful for that

https://gitlab.com/kraxel/edk2-tests/-/blob/master/tools/vars.py

Thanks,
drew

> 
> Thanks,
> drew
> 
> 
> > 2. The guest boots up and reads data from these files through UEFI file
> > operation services
> > 3. The file data is passed to corresponding setup functions
> > 
> > As a result, several x86 test cases (e.g., kvmclock_test and vmexit)
> > can now get envs/args from the host [1], thus do not report FAIL when
> > running ./run-tests.sh.
> > 
> > An alternative approach for envs/args passing under UEFI is to use
> > QEMU's -append/-initrd options. However, this approach requires EFI
> > binaries to be passed through QEMU's -kernel option. While currently,
> > EFI binaries are loaded from a disk image. Changing this bootup process
> > may make kvm-unit-tests (under UEFI) unable to run on bare-metal [2].
> > On the other hand, passing envs/args through files should work on
> > bare-metal because UEFI's file operation services do not rely on QEMU's
> > functionalities, thus working on bare-metal.
> > 
> > The summary of this patch series:
> > 
> > Patch #1 pulls Linux kernel's UEFI definitions for file operations.
> > 
> > Patch #2 implements file read functions and envs setup functions.
> > 
> > Patch #3 implements the args setup functions.
> > 
> > Best regards,
> > Zixuan
> > 
> > [1] https://github.com/TheNetAdmin/KVM-Unit-Tests-dev-fork/issues/8
> > [2] https://lore.kernel.org/kvm/CAEDJ5ZQLm1rz+0a7MPPz3wMAoeTq2oH9z92sd0ZhCxEjWMkOpg@mail.gmail.com
> > 
> > Zixuan Wang (3):
> >   x86 UEFI: pull UEFI definitions for file operations
> >   x86 UEFI: read envs from file
> >   x86 UEFI: read args from file
> > 
> >  lib/efi.c       | 150 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/linux/efi.h |  82 +++++++++++++++++++++++++-
> >  x86/efi/run     |  36 +++++++++++-
> >  3 files changed, 265 insertions(+), 3 deletions(-)
> > 
> > -- 
> > 2.35.1
> > 

Re: [kvm-unit-tests PATCH v1 0/3] x86 UEFI: pass envs and args

[Zixuan Wang]

On Mon, Feb 21, 2022 at 7:26 AM Andrew Jones <drjones@redhat.com> wrote:
>
> On Mon, Feb 21, 2022 at 09:40:56AM +0100, Andrew Jones wrote:
> > On Sun, Feb 20, 2022 at 02:42:31PM -0800, Zixuan Wang wrote:
> > > Hello,
> > >
> > > This patch series enables kvm-unit-tests to get envs and args under
> > > UEFI. The host passes envs and args through files:
> > >
> > > 1. The host stores envs into ENVS.TXT and args into ARGS.TXT
> >
> > EFI already has support for an environment and EFI apps can accept args.
> > Why not find a way to convert kvm-unit-tests ENV and unit tests args
> > into the EFI system and then use that?
> >
> > efi_setup_argv()[*] in my original PoC does that. It uses gnu-efi, but
> > it should be easy to strip away the gnu-efi stuff and go straight for
> > the underlining EFI functions.
> >
> > [*] https://github.com/rhdrjones/kvm-unit-tests/commit/12a49a2e97b457e23af10bb25cd972362b379951#:~:text=static%20void%20efi_setup_argv(EFI_HANDLE%20Image%2C%20EFI_SYSTEM_TABLE%20*SysTab)
> >
> > If you want to mimic efi_setup_argv(), then you'll also need 85baf398
> > ("lib/argv: Allow environ to be primed") from that same branch.

I think one way to implement EFI environment passing is to generate a
startup.nsh which sets the envs. But this could significantly degrade
the bootup speed because UEFI firmware waits 30 seconds for user input
before executing the startup.nsh. This slow bootup causes
./run-tests.sh to run extremely slower (~30mins or even longer), and
motivated us to implement the faster bootup process which does not
rely on the startup.nsh [**].

[**] https://lore.kernel.org/all/20211116204053.220523-10-zxwang42@gmail.com/

> > EFI wrapper scripts for each unit test can be generated to pass the args
> > to the unit test EFI apps automatically. For the environment, the EFI
> > vars can be set as usual for the system. For QEMU, that means creating
> > a VARS.fd and then adding another flash device to the VM to exposes it.

Setting up a .fd file (or an EFI NVRAM if I understand it correctly)
is interesting. Actually, I previously tried it in another way, which
is to set variables from the guest:

1. The host generates and runs a temporary test case that writes envs
and args to EFI NVRAM. This test case uses EFI firmware's
SetVariable() service with NON_VOLATILE | BOOTSERVICE_ACCESS
attributes.
2. The host calls the actual test case which reads the modified NVRAM.

This approach does not work because the guest cannot persist the NVRAM
variables in main(). This is because main() runs after EFI's
ExitBootServices(), and thus cannot set a new non-volatile variable
[***].

I can try it again with some other tricks, e.g., efi_main() creates
the non-volatile variable if not exists, then main() sets the
non-volatile variable.

[***] SetVariable(), Page 241,
https://uefi.org/sites/default/files/resources/UEFI%20Spec%202_6.pdf

> BTW, this tool from Gerd might be useful for that
>
> https://gitlab.com/kraxel/edk2-tests/-/blob/master/tools/vars.py

This script seems to modify the EFI .fd file from the host. I
previously checked several similar scripts but didn't try any.

One thing I was hesitant to modify EFI NVRAM from the host is, it may
introduce more lines of code than the current file-based envs/args
passing. I can try and see if I can simplify the script mentioned
above.

> Thanks,
> drew
>
> >
> > Thanks,
> > drew
> >
> >
> > > 2. The guest boots up and reads data from these files through UEFI file
> > > operation services
> > > 3. The file data is passed to corresponding setup functions
> > >
> > > As a result, several x86 test cases (e.g., kvmclock_test and vmexit)
> > > can now get envs/args from the host [1], thus do not report FAIL when
> > > running ./run-tests.sh.
> > >
> > > An alternative approach for envs/args passing under UEFI is to use
> > > QEMU's -append/-initrd options. However, this approach requires EFI
> > > binaries to be passed through QEMU's -kernel option. While currently,
> > > EFI binaries are loaded from a disk image. Changing this bootup process
> > > may make kvm-unit-tests (under UEFI) unable to run on bare-metal [2].
> > > On the other hand, passing envs/args through files should work on
> > > bare-metal because UEFI's file operation services do not rely on QEMU's
> > > functionalities, thus working on bare-metal.
> > >
> > > The summary of this patch series:
> > >
> > > Patch #1 pulls Linux kernel's UEFI definitions for file operations.
> > >
> > > Patch #2 implements file read functions and envs setup functions.
> > >
> > > Patch #3 implements the args setup functions.
> > >
> > > Best regards,
> > > Zixuan
> > >
> > > [1] https://github.com/TheNetAdmin/KVM-Unit-Tests-dev-fork/issues/8
> > > [2] https://lore.kernel.org/kvm/CAEDJ5ZQLm1rz+0a7MPPz3wMAoeTq2oH9z92sd0ZhCxEjWMkOpg@mail.gmail.com
> > >
> > > Zixuan Wang (3):
> > >   x86 UEFI: pull UEFI definitions for file operations
> > >   x86 UEFI: read envs from file
> > >   x86 UEFI: read args from file
> > >
> > >  lib/efi.c       | 150 ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  lib/linux/efi.h |  82 +++++++++++++++++++++++++-
> > >  x86/efi/run     |  36 +++++++++++-
> > >  3 files changed, 265 insertions(+), 3 deletions(-)
> > >
> > > --
> > > 2.35.1
> > >
>

Best regards,
Zixuan

Re: [kvm-unit-tests PATCH v1 0/3] x86 UEFI: pass envs and args

[Gerd Hoffmann]

  Hi,

> > EFI wrapper scripts for each unit test can be generated to pass the args
> > to the unit test EFI apps automatically. For the environment, the EFI
> > vars can be set as usual for the system. For QEMU, that means creating
> > a VARS.fd and then adding another flash device to the VM to exposes it.
> 
> BTW, this tool from Gerd might be useful for that
> 
> https://gitlab.com/kraxel/edk2-tests/-/blob/master/tools/vars.py

Probably not (yet) as this right now focuses on stuff needed for secure
boot, i.e. enroll certificates etc.

Setting variables with ascii (or unicode) strings should be rather easy
to add though.

take care,
  Gerd