[kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[Alexandru Elisei]

The first two patches are fixes for stuff I found while working on patch
#3.

Patch #3 ("configure: Make the --target option available to all
architectures") was suggested by Drew when he reviewed my series to add
support to run_tests.sh for kvmtool [1] (kvmtool can only be used to run
the arm/arm64 tests). With this patch, now all architecture have --target
as a configure option, with the only valid value being "qemu" (with the
exception of arm and arm64, of course). This was suggested by Drew for two
reasons:

* There's a possibility that kvm-unit-tests will get support for other VMMs
  in the future (his example was Rust VMM).

* The changes to the scripts to support kvmtool were rather awkward, as
  testing the value of $TARGET was some of the time accompanied by testing
  the value of $ARCH.

I renamed --target-efi to --efi-payload in the last patch because I felt it
looked rather confusing to do ./configure --target=qemu --target-efi when
configuring the tests. If the rename is not acceptable, I can think of a
few other options:

1. Rename --target to --vmm. That was actually the original name for the
option, but I changed it because I thought --target was more generic and
that --target=efi would be the way going forward to compile kvm-unit-tests
to run as an EFI payload. I realize now that separating the VMM from
compiling kvm-unit-tests to run as an EFI payload is better, as there can
be multiple VMMs that can run UEFI in a VM. Not many people use kvmtool as
a test runner, so I think the impact on users should be minimal.

2. Keep both option as they are. Personally, I think that would be
confusing to the end user, but I don't have a strong opinion against it.

[1] https://www.spinics.net/lists/kvm/msg247896.html

Alexandru Elisei (4):
  configure: Fix whitespaces for the --gen-se-header help text
  configure: Restrict --target-efi to x86_64
  configure: Make the --target option available to all architectures
  Rename --target-efi to --efi-payload

 Makefile             | 10 +++-------
 configure            | 45 +++++++++++++++++++++++++++-----------------
 lib/x86/acpi.c       |  4 ++--
 lib/x86/amd_sev.h    |  4 ++--
 lib/x86/asm/page.h   |  8 ++++----
 lib/x86/asm/setup.h  |  4 ++--
 lib/x86/setup.c      |  4 ++--
 lib/x86/vm.c         | 12 ++++++------
 scripts/runtime.bash |  4 ++--
 x86/Makefile.common  |  6 +++---
 x86/Makefile.x86_64  |  6 +++---
 x86/access_test.c    |  2 +-
 x86/efi/README.md    |  2 +-
 x86/efi/run          |  2 +-
 x86/run              |  4 ++--
 15 files changed, 62 insertions(+), 55 deletions(-)

-- 
2.35.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[kvm-unit-tests PATCH 1/4] configure: Fix whitespaces for the --gen-se-header help text

[Alexandru Elisei]

Replace some of the tabs with spaces to display the help text for the
--gen-se-header option like this:

    --gen-se-header=GEN_SE_HEADER
                           Provide an executable to generate a PV header
                           requires --host-key-document. (s390x-snippets only)

instead of:

    --gen-se-header=GEN_SE_HEADER
                           Provide an executable to generate a PV header
   requires --host-key-document. (s390x-snippets only)

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 2d9c3e051103..0ac9c85502ff 100755
--- a/configure
+++ b/configure
@@ -58,7 +58,7 @@ usage() {
 	                           a PVM image with 'genprotimg' (s390x only)
 	    --gen-se-header=GEN_SE_HEADER
 	                           Provide an executable to generate a PV header
-				   requires --host-key-document. (s390x-snippets only)
+	                           requires --host-key-document. (s390x-snippets only)
 	    --page-size=PAGE_SIZE
 	                           Specify the page size (translation granule) (4k, 16k or
 	                           64k, default is 64k, arm64 only)
-- 
2.35.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[kvm-unit-tests PATCH 2/4] configure: Restrict --target-efi to x86_64

[Alexandru Elisei]

Setting the --target-efi option for any architecture but x86_64 results in
an error while trying to compile the tests:

$ ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu- --target-efi
$ make clean && make
Makefile:46: *** Cannot build aarch64 tests as EFI apps.  Stop.

Which might come as a surprise for users, as the help message for the
configure script makes no mention of an architectur being incompatible
with the option.

Document that --target-efi applies only to the x86_64 architecture and
check for illegal usage in the configure script, instead of failing later,
at compile time.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 Makefile  | 4 ----
 configure | 7 ++++++-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 4f4ad235fe0c..5af17f129ced 100644
--- a/Makefile
+++ b/Makefile
@@ -40,11 +40,7 @@ OBJDIRS += $(LIBFDT_objdir)
 
 # EFI App
 ifeq ($(TARGET_EFI),y)
-ifeq ($(ARCH_NAME),x86_64)
 EFI_ARCH = x86_64
-else
-$(error Cannot build $(ARCH_NAME) tests as EFI apps)
-endif
 EFI_CFLAGS := -DTARGET_EFI
 # The following CFLAGS and LDFLAGS come from:
 #   - GNU-EFI/Makefile.defaults
diff --git a/configure b/configure
index 0ac9c85502ff..6620e78ec09c 100755
--- a/configure
+++ b/configure
@@ -74,7 +74,7 @@ usage() {
 	               pl011,mmio32,ADDR
 	                           Specify a PL011 compatible UART at address ADDR. Supported
 	                           register stride is 32 bit only.
-	    --target-efi           Boot and run from UEFI
+	    --target-efi           Boot and run from UEFI (x86_64 only)
 EOF
     exit 1
 }
@@ -177,6 +177,11 @@ else
     fi
 fi
 
+if [ "$target_efi" ] && [ "$arch" != "x86_64" ]; then
+    echo "--target-efi is not supported for $arch"
+    usage
+fi
+
 if [ -z "$page_size" ]; then
     [ "$arch" = "arm64" ] && page_size="65536"
     [ "$arch" = "arm" ] && page_size="4096"
-- 
2.35.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[kvm-unit-tests PATCH 3/4] configure: Make the --target option available to all architectures

[Alexandru Elisei]

The arm and arm64 architectures got the --target option to support running
with either qemu or kvmtool as the virtual machine manager. Make --target
valid for the other architectures, in which case qemu will be the only
valid target.

Generating the $TARGET variable in config.mak regardless of the
architecture will make adding support for another VMM to run_tests.sh
easier.

Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 configure | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/configure b/configure
index 6620e78ec09c..2720b7efd64a 100755
--- a/configure
+++ b/configure
@@ -22,7 +22,7 @@ pretty_print_stacks=yes
 environ_default=yes
 u32_long=
 wa_divide=
-target=
+target=qemu
 errata_force=0
 erratatxt="$srcdir/errata.txt"
 host_key_document=
@@ -38,8 +38,8 @@ usage() {
 	Options include:
 	    --arch=ARCH            architecture to compile for ($arch)
 	    --processor=PROCESSOR  processor to compile for ($arch)
-	    --target=TARGET        target platform that the tests will be running on (qemu or
-	                           kvmtool, default is qemu) (arm/arm64 only)
+	    --target=TARGET        target platform that the tests will be running on (default is qemu)
+	                           (qemu or kvmtool for arm and arm64, qemu for all other architectures)
 	    --cross-prefix=PREFIX  cross compiler prefix
 	    --cc=CC                c compiler to use ($cc)
 	    --cflags=FLAGS         extra options to be passed to the c compiler
@@ -168,14 +168,22 @@ arch_name=$arch
 [ "$arch" = "aarch64" ] && arch="arm64"
 [ "$arch_name" = "arm64" ] && arch_name="aarch64"
 
-if [ -z "$target" ]; then
-    target="qemu"
-else
+case "$target" in
+qemu)
+    # All architectures support qemu as the target.
+    ;;
+kvmtool)
+    # Only arm and arm64 support running under kvmtool.
     if [ "$arch" != "arm64" ] && [ "$arch" != "arm" ]; then
-        echo "--target is not supported for $arch"
+        echo "--target=$target is not supported for $arch"
         usage
     fi
-fi
+    ;;
+*)
+    echo "--target=$target is not supported for $arch"
+    usage
+    ;;
+esac
 
 if [ "$target_efi" ] && [ "$arch" != "x86_64" ]; then
     echo "--target-efi is not supported for $arch"
@@ -369,10 +377,8 @@ GENPROTIMG=${GENPROTIMG-genprotimg}
 HOST_KEY_DOCUMENT=$host_key_document
 TARGET_EFI=$target_efi
 GEN_SE_HEADER=$gen_se_header
+TARGET=$target
 EOF
-if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
-    echo "TARGET=$target" >> config.mak
-fi
 
 cat <<EOF > lib/config.h
 #ifndef _CONFIG_H_
-- 
2.35.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[kvm-unit-tests PATCH RFC 4/4] Rename --target-efi to --efi-payload

[Alexandru Elisei]

Now that the --target configure option is available to all architectures,
rename --target-efi to --efi-payload to avoid confusion between the two.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 Makefile             |  6 +++---
 configure            | 16 ++++++++--------
 lib/x86/acpi.c       |  4 ++--
 lib/x86/amd_sev.h    |  4 ++--
 lib/x86/asm/page.h   |  8 ++++----
 lib/x86/asm/setup.h  |  4 ++--
 lib/x86/setup.c      |  4 ++--
 lib/x86/vm.c         | 12 ++++++------
 scripts/runtime.bash |  4 ++--
 x86/Makefile.common  |  6 +++---
 x86/Makefile.x86_64  |  6 +++---
 x86/access_test.c    |  2 +-
 x86/efi/README.md    |  2 +-
 x86/efi/run          |  2 +-
 x86/run              |  4 ++--
 15 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/Makefile b/Makefile
index 5af17f129ced..cc4b37ac6152 100644
--- a/Makefile
+++ b/Makefile
@@ -39,9 +39,9 @@ LIBFDT_archive = $(LIBFDT_objdir)/libfdt.a
 OBJDIRS += $(LIBFDT_objdir)
 
 # EFI App
-ifeq ($(TARGET_EFI),y)
+ifeq ($(EFI_PAYLOAD),y)
 EFI_ARCH = x86_64
-EFI_CFLAGS := -DTARGET_EFI
+EFI_CFLAGS := -DEFI_PAYLOAD
 # The following CFLAGS and LDFLAGS come from:
 #   - GNU-EFI/Makefile.defaults
 #   - GNU-EFI/apps/Makefile
@@ -81,7 +81,7 @@ COMMON_CFLAGS += $(fno_stack_protector)
 COMMON_CFLAGS += $(fno_stack_protector_all)
 COMMON_CFLAGS += $(wno_frame_address)
 COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
-ifeq ($(TARGET_EFI),y)
+ifeq ($(EFI_PAYLOAD),y)
 COMMON_CFLAGS += $(EFI_CFLAGS)
 else
 COMMON_CFLAGS += $(fno_pic) $(no_pie)
diff --git a/configure b/configure
index 2720b7efd64a..95a44ac7ff44 100755
--- a/configure
+++ b/configure
@@ -29,7 +29,7 @@ host_key_document=
 gen_se_header=
 page_size=
 earlycon=
-target_efi=
+efi_payload=
 
 usage() {
     cat <<-EOF
@@ -74,7 +74,7 @@ usage() {
 	               pl011,mmio32,ADDR
 	                           Specify a PL011 compatible UART at address ADDR. Supported
 	                           register stride is 32 bit only.
-	    --target-efi           Boot and run from UEFI (x86_64 only)
+	    --efi-payload          Boot and run from UEFI (x86_64 only)
 EOF
     exit 1
 }
@@ -142,8 +142,8 @@ while [[ "$1" = -* ]]; do
 	--earlycon)
 	    earlycon="$arg"
 	    ;;
-	--target-efi)
-	    target_efi=y
+	--efi-payload)
+	    efi_payload=y
 	    ;;
 	--help)
 	    usage
@@ -185,8 +185,8 @@ kvmtool)
     ;;
 esac
 
-if [ "$target_efi" ] && [ "$arch" != "x86_64" ]; then
-    echo "--target-efi is not supported for $arch"
+if [ "$efi_payload" ] && [ "$arch" != "x86_64" ]; then
+    echo "--efi-payload is not supported for $arch"
     usage
 fi
 
@@ -286,7 +286,7 @@ if [ -f "$srcdir/$testdir/run" ]; then
 fi
 
 testsubdir=$testdir
-if [ -n "$target_efi" ]; then
+if [ -n "$efi_payload" ]; then
     testsubdir=$testdir/efi
 fi
 
@@ -375,7 +375,7 @@ U32_LONG_FMT=$u32_long
 WA_DIVIDE=$wa_divide
 GENPROTIMG=${GENPROTIMG-genprotimg}
 HOST_KEY_DOCUMENT=$host_key_document
-TARGET_EFI=$target_efi
+EFI_PAYLOAD=$efi_payload
 GEN_SE_HEADER=$gen_se_header
 TARGET=$target
 EOF
diff --git a/lib/x86/acpi.c b/lib/x86/acpi.c
index 1a82ced0b90f..dbdc3fb3da85 100644
--- a/lib/x86/acpi.c
+++ b/lib/x86/acpi.c
@@ -1,7 +1,7 @@
 #include "libcflat.h"
 #include "acpi.h"
 
-#ifdef TARGET_EFI
+#ifdef EFI_PAYLOAD
 struct rsdp_descriptor *efi_rsdp = NULL;
 
 void set_efi_rsdp(struct rsdp_descriptor *rsdp)
@@ -34,7 +34,7 @@ static struct rsdp_descriptor *get_rsdp(void)
 
 	return rsdp;
 }
-#endif /* TARGET_EFI */
+#endif /* EFI_PAYLOAD */
 
 void* find_acpi_table_addr(u32 sig)
 {
diff --git a/lib/x86/amd_sev.h b/lib/x86/amd_sev.h
index 6a10f845daba..e513b82e1634 100644
--- a/lib/x86/amd_sev.h
+++ b/lib/x86/amd_sev.h
@@ -12,7 +12,7 @@
 #ifndef _X86_AMD_SEV_H_
 #define _X86_AMD_SEV_H_
 
-#ifdef TARGET_EFI
+#ifdef EFI_PAYLOAD
 
 #include "libcflat.h"
 #include "desc.h"
@@ -58,6 +58,6 @@ void setup_ghcb_pte(pgd_t *page_table);
 unsigned long long get_amd_sev_c_bit_mask(void);
 unsigned long long get_amd_sev_addr_upperbound(void);
 
-#endif /* TARGET_EFI */
+#endif /* EFI_PAYLOAD */
 
 #endif /* _X86_AMD_SEV_H_ */
diff --git a/lib/x86/asm/page.h b/lib/x86/asm/page.h
index c25bc66b7aa4..bfa855cd3204 100644
--- a/lib/x86/asm/page.h
+++ b/lib/x86/asm/page.h
@@ -25,11 +25,11 @@ typedef unsigned long pgd_t;
 #define LARGE_PAGE_SIZE	(1024 * PAGE_SIZE)
 #endif
 
-#ifdef TARGET_EFI
+#ifdef EFI_PAYLOAD
 /* lib/x86/amd_sev.c */
 extern unsigned long long get_amd_sev_c_bit_mask(void);
 extern unsigned long long get_amd_sev_addr_upperbound(void);
-#endif /* TARGET_EFI */
+#endif /* EFI_PAYLOAD */
 
 #define PT_PRESENT_MASK		(1ull << 0)
 #define PT_WRITABLE_MASK	(1ull << 1)
@@ -47,11 +47,11 @@ extern unsigned long long get_amd_sev_addr_upperbound(void);
  */
 #define PT_ADDR_UPPER_BOUND_DEFAULT	(51)
 
-#ifdef TARGET_EFI
+#ifdef EFI_PAYLOAD
 #define PT_ADDR_UPPER_BOUND	(get_amd_sev_addr_upperbound())
 #else
 #define PT_ADDR_UPPER_BOUND	(PT_ADDR_UPPER_BOUND_DEFAULT)
-#endif /* TARGET_EFI */
+#endif /* EFI_PAYLOAD */
 
 #define PT_ADDR_LOWER_BOUND	(PAGE_SHIFT)
 #define PT_ADDR_MASK		GENMASK_ULL(PT_ADDR_UPPER_BOUND, PT_ADDR_LOWER_BOUND)
diff --git a/lib/x86/asm/setup.h b/lib/x86/asm/setup.h
index dbfb2a22bc1b..c0a1e0934dbb 100644
--- a/lib/x86/asm/setup.h
+++ b/lib/x86/asm/setup.h
@@ -3,7 +3,7 @@
 
 unsigned long setup_tss(u8 *stacktop);
 
-#ifdef TARGET_EFI
+#ifdef EFI_PAYLOAD
 #include "x86/acpi.h"
 #include "x86/apic.h"
 #include "x86/processor.h"
@@ -14,6 +14,6 @@ unsigned long setup_tss(u8 *stacktop);
 
 efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo);
 void setup_5level_page_table(void);
-#endif /* TARGET_EFI */
+#endif /* EFI_PAYLOAD */
 
 #endif /* _X86_ASM_SETUP_H_ */
diff --git a/lib/x86/setup.c b/lib/x86/setup.c
index bbd34682b79e..64ea802d4f3d 100644
--- a/lib/x86/setup.c
+++ b/lib/x86/setup.c
@@ -167,7 +167,7 @@ void setup_multiboot(struct mbi_bootinfo *bi)
 	initrd_size = mods->end - mods->start;
 }
 
-#ifdef TARGET_EFI
+#ifdef EFI_PAYLOAD
 
 /* From x86/efi/efistart64.S */
 extern void load_idt(void);
@@ -330,7 +330,7 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
 	return EFI_SUCCESS;
 }
 
-#endif /* TARGET_EFI */
+#endif /* EFI_PAYLOAD */
 
 void setup_libcflat(void)
 {
diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index 56be57be673a..110c2309defd 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -26,9 +26,9 @@ pteval_t *install_pte(pgd_t *cr3,
                 pt_page = 0;
 	    memset(new_pt, 0, PAGE_SIZE);
 	    pt[offset] = virt_to_phys(new_pt) | PT_PRESENT_MASK | PT_WRITABLE_MASK | pte_opt_mask;
-#ifdef TARGET_EFI
+#ifdef EFI_PAYLOAD
 	    pt[offset] |= get_amd_sev_c_bit_mask();
-#endif /* TARGET_EFI */
+#endif /* EFI_PAYLOAD */
 	}
 	pt = phys_to_virt(pt[offset] & PT_ADDR_MASK);
     }
@@ -98,18 +98,18 @@ pteval_t *get_pte_level(pgd_t *cr3, void *virt, int pte_level)
 pteval_t *install_large_page(pgd_t *cr3, phys_addr_t phys, void *virt)
 {
     phys_addr_t flags = PT_PRESENT_MASK | PT_WRITABLE_MASK | pte_opt_mask | PT_PAGE_SIZE_MASK;
-#ifdef TARGET_EFI
+#ifdef EFI_PAYLOAD
     flags |= get_amd_sev_c_bit_mask();
-#endif /* TARGET_EFI */
+#endif /* EFI_PAYLOAD */
     return install_pte(cr3, 2, virt, phys | flags, 0);
 }
 
 pteval_t *install_page(pgd_t *cr3, phys_addr_t phys, void *virt)
 {
     phys_addr_t flags = PT_PRESENT_MASK | PT_WRITABLE_MASK | pte_opt_mask;
-#ifdef TARGET_EFI
+#ifdef EFI_PAYLOAD
     flags |= get_amd_sev_c_bit_mask();
-#endif /* TARGET_EFI */
+#endif /* EFI_PAYLOAD */
     return install_pte(cr3, 1, virt, phys | flags, 0);
 }
 
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 6d5fced94246..140f704ed2a0 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -82,7 +82,7 @@ function run()
     local accel="$8"
     local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
 
-    if [ "${TARGET_EFI}" == "y" ]; then
+    if [ "${EFI_PAYLOAD}" == "y" ]; then
         kernel=$(basename $kernel .flat)
     fi
 
@@ -132,7 +132,7 @@ function run()
 
     last_line=$(premature_failure > >(tail -1)) && {
         skip=true
-        if [ "${TARGET_EFI}" == "y" ] && [[ "${last_line}" =~ "enabling apic" ]]; then
+        if [ "${EFI_PAYLOAD}" == "y" ] && [[ "${last_line}" =~ "enabling apic" ]]; then
             skip=false
         fi
         if [ ${skip} == true ]; then
diff --git a/x86/Makefile.common b/x86/Makefile.common
index ff02d9822321..b95a12372350 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -22,7 +22,7 @@ cflatobjs += lib/x86/acpi.o
 cflatobjs += lib/x86/stack.o
 cflatobjs += lib/x86/fault_test.o
 cflatobjs += lib/x86/delay.o
-ifeq ($(TARGET_EFI),y)
+ifeq ($(EFI_PAYLOAD),y)
 cflatobjs += lib/x86/amd_sev.o
 cflatobjs += lib/efi.o
 cflatobjs += x86/efi/reloc_x86_64.o
@@ -44,7 +44,7 @@ KEEP_FRAME_POINTER := y
 
 FLATLIBS = lib/libcflat.a
 
-ifeq ($(TARGET_EFI),y)
+ifeq ($(EFI_PAYLOAD),y)
 .PRECIOUS: %.efi %.so
 
 %.so: %.o $(FLATLIBS) $(SRCDIR)/x86/efi/elf_x86_64_efi.lds $(cstart.o)
@@ -89,7 +89,7 @@ tests-common = $(TEST_DIR)/vmexit.$(exe) $(TEST_DIR)/tsc.$(exe) \
 # The following test cases are disabled when building EFI tests because they
 # use absolute addresses in their inline assembly code, which cannot compile
 # with the '-fPIC' flag
-ifneq ($(TARGET_EFI),y)
+ifneq ($(EFI_PAYLOAD),y)
 tests-common += $(TEST_DIR)/realmode.$(exe)
 endif
 
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
index a3cb75ae5868..3765b3db1e08 100644
--- a/x86/Makefile.x86_64
+++ b/x86/Makefile.x86_64
@@ -1,7 +1,7 @@
 cstart.o = $(TEST_DIR)/cstart64.o
 bits = 64
 ldarch = elf64-x86-64
-ifeq ($(TARGET_EFI),y)
+ifeq ($(EFI_PAYLOAD),y)
 exe = efi
 bin = so
 FORMAT = efi-app-x86_64
@@ -32,14 +32,14 @@ tests += $(TEST_DIR)/rdpru.$(exe)
 tests += $(TEST_DIR)/pks.$(exe)
 tests += $(TEST_DIR)/pmu_lbr.$(exe)
 
-ifeq ($(TARGET_EFI),y)
+ifeq ($(EFI_PAYLOAD),y)
 tests += $(TEST_DIR)/amd_sev.$(exe)
 endif
 
 # The following test cases are disabled when building EFI tests because they
 # use absolute addresses in their inline assembly code, which cannot compile
 # with the '-fPIC' flag
-ifneq ($(TARGET_EFI),y)
+ifneq ($(EFI_PAYLOAD),y)
 tests += $(TEST_DIR)/access_test.$(exe)
 tests += $(TEST_DIR)/svm.$(exe)
 tests += $(TEST_DIR)/vmx.$(exe)
diff --git a/x86/access_test.c b/x86/access_test.c
index ef1243f0c151..fe9ed840519e 100644
--- a/x86/access_test.c
+++ b/x86/access_test.c
@@ -10,7 +10,7 @@ int main(void)
     printf("starting test\n\n");
     r = ac_test_run(PT_LEVEL_PML4);
 
-#ifndef TARGET_EFI
+#ifndef EFI_PAYLOAD
     /*
      * Not supported yet for UEFI, because setting up 5
      * level page table requires entering real mode.
diff --git a/x86/efi/README.md b/x86/efi/README.md
index a39f509cd9aa..459c82dfcc5e 100644
--- a/x86/efi/README.md
+++ b/x86/efi/README.md
@@ -15,7 +15,7 @@ The following dependencies should be installed:
 
 To build:
 
-    ./configure --target-efi
+    ./configure --efi-payload
     make
 
 ### Run test cases with UEFI
diff --git a/x86/efi/run b/x86/efi/run
index ac368a59ba9f..9e77b51b9f3c 100755
--- a/x86/efi/run
+++ b/x86/efi/run
@@ -8,7 +8,7 @@ if [ $# -eq 0 ]; then
 fi
 
 if [ ! -f config.mak ]; then
-	echo "run './configure --target-efi && make' first. See ./configure -h"
+	echo "run './configure --efi-payload && make' first. See ./configure -h"
 	exit 2
 fi
 source config.mak
diff --git a/x86/run b/x86/run
index 582d1eda0fd9..58f81d4853f0 100755
--- a/x86/run
+++ b/x86/run
@@ -39,12 +39,12 @@ fi
 
 command="${qemu} --no-reboot -nodefaults $pc_testdev -vnc none -serial stdio $pci_testdev"
 command+=" -machine accel=$ACCEL"
-if [ "${TARGET_EFI}" != y ]; then
+if [ "${EFI_PAYLOAD}" != y ]; then
 	command+=" -kernel"
 fi
 command="$(timeout_cmd) $command"
 
-if [ "${TARGET_EFI}" = y ]; then
+if [ "${EFI_PAYLOAD}" = y ]; then
 	# Set ENVIRON_DEFAULT=n to remove '-initrd' flag for QEMU (see
 	# 'scripts/arch-run.bash' for more details). This is because when using
 	# UEFI, the test case binaries are passed to QEMU through the disk
-- 
2.35.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[Sean Christopherson]

On Thu, Feb 10, 2022, Alexandru Elisei wrote:
> I renamed --target-efi to --efi-payload in the last patch because I felt it
> looked rather confusing to do ./configure --target=qemu --target-efi when
> configuring the tests. If the rename is not acceptable, I can think of a
> few other options:

I find --target-efi to be odd irrespective of this new conflict.  A simple --efi
seems like it would be sufficient.

> 1. Rename --target to --vmm. That was actually the original name for the
> option, but I changed it because I thought --target was more generic and
> that --target=efi would be the way going forward to compile kvm-unit-tests
> to run as an EFI payload. I realize now that separating the VMM from
> compiling kvm-unit-tests to run as an EFI payload is better, as there can
> be multiple VMMs that can run UEFI in a VM. Not many people use kvmtool as
> a test runner, so I think the impact on users should be minimal.

Again irrespective of --target-efi, I think --target for the VMM is a potentially
confusing name.  Target Triplet[*] and --target have specific meaning for the
compiler, usurping that for something similar but slightly different is odd.

But why is the VMM specified at ./configure time?  Why can't it be an option to
run_tests.sh?  E.g. --target-efi in configure makes sense because it currently
requires different compilation options, but even that I hope we can someday change
so that x86-64 always builds EFI-friendly tests.  I really don't want to get to a
point where tests themselves have to be recompiled to run under different VMMs.

[*] https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Specifying-Target-Triplets.html
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[Alexandru Elisei]

Hi,

On Thu, Feb 10, 2022 at 05:25:46PM +0000, Sean Christopherson wrote:
> On Thu, Feb 10, 2022, Alexandru Elisei wrote:
> > I renamed --target-efi to --efi-payload in the last patch because I felt it
> > looked rather confusing to do ./configure --target=qemu --target-efi when
> > configuring the tests. If the rename is not acceptable, I can think of a
> > few other options:
> 
> I find --target-efi to be odd irrespective of this new conflict.  A simple --efi
> seems like it would be sufficient.
> 
> > 1. Rename --target to --vmm. That was actually the original name for the
> > option, but I changed it because I thought --target was more generic and
> > that --target=efi would be the way going forward to compile kvm-unit-tests
> > to run as an EFI payload. I realize now that separating the VMM from
> > compiling kvm-unit-tests to run as an EFI payload is better, as there can
> > be multiple VMMs that can run UEFI in a VM. Not many people use kvmtool as
> > a test runner, so I think the impact on users should be minimal.
> 
> Again irrespective of --target-efi, I think --target for the VMM is a potentially
> confusing name.  Target Triplet[*] and --target have specific meaning for the
> compiler, usurping that for something similar but slightly different is odd.

Wouldn't that mean that --target-efi is equally confusing? Do you have
suggestions for other names?

> 
> But why is the VMM specified at ./configure time?  Why can't it be an option to
> run_tests.sh?  E.g. --target-efi in configure makes sense because it currently
> requires different compilation options, but even that I hope we can someday change
> so that x86-64 always builds EFI-friendly tests.  I really don't want to get to a
> point where tests themselves have to be recompiled to run under different VMMs.

Setting the VMM at configure time was initially added to remove a warning
from lib/arm/io.c, where if the UART address if different than what
kvm-unit-tests expects the test would print:

WARNING: early print support may not work. Found uart at 0x1000000, but early base is 0x9000000.

kvmtool emulates a different UART, at a different address than what qemu
emulates, and kvm-unit-tests compares the address found in the DTB with the
qemu UART's address (the address is used to be a #define lib/arm/io.c, now
it's generated at configure time in lib/config.h).

On top of that, configuring kvm-unit-tests to run under kvmtool will also
set CONFIG_ERRATA_FORCE to 1. At the time when kvmtool support was added,
kvmtool didn't have support for running a test with an initrd from which to
extract erratas. This has been recently fixed in kvmtool, now it can run a
test with an initrd.

Thanks,
Alex

> 
> [*] https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Specifying-Target-Triplets.html
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[Alexandru Elisei]

Hi,

On Thu, Feb 10, 2022 at 05:45:47PM +0000, Alexandru Elisei wrote:
> Hi,
> 
> On Thu, Feb 10, 2022 at 05:25:46PM +0000, Sean Christopherson wrote:
> > On Thu, Feb 10, 2022, Alexandru Elisei wrote:
> > > I renamed --target-efi to --efi-payload in the last patch because I felt it
> > > looked rather confusing to do ./configure --target=qemu --target-efi when
> > > configuring the tests. If the rename is not acceptable, I can think of a
> > > few other options:
> > 
> > I find --target-efi to be odd irrespective of this new conflict.  A simple --efi
> > seems like it would be sufficient.

I missed this bit in my earlier reply, I like --efi better. I would also like to
hear the opinion of the people who added EFI support before reworking the patch.

> > 
> > > 1. Rename --target to --vmm. That was actually the original name for the
> > > option, but I changed it because I thought --target was more generic and
> > > that --target=efi would be the way going forward to compile kvm-unit-tests
> > > to run as an EFI payload. I realize now that separating the VMM from
> > > compiling kvm-unit-tests to run as an EFI payload is better, as there can
> > > be multiple VMMs that can run UEFI in a VM. Not many people use kvmtool as
> > > a test runner, so I think the impact on users should be minimal.
> > 
> > Again irrespective of --target-efi, I think --target for the VMM is a potentially
> > confusing name.  Target Triplet[*] and --target have specific meaning for the
> > compiler, usurping that for something similar but slightly different is odd.
> 
> Wouldn't that mean that --target-efi is equally confusing? Do you have
> suggestions for other names?

Thanks,
Alex
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[Zixuan Wang]

On Thu, Feb 10, 2022 at 11:05 AM Alexandru Elisei
<alexandru.elisei@arm.com> wrote:
>
> Hi,
>
> On Thu, Feb 10, 2022 at 05:25:46PM +0000, Sean Christopherson wrote:
> > On Thu, Feb 10, 2022, Alexandru Elisei wrote:
> > > I renamed --target-efi to --efi-payload in the last patch because I felt it
> > > looked rather confusing to do ./configure --target=qemu --target-efi when
> > > configuring the tests. If the rename is not acceptable, I can think of a
> > > few other options:
> >
> > I find --target-efi to be odd irrespective of this new conflict.  A simple --efi
> > seems like it would be sufficient.
> >
> > > 1. Rename --target to --vmm. That was actually the original name for the
> > > option, but I changed it because I thought --target was more generic and
> > > that --target=efi would be the way going forward to compile kvm-unit-tests
> > > to run as an EFI payload. I realize now that separating the VMM from
> > > compiling kvm-unit-tests to run as an EFI payload is better, as there can
> > > be multiple VMMs that can run UEFI in a VM. Not many people use kvmtool as
> > > a test runner, so I think the impact on users should be minimal.
> >
> > Again irrespective of --target-efi, I think --target for the VMM is a potentially
> > confusing name.  Target Triplet[*] and --target have specific meaning for the
> > compiler, usurping that for something similar but slightly different is odd.
>
> Wouldn't that mean that --target-efi is equally confusing? Do you have
> suggestions for other names?

How about --config-efi for configure, and CONFIG_EFI for source code?
I thought about this name when I was developing the initial patch, and
Varad also proposed similar names in his initial patch series [1]:
--efi and CONFIG_EFI.

[1] https://lore.kernel.org/kvm/20210819113400.26516-1-varad.gautam@suse.com/
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[Sean Christopherson]

On Thu, Feb 10, 2022, Zixuan Wang wrote:
> On Thu, Feb 10, 2022 at 11:05 AM Alexandru Elisei
> <alexandru.elisei@arm.com> wrote:
> >
> > Hi,
> >
> > On Thu, Feb 10, 2022 at 05:25:46PM +0000, Sean Christopherson wrote:
> > > On Thu, Feb 10, 2022, Alexandru Elisei wrote:
> > > > I renamed --target-efi to --efi-payload in the last patch because I felt it
> > > > looked rather confusing to do ./configure --target=qemu --target-efi when
> > > > configuring the tests. If the rename is not acceptable, I can think of a
> > > > few other options:
> > >
> > > I find --target-efi to be odd irrespective of this new conflict.  A simple --efi
> > > seems like it would be sufficient.
> > >
> > > > 1. Rename --target to --vmm. That was actually the original name for the
> > > > option, but I changed it because I thought --target was more generic and
> > > > that --target=efi would be the way going forward to compile kvm-unit-tests
> > > > to run as an EFI payload. I realize now that separating the VMM from
> > > > compiling kvm-unit-tests to run as an EFI payload is better, as there can
> > > > be multiple VMMs that can run UEFI in a VM. Not many people use kvmtool as
> > > > a test runner, so I think the impact on users should be minimal.
> > >
> > > Again irrespective of --target-efi, I think --target for the VMM is a potentially
> > > confusing name.  Target Triplet[*] and --target have specific meaning for the
> > > compiler, usurping that for something similar but slightly different is odd.
> >
> > Wouldn't that mean that --target-efi is equally confusing? Do you have
> > suggestions for other names?
> 
> How about --config-efi for configure, and CONFIG_EFI for source code?
> I thought about this name when I was developing the initial patch, and
> Varad also proposed similar names in his initial patch series [1]:
> --efi and CONFIG_EFI.

I don't mind CONFIG_EFI for the source, that provides a nice hint that it's a
configure option and is familiar for kernel developers.  But for the actually
option, why require more typing?  I really don't see any benefit of --config-efi
over --efi.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[Zixuan Wang]

On Thu, Feb 10, 2022 at 11:36 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Feb 10, 2022, Zixuan Wang wrote:
> > On Thu, Feb 10, 2022 at 11:05 AM Alexandru Elisei
> > <alexandru.elisei@arm.com> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, Feb 10, 2022 at 05:25:46PM +0000, Sean Christopherson wrote:
> > > > On Thu, Feb 10, 2022, Alexandru Elisei wrote:
> > > > > I renamed --target-efi to --efi-payload in the last patch because I felt it
> > > > > looked rather confusing to do ./configure --target=qemu --target-efi when
> > > > > configuring the tests. If the rename is not acceptable, I can think of a
> > > > > few other options:
> > > >
> > > > I find --target-efi to be odd irrespective of this new conflict.  A simple --efi
> > > > seems like it would be sufficient.
> > > >
> > > > > 1. Rename --target to --vmm. That was actually the original name for the
> > > > > option, but I changed it because I thought --target was more generic and
> > > > > that --target=efi would be the way going forward to compile kvm-unit-tests
> > > > > to run as an EFI payload. I realize now that separating the VMM from
> > > > > compiling kvm-unit-tests to run as an EFI payload is better, as there can
> > > > > be multiple VMMs that can run UEFI in a VM. Not many people use kvmtool as
> > > > > a test runner, so I think the impact on users should be minimal.
> > > >
> > > > Again irrespective of --target-efi, I think --target for the VMM is a potentially
> > > > confusing name.  Target Triplet[*] and --target have specific meaning for the
> > > > compiler, usurping that for something similar but slightly different is odd.
> > >
> > > Wouldn't that mean that --target-efi is equally confusing? Do you have
> > > suggestions for other names?
> >
> > How about --config-efi for configure, and CONFIG_EFI for source code?
> > I thought about this name when I was developing the initial patch, and
> > Varad also proposed similar names in his initial patch series [1]:
> > --efi and CONFIG_EFI.
>
> I don't mind CONFIG_EFI for the source, that provides a nice hint that it's a
> configure option and is familiar for kernel developers.  But for the actually
> option, why require more typing?  I really don't see any benefit of --config-efi
> over --efi.

I agree, --efi looks better than --target-efi or --config-efi.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[Andrew Jones]

On Thu, Feb 10, 2022 at 11:48:03AM -0800, Zixuan Wang wrote:
> On Thu, Feb 10, 2022 at 11:36 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Thu, Feb 10, 2022, Zixuan Wang wrote:
> > > On Thu, Feb 10, 2022 at 11:05 AM Alexandru Elisei
> > > <alexandru.elisei@arm.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Thu, Feb 10, 2022 at 05:25:46PM +0000, Sean Christopherson wrote:
> > > > > On Thu, Feb 10, 2022, Alexandru Elisei wrote:
> > > > > > I renamed --target-efi to --efi-payload in the last patch because I felt it
> > > > > > looked rather confusing to do ./configure --target=qemu --target-efi when
> > > > > > configuring the tests. If the rename is not acceptable, I can think of a
> > > > > > few other options:
> > > > >
> > > > > I find --target-efi to be odd irrespective of this new conflict.  A simple --efi
> > > > > seems like it would be sufficient.
> > > > >
> > > > > > 1. Rename --target to --vmm. That was actually the original name for the
> > > > > > option, but I changed it because I thought --target was more generic and
> > > > > > that --target=efi would be the way going forward to compile kvm-unit-tests
> > > > > > to run as an EFI payload. I realize now that separating the VMM from
> > > > > > compiling kvm-unit-tests to run as an EFI payload is better, as there can
> > > > > > be multiple VMMs that can run UEFI in a VM. Not many people use kvmtool as
> > > > > > a test runner, so I think the impact on users should be minimal.
> > > > >
> > > > > Again irrespective of --target-efi, I think --target for the VMM is a potentially
> > > > > confusing name.  Target Triplet[*] and --target have specific meaning for the
> > > > > compiler, usurping that for something similar but slightly different is odd.
> > > >
> > > > Wouldn't that mean that --target-efi is equally confusing? Do you have
> > > > suggestions for other names?
> > >
> > > How about --config-efi for configure, and CONFIG_EFI for source code?
> > > I thought about this name when I was developing the initial patch, and
> > > Varad also proposed similar names in his initial patch series [1]:
> > > --efi and CONFIG_EFI.
> >
> > I don't mind CONFIG_EFI for the source, that provides a nice hint that it's a
> > configure option and is familiar for kernel developers.  But for the actually
> > option, why require more typing?  I really don't see any benefit of --config-efi
> > over --efi.
> 
> I agree, --efi looks better than --target-efi or --config-efi.
>

Works for me.

Thanks,
drew 

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[Thomas Huth]

On 10/02/2022 20.48, Zixuan Wang wrote:
> On Thu, Feb 10, 2022 at 11:36 AM Sean Christopherson <seanjc@google.com> wrote:
>>
>> On Thu, Feb 10, 2022, Zixuan Wang wrote:
>>> On Thu, Feb 10, 2022 at 11:05 AM Alexandru Elisei
>>> <alexandru.elisei@arm.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Thu, Feb 10, 2022 at 05:25:46PM +0000, Sean Christopherson wrote:
>>>>> On Thu, Feb 10, 2022, Alexandru Elisei wrote:
>>>>>> I renamed --target-efi to --efi-payload in the last patch because I felt it
>>>>>> looked rather confusing to do ./configure --target=qemu --target-efi when
>>>>>> configuring the tests. If the rename is not acceptable, I can think of a
>>>>>> few other options:
>>>>>
>>>>> I find --target-efi to be odd irrespective of this new conflict.  A simple --efi
>>>>> seems like it would be sufficient.
>>>>>
>>>>>> 1. Rename --target to --vmm. That was actually the original name for the
>>>>>> option, but I changed it because I thought --target was more generic and
>>>>>> that --target=efi would be the way going forward to compile kvm-unit-tests
>>>>>> to run as an EFI payload. I realize now that separating the VMM from
>>>>>> compiling kvm-unit-tests to run as an EFI payload is better, as there can
>>>>>> be multiple VMMs that can run UEFI in a VM. Not many people use kvmtool as
>>>>>> a test runner, so I think the impact on users should be minimal.
>>>>>
>>>>> Again irrespective of --target-efi, I think --target for the VMM is a potentially
>>>>> confusing name.  Target Triplet[*] and --target have specific meaning for the
>>>>> compiler, usurping that for something similar but slightly different is odd.
>>>>
>>>> Wouldn't that mean that --target-efi is equally confusing? Do you have
>>>> suggestions for other names?
>>>
>>> How about --config-efi for configure, and CONFIG_EFI for source code?
>>> I thought about this name when I was developing the initial patch, and
>>> Varad also proposed similar names in his initial patch series [1]:
>>> --efi and CONFIG_EFI.
>>
>> I don't mind CONFIG_EFI for the source, that provides a nice hint that it's a
>> configure option and is familiar for kernel developers.  But for the actually
>> option, why require more typing?  I really don't see any benefit of --config-efi
>> over --efi.
> 
> I agree, --efi looks better than --target-efi or --config-efi.

<bikeshedding>
Or maybe --enable-efi ... since configure scripts normally take 
"--enable-..." or "--disable-..." parameters for stuff like this?
</bikeshedding>

  Thomas

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[Sean Christopherson]

On Fri, Feb 11, 2022, Thomas Huth wrote:
> On 10/02/2022 20.48, Zixuan Wang wrote:
> > On Thu, Feb 10, 2022 at 11:36 AM Sean Christopherson <seanjc@google.com> wrote:
> > > 
> > > On Thu, Feb 10, 2022, Zixuan Wang wrote:
> > > > On Thu, Feb 10, 2022 at 11:05 AM Alexandru Elisei
> > > > <alexandru.elisei@arm.com> wrote:
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > On Thu, Feb 10, 2022 at 05:25:46PM +0000, Sean Christopherson wrote:
> > > > > > On Thu, Feb 10, 2022, Alexandru Elisei wrote:
> > > > > > > I renamed --target-efi to --efi-payload in the last patch because I felt it
> > > > > > > looked rather confusing to do ./configure --target=qemu --target-efi when
> > > > > > > configuring the tests. If the rename is not acceptable, I can think of a
> > > > > > > few other options:
> > > > > > 
> > > > > > I find --target-efi to be odd irrespective of this new conflict.  A simple --efi
> > > > > > seems like it would be sufficient.
> > > > > > 
> > > > > > > 1. Rename --target to --vmm. That was actually the original name for the
> > > > > > > option, but I changed it because I thought --target was more generic and
> > > > > > > that --target=efi would be the way going forward to compile kvm-unit-tests
> > > > > > > to run as an EFI payload. I realize now that separating the VMM from
> > > > > > > compiling kvm-unit-tests to run as an EFI payload is better, as there can
> > > > > > > be multiple VMMs that can run UEFI in a VM. Not many people use kvmtool as
> > > > > > > a test runner, so I think the impact on users should be minimal.
> > > > > > 
> > > > > > Again irrespective of --target-efi, I think --target for the VMM is a potentially
> > > > > > confusing name.  Target Triplet[*] and --target have specific meaning for the
> > > > > > compiler, usurping that for something similar but slightly different is odd.
> > > > > 
> > > > > Wouldn't that mean that --target-efi is equally confusing? Do you have
> > > > > suggestions for other names?
> > > > 
> > > > How about --config-efi for configure, and CONFIG_EFI for source code?
> > > > I thought about this name when I was developing the initial patch, and
> > > > Varad also proposed similar names in his initial patch series [1]:
> > > > --efi and CONFIG_EFI.
> > > 
> > > I don't mind CONFIG_EFI for the source, that provides a nice hint that it's a
> > > configure option and is familiar for kernel developers.  But for the actually
> > > option, why require more typing?  I really don't see any benefit of --config-efi
> > > over --efi.
> > 
> > I agree, --efi looks better than --target-efi or --config-efi.
> 
> <bikeshedding>
> Or maybe --enable-efi ... since configure scripts normally take
> "--enable-..." or "--disable-..." parameters for stuff like this?
> </bikeshedding>

I don't hate it :-)  It'll also future-proof things if we ever make UEFI the
default for x86.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [kvm-unit-tests PATCH 0/4] configure changes and rename --target-efi

[Alexandru Elisei]

Hi,

On Fri, Feb 11, 2022 at 04:19:55PM +0000, Sean Christopherson wrote:
> On Fri, Feb 11, 2022, Thomas Huth wrote:
> > On 10/02/2022 20.48, Zixuan Wang wrote:
> > > On Thu, Feb 10, 2022 at 11:36 AM Sean Christopherson <seanjc@google.com> wrote:
> > > > 
> > > > On Thu, Feb 10, 2022, Zixuan Wang wrote:
> > > > > On Thu, Feb 10, 2022 at 11:05 AM Alexandru Elisei
> > > > > <alexandru.elisei@arm.com> wrote:
> > > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > On Thu, Feb 10, 2022 at 05:25:46PM +0000, Sean Christopherson wrote:
> > > > > > > On Thu, Feb 10, 2022, Alexandru Elisei wrote:
> > > > > > > > I renamed --target-efi to --efi-payload in the last patch because I felt it
> > > > > > > > looked rather confusing to do ./configure --target=qemu --target-efi when
> > > > > > > > configuring the tests. If the rename is not acceptable, I can think of a
> > > > > > > > few other options:
> > > > > > > 
> > > > > > > I find --target-efi to be odd irrespective of this new conflict.  A simple --efi
> > > > > > > seems like it would be sufficient.
> > > > > > > 
> > > > > > > > 1. Rename --target to --vmm. That was actually the original name for the
> > > > > > > > option, but I changed it because I thought --target was more generic and
> > > > > > > > that --target=efi would be the way going forward to compile kvm-unit-tests
> > > > > > > > to run as an EFI payload. I realize now that separating the VMM from
> > > > > > > > compiling kvm-unit-tests to run as an EFI payload is better, as there can
> > > > > > > > be multiple VMMs that can run UEFI in a VM. Not many people use kvmtool as
> > > > > > > > a test runner, so I think the impact on users should be minimal.
> > > > > > > 
> > > > > > > Again irrespective of --target-efi, I think --target for the VMM is a potentially
> > > > > > > confusing name.  Target Triplet[*] and --target have specific meaning for the
> > > > > > > compiler, usurping that for something similar but slightly different is odd.
> > > > > > 
> > > > > > Wouldn't that mean that --target-efi is equally confusing? Do you have
> > > > > > suggestions for other names?
> > > > > 
> > > > > How about --config-efi for configure, and CONFIG_EFI for source code?
> > > > > I thought about this name when I was developing the initial patch, and
> > > > > Varad also proposed similar names in his initial patch series [1]:
> > > > > --efi and CONFIG_EFI.
> > > > 
> > > > I don't mind CONFIG_EFI for the source, that provides a nice hint that it's a
> > > > configure option and is familiar for kernel developers.  But for the actually
> > > > option, why require more typing?  I really don't see any benefit of --config-efi
> > > > over --efi.
> > > 
> > > I agree, --efi looks better than --target-efi or --config-efi.
> > 
> > <bikeshedding>
> > Or maybe --enable-efi ... since configure scripts normally take
> > "--enable-..." or "--disable-..." parameters for stuff like this?
> > </bikeshedding>
> 
> I don't hate it :-)  It'll also future-proof things if we ever make UEFI the
> default for x86.

Thank you all for the feedback.

I'll respin the series and rename --target-efi to --enable-efi.

Thanks,
Alex
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm