Re: [kvm-unit-tests PATCH v2 05/18] arm64: efi: Remove redundant dtb generation

[Nikos Nikoleris <nikos.nikoleris@arm.com>]

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[@]}"