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

[Alexandru Elisei <alexandru.elisei@arm.com>]

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).

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
>