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

Nikos Nikoleris
2024-03-04T07:16:42Z

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