| Message ID | 20211031055634.894263-1-zxwang42@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | x86_64 UEFI set up process refactor and scripts fixes | expand |
On 31/10/21 06:56, Zixuan Wang wrote: > Hello, > > This patch series refactors the x86_64 UEFI set up process and fixes the > `run-tests.sh` script to run under UEFI. The patches are organized as > three parts. > > The first part (patches 1-2) refactors the x86_64 UEFI set up process. > The previous UEFI setup calls arch-specific setup functions twice and > generates arch-specific data structure. As Andrew suggested [1], we > refactor this process to make only one call to the arch-specific > function and generate arch-neutral data structures. This simplifies the > set up process and makes it easier to develop UEFI support for other > architectures. > > The second part (patch 3) converts several x86 test cases to > Position-Independent Code (PIC) to run under UEFI. This patch is ported > from the initial UEFI support patchset [2] with fixes to the 32-bit > compilation. > > The third part (patches 4-7) fixes the UEFI runner scripts. Patch 4 sets > UEFI OVMF image as readonly. Patch 5 fixes test cases' return code under > UEFI, enabling Patch 6-7 to fix the `run-tests.sh` script under UEFI. > > This patch set is based on the `uefi` branch. Thank you, for patches 1-6 I have squashed the patches when applicable (1, 4, 5, 6) and queued the others (2 and 3). I did not queue patch 7 yet, it seems okay but I want to understand better the changes it needs in the harness and what is missing. I'll take a look during the week. Paolo > Best regards, > Zixuan Wang and Marc Orr > > [1] https://lore.kernel.org/kvm/20211005060549.clar5nakynz2zecl@gator.home/ > [2] https://lore.kernel.org/kvm/20211004204931.1537823-1-zxwang42@gmail.com/ > > Marc Orr (2): > scripts: Generalize EFI check > x86 UEFI: Make run_tests.sh (mostly) work under UEFI > > Zixuan Wang (5): > x86 UEFI: Remove mixed_mode > x86 UEFI: Refactor set up process > x86 UEFI: Convert x86 test cases to PIC > x86 UEFI: Set UEFI OVMF as readonly > x86 UEFI: Exit QEMU with return code > > lib/efi.c | 54 ++++++-- > lib/efi.h | 19 ++- > lib/linux/efi.h | 317 ++++++++++++++----------------------------- > lib/x86/acpi.c | 36 +++-- > lib/x86/acpi.h | 5 +- > lib/x86/asm/setup.h | 16 +-- > lib/x86/setup.c | 153 ++++++++++----------- > lib/x86/usermode.c | 3 +- > scripts/common.bash | 9 +- > scripts/runtime.bash | 15 +- > x86/Makefile.common | 10 +- > x86/Makefile.x86_64 | 7 +- > x86/access.c | 9 +- > x86/cet.c | 8 +- > x86/efi/run | 27 +++- > x86/emulator.c | 5 +- > x86/eventinj.c | 8 ++ > x86/run | 6 +- > x86/smap.c | 13 +- > x86/umip.c | 26 +++- > 20 files changed, 360 insertions(+), 386 deletions(-) >
On Sun, Oct 31, 2021 at 12:28 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 31/10/21 06:56, Zixuan Wang wrote: > > Hello, > > > > This patch series refactors the x86_64 UEFI set up process and fixes the > > `run-tests.sh` script to run under UEFI. The patches are organized as > > three parts. > > > > The first part (patches 1-2) refactors the x86_64 UEFI set up process. > > The previous UEFI setup calls arch-specific setup functions twice and > > generates arch-specific data structure. As Andrew suggested [1], we > > refactor this process to make only one call to the arch-specific > > function and generate arch-neutral data structures. This simplifies the > > set up process and makes it easier to develop UEFI support for other > > architectures. > > > > The second part (patch 3) converts several x86 test cases to > > Position-Independent Code (PIC) to run under UEFI. This patch is ported > > from the initial UEFI support patchset [2] with fixes to the 32-bit > > compilation. > > > > The third part (patches 4-7) fixes the UEFI runner scripts. Patch 4 sets > > UEFI OVMF image as readonly. Patch 5 fixes test cases' return code under > > UEFI, enabling Patch 6-7 to fix the `run-tests.sh` script under UEFI. > > > > This patch set is based on the `uefi` branch. > > Thank you, for patches 1-6 I have squashed the patches when applicable > (1, 4, 5, 6) and queued the others (2 and 3). > > I did not queue patch 7 yet, it seems okay but I want to understand > better the changes it needs in the harness and what is missing. I'll > take a look during the week. SGTM, thank you! Zixuan and I discussed a few things that are missing: 1. Test cases that take the `-append` arg are currently marked `SKIP`. Two issues need to be resolved here. First, we're not using QEMU's `-kernel` flag for EFI test cases [1]. And the `-append` flag does not work without the `-kernel` flag. I don't understand the details on why we don't use the `-kernel` flag myself. Maybe Zixuan can elaborate. Second, assuming we fix the first issue, then we need to enlighten the KVM-Unit-Tests under UEFI to parse kernel command line arguments and pass them down to the test cases via `argv`. Zixuan pointed out to me that there is some prior work from Drew [2] that we should be able to follow to make this work. So I'm hoping that Zixuan and I can work together on solving these issues to get the argument passing working next. 2. We need a way to annotate test cases in `x86/unittests.cfg` as known to work under SEV. I'm thinking of doing this via new (very broad) test groups in `unittests.cfg`. I _think_ SEV is the primary scenario we care about. However, folks may care about running the test cases under UEFI outside of SEV. For example, last time I checked, emulator runs OK under UEFI minus SEV-ES but fails under SEV-ES. And similarly, while most test cases work under UEFI minus SEV, there are a few that do mis-behave -- and it probably makes sense to document this (e.g., via annotations in `unittests.cfg`). Also, there are many variations of SEV (SEV, SEV-ES, SEV-SNP)... And hopefully some of this will eventually be applicable to TDX as well. So many testgroups is not a good solution. I'm not sure. 3. Multi-CPU needs to be made to work under UEFI. For now, patch #7 forces all EFI test cases to run with 1 vCPU. I chatted with Brijesh, and he mentioned that Varad would like to work on this. However, if anything here changes, please let me know, because we can work on this as well. But for now, I'm not planning to work on it so we can avoid duplicating work. 4. UEFI runs a lot slower than SEABIOS. It doesn't help that the test harness launches QEMU more than once for each test case (i.e., it runs the `_NO_FILE_4Uhere_` scenario to check QEMU arguments). I'm not sure how much of an issue this is in practice. Depending on the answer, I know Zixuan had some ideas on how to speed this up in the current test harness. Or maybe we can explore an alternative to the `_NO_FILE_4Uhere_` approach instead. Zixuan: Please add/correct anything as needed! [1] https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/blob/uefi/x86/run#L42-44 [2] https://github.com/rhdrjones/kvm-unit-tests/blob/target-efi/scripts/mkefi.sh Thanks, Marc
On Sun, Oct 31, 2021 at 9:14 AM Marc Orr <marcorr@google.com> wrote: > > On Sun, Oct 31, 2021 at 12:28 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 31/10/21 06:56, Zixuan Wang wrote: > > > Hello, > > > > > > This patch series refactors the x86_64 UEFI set up process and fixes the > > > `run-tests.sh` script to run under UEFI. The patches are organized as > > > three parts. > > > > > > The first part (patches 1-2) refactors the x86_64 UEFI set up process. > > > The previous UEFI setup calls arch-specific setup functions twice and > > > generates arch-specific data structure. As Andrew suggested [1], we > > > refactor this process to make only one call to the arch-specific > > > function and generate arch-neutral data structures. This simplifies the > > > set up process and makes it easier to develop UEFI support for other > > > architectures. > > > > > > The second part (patch 3) converts several x86 test cases to > > > Position-Independent Code (PIC) to run under UEFI. This patch is ported > > > from the initial UEFI support patchset [2] with fixes to the 32-bit > > > compilation. > > > > > > The third part (patches 4-7) fixes the UEFI runner scripts. Patch 4 sets > > > UEFI OVMF image as readonly. Patch 5 fixes test cases' return code under > > > UEFI, enabling Patch 6-7 to fix the `run-tests.sh` script under UEFI. > > > > > > This patch set is based on the `uefi` branch. > > > > Thank you, for patches 1-6 I have squashed the patches when applicable > > (1, 4, 5, 6) and queued the others (2 and 3). > > > > I did not queue patch 7 yet, it seems okay but I want to understand > > better the changes it needs in the harness and what is missing. I'll > > take a look during the week. > > SGTM, thank you! Zixuan and I discussed a few things that are missing: > > 1. Test cases that take the `-append` arg are currently marked `SKIP`. > Two issues need to be resolved here. First, we're not using QEMU's > `-kernel` flag for EFI test cases [1]. And the `-append` flag does not > work without the `-kernel` flag. I don't understand the details on why > we don't use the `-kernel` flag myself. Maybe Zixuan can elaborate. > Second, assuming we fix the first issue, then we need to enlighten the > KVM-Unit-Tests under UEFI to parse kernel command line arguments and > pass them down to the test cases via `argv`. Zixuan pointed out to me > that there is some prior work from Drew [2] that we should be able to > follow to make this work. So I'm hoping that Zixuan and I can work > together on solving these issues to get the argument passing working > next. Thank you for the detailed summary! Current kvm-unit-tests pass an EFI binary as part of a disk image, instead of using the `-kernel` argument. I just tested the `-kernel` argument and it seems to work with EFI binaries, and more importantly, it's really fast (bypassing the default 5-second user input waiting). I will update the `x86/efi/run` to use `-kernel` argument to pass the EFI binaries. Since `-kernel` is working, I can start to investigate how to use `-append` to pass arguments. If that doesn't work well, an alternative approach could be: 1. (host) create a file `args.txt` in the disk image, which contains all the arguments needed 2. (guest) call UEFI filesystem interface to read this `args.txt` from the disk image, parse it and pass the arguments to `main()` > 2. We need a way to annotate test cases in `x86/unittests.cfg` as > known to work under SEV. I'm thinking of doing this via new (very > broad) test groups in `unittests.cfg`. I _think_ SEV is the primary > scenario we care about. However, folks may care about running the test > cases under UEFI outside of SEV. For example, last time I checked, > emulator runs OK under UEFI minus SEV-ES but fails under SEV-ES. And > similarly, while most test cases work under UEFI minus SEV, there are > a few that do mis-behave -- and it probably makes sense to document > this (e.g., via annotations in `unittests.cfg`). Also, there are many > variations of SEV (SEV, SEV-ES, SEV-SNP)... And hopefully some of this > will eventually be applicable to TDX as well. So many testgroups is > not a good solution. I'm not sure. Adding an `efi` group seems helpful. E.g., the current `x86/smap.c` does not work under UEFI; but the `run-tests.sh` still tries to run this test case, even if this test case is not compiled. > 3. Multi-CPU needs to be made to work under UEFI. For now, patch #7 > forces all EFI test cases to run with 1 vCPU. I chatted with Brijesh, > and he mentioned that Varad would like to work on this. However, if > anything here changes, please let me know, because we can work on this > as well. But for now, I'm not planning to work on it so we can avoid > duplicating work. > 4. UEFI runs a lot slower than SEABIOS. It doesn't help that the test > harness launches QEMU more than once for each test case (i.e., it runs > the `_NO_FILE_4Uhere_` scenario to check QEMU arguments). I'm not sure > how much of an issue this is in practice. Depending on the answer, I > know Zixuan had some ideas on how to speed this up in the current test > harness. Or maybe we can explore an alternative to the > `_NO_FILE_4Uhere_` approach instead. As the `-kernel` argument now works with the EFI binaries and is significantly faster, this should not be an issue anymore. We just need to update the runner scripts to use `-kernel` argument. > Zixuan: Please add/correct anything as needed! > > [1] https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/blob/uefi/x86/run#L42-44 > [2] https://github.com/rhdrjones/kvm-unit-tests/blob/target-efi/scripts/mkefi.sh > > Thanks, > Marc Best regards, Zixuan
On Sun, Oct 31, 2021 at 02:54:00PM -0700, Zixuan Wang wrote: > On Sun, Oct 31, 2021 at 9:14 AM Marc Orr <marcorr@google.com> wrote: > > > > On Sun, Oct 31, 2021 at 12:28 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > > > On 31/10/21 06:56, Zixuan Wang wrote: > > > > Hello, > > > > > > > > This patch series refactors the x86_64 UEFI set up process and fixes the > > > > `run-tests.sh` script to run under UEFI. The patches are organized as > > > > three parts. > > > > > > > > The first part (patches 1-2) refactors the x86_64 UEFI set up process. > > > > The previous UEFI setup calls arch-specific setup functions twice and > > > > generates arch-specific data structure. As Andrew suggested [1], we > > > > refactor this process to make only one call to the arch-specific > > > > function and generate arch-neutral data structures. This simplifies the > > > > set up process and makes it easier to develop UEFI support for other > > > > architectures. > > > > > > > > The second part (patch 3) converts several x86 test cases to > > > > Position-Independent Code (PIC) to run under UEFI. This patch is ported > > > > from the initial UEFI support patchset [2] with fixes to the 32-bit > > > > compilation. > > > > > > > > The third part (patches 4-7) fixes the UEFI runner scripts. Patch 4 sets > > > > UEFI OVMF image as readonly. Patch 5 fixes test cases' return code under > > > > UEFI, enabling Patch 6-7 to fix the `run-tests.sh` script under UEFI. > > > > > > > > This patch set is based on the `uefi` branch. > > > > > > Thank you, for patches 1-6 I have squashed the patches when applicable > > > (1, 4, 5, 6) and queued the others (2 and 3). > > > > > > I did not queue patch 7 yet, it seems okay but I want to understand > > > better the changes it needs in the harness and what is missing. I'll > > > take a look during the week. > > > > SGTM, thank you! Zixuan and I discussed a few things that are missing: > > > > 1. Test cases that take the `-append` arg are currently marked `SKIP`. > > Two issues need to be resolved here. First, we're not using QEMU's > > `-kernel` flag for EFI test cases [1]. And the `-append` flag does not > > work without the `-kernel` flag. I don't understand the details on why > > we don't use the `-kernel` flag myself. Maybe Zixuan can elaborate. > > Second, assuming we fix the first issue, then we need to enlighten the > > KVM-Unit-Tests under UEFI to parse kernel command line arguments and > > pass them down to the test cases via `argv`. Zixuan pointed out to me > > that there is some prior work from Drew [2] that we should be able to > > follow to make this work. So I'm hoping that Zixuan and I can work > > together on solving these issues to get the argument passing working > > next. > > Thank you for the detailed summary! > > Current kvm-unit-tests pass an EFI binary as part of a disk image, > instead of using the `-kernel` argument. > > I just tested the `-kernel` argument and it seems to work with EFI > binaries, and more importantly, it's really fast (bypassing the > default 5-second user input waiting). I will update the `x86/efi/run` > to use `-kernel` argument to pass the EFI binaries. > > Since `-kernel` is working, I can start to investigate how to use > `-append` to pass arguments. If that doesn't work well, an alternative > approach could be: > > 1. (host) create a file `args.txt` in the disk image, which contains > all the arguments needed > 2. (guest) call UEFI filesystem interface to read this `args.txt` from > the disk image, parse it and pass the arguments to `main()` > > > 2. We need a way to annotate test cases in `x86/unittests.cfg` as > > known to work under SEV. I'm thinking of doing this via new (very > > broad) test groups in `unittests.cfg`. I _think_ SEV is the primary > > scenario we care about. However, folks may care about running the test > > cases under UEFI outside of SEV. For example, last time I checked, > > emulator runs OK under UEFI minus SEV-ES but fails under SEV-ES. And > > similarly, while most test cases work under UEFI minus SEV, there are > > a few that do mis-behave -- and it probably makes sense to document > > this (e.g., via annotations in `unittests.cfg`). Also, there are many > > variations of SEV (SEV, SEV-ES, SEV-SNP)... And hopefully some of this > > will eventually be applicable to TDX as well. So many testgroups is > > not a good solution. I'm not sure. > > Adding an `efi` group seems helpful. E.g., the current `x86/smap.c` > does not work under UEFI; but the `run-tests.sh` still tries to run > this test case, even if this test case is not compiled. > > > 3. Multi-CPU needs to be made to work under UEFI. For now, patch #7 > > forces all EFI test cases to run with 1 vCPU. I chatted with Brijesh, > > and he mentioned that Varad would like to work on this. However, if > > anything here changes, please let me know, because we can work on this > > as well. But for now, I'm not planning to work on it so we can avoid > > duplicating work. > > 4. UEFI runs a lot slower than SEABIOS. It doesn't help that the test > > harness launches QEMU more than once for each test case (i.e., it runs > > the `_NO_FILE_4Uhere_` scenario to check QEMU arguments). I'm not sure > > how much of an issue this is in practice. Depending on the answer, I > > know Zixuan had some ideas on how to speed this up in the current test > > harness. Or maybe we can explore an alternative to the > > `_NO_FILE_4Uhere_` approach instead. > > As the `-kernel` argument now works with the EFI binaries and is > significantly faster, this should not be an issue anymore. We just > need to update the runner scripts to use `-kernel` argument. You can add an additional '-kernel' + EFI binary runner if you want, but the goal of being able to run kvm-unit-tests on bare-metal means we shouldn't be counting on QEMU/OVMF to do magic stuff with the kernel. We need to build disk images. Argument passing works with EFI apps, when implemented, so that's not a problem. I also created a script that uses the framework's for_each_unittest to generate an EFI script that allowed each test to be easily run with its arguments. Thanks, drew > > > Zixuan: Please add/correct anything as needed! > > > > [1] https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/blob/uefi/x86/run#L42-44 > > [2] https://github.com/rhdrjones/kvm-unit-tests/blob/target-efi/scripts/mkefi.sh > > > > Thanks, > > Marc > > Best regards, > Zixuan >
On Mon, Nov 1, 2021 at 12:11 AM Andrew Jones <drjones@redhat.com> wrote: > > On Sun, Oct 31, 2021 at 02:54:00PM -0700, Zixuan Wang wrote: > > On Sun, Oct 31, 2021 at 9:14 AM Marc Orr <marcorr@google.com> wrote: > > > > > > On Sun, Oct 31, 2021 at 12:28 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > > > > > On 31/10/21 06:56, Zixuan Wang wrote: > > > > > Hello, > > > > > > > > > > This patch series refactors the x86_64 UEFI set up process and fixes the > > > > > `run-tests.sh` script to run under UEFI. The patches are organized as > > > > > three parts. > > > > > > > > > > The first part (patches 1-2) refactors the x86_64 UEFI set up process. > > > > > The previous UEFI setup calls arch-specific setup functions twice and > > > > > generates arch-specific data structure. As Andrew suggested [1], we > > > > > refactor this process to make only one call to the arch-specific > > > > > function and generate arch-neutral data structures. This simplifies the > > > > > set up process and makes it easier to develop UEFI support for other > > > > > architectures. > > > > > > > > > > The second part (patch 3) converts several x86 test cases to > > > > > Position-Independent Code (PIC) to run under UEFI. This patch is ported > > > > > from the initial UEFI support patchset [2] with fixes to the 32-bit > > > > > compilation. > > > > > > > > > > The third part (patches 4-7) fixes the UEFI runner scripts. Patch 4 sets > > > > > UEFI OVMF image as readonly. Patch 5 fixes test cases' return code under > > > > > UEFI, enabling Patch 6-7 to fix the `run-tests.sh` script under UEFI. > > > > > > > > > > This patch set is based on the `uefi` branch. > > > > > > > > Thank you, for patches 1-6 I have squashed the patches when applicable > > > > (1, 4, 5, 6) and queued the others (2 and 3). > > > > > > > > I did not queue patch 7 yet, it seems okay but I want to understand > > > > better the changes it needs in the harness and what is missing. I'll > > > > take a look during the week. > > > > > > SGTM, thank you! Zixuan and I discussed a few things that are missing: > > > > > > 1. Test cases that take the `-append` arg are currently marked `SKIP`. > > > Two issues need to be resolved here. First, we're not using QEMU's > > > `-kernel` flag for EFI test cases [1]. And the `-append` flag does not > > > work without the `-kernel` flag. I don't understand the details on why > > > we don't use the `-kernel` flag myself. Maybe Zixuan can elaborate. > > > Second, assuming we fix the first issue, then we need to enlighten the > > > KVM-Unit-Tests under UEFI to parse kernel command line arguments and > > > pass them down to the test cases via `argv`. Zixuan pointed out to me > > > that there is some prior work from Drew [2] that we should be able to > > > follow to make this work. So I'm hoping that Zixuan and I can work > > > together on solving these issues to get the argument passing working > > > next. > > > > Thank you for the detailed summary! > > > > Current kvm-unit-tests pass an EFI binary as part of a disk image, > > instead of using the `-kernel` argument. > > > > I just tested the `-kernel` argument and it seems to work with EFI > > binaries, and more importantly, it's really fast (bypassing the > > default 5-second user input waiting). I will update the `x86/efi/run` > > to use `-kernel` argument to pass the EFI binaries. > > > > Since `-kernel` is working, I can start to investigate how to use > > `-append` to pass arguments. If that doesn't work well, an alternative > > approach could be: > > > > 1. (host) create a file `args.txt` in the disk image, which contains > > all the arguments needed > > 2. (guest) call UEFI filesystem interface to read this `args.txt` from > > the disk image, parse it and pass the arguments to `main()` > > > > > 2. We need a way to annotate test cases in `x86/unittests.cfg` as > > > known to work under SEV. I'm thinking of doing this via new (very > > > broad) test groups in `unittests.cfg`. I _think_ SEV is the primary > > > scenario we care about. However, folks may care about running the test > > > cases under UEFI outside of SEV. For example, last time I checked, > > > emulator runs OK under UEFI minus SEV-ES but fails under SEV-ES. And > > > similarly, while most test cases work under UEFI minus SEV, there are > > > a few that do mis-behave -- and it probably makes sense to document > > > this (e.g., via annotations in `unittests.cfg`). Also, there are many > > > variations of SEV (SEV, SEV-ES, SEV-SNP)... And hopefully some of this > > > will eventually be applicable to TDX as well. So many testgroups is > > > not a good solution. I'm not sure. > > > > Adding an `efi` group seems helpful. E.g., the current `x86/smap.c` > > does not work under UEFI; but the `run-tests.sh` still tries to run > > this test case, even if this test case is not compiled. > > > > > 3. Multi-CPU needs to be made to work under UEFI. For now, patch #7 > > > forces all EFI test cases to run with 1 vCPU. I chatted with Brijesh, > > > and he mentioned that Varad would like to work on this. However, if > > > anything here changes, please let me know, because we can work on this > > > as well. But for now, I'm not planning to work on it so we can avoid > > > duplicating work. > > > 4. UEFI runs a lot slower than SEABIOS. It doesn't help that the test > > > harness launches QEMU more than once for each test case (i.e., it runs > > > the `_NO_FILE_4Uhere_` scenario to check QEMU arguments). I'm not sure > > > how much of an issue this is in practice. Depending on the answer, I > > > know Zixuan had some ideas on how to speed this up in the current test > > > harness. Or maybe we can explore an alternative to the > > > `_NO_FILE_4Uhere_` approach instead. > > > > As the `-kernel` argument now works with the EFI binaries and is > > significantly faster, this should not be an issue anymore. We just > > need to update the runner scripts to use `-kernel` argument. > > You can add an additional '-kernel' + EFI binary runner if you want, but > the goal of being able to run kvm-unit-tests on bare-metal means we > shouldn't be counting on QEMU/OVMF to do magic stuff with the kernel. We > need to build disk images. Argument passing works with EFI apps, when > implemented, so that's not a problem. I also created a script that uses > the framework's for_each_unittest to generate an EFI script that allowed > each test to be easily run with its arguments. > > Thanks, > drew I see, I think an alternative approach is to rename test case binaries to UEFI default binary filename, which is EFI/BOOT/BOOTX64.EFI for x86_64. This should work just like the `-kernel` argument. I will explore this approach with the argument passing mechanisms. Best regards, Zixuan