diff mbox series

[kvm-unit-tests,RFC,5/5] configure: Ignore --erratatxt when --target=kvmtool

Message ID 20210702163122.96110-6-alexandru.elisei@arm.com (mailing list archive)
State New, archived
Headers show
Series arm: Add kvmtool to the runner script | expand

Commit Message

Alexandru Elisei July 2, 2021, 4:31 p.m. UTC
kvmtool runs a test using the -f/--firmware argument, which doesn't load an
initrd, making specifying an errata file useless. Instead, configure forces
all erratas to be enabled via the CONFIG_ERRATA_FORCE define in
lib/config.h.

Forbid the --erratatxt option when kvm-unit-tests is configured for kvmtool
and let the user know that all erratas are enabled by default.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
I'm not sure if printing an error is too strong here and a simple warning would
suffice. Suggestions welcome!

 configure | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

Comments

Andrew Jones Sept. 7, 2021, 10:25 a.m. UTC | #1
On Fri, Jul 02, 2021 at 05:31:22PM +0100, Alexandru Elisei wrote:
> kvmtool runs a test using the -f/--firmware argument, which doesn't load an
> initrd, making specifying an errata file useless. Instead, configure forces
> all erratas to be enabled via the CONFIG_ERRATA_FORCE define in
> lib/config.h.
> 
> Forbid the --erratatxt option when kvm-unit-tests is configured for kvmtool
> and let the user know that all erratas are enabled by default.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> I'm not sure if printing an error is too strong here and a simple warning would
> suffice. Suggestions welcome!
> 
>  configure | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/configure b/configure
> index 395c809c9c02..acd288239f80 100755
> --- a/configure
> +++ b/configure
> @@ -24,7 +24,8 @@ u32_long=
>  wa_divide=
>  target=
>  errata_force=0
> -erratatxt="$srcdir/errata.txt"
> +erratatxt_default="$srcdir/errata.txt"
> +erratatxt="_NO_FILE_4Uhere_"
>  host_key_document=
>  page_size=
>  earlycon=
> @@ -50,7 +51,8 @@ usage() {
>  	                           enable or disable the generation of a default environ when
>  	                           no environ is provided by the user (enabled by default)
>  	    --erratatxt=FILE       specify a file to use instead of errata.txt. Use
> -	                           '--erratatxt=' to ensure no file is used.
> +	                           '--erratatxt=' to ensure no file is used. This option is
> +	                           invalid for arm/arm64 when target=kvmtool.

Do we need to always specifiy arm/arm64 when talking about target=kvmtool?
How much more effort would an x86 kvmtool target be to add?

>  	    --host-key-document=HOST_KEY_DOCUMENT
>  	                           Specify the machine-specific host-key document for creating
>  	                           a PVM image with 'genprotimg' (s390x only)
> @@ -147,11 +149,6 @@ if [ -n "$host_key_document" ] && [ ! -f "$host_key_document" ]; then
>      exit 1
>  fi
>  
> -if [ "$erratatxt" ] && [ ! -f "$erratatxt" ]; then
> -    echo "erratatxt: $erratatxt does not exist or is not a regular file"
> -    exit 1
> -fi
> -
>  arch_name=$arch
>  [ "$arch" = "aarch64" ] && arch="arm64"
>  [ "$arch_name" = "arm64" ] && arch_name="aarch64"
> @@ -184,6 +181,21 @@ else
>      fi
>  fi
>  
> +if [ "$target" = "kvmtool" ]; then
> +    if [ "$erratatxt" ] && [ "$erratatxt" != "_NO_FILE_4Uhere_" ]; then
> +        echo "--erratatxt is not supported for target=kvmtool (all erratas enabled by default)"
> +        usage
> +    fi
> +else
> +    if [ "$erratatxt" = "_NO_FILE_4Uhere_" ]; then
> +        erratatxt=$erratatxt_default
> +    fi
> +    if [ "$erratatxt" ] && [ ! -f "$erratatxt" ]; then
> +        echo "erratatxt: $erratatxt does not exist or is not a regular file"
> +        exit 1
> +    fi
> +fi

switch

> +
>  [ -z "$processor" ] && processor="$arch"
>  
>  if [ "$processor" = "arm64" ]; then
> -- 
> 2.32.0
>

Otherwise looks good to me.

Thanks,
drew
Alexandru Elisei Sept. 8, 2021, 4:13 p.m. UTC | #2
Hi Drew,

On 9/7/21 11:25 AM, Andrew Jones wrote:
> On Fri, Jul 02, 2021 at 05:31:22PM +0100, Alexandru Elisei wrote:
>> kvmtool runs a test using the -f/--firmware argument, which doesn't load an
>> initrd, making specifying an errata file useless. Instead, configure forces
>> all erratas to be enabled via the CONFIG_ERRATA_FORCE define in
>> lib/config.h.
>>
>> Forbid the --erratatxt option when kvm-unit-tests is configured for kvmtool
>> and let the user know that all erratas are enabled by default.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>> I'm not sure if printing an error is too strong here and a simple warning would
>> suffice. Suggestions welcome!
>>
>>  configure | 26 +++++++++++++++++++-------
>>  1 file changed, 19 insertions(+), 7 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 395c809c9c02..acd288239f80 100755
>> --- a/configure
>> +++ b/configure
>> @@ -24,7 +24,8 @@ u32_long=
>>  wa_divide=
>>  target=
>>  errata_force=0
>> -erratatxt="$srcdir/errata.txt"
>> +erratatxt_default="$srcdir/errata.txt"
>> +erratatxt="_NO_FILE_4Uhere_"
>>  host_key_document=
>>  page_size=
>>  earlycon=
>> @@ -50,7 +51,8 @@ usage() {
>>  	                           enable or disable the generation of a default environ when
>>  	                           no environ is provided by the user (enabled by default)
>>  	    --erratatxt=FILE       specify a file to use instead of errata.txt. Use
>> -	                           '--erratatxt=' to ensure no file is used.
>> +	                           '--erratatxt=' to ensure no file is used. This option is
>> +	                           invalid for arm/arm64 when target=kvmtool.
> Do we need to always specifiy arm/arm64 when talking about target=kvmtool?
> How much more effort would an x86 kvmtool target be to add?

Err.. there haven't been any commits to the x86 code, other than those caused by
refactoring in the other parts of the code, since 2015.

Ballpark estimate, it's going to be very hard even to make 50% of the tests run on
kvmtool. I think adding adding --target=uefi is much more likely to happen for x86
than --target=kvmtool.

>
>>  	    --host-key-document=HOST_KEY_DOCUMENT
>>  	                           Specify the machine-specific host-key document for creating
>>  	                           a PVM image with 'genprotimg' (s390x only)
>> @@ -147,11 +149,6 @@ if [ -n "$host_key_document" ] && [ ! -f "$host_key_document" ]; then
>>      exit 1
>>  fi
>>  
>> -if [ "$erratatxt" ] && [ ! -f "$erratatxt" ]; then
>> -    echo "erratatxt: $erratatxt does not exist or is not a regular file"
>> -    exit 1
>> -fi
>> -
>>  arch_name=$arch
>>  [ "$arch" = "aarch64" ] && arch="arm64"
>>  [ "$arch_name" = "arm64" ] && arch_name="aarch64"
>> @@ -184,6 +181,21 @@ else
>>      fi
>>  fi
>>  
>> +if [ "$target" = "kvmtool" ]; then
>> +    if [ "$erratatxt" ] && [ "$erratatxt" != "_NO_FILE_4Uhere_" ]; then
>> +        echo "--erratatxt is not supported for target=kvmtool (all erratas enabled by default)"
>> +        usage
>> +    fi
>> +else
>> +    if [ "$erratatxt" = "_NO_FILE_4Uhere_" ]; then
>> +        erratatxt=$erratatxt_default
>> +    fi
>> +    if [ "$erratatxt" ] && [ ! -f "$erratatxt" ]; then
>> +        echo "erratatxt: $erratatxt does not exist or is not a regular file"
>> +        exit 1
>> +    fi
>> +fi
> switch

Sure.

Thanks,

Alex

>
>> +
>>  [ -z "$processor" ] && processor="$arch"
>>  
>>  if [ "$processor" = "arm64" ]; then
>> -- 
>> 2.32.0
>>
> Otherwise looks good to me.
>
> Thanks,
> drew 
>
diff mbox series

Patch

diff --git a/configure b/configure
index 395c809c9c02..acd288239f80 100755
--- a/configure
+++ b/configure
@@ -24,7 +24,8 @@  u32_long=
 wa_divide=
 target=
 errata_force=0
-erratatxt="$srcdir/errata.txt"
+erratatxt_default="$srcdir/errata.txt"
+erratatxt="_NO_FILE_4Uhere_"
 host_key_document=
 page_size=
 earlycon=
@@ -50,7 +51,8 @@  usage() {
 	                           enable or disable the generation of a default environ when
 	                           no environ is provided by the user (enabled by default)
 	    --erratatxt=FILE       specify a file to use instead of errata.txt. Use
-	                           '--erratatxt=' to ensure no file is used.
+	                           '--erratatxt=' to ensure no file is used. This option is
+	                           invalid for arm/arm64 when target=kvmtool.
 	    --host-key-document=HOST_KEY_DOCUMENT
 	                           Specify the machine-specific host-key document for creating
 	                           a PVM image with 'genprotimg' (s390x only)
@@ -147,11 +149,6 @@  if [ -n "$host_key_document" ] && [ ! -f "$host_key_document" ]; then
     exit 1
 fi
 
-if [ "$erratatxt" ] && [ ! -f "$erratatxt" ]; then
-    echo "erratatxt: $erratatxt does not exist or is not a regular file"
-    exit 1
-fi
-
 arch_name=$arch
 [ "$arch" = "aarch64" ] && arch="arm64"
 [ "$arch_name" = "arm64" ] && arch_name="aarch64"
@@ -184,6 +181,21 @@  else
     fi
 fi
 
+if [ "$target" = "kvmtool" ]; then
+    if [ "$erratatxt" ] && [ "$erratatxt" != "_NO_FILE_4Uhere_" ]; then
+        echo "--erratatxt is not supported for target=kvmtool (all erratas enabled by default)"
+        usage
+    fi
+else
+    if [ "$erratatxt" = "_NO_FILE_4Uhere_" ]; then
+        erratatxt=$erratatxt_default
+    fi
+    if [ "$erratatxt" ] && [ ! -f "$erratatxt" ]; then
+        echo "erratatxt: $erratatxt does not exist or is not a regular file"
+        exit 1
+    fi
+fi
+
 [ -z "$processor" ] && processor="$arch"
 
 if [ "$processor" = "arm64" ]; then