All of lore.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH v2 0/3] arm64: runtime scripts improvements on efi
@ 2023-11-30  3:29 Shaoqin Huang
  2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 1/3] runtime: Fix the missing last_line Shaoqin Huang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Shaoqin Huang @ 2023-11-30  3:29 UTC (permalink / raw)
  To: Andrew Jones, kvmarm
  Cc: Shaoqin Huang, Alexandru Elisei, Colton Lewis, Eric Auger, kvm,
	Nico Boehr, Nikos Nikoleris, Nina Schoetterl-Glausch,
	Ricardo Koller, Sean Christopherson, Thomas Huth

When I run the arm64 tests on efi, I found several runtime scripts issues. This
patch series try to fix all the issues.

The first patch add a missing error reporting.

The second patch skip the migration tests when run on efi since it will always
fail.

The thrid patch fix the issue when parallel running tests on efi.

Changelog:
----------
v1->v2:
  - Add the fixes tag in patch-1.
  - Change the $last_line to embedeed format in patch-1.
  - Move the check EFI into the check for migration, which decrease one check in
  patch-2.
  - Add more detailed comments about how tests fail in patch-3.

v1: https://lore.kernel.org/all/20231129032123.2658343-1-shahuang@redhat.com/

Shaoqin Huang (3):
  runtime: Fix the missing last_line
  runtime: arm64: Skip the migration tests when run on EFI
  arm64: efi: Make running tests on EFI can be parallel

 arm/efi/run          | 16 +++++++++-------
 scripts/runtime.bash |  6 +++++-
 2 files changed, 14 insertions(+), 8 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [kvm-unit-tests PATCH v2 1/3] runtime: Fix the missing last_line
  2023-11-30  3:29 [kvm-unit-tests PATCH v2 0/3] arm64: runtime scripts improvements on efi Shaoqin Huang
@ 2023-11-30  3:29 ` Shaoqin Huang
  2024-01-15 12:26   ` Andrew Jones
  2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 2/3] runtime: arm64: Skip the migration tests when run on EFI Shaoqin Huang
  2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 3/3] arm64: efi: Make running tests on EFI can be parallel Shaoqin Huang
  2 siblings, 1 reply; 10+ messages in thread
From: Shaoqin Huang @ 2023-11-30  3:29 UTC (permalink / raw)
  To: Andrew Jones, kvmarm
  Cc: Shaoqin Huang, Nico Boehr, Thomas Huth, Ricardo Koller,
	Sean Christopherson, Colton Lewis, Nikos Nikoleris,
	Nina Schoetterl-Glausch, kvm

The last_line is deleted by the 2607d2d6 ("arm64: Add an efi/run script").
This lead to when SKIP test, the reason is missing. Fix the problem by
adding last_line back.

Fixes: 2607d2d6 ("arm64: Add an efi/run script")
Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
---
 scripts/runtime.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index fc156f2f..c73fb024 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -149,7 +149,7 @@ function run()
         fi
 
         if [ ${skip} == true ]; then
-            print_result "SKIP" $testname "" "$last_line"
+            print_result "SKIP" $testname "" "$(tail -1 <<<"$log")"
             return 77
         fi
     }
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [kvm-unit-tests PATCH v2 2/3] runtime: arm64: Skip the migration tests when run on EFI
  2023-11-30  3:29 [kvm-unit-tests PATCH v2 0/3] arm64: runtime scripts improvements on efi Shaoqin Huang
  2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 1/3] runtime: Fix the missing last_line Shaoqin Huang
@ 2023-11-30  3:29 ` Shaoqin Huang
  2023-11-30  4:17   ` Shaoqin Huang
  2024-01-15 12:25   ` Andrew Jones
  2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 3/3] arm64: efi: Make running tests on EFI can be parallel Shaoqin Huang
  2 siblings, 2 replies; 10+ messages in thread
From: Shaoqin Huang @ 2023-11-30  3:29 UTC (permalink / raw)
  To: Andrew Jones, kvmarm
  Cc: Shaoqin Huang, Thomas Huth, Nico Boehr, Colton Lewis,
	Nina Schoetterl-Glausch, Nikos Nikoleris, kvm

When running the migration tests on EFI, the migration will always fail
since the efi/run use the vvfat format to run test, but the vvfat format
does not support live migration. So those migration tests will always
fail.

Instead of waiting for fail everytime when run migration tests on EFI,
skip those tests if running on EFI.

Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
---
 scripts/runtime.bash | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index c73fb024..64d223e8 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -156,6 +156,10 @@ function run()
 
     cmdline=$(get_cmdline $kernel)
     if find_word "migration" "$groups"; then
+        if [ "{CONFIG_EFI}" == "y" ]; then
+            print_result "SKIP" $testname "" "migration tests are not supported with efi"
+            return 2
+        fi
         cmdline="MIGRATION=yes $cmdline"
     fi
     if find_word "panic" "$groups"; then
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [kvm-unit-tests PATCH v2 3/3] arm64: efi: Make running tests on EFI can be parallel
  2023-11-30  3:29 [kvm-unit-tests PATCH v2 0/3] arm64: runtime scripts improvements on efi Shaoqin Huang
  2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 1/3] runtime: Fix the missing last_line Shaoqin Huang
  2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 2/3] runtime: arm64: Skip the migration tests when run on EFI Shaoqin Huang
@ 2023-11-30  3:29 ` Shaoqin Huang
  2024-01-15 12:23   ` Andrew Jones
  2 siblings, 1 reply; 10+ messages in thread
From: Shaoqin Huang @ 2023-11-30  3:29 UTC (permalink / raw)
  To: Andrew Jones, kvmarm
  Cc: Shaoqin Huang, Alexandru Elisei, Eric Auger, Nikos Nikoleris,
	Ricardo Koller, kvm

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.

Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
---
 arm/efi/run | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/arm/efi/run b/arm/efi/run
index 6872c337..03bfbef4 100755
--- a/arm/efi/run
+++ b/arm/efi/run
@@ -24,6 +24,8 @@ fi
 : "${EFI_SRC:=$TEST_DIR}"
 : "${EFI_UEFI:=$DEFAULT_UEFI}"
 : "${EFI_TEST:=efi-tests}"
+: "${EFI_TESTNAME:=$TESTNAME}"
+: "${EFI_TESTNAME:=$(basename $1 .efi)}"
 : "${EFI_CASE:=$(basename $1 .efi)}"
 : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}"
 
@@ -56,20 +58,20 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then
 	EFI_CASE=dummy
 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.40.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [kvm-unit-tests PATCH v2 2/3] runtime: arm64: Skip the migration tests when run on EFI
  2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 2/3] runtime: arm64: Skip the migration tests when run on EFI Shaoqin Huang
@ 2023-11-30  4:17   ` Shaoqin Huang
  2024-01-15 12:25   ` Andrew Jones
  1 sibling, 0 replies; 10+ messages in thread
From: Shaoqin Huang @ 2023-11-30  4:17 UTC (permalink / raw)
  To: Andrew Jones, kvmarm
  Cc: Thomas Huth, Nico Boehr, Colton Lewis, Nina Schoetterl-Glausch,
	Nikos Nikoleris, kvm



On 11/30/23 11:29, Shaoqin Huang wrote:
> When running the migration tests on EFI, the migration will always fail
> since the efi/run use the vvfat format to run test, but the vvfat format
> does not support live migration. So those migration tests will always
> fail.
> 
> Instead of waiting for fail everytime when run migration tests on EFI,
> skip those tests if running on EFI.
> 
> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> ---
>   scripts/runtime.bash | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index c73fb024..64d223e8 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -156,6 +156,10 @@ function run()
>   
>       cmdline=$(get_cmdline $kernel)
>       if find_word "migration" "$groups"; then
> +        if [ "{CONFIG_EFI}" == "y" ]; then

I'm stupid. Should be ${CONFIG_EFI} here.

> +            print_result "SKIP" $testname "" "migration tests are not supported with efi"
> +            return 2
> +        fi
>           cmdline="MIGRATION=yes $cmdline"
>       fi
>       if find_word "panic" "$groups"; then

-- 
Shaoqin


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/3] arm64: efi: Make running tests on EFI can be parallel
  2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 3/3] arm64: efi: Make running tests on EFI can be parallel Shaoqin Huang
@ 2024-01-15 12:23   ` Andrew Jones
  2024-01-16  6:37     ` Shaoqin Huang
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Jones @ 2024-01-15 12:23 UTC (permalink / raw)
  To: Shaoqin Huang
  Cc: kvmarm, Alexandru Elisei, Eric Auger, Nikos Nikoleris,
	Ricardo Koller, kvm

On Wed, Nov 29, 2023 at 10:29:40PM -0500, Shaoqin Huang wrote:
> 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.
> 
> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> ---
>  arm/efi/run | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/arm/efi/run b/arm/efi/run
> index 6872c337..03bfbef4 100755
> --- a/arm/efi/run
> +++ b/arm/efi/run
> @@ -24,6 +24,8 @@ fi
>  : "${EFI_SRC:=$TEST_DIR}"
>  : "${EFI_UEFI:=$DEFAULT_UEFI}"
>  : "${EFI_TEST:=efi-tests}"
> +: "${EFI_TESTNAME:=$TESTNAME}"
> +: "${EFI_TESTNAME:=$(basename $1 .efi)}"
>  : "${EFI_CASE:=$(basename $1 .efi)}"

We can write it the above like the following to avoid duplicating the
basename call

: "${EFI_CASE:=$(basename $1 .efi)}"
: "${EFI_TESTNAME:=$TESTNAME}"
: "${EFI_TESTNAME:=$EFI_CASE}"


>  : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}"
>  
> @@ -56,20 +58,20 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then
>  	EFI_CASE=dummy
>  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.40.1
>

Otherwise

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [kvm-unit-tests PATCH v2 2/3] runtime: arm64: Skip the migration tests when run on EFI
  2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 2/3] runtime: arm64: Skip the migration tests when run on EFI Shaoqin Huang
  2023-11-30  4:17   ` Shaoqin Huang
@ 2024-01-15 12:25   ` Andrew Jones
  2024-01-16  6:19     ` Shaoqin Huang
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Jones @ 2024-01-15 12:25 UTC (permalink / raw)
  To: Shaoqin Huang
  Cc: kvmarm, Thomas Huth, Nico Boehr, Colton Lewis,
	Nina Schoetterl-Glausch, Nikos Nikoleris, kvm

On Wed, Nov 29, 2023 at 10:29:39PM -0500, Shaoqin Huang wrote:
> When running the migration tests on EFI, the migration will always fail
> since the efi/run use the vvfat format to run test, but the vvfat format
> does not support live migration. So those migration tests will always
> fail.
> 
> Instead of waiting for fail everytime when run migration tests on EFI,
> skip those tests if running on EFI.
> 
> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> ---
>  scripts/runtime.bash | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index c73fb024..64d223e8 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -156,6 +156,10 @@ function run()
>  
>      cmdline=$(get_cmdline $kernel)
>      if find_word "migration" "$groups"; then
> +        if [ "{CONFIG_EFI}" == "y" ]; then
> +            print_result "SKIP" $testname "" "migration tests are not supported with efi"
> +            return 2
> +        fi
>          cmdline="MIGRATION=yes $cmdline"
>      fi
>      if find_word "panic" "$groups"; then
> -- 
> 2.40.1
> 

This isn't arm-specific, so we should drop the arm64 prefix from the patch
summary and get an ack from x86 people.

Thanks,
drew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [kvm-unit-tests PATCH v2 1/3] runtime: Fix the missing last_line
  2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 1/3] runtime: Fix the missing last_line Shaoqin Huang
@ 2024-01-15 12:26   ` Andrew Jones
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Jones @ 2024-01-15 12:26 UTC (permalink / raw)
  To: Shaoqin Huang
  Cc: kvmarm, Nico Boehr, Thomas Huth, Ricardo Koller,
	Sean Christopherson, Colton Lewis, Nikos Nikoleris,
	Nina Schoetterl-Glausch, kvm

On Wed, Nov 29, 2023 at 10:29:38PM -0500, Shaoqin Huang wrote:
> The last_line is deleted by the 2607d2d6 ("arm64: Add an efi/run script").
> This lead to when SKIP test, the reason is missing. Fix the problem by
> adding last_line back.
> 
> Fixes: 2607d2d6 ("arm64: Add an efi/run script")
> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> ---
>  scripts/runtime.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index fc156f2f..c73fb024 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -149,7 +149,7 @@ function run()
>          fi
>  
>          if [ ${skip} == true ]; then
> -            print_result "SKIP" $testname "" "$last_line"
> +            print_result "SKIP" $testname "" "$(tail -1 <<<"$log")"
>              return 77
>          fi
>      }
> -- 
> 2.40.1
>

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [kvm-unit-tests PATCH v2 2/3] runtime: arm64: Skip the migration tests when run on EFI
  2024-01-15 12:25   ` Andrew Jones
@ 2024-01-16  6:19     ` Shaoqin Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Shaoqin Huang @ 2024-01-16  6:19 UTC (permalink / raw)
  To: Andrew Jones
  Cc: kvmarm, Thomas Huth, Nico Boehr, Colton Lewis,
	Nina Schoetterl-Glausch, Nikos Nikoleris, kvm



On 1/15/24 20:25, Andrew Jones wrote:
> On Wed, Nov 29, 2023 at 10:29:39PM -0500, Shaoqin Huang wrote:
>> When running the migration tests on EFI, the migration will always fail
>> since the efi/run use the vvfat format to run test, but the vvfat format
>> does not support live migration. So those migration tests will always
>> fail.
>>
>> Instead of waiting for fail everytime when run migration tests on EFI,
>> skip those tests if running on EFI.
>>
>> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
>> ---
>>   scripts/runtime.bash | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
>> index c73fb024..64d223e8 100644
>> --- a/scripts/runtime.bash
>> +++ b/scripts/runtime.bash
>> @@ -156,6 +156,10 @@ function run()
>>   
>>       cmdline=$(get_cmdline $kernel)
>>       if find_word "migration" "$groups"; then
>> +        if [ "{CONFIG_EFI}" == "y" ]; then
>> +            print_result "SKIP" $testname "" "migration tests are not supported with efi"
>> +            return 2
>> +        fi
>>           cmdline="MIGRATION=yes $cmdline"
>>       fi
>>       if find_word "panic" "$groups"; then
>> -- 
>> 2.40.1
>>
> 
> This isn't arm-specific, so we should drop the arm64 prefix from the patch
> summary and get an ack from x86 people.
> 

Ok, I can fix it and respin.

> Thanks,
> drew
> 

-- 
Shaoqin


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/3] arm64: efi: Make running tests on EFI can be parallel
  2024-01-15 12:23   ` Andrew Jones
@ 2024-01-16  6:37     ` Shaoqin Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Shaoqin Huang @ 2024-01-16  6:37 UTC (permalink / raw)
  To: Andrew Jones
  Cc: kvmarm, Alexandru Elisei, Eric Auger, Nikos Nikoleris,
	Ricardo Koller, kvm



On 1/15/24 20:23, Andrew Jones wrote:
> On Wed, Nov 29, 2023 at 10:29:40PM -0500, Shaoqin Huang wrote:
>> 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.
>>
>> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
>> ---
>>   arm/efi/run | 16 +++++++++-------
>>   1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/arm/efi/run b/arm/efi/run
>> index 6872c337..03bfbef4 100755
>> --- a/arm/efi/run
>> +++ b/arm/efi/run
>> @@ -24,6 +24,8 @@ fi
>>   : "${EFI_SRC:=$TEST_DIR}"
>>   : "${EFI_UEFI:=$DEFAULT_UEFI}"
>>   : "${EFI_TEST:=efi-tests}"
>> +: "${EFI_TESTNAME:=$TESTNAME}"
>> +: "${EFI_TESTNAME:=$(basename $1 .efi)}"
>>   : "${EFI_CASE:=$(basename $1 .efi)}"
> 
> We can write it the above like the following to avoid duplicating the
> basename call
> 
> : "${EFI_CASE:=$(basename $1 .efi)}"
> : "${EFI_TESTNAME:=$TESTNAME}"
> : "${EFI_TESTNAME:=$EFI_CASE}"

Yep. It actually looks good. I will use that format.

> 
> 
>>   : "${EFI_VAR_GUID:=97ef3e03-7329-4a6a-b9ba-6c1fdcc5f823}"
>>   
>> @@ -56,20 +58,20 @@ if [ "$EFI_CASE" = "_NO_FILE_4Uhere_" ]; then
>>   	EFI_CASE=dummy
>>   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.40.1
>>
> 
> Otherwise
> 
> Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
> 

-- 
Shaoqin


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-01-16  6:38 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-30  3:29 [kvm-unit-tests PATCH v2 0/3] arm64: runtime scripts improvements on efi Shaoqin Huang
2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 1/3] runtime: Fix the missing last_line Shaoqin Huang
2024-01-15 12:26   ` Andrew Jones
2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 2/3] runtime: arm64: Skip the migration tests when run on EFI Shaoqin Huang
2023-11-30  4:17   ` Shaoqin Huang
2024-01-15 12:25   ` Andrew Jones
2024-01-16  6:19     ` Shaoqin Huang
2023-11-30  3:29 ` [kvm-unit-tests PATCH v2 3/3] arm64: efi: Make running tests on EFI can be parallel Shaoqin Huang
2024-01-15 12:23   ` Andrew Jones
2024-01-16  6:37     ` Shaoqin Huang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.