* [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements
@ 2024-02-27 19:21 Andrew Jones
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 01/18] runtime: Update MAX_SMP probe Andrew Jones
` (17 more replies)
0 siblings, 18 replies; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
This series collects one fix ("Update MAX_SMP probe") with a bunch of
improvements to the EFI setup code and run script. With the series
applied one can add --enable-efi-direct when configuring and then
run the EFI tests on QEMU much, much faster by using direct kernel
boot for them (and environment variables will work too). The non-
direct (original) way of running the EFI tests has also been sped up
a bit by not running the dummy test and not generating the dtb twice.
The cleanups in the setup code allow duplicated code to be removed
(by sharing with the non-EFI setup code) and eventually for riscv
to share some code too with the introduction of memregions_efi_init().
v2:
- Add another improvement (patches 15-17), which is to stop mapping
EFI regions which we consider reserved (including
EFI_BOOT_SERVICES_DATA regions which requires moving the primary stack)
- Add EFI gitlab CI tests
- Fix one typo in configure help text
Thanks,
drew
Andrew Jones (17):
runtime: Update MAX_SMP probe
runtime: Add yet another 'no kernel' error message
arm64: efi: Don't create dummy test
arm64: efi: Remove redundant dtb generation
arm64: efi: Move run code into a function
arm64: efi: Remove EFI_USE_DTB
arm64: efi: Improve device tree discovery
lib/efi: Add support for loading the initrd
arm64: efi: Allow running tests directly
arm/arm64: Factor out some initial setup
arm/arm64: Factor out allocator init from mem_init
arm64: Simplify efi_mem_init
arm64: Add memregions_efi_init
arm64: efi: Don't map reserved regions
arm64: efi: Fix _start returns from failed _relocate
arm64: efi: Switch to our own stack
arm64: efi: Add gitlab CI
Shaoqin Huang (1):
arm64: efi: Make running tests on EFI can be parallel
.gitlab-ci.yml | 32 +++++-
arm/efi/crt0-efi-aarch64.S | 37 ++++--
arm/efi/run | 65 +++++++----
arm/run | 6 +-
configure | 17 +++
lib/arm/mmu.c | 6 +-
lib/arm/setup.c | 227 +++++++++++++++----------------------
lib/efi.c | 93 +++++++++++++--
lib/efi.h | 3 +-
lib/linux/efi.h | 29 +++++
lib/memregions.c | 53 +++++++++
lib/memregions.h | 6 +
run_tests.sh | 5 +-
scripts/runtime.bash | 21 ++--
14 files changed, 396 insertions(+), 204 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 01/18] runtime: Update MAX_SMP probe
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-03 21:43 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 02/18] runtime: Add yet another 'no kernel' error message Andrew Jones
` (16 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
Arm's MAX_SMP probing must have stopped working at some point due to
QEMU's error message changing, but nobody noticed. Also, the probing
should work for at least x86 now too, so the comment isn't correct
anymore either. We could probably just delete this probe thing, but
in case it could still serve some purpose we can also keep it, but
updated for later QEMU, and only enabled when a new run_tests.sh
command line option is provided.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
run_tests.sh | 5 ++++-
scripts/runtime.bash | 19 ++++++++++---------
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/run_tests.sh b/run_tests.sh
index abb0ab773362..bb3024ff95b1 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -44,7 +44,7 @@ fi
only_tests=""
list_tests=""
-args=$(getopt -u -o ag:htj:vl -l all,group:,help,tap13,parallel:,verbose,list -- $*)
+args=$(getopt -u -o ag:htj:vl -l all,group:,help,tap13,parallel:,verbose,list,probe-maxsmp -- $*)
[ $? -ne 0 ] && exit 2;
set -- $args;
while [ $# -gt 0 ]; do
@@ -78,6 +78,9 @@ while [ $# -gt 0 ]; do
-l | --list)
list_tests="yes"
;;
+ --probe-maxsmp)
+ probe_maxsmp
+ ;;
--)
;;
*)
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index c73fb0240d12..f2e43bb1ed60 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -200,12 +200,13 @@ function run()
#
# Probe for MAX_SMP, in case it's less than the number of host cpus.
#
-# This probing currently only works for ARM, as x86 bails on another
-# error first, so this check is only run for ARM and ARM64. The
-# parameter expansion takes the last number from the QEMU error
-# message, which gives the allowable MAX_SMP.
-if [[ $ARCH == 'arm' || $ARCH == 'arm64' ]] &&
- smp=$($RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP |& grep 'exceeds max CPUs'); then
- smp=${smp##*(}
- MAX_SMP=${smp:0:-1}
-fi
+function probe_maxsmp()
+{
+ local smp
+
+ if smp=$($RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP |& grep 'Invalid SMP CPUs'); then
+ smp=${smp##* }
+ echo "Restricting MAX_SMP from ($MAX_SMP) to the max supported ($smp)" >&2
+ MAX_SMP=$smp
+ fi
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 02/18] runtime: Add yet another 'no kernel' error message
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 01/18] runtime: Update MAX_SMP probe Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-03 21:50 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 03/18] arm64: efi: Don't create dummy test Andrew Jones
` (15 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
When booting an Arm machine with the -bios command line option we
get yet another error message from QEMU when using _NO_FILE_4Uhere_
to probe command line support. Add it to the check in
premature_failure()
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
scripts/runtime.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index f2e43bb1ed60..255e756f2cb2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -18,7 +18,7 @@ premature_failure()
local log="$(eval "$(get_cmdline _NO_FILE_4Uhere_)" 2>&1)"
echo "$log" | grep "_NO_FILE_4Uhere_" |
- grep -q -e "could not \(load\|open\) kernel" -e "error loading" &&
+ grep -q -e "could not \(load\|open\) kernel" -e "error loading" -e "failed to load" &&
return 1
RUNTIME_log_stderr <<< "$log"
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 03/18] arm64: efi: Don't create dummy test
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 01/18] runtime: Update MAX_SMP probe Andrew Jones
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 02/18] runtime: Add yet another 'no kernel' error message Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-03 21:57 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 04/18] arm64: efi: Make running tests on EFI can be parallel Andrew Jones
` (14 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
The purpose of the _NO_FILE_4Uhere_ kernel is to check that all the
QEMU command line options that have been pulled together by the
scripts will work. Since booting with UEFI and the -kernel command
line is supported by QEMU, then we don't need to create a dummy
test for _NO_FILE_4Uhere_ and go all the way into UEFI's shell and
execute it to prove the command line is OK, since we would have
failed much before all that if it wasn't. Just run QEMU "normally",
i.e. no EFI_RUN=y, but add the UEFI -bios and its file system command
line options, in order to check the full command line.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
arm/efi/run | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arm/efi/run b/arm/efi/run
index 6872c337c945..e629abde5273 100755
--- a/arm/efi/run
+++ b/arm/efi/run
@@ -53,7 +53,14 @@ while (( "$#" )); do
done
if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then
- EFI_CASE=dummy
+ EFI_CASE_DIR="$EFI_TEST/dummy"
+ mkdir -p "$EFI_CASE_DIR"
+ $TEST_DIR/run \
+ $EFI_CASE \
+ -bios "$EFI_UEFI" \
+ -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
+ "${qemu_args[@]}"
+ exit
fi
: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_CASE"}"
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 04/18] arm64: efi: Make running tests on EFI can be parallel
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (2 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 03/18] arm64: efi: Don't create dummy test Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-03 22:06 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 05/18] arm64: efi: Remove redundant dtb generation Andrew Jones
` (13 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
From: Shaoqin Huang <shahuang@redhat.com>
Currently running tests on EFI in parallel can cause part of tests to
fail, this is because arm/efi/run script use the EFI_CASE to create the
subdir under the efi-tests, and the EFI_CASE is the filename of the
test, when running tests in parallel, the multiple tests exist in the
same filename will execute at the same time, which will use the same
directory and write the test specific things into it, this cause
chaotic and make some tests fail.
For example, if we running the pmu-sw-incr and pmu-chained-counters
and other pmu tests on EFI at the same time, the EFI_CASE will be pmu.
So they will write their $cmd_args to the $EFI/TEST/pmu/startup.nsh
at the same time, which will corrupt the startup.nsh file.
And we can get the log which outputs:
* pmu-sw-incr.log:
- ABORT: pmu: Unknown sub-test 'pmu-mem-acce'
* pmu-chained-counters.log
- ABORT: pmu: Unknown sub-test 'pmu-mem-access-reliab'
And the efi-tests/pmu/startup.nsh:
@echo -off
setvar fdtfile -guid 97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823 -rt =L"dtb"
pmu.efi pmu-mem-access-reliability
setvar fdtfile -guid 97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823 -rt =L"dtb"
pmu.efi pmu-chained-sw-incr
As you can see, when multiple tests write to the same startup.nsh file,
it causes the issue.
To Fix this issue, use the testname instead of the filename to create
the subdir under the efi-tests. We use the EFI_TESTNAME to replace the
EFI_CASE in script. Since every testname is specific, now the tests
can be run parallel. It also considers when user directly use the
arm/efi/run to run test, in this case, still use the filename.
Besides, replace multiple $EFI_TEST/$EFI_CASE to the $EFI_CASE_DIR, this
makes the script looks more clean and we don'e need to replace many
EFI_CASE to EFI_TESTNAME.
Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
arm/efi/run | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/arm/efi/run b/arm/efi/run
index e629abde5273..8b6512520026 100755
--- a/arm/efi/run
+++ b/arm/efi/run
@@ -25,6 +25,8 @@ fi
: "${EFI_UEFI:=$DEFAULT_UEFI}"
: "${EFI_TEST:=efi-tests}"
: "${EFI_CASE:=$(basename $1 .efi)}"
+: "${EFI_TESTNAME:=$TESTNAME}"
+: "${EFI_TESTNAME:=$EFI_CASE}"
: "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}"
[ "$EFI_USE_ACPI" = "y" ] || EFI_USE_DTB=y
@@ -63,20 +65,20 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then
exit
fi
-: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_CASE"}"
+: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}"
mkdir -p "$EFI_CASE_DIR"
-cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/"
-echo "@echo -off" > "$EFI_TEST/$EFI_CASE/startup.nsh"
+cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/"
+echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh"
if [ "$EFI_USE_DTB" = "y" ]; then
qemu_args+=(-machine acpi=off)
FDT_BASENAME="dtb"
- $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_TEST/$EFI_CASE/$FDT_BASENAME" "${qemu_args[@]}")
- echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_TEST/$EFI_CASE/startup.nsh"
+ $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}")
+ echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_CASE_DIR/startup.nsh"
fi
-echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_TEST/$EFI_CASE/startup.nsh"
+echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_CASE_DIR/startup.nsh"
EFI_RUN=y $TEST_DIR/run \
-bios "$EFI_UEFI" \
- -drive file.dir="$EFI_TEST/$EFI_CASE/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
+ -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
"${qemu_args[@]}"
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 05/18] arm64: efi: Remove redundant dtb generation
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (3 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 04/18] arm64: efi: Make running tests on EFI can be parallel Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 7:16 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 06/18] arm64: efi: Move run code into a function Andrew Jones
` (12 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
When a line in bash is written as
$(some-line)
Then 'some-line' will be evaluated and then whatever some-line outputs
will be evaluated. The dtb is getting generated twice since the line
that should generate it is within $() and the output of that is the
command itself (since arm/run outputs its command), so the command
gets executed again. Remove the $() to just execute dtb generation
once.
While mucking with arm/efi/run tidy it a bit by by removing the unused
sourcing of common.bash and the unnecessary 'set -e' (we check for and
propagate errors ourselves). Finally, make one reorganization change
and some whitespace fixes.
Fixes: 2607d2d6946a ("arm64: Add an efi/run script")
Fixes: 2e080dafec2a ("arm64: Use the provided fdt when booting through EFI")
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
arm/efi/run | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/arm/efi/run b/arm/efi/run
index 8b6512520026..e45cecfa3265 100755
--- a/arm/efi/run
+++ b/arm/efi/run
@@ -1,7 +1,5 @@
#!/bin/bash
-set -e
-
if [ $# -eq 0 ]; then
echo "Usage $0 TEST_CASE [QEMU_ARGS]"
exit 2
@@ -13,7 +11,6 @@ if [ ! -f config.mak ]; then
fi
source config.mak
source scripts/arch-run.bash
-source scripts/common.bash
if [ -f /usr/share/qemu-efi-aarch64/QEMU_EFI.fd ]; then
DEFAULT_UEFI=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd
@@ -27,6 +24,7 @@ fi
: "${EFI_CASE:=$(basename $1 .efi)}"
: "${EFI_TESTNAME:=$TESTNAME}"
: "${EFI_TESTNAME:=$EFI_CASE}"
+: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}"
: "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}"
[ "$EFI_USE_ACPI" = "y" ] || EFI_USE_DTB=y
@@ -65,20 +63,18 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then
exit
fi
-: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}"
mkdir -p "$EFI_CASE_DIR"
-
cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/"
echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh"
if [ "$EFI_USE_DTB" = "y" ]; then
qemu_args+=(-machine acpi=off)
FDT_BASENAME="dtb"
- $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}")
+ EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}"
echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_CASE_DIR/startup.nsh"
fi
echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_CASE_DIR/startup.nsh"
EFI_RUN=y $TEST_DIR/run \
- -bios "$EFI_UEFI" \
- -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
- "${qemu_args[@]}"
+ -bios "$EFI_UEFI" \
+ -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
+ "${qemu_args[@]}"
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 06/18] arm64: efi: Move run code into a function
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (4 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 05/18] arm64: efi: Remove redundant dtb generation Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 7:19 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 07/18] arm64: efi: Remove EFI_USE_DTB Andrew Jones
` (11 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
Push the run code in arm/efi/run into a function named
uefi_shell_run() since it will create an EFI file system, copy
the test and possibly the DTB there, and create a startup.nsh
which executes the test from the UEFI shell. Pushing this
code into a function allows additional execution paths to be
created in the script. Also rename EFI_RUN to UEFI_SHELL_RUN
to pass the information on to arm/run that it's being called
from uefi_shell_run().
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
arm/efi/run | 33 +++++++++++++++++++--------------
arm/run | 4 ++--
2 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/arm/efi/run b/arm/efi/run
index e45cecfa3265..494ba9e7efe7 100755
--- a/arm/efi/run
+++ b/arm/efi/run
@@ -63,18 +63,23 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then
exit
fi
-mkdir -p "$EFI_CASE_DIR"
-cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/"
-echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh"
-if [ "$EFI_USE_DTB" = "y" ]; then
- qemu_args+=(-machine acpi=off)
- FDT_BASENAME="dtb"
- EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}"
- echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_CASE_DIR/startup.nsh"
-fi
-echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_CASE_DIR/startup.nsh"
+uefi_shell_run()
+{
+ mkdir -p "$EFI_CASE_DIR"
+ cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/"
+ echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh"
+ if [ "$EFI_USE_DTB" = "y" ]; then
+ qemu_args+=(-machine acpi=off)
+ FDT_BASENAME="dtb"
+ UEFI_SHELL_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}"
+ echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_CASE_DIR/startup.nsh"
+ fi
+ echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_CASE_DIR/startup.nsh"
+
+ UEFI_SHELL_RUN=y $TEST_DIR/run \
+ -bios "$EFI_UEFI" \
+ -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
+ "${qemu_args[@]}"
+}
-EFI_RUN=y $TEST_DIR/run \
- -bios "$EFI_UEFI" \
- -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
- "${qemu_args[@]}"
+uefi_shell_run
diff --git a/arm/run b/arm/run
index ac64b3b461a2..40c2ca66ba7e 100755
--- a/arm/run
+++ b/arm/run
@@ -60,7 +60,7 @@ if ! $qemu $M -chardev '?' | grep -q testdev; then
exit 2
fi
-if [ "$EFI_RUN" != "y" ]; then
+if [ "$UEFI_SHELL_RUN" != "y" ]; then
chr_testdev='-device virtio-serial-device'
chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
fi
@@ -75,7 +75,7 @@ command="$qemu -nodefaults $M $A -cpu $processor $chr_testdev $pci_testdev"
command+=" -display none -serial stdio"
command="$(migration_cmd) $(timeout_cmd) $command"
-if [ "$EFI_RUN" = "y" ]; then
+if [ "$UEFI_SHELL_RUN" = "y" ]; then
ENVIRON_DEFAULT=n run_qemu_status $command "$@"
else
run_qemu $command -kernel "$@"
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 07/18] arm64: efi: Remove EFI_USE_DTB
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (5 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 06/18] arm64: efi: Move run code into a function Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 7:20 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 08/18] arm64: efi: Improve device tree discovery Andrew Jones
` (10 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
We don't need two variables for one boolean property. Just use
!EFI_USE_ACPI to infer efi-use-dtb.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
arm/efi/run | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arm/efi/run b/arm/efi/run
index 494ba9e7efe7..b7a8418a07f8 100755
--- a/arm/efi/run
+++ b/arm/efi/run
@@ -27,8 +27,6 @@ fi
: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}"
: "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}"
-[ "$EFI_USE_ACPI" = "y" ] || EFI_USE_DTB=y
-
if [ ! -f "$EFI_UEFI" ]; then
echo "UEFI firmware not found."
echo "Please specify the path with the env variable EFI_UEFI"
@@ -68,7 +66,7 @@ uefi_shell_run()
mkdir -p "$EFI_CASE_DIR"
cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/"
echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh"
- if [ "$EFI_USE_DTB" = "y" ]; then
+ if [ "$EFI_USE_ACPI" != "y" ]; then
qemu_args+=(-machine acpi=off)
FDT_BASENAME="dtb"
UEFI_SHELL_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}"
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 08/18] arm64: efi: Improve device tree discovery
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (6 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 07/18] arm64: efi: Remove EFI_USE_DTB Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 7:34 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 09/18] lib/efi: Add support for loading the initrd Andrew Jones
` (9 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
Zero is a valid address for the device tree so add an fdt_valid data
member to determine when the address is valid or not. Also, check the
device tree GUID when the environment variable is missing. The latter
change allows directly loading the unit test with QEMU's '-kernel'
command line parameter, which is much faster than putting the test
in the EFI file system and then running it from the UEFI shell.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
lib/arm/setup.c | 3 ++-
lib/efi.c | 28 +++++++++++++++++-----------
lib/efi.h | 3 ++-
lib/linux/efi.h | 2 ++
4 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 0382cbdaf5a1..76aae4627a7b 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -342,7 +342,8 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
}
memregions_add(&r);
}
- if (fdt) {
+
+ if (efi_bootinfo->fdt_valid) {
/* Move the FDT to the base of free memory */
fdt_size = fdt_totalsize(fdt);
ret = fdt_move(fdt, (void *)free_mem_start, fdt_size);
diff --git a/lib/efi.c b/lib/efi.c
index d94f0fa16fc0..0785bd3e8916 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -6,13 +6,13 @@
*
* SPDX-License-Identifier: LGPL-2.0-or-later
*/
-
-#include "efi.h"
+#include <libcflat.h>
#include <argv.h>
-#include <stdlib.h>
#include <ctype.h>
-#include <libcflat.h>
+#include <stdlib.h>
#include <asm/setup.h>
+#include "efi.h"
+#include "libfdt/libfdt.h"
/* From lib/argv.c */
extern int __argc, __envc;
@@ -283,18 +283,24 @@ static void* efi_get_var(efi_handle_t handle, struct efi_loaded_image_64 *image,
return val;
}
-static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
+static bool efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image, void **fdt)
{
efi_char16_t var[] = ENV_VARNAME_DTBFILE;
efi_char16_t *val;
- void *fdt = NULL;
- int fdtsize;
+ int fdtsize = 0;
+
+ *fdt = NULL;
val = efi_get_var(handle, image, var);
- if (val)
- efi_load_image(handle, image, &fdt, &fdtsize, val);
+ if (val) {
+ efi_load_image(handle, image, fdt, &fdtsize, val);
+ if (fdtsize == 0)
+ return false;
+ } else if (efi_get_system_config_table(DEVICE_TREE_GUID, fdt) != EFI_SUCCESS) {
+ return false;
+ }
- return fdt;
+ return fdt_check_header(*fdt) == 0;
}
efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
@@ -335,7 +341,7 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
}
setup_args(cmdline_ptr);
- efi_bootinfo.fdt = efi_get_fdt(handle, image);
+ efi_bootinfo.fdt_valid = efi_get_fdt(handle, image, &efi_bootinfo.fdt);
/* Set up efi_bootinfo */
efi_bootinfo.mem_map.map = ↦
efi_bootinfo.mem_map.map_size = &map_size;
diff --git a/lib/efi.h b/lib/efi.h
index db46d45068ee..4bd01f7199ce 100644
--- a/lib/efi.h
+++ b/lib/efi.h
@@ -30,7 +30,8 @@
*/
typedef struct {
struct efi_boot_memmap mem_map;
- const void *fdt;
+ void *fdt;
+ bool fdt_valid;
} efi_bootinfo_t;
efi_status_t _relocate(long ldbase, Elf64_Dyn *dyn, efi_handle_t handle,
diff --git a/lib/linux/efi.h b/lib/linux/efi.h
index 410f0b1a0da1..92d798f79767 100644
--- a/lib/linux/efi.h
+++ b/lib/linux/efi.h
@@ -66,6 +66,8 @@ typedef guid_t efi_guid_t;
#define ACPI_TABLE_GUID EFI_GUID(0xeb9d2d30, 0x2d88, 0x11d3, 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
#define ACPI_20_TABLE_GUID EFI_GUID(0x8868e871, 0xe4f1, 0x11d3, 0xbc, 0x22, 0x00, 0x80, 0xc7, 0x3c, 0x88, 0x81)
+#define DEVICE_TREE_GUID EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
+
#define LOADED_IMAGE_PROTOCOL_GUID EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
typedef struct {
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 09/18] lib/efi: Add support for loading the initrd
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (7 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 08/18] arm64: efi: Improve device tree discovery Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 7:44 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 10/18] arm64: efi: Allow running tests directly Andrew Jones
` (8 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
When loading non-efi tests with QEMU's '-kernel' option we also load
an environ with the '-initrd' option. Now that efi tests can also be
loaded with the '-kernel' option also provide the '-initrd' environ.
For EFI, we use the EFI_LOAD_FILE2_PROTOCOL_GUID protocol to load
LINUX_EFI_INITRD_MEDIA_GUID. Each architecture which wants to use the
initrd for the environ will need to call setup_env() on the initrd
data. As usual, the new efi function is heavily influenced by Linux's
implementation.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
lib/efi.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++
lib/linux/efi.h | 27 ++++++++++++++++++++
2 files changed, 92 insertions(+)
diff --git a/lib/efi.c b/lib/efi.c
index 0785bd3e8916..edfcc80ef114 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -14,6 +14,10 @@
#include "efi.h"
#include "libfdt/libfdt.h"
+/* From each arch */
+extern char *initrd;
+extern u32 initrd_size;
+
/* From lib/argv.c */
extern int __argc, __envc;
extern char *__argv[100];
@@ -303,6 +307,65 @@ static bool efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image,
return fdt_check_header(*fdt) == 0;
}
+static const struct {
+ struct efi_vendor_dev_path vendor;
+ struct efi_generic_dev_path end;
+} __packed initrd_dev_path = {
+ {
+ {
+ EFI_DEV_MEDIA,
+ EFI_DEV_MEDIA_VENDOR,
+ sizeof(struct efi_vendor_dev_path),
+ },
+ LINUX_EFI_INITRD_MEDIA_GUID
+ }, {
+ EFI_DEV_END_PATH,
+ EFI_DEV_END_ENTIRE,
+ sizeof(struct efi_generic_dev_path)
+ }
+};
+
+static void efi_load_initrd(void)
+{
+ efi_guid_t lf2_proto_guid = EFI_LOAD_FILE2_PROTOCOL_GUID;
+ efi_device_path_protocol_t *dp;
+ efi_load_file2_protocol_t *lf2;
+ efi_handle_t handle;
+ efi_status_t status;
+ unsigned long file_size = 0;
+
+ initrd = NULL;
+ initrd_size = 0;
+
+ dp = (efi_device_path_protocol_t *)&initrd_dev_path;
+ status = efi_bs_call(locate_device_path, &lf2_proto_guid, &dp, &handle);
+ if (status != EFI_SUCCESS)
+ return;
+
+ status = efi_bs_call(handle_protocol, handle, &lf2_proto_guid, (void **)&lf2);
+ assert(status == EFI_SUCCESS);
+
+ status = efi_call_proto(lf2, load_file, dp, false, &file_size, NULL);
+ assert(status == EFI_BUFFER_TOO_SMALL);
+
+ status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, file_size, (void **)&initrd);
+ assert(status == EFI_SUCCESS);
+
+ status = efi_call_proto(lf2, load_file, dp, false, &file_size, (void *)initrd);
+ assert(status == EFI_SUCCESS);
+
+ initrd_size = (u32)file_size;
+
+ /*
+ * UEFI appends initrd=initrd to the command line when an initrd is present.
+ * Remove it in order to avoid confusing unit tests.
+ */
+ if (!strcmp(__argv[__argc - 1], "initrd=initrd")) {
+ __argv[__argc - 1] = NULL;
+ __argc -= 1;
+ }
+}
+
efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
{
int ret;
@@ -341,6 +404,8 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
}
setup_args(cmdline_ptr);
+ efi_load_initrd();
+
efi_bootinfo.fdt_valid = efi_get_fdt(handle, image, &efi_bootinfo.fdt);
/* Set up efi_bootinfo */
efi_bootinfo.mem_map.map = ↦
diff --git a/lib/linux/efi.h b/lib/linux/efi.h
index 92d798f79767..8fa23ad078ce 100644
--- a/lib/linux/efi.h
+++ b/lib/linux/efi.h
@@ -70,6 +70,9 @@ typedef guid_t efi_guid_t;
#define LOADED_IMAGE_PROTOCOL_GUID EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
+#define EFI_LOAD_FILE2_PROTOCOL_GUID EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)
+#define LINUX_EFI_INITRD_MEDIA_GUID EFI_GUID(0x5568e427, 0x68fc, 0x4f3d, 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
+
typedef struct {
efi_guid_t guid;
void *table;
@@ -248,6 +251,12 @@ struct efi_generic_dev_path {
u16 length;
} __packed;
+struct efi_vendor_dev_path {
+ struct efi_generic_dev_path header;
+ efi_guid_t vendorguid;
+ u8 vendordata[];
+} __packed;
+
typedef struct efi_generic_dev_path efi_device_path_protocol_t;
/*
@@ -449,6 +458,19 @@ typedef struct _efi_simple_file_system_protocol efi_simple_file_system_protocol_
typedef struct _efi_file_protocol efi_file_protocol_t;
typedef efi_simple_file_system_protocol_t efi_file_io_interface_t;
typedef efi_file_protocol_t efi_file_t;
+typedef union efi_load_file_protocol efi_load_file_protocol_t;
+typedef union efi_load_file_protocol efi_load_file2_protocol_t;
+
+union efi_load_file_protocol {
+ struct {
+ efi_status_t (__efiapi *load_file)(efi_load_file_protocol_t *,
+ efi_device_path_protocol_t *,
+ bool, unsigned long *, void *);
+ };
+ struct {
+ u32 load_file;
+ } mixed_mode;
+};
typedef efi_status_t efi_simple_file_system_protocol_open_volume(
efi_simple_file_system_protocol_t *this,
@@ -544,7 +566,12 @@ typedef struct {
efi_char16_t file_name[1];
} efi_file_info_t;
+#define efi_fn_call(inst, func, ...) (inst)->func(__VA_ARGS__)
#define efi_bs_call(func, ...) efi_system_table->boottime->func(__VA_ARGS__)
#define efi_rs_call(func, ...) efi_system_table->runtime->func(__VA_ARGS__)
+#define efi_call_proto(inst, func, ...) ({ \
+ __typeof__(inst) __inst = (inst); \
+ efi_fn_call(__inst, func, __inst, ##__VA_ARGS__); \
+})
#endif /* __LINUX_UEFI_H */
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 10/18] arm64: efi: Allow running tests directly
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (8 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 09/18] lib/efi: Add support for loading the initrd Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 7:52 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 11/18] arm/arm64: Factor out some initial setup Andrew Jones
` (7 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
Since it's possible to run tests with UEFI and the QEMU -kernel
option (and now the DTB will be found and even the environ will
be set up from an initrd if given with the -initrd option), then
we can skip the loading of EFI tests into a file system and booting
to the shell to run them. Just run them directly. Running directly
is waaaaaay faster than booting the shell first. We keep the UEFI
shell as the default behavior, though, and provide a new configure
option to enable the direct running.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
arm/efi/run | 17 +++++++++++++++--
arm/run | 4 +++-
configure | 17 +++++++++++++++++
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/arm/efi/run b/arm/efi/run
index b7a8418a07f8..af7b593c2bb8 100755
--- a/arm/efi/run
+++ b/arm/efi/run
@@ -18,10 +18,12 @@ elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
DEFAULT_UEFI=/usr/share/edk2/aarch64/QEMU_EFI.silent.fd
fi
+KERNEL_NAME=$1
+
: "${EFI_SRC:=$TEST_DIR}"
: "${EFI_UEFI:=$DEFAULT_UEFI}"
: "${EFI_TEST:=efi-tests}"
-: "${EFI_CASE:=$(basename $1 .efi)}"
+: "${EFI_CASE:=$(basename $KERNEL_NAME .efi)}"
: "${EFI_TESTNAME:=$TESTNAME}"
: "${EFI_TESTNAME:=$EFI_CASE}"
: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}"
@@ -80,4 +82,15 @@ uefi_shell_run()
"${qemu_args[@]}"
}
-uefi_shell_run
+if [ "$EFI_DIRECT" = "y" ]; then
+ if [ "$EFI_USE_ACPI" != "y" ]; then
+ qemu_args+=(-machine acpi=off)
+ fi
+ $TEST_DIR/run \
+ $KERNEL_NAME \
+ -append "$(basename $KERNEL_NAME) ${cmd_args[@]}" \
+ -bios "$EFI_UEFI" \
+ "${qemu_args[@]}"
+else
+ uefi_shell_run
+fi
diff --git a/arm/run b/arm/run
index 40c2ca66ba7e..efdd44ce86a7 100755
--- a/arm/run
+++ b/arm/run
@@ -60,7 +60,7 @@ if ! $qemu $M -chardev '?' | grep -q testdev; then
exit 2
fi
-if [ "$UEFI_SHELL_RUN" != "y" ]; then
+if [ "$UEFI_SHELL_RUN" != "y" ] && [ "$EFI_USE_ACPI" != "y" ]; then
chr_testdev='-device virtio-serial-device'
chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
fi
@@ -77,6 +77,8 @@ command="$(migration_cmd) $(timeout_cmd) $command"
if [ "$UEFI_SHELL_RUN" = "y" ]; then
ENVIRON_DEFAULT=n run_qemu_status $command "$@"
+elif [ "$EFI_USE_ACPI" = "y" ]; then
+ run_qemu_status $command -kernel "$@"
else
run_qemu $command -kernel "$@"
fi
diff --git a/configure b/configure
index 05e6702eab06..283c959973fd 100755
--- a/configure
+++ b/configure
@@ -32,6 +32,7 @@ enable_dump=no
page_size=
earlycon=
efi=
+efi_direct=
# Enable -Werror by default for git repositories only (i.e. developer builds)
if [ -e "$srcdir"/.git ]; then
@@ -89,6 +90,11 @@ usage() {
--[enable|disable]-efi Boot and run from UEFI (disabled by default, x86_64 and arm64 only)
--[enable|disable]-werror
Select whether to compile with the -Werror compiler flag
+ --[enable|disable]-efi-direct
+ Select whether to run EFI tests directly with QEMU's -kernel
+ option. When not enabled, tests will be placed in an EFI file
+ system and run from the UEFI shell. Ignored when efi isn't enabled.
+ (arm64 only)
EOF
exit 1
}
@@ -168,6 +174,12 @@ while [[ "$1" = -* ]]; do
--disable-efi)
efi=n
;;
+ --enable-efi-direct)
+ efi_direct=y
+ ;;
+ --disable-efi-direct)
+ efi_direct=n
+ ;;
--enable-werror)
werror=-Werror
;;
@@ -185,6 +197,10 @@ while [[ "$1" = -* ]]; do
esac
done
+if [ -z "$efi" ] || [ "$efi" = "n" ]; then
+ [ "$efi_direct" = "y" ] && efi_direct=
+fi
+
if [ -n "$host_key_document" ] && [ ! -f "$host_key_document" ]; then
echo "Host key document doesn't exist at the specified location."
exit 1
@@ -423,6 +439,7 @@ GENPROTIMG=${GENPROTIMG-genprotimg}
HOST_KEY_DOCUMENT=$host_key_document
CONFIG_DUMP=$enable_dump
CONFIG_EFI=$efi
+EFI_DIRECT=$efi_direct
CONFIG_WERROR=$werror
GEN_SE_HEADER=$gen_se_header
EOF
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 11/18] arm/arm64: Factor out some initial setup
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (9 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 10/18] arm64: efi: Allow running tests directly Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 7:59 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 12/18] arm/arm64: Factor out allocator init from mem_init Andrew Jones
` (6 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
Factor out some initial setup code into separate functions in order
to share more code between setup() and setup_efi().
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
lib/arm/setup.c | 81 ++++++++++++++++++++++++++++---------------------
1 file changed, 47 insertions(+), 34 deletions(-)
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 76aae4627a7b..f96ee04ddd68 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -182,32 +182,57 @@ static void mem_init(phys_addr_t freemem_start)
page_alloc_ops_enable();
}
-void setup(const void *fdt, phys_addr_t freemem_start)
+static void freemem_push_fdt(void **freemem, const void *fdt)
{
- void *freemem;
- const char *bootargs, *tmp;
u32 fdt_size;
int ret;
- assert(sizeof(long) == 8 || freemem_start < (3ul << 30));
- freemem = (void *)(unsigned long)freemem_start;
-
- /* Move the FDT to the base of free memory */
fdt_size = fdt_totalsize(fdt);
- ret = fdt_move(fdt, freemem, fdt_size);
+ ret = fdt_move(fdt, *freemem, fdt_size);
assert(ret == 0);
- ret = dt_init(freemem);
+ ret = dt_init(*freemem);
assert(ret == 0);
- freemem += fdt_size;
+ *freemem += fdt_size;
+}
+
+static void freemem_push_dt_initrd(void **freemem)
+{
+ const char *tmp;
+ int ret;
- /* Move the initrd to the top of the FDT */
ret = dt_get_initrd(&tmp, &initrd_size);
assert(ret == 0 || ret == -FDT_ERR_NOTFOUND);
if (ret == 0) {
- initrd = freemem;
+ initrd = *freemem;
memmove(initrd, tmp, initrd_size);
- freemem += initrd_size;
+ *freemem += initrd_size;
}
+}
+
+static void initrd_setup(void)
+{
+ char *env;
+
+ if (!initrd)
+ return;
+
+ /* environ is currently the only file in the initrd */
+ env = malloc(initrd_size);
+ memcpy(env, initrd, initrd_size);
+ setup_env(env, initrd_size);
+}
+
+void setup(const void *fdt, phys_addr_t freemem_start)
+{
+ void *freemem;
+ const char *bootargs;
+ int ret;
+
+ assert(sizeof(long) == 8 || freemem_start < (3ul << 30));
+ freemem = (void *)(unsigned long)freemem_start;
+
+ freemem_push_fdt(&freemem, fdt);
+ freemem_push_dt_initrd(&freemem);
memregions_init(arm_mem_regions, NR_MEM_REGIONS);
memregions_add_dt_regions(MAX_DT_MEM_REGIONS);
@@ -229,12 +254,7 @@ void setup(const void *fdt, phys_addr_t freemem_start)
assert(ret == 0 || ret == -FDT_ERR_NOTFOUND);
setup_args_progname(bootargs);
- if (initrd) {
- /* environ is currently the only file in the initrd */
- char *env = malloc(initrd_size);
- memcpy(env, initrd, initrd_size);
- setup_env(env, initrd_size);
- }
+ initrd_setup();
if (!(auxinfo.flags & AUXINFO_MMU_OFF))
setup_vm();
@@ -277,7 +297,6 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
uintptr_t text = (uintptr_t)&_text, etext = ALIGN((uintptr_t)&_etext, 4096);
uintptr_t data = (uintptr_t)&_data, edata = ALIGN((uintptr_t)&_edata, 4096);
const void *fdt = efi_bootinfo->fdt;
- int fdt_size, ret;
/*
* Record the largest free EFI_CONVENTIONAL_MEMORY region
@@ -344,14 +363,13 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
}
if (efi_bootinfo->fdt_valid) {
- /* Move the FDT to the base of free memory */
- fdt_size = fdt_totalsize(fdt);
- ret = fdt_move(fdt, (void *)free_mem_start, fdt_size);
- assert(ret == 0);
- ret = dt_init((void *)free_mem_start);
- assert(ret == 0);
- free_mem_start += ALIGN(fdt_size, EFI_PAGE_SIZE);
- free_mem_pages -= ALIGN(fdt_size, EFI_PAGE_SIZE) >> EFI_PAGE_SHIFT;
+ unsigned long old_start = free_mem_start;
+ void *freemem = (void *)free_mem_start;
+
+ freemem_push_fdt(&freemem, fdt);
+
+ free_mem_start = ALIGN((unsigned long)freemem, EFI_PAGE_SIZE);
+ free_mem_pages = (free_mem_start - old_start) >> EFI_PAGE_SHIFT;
}
__phys_end &= PHYS_MASK;
@@ -419,13 +437,8 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
io_init();
timer_save_state();
- if (initrd) {
- /* environ is currently the only file in the initrd */
- char *env = malloc(initrd_size);
- memcpy(env, initrd, initrd_size);
- setup_env(env, initrd_size);
- }
+ initrd_setup();
if (!(auxinfo.flags & AUXINFO_MMU_OFF))
setup_vm();
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 12/18] arm/arm64: Factor out allocator init from mem_init
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (10 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 11/18] arm/arm64: Factor out some initial setup Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 8:01 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 13/18] arm64: Simplify efi_mem_init Andrew Jones
` (5 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
The allocator init is identical for mem_init() and efi_mem_init().
Share it.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
lib/arm/setup.c | 46 ++++++++++++++++++++++------------------------
1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index f96ee04ddd68..d0be4c437708 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -136,9 +136,28 @@ static void arm_memregions_add_assumed(void)
#endif
}
-static void mem_init(phys_addr_t freemem_start)
+static void mem_allocator_init(phys_addr_t freemem_start, phys_addr_t freemem_end)
{
phys_addr_t base, top;
+
+ freemem_start = PAGE_ALIGN(freemem_start);
+ freemem_end &= PAGE_MASK;
+
+ phys_alloc_init(freemem_start, freemem_end - freemem_start);
+ phys_alloc_set_minimum_alignment(SMP_CACHE_BYTES);
+
+ phys_alloc_get_unused(&base, &top);
+ base = PAGE_ALIGN(base);
+ top &= PAGE_MASK;
+ assert(sizeof(long) == 8 || !(base >> 32));
+ if (sizeof(long) != 8 && (top >> 32) != 0)
+ top = ((uint64_t)1 << 32);
+ page_alloc_init_area(0, base >> PAGE_SHIFT, top >> PAGE_SHIFT);
+ page_alloc_ops_enable();
+}
+
+static void mem_init(phys_addr_t freemem_start)
+{
struct mem_region *freemem, *r, mem = {
.start = (phys_addr_t)-1,
};
@@ -169,17 +188,7 @@ static void mem_init(phys_addr_t freemem_start)
__phys_offset = mem.start; /* PHYS_OFFSET */
__phys_end = mem.end; /* PHYS_END */
- phys_alloc_init(freemem_start, freemem->end - freemem_start);
- phys_alloc_set_minimum_alignment(SMP_CACHE_BYTES);
-
- phys_alloc_get_unused(&base, &top);
- base = PAGE_ALIGN(base);
- top = top & PAGE_MASK;
- assert(sizeof(long) == 8 || !(base >> 32));
- if (sizeof(long) != 8 && (top >> 32) != 0)
- top = ((uint64_t)1 << 32);
- page_alloc_init_area(0, base >> PAGE_SHIFT, top >> PAGE_SHIFT);
- page_alloc_ops_enable();
+ mem_allocator_init(freemem_start, freemem->end);
}
static void freemem_push_fdt(void **freemem, const void *fdt)
@@ -292,7 +301,6 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
struct efi_boot_memmap *map = &(efi_bootinfo->mem_map);
efi_memory_desc_t *buffer = *map->map;
efi_memory_desc_t *d = NULL;
- phys_addr_t base, top;
struct mem_region r;
uintptr_t text = (uintptr_t)&_text, etext = ALIGN((uintptr_t)&_etext, 4096);
uintptr_t data = (uintptr_t)&_data, edata = ALIGN((uintptr_t)&_edata, 4096);
@@ -380,17 +388,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
assert(sizeof(long) == 8 || free_mem_start < (3ul << 30));
- phys_alloc_init(free_mem_start, free_mem_pages << EFI_PAGE_SHIFT);
- phys_alloc_set_minimum_alignment(SMP_CACHE_BYTES);
-
- phys_alloc_get_unused(&base, &top);
- base = PAGE_ALIGN(base);
- top = top & PAGE_MASK;
- assert(sizeof(long) == 8 || !(base >> 32));
- if (sizeof(long) != 8 && (top >> 32) != 0)
- top = ((uint64_t)1 << 32);
- page_alloc_init_area(0, base >> PAGE_SHIFT, top >> PAGE_SHIFT);
- page_alloc_ops_enable();
+ mem_allocator_init(free_mem_start, free_mem_start + (free_mem_pages << EFI_PAGE_SHIFT));
return EFI_SUCCESS;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 13/18] arm64: Simplify efi_mem_init
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (11 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 12/18] arm/arm64: Factor out allocator init from mem_init Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 8:10 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 14/18] arm64: Add memregions_efi_init Andrew Jones
` (4 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
Reduce the EFI mem_map loop to only setting flags and finding the
largest free memory region. Then, apply memregions_split() for
the code/data region split and do the rest of the things that
used to be done in the EFI mem_map loop in a separate mem_region
loop.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
lib/arm/setup.c | 45 ++++++++++++++++++++-------------------------
1 file changed, 20 insertions(+), 25 deletions(-)
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index d0be4c437708..631597b343f1 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -301,9 +301,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
struct efi_boot_memmap *map = &(efi_bootinfo->mem_map);
efi_memory_desc_t *buffer = *map->map;
efi_memory_desc_t *d = NULL;
- struct mem_region r;
- uintptr_t text = (uintptr_t)&_text, etext = ALIGN((uintptr_t)&_etext, 4096);
- uintptr_t data = (uintptr_t)&_data, edata = ALIGN((uintptr_t)&_edata, 4096);
+ struct mem_region r, *code, *data;
const void *fdt = efi_bootinfo->fdt;
/*
@@ -337,21 +335,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
r.flags = MR_F_IO;
break;
case EFI_LOADER_CODE:
- if (r.start <= text && r.end > text) {
- /* This is the unit test region. Flag the code separately. */
- phys_addr_t tmp = r.end;
-
- assert(etext <= data);
- assert(edata <= r.end);
- r.flags = MR_F_CODE;
- r.end = data;
- memregions_add(&r);
- r.start = data;
- r.end = tmp;
- r.flags = 0;
- } else {
- r.flags = MR_F_RESERVED;
- }
+ r.flags = MR_F_CODE;
break;
case EFI_CONVENTIONAL_MEMORY:
if (free_mem_pages < d->num_pages) {
@@ -361,15 +345,27 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
break;
}
- if (!(r.flags & MR_F_IO)) {
- if (r.start < __phys_offset)
- __phys_offset = r.start;
- if (r.end > __phys_end)
- __phys_end = r.end;
- }
memregions_add(&r);
}
+ memregions_split((unsigned long)&_etext, &code, &data);
+ assert(code && (code->flags & MR_F_CODE));
+ if (data)
+ data->flags &= ~MR_F_CODE;
+
+ for (struct mem_region *m = mem_regions; m->end; ++m) {
+ if (m != code && (m->flags & MR_F_CODE))
+ m->flags = MR_F_RESERVED;
+
+ if (!(m->flags & MR_F_IO)) {
+ if (m->start < __phys_offset)
+ __phys_offset = m->start;
+ if (m->end > __phys_end)
+ __phys_end = m->end;
+ }
+ }
+ __phys_end &= PHYS_MASK;
+
if (efi_bootinfo->fdt_valid) {
unsigned long old_start = free_mem_start;
void *freemem = (void *)free_mem_start;
@@ -380,7 +376,6 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
free_mem_pages = (free_mem_start - old_start) >> EFI_PAGE_SHIFT;
}
- __phys_end &= PHYS_MASK;
asm_mmu_disable();
if (free_mem_pages == 0)
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 14/18] arm64: Add memregions_efi_init
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (12 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 13/18] arm64: Simplify efi_mem_init Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 8:16 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 15/18] arm64: efi: Don't map reserved regions Andrew Jones
` (3 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
Provide a memregions function which initialized memregions from an
EFI memory map. Add a new memregions flag (MR_F_PERSISTENT) for
EFI_PERSISTENT_MEMORY since that type should not be reserved, but it
should also be distinct from conventional memory. The function also
points out the largest conventional memory region by returning a
pointer to it in the freemem parameter. Immediately apply this
function to arm64's efi_mem_init(). riscv will make use of it as well.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
lib/arm/setup.c | 76 ++++++++----------------------------------------
lib/memregions.c | 51 ++++++++++++++++++++++++++++++++
lib/memregions.h | 6 ++++
3 files changed, 69 insertions(+), 64 deletions(-)
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 631597b343f1..521928186fb0 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -295,58 +295,13 @@ static efi_status_t setup_rsdp(efi_bootinfo_t *efi_bootinfo)
static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
{
- int i;
- unsigned long free_mem_pages = 0;
- unsigned long free_mem_start = 0;
- struct efi_boot_memmap *map = &(efi_bootinfo->mem_map);
- efi_memory_desc_t *buffer = *map->map;
- efi_memory_desc_t *d = NULL;
- struct mem_region r, *code, *data;
- const void *fdt = efi_bootinfo->fdt;
-
- /*
- * Record the largest free EFI_CONVENTIONAL_MEMORY region
- * which will be used to set up the memory allocator, so that
- * the memory allocator can work in the largest free
- * continuous memory region.
- */
- for (i = 0; i < *(map->map_size); i += *(map->desc_size)) {
- d = (efi_memory_desc_t *)(&((u8 *)buffer)[i]);
-
- r.start = d->phys_addr;
- r.end = d->phys_addr + d->num_pages * EFI_PAGE_SIZE;
- r.flags = 0;
-
- switch (d->type) {
- case EFI_RESERVED_TYPE:
- case EFI_LOADER_DATA:
- case EFI_BOOT_SERVICES_CODE:
- case EFI_BOOT_SERVICES_DATA:
- case EFI_RUNTIME_SERVICES_CODE:
- case EFI_RUNTIME_SERVICES_DATA:
- case EFI_UNUSABLE_MEMORY:
- case EFI_ACPI_RECLAIM_MEMORY:
- case EFI_ACPI_MEMORY_NVS:
- case EFI_PAL_CODE:
- r.flags = MR_F_RESERVED;
- break;
- case EFI_MEMORY_MAPPED_IO:
- case EFI_MEMORY_MAPPED_IO_PORT_SPACE:
- r.flags = MR_F_IO;
- break;
- case EFI_LOADER_CODE:
- r.flags = MR_F_CODE;
- break;
- case EFI_CONVENTIONAL_MEMORY:
- if (free_mem_pages < d->num_pages) {
- free_mem_pages = d->num_pages;
- free_mem_start = d->phys_addr;
- }
- break;
- }
+ struct mem_region *freemem_mr = NULL, *code, *data;
+ phys_addr_t freemem_start;
+ void *freemem;
- memregions_add(&r);
- }
+ memregions_efi_init(&efi_bootinfo->mem_map, &freemem_mr);
+ if (!freemem_mr)
+ return EFI_OUT_OF_RESOURCES;
memregions_split((unsigned long)&_etext, &code, &data);
assert(code && (code->flags & MR_F_CODE));
@@ -366,24 +321,17 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
}
__phys_end &= PHYS_MASK;
- if (efi_bootinfo->fdt_valid) {
- unsigned long old_start = free_mem_start;
- void *freemem = (void *)free_mem_start;
+ freemem = (void *)PAGE_ALIGN(freemem_mr->start);
- freemem_push_fdt(&freemem, fdt);
+ if (efi_bootinfo->fdt_valid)
+ freemem_push_fdt(&freemem, efi_bootinfo->fdt);
- free_mem_start = ALIGN((unsigned long)freemem, EFI_PAGE_SIZE);
- free_mem_pages = (free_mem_start - old_start) >> EFI_PAGE_SHIFT;
- }
+ freemem_start = PAGE_ALIGN((unsigned long)freemem);
+ assert(sizeof(long) == 8 || freemem_start < (3ul << 30));
asm_mmu_disable();
- if (free_mem_pages == 0)
- return EFI_OUT_OF_RESOURCES;
-
- assert(sizeof(long) == 8 || free_mem_start < (3ul << 30));
-
- mem_allocator_init(free_mem_start, free_mem_start + (free_mem_pages << EFI_PAGE_SHIFT));
+ mem_allocator_init(freemem_start, freemem_mr->end);
return EFI_SUCCESS;
}
diff --git a/lib/memregions.c b/lib/memregions.c
index 96de86b27333..9cdbb639ab62 100644
--- a/lib/memregions.c
+++ b/lib/memregions.c
@@ -80,3 +80,54 @@ void memregions_add_dt_regions(size_t max_nr)
});
}
}
+
+#ifdef CONFIG_EFI
+/*
+ * Add memory regions based on the EFI memory map. Also set a pointer to the
+ * memory region which corresponds to the largest EFI_CONVENTIONAL_MEMORY
+ * region, as that region is the largest free, continuous region, making it
+ * a good choice for the memory allocator.
+ */
+void memregions_efi_init(struct efi_boot_memmap *mem_map,
+ struct mem_region **freemem)
+{
+ u8 *buffer = (u8 *)*mem_map->map;
+ u64 freemem_pages = 0;
+
+ *freemem = NULL;
+
+ for (int i = 0; i < *mem_map->map_size; i += *mem_map->desc_size) {
+ efi_memory_desc_t *d = (efi_memory_desc_t *)&buffer[i];
+ struct mem_region r = {
+ .start = d->phys_addr,
+ .end = d->phys_addr + d->num_pages * EFI_PAGE_SIZE,
+ .flags = 0,
+ };
+
+ switch (d->type) {
+ case EFI_MEMORY_MAPPED_IO:
+ case EFI_MEMORY_MAPPED_IO_PORT_SPACE:
+ r.flags = MR_F_IO;
+ break;
+ case EFI_LOADER_CODE:
+ r.flags = MR_F_CODE;
+ break;
+ case EFI_PERSISTENT_MEMORY:
+ r.flags = MR_F_PERSISTENT;
+ break;
+ case EFI_CONVENTIONAL_MEMORY:
+ if (freemem_pages < d->num_pages) {
+ freemem_pages = d->num_pages;
+ *freemem = memregions_add(&r);
+ continue;
+ }
+ break;
+ default:
+ r.flags = MR_F_RESERVED;
+ break;
+ }
+
+ memregions_add(&r);
+ }
+}
+#endif /* CONFIG_EFI */
diff --git a/lib/memregions.h b/lib/memregions.h
index 9a8e33182fe5..1600530ad7bf 100644
--- a/lib/memregions.h
+++ b/lib/memregions.h
@@ -9,6 +9,7 @@
#define MR_F_IO BIT(0)
#define MR_F_CODE BIT(1)
#define MR_F_RESERVED BIT(2)
+#define MR_F_PERSISTENT BIT(3)
#define MR_F_UNKNOWN BIT(31)
struct mem_region {
@@ -26,4 +27,9 @@ uint32_t memregions_get_flags(phys_addr_t paddr);
void memregions_split(phys_addr_t addr, struct mem_region **r1, struct mem_region **r2);
void memregions_add_dt_regions(size_t max_nr);
+#ifdef CONFIG_EFI
+#include <efi.h>
+void memregions_efi_init(struct efi_boot_memmap *mem_map, struct mem_region **freemem);
+#endif
+
#endif /* _MEMREGIONS_H_ */
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 15/18] arm64: efi: Don't map reserved regions
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (13 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 14/18] arm64: Add memregions_efi_init Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 8:18 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 16/18] arm64: efi: Fix _start returns from failed _relocate Andrew Jones
` (2 subsequent siblings)
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
We shouldn't need to map all the regions that the EFI memory map
contains. Just map EFI_LOADER_CODE and EFI_LOADER_DATA, since
those are for the loaded unit test, and any region types which
could be used by the unit test for its own memory allocations. We
still map EFI_BOOT_SERVICES_DATA since the primary stack is on a
region of that type. In a later patch we'll switch to a stack we
allocate ourselves to drop that one too.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
lib/arm/mmu.c | 6 +-----
lib/arm/setup.c | 4 ++--
lib/memregions.c | 8 ++++++++
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
index eb5e82a95f06..9dce7da85709 100644
--- a/lib/arm/mmu.c
+++ b/lib/arm/mmu.c
@@ -221,12 +221,8 @@ void *setup_mmu(phys_addr_t phys_end, void *unused)
mmu_idmap = alloc_page();
for (r = mem_regions; r->end; ++r) {
- if (r->flags & MR_F_IO) {
+ if (r->flags & (MR_F_IO | MR_F_RESERVED)) {
continue;
- } else if (r->flags & MR_F_RESERVED) {
- /* Reserved pages need to be writable for whatever reserved them */
- mmu_set_range_ptes(mmu_idmap, r->start, r->start, r->end,
- __pgprot(PTE_WBWA));
} else if (r->flags & MR_F_CODE) {
/* armv8 requires code shared between EL1 and EL0 to be read-only */
mmu_set_range_ptes(mmu_idmap, r->start, r->start, r->end,
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 521928186fb0..08658b9a222b 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -309,8 +309,8 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
data->flags &= ~MR_F_CODE;
for (struct mem_region *m = mem_regions; m->end; ++m) {
- if (m != code && (m->flags & MR_F_CODE))
- m->flags = MR_F_RESERVED;
+ if (m != code)
+ assert(!(m->flags & MR_F_CODE));
if (!(m->flags & MR_F_IO)) {
if (m->start < __phys_offset)
diff --git a/lib/memregions.c b/lib/memregions.c
index 9cdbb639ab62..3c6f751eb4f2 100644
--- a/lib/memregions.c
+++ b/lib/memregions.c
@@ -112,6 +112,14 @@ void memregions_efi_init(struct efi_boot_memmap *mem_map,
case EFI_LOADER_CODE:
r.flags = MR_F_CODE;
break;
+ case EFI_LOADER_DATA:
+ break;
+ case EFI_BOOT_SERVICES_DATA:
+ /*
+ * FIXME: This would ideally be MR_F_RESERVED, but the
+ * primary stack is in a region of this EFI type.
+ */
+ break;
case EFI_PERSISTENT_MEMORY:
r.flags = MR_F_PERSISTENT;
break;
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 16/18] arm64: efi: Fix _start returns from failed _relocate
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (14 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 15/18] arm64: efi: Don't map reserved regions Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 8:58 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 17/18] arm64: efi: Switch to our own stack Andrew Jones
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 18/18] arm64: efi: Add gitlab CI Andrew Jones
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
If _relocate fails we need to restore the frame pointer and the link
register and return from _start. But we've pushed x0 and x1 on below
the fp and lr, so, as the code was, we'd restore the wrong values.
Revert parts of the code back to the way they are in gnu-efi and move
the stack alignment below the loading of x0 and x1, after we've
confirmed _relocate didn't fail.
Fixes: d231b539a41f ("arm64: Use code from the gnu-efi when booting with EFI")
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
arm/efi/crt0-efi-aarch64.S | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/arm/efi/crt0-efi-aarch64.S b/arm/efi/crt0-efi-aarch64.S
index 5d0dc04af54a..5fd3dc94dae8 100644
--- a/arm/efi/crt0-efi-aarch64.S
+++ b/arm/efi/crt0-efi-aarch64.S
@@ -111,17 +111,10 @@ section_table:
.align 12
_start:
- stp x29, x30, [sp, #-16]!
-
- /* Align sp; this is necessary due to way we store cpu0's thread_info */
+ stp x29, x30, [sp, #-32]!
mov x29, sp
- mov x30, sp
- and x30, x30, #THREAD_MASK
- mov sp, x30
- str x29, [sp, #-16]!
-
- stp x0, x1, [sp, #-16]!
+ stp x0, x1, [sp, #16]
mov x2, x0
mov x3, x1
adr x0, ImageBase
@@ -130,12 +123,20 @@ _start:
bl _relocate
cbnz x0, 0f
- ldp x0, x1, [sp], #16
+ ldp x0, x1, [sp, #16]
+
+ /* Align sp; this is necessary due to way we store cpu0's thread_info */
+ mov x29, sp
+ mov x30, sp
+ and x30, x30, #THREAD_MASK
+ mov sp, x30
+ str x29, [sp, #-16]!
+
bl efi_main
/* Restore sp */
ldr x30, [sp], #16
- mov sp, x30
+ mov sp, x30
-0: ldp x29, x30, [sp], #16
+0: ldp x29, x30, [sp], #32
ret
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 17/18] arm64: efi: Switch to our own stack
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (15 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 16/18] arm64: efi: Fix _start returns from failed _relocate Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 9:03 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 18/18] arm64: efi: Add gitlab CI Andrew Jones
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
We don't want to map EFI_BOOT_SERVICES_DATA regions, so move the
stack from its EFI_BOOT_SERVICES_DATA region to EFI_LOADER_CODE,
which we always map. We'll still map the stack as R/W instead of
R/X because we split EFI_LOADER_CODE regions on the _etext boundary
and map addresses before _etext as R/X and the rest as R/W.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
arm/efi/crt0-efi-aarch64.S | 22 +++++++++++++++++-----
lib/arm/setup.c | 4 ----
lib/memregions.c | 6 ------
3 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/arm/efi/crt0-efi-aarch64.S b/arm/efi/crt0-efi-aarch64.S
index 5fd3dc94dae8..71ce2794f059 100644
--- a/arm/efi/crt0-efi-aarch64.S
+++ b/arm/efi/crt0-efi-aarch64.S
@@ -125,12 +125,18 @@ _start:
ldp x0, x1, [sp, #16]
- /* Align sp; this is necessary due to way we store cpu0's thread_info */
+ /*
+ * Switch to our own stack and align sp; this is necessary due
+ * to way we store cpu0's thread_info
+ */
+ adrp x2, stacktop
+ add x2, x2, :lo12:stacktop
+ and x2, x2, #THREAD_MASK
+ mov x3, sp
+ mov sp, x2
+ stp xzr, xzr, [sp, #-16]!
mov x29, sp
- mov x30, sp
- and x30, x30, #THREAD_MASK
- mov sp, x30
- str x29, [sp, #-16]!
+ str x3, [sp, #-16]!
bl efi_main
@@ -140,3 +146,9 @@ _start:
0: ldp x29, x30, [sp], #32
ret
+
+ .section .data
+
+.balign 65536
+.space 65536
+stacktop:
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 08658b9a222b..d535cec88709 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -340,10 +340,6 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
{
efi_status_t status;
- struct thread_info *ti = current_thread_info();
-
- memset(ti, 0, sizeof(*ti));
-
exceptions_init();
memregions_init(arm_mem_regions, NR_MEM_REGIONS);
diff --git a/lib/memregions.c b/lib/memregions.c
index 3c6f751eb4f2..53fc0c7cfc58 100644
--- a/lib/memregions.c
+++ b/lib/memregions.c
@@ -114,12 +114,6 @@ void memregions_efi_init(struct efi_boot_memmap *mem_map,
break;
case EFI_LOADER_DATA:
break;
- case EFI_BOOT_SERVICES_DATA:
- /*
- * FIXME: This would ideally be MR_F_RESERVED, but the
- * primary stack is in a region of this EFI type.
- */
- break;
case EFI_PERSISTENT_MEMORY:
r.flags = MR_F_PERSISTENT;
break;
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [kvm-unit-tests PATCH v2 18/18] arm64: efi: Add gitlab CI
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
` (16 preceding siblings ...)
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 17/18] arm64: efi: Switch to our own stack Andrew Jones
@ 2024-02-27 19:21 ` Andrew Jones
2024-03-04 9:06 ` Nikos Nikoleris
17 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-02-27 19:21 UTC (permalink / raw
To: kvm, kvmarm
Cc: alexandru.elisei, eric.auger, nikos.nikoleris, shahuang, pbonzini,
thuth
Now that we have efi-direct and tests run much faster, add a few
(just selftests) to the CI. Test with both DT and ACPI. While
touching the file update arm and arm64's pass/fail criteria to
the new style that ensures they're not all skips.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
.gitlab-ci.yml | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 71d986e9884e..ff34b1f5062e 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -44,7 +44,35 @@ build-aarch64:
selftest-vectors-user
timer
| tee results.txt
- - if grep -q FAIL results.txt ; then exit 1 ; fi
+ - grep -q PASS results.txt && ! grep -q FAIL results.txt
+
+build-aarch64-efi:
+ extends: .intree_template
+ script:
+ - dnf install -y qemu-system-aarch64 gcc-aarch64-linux-gnu edk2-aarch64
+ - ./configure --arch=aarch64 --cross-prefix=aarch64-linux-gnu- --enable-efi --enable-efi-direct
+ - make -j2
+ - ACCEL=tcg MAX_SMP=8 ./run_tests.sh
+ selftest-setup
+ selftest-smp
+ selftest-vectors-kernel
+ selftest-vectors-user
+ | tee results.txt
+ - grep -q PASS results.txt && ! grep -q FAIL results.txt
+
+build-aarch64-efi-acpi:
+ extends: .intree_template
+ script:
+ - dnf install -y qemu-system-aarch64 gcc-aarch64-linux-gnu edk2-aarch64
+ - ./configure --arch=aarch64 --cross-prefix=aarch64-linux-gnu- --enable-efi --enable-efi-direct
+ - make -j2
+ - EFI_USE_ACPI=y ACCEL=tcg MAX_SMP=8 ./run_tests.sh
+ selftest-setup
+ selftest-smp
+ selftest-vectors-kernel
+ selftest-vectors-user
+ | tee results.txt
+ - grep -q PASS results.txt && ! grep -q FAIL results.txt
build-arm:
extends: .outoftree_template
@@ -59,7 +87,7 @@ build-arm:
pci-test pmu-cycle-counter gicv2-ipi gicv2-mmio gicv3-ipi gicv2-active
gicv3-active
| tee results.txt
- - if grep -q FAIL results.txt ; then exit 1 ; fi
+ - grep -q PASS results.txt && ! grep -q FAIL results.txt
build-ppc64be:
extends: .outoftree_template
--
2.43.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 01/18] runtime: Update MAX_SMP probe
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 01/18] runtime: Update MAX_SMP probe Andrew Jones
@ 2024-03-03 21:43 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-03 21:43 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> Arm's MAX_SMP probing must have stopped working at some point due to
> QEMU's error message changing, but nobody noticed. Also, the probing
> should work for at least x86 now too, so the comment isn't correct
> anymore either. We could probably just delete this probe thing, but
> in case it could still serve some purpose we can also keep it, but
> updated for later QEMU, and only enabled when a new run_tests.sh
> command line option is provided.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
FWIW, probe_maxsmp doesn't have the expected outcome on MacOS. Not sure
why, but on my MBP (M1 Pro), HVF supports up to 64 vCPUs and machine
'virt-8.2' supports up to 512. Probably, this is another argument why
this should be optional.
Thanks,
Nikos
> ---
> run_tests.sh | 5 ++++-
> scripts/runtime.bash | 19 ++++++++++---------
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/run_tests.sh b/run_tests.sh
> index abb0ab773362..bb3024ff95b1 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -44,7 +44,7 @@ fi
>
> only_tests=""
> list_tests=""
> -args=$(getopt -u -o ag:htj:vl -l all,group:,help,tap13,parallel:,verbose,list -- $*)
> +args=$(getopt -u -o ag:htj:vl -l all,group:,help,tap13,parallel:,verbose,list,probe-maxsmp -- $*)
> [ $? -ne 0 ] && exit 2;
> set -- $args;
> while [ $# -gt 0 ]; do
> @@ -78,6 +78,9 @@ while [ $# -gt 0 ]; do
> -l | --list)
> list_tests="yes"
> ;;
> + --probe-maxsmp)
> + probe_maxsmp
> + ;;
> --)
> ;;
> *)
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index c73fb0240d12..f2e43bb1ed60 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -200,12 +200,13 @@ function run()
> #
> # Probe for MAX_SMP, in case it's less than the number of host cpus.
> #
> -# This probing currently only works for ARM, as x86 bails on another
> -# error first, so this check is only run for ARM and ARM64. The
> -# parameter expansion takes the last number from the QEMU error
> -# message, which gives the allowable MAX_SMP.
> -if [[ $ARCH == 'arm' || $ARCH == 'arm64' ]] &&
> - smp=$($RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP |& grep 'exceeds max CPUs'); then
> - smp=${smp##*(}
> - MAX_SMP=${smp:0:-1}
> -fi
> +function probe_maxsmp()
> +{
> + local smp
> +
> + if smp=$($RUNTIME_arch_run _NO_FILE_4Uhere_ -smp $MAX_SMP |& grep 'Invalid SMP CPUs'); then
> + smp=${smp##* }
> + echo "Restricting MAX_SMP from ($MAX_SMP) to the max supported ($smp)" >&2
> + MAX_SMP=$smp
> + fi
> +}
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 02/18] runtime: Add yet another 'no kernel' error message
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 02/18] runtime: Add yet another 'no kernel' error message Andrew Jones
@ 2024-03-03 21:50 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-03 21:50 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> When booting an Arm machine with the -bios command line option we
> get yet another error message from QEMU when using _NO_FILE_4Uhere_
> to probe command line support. Add it to the check in
> premature_failure()
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> ---
> scripts/runtime.bash | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index f2e43bb1ed60..255e756f2cb2 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -18,7 +18,7 @@ premature_failure()
> local log="$(eval "$(get_cmdline _NO_FILE_4Uhere_)" 2>&1)"
>
> echo "$log" | grep "_NO_FILE_4Uhere_" |
> - grep -q -e "could not \(load\|open\) kernel" -e "error loading" &&
> + grep -q -e "could not \(load\|open\) kernel" -e "error loading" -e "failed to load" &&
> return 1
>
> RUNTIME_log_stderr <<< "$log"
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 03/18] arm64: efi: Don't create dummy test
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 03/18] arm64: efi: Don't create dummy test Andrew Jones
@ 2024-03-03 21:57 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-03 21:57 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> The purpose of the _NO_FILE_4Uhere_ kernel is to check that all the
> QEMU command line options that have been pulled together by the
> scripts will work. Since booting with UEFI and the -kernel command
> line is supported by QEMU, then we don't need to create a dummy
> test for _NO_FILE_4Uhere_ and go all the way into UEFI's shell and
> execute it to prove the command line is OK, since we would have
> failed much before all that if it wasn't. Just run QEMU "normally",
> i.e. no EFI_RUN=y, but add the UEFI -bios and its file system command
> line options, in order to check the full command line.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> ---
> arm/efi/run | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arm/efi/run b/arm/efi/run
> index 6872c337c945..e629abde5273 100755
> --- a/arm/efi/run
> +++ b/arm/efi/run
> @@ -53,7 +53,14 @@ while (( "$#" )); do
> done
>
> if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then
> - EFI_CASE=dummy
> + EFI_CASE_DIR="$EFI_TEST/dummy"
> + mkdir -p "$EFI_CASE_DIR"
> + $TEST_DIR/run \
> + $EFI_CASE \
> + -bios "$EFI_UEFI" \
> + -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
> + "${qemu_args[@]}"
> + exit
> fi
>
> : "${EFI_CASE_DIR:="$EFI_TEST/$EFI_CASE"}"
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 04/18] arm64: efi: Make running tests on EFI can be parallel
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 04/18] arm64: efi: Make running tests on EFI can be parallel Andrew Jones
@ 2024-03-03 22:06 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-03 22:06 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> From: Shaoqin Huang <shahuang@redhat.com>
>
> Currently running tests on EFI in parallel can cause part of tests to
> fail, this is because arm/efi/run script use the EFI_CASE to create the
> subdir under the efi-tests, and the EFI_CASE is the filename of the
> test, when running tests in parallel, the multiple tests exist in the
> same filename will execute at the same time, which will use the same
> directory and write the test specific things into it, this cause
> chaotic and make some tests fail.
>
> For example, if we running the pmu-sw-incr and pmu-chained-counters
> and other pmu tests on EFI at the same time, the EFI_CASE will be pmu.
> So they will write their $cmd_args to the $EFI/TEST/pmu/startup.nsh
> at the same time, which will corrupt the startup.nsh file.
>
> And we can get the log which outputs:
>
> * pmu-sw-incr.log:
> - ABORT: pmu: Unknown sub-test 'pmu-mem-acce'
> * pmu-chained-counters.log
> - ABORT: pmu: Unknown sub-test 'pmu-mem-access-reliab'
>
> And the efi-tests/pmu/startup.nsh:
>
> @echo -off
> setvar fdtfile -guid 97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823 -rt =L"dtb"
> pmu.efi pmu-mem-access-reliability
> setvar fdtfile -guid 97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823 -rt =L"dtb"
> pmu.efi pmu-chained-sw-incr
>
> As you can see, when multiple tests write to the same startup.nsh file,
> it causes the issue.
>
> To Fix this issue, use the testname instead of the filename to create
> the subdir under the efi-tests. We use the EFI_TESTNAME to replace the
> EFI_CASE in script. Since every testname is specific, now the tests
> can be run parallel. It also considers when user directly use the
> arm/efi/run to run test, in this case, still use the filename.
>
> Besides, replace multiple $EFI_TEST/$EFI_CASE to the $EFI_CASE_DIR, this
> makes the script looks more clean and we don'e need to replace many
> EFI_CASE to EFI_TESTNAME.
>
> Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> ---
> arm/efi/run | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arm/efi/run b/arm/efi/run
> index e629abde5273..8b6512520026 100755
> --- a/arm/efi/run
> +++ b/arm/efi/run
> @@ -25,6 +25,8 @@ fi
> : "${EFI_UEFI:=$DEFAULT_UEFI}"
> : "${EFI_TEST:=efi-tests}"
> : "${EFI_CASE:=$(basename $1 .efi)}"
> +: "${EFI_TESTNAME:=$TESTNAME}"
> +: "${EFI_TESTNAME:=$EFI_CASE}"
> : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}"
>
> [ "$EFI_USE_ACPI" = "y" ] || EFI_USE_DTB=y
> @@ -63,20 +65,20 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then
> exit
> fi
>
> -: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_CASE"}"
> +: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}"
> mkdir -p "$EFI_CASE_DIR"
>
> -cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_TEST/$EFI_CASE/"
> -echo "@echo -off" > "$EFI_TEST/$EFI_CASE/startup.nsh"
> +cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/"
> +echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh"
> if [ "$EFI_USE_DTB" = "y" ]; then
> qemu_args+=(-machine acpi=off)
> FDT_BASENAME="dtb"
> - $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_TEST/$EFI_CASE/$FDT_BASENAME" "${qemu_args[@]}")
> - echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_TEST/$EFI_CASE/startup.nsh"
> + $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}")
> + echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_CASE_DIR/startup.nsh"
> fi
> -echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_TEST/$EFI_CASE/startup.nsh"
> +echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_CASE_DIR/startup.nsh"
>
> EFI_RUN=y $TEST_DIR/run \
> -bios "$EFI_UEFI" \
> - -drive file.dir="$EFI_TEST/$EFI_CASE/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
> + -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
> "${qemu_args[@]}"
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 05/18] arm64: efi: Remove redundant dtb generation
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 05/18] arm64: efi: Remove redundant dtb generation Andrew Jones
@ 2024-03-04 7:16 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 7:16 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> When a line in bash is written as
>
> $(some-line)
>
> Then 'some-line' will be evaluated and then whatever some-line outputs
> will be evaluated. The dtb is getting generated twice since the line
> that should generate it is within $() and the output of that is the
> command itself (since arm/run outputs its command), so the command
> gets executed again. Remove the $() to just execute dtb generation
> once.
My bad, don't know what I was thinking.
>
> While mucking with arm/efi/run tidy it a bit by by removing the unused
> sourcing of common.bash and the unnecessary 'set -e' (we check for and
> propagate errors ourselves). Finally, make one reorganization change
> and some whitespace fixes.
>
> Fixes: 2607d2d6946a ("arm64: Add an efi/run script")
> Fixes: 2e080dafec2a ("arm64: Use the provided fdt when booting through EFI")
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> arm/efi/run | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/arm/efi/run b/arm/efi/run
> index 8b6512520026..e45cecfa3265 100755
> --- a/arm/efi/run
> +++ b/arm/efi/run
> @@ -1,7 +1,5 @@
> #!/bin/bash
>
> -set -e
> -
> if [ $# -eq 0 ]; then
> echo "Usage $0 TEST_CASE [QEMU_ARGS]"
> exit 2
> @@ -13,7 +11,6 @@ if [ ! -f config.mak ]; then
> fi
> source config.mak
> source scripts/arch-run.bash
> -source scripts/common.bash
>
> if [ -f /usr/share/qemu-efi-aarch64/QEMU_EFI.fd ]; then
> DEFAULT_UEFI=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd
> @@ -27,6 +24,7 @@ fi
> : "${EFI_CASE:=$(basename $1 .efi)}"
> : "${EFI_TESTNAME:=$TESTNAME}"
> : "${EFI_TESTNAME:=$EFI_CASE}"
> +: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}"
> : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}"
>
> [ "$EFI_USE_ACPI" = "y" ] || EFI_USE_DTB=y
> @@ -65,20 +63,18 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then
> exit
> fi
>
> -: "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}"
> mkdir -p "$EFI_CASE_DIR"
> -
> cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/"
> echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh"
> if [ "$EFI_USE_DTB" = "y" ]; then
> qemu_args+=(-machine acpi=off)
> FDT_BASENAME="dtb"
> - $(EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}")
> + EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}"
> echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_CASE_DIR/startup.nsh"
> fi
> echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_CASE_DIR/startup.nsh"
>
> EFI_RUN=y $TEST_DIR/run \
> - -bios "$EFI_UEFI" \
> - -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
> - "${qemu_args[@]}"
> + -bios "$EFI_UEFI" \
> + -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
> + "${qemu_args[@]}"
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 06/18] arm64: efi: Move run code into a function
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 06/18] arm64: efi: Move run code into a function Andrew Jones
@ 2024-03-04 7:19 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 7:19 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> Push the run code in arm/efi/run into a function named
> uefi_shell_run() since it will create an EFI file system, copy
> the test and possibly the DTB there, and create a startup.nsh
> which executes the test from the UEFI shell. Pushing this
> code into a function allows additional execution paths to be
> created in the script. Also rename EFI_RUN to UEFI_SHELL_RUN
> to pass the information on to arm/run that it's being called
> from uefi_shell_run().
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> arm/efi/run | 33 +++++++++++++++++++--------------
> arm/run | 4 ++--
> 2 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/arm/efi/run b/arm/efi/run
> index e45cecfa3265..494ba9e7efe7 100755
> --- a/arm/efi/run
> +++ b/arm/efi/run
> @@ -63,18 +63,23 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then
> exit
> fi
>
> -mkdir -p "$EFI_CASE_DIR"
> -cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/"
> -echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh"
> -if [ "$EFI_USE_DTB" = "y" ]; then
> - qemu_args+=(-machine acpi=off)
> - FDT_BASENAME="dtb"
> - EFI_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}"
> - echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_CASE_DIR/startup.nsh"
> -fi
> -echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_CASE_DIR/startup.nsh"
> +uefi_shell_run()
> +{
> + mkdir -p "$EFI_CASE_DIR"
> + cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/"
> + echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh"
> + if [ "$EFI_USE_DTB" = "y" ]; then
> + qemu_args+=(-machine acpi=off)
> + FDT_BASENAME="dtb"
> + UEFI_SHELL_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}"
> + echo "setvar fdtfile -guid $EFI_VAR_GUID -rt =L\"$FDT_BASENAME\"" >> "$EFI_CASE_DIR/startup.nsh"
> + fi
> + echo "$EFI_CASE.efi" "${cmd_args[@]}" >> "$EFI_CASE_DIR/startup.nsh"
> +
> + UEFI_SHELL_RUN=y $TEST_DIR/run \
> + -bios "$EFI_UEFI" \
> + -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
> + "${qemu_args[@]}"
> +}
>
> -EFI_RUN=y $TEST_DIR/run \
> - -bios "$EFI_UEFI" \
> - -drive file.dir="$EFI_CASE_DIR/",file.driver=vvfat,file.rw=on,format=raw,if=virtio \
> - "${qemu_args[@]}"
> +uefi_shell_run
> diff --git a/arm/run b/arm/run
> index ac64b3b461a2..40c2ca66ba7e 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -60,7 +60,7 @@ if ! $qemu $M -chardev '?' | grep -q testdev; then
> exit 2
> fi
>
> -if [ "$EFI_RUN" != "y" ]; then
> +if [ "$UEFI_SHELL_RUN" != "y" ]; then
> chr_testdev='-device virtio-serial-device'
> chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
> fi
> @@ -75,7 +75,7 @@ command="$qemu -nodefaults $M $A -cpu $processor $chr_testdev $pci_testdev"
> command+=" -display none -serial stdio"
> command="$(migration_cmd) $(timeout_cmd) $command"
>
> -if [ "$EFI_RUN" = "y" ]; then
> +if [ "$UEFI_SHELL_RUN" = "y" ]; then
> ENVIRON_DEFAULT=n run_qemu_status $command "$@"
> else
> run_qemu $command -kernel "$@"
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 07/18] arm64: efi: Remove EFI_USE_DTB
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 07/18] arm64: efi: Remove EFI_USE_DTB Andrew Jones
@ 2024-03-04 7:20 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 7:20 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> We don't need two variables for one boolean property. Just use
> !EFI_USE_ACPI to infer efi-use-dtb.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> arm/efi/run | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arm/efi/run b/arm/efi/run
> index 494ba9e7efe7..b7a8418a07f8 100755
> --- a/arm/efi/run
> +++ b/arm/efi/run
> @@ -27,8 +27,6 @@ fi
> : "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}"
> : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}"
>
> -[ "$EFI_USE_ACPI" = "y" ] || EFI_USE_DTB=y
> -
> if [ ! -f "$EFI_UEFI" ]; then
> echo "UEFI firmware not found."
> echo "Please specify the path with the env variable EFI_UEFI"
> @@ -68,7 +66,7 @@ uefi_shell_run()
> mkdir -p "$EFI_CASE_DIR"
> cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_DIR/"
> echo "@echo -off" > "$EFI_CASE_DIR/startup.nsh"
> - if [ "$EFI_USE_DTB" = "y" ]; then
> + if [ "$EFI_USE_ACPI" != "y" ]; then
> qemu_args+=(-machine acpi=off)
> FDT_BASENAME="dtb"
> UEFI_SHELL_RUN=y $TEST_DIR/run -machine dumpdtb="$EFI_CASE_DIR/$FDT_BASENAME" "${qemu_args[@]}"
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 08/18] arm64: efi: Improve device tree discovery
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 08/18] arm64: efi: Improve device tree discovery Andrew Jones
@ 2024-03-04 7:34 ` Nikos Nikoleris
2024-03-04 9:35 ` Andrew Jones
0 siblings, 1 reply; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 7:34 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> Zero is a valid address for the device tree so add an fdt_valid data
> member to determine when the address is valid or not. Also, check the
> device tree GUID when the environment variable is missing. The latter
> change allows directly loading the unit test with QEMU's '-kernel'
> command line parameter, which is much faster than putting the test
> in the EFI file system and then running it from the UEFI shell.
>
Out of curiosity, the fdt pointer can be zero just in KUT or zero is an
address that efi_load_image or efi_get_system_config_table could return?
Similar code in Linux treats 0 an non valid address
https://elixir.bootlin.com/linux/latest/source/drivers/firmware/efi/libstub/fdt.c#L370
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
In any case, this won't hurt:
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> lib/arm/setup.c | 3 ++-
> lib/efi.c | 28 +++++++++++++++++-----------
> lib/efi.h | 3 ++-
> lib/linux/efi.h | 2 ++
> 4 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 0382cbdaf5a1..76aae4627a7b 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -342,7 +342,8 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> }
> memregions_add(&r);
> }
> - if (fdt) {
> +
> + if (efi_bootinfo->fdt_valid) {
> /* Move the FDT to the base of free memory */
> fdt_size = fdt_totalsize(fdt);
> ret = fdt_move(fdt, (void *)free_mem_start, fdt_size);
> diff --git a/lib/efi.c b/lib/efi.c
> index d94f0fa16fc0..0785bd3e8916 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -6,13 +6,13 @@
> *
> * SPDX-License-Identifier: LGPL-2.0-or-later
> */
> -
> -#include "efi.h"
> +#include <libcflat.h>
> #include <argv.h>
> -#include <stdlib.h>
> #include <ctype.h>
> -#include <libcflat.h>
> +#include <stdlib.h>
> #include <asm/setup.h>
> +#include "efi.h"
> +#include "libfdt/libfdt.h"
>
> /* From lib/argv.c */
> extern int __argc, __envc;
> @@ -283,18 +283,24 @@ static void* efi_get_var(efi_handle_t handle, struct efi_loaded_image_64 *image,
> return val;
> }
>
> -static void *efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image)
> +static bool efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image, void **fdt)
> {
> efi_char16_t var[] = ENV_VARNAME_DTBFILE;
> efi_char16_t *val;
> - void *fdt = NULL;
> - int fdtsize;
> + int fdtsize = 0;
> +
> + *fdt = NULL;
>
> val = efi_get_var(handle, image, var);
> - if (val)
> - efi_load_image(handle, image, &fdt, &fdtsize, val);
> + if (val) {
> + efi_load_image(handle, image, fdt, &fdtsize, val);
> + if (fdtsize == 0)
> + return false;
> + } else if (efi_get_system_config_table(DEVICE_TREE_GUID, fdt) != EFI_SUCCESS) {
> + return false;
> + }
>
> - return fdt;
> + return fdt_check_header(*fdt) == 0;
> }
>
> efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> @@ -335,7 +341,7 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> }
> setup_args(cmdline_ptr);
>
> - efi_bootinfo.fdt = efi_get_fdt(handle, image);
> + efi_bootinfo.fdt_valid = efi_get_fdt(handle, image, &efi_bootinfo.fdt);
> /* Set up efi_bootinfo */
> efi_bootinfo.mem_map.map = ↦
> efi_bootinfo.mem_map.map_size = &map_size;
> diff --git a/lib/efi.h b/lib/efi.h
> index db46d45068ee..4bd01f7199ce 100644
> --- a/lib/efi.h
> +++ b/lib/efi.h
> @@ -30,7 +30,8 @@
> */
> typedef struct {
> struct efi_boot_memmap mem_map;
> - const void *fdt;
> + void *fdt;
> + bool fdt_valid;
> } efi_bootinfo_t;
>
> efi_status_t _relocate(long ldbase, Elf64_Dyn *dyn, efi_handle_t handle,
> diff --git a/lib/linux/efi.h b/lib/linux/efi.h
> index 410f0b1a0da1..92d798f79767 100644
> --- a/lib/linux/efi.h
> +++ b/lib/linux/efi.h
> @@ -66,6 +66,8 @@ typedef guid_t efi_guid_t;
> #define ACPI_TABLE_GUID EFI_GUID(0xeb9d2d30, 0x2d88, 0x11d3, 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
> #define ACPI_20_TABLE_GUID EFI_GUID(0x8868e871, 0xe4f1, 0x11d3, 0xbc, 0x22, 0x00, 0x80, 0xc7, 0x3c, 0x88, 0x81)
>
> +#define DEVICE_TREE_GUID EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
> +
> #define LOADED_IMAGE_PROTOCOL_GUID EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>
> typedef struct {
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 09/18] lib/efi: Add support for loading the initrd
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 09/18] lib/efi: Add support for loading the initrd Andrew Jones
@ 2024-03-04 7:44 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 7:44 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> When loading non-efi tests with QEMU's '-kernel' option we also load
> an environ with the '-initrd' option. Now that efi tests can also be
> loaded with the '-kernel' option also provide the '-initrd' environ.
> For EFI, we use the EFI_LOAD_FILE2_PROTOCOL_GUID protocol to load
> LINUX_EFI_INITRD_MEDIA_GUID. Each architecture which wants to use the
> initrd for the environ will need to call setup_env() on the initrd
> data. As usual, the new efi function is heavily influenced by Linux's
> implementation.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> lib/efi.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++
> lib/linux/efi.h | 27 ++++++++++++++++++++
> 2 files changed, 92 insertions(+)
>
> diff --git a/lib/efi.c b/lib/efi.c
> index 0785bd3e8916..edfcc80ef114 100644
> --- a/lib/efi.c
> +++ b/lib/efi.c
> @@ -14,6 +14,10 @@
> #include "efi.h"
> #include "libfdt/libfdt.h"
>
> +/* From each arch */
> +extern char *initrd;
> +extern u32 initrd_size;
> +
> /* From lib/argv.c */
> extern int __argc, __envc;
> extern char *__argv[100];
> @@ -303,6 +307,65 @@ static bool efi_get_fdt(efi_handle_t handle, struct efi_loaded_image_64 *image,
> return fdt_check_header(*fdt) == 0;
> }
>
> +static const struct {
> + struct efi_vendor_dev_path vendor;
> + struct efi_generic_dev_path end;
> +} __packed initrd_dev_path = {
> + {
> + {
> + EFI_DEV_MEDIA,
> + EFI_DEV_MEDIA_VENDOR,
> + sizeof(struct efi_vendor_dev_path),
> + },
> + LINUX_EFI_INITRD_MEDIA_GUID
> + }, {
> + EFI_DEV_END_PATH,
> + EFI_DEV_END_ENTIRE,
> + sizeof(struct efi_generic_dev_path)
> + }
> +};
> +
> +static void efi_load_initrd(void)
> +{
> + efi_guid_t lf2_proto_guid = EFI_LOAD_FILE2_PROTOCOL_GUID;
> + efi_device_path_protocol_t *dp;
> + efi_load_file2_protocol_t *lf2;
> + efi_handle_t handle;
> + efi_status_t status;
> + unsigned long file_size = 0;
> +
> + initrd = NULL;
> + initrd_size = 0;
> +
> + dp = (efi_device_path_protocol_t *)&initrd_dev_path;
> + status = efi_bs_call(locate_device_path, &lf2_proto_guid, &dp, &handle);
> + if (status != EFI_SUCCESS)
> + return;
> +
> + status = efi_bs_call(handle_protocol, handle, &lf2_proto_guid, (void **)&lf2);
> + assert(status == EFI_SUCCESS);
> +
> + status = efi_call_proto(lf2, load_file, dp, false, &file_size, NULL);
> + assert(status == EFI_BUFFER_TOO_SMALL);
> +
> + status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, file_size, (void **)&initrd);
> + assert(status == EFI_SUCCESS);
> +
> + status = efi_call_proto(lf2, load_file, dp, false, &file_size, (void *)initrd);
> + assert(status == EFI_SUCCESS);
> +
> + initrd_size = (u32)file_size;
> +
> + /*
> + * UEFI appends initrd=initrd to the command line when an initrd is present.
> + * Remove it in order to avoid confusing unit tests.
> + */
> + if (!strcmp(__argv[__argc - 1], "initrd=initrd")) {
> + __argv[__argc - 1] = NULL;
> + __argc -= 1;
> + }
> +}
> +
> efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> {
> int ret;
> @@ -341,6 +404,8 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> }
> setup_args(cmdline_ptr);
>
> + efi_load_initrd();
> +
> efi_bootinfo.fdt_valid = efi_get_fdt(handle, image, &efi_bootinfo.fdt);
> /* Set up efi_bootinfo */
> efi_bootinfo.mem_map.map = ↦
> diff --git a/lib/linux/efi.h b/lib/linux/efi.h
> index 92d798f79767..8fa23ad078ce 100644
> --- a/lib/linux/efi.h
> +++ b/lib/linux/efi.h
> @@ -70,6 +70,9 @@ typedef guid_t efi_guid_t;
>
> #define LOADED_IMAGE_PROTOCOL_GUID EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>
> +#define EFI_LOAD_FILE2_PROTOCOL_GUID EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)
> +#define LINUX_EFI_INITRD_MEDIA_GUID EFI_GUID(0x5568e427, 0x68fc, 0x4f3d, 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
> +
> typedef struct {
> efi_guid_t guid;
> void *table;
> @@ -248,6 +251,12 @@ struct efi_generic_dev_path {
> u16 length;
> } __packed;
>
> +struct efi_vendor_dev_path {
> + struct efi_generic_dev_path header;
> + efi_guid_t vendorguid;
> + u8 vendordata[];
> +} __packed;
> +
> typedef struct efi_generic_dev_path efi_device_path_protocol_t;
>
> /*
> @@ -449,6 +458,19 @@ typedef struct _efi_simple_file_system_protocol efi_simple_file_system_protocol_
> typedef struct _efi_file_protocol efi_file_protocol_t;
> typedef efi_simple_file_system_protocol_t efi_file_io_interface_t;
> typedef efi_file_protocol_t efi_file_t;
> +typedef union efi_load_file_protocol efi_load_file_protocol_t;
> +typedef union efi_load_file_protocol efi_load_file2_protocol_t;
> +
> +union efi_load_file_protocol {
> + struct {
> + efi_status_t (__efiapi *load_file)(efi_load_file_protocol_t *,
> + efi_device_path_protocol_t *,
> + bool, unsigned long *, void *);
> + };
> + struct {
> + u32 load_file;
> + } mixed_mode;
> +};
>
> typedef efi_status_t efi_simple_file_system_protocol_open_volume(
> efi_simple_file_system_protocol_t *this,
> @@ -544,7 +566,12 @@ typedef struct {
> efi_char16_t file_name[1];
> } efi_file_info_t;
>
> +#define efi_fn_call(inst, func, ...) (inst)->func(__VA_ARGS__)
> #define efi_bs_call(func, ...) efi_system_table->boottime->func(__VA_ARGS__)
> #define efi_rs_call(func, ...) efi_system_table->runtime->func(__VA_ARGS__)
> +#define efi_call_proto(inst, func, ...) ({ \
> + __typeof__(inst) __inst = (inst); \
> + efi_fn_call(__inst, func, __inst, ##__VA_ARGS__); \
> +})
>
> #endif /* __LINUX_UEFI_H */
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 10/18] arm64: efi: Allow running tests directly
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 10/18] arm64: efi: Allow running tests directly Andrew Jones
@ 2024-03-04 7:52 ` Nikos Nikoleris
2024-03-04 9:43 ` Andrew Jones
0 siblings, 1 reply; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 7:52 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> Since it's possible to run tests with UEFI and the QEMU -kernel
> option (and now the DTB will be found and even the environ will
> be set up from an initrd if given with the -initrd option), then
> we can skip the loading of EFI tests into a file system and booting
> to the shell to run them. Just run them directly. Running directly
> is waaaaaay faster than booting the shell first. We keep the UEFI
> shell as the default behavior, though, and provide a new configure
> option to enable the direct running.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Just a minor nit, see below, but in any case:
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> ---
> arm/efi/run | 17 +++++++++++++++--
> arm/run | 4 +++-
> configure | 17 +++++++++++++++++
> 3 files changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/arm/efi/run b/arm/efi/run
> index b7a8418a07f8..af7b593c2bb8 100755
> --- a/arm/efi/run
> +++ b/arm/efi/run
> @@ -18,10 +18,12 @@ elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
> DEFAULT_UEFI=/usr/share/edk2/aarch64/QEMU_EFI.silent.fd
> fi
>
> +KERNEL_NAME=$1
> +
> : "${EFI_SRC:=$TEST_DIR}"
> : "${EFI_UEFI:=$DEFAULT_UEFI}"
> : "${EFI_TEST:=efi-tests}"
> -: "${EFI_CASE:=$(basename $1 .efi)}"
> +: "${EFI_CASE:=$(basename $KERNEL_NAME .efi)}"
> : "${EFI_TESTNAME:=$TESTNAME}"
> : "${EFI_TESTNAME:=$EFI_CASE}"
> : "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}"
> @@ -80,4 +82,15 @@ uefi_shell_run()
> "${qemu_args[@]}"
> }
>
> -uefi_shell_run
> +if [ "$EFI_DIRECT" = "y" ]; then
> + if [ "$EFI_USE_ACPI" != "y" ]; then
> + qemu_args+=(-machine acpi=off)
> + fi
The if statement above is common for the efi and efi_direct paths. You
could also move it above to avoid the code replication.
Thanks,
Nikos
> + $TEST_DIR/run \
> + $KERNEL_NAME \
> + -append "$(basename $KERNEL_NAME) ${cmd_args[@]}" \
> + -bios "$EFI_UEFI" \
> + "${qemu_args[@]}"
> +else
> + uefi_shell_run
> +fi
> diff --git a/arm/run b/arm/run
> index 40c2ca66ba7e..efdd44ce86a7 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -60,7 +60,7 @@ if ! $qemu $M -chardev '?' | grep -q testdev; then
> exit 2
> fi
>
> -if [ "$UEFI_SHELL_RUN" != "y" ]; then
> +if [ "$UEFI_SHELL_RUN" != "y" ] && [ "$EFI_USE_ACPI" != "y" ]; then
> chr_testdev='-device virtio-serial-device'
> chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
> fi
> @@ -77,6 +77,8 @@ command="$(migration_cmd) $(timeout_cmd) $command"
>
> if [ "$UEFI_SHELL_RUN" = "y" ]; then
> ENVIRON_DEFAULT=n run_qemu_status $command "$@"
> +elif [ "$EFI_USE_ACPI" = "y" ]; then
> + run_qemu_status $command -kernel "$@"
> else
> run_qemu $command -kernel "$@"
> fi
> diff --git a/configure b/configure
> index 05e6702eab06..283c959973fd 100755
> --- a/configure
> +++ b/configure
> @@ -32,6 +32,7 @@ enable_dump=no
> page_size=
> earlycon=
> efi=
> +efi_direct=
>
> # Enable -Werror by default for git repositories only (i.e. developer builds)
> if [ -e "$srcdir"/.git ]; then
> @@ -89,6 +90,11 @@ usage() {
> --[enable|disable]-efi Boot and run from UEFI (disabled by default, x86_64 and arm64 only)
> --[enable|disable]-werror
> Select whether to compile with the -Werror compiler flag
> + --[enable|disable]-efi-direct
> + Select whether to run EFI tests directly with QEMU's -kernel
> + option. When not enabled, tests will be placed in an EFI file
> + system and run from the UEFI shell. Ignored when efi isn't enabled.
> + (arm64 only)
> EOF
> exit 1
> }
> @@ -168,6 +174,12 @@ while [[ "$1" = -* ]]; do
> --disable-efi)
> efi=n
> ;;
> + --enable-efi-direct)
> + efi_direct=y
> + ;;
> + --disable-efi-direct)
> + efi_direct=n
> + ;;
> --enable-werror)
> werror=-Werror
> ;;
> @@ -185,6 +197,10 @@ while [[ "$1" = -* ]]; do
> esac
> done
>
> +if [ -z "$efi" ] || [ "$efi" = "n" ]; then
> + [ "$efi_direct" = "y" ] && efi_direct=
> +fi
> +
> if [ -n "$host_key_document" ] && [ ! -f "$host_key_document" ]; then
> echo "Host key document doesn't exist at the specified location."
> exit 1
> @@ -423,6 +439,7 @@ GENPROTIMG=${GENPROTIMG-genprotimg}
> HOST_KEY_DOCUMENT=$host_key_document
> CONFIG_DUMP=$enable_dump
> CONFIG_EFI=$efi
> +EFI_DIRECT=$efi_direct
> CONFIG_WERROR=$werror
> GEN_SE_HEADER=$gen_se_header
> EOF
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 11/18] arm/arm64: Factor out some initial setup
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 11/18] arm/arm64: Factor out some initial setup Andrew Jones
@ 2024-03-04 7:59 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 7:59 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> Factor out some initial setup code into separate functions in order
> to share more code between setup() and setup_efi().
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> lib/arm/setup.c | 81 ++++++++++++++++++++++++++++---------------------
> 1 file changed, 47 insertions(+), 34 deletions(-)
>
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 76aae4627a7b..f96ee04ddd68 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -182,32 +182,57 @@ static void mem_init(phys_addr_t freemem_start)
> page_alloc_ops_enable();
> }
>
> -void setup(const void *fdt, phys_addr_t freemem_start)
> +static void freemem_push_fdt(void **freemem, const void *fdt)
> {
> - void *freemem;
> - const char *bootargs, *tmp;
> u32 fdt_size;
> int ret;
>
> - assert(sizeof(long) == 8 || freemem_start < (3ul << 30));
> - freemem = (void *)(unsigned long)freemem_start;
> -
> - /* Move the FDT to the base of free memory */
> fdt_size = fdt_totalsize(fdt);
> - ret = fdt_move(fdt, freemem, fdt_size);
> + ret = fdt_move(fdt, *freemem, fdt_size);
> assert(ret == 0);
> - ret = dt_init(freemem);
> + ret = dt_init(*freemem);
> assert(ret == 0);
> - freemem += fdt_size;
> + *freemem += fdt_size;
> +}
> +
> +static void freemem_push_dt_initrd(void **freemem)
> +{
> + const char *tmp;
> + int ret;
>
> - /* Move the initrd to the top of the FDT */
> ret = dt_get_initrd(&tmp, &initrd_size);
> assert(ret == 0 || ret == -FDT_ERR_NOTFOUND);
> if (ret == 0) {
> - initrd = freemem;
> + initrd = *freemem;
> memmove(initrd, tmp, initrd_size);
> - freemem += initrd_size;
> + *freemem += initrd_size;
> }
> +}
> +
> +static void initrd_setup(void)
> +{
> + char *env;
> +
> + if (!initrd)
> + return;
> +
> + /* environ is currently the only file in the initrd */
> + env = malloc(initrd_size);
> + memcpy(env, initrd, initrd_size);
> + setup_env(env, initrd_size);
> +}
> +
> +void setup(const void *fdt, phys_addr_t freemem_start)
> +{
> + void *freemem;
> + const char *bootargs;
> + int ret;
> +
> + assert(sizeof(long) == 8 || freemem_start < (3ul << 30));
> + freemem = (void *)(unsigned long)freemem_start;
> +
> + freemem_push_fdt(&freemem, fdt);
> + freemem_push_dt_initrd(&freemem);
>
> memregions_init(arm_mem_regions, NR_MEM_REGIONS);
> memregions_add_dt_regions(MAX_DT_MEM_REGIONS);
> @@ -229,12 +254,7 @@ void setup(const void *fdt, phys_addr_t freemem_start)
> assert(ret == 0 || ret == -FDT_ERR_NOTFOUND);
> setup_args_progname(bootargs);
>
> - if (initrd) {
> - /* environ is currently the only file in the initrd */
> - char *env = malloc(initrd_size);
> - memcpy(env, initrd, initrd_size);
> - setup_env(env, initrd_size);
> - }
> + initrd_setup();
>
> if (!(auxinfo.flags & AUXINFO_MMU_OFF))
> setup_vm();
> @@ -277,7 +297,6 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> uintptr_t text = (uintptr_t)&_text, etext = ALIGN((uintptr_t)&_etext, 4096);
> uintptr_t data = (uintptr_t)&_data, edata = ALIGN((uintptr_t)&_edata, 4096);
> const void *fdt = efi_bootinfo->fdt;
> - int fdt_size, ret;
>
> /*
> * Record the largest free EFI_CONVENTIONAL_MEMORY region
> @@ -344,14 +363,13 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> }
>
> if (efi_bootinfo->fdt_valid) {
> - /* Move the FDT to the base of free memory */
> - fdt_size = fdt_totalsize(fdt);
> - ret = fdt_move(fdt, (void *)free_mem_start, fdt_size);
> - assert(ret == 0);
> - ret = dt_init((void *)free_mem_start);
> - assert(ret == 0);
> - free_mem_start += ALIGN(fdt_size, EFI_PAGE_SIZE);
> - free_mem_pages -= ALIGN(fdt_size, EFI_PAGE_SIZE) >> EFI_PAGE_SHIFT;
> + unsigned long old_start = free_mem_start;
> + void *freemem = (void *)free_mem_start;
> +
> + freemem_push_fdt(&freemem, fdt);
> +
> + free_mem_start = ALIGN((unsigned long)freemem, EFI_PAGE_SIZE);
> + free_mem_pages = (free_mem_start - old_start) >> EFI_PAGE_SHIFT;
> }
>
> __phys_end &= PHYS_MASK;
> @@ -419,13 +437,8 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
> io_init();
>
> timer_save_state();
> - if (initrd) {
> - /* environ is currently the only file in the initrd */
> - char *env = malloc(initrd_size);
>
> - memcpy(env, initrd, initrd_size);
> - setup_env(env, initrd_size);
> - }
> + initrd_setup();
>
> if (!(auxinfo.flags & AUXINFO_MMU_OFF))
> setup_vm();
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 12/18] arm/arm64: Factor out allocator init from mem_init
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 12/18] arm/arm64: Factor out allocator init from mem_init Andrew Jones
@ 2024-03-04 8:01 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 8:01 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> The allocator init is identical for mem_init() and efi_mem_init().
> Share it.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> lib/arm/setup.c | 46 ++++++++++++++++++++++------------------------
> 1 file changed, 22 insertions(+), 24 deletions(-)
>
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index f96ee04ddd68..d0be4c437708 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -136,9 +136,28 @@ static void arm_memregions_add_assumed(void)
> #endif
> }
>
> -static void mem_init(phys_addr_t freemem_start)
> +static void mem_allocator_init(phys_addr_t freemem_start, phys_addr_t freemem_end)
> {
> phys_addr_t base, top;
> +
> + freemem_start = PAGE_ALIGN(freemem_start);
> + freemem_end &= PAGE_MASK;
> +
> + phys_alloc_init(freemem_start, freemem_end - freemem_start);
> + phys_alloc_set_minimum_alignment(SMP_CACHE_BYTES);
> +
> + phys_alloc_get_unused(&base, &top);
> + base = PAGE_ALIGN(base);
> + top &= PAGE_MASK;
> + assert(sizeof(long) == 8 || !(base >> 32));
> + if (sizeof(long) != 8 && (top >> 32) != 0)
> + top = ((uint64_t)1 << 32);
> + page_alloc_init_area(0, base >> PAGE_SHIFT, top >> PAGE_SHIFT);
> + page_alloc_ops_enable();
> +}
> +
> +static void mem_init(phys_addr_t freemem_start)
> +{
> struct mem_region *freemem, *r, mem = {
> .start = (phys_addr_t)-1,
> };
> @@ -169,17 +188,7 @@ static void mem_init(phys_addr_t freemem_start)
> __phys_offset = mem.start; /* PHYS_OFFSET */
> __phys_end = mem.end; /* PHYS_END */
>
> - phys_alloc_init(freemem_start, freemem->end - freemem_start);
> - phys_alloc_set_minimum_alignment(SMP_CACHE_BYTES);
> -
> - phys_alloc_get_unused(&base, &top);
> - base = PAGE_ALIGN(base);
> - top = top & PAGE_MASK;
> - assert(sizeof(long) == 8 || !(base >> 32));
> - if (sizeof(long) != 8 && (top >> 32) != 0)
> - top = ((uint64_t)1 << 32);
> - page_alloc_init_area(0, base >> PAGE_SHIFT, top >> PAGE_SHIFT);
> - page_alloc_ops_enable();
> + mem_allocator_init(freemem_start, freemem->end);
> }
>
> static void freemem_push_fdt(void **freemem, const void *fdt)
> @@ -292,7 +301,6 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> struct efi_boot_memmap *map = &(efi_bootinfo->mem_map);
> efi_memory_desc_t *buffer = *map->map;
> efi_memory_desc_t *d = NULL;
> - phys_addr_t base, top;
> struct mem_region r;
> uintptr_t text = (uintptr_t)&_text, etext = ALIGN((uintptr_t)&_etext, 4096);
> uintptr_t data = (uintptr_t)&_data, edata = ALIGN((uintptr_t)&_edata, 4096);
> @@ -380,17 +388,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
>
> assert(sizeof(long) == 8 || free_mem_start < (3ul << 30));
>
> - phys_alloc_init(free_mem_start, free_mem_pages << EFI_PAGE_SHIFT);
> - phys_alloc_set_minimum_alignment(SMP_CACHE_BYTES);
> -
> - phys_alloc_get_unused(&base, &top);
> - base = PAGE_ALIGN(base);
> - top = top & PAGE_MASK;
> - assert(sizeof(long) == 8 || !(base >> 32));
> - if (sizeof(long) != 8 && (top >> 32) != 0)
> - top = ((uint64_t)1 << 32);
> - page_alloc_init_area(0, base >> PAGE_SHIFT, top >> PAGE_SHIFT);
> - page_alloc_ops_enable();
> + mem_allocator_init(free_mem_start, free_mem_start + (free_mem_pages << EFI_PAGE_SHIFT));
>
> return EFI_SUCCESS;
> }
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 13/18] arm64: Simplify efi_mem_init
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 13/18] arm64: Simplify efi_mem_init Andrew Jones
@ 2024-03-04 8:10 ` Nikos Nikoleris
2024-03-04 9:55 ` Andrew Jones
0 siblings, 1 reply; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 8:10 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> Reduce the EFI mem_map loop to only setting flags and finding the
> largest free memory region. Then, apply memregions_split() for
> the code/data region split and do the rest of the things that
> used to be done in the EFI mem_map loop in a separate mem_region
> loop.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> lib/arm/setup.c | 45 ++++++++++++++++++++-------------------------
> 1 file changed, 20 insertions(+), 25 deletions(-)
>
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index d0be4c437708..631597b343f1 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -301,9 +301,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> struct efi_boot_memmap *map = &(efi_bootinfo->mem_map);
> efi_memory_desc_t *buffer = *map->map;
> efi_memory_desc_t *d = NULL;
> - struct mem_region r;
> - uintptr_t text = (uintptr_t)&_text, etext = ALIGN((uintptr_t)&_etext, 4096);
> - uintptr_t data = (uintptr_t)&_data, edata = ALIGN((uintptr_t)&_edata, 4096);
> + struct mem_region r, *code, *data;
> const void *fdt = efi_bootinfo->fdt;
>
> /*
> @@ -337,21 +335,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> r.flags = MR_F_IO;
> break;
> case EFI_LOADER_CODE:
> - if (r.start <= text && r.end > text) {
> - /* This is the unit test region. Flag the code separately. */
> - phys_addr_t tmp = r.end;
> -
> - assert(etext <= data);
> - assert(edata <= r.end);
> - r.flags = MR_F_CODE;
> - r.end = data;
> - memregions_add(&r);
> - r.start = data;
> - r.end = tmp;
> - r.flags = 0;
> - } else {
> - r.flags = MR_F_RESERVED;
> - }
> + r.flags = MR_F_CODE;
> break;
> case EFI_CONVENTIONAL_MEMORY:
> if (free_mem_pages < d->num_pages) {
> @@ -361,15 +345,27 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> break;
> }
>
> - if (!(r.flags & MR_F_IO)) {
> - if (r.start < __phys_offset)
> - __phys_offset = r.start;
> - if (r.end > __phys_end)
> - __phys_end = r.end;
> - }
> memregions_add(&r);
> }
>
> + memregions_split((unsigned long)&_etext, &code, &data);
> + assert(code && (code->flags & MR_F_CODE));
> + if (data)
> + data->flags &= ~MR_F_CODE;
> +
> + for (struct mem_region *m = mem_regions; m->end; ++m) {
> + if (m != code && (m->flags & MR_F_CODE))
> + m->flags = MR_F_RESERVED;
> +
> + if (!(m->flags & MR_F_IO)) {
> + if (m->start < __phys_offset)
> + __phys_offset = m->start;
> + if (m->end > __phys_end)
> + __phys_end = m->end;
> + }
> + }
> + __phys_end &= PHYS_MASK;
> +
> if (efi_bootinfo->fdt_valid) {
> unsigned long old_start = free_mem_start;
> void *freemem = (void *)free_mem_start;
> @@ -380,7 +376,6 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> free_mem_pages = (free_mem_start - old_start) >> EFI_PAGE_SHIFT;
> }
>
> - __phys_end &= PHYS_MASK;
> asm_mmu_disable();
>
> if (free_mem_pages == 0)
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 14/18] arm64: Add memregions_efi_init
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 14/18] arm64: Add memregions_efi_init Andrew Jones
@ 2024-03-04 8:16 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 8:16 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> Provide a memregions function which initialized memregions from an
> EFI memory map. Add a new memregions flag (MR_F_PERSISTENT) for
> EFI_PERSISTENT_MEMORY since that type should not be reserved, but it
> should also be distinct from conventional memory. The function also
> points out the largest conventional memory region by returning a
> pointer to it in the freemem parameter. Immediately apply this
> function to arm64's efi_mem_init(). riscv will make use of it as well.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> lib/arm/setup.c | 76 ++++++++----------------------------------------
> lib/memregions.c | 51 ++++++++++++++++++++++++++++++++
> lib/memregions.h | 6 ++++
> 3 files changed, 69 insertions(+), 64 deletions(-)
>
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 631597b343f1..521928186fb0 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -295,58 +295,13 @@ static efi_status_t setup_rsdp(efi_bootinfo_t *efi_bootinfo)
>
> static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> {
> - int i;
> - unsigned long free_mem_pages = 0;
> - unsigned long free_mem_start = 0;
> - struct efi_boot_memmap *map = &(efi_bootinfo->mem_map);
> - efi_memory_desc_t *buffer = *map->map;
> - efi_memory_desc_t *d = NULL;
> - struct mem_region r, *code, *data;
> - const void *fdt = efi_bootinfo->fdt;
> -
> - /*
> - * Record the largest free EFI_CONVENTIONAL_MEMORY region
> - * which will be used to set up the memory allocator, so that
> - * the memory allocator can work in the largest free
> - * continuous memory region.
> - */
> - for (i = 0; i < *(map->map_size); i += *(map->desc_size)) {
> - d = (efi_memory_desc_t *)(&((u8 *)buffer)[i]);
> -
> - r.start = d->phys_addr;
> - r.end = d->phys_addr + d->num_pages * EFI_PAGE_SIZE;
> - r.flags = 0;
> -
> - switch (d->type) {
> - case EFI_RESERVED_TYPE:
> - case EFI_LOADER_DATA:
> - case EFI_BOOT_SERVICES_CODE:
> - case EFI_BOOT_SERVICES_DATA:
> - case EFI_RUNTIME_SERVICES_CODE:
> - case EFI_RUNTIME_SERVICES_DATA:
> - case EFI_UNUSABLE_MEMORY:
> - case EFI_ACPI_RECLAIM_MEMORY:
> - case EFI_ACPI_MEMORY_NVS:
> - case EFI_PAL_CODE:
> - r.flags = MR_F_RESERVED;
> - break;
> - case EFI_MEMORY_MAPPED_IO:
> - case EFI_MEMORY_MAPPED_IO_PORT_SPACE:
> - r.flags = MR_F_IO;
> - break;
> - case EFI_LOADER_CODE:
> - r.flags = MR_F_CODE;
> - break;
> - case EFI_CONVENTIONAL_MEMORY:
> - if (free_mem_pages < d->num_pages) {
> - free_mem_pages = d->num_pages;
> - free_mem_start = d->phys_addr;
> - }
> - break;
> - }
> + struct mem_region *freemem_mr = NULL, *code, *data;
> + phys_addr_t freemem_start;
> + void *freemem;
>
> - memregions_add(&r);
> - }
> + memregions_efi_init(&efi_bootinfo->mem_map, &freemem_mr);
> + if (!freemem_mr)
> + return EFI_OUT_OF_RESOURCES;
>
> memregions_split((unsigned long)&_etext, &code, &data);
> assert(code && (code->flags & MR_F_CODE));
> @@ -366,24 +321,17 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> }
> __phys_end &= PHYS_MASK;
>
> - if (efi_bootinfo->fdt_valid) {
> - unsigned long old_start = free_mem_start;
> - void *freemem = (void *)free_mem_start;
> + freemem = (void *)PAGE_ALIGN(freemem_mr->start);
>
> - freemem_push_fdt(&freemem, fdt);
> + if (efi_bootinfo->fdt_valid)
> + freemem_push_fdt(&freemem, efi_bootinfo->fdt);
>
> - free_mem_start = ALIGN((unsigned long)freemem, EFI_PAGE_SIZE);
> - free_mem_pages = (free_mem_start - old_start) >> EFI_PAGE_SHIFT;
> - }
> + freemem_start = PAGE_ALIGN((unsigned long)freemem);
> + assert(sizeof(long) == 8 || freemem_start < (3ul << 30));
>
> asm_mmu_disable();
>
> - if (free_mem_pages == 0)
> - return EFI_OUT_OF_RESOURCES;
> -
> - assert(sizeof(long) == 8 || free_mem_start < (3ul << 30));
> -
> - mem_allocator_init(free_mem_start, free_mem_start + (free_mem_pages << EFI_PAGE_SHIFT));
> + mem_allocator_init(freemem_start, freemem_mr->end);
>
> return EFI_SUCCESS;
> }
> diff --git a/lib/memregions.c b/lib/memregions.c
> index 96de86b27333..9cdbb639ab62 100644
> --- a/lib/memregions.c
> +++ b/lib/memregions.c
> @@ -80,3 +80,54 @@ void memregions_add_dt_regions(size_t max_nr)
> });
> }
> }
> +
> +#ifdef CONFIG_EFI
> +/*
> + * Add memory regions based on the EFI memory map. Also set a pointer to the
> + * memory region which corresponds to the largest EFI_CONVENTIONAL_MEMORY
> + * region, as that region is the largest free, continuous region, making it
> + * a good choice for the memory allocator.
> + */
> +void memregions_efi_init(struct efi_boot_memmap *mem_map,
> + struct mem_region **freemem)
> +{
> + u8 *buffer = (u8 *)*mem_map->map;
> + u64 freemem_pages = 0;
> +
> + *freemem = NULL;
> +
> + for (int i = 0; i < *mem_map->map_size; i += *mem_map->desc_size) {
> + efi_memory_desc_t *d = (efi_memory_desc_t *)&buffer[i];
> + struct mem_region r = {
> + .start = d->phys_addr,
> + .end = d->phys_addr + d->num_pages * EFI_PAGE_SIZE,
> + .flags = 0,
> + };
> +
> + switch (d->type) {
> + case EFI_MEMORY_MAPPED_IO:
> + case EFI_MEMORY_MAPPED_IO_PORT_SPACE:
> + r.flags = MR_F_IO;
> + break;
> + case EFI_LOADER_CODE:
> + r.flags = MR_F_CODE;
> + break;
> + case EFI_PERSISTENT_MEMORY:
> + r.flags = MR_F_PERSISTENT;
> + break;
> + case EFI_CONVENTIONAL_MEMORY:
> + if (freemem_pages < d->num_pages) {
> + freemem_pages = d->num_pages;
> + *freemem = memregions_add(&r);
> + continue;
> + }
> + break;
> + default:
> + r.flags = MR_F_RESERVED;
> + break;
> + }
> +
> + memregions_add(&r);
> + }
> +}
> +#endif /* CONFIG_EFI */
> diff --git a/lib/memregions.h b/lib/memregions.h
> index 9a8e33182fe5..1600530ad7bf 100644
> --- a/lib/memregions.h
> +++ b/lib/memregions.h
> @@ -9,6 +9,7 @@
> #define MR_F_IO BIT(0)
> #define MR_F_CODE BIT(1)
> #define MR_F_RESERVED BIT(2)
> +#define MR_F_PERSISTENT BIT(3)
> #define MR_F_UNKNOWN BIT(31)
>
> struct mem_region {
> @@ -26,4 +27,9 @@ uint32_t memregions_get_flags(phys_addr_t paddr);
> void memregions_split(phys_addr_t addr, struct mem_region **r1, struct mem_region **r2);
> void memregions_add_dt_regions(size_t max_nr);
>
> +#ifdef CONFIG_EFI
> +#include <efi.h>
> +void memregions_efi_init(struct efi_boot_memmap *mem_map, struct mem_region **freemem);
> +#endif
> +
> #endif /* _MEMREGIONS_H_ */
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 15/18] arm64: efi: Don't map reserved regions
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 15/18] arm64: efi: Don't map reserved regions Andrew Jones
@ 2024-03-04 8:18 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 8:18 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> We shouldn't need to map all the regions that the EFI memory map
> contains. Just map EFI_LOADER_CODE and EFI_LOADER_DATA, since
> those are for the loaded unit test, and any region types which
> could be used by the unit test for its own memory allocations. We
> still map EFI_BOOT_SERVICES_DATA since the primary stack is on a
> region of that type. In a later patch we'll switch to a stack we
> allocate ourselves to drop that one too.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> lib/arm/mmu.c | 6 +-----
> lib/arm/setup.c | 4 ++--
> lib/memregions.c | 8 ++++++++
> 3 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
> index eb5e82a95f06..9dce7da85709 100644
> --- a/lib/arm/mmu.c
> +++ b/lib/arm/mmu.c
> @@ -221,12 +221,8 @@ void *setup_mmu(phys_addr_t phys_end, void *unused)
> mmu_idmap = alloc_page();
>
> for (r = mem_regions; r->end; ++r) {
> - if (r->flags & MR_F_IO) {
> + if (r->flags & (MR_F_IO | MR_F_RESERVED)) {
> continue;
> - } else if (r->flags & MR_F_RESERVED) {
> - /* Reserved pages need to be writable for whatever reserved them */
> - mmu_set_range_ptes(mmu_idmap, r->start, r->start, r->end,
> - __pgprot(PTE_WBWA));
> } else if (r->flags & MR_F_CODE) {
> /* armv8 requires code shared between EL1 and EL0 to be read-only */
> mmu_set_range_ptes(mmu_idmap, r->start, r->start, r->end,
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 521928186fb0..08658b9a222b 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -309,8 +309,8 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> data->flags &= ~MR_F_CODE;
>
> for (struct mem_region *m = mem_regions; m->end; ++m) {
> - if (m != code && (m->flags & MR_F_CODE))
> - m->flags = MR_F_RESERVED;
> + if (m != code)
> + assert(!(m->flags & MR_F_CODE));
>
> if (!(m->flags & MR_F_IO)) {
> if (m->start < __phys_offset)
> diff --git a/lib/memregions.c b/lib/memregions.c
> index 9cdbb639ab62..3c6f751eb4f2 100644
> --- a/lib/memregions.c
> +++ b/lib/memregions.c
> @@ -112,6 +112,14 @@ void memregions_efi_init(struct efi_boot_memmap *mem_map,
> case EFI_LOADER_CODE:
> r.flags = MR_F_CODE;
> break;
> + case EFI_LOADER_DATA:
> + break;
> + case EFI_BOOT_SERVICES_DATA:
> + /*
> + * FIXME: This would ideally be MR_F_RESERVED, but the
> + * primary stack is in a region of this EFI type.
> + */
> + break;
> case EFI_PERSISTENT_MEMORY:
> r.flags = MR_F_PERSISTENT;
> break;
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 16/18] arm64: efi: Fix _start returns from failed _relocate
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 16/18] arm64: efi: Fix _start returns from failed _relocate Andrew Jones
@ 2024-03-04 8:58 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 8:58 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> If _relocate fails we need to restore the frame pointer and the link
> register and return from _start. But we've pushed x0 and x1 on below
> the fp and lr, so, as the code was, we'd restore the wrong values.
> Revert parts of the code back to the way they are in gnu-efi and move
> the stack alignment below the loading of x0 and x1, after we've
> confirmed _relocate didn't fail.
>
> Fixes: d231b539a41f ("arm64: Use code from the gnu-efi when booting with EFI")
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> arm/efi/crt0-efi-aarch64.S | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/arm/efi/crt0-efi-aarch64.S b/arm/efi/crt0-efi-aarch64.S
> index 5d0dc04af54a..5fd3dc94dae8 100644
> --- a/arm/efi/crt0-efi-aarch64.S
> +++ b/arm/efi/crt0-efi-aarch64.S
> @@ -111,17 +111,10 @@ section_table:
>
> .align 12
> _start:
> - stp x29, x30, [sp, #-16]!
> -
> - /* Align sp; this is necessary due to way we store cpu0's thread_info */
> + stp x29, x30, [sp, #-32]!
> mov x29, sp
> - mov x30, sp
> - and x30, x30, #THREAD_MASK
> - mov sp, x30
> - str x29, [sp, #-16]!
> -
> - stp x0, x1, [sp, #-16]!
>
> + stp x0, x1, [sp, #16]
> mov x2, x0
> mov x3, x1
> adr x0, ImageBase
> @@ -130,12 +123,20 @@ _start:
> bl _relocate
> cbnz x0, 0f
>
> - ldp x0, x1, [sp], #16
> + ldp x0, x1, [sp, #16]
> +
> + /* Align sp; this is necessary due to way we store cpu0's thread_info */
> + mov x29, sp
> + mov x30, sp
> + and x30, x30, #THREAD_MASK
> + mov sp, x30
> + str x29, [sp, #-16]!
> +
> bl efi_main
>
> /* Restore sp */
> ldr x30, [sp], #16
> - mov sp, x30
> + mov sp, x30
>
> -0: ldp x29, x30, [sp], #16
> +0: ldp x29, x30, [sp], #32
> ret
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 17/18] arm64: efi: Switch to our own stack
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 17/18] arm64: efi: Switch to our own stack Andrew Jones
@ 2024-03-04 9:03 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 9:03 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> We don't want to map EFI_BOOT_SERVICES_DATA regions, so move the
> stack from its EFI_BOOT_SERVICES_DATA region to EFI_LOADER_CODE,
> which we always map. We'll still map the stack as R/W instead of
> R/X because we split EFI_LOADER_CODE regions on the _etext boundary
> and map addresses before _etext as R/X and the rest as R/W.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
Nikos
> ---
> arm/efi/crt0-efi-aarch64.S | 22 +++++++++++++++++-----
> lib/arm/setup.c | 4 ----
> lib/memregions.c | 6 ------
> 3 files changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/arm/efi/crt0-efi-aarch64.S b/arm/efi/crt0-efi-aarch64.S
> index 5fd3dc94dae8..71ce2794f059 100644
> --- a/arm/efi/crt0-efi-aarch64.S
> +++ b/arm/efi/crt0-efi-aarch64.S
> @@ -125,12 +125,18 @@ _start:
>
> ldp x0, x1, [sp, #16]
>
> - /* Align sp; this is necessary due to way we store cpu0's thread_info */
> + /*
> + * Switch to our own stack and align sp; this is necessary due
> + * to way we store cpu0's thread_info
> + */
> + adrp x2, stacktop
> + add x2, x2, :lo12:stacktop
> + and x2, x2, #THREAD_MASK
> + mov x3, sp
> + mov sp, x2
> + stp xzr, xzr, [sp, #-16]!
> mov x29, sp
> - mov x30, sp
> - and x30, x30, #THREAD_MASK
> - mov sp, x30
> - str x29, [sp, #-16]!
> + str x3, [sp, #-16]!
>
> bl efi_main
>
> @@ -140,3 +146,9 @@ _start:
>
> 0: ldp x29, x30, [sp], #32
> ret
> +
> + .section .data
> +
> +.balign 65536
> +.space 65536
> +stacktop:
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 08658b9a222b..d535cec88709 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -340,10 +340,6 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
> {
> efi_status_t status;
>
> - struct thread_info *ti = current_thread_info();
> -
> - memset(ti, 0, sizeof(*ti));
> -
> exceptions_init();
>
> memregions_init(arm_mem_regions, NR_MEM_REGIONS);
> diff --git a/lib/memregions.c b/lib/memregions.c
> index 3c6f751eb4f2..53fc0c7cfc58 100644
> --- a/lib/memregions.c
> +++ b/lib/memregions.c
> @@ -114,12 +114,6 @@ void memregions_efi_init(struct efi_boot_memmap *mem_map,
> break;
> case EFI_LOADER_DATA:
> break;
> - case EFI_BOOT_SERVICES_DATA:
> - /*
> - * FIXME: This would ideally be MR_F_RESERVED, but the
> - * primary stack is in a region of this EFI type.
> - */
> - break;
> case EFI_PERSISTENT_MEMORY:
> r.flags = MR_F_PERSISTENT;
> break;
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 18/18] arm64: efi: Add gitlab CI
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 18/18] arm64: efi: Add gitlab CI Andrew Jones
@ 2024-03-04 9:06 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 9:06 UTC (permalink / raw
To: Andrew Jones, kvm, kvmarm
Cc: alexandru.elisei, eric.auger, shahuang, pbonzini, thuth
On 27/02/2024 19:21, Andrew Jones wrote:
> Now that we have efi-direct and tests run much faster, add a few
> (just selftests) to the CI. Test with both DT and ACPI. While
> touching the file update arm and arm64's pass/fail criteria to
> the new style that ensures they're not all skips.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
I really like --enable-efi-direct, thanks for adding support for this
and all the clean-ups!
Thanks,
Nikos
> ---
> .gitlab-ci.yml | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 71d986e9884e..ff34b1f5062e 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -44,7 +44,35 @@ build-aarch64:
> selftest-vectors-user
> timer
> | tee results.txt
> - - if grep -q FAIL results.txt ; then exit 1 ; fi
> + - grep -q PASS results.txt && ! grep -q FAIL results.txt
> +
> +build-aarch64-efi:
> + extends: .intree_template
> + script:
> + - dnf install -y qemu-system-aarch64 gcc-aarch64-linux-gnu edk2-aarch64
> + - ./configure --arch=aarch64 --cross-prefix=aarch64-linux-gnu- --enable-efi --enable-efi-direct
> + - make -j2
> + - ACCEL=tcg MAX_SMP=8 ./run_tests.sh
> + selftest-setup
> + selftest-smp
> + selftest-vectors-kernel
> + selftest-vectors-user
> + | tee results.txt
> + - grep -q PASS results.txt && ! grep -q FAIL results.txt
> +
> +build-aarch64-efi-acpi:
> + extends: .intree_template
> + script:
> + - dnf install -y qemu-system-aarch64 gcc-aarch64-linux-gnu edk2-aarch64
> + - ./configure --arch=aarch64 --cross-prefix=aarch64-linux-gnu- --enable-efi --enable-efi-direct
> + - make -j2
> + - EFI_USE_ACPI=y ACCEL=tcg MAX_SMP=8 ./run_tests.sh
> + selftest-setup
> + selftest-smp
> + selftest-vectors-kernel
> + selftest-vectors-user
> + | tee results.txt
> + - grep -q PASS results.txt && ! grep -q FAIL results.txt
>
> build-arm:
> extends: .outoftree_template
> @@ -59,7 +87,7 @@ build-arm:
> pci-test pmu-cycle-counter gicv2-ipi gicv2-mmio gicv3-ipi gicv2-active
> gicv3-active
> | tee results.txt
> - - if grep -q FAIL results.txt ; then exit 1 ; fi
> + - grep -q PASS results.txt && ! grep -q FAIL results.txt
>
> build-ppc64be:
> extends: .outoftree_template
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 08/18] arm64: efi: Improve device tree discovery
2024-03-04 7:34 ` Nikos Nikoleris
@ 2024-03-04 9:35 ` Andrew Jones
0 siblings, 0 replies; 41+ messages in thread
From: Andrew Jones @ 2024-03-04 9:35 UTC (permalink / raw
To: Nikos Nikoleris
Cc: kvm, kvmarm, alexandru.elisei, eric.auger, shahuang, pbonzini,
thuth
On Mon, Mar 04, 2024 at 07:34:44AM +0000, Nikos Nikoleris wrote:
> On 27/02/2024 19:21, Andrew Jones wrote:
> > Zero is a valid address for the device tree so add an fdt_valid data
> > member to determine when the address is valid or not. Also, check the
> > device tree GUID when the environment variable is missing. The latter
> > change allows directly loading the unit test with QEMU's '-kernel'
> > command line parameter, which is much faster than putting the test
> > in the EFI file system and then running it from the UEFI shell.
> >
>
> Out of curiosity, the fdt pointer can be zero just in KUT or zero is an
> address that efi_load_image or efi_get_system_config_table could return?
> Similar code in Linux treats 0 an non valid address https://elixir.bootlin.com/linux/latest/source/drivers/firmware/efi/libstub/fdt.c#L370
Actually, on second thought, it can't be zero. I momentarily forgot that
when we get the fdt pointer from EFI it'll be a virtual address (unlike
when we get it from x0). For v2, I'll drop the fdt_valid since fdt==NULL
is sufficient.
>
> > Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
>
> In any case, this won't hurt:
>
> Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks,
drew
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 10/18] arm64: efi: Allow running tests directly
2024-03-04 7:52 ` Nikos Nikoleris
@ 2024-03-04 9:43 ` Andrew Jones
0 siblings, 0 replies; 41+ messages in thread
From: Andrew Jones @ 2024-03-04 9:43 UTC (permalink / raw
To: Nikos Nikoleris
Cc: kvm, kvmarm, alexandru.elisei, eric.auger, shahuang, pbonzini,
thuth
On Mon, Mar 04, 2024 at 07:52:40AM +0000, Nikos Nikoleris wrote:
> On 27/02/2024 19:21, Andrew Jones wrote:
> > Since it's possible to run tests with UEFI and the QEMU -kernel
> > option (and now the DTB will be found and even the environ will
> > be set up from an initrd if given with the -initrd option), then
> > we can skip the loading of EFI tests into a file system and booting
> > to the shell to run them. Just run them directly. Running directly
> > is waaaaaay faster than booting the shell first. We keep the UEFI
> > shell as the default behavior, though, and provide a new configure
> > option to enable the direct running.
> >
> > Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
>
> Just a minor nit, see below, but in any case:
>
> Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
>
> > ---
> > arm/efi/run | 17 +++++++++++++++--
> > arm/run | 4 +++-
> > configure | 17 +++++++++++++++++
> > 3 files changed, 35 insertions(+), 3 deletions(-)
> >
> > diff --git a/arm/efi/run b/arm/efi/run
> > index b7a8418a07f8..af7b593c2bb8 100755
> > --- a/arm/efi/run
> > +++ b/arm/efi/run
> > @@ -18,10 +18,12 @@ elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
> > DEFAULT_UEFI=/usr/share/edk2/aarch64/QEMU_EFI.silent.fd
> > fi
> > +KERNEL_NAME=$1
> > +
> > : "${EFI_SRC:=$TEST_DIR}"
> > : "${EFI_UEFI:=$DEFAULT_UEFI}"
> > : "${EFI_TEST:=efi-tests}"
> > -: "${EFI_CASE:=$(basename $1 .efi)}"
> > +: "${EFI_CASE:=$(basename $KERNEL_NAME .efi)}"
> > : "${EFI_TESTNAME:=$TESTNAME}"
> > : "${EFI_TESTNAME:=$EFI_CASE}"
> > : "${EFI_CASE_DIR:="$EFI_TEST/$EFI_TESTNAME"}"
> > @@ -80,4 +82,15 @@ uefi_shell_run()
> > "${qemu_args[@]}"
> > }
> > -uefi_shell_run
> > +if [ "$EFI_DIRECT" = "y" ]; then
> > + if [ "$EFI_USE_ACPI" != "y" ]; then
> > + qemu_args+=(-machine acpi=off)
> > + fi
>
> The if statement above is common for the efi and efi_direct paths. You could
> also move it above to avoid the code replication.
Will do.
Thanks,
drew
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 13/18] arm64: Simplify efi_mem_init
2024-03-04 8:10 ` Nikos Nikoleris
@ 2024-03-04 9:55 ` Andrew Jones
2024-03-04 10:01 ` Nikos Nikoleris
0 siblings, 1 reply; 41+ messages in thread
From: Andrew Jones @ 2024-03-04 9:55 UTC (permalink / raw
To: Nikos Nikoleris
Cc: kvm, kvmarm, alexandru.elisei, eric.auger, shahuang, pbonzini,
thuth
On Mon, Mar 04, 2024 at 08:10:40AM +0000, Nikos Nikoleris wrote:
> On 27/02/2024 19:21, Andrew Jones wrote:
> > Reduce the EFI mem_map loop to only setting flags and finding the
> > largest free memory region. Then, apply memregions_split() for
> > the code/data region split and do the rest of the things that
> > used to be done in the EFI mem_map loop in a separate mem_region
> > loop.
> >
> > Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
>
> Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Thanks. While skimming this patch now to remind myself about it for v3,
I see the etext = ALIGN() below which I forgot to consider. We certainly
need the end of the text section to be on a page boundary, but that
doesn't seem to be the case right now. I think we need to add this
change
diff --git a/arm/efi/elf_aarch64_efi.lds b/arm/efi/elf_aarch64_efi.lds
index 836d98255d88..7a4192b77900 100644
--- a/arm/efi/elf_aarch64_efi.lds
+++ b/arm/efi/elf_aarch64_efi.lds
@@ -13,6 +13,7 @@ SECTIONS
*(.rodata*)
. = ALIGN(16);
}
+ . = ALIGN(4096);
_etext = .;
_text_size = . - _text;
.dynamic : { *(.dynamic) }
Thanks,
drew
>
> Thanks,
>
> Nikos
>
> > ---
> > lib/arm/setup.c | 45 ++++++++++++++++++++-------------------------
> > 1 file changed, 20 insertions(+), 25 deletions(-)
> >
> > diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> > index d0be4c437708..631597b343f1 100644
> > --- a/lib/arm/setup.c
> > +++ b/lib/arm/setup.c
> > @@ -301,9 +301,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> > struct efi_boot_memmap *map = &(efi_bootinfo->mem_map);
> > efi_memory_desc_t *buffer = *map->map;
> > efi_memory_desc_t *d = NULL;
> > - struct mem_region r;
> > - uintptr_t text = (uintptr_t)&_text, etext = ALIGN((uintptr_t)&_etext, 4096);
> > - uintptr_t data = (uintptr_t)&_data, edata = ALIGN((uintptr_t)&_edata, 4096);
> > + struct mem_region r, *code, *data;
> > const void *fdt = efi_bootinfo->fdt;
> > /*
> > @@ -337,21 +335,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> > r.flags = MR_F_IO;
> > break;
> > case EFI_LOADER_CODE:
> > - if (r.start <= text && r.end > text) {
> > - /* This is the unit test region. Flag the code separately. */
> > - phys_addr_t tmp = r.end;
> > -
> > - assert(etext <= data);
> > - assert(edata <= r.end);
> > - r.flags = MR_F_CODE;
> > - r.end = data;
> > - memregions_add(&r);
> > - r.start = data;
> > - r.end = tmp;
> > - r.flags = 0;
> > - } else {
> > - r.flags = MR_F_RESERVED;
> > - }
> > + r.flags = MR_F_CODE;
> > break;
> > case EFI_CONVENTIONAL_MEMORY:
> > if (free_mem_pages < d->num_pages) {
> > @@ -361,15 +345,27 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> > break;
> > }
> > - if (!(r.flags & MR_F_IO)) {
> > - if (r.start < __phys_offset)
> > - __phys_offset = r.start;
> > - if (r.end > __phys_end)
> > - __phys_end = r.end;
> > - }
> > memregions_add(&r);
> > }
> > + memregions_split((unsigned long)&_etext, &code, &data);
> > + assert(code && (code->flags & MR_F_CODE));
> > + if (data)
> > + data->flags &= ~MR_F_CODE;
> > +
> > + for (struct mem_region *m = mem_regions; m->end; ++m) {
> > + if (m != code && (m->flags & MR_F_CODE))
> > + m->flags = MR_F_RESERVED;
> > +
> > + if (!(m->flags & MR_F_IO)) {
> > + if (m->start < __phys_offset)
> > + __phys_offset = m->start;
> > + if (m->end > __phys_end)
> > + __phys_end = m->end;
> > + }
> > + }
> > + __phys_end &= PHYS_MASK;
> > +
> > if (efi_bootinfo->fdt_valid) {
> > unsigned long old_start = free_mem_start;
> > void *freemem = (void *)free_mem_start;
> > @@ -380,7 +376,6 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
> > free_mem_pages = (free_mem_start - old_start) >> EFI_PAGE_SHIFT;
> > }
> > - __phys_end &= PHYS_MASK;
> > asm_mmu_disable();
> > if (free_mem_pages == 0)
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [kvm-unit-tests PATCH v2 13/18] arm64: Simplify efi_mem_init
2024-03-04 9:55 ` Andrew Jones
@ 2024-03-04 10:01 ` Nikos Nikoleris
0 siblings, 0 replies; 41+ messages in thread
From: Nikos Nikoleris @ 2024-03-04 10:01 UTC (permalink / raw
To: Andrew Jones
Cc: kvm, kvmarm, alexandru.elisei, eric.auger, shahuang, pbonzini,
thuth
On 04/03/2024 09:55, Andrew Jones wrote:
> On Mon, Mar 04, 2024 at 08:10:40AM +0000, Nikos Nikoleris wrote:
>> On 27/02/2024 19:21, Andrew Jones wrote:
>>> Reduce the EFI mem_map loop to only setting flags and finding the
>>> largest free memory region. Then, apply memregions_split() for
>>> the code/data region split and do the rest of the things that
>>> used to be done in the EFI mem_map loop in a separate mem_region
>>> loop.
>>>
>>> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
>>
>> Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
>
> Thanks. While skimming this patch now to remind myself about it for v3,
> I see the etext = ALIGN() below which I forgot to consider. We certainly
> need the end of the text section to be on a page boundary, but that
> doesn't seem to be the case right now. I think we need to add this
> change
I missed and it didn't manifest in any of the runs I did. However, it
makes sense.
Thanks,
Nikos
>
> diff --git a/arm/efi/elf_aarch64_efi.lds b/arm/efi/elf_aarch64_efi.lds
> index 836d98255d88..7a4192b77900 100644
> --- a/arm/efi/elf_aarch64_efi.lds
> +++ b/arm/efi/elf_aarch64_efi.lds
> @@ -13,6 +13,7 @@ SECTIONS
> *(.rodata*)
> . = ALIGN(16);
> }
> + . = ALIGN(4096);
> _etext = .;
> _text_size = . - _text;
> .dynamic : { *(.dynamic) }
>
> Thanks,
> drew
>
>>
>> Thanks,
>>
>> Nikos
>>
>>> ---
>>> lib/arm/setup.c | 45 ++++++++++++++++++++-------------------------
>>> 1 file changed, 20 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
>>> index d0be4c437708..631597b343f1 100644
>>> --- a/lib/arm/setup.c
>>> +++ b/lib/arm/setup.c
>>> @@ -301,9 +301,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
>>> struct efi_boot_memmap *map = &(efi_bootinfo->mem_map);
>>> efi_memory_desc_t *buffer = *map->map;
>>> efi_memory_desc_t *d = NULL;
>>> - struct mem_region r;
>>> - uintptr_t text = (uintptr_t)&_text, etext = ALIGN((uintptr_t)&_etext, 4096);
>>> - uintptr_t data = (uintptr_t)&_data, edata = ALIGN((uintptr_t)&_edata, 4096);
>>> + struct mem_region r, *code, *data;
>>> const void *fdt = efi_bootinfo->fdt;
>>> /*
>>> @@ -337,21 +335,7 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
>>> r.flags = MR_F_IO;
>>> break;
>>> case EFI_LOADER_CODE:
>>> - if (r.start <= text && r.end > text) {
>>> - /* This is the unit test region. Flag the code separately. */
>>> - phys_addr_t tmp = r.end;
>>> -
>>> - assert(etext <= data);
>>> - assert(edata <= r.end);
>>> - r.flags = MR_F_CODE;
>>> - r.end = data;
>>> - memregions_add(&r);
>>> - r.start = data;
>>> - r.end = tmp;
>>> - r.flags = 0;
>>> - } else {
>>> - r.flags = MR_F_RESERVED;
>>> - }
>>> + r.flags = MR_F_CODE;
>>> break;
>>> case EFI_CONVENTIONAL_MEMORY:
>>> if (free_mem_pages < d->num_pages) {
>>> @@ -361,15 +345,27 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
>>> break;
>>> }
>>> - if (!(r.flags & MR_F_IO)) {
>>> - if (r.start < __phys_offset)
>>> - __phys_offset = r.start;
>>> - if (r.end > __phys_end)
>>> - __phys_end = r.end;
>>> - }
>>> memregions_add(&r);
>>> }
>>> + memregions_split((unsigned long)&_etext, &code, &data);
>>> + assert(code && (code->flags & MR_F_CODE));
>>> + if (data)
>>> + data->flags &= ~MR_F_CODE;
>>> +
>>> + for (struct mem_region *m = mem_regions; m->end; ++m) {
>>> + if (m != code && (m->flags & MR_F_CODE))
>>> + m->flags = MR_F_RESERVED;
>>> +
>>> + if (!(m->flags & MR_F_IO)) {
>>> + if (m->start < __phys_offset)
>>> + __phys_offset = m->start;
>>> + if (m->end > __phys_end)
>>> + __phys_end = m->end;
>>> + }
>>> + }
>>> + __phys_end &= PHYS_MASK;
>>> +
>>> if (efi_bootinfo->fdt_valid) {
>>> unsigned long old_start = free_mem_start;
>>> void *freemem = (void *)free_mem_start;
>>> @@ -380,7 +376,6 @@ static efi_status_t efi_mem_init(efi_bootinfo_t *efi_bootinfo)
>>> free_mem_pages = (free_mem_start - old_start) >> EFI_PAGE_SHIFT;
>>> }
>>> - __phys_end &= PHYS_MASK;
>>> asm_mmu_disable();
>>> if (free_mem_pages == 0)
^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2024-03-04 10:01 UTC | newest]
Thread overview: 41+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-27 19:21 [kvm-unit-tests PATCH v2 00/18] arm64: EFI improvements Andrew Jones
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 01/18] runtime: Update MAX_SMP probe Andrew Jones
2024-03-03 21:43 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 02/18] runtime: Add yet another 'no kernel' error message Andrew Jones
2024-03-03 21:50 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 03/18] arm64: efi: Don't create dummy test Andrew Jones
2024-03-03 21:57 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 04/18] arm64: efi: Make running tests on EFI can be parallel Andrew Jones
2024-03-03 22:06 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 05/18] arm64: efi: Remove redundant dtb generation Andrew Jones
2024-03-04 7:16 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 06/18] arm64: efi: Move run code into a function Andrew Jones
2024-03-04 7:19 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 07/18] arm64: efi: Remove EFI_USE_DTB Andrew Jones
2024-03-04 7:20 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 08/18] arm64: efi: Improve device tree discovery Andrew Jones
2024-03-04 7:34 ` Nikos Nikoleris
2024-03-04 9:35 ` Andrew Jones
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 09/18] lib/efi: Add support for loading the initrd Andrew Jones
2024-03-04 7:44 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 10/18] arm64: efi: Allow running tests directly Andrew Jones
2024-03-04 7:52 ` Nikos Nikoleris
2024-03-04 9:43 ` Andrew Jones
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 11/18] arm/arm64: Factor out some initial setup Andrew Jones
2024-03-04 7:59 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 12/18] arm/arm64: Factor out allocator init from mem_init Andrew Jones
2024-03-04 8:01 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 13/18] arm64: Simplify efi_mem_init Andrew Jones
2024-03-04 8:10 ` Nikos Nikoleris
2024-03-04 9:55 ` Andrew Jones
2024-03-04 10:01 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 14/18] arm64: Add memregions_efi_init Andrew Jones
2024-03-04 8:16 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 15/18] arm64: efi: Don't map reserved regions Andrew Jones
2024-03-04 8:18 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 16/18] arm64: efi: Fix _start returns from failed _relocate Andrew Jones
2024-03-04 8:58 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 17/18] arm64: efi: Switch to our own stack Andrew Jones
2024-03-04 9:03 ` Nikos Nikoleris
2024-02-27 19:21 ` [kvm-unit-tests PATCH v2 18/18] arm64: efi: Add gitlab CI Andrew Jones
2024-03-04 9:06 ` Nikos Nikoleris
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).