| Message ID | 20220210150943.1280146-1-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | configure changes and rename --target-efi | expand |
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
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
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
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/
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.
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.
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
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
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.
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
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(-)