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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 29/04/2023 17:21, Andrew Jones wrote:
On Sat, Apr 29, 2023 at 06:18:25PM +0200, Andrew Jones wrote:
On Fri, Apr 28, 2023 at 01:03:36PM +0100, Nikos Nikoleris wrote:
Hello,

This series adds initial support for building arm64 tests as EFI
apps 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. We add support for setting up the PSCI
conduit, discovering the UART, timers, GIC 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
    test starts EL1. This will be required to run EFI tests on sytems
    that implement EL2.
  - 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.
  - Various fixes related to cache maintaince to better support turn the
    MMU off.
  - Switch to a new stack and avoid relying on the one provided by EFI.

git branch: https://github.com/relokin/kvm-unit-tests/pull/new/target-efi-upstream-v5

v4: https://lore.kernel.org/kvmarm/20230213101759.2577077-1-nikos.nikoleris@xxxxxxx/
v3: https://lore.kernel.org/all/20220630100324.3153655-1-nikos.nikoleris@xxxxxxx/
v2: https://lore.kernel.org/kvm/20220506205605.359830-1-nikos.nikoleris@xxxxxxx/

Changes in v5:
  - Minor style changes (thanks Shaoqin).
  - Avoid including lib/acpi.o to cflatobjs twice (thanks Drew).
  - Increase NR_INITIAL_MEM_REGIONS to avoid overflows and add check when
    we run out of space (thanks Shaoqin).

Changes in v4:
  - Removed patch that reworks cache maintenance when turning the MMU
    off. This is not strictly required for EFI tests running with tcg and
    will be addressed in a separate series by Alex.
  - Fix compilation for arm (Alex).
  - Convert ACPI tables to Linux style (Alex).

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!

I'd like to get an ack from either Paolo or Sean on the changes to ACPI,
as they're shared with x86, and there are also some x86 code changes.

Actually, there are two build pipeline failures with the new ACPI code.
Please take a look at

https://gitlab.com/jones-drew/kvm-unit-tests/-/pipelines/852864569


Thanks for reviewing the series!

I think this fixes the compilation issues:

diff --git a/lib/acpi.h b/lib/acpi.h
index 202d832e..c330c877 100644
--- a/lib/acpi.h
+++ b/lib/acpi.h
@@ -292,7 +292,8 @@ struct acpi_table_gtdt {
        u32 platform_timer_offset;
 };

-#pragma pack(0)
+/* Reset to default packing */
+#pragma pack()

 void set_efi_rsdp(struct acpi_table_rsdp *rsdp);
 void *find_acpi_table_addr(u32 sig);
diff --git a/lib/acpi.c b/lib/acpi.c
index 760cd8b2..0440cddb 100644
--- a/lib/acpi.c
+++ b/lib/acpi.c
@@ -70,7 +70,7 @@ void *find_acpi_table_addr(u32 sig)
                return rsdt;

        if (rsdp->revision >= 2) {
-               xsdt = (void *)rsdp->xsdt_physical_address;
+               xsdt = (void *)(ulong) rsdp->xsdt_physical_address;
                if (xsdt && xsdt->signature != XSDT_SIGNATURE)
                        xsdt = NULL;
        }

Thanks,
drew


Also,

   1) It'd be nice if this worked with DT, too. We can use UEFI with DT
      when adding '-no-acpi' to the QEMU command line. setup_efi() needs
      to learn how to find the dtb and most the '#ifdef CONFIG_EFI's
      would need to change to a new CONFIG_ACPI guard.


I had a quick look at it at some point and it didn't look straightforward but I'll check again.

   2) The debug bp and ss tests fail with EFI, but not without, for me.


I think, I've found the problem, the patch below fixes it for me.

diff --git a/arm/debug.c b/arm/debug.c
index b3e9749c..126fa267 100644
--- a/arm/debug.c
+++ b/arm/debug.c
@@ -292,11 +292,14 @@ static noinline void test_hw_bp(bool migrate)
        hw_bp_idx = 0;

        /* Trap on up to 16 debug exception unmask instructions. */
-       asm volatile("hw_bp0:\n"
- "msr daifclr, #8; msr daifclr, #8; msr daifclr, #8; msr daifclr, #8\n" - "msr daifclr, #8; msr daifclr, #8; msr daifclr, #8; msr daifclr, #8\n" - "msr daifclr, #8; msr daifclr, #8; msr daifclr, #8; msr daifclr, #8\n" - "msr daifclr, #8; msr daifclr, #8; msr daifclr, #8; msr daifclr, #8\n");
+       asm volatile(
+               ".globl hw_bp0\n"
+               "hw_bp0:\n"
+ "msr daifclr, #8; msr daifclr, #8; msr daifclr, #8; msr daifclr, #8\n" + "msr daifclr, #8; msr daifclr, #8; msr daifclr, #8; msr daifclr, #8\n" + "msr daifclr, #8; msr daifclr, #8; msr daifclr, #8; msr daifclr, #8\n" + "msr daifclr, #8; msr daifclr, #8; msr daifclr, #8; msr daifclr, #8\n"
+               );

        for (i = 0, addr = (uint64_t)&hw_bp0; i < num_bp; i++, addr += 4)
                report(hw_bp_addr[i] == addr, "hw breakpoint: %d", i);
@@ -367,11 +370,14 @@ static noinline void test_ss(bool migrate)

        asm volatile("msr daifclr, #8");

-       asm volatile("ss_start:\n"
+       asm volatile(
+               ".globl ss_start\n"
+               "ss_start:\n"
                        "mrs x0, esr_el1\n"
                        "add x0, x0, #1\n"
                        "msr daifset, #8\n"
-                       : : : "x0");
+                       : : : "x0"
+               );

        report(ss_addr[0] == (uint64_t)&ss_start, "single step");
 }

   3) The timer test runs (and succeeds) when run with
      './arm/efi/run ./arm/timer.efi', but not when run with
      './run_tests.sh -g timer'. This is because UEFI takes
      up all the given timeout time (10s), and then the test times out.
      The hackyish fix below resolves it for me. I'll consider posting it
      as a real patch


I see. I didn't hit the timeout on my test machine but I tried on a slower machine and I did.

New branch with the fixups here:

https://github.com/relokin/kvm-unit-tests/pull/new/target-efi-upstream-v5

Many thanks,

Nikos

Thanks,
drew

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 51e4b97b27d1..72ce718b1170 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -94,7 +94,17 @@ run_qemu_status ()
timeout_cmd ()
  {
+	local s
+
  	if [ "$TIMEOUT" ] && [ "$TIMEOUT" != "0" ]; then
+		if [ "$CONFIG_EFI" = 'y' ]; then
+			s=${TIMEOUT: -1}
+			if [ "$s" = 's' ]; then
+				TIMEOUT=${TIMEOUT:0:-1}
+				((TIMEOUT += 10)) # Add 10 seconds for booting UEFI
+				TIMEOUT="${TIMEOUT}s"
+			fi
+		fi
  		echo "timeout -k 1s --foreground $TIMEOUT"
  	fi
  }



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux