| Message ID | 20210702163122.96110-6-alexandru.elisei@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | arm: Add kvmtool to the runner script | expand |
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
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 --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
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(-)