[kvm-unit-tests PATCH 0/3] EFI runtime fixes

[kvm-unit-tests PATCH 0/3] EFI runtime fixes

[Andrew Jones]

While testing the new EFI support for Arm, I fixed a couple runtime
script issues.

Andrew Jones (3):
  arch-run: Extend timeout when booting with UEFI
  arm/efi/run: Add Fedora's path to QEMU_EFI
  configure: efi: Link correct run script

 arm/efi/run           | 15 ++++++++++-----
 configure             |  5 ++++-
 scripts/arch-run.bash | 10 ++++++++++
 x86/efi/run           |  2 +-
 4 files changed, 25 insertions(+), 7 deletions(-)

-- 
2.40.1

[kvm-unit-tests PATCH 1/3] arch-run: Extend timeout when booting with UEFI

[Andrew Jones]

Booting UEFI can take a long time. Give the timeout some extra time
to compensate for it.

Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
 scripts/arch-run.bash | 10 ++++++++++
 1 file changed, 10 insertions(+)

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

[kvm-unit-tests PATCH 2/3] arm/efi/run: Add Fedora's path to QEMU_EFI

[Andrew Jones]

Try Fedora's default path too.

Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
 arm/efi/run | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arm/efi/run b/arm/efi/run
index c61da31183a7..f75ef157acf3 100755
--- a/arm/efi/run
+++ b/arm/efi/run
@@ -15,8 +15,14 @@ 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
+elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
+	DEFAULT_UEFI=/usr/share/edk2/aarch64/QEMU_EFI.silent.fd
+fi
+
 : "${EFI_SRC:=$(realpath "$(dirname "$0")/../")}"
-: "${EFI_UEFI:=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd}"
+: "${EFI_UEFI:=$DEFAULT_UEFI}"
 : "${EFI_TEST:=efi-tests}"
 : "${EFI_CASE:=$(basename $1 .efi)}"
 : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}"
@@ -24,9 +30,8 @@ source scripts/common.bash
 [ "$EFI_USE_ACPI" = "y" ] || EFI_USE_DTB=y
 
 if [ ! -f "$EFI_UEFI" ]; then
-	echo "UEFI firmware not found: $EFI_UEFI"
-	echo "Please install the UEFI firmware to this path"
-	echo "Or specify the correct path with the env variable EFI_UEFI"
+	echo "UEFI firmware not found."
+	echo "Please specify the path with the env variable EFI_UEFI"
 	exit 2
 fi
 
-- 
2.40.1

[kvm-unit-tests PATCH 3/3] configure: efi: Link correct run script

[Andrew Jones]

EFI built tests use $TEST_DIR/efi/run, not $TEST_DIR/run.
Also, now that we may be using the link, rather than the
script directly, make sure we use an absolute path to the
EFI source rather than assuming it's the parent directory.
TEST_DIR already points there, so we can just use that.

Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
 arm/efi/run | 2 +-
 configure   | 5 ++++-
 x86/efi/run | 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arm/efi/run b/arm/efi/run
index f75ef157acf3..6872c337c945 100755
--- a/arm/efi/run
+++ b/arm/efi/run
@@ -21,7 +21,7 @@ elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
 	DEFAULT_UEFI=/usr/share/edk2/aarch64/QEMU_EFI.silent.fd
 fi
 
-: "${EFI_SRC:=$(realpath "$(dirname "$0")/../")}"
+: "${EFI_SRC:=$TEST_DIR}"
 : "${EFI_UEFI:=$DEFAULT_UEFI}"
 : "${EFI_TEST:=efi-tests}"
 : "${EFI_CASE:=$(basename $1 .efi)}"
diff --git a/configure b/configure
index b665f7d586c2..6ee9b27a6af2 100755
--- a/configure
+++ b/configure
@@ -313,7 +313,10 @@ if [ ! -d "$srcdir/$testdir" ]; then
     echo "$testdir does not exist!"
     exit 1
 fi
-if [ -f "$srcdir/$testdir/run" ]; then
+
+if [ "$efi" = "y" ] && [ -f "$srcdir/$testdir/efi/run" ]; then
+    ln -fs "$srcdir/$testdir/efi/run" $testdir-run
+elif [ -f "$srcdir/$testdir/run" ]; then
     ln -fs "$srcdir/$testdir/run" $testdir-run
 fi
 
diff --git a/x86/efi/run b/x86/efi/run
index 322cb7567fdc..85aeb94fe605 100755
--- a/x86/efi/run
+++ b/x86/efi/run
@@ -13,7 +13,7 @@ if [ ! -f config.mak ]; then
 fi
 source config.mak
 
-: "${EFI_SRC:=$(realpath "$(dirname "$0")/../")}"
+: "${EFI_SRC:=$TEST_DIR}"
 : "${EFI_UEFI:=/usr/share/ovmf/OVMF.fd}"
 : "${EFI_TEST:=efi-tests}"
 : "${EFI_SMP:=1}"
-- 
2.40.1

Re: [kvm-unit-tests PATCH 1/3] arch-run: Extend timeout when booting with UEFI

[Nikos Nikoleris]

On 07/06/2023 19:59, Andrew Jones wrote:
> Booting UEFI can take a long time. Give the timeout some extra time
> to compensate for it.
> 
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> ---
>   scripts/arch-run.bash | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 51e4b97b27d1..72ce718b1170 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -94,7 +94,17 @@ run_qemu_status ()
>   
>   timeout_cmd ()
>   {
> +	local s
> +
>   	if [ "$TIMEOUT" ] && [ "$TIMEOUT" != "0" ]; then
> +		if [ "$CONFIG_EFI" = 'y' ]; then
> +			s=${TIMEOUT: -1}
> +			if [ "$s" = 's' ]; then
> +				TIMEOUT=${TIMEOUT:0:-1}
> +				((TIMEOUT += 10)) # Add 10 seconds for booting UEFI
> +				TIMEOUT="${TIMEOUT}s"
> +			fi
> +		fi
>   		echo "timeout -k 1s --foreground $TIMEOUT"
>   	fi
>   }

This looks fine to me but at the same time, I wonder if it's worth the 
complexity. In arm/unittests.cfg, timer is the only test where we 
specify a timeout. If we were to bump it from 10s to 20s it would solve 
the problem too but also the timeout would be extended for non EFI runs too.

In any case:

Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>

Thanks,

Nikos

Re: [kvm-unit-tests PATCH 2/3] arm/efi/run: Add Fedora's path to QEMU_EFI

[Nikos Nikoleris]

On 07/06/2023 19:59, Andrew Jones wrote:
> Try Fedora's default path too.
> 
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>

Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>

> ---
>   arm/efi/run | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/arm/efi/run b/arm/efi/run
> index c61da31183a7..f75ef157acf3 100755
> --- a/arm/efi/run
> +++ b/arm/efi/run
> @@ -15,8 +15,14 @@ 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
> +elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
> +	DEFAULT_UEFI=/usr/share/edk2/aarch64/QEMU_EFI.silent.fd
> +fi
> +
>   : "${EFI_SRC:=$(realpath "$(dirname "$0")/../")}"
> -: "${EFI_UEFI:=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd}"
> +: "${EFI_UEFI:=$DEFAULT_UEFI}"
>   : "${EFI_TEST:=efi-tests}"
>   : "${EFI_CASE:=$(basename $1 .efi)}"
>   : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}"
> @@ -24,9 +30,8 @@ source scripts/common.bash
>   [ "$EFI_USE_ACPI" = "y" ] || EFI_USE_DTB=y
>   
>   if [ ! -f "$EFI_UEFI" ]; then
> -	echo "UEFI firmware not found: $EFI_UEFI"
> -	echo "Please install the UEFI firmware to this path"
> -	echo "Or specify the correct path with the env variable EFI_UEFI"
> +	echo "UEFI firmware not found."
> +	echo "Please specify the path with the env variable EFI_UEFI"
>   	exit 2
>   fi
>   

Re: [kvm-unit-tests PATCH 3/3] configure: efi: Link correct run script

[Nikos Nikoleris]

On 07/06/2023 19:59, Andrew Jones wrote:
> EFI built tests use $TEST_DIR/efi/run, not $TEST_DIR/run.
> Also, now that we may be using the link, rather than the
> script directly, make sure we use an absolute path to the
> EFI source rather than assuming it's the parent directory.
> TEST_DIR already points there, so we can just use that.
> 
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>

Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>

> ---
>   arm/efi/run | 2 +-
>   configure   | 5 ++++-
>   x86/efi/run | 2 +-
>   3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arm/efi/run b/arm/efi/run
> index f75ef157acf3..6872c337c945 100755
> --- a/arm/efi/run
> +++ b/arm/efi/run
> @@ -21,7 +21,7 @@ elif [ -f /usr/share/edk2/aarch64/QEMU_EFI.silent.fd ]; then
>   	DEFAULT_UEFI=/usr/share/edk2/aarch64/QEMU_EFI.silent.fd
>   fi
>   
> -: "${EFI_SRC:=$(realpath "$(dirname "$0")/../")}"
> +: "${EFI_SRC:=$TEST_DIR}"
>   : "${EFI_UEFI:=$DEFAULT_UEFI}"
>   : "${EFI_TEST:=efi-tests}"
>   : "${EFI_CASE:=$(basename $1 .efi)}"
> diff --git a/configure b/configure
> index b665f7d586c2..6ee9b27a6af2 100755
> --- a/configure
> +++ b/configure
> @@ -313,7 +313,10 @@ if [ ! -d "$srcdir/$testdir" ]; then
>       echo "$testdir does not exist!"
>       exit 1
>   fi
> -if [ -f "$srcdir/$testdir/run" ]; then
> +
> +if [ "$efi" = "y" ] && [ -f "$srcdir/$testdir/efi/run" ]; then
> +    ln -fs "$srcdir/$testdir/efi/run" $testdir-run
> +elif [ -f "$srcdir/$testdir/run" ]; then
>       ln -fs "$srcdir/$testdir/run" $testdir-run
>   fi
>   
> diff --git a/x86/efi/run b/x86/efi/run
> index 322cb7567fdc..85aeb94fe605 100755
> --- a/x86/efi/run
> +++ b/x86/efi/run
> @@ -13,7 +13,7 @@ if [ ! -f config.mak ]; then
>   fi
>   source config.mak
>   
> -: "${EFI_SRC:=$(realpath "$(dirname "$0")/../")}"
> +: "${EFI_SRC:=$TEST_DIR}"
>   : "${EFI_UEFI:=/usr/share/ovmf/OVMF.fd}"
>   : "${EFI_TEST:=efi-tests}"
>   : "${EFI_SMP:=1}"

Re: [kvm-unit-tests PATCH 0/3] EFI runtime fixes

[Andrew Jones]

On Wed, Jun 07, 2023 at 08:59:02PM +0200, Andrew Jones wrote:
> While testing the new EFI support for Arm, I fixed a couple runtime
> script issues.
> 
> Andrew Jones (3):
>   arch-run: Extend timeout when booting with UEFI
>   arm/efi/run: Add Fedora's path to QEMU_EFI
>   configure: efi: Link correct run script
> 
>  arm/efi/run           | 15 ++++++++++-----
>  configure             |  5 ++++-
>  scripts/arch-run.bash | 10 ++++++++++
>  x86/efi/run           |  2 +-
>  4 files changed, 25 insertions(+), 7 deletions(-)
> 
> -- 
> 2.40.1
>

Applied to arm/queue

https://gitlab.com/jones-drew/kvm-unit-tests/-/commits/arm/queue

Thanks,
drew