Re: [kvm-unit-tests PATCH v3 00/27] EFI and ACPI support for arm64

Nikos Nikoleris
2022-08-09T16:10:00Z

Hi,

On 09/08/2022 12:16, Alexandru Elisei wrote:
> Hi,
>
> Adding Sean and Zixuan, as they were involved in the initial x86 UEFI
> support.
>
> This version of the UEFI support for arm64 jumps to lib/efi.c::efi_main
> after performing the relocation. I'll post an abbreviated/simplified
> version of efi_main() for reference:
>
> efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> {
> /* Get image, cmdline and memory map parameters from UEFI */
>
> efi_exit_boot_services(handle, &efi_bootinfo.mem_map);
>
> /* Set up arch-specific resources */
> setup_efi(&efi_bootinfo);
>
> /* Run the test case */
> ret = main(__argc, __argv, __environ);
>
> /* Shutdown the guest VM */
> efi_exit(ret);
>
> /* Unreachable */
> return EFI_UNSUPPORTED;
> }
>
> Note that the assumption that efi_main() makes is that setup_efi() doesn't
> change the stack from the stack that the UEFI implementation allocated, in
> order for setup_efi() to be able to return to efi_main().
>
> arm64 requires explicit data cache maintenance to keep the contents of the
> caches in sync with memory when writing with MMU off/reading with MMU on
> and viceversa. More details of what is needed is why here [1] and here [2].
> These operations must also be performed for the stack because the stack is
> always used when running C code.
>
> What this means is that if arm64 wants to be able to run C code when the
> MMU is disabled and when it is enabled, then it must perform data cache
> operations for the stack memory. Which is impossible if the stack has been
> allocated by UEFI, as kvm-unit-tests has no way of knowing its size, as it
> isn't specified in the UEFI spec*. As a result, either efi_main needs to be
> changed such that setup_efi() never returns, or arm64 must implement its
> own version of efi_main() (or however it will end up being called).
>

I think it's possible to know the size of the stack. In 22/27 "arm64:
Use code from the gnu-efi when booting with EFI", we change the top of
the stack we start executing C code. Then at any point we can get the
bottom of the stack.

But I doubt cleaning the stack is sufficient. What about the .data
segment. Take for example, mem_regions. We populate them with the MMU
on. We then use them to create page tables. We will need to clean them
too. I won't be surprised if there is more data we would need to clean.

Thanks,

Nikos

> One way to get around this is never to run C code with the MMU off. That's
> relatively easy to do in the boot code, as the translation tables can be
> constructed with the MMU on, and then a fairly small assembly sequence is
> required to install them.
>
> But arm64 also has two mechanisms for disabling the MMU:
>
> 1. At compile time, the user can request a test to start with the MMU off,
> by setting the flag AUXINFO_MMU_OFF. So when booting as an UEFI app,
> kvm-unit-tests must disable the MMU.
>
> 2. A function called mmu_disable() which allows a test to explicitly
> disable the MMU.
>
> If we want to keep the UEFI allocated stack, then both mechanism must be
> forbidden when running under UEFI. I dislike this idea, because those two
> mechanisms allow kvm-unit-tests to run tests which otherwise wouldn't have
> been possible with a normal operating system, which, except for the early
> boot code, runs with the MMU enabled.
>
> Any thoughts or comments about this?
>
> *UEFI v2.8 states about the stack: "128 KiB or more of available stack
> space" (page 35), but EDK2 allocates 64KiB [3]. So without any firmware
> call to query the size of the stack, kvm-unit-tests cannot rely on it being
> a specific size.
>
> [1] https://lore.kernel.org/kvm/20220809091558.14379-19-alexandru.elisei@arm.com/
> [2] https://lore.kernel.org/kvm/20220809091558.14379-20-alexandru.elisei@arm.com/
> [3] https://github.com/tianocore/edk2/blob/master/ArmPlatformPkg/ArmPlatformPkg.dec#L71
>
> Thanks,
> Alex
>
> On Thu, Jun 30, 2022 at 11:02:57AM +0100, Nikos Nikoleris wrote:
>> Hello,
>>
>> This patch series adds initial support for building arm64 tests as EFI
>> tests and running them under QEMU. Much like x86_64 we import external
>> dependencies from gnu-efi and adapt them to work with types and other
>> assumptions from kvm-unit-tests. In addition, this series adds support
>> for enumerating parts of the system using ACPI.
>>
>> The first set of patches moves the existing ACPI code to the common
>> lib path. Then, it extends definitions and functions to allow for more
>> robust discovery of ACPI tables. In arm64, we add support for setting
>> up the PSCI conduit, discovering the UART, timers and cpus via
>> ACPI. The code retains existing behavior and gives priority to
>> discovery through DT when one has been provided.
>>
>> In the second set of patches, we add support for getting the command
>> line from the EFI shell. This is a requirement for many of the
>> existing arm64 tests.
>>
>> In the third set of patches, we import code from gnu-efi, make minor
>> changes and add an alternative setup sequence from arm64 systems that
>> boot through EFI. Finally, we add support in the build system and a
>> run script which is used to run an EFI app.
>>
>> After this set of patches one can build arm64 EFI tests:
>>
>> $> ./configure --enable-efi
>> $> make
>>
>> And use the run script to run an EFI tests:
>>
>> $> ./arm/efi/run ./arm/selftest.efi -smp 2 -m 256 -append "setup smp=2 mem=256"
>>
>> Or all tests:
>>
>> $> ./run_tests.sh
>>
>> There are a few items that this series does not address but they would
>> be useful to have:
>> - Support for booting the system from EL2. Currently, we assume that a
>> tests starts running at EL1. This the case when we run with EFI, it's
>> not always the case in hardware.
>> - Support for reading environment variables and populating __envp.
>> - Support for discovering the PCI subsystem using ACPI.
>> - Get rid of other assumptions (e.g., vmalloc area) that don't hold on real HW.
>>
>> git branch: https://github.com/relokin/kvm-unit-tests/pull/new/target-efi-upstream-v3-rebased
>>
>> v2: https://lore.kernel.org/kvm/20220506205605.359830-1-nikos.nikoleris@arm.com/
>>
>> Changes in v3:
>> - Addressed feedback from Drew, Alex and Ricardo. Many thanks for the reviews!
>> - Added support for discovering the GIC through ACPI
>> - Added a missing header file (<elf.h>)
>> - Added support for correctly parsing the outcome of tests (./run_tests)
>>
>> Thanks,
>>
>> Nikos
>>
>> Alexandru Elisei (1):
>> lib: arm: Print test exit status
>>
>> Andrew Jones (3):
>> arm/arm64: mmu_disable: Clean and invalidate before disabling
>> arm/arm64: Rename etext to _etext
>> arm64: Add a new type of memory type flag MR_F_RESERVED
>>
>> Nikos Nikoleris (23):
>> lib: Fix style for acpi.{c,h}
>> x86: Avoid references to fields of ACPI tables
>> lib: Ensure all struct definition for ACPI tables are packed
>> lib: Add support for the XSDT ACPI table
>> lib: Extend the definition of the ACPI table FADT
>> devicetree: Check if fdt is NULL before returning that a DT is
>> available
>> arm/arm64: Add support for setting up the PSCI conduit through ACPI
>> arm/arm64: Add support for discovering the UART through ACPI
>> arm/arm64: Add support for timer initialization through ACPI
>> arm/arm64: Add support for cpu initialization through ACPI
>> arm/arm64: Add support for gic initialization through ACPI
>> lib/printf: Support for precision modifier in printf
>> lib/printf: Add support for printing wide strings
>> lib/efi: Add support for getting the cmdline
>> lib: Avoid ms_abi for calls related to EFI on arm64
>> arm/arm64: Add a setup sequence for systems that boot through EFI
>> arm64: Copy code from GNU-EFI
>> arm64: Change GNU-EFI imported file to use defined types
>> arm64: Use code from the gnu-efi when booting with EFI
>> lib: Avoid external dependency in libelf
>> x86: Move x86_64-specific EFI CFLAGS to x86_64 Makefile
>> arm64: Add support for efi in Makefile
>> arm64: Add an efi/run script
>>
>> scripts/runtime.bash | 14 +-
>> arm/efi/run | 61 +++++++
>> arm/run | 14 +-
>> configure | 15 +-
>> Makefile | 4 -
>> arm/Makefile.arm | 6 +
>> arm/Makefile.arm64 | 18 +-
>> arm/Makefile.common | 48 +++--
>> x86/Makefile.x86_64 | 4 +
>> lib/linux/efi.h | 25 +++
>> lib/arm/asm/setup.h | 3 +
>> lib/arm/asm/timer.h | 2 +
>> lib/acpi.h | 348 ++++++++++++++++++++++++++++--------
>> lib/argv.h | 1 +
>> lib/elf.h | 57 ++++++
>> lib/libcflat.h | 1 +
>> lib/acpi.c | 129 ++++++++-----
>> lib/argv.c | 2 +-
>> lib/arm/gic.c | 127 ++++++++++++-
>> lib/arm/io.c | 29 ++-
>> lib/arm/mmu.c | 4 +
>> lib/arm/psci.c | 25 ++-
>> lib/arm/setup.c | 247 ++++++++++++++++++++-----
>> lib/arm/timer.c | 79 ++++++++
>> lib/devicetree.c | 2 +-
>> lib/efi.c | 102 +++++++++++
>> lib/printf.c | 194 ++++++++++++++++++--
>> arm/efi/elf_aarch64_efi.lds | 63 +++++++
>> arm/flat.lds | 2 +-
>> arm/cstart.S | 29 ++-
>> arm/cstart64.S | 28 ++-
>> arm/efi/crt0-efi-aarch64.S | 143 +++++++++++++++
>> arm/dummy.c | 4 +
>> arm/efi/reloc_aarch64.c | 93 ++++++++++
>> arm/micro-bench.c | 4 +-
>> arm/timer.c | 10 +-
>> x86/s3.c | 19 +-
>> x86/vmexit.c | 2 +-
>> 38 files changed, 1700 insertions(+), 258 deletions(-)
>> create mode 100755 arm/efi/run
>> create mode 100644 lib/elf.h
>> create mode 100644 lib/arm/timer.c
>> create mode 100644 arm/efi/elf_aarch64_efi.lds
>> create mode 100644 arm/efi/crt0-efi-aarch64.S
>> create mode 100644 arm/dummy.c
>> create mode 100644 arm/efi/reloc_aarch64.c
>>
>> --
>> 2.25.1
>>