[kvm-unit-tests PATCH v1 0/5] arm64: Change the default --processor to max

[kvm-unit-tests PATCH v1 0/5] arm64: Change the default --processor to max

[Alexandru Elisei]

(CC'ing everyone from MAINTAINERS because I'm touching configure)

Vladimir sent a test for MTE [1], which didn't work on the default -cpu
model, cortex-a57, because that CPU didn't implement MTE. There were two
options to get it working:

1. Add -cpu max to the extra_params unittest parameter.
2. Make the default value for the configure --processor option 'max'.

We decided that the second option was preferable, so here it is.

The first patch might look unrelated, but when I was writing the function
to select the default processor based on the architecture I noticed that
for arm64, $arch is first equal to aarch64, then it gets changed to arm64.
My first instinct was to have it be arm64 from the start, but then I
realized that, despite the help text, --arch=aarch64 has been supported
ever since arm64 was added to kvm-unit-tests. So I decided that it might
more prudent to go with it and document it.

[1] https://lore.kernel.org/all/20241212103447.34593-1-vladimir.murzin@arm.com/

Alexandru Elisei (5):
  configure: Document that the architecture name 'aarch64' is also
    supported
  configure: Display the default processor for arm and arm64
  arm64: Implement the ./configure --processor option
  arm/arm64: Add support for --processor=max
  configure: arm64: Make 'max' the default for --processor

 arm/Makefile.arm    |  1 -
 arm/Makefile.common |  3 +++
 configure           | 35 ++++++++++++++++++++++++++---------
 3 files changed, 29 insertions(+), 10 deletions(-)


base-commit: 0ed2cdf3c80ee803b9150898e687e77e4d6f5db2
-- 
2.47.1

[kvm-unit-tests PATCH v1 1/5] configure: Document that the architecture name 'aarch64' is also supported

[Alexandru Elisei]

$arch, on arm64, defaults to 'aarch64', and later in the script is replaced
by 'arm64'. Intentional or not, document that the name 'aarch64' is also
supported when configuring for the arm64 architecture. This has been the
case since the initial commit that added support for the arm64
architecture, commit 39ac3f8494be ("arm64: initial drop").

The help text for --arch changes from*:

   --arch=ARCH            architecture to compile for (aarch64). ARCH can be one of:
                           arm, arm64, i386, ppc64, riscv32, riscv64, s390x, x86_64

to:

    --arch=ARCH            architecture to compile for (aarch64). ARCH can be one of:
                           arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64

*Worth pointing out that the default architecture is 'aarch64', even though
the rest of the help text doesn't have it as one of the supported
architectures.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 86cf1da36467..5b0a2d7f39c0 100755
--- a/configure
+++ b/configure
@@ -47,7 +47,7 @@ usage() {
 
 	Options include:
 	    --arch=ARCH            architecture to compile for ($arch). ARCH can be one of:
-	                           arm, arm64, i386, ppc64, riscv32, riscv64, s390x, x86_64
+	                           arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
 	    --processor=PROCESSOR  processor to compile for ($arch)
 	    --target=TARGET        target platform that the tests will be running on (qemu or
 	                           kvmtool, default is qemu) (arm/arm64 only)
-- 
2.47.1

[kvm-unit-tests PATCH v1 2/5] configure: Display the default processor for arm and arm64

[Alexandru Elisei]

The help text for the --processor option displays the architecture name as
the default processor type. But the default for arm is cortex-a15, and for
arm64 is cortex-a57. Teach configure to display the correct default
processor type for these two architectures.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 configure | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index 5b0a2d7f39c0..138840c3f76d 100755
--- a/configure
+++ b/configure
@@ -5,6 +5,24 @@ if [ -z "${BASH_VERSINFO[0]}" ] || [ "${BASH_VERSINFO[0]}" -lt 4 ] ; then
     exit 1
 fi
 
+function get_default_processor()
+{
+    local arch="$1"
+
+    case "$arch" in
+    "arm")
+        default_processor="cortex-a15"
+        ;;
+    "arm64" | "aarch64")
+        default_processor="cortex-a57"
+        ;;
+    *)
+        default_processor=$arch
+    esac
+
+    echo "$default_processor"
+}
+
 srcdir=$(cd "$(dirname "$0")"; pwd)
 prefix=/usr/local
 cc=gcc
@@ -33,6 +51,7 @@ page_size=
 earlycon=
 efi=
 efi_direct=
+default_processor=$(get_default_processor $arch)
 
 # Enable -Werror by default for git repositories only (i.e. developer builds)
 if [ -e "$srcdir"/.git ]; then
@@ -48,7 +67,7 @@ usage() {
 	Options include:
 	    --arch=ARCH            architecture to compile for ($arch). ARCH can be one of:
 	                           arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
-	    --processor=PROCESSOR  processor to compile for ($arch)
+	    --processor=PROCESSOR  processor to compile for ($default_processor)
 	    --target=TARGET        target platform that the tests will be running on (qemu or
 	                           kvmtool, default is qemu) (arm/arm64 only)
 	    --cross-prefix=PREFIX  cross compiler prefix
@@ -283,13 +302,8 @@ else
     fi
 fi
 
-[ -z "$processor" ] && processor="$arch"
-
-if [ "$processor" = "arm64" ]; then
-    processor="cortex-a57"
-elif [ "$processor" = "arm" ]; then
-    processor="cortex-a15"
-fi
+# $arch will have changed when cross-compiling.
+[ -z "$processor" ] && processor=$(get_default_processor $arch)
 
 if [ "$arch" = "i386" ] || [ "$arch" = "x86_64" ]; then
     testdir=x86
-- 
2.47.1

[kvm-unit-tests PATCH v1 3/5] arm64: Implement the ./configure --processor option

[Alexandru Elisei]

The help text for the ./configure --processor option says:

    --processor=PROCESSOR  processor to compile for (cortex-a57)

but, unlike arm, the build system does not pass a -mcpu argument to the
compiler. Fix it, and bring arm64 at parity with arm.

Note that this introduces a regression, which is also present on arm: if
the --processor argument is something that the compiler doesn't understand,
but qemu does (like 'max'), then compilation fails. This will be fixed in a
following patch; another fix is to specify a CPU model that gcc implements
by using --cflags=-mcpu=<cpu>.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/Makefile.arm    | 1 -
 arm/Makefile.common | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arm/Makefile.arm b/arm/Makefile.arm
index 7fd39f3ada64..d6250b7fb686 100644
--- a/arm/Makefile.arm
+++ b/arm/Makefile.arm
@@ -12,7 +12,6 @@ $(error Cannot build arm32 tests as EFI apps)
 endif
 
 CFLAGS += $(machine)
-CFLAGS += -mcpu=$(PROCESSOR)
 CFLAGS += -mno-unaligned-access
 
 ifeq ($(TARGET),qemu)
diff --git a/arm/Makefile.common b/arm/Makefile.common
index f828dbe01d33..a5d97bcf477a 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -25,6 +25,7 @@ AUXFLAGS ?= 0x0
 # stack.o relies on frame pointers.
 KEEP_FRAME_POINTER := y
 
+CFLAGS += -mcpu=$(PROCESSOR)
 CFLAGS += -std=gnu99
 CFLAGS += -ffreestanding
 CFLAGS += -O2
-- 
2.47.1

[kvm-unit-tests PATCH v1 4/5] arm/arm64: Add support for --processor=max

[Alexandru Elisei]

For arm64, newer architecture features are supported only on newer CPUs.
Instead of expecting the user to know which CPU model supports which
feature when using the TCG accelerator for qemu, let's make it easier and
add support for the --processor 'max' value.

The --processor value is passed to the compiler's -mcpu argument and to
qemu's -cpu argument. 'max' is a special value that only qemu understands -
it means that all CPU features that qemu implements are supported by the
guest CPU, and passing it to the compiler causes a build error. So omit the
-mcpu argument when $PROCESSOR=max.

This affects only the TCG accelerator; when using KVM or HVF,
kvm-unit-tests sets the cpu model to 'host'.

Note that using --processor=max with a 32 bit compiler will cause a build
error: the CPU model that the compiler defaults to when the -mcpu argument
is missing lacks support for some of the instructions that kvm-unit-tests
uses. The solution in the case is to specify a CPU model for the compiler
using --cflags:

  ./configure --arch=arm --processor=max --cflags=-mcpu=<cpu>

This patch doesn't introduce a regression for arm when --processor=max is
used, it's only the error that changes: from an unknown processor type to
using instructions that are not available on the processor.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/Makefile.common | 2 ++
 configure           | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arm/Makefile.common b/arm/Makefile.common
index a5d97bcf477a..b757250dc9ae 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -25,7 +25,9 @@ AUXFLAGS ?= 0x0
 # stack.o relies on frame pointers.
 KEEP_FRAME_POINTER := y
 
+ifneq ($(PROCESSOR),max)
 CFLAGS += -mcpu=$(PROCESSOR)
+endif
 CFLAGS += -std=gnu99
 CFLAGS += -ffreestanding
 CFLAGS += -O2
diff --git a/configure b/configure
index 138840c3f76d..46964d36a7d8 100755
--- a/configure
+++ b/configure
@@ -67,7 +67,10 @@ usage() {
 	Options include:
 	    --arch=ARCH            architecture to compile for ($arch). ARCH can be one of:
 	                           arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
-	    --processor=PROCESSOR  processor to compile for ($default_processor)
+	    --processor=PROCESSOR  processor to compile for ($default_processor). For arm and arm64, the
+	                           value 'max' is special and it will be passed directly to
+	                           qemu, bypassing the compiler. In this case, --cflags can be
+	                           used to compile for a specific processor.
 	    --target=TARGET        target platform that the tests will be running on (qemu or
 	                           kvmtool, default is qemu) (arm/arm64 only)
 	    --cross-prefix=PREFIX  cross compiler prefix
-- 
2.47.1

[kvm-unit-tests PATCH v1 5/5] configure: arm64: Make 'max' the default for --processor

[Alexandru Elisei]

Newer architecture features are supported by qemu TCG on newer CPUs. When
writing a test for such architecture features, it is necessary to pass the
correct -cpu argument to qemu. Make it easier on users and test authors
alike by making 'max' the default value for --processor. The 'max' CPU
model contains all the features of the cortex-a57 CPU (the old default), so
no regression should be possible.

A side effect is that, by default, the compiler will not receive a -mcpu
argument for compiling the code. The expectation is that this is fine,
since support for -mcpu=$PROCESSOR has only been added for arm64 in the
last commit.

The default for arm (cortex-a15) has been kept unchanged, because passing
--processor=max will cause compilation to break. If the user wants the qemu
CPU model to be 'max', the user will also have to supply a suitable compile
CPU target via --cflags=-mcpu=<cpu> configure option.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 46964d36a7d8..3ab0ec208e10 100755
--- a/configure
+++ b/configure
@@ -14,7 +14,7 @@ function get_default_processor()
         default_processor="cortex-a15"
         ;;
     "arm64" | "aarch64")
-        default_processor="cortex-a57"
+        default_processor="max"
         ;;
     *)
         default_processor=$arch
-- 
2.47.1

Re: [kvm-unit-tests PATCH v1 0/5] arm64: Change the default --processor to max

[Vladimir Murzin]

On 1/10/25 13:58, Alexandru Elisei wrote:
> (CC'ing everyone from MAINTAINERS because I'm touching configure)
> 
> Vladimir sent a test for MTE [1], which didn't work on the default -cpu
> model, cortex-a57, because that CPU didn't implement MTE. There were two
> options to get it working:
> 
> 1. Add -cpu max to the extra_params unittest parameter.
> 2. Make the default value for the configure --processor option 'max'.
> 
> We decided that the second option was preferable, so here it is.
> 
> The first patch might look unrelated, but when I was writing the function
> to select the default processor based on the architecture I noticed that
> for arm64, $arch is first equal to aarch64, then it gets changed to arm64.
> My first instinct was to have it be arm64 from the start, but then I
> realized that, despite the help text, --arch=aarch64 has been supported
> ever since arm64 was added to kvm-unit-tests. So I decided that it might
> more prudent to go with it and document it.
> 
> [1] https://lore.kernel.org/all/20241212103447.34593-1-vladimir.murzin@arm.com/
> 

Thanks Alex! That removes extra hassle of setting up -cpu to match required
feature. My MTE test continue working fine and require one less configuration
option - undeniable improvement in user experience!

FWIW:
Tested-by: Vladimir Murzin <vladimir.murzin@arm.com> # arm64

Vladimir

> Alexandru Elisei (5):
>   configure: Document that the architecture name 'aarch64' is also
>     supported
>   configure: Display the default processor for arm and arm64
>   arm64: Implement the ./configure --processor option
>   arm/arm64: Add support for --processor=max
>   configure: arm64: Make 'max' the default for --processor
> 
>  arm/Makefile.arm    |  1 -
>  arm/Makefile.common |  3 +++
>  configure           | 35 ++++++++++++++++++++++++++---------
>  3 files changed, 29 insertions(+), 10 deletions(-)
> 
> 
> base-commit: 0ed2cdf3c80ee803b9150898e687e77e4d6f5db2
> -- 2.47.1
> 

Re: [kvm-unit-tests PATCH v1 1/5] configure: Document that the architecture name 'aarch64' is also supported

[Andrew Jones]

On Fri, Jan 10, 2025 at 01:58:44PM +0000, Alexandru Elisei wrote:
> $arch, on arm64, defaults to 'aarch64', and later in the script is replaced
> by 'arm64'. Intentional or not, document that the name 'aarch64' is also
> supported when configuring for the arm64 architecture. This has been the
> case since the initial commit that added support for the arm64
> architecture, commit 39ac3f8494be ("arm64: initial drop").
> 
> The help text for --arch changes from*:
> 
>    --arch=ARCH            architecture to compile for (aarch64). ARCH can be one of:
>                            arm, arm64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> 
> to:
> 
>     --arch=ARCH            architecture to compile for (aarch64). ARCH can be one of:
>                            arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> 
> *Worth pointing out that the default architecture is 'aarch64', even though
> the rest of the help text doesn't have it as one of the supported
> architectures.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 86cf1da36467..5b0a2d7f39c0 100755
> --- a/configure
> +++ b/configure
> @@ -47,7 +47,7 @@ usage() {
>  
>  	Options include:
>  	    --arch=ARCH            architecture to compile for ($arch). ARCH can be one of:
> -	                           arm, arm64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> +	                           arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
>  	    --processor=PROCESSOR  processor to compile for ($arch)
>  	    --target=TARGET        target platform that the tests will be running on (qemu or
>  	                           kvmtool, default is qemu) (arm/arm64 only)
> -- 
> 2.47.1
>

I'd prefer to support --arch=aarch64, but then always refer to it as only
arm64 everywhere else. We need to support arch=aarch64 since that's what
'uname -m' returns, but I don't think we need to change the help text for
it. If we don't want to trust our users to figure out arm64==aarch64,
then we can do something like

@@ -216,12 +197,12 @@ while [[ $optno -le $argc ]]; do
            werror=
            ;;
        --help)
-           usage
+           do_help=1
            ;;
        *)
            echo "Unknown option '$opt'"
            echo
-           usage
+           do_help=1
            ;;
     esac
 done

And then only do

 if [ $do_help ]; then
    usage
 fi

after $arch and other variables have had a chance to be converted.

Thanks,
drew

Re: [kvm-unit-tests PATCH v1 2/5] configure: Display the default processor for arm and arm64

[Andrew Jones]

On Fri, Jan 10, 2025 at 01:58:45PM +0000, Alexandru Elisei wrote:
> The help text for the --processor option displays the architecture name as
> the default processor type. But the default for arm is cortex-a15, and for
> arm64 is cortex-a57. Teach configure to display the correct default
> processor type for these two architectures.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  configure | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/configure b/configure
> index 5b0a2d7f39c0..138840c3f76d 100755
> --- a/configure
> +++ b/configure
> @@ -5,6 +5,24 @@ if [ -z "${BASH_VERSINFO[0]}" ] || [ "${BASH_VERSINFO[0]}" -lt 4 ] ; then
>      exit 1
>  fi
>  
> +function get_default_processor()
> +{
> +    local arch="$1"
> +
> +    case "$arch" in
> +    "arm")
> +        default_processor="cortex-a15"
> +        ;;
> +    "arm64" | "aarch64")
> +        default_processor="cortex-a57"
> +        ;;
> +    *)
> +        default_processor=$arch
> +    esac
> +
> +    echo "$default_processor"
> +}
> +
>  srcdir=$(cd "$(dirname "$0")"; pwd)
>  prefix=/usr/local
>  cc=gcc
> @@ -33,6 +51,7 @@ page_size=
>  earlycon=
>  efi=
>  efi_direct=
> +default_processor=$(get_default_processor $arch)
>  
>  # Enable -Werror by default for git repositories only (i.e. developer builds)
>  if [ -e "$srcdir"/.git ]; then
> @@ -48,7 +67,7 @@ usage() {
>  	Options include:
>  	    --arch=ARCH            architecture to compile for ($arch). ARCH can be one of:
>  	                           arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> -	    --processor=PROCESSOR  processor to compile for ($arch)
> +	    --processor=PROCESSOR  processor to compile for ($default_processor)
>  	    --target=TARGET        target platform that the tests will be running on (qemu or
>  	                           kvmtool, default is qemu) (arm/arm64 only)
>  	    --cross-prefix=PREFIX  cross compiler prefix
> @@ -283,13 +302,8 @@ else
>      fi
>  fi
>  
> -[ -z "$processor" ] && processor="$arch"
> -
> -if [ "$processor" = "arm64" ]; then
> -    processor="cortex-a57"
> -elif [ "$processor" = "arm" ]; then
> -    processor="cortex-a15"
> -fi
> +# $arch will have changed when cross-compiling.
> +[ -z "$processor" ] && processor=$(get_default_processor $arch)

The fact that $arch and $processor are wrong until they've had a chance to
be converted might be another reason for the $do_help idea. But it'll
always be fragile since another change that does some sort of conversion
could end up getting added after the '[ $do_help ] && usage' someday.

Thanks,
drew

>  
>  if [ "$arch" = "i386" ] || [ "$arch" = "x86_64" ]; then
>      testdir=x86
> -- 
> 2.47.1
> 
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [kvm-unit-tests PATCH v1 3/5] arm64: Implement the ./configure --processor option

[Andrew Jones]

On Fri, Jan 10, 2025 at 01:58:46PM +0000, Alexandru Elisei wrote:
> The help text for the ./configure --processor option says:
> 
>     --processor=PROCESSOR  processor to compile for (cortex-a57)
> 
> but, unlike arm, the build system does not pass a -mcpu argument to the
> compiler. Fix it, and bring arm64 at parity with arm.
> 
> Note that this introduces a regression, which is also present on arm: if
> the --processor argument is something that the compiler doesn't understand,
> but qemu does (like 'max'), then compilation fails. This will be fixed in a
> following patch; another fix is to specify a CPU model that gcc implements
> by using --cflags=-mcpu=<cpu>.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arm/Makefile.arm    | 1 -
>  arm/Makefile.common | 1 +
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index 7fd39f3ada64..d6250b7fb686 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -12,7 +12,6 @@ $(error Cannot build arm32 tests as EFI apps)
>  endif
>  
>  CFLAGS += $(machine)
> -CFLAGS += -mcpu=$(PROCESSOR)
>  CFLAGS += -mno-unaligned-access
>  
>  ifeq ($(TARGET),qemu)
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index f828dbe01d33..a5d97bcf477a 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -25,6 +25,7 @@ AUXFLAGS ?= 0x0
>  # stack.o relies on frame pointers.
>  KEEP_FRAME_POINTER := y
>  
> +CFLAGS += -mcpu=$(PROCESSOR)
>  CFLAGS += -std=gnu99
>  CFLAGS += -ffreestanding
>  CFLAGS += -O2
> -- 
> 2.47.1
>

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

Re: [kvm-unit-tests PATCH v1 4/5] arm/arm64: Add support for --processor=max

[Andrew Jones]

On Fri, Jan 10, 2025 at 01:58:47PM +0000, Alexandru Elisei wrote:
> For arm64, newer architecture features are supported only on newer CPUs.
> Instead of expecting the user to know which CPU model supports which
> feature when using the TCG accelerator for qemu, let's make it easier and
> add support for the --processor 'max' value.
> 
> The --processor value is passed to the compiler's -mcpu argument and to
> qemu's -cpu argument. 'max' is a special value that only qemu understands -
> it means that all CPU features that qemu implements are supported by the
> guest CPU, and passing it to the compiler causes a build error. So omit the
> -mcpu argument when $PROCESSOR=max.
> 
> This affects only the TCG accelerator; when using KVM or HVF,
> kvm-unit-tests sets the cpu model to 'host'.
> 
> Note that using --processor=max with a 32 bit compiler will cause a build
> error: the CPU model that the compiler defaults to when the -mcpu argument
> is missing lacks support for some of the instructions that kvm-unit-tests
> uses. The solution in the case is to specify a CPU model for the compiler
> using --cflags:
> 
>   ./configure --arch=arm --processor=max --cflags=-mcpu=<cpu>
> 
> This patch doesn't introduce a regression for arm when --processor=max is
> used, it's only the error that changes: from an unknown processor type to
> using instructions that are not available on the processor.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arm/Makefile.common | 2 ++
>  configure           | 5 ++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index a5d97bcf477a..b757250dc9ae 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -25,7 +25,9 @@ AUXFLAGS ?= 0x0
>  # stack.o relies on frame pointers.
>  KEEP_FRAME_POINTER := y
>  
> +ifneq ($(PROCESSOR),max)
>  CFLAGS += -mcpu=$(PROCESSOR)
> +endif
>  CFLAGS += -std=gnu99
>  CFLAGS += -ffreestanding
>  CFLAGS += -O2
> diff --git a/configure b/configure
> index 138840c3f76d..46964d36a7d8 100755
> --- a/configure
> +++ b/configure
> @@ -67,7 +67,10 @@ usage() {
>  	Options include:
>  	    --arch=ARCH            architecture to compile for ($arch). ARCH can be one of:
>  	                           arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> -	    --processor=PROCESSOR  processor to compile for ($default_processor)
> +	    --processor=PROCESSOR  processor to compile for ($default_processor). For arm and arm64, the
> +	                           value 'max' is special and it will be passed directly to
> +	                           qemu, bypassing the compiler. In this case, --cflags can be
> +	                           used to compile for a specific processor.
>  	    --target=TARGET        target platform that the tests will be running on (qemu or
>  	                           kvmtool, default is qemu) (arm/arm64 only)
>  	    --cross-prefix=PREFIX  cross compiler prefix
> -- 
> 2.47.1
>

I don't think we want to overload processor this way. While mcpu and QEMU
could both understand the same cpu names, then it was mostly fine
(although it probably shouldn't have been overloaded before either). Now
that we want one name for compiling and another for running, then I think
we need another configure parameter, something like --qemu-cpu.

Thanks,
drew

> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [kvm-unit-tests PATCH v1 5/5] configure: arm64: Make 'max' the default for --processor

[Andrew Jones]

On Fri, Jan 10, 2025 at 01:58:48PM +0000, Alexandru Elisei wrote:
> Newer architecture features are supported by qemu TCG on newer CPUs. When
> writing a test for such architecture features, it is necessary to pass the
> correct -cpu argument to qemu. Make it easier on users and test authors
> alike by making 'max' the default value for --processor. The 'max' CPU
> model contains all the features of the cortex-a57 CPU (the old default), so
> no regression should be possible.
> 
> A side effect is that, by default, the compiler will not receive a -mcpu
> argument for compiling the code. The expectation is that this is fine,
> since support for -mcpu=$PROCESSOR has only been added for arm64 in the
> last commit.
> 
> The default for arm (cortex-a15) has been kept unchanged, because passing
> --processor=max will cause compilation to break. If the user wants the qemu
> CPU model to be 'max', the user will also have to supply a suitable compile
> CPU target via --cflags=-mcpu=<cpu> configure option.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 46964d36a7d8..3ab0ec208e10 100755
> --- a/configure
> +++ b/configure
> @@ -14,7 +14,7 @@ function get_default_processor()
>          default_processor="cortex-a15"
>          ;;
>      "arm64" | "aarch64")
> -        default_processor="cortex-a57"
> +        default_processor="max"
>          ;;
>      *)
>          default_processor=$arch
> -- 
> 2.47.1
>

Another reason to introduce a new parameter (qemu_cpu) is that we can also
change arm32 to 'max', reducing divergence between arm32 and arm64.

Thanks,
drew

Re: [kvm-unit-tests PATCH v1 1/5] configure: Document that the architecture name 'aarch64' is also supported

[Alexandru Elisei]

Hi Drew,

On Mon, Jan 13, 2025 at 04:01:58PM +0100, Andrew Jones wrote:
> On Fri, Jan 10, 2025 at 01:58:44PM +0000, Alexandru Elisei wrote:
> > $arch, on arm64, defaults to 'aarch64', and later in the script is replaced
> > by 'arm64'. Intentional or not, document that the name 'aarch64' is also
> > supported when configuring for the arm64 architecture. This has been the
> > case since the initial commit that added support for the arm64
> > architecture, commit 39ac3f8494be ("arm64: initial drop").
> > 
> > The help text for --arch changes from*:
> > 
> >    --arch=ARCH            architecture to compile for (aarch64). ARCH can be one of:
> >                            arm, arm64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> > 
> > to:
> > 
> >     --arch=ARCH            architecture to compile for (aarch64). ARCH can be one of:
> >                            arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> > 
> > *Worth pointing out that the default architecture is 'aarch64', even though
> > the rest of the help text doesn't have it as one of the supported
> > architectures.
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  configure | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/configure b/configure
> > index 86cf1da36467..5b0a2d7f39c0 100755
> > --- a/configure
> > +++ b/configure
> > @@ -47,7 +47,7 @@ usage() {
> >  
> >  	Options include:
> >  	    --arch=ARCH            architecture to compile for ($arch). ARCH can be one of:
> > -	                           arm, arm64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> > +	                           arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> >  	    --processor=PROCESSOR  processor to compile for ($arch)
> >  	    --target=TARGET        target platform that the tests will be running on (qemu or
> >  	                           kvmtool, default is qemu) (arm/arm64 only)
> > -- 
> > 2.47.1
> >
> 
> I'd prefer to support --arch=aarch64, but then always refer to it as only
> arm64 everywhere else. We need to support arch=aarch64 since that's what
> 'uname -m' returns, but I don't think we need to change the help text for
> it. If we don't want to trust our users to figure out arm64==aarch64,

I sincerely dislike the fact that in the help text the default architecture on
arm64 is not among the list of supported architectures.

> then we can do something like
> 
> @@ -216,12 +197,12 @@ while [[ $optno -le $argc ]]; do
>             werror=
>             ;;
>         --help)
> -           usage
> +           do_help=1
>             ;;
>         *)
>             echo "Unknown option '$opt'"
>             echo
> -           usage
> +           do_help=1
>             ;;
>      esac
>  done
> 
> And then only do
> 
>  if [ $do_help ]; then
>     usage
>  fi
> 
> after $arch and other variables have had a chance to be converted.

That still doesn't work if displaying the help text on an arm64 board:
$arch=aarch64 if compiling natively, because that's what uname -m prints, and
$arch gets converted to 'arm64' later in the script. We could move the
conversion before calling usage, but at that point I wonder if it wouldn't be
better to never set $arch to 'aarch64' in the first place.

If you don't want to modify the help text to say that aarch64 is supported, even
though it's displayed as the default architecture on arm64, we could modify
$arch to never be set to 'aarch64', i.e:

diff --git a/configure b/configure
index 86cf1da36467..1362b68dd68b 100755
--- a/configure
+++ b/configure
@@ -15,8 +15,8 @@ objdump=objdump
 readelf=readelf
 ar=ar
 addr2line=addr2line
-arch=$(uname -m | sed -e 's/i.86/i386/;s/arm64/aarch64/;s/arm.*/arm/;s/ppc64.*/ppc64/')
-host=$arch
+host=$(uname -m | sed -e 's/i.86/i386/;s/arm64/aarch64/;s/arm.*/arm/;s/ppc64.*/ppc64/')
+arch=$(echo $host | sed -e 's/aarch64/arm64/')
 cross_prefix=
 endian=""
 pretty_print_stacks=yes

and keep the conversion from aarch64 to arm64 where it is, and still keep it
undocumented, just in case someone is using that.

($host still needs to be aarch64, because that's the name of the qemu
executable).

Thanks,
Alex

Re: [kvm-unit-tests PATCH v1 2/5] configure: Display the default processor for arm and arm64

[Alexandru Elisei]

Hi Drew,

On Mon, Jan 13, 2025 at 04:11:06PM +0100, Andrew Jones wrote:
> On Fri, Jan 10, 2025 at 01:58:45PM +0000, Alexandru Elisei wrote:
> > The help text for the --processor option displays the architecture name as
> > the default processor type. But the default for arm is cortex-a15, and for
> > arm64 is cortex-a57. Teach configure to display the correct default
> > processor type for these two architectures.
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  configure | 30 ++++++++++++++++++++++--------
> >  1 file changed, 22 insertions(+), 8 deletions(-)
> > 
> > diff --git a/configure b/configure
> > index 5b0a2d7f39c0..138840c3f76d 100755
> > --- a/configure
> > +++ b/configure
> > @@ -5,6 +5,24 @@ if [ -z "${BASH_VERSINFO[0]}" ] || [ "${BASH_VERSINFO[0]}" -lt 4 ] ; then
> >      exit 1
> >  fi
> >  
> > +function get_default_processor()
> > +{
> > +    local arch="$1"
> > +
> > +    case "$arch" in
> > +    "arm")
> > +        default_processor="cortex-a15"
> > +        ;;
> > +    "arm64" | "aarch64")
> > +        default_processor="cortex-a57"
> > +        ;;
> > +    *)
> > +        default_processor=$arch
> > +    esac
> > +
> > +    echo "$default_processor"
> > +}
> > +
> >  srcdir=$(cd "$(dirname "$0")"; pwd)
> >  prefix=/usr/local
> >  cc=gcc
> > @@ -33,6 +51,7 @@ page_size=
> >  earlycon=
> >  efi=
> >  efi_direct=
> > +default_processor=$(get_default_processor $arch)
> >  
> >  # Enable -Werror by default for git repositories only (i.e. developer builds)
> >  if [ -e "$srcdir"/.git ]; then
> > @@ -48,7 +67,7 @@ usage() {
> >  	Options include:
> >  	    --arch=ARCH            architecture to compile for ($arch). ARCH can be one of:
> >  	                           arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> > -	    --processor=PROCESSOR  processor to compile for ($arch)
> > +	    --processor=PROCESSOR  processor to compile for ($default_processor)
> >  	    --target=TARGET        target platform that the tests will be running on (qemu or
> >  	                           kvmtool, default is qemu) (arm/arm64 only)
> >  	    --cross-prefix=PREFIX  cross compiler prefix
> > @@ -283,13 +302,8 @@ else
> >      fi
> >  fi
> >  
> > -[ -z "$processor" ] && processor="$arch"
> > -
> > -if [ "$processor" = "arm64" ]; then
> > -    processor="cortex-a57"
> > -elif [ "$processor" = "arm" ]; then
> > -    processor="cortex-a15"
> > -fi
> > +# $arch will have changed when cross-compiling.
> > +[ -z "$processor" ] && processor=$(get_default_processor $arch)
> 
> The fact that $arch and $processor are wrong until they've had a chance to

$processor is never wrong. $processor is unset until either the user sets it
with --processor, or until this line. This patch introduces $default_processor
only for the purpose of having an accurate help text, it doesn't change when and
how $processor is assigned.

> be converted might be another reason for the $do_help idea. But it'll
> always be fragile since another change that does some sort of conversion
> could end up getting added after the '[ $do_help ] && usage' someday.

configure needs to distinguish between:

1. The user not having specified --processor when doing ./configure.
2. The user having set --processor.

If 1, then kvm-unit-tests can use the default $processor value for $arch,
which could have also been specified by the user.

If 2, then kvm-unit-tests should not touch $processor because that's what the
user wants.

Do you see something wrong with that reasoning?

Also, I don't understand why you say it's fragile, since configure doesn't
touch $processor until this point (and unless the user sets it, of course).

Thanks,
Alex

Re: [kvm-unit-tests PATCH v1 4/5] arm/arm64: Add support for --processor=max

[Alexandru Elisei]

Hi Drew,

On Mon, Jan 13, 2025 at 04:21:45PM +0100, Andrew Jones wrote:
> On Fri, Jan 10, 2025 at 01:58:47PM +0000, Alexandru Elisei wrote:
> > For arm64, newer architecture features are supported only on newer CPUs.
> > Instead of expecting the user to know which CPU model supports which
> > feature when using the TCG accelerator for qemu, let's make it easier and
> > add support for the --processor 'max' value.
> > 
> > The --processor value is passed to the compiler's -mcpu argument and to
> > qemu's -cpu argument. 'max' is a special value that only qemu understands -
> > it means that all CPU features that qemu implements are supported by the
> > guest CPU, and passing it to the compiler causes a build error. So omit the
> > -mcpu argument when $PROCESSOR=max.
> > 
> > This affects only the TCG accelerator; when using KVM or HVF,
> > kvm-unit-tests sets the cpu model to 'host'.
> > 
> > Note that using --processor=max with a 32 bit compiler will cause a build
> > error: the CPU model that the compiler defaults to when the -mcpu argument
> > is missing lacks support for some of the instructions that kvm-unit-tests
> > uses. The solution in the case is to specify a CPU model for the compiler
> > using --cflags:
> > 
> >   ./configure --arch=arm --processor=max --cflags=-mcpu=<cpu>
> > 
> > This patch doesn't introduce a regression for arm when --processor=max is
> > used, it's only the error that changes: from an unknown processor type to
> > using instructions that are not available on the processor.
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  arm/Makefile.common | 2 ++
> >  configure           | 5 ++++-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arm/Makefile.common b/arm/Makefile.common
> > index a5d97bcf477a..b757250dc9ae 100644
> > --- a/arm/Makefile.common
> > +++ b/arm/Makefile.common
> > @@ -25,7 +25,9 @@ AUXFLAGS ?= 0x0
> >  # stack.o relies on frame pointers.
> >  KEEP_FRAME_POINTER := y
> >  
> > +ifneq ($(PROCESSOR),max)
> >  CFLAGS += -mcpu=$(PROCESSOR)
> > +endif
> >  CFLAGS += -std=gnu99
> >  CFLAGS += -ffreestanding
> >  CFLAGS += -O2
> > diff --git a/configure b/configure
> > index 138840c3f76d..46964d36a7d8 100755
> > --- a/configure
> > +++ b/configure
> > @@ -67,7 +67,10 @@ usage() {
> >  	Options include:
> >  	    --arch=ARCH            architecture to compile for ($arch). ARCH can be one of:
> >  	                           arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> > -	    --processor=PROCESSOR  processor to compile for ($default_processor)
> > +	    --processor=PROCESSOR  processor to compile for ($default_processor). For arm and arm64, the
> > +	                           value 'max' is special and it will be passed directly to
> > +	                           qemu, bypassing the compiler. In this case, --cflags can be
> > +	                           used to compile for a specific processor.
> >  	    --target=TARGET        target platform that the tests will be running on (qemu or
> >  	                           kvmtool, default is qemu) (arm/arm64 only)
> >  	    --cross-prefix=PREFIX  cross compiler prefix
> > -- 
> > 2.47.1
> >
> 
> I don't think we want to overload processor this way. While mcpu and QEMU
> could both understand the same cpu names, then it was mostly fine
> (although it probably shouldn't have been overloaded before either). Now
> that we want one name for compiling and another for running, then I think
> we need another configure parameter, something like --qemu-cpu.

I agree, that's a better approach than overloading --processor. I'll try that
for the next iteration.

Thanks,
Alex

Re: [kvm-unit-tests PATCH v1 5/5] configure: arm64: Make 'max' the default for --processor

[Alexandru Elisei]

Hi Drew,

On Mon, Jan 13, 2025 at 04:29:21PM +0100, Andrew Jones wrote:
> On Fri, Jan 10, 2025 at 01:58:48PM +0000, Alexandru Elisei wrote:
> > Newer architecture features are supported by qemu TCG on newer CPUs. When
> > writing a test for such architecture features, it is necessary to pass the
> > correct -cpu argument to qemu. Make it easier on users and test authors
> > alike by making 'max' the default value for --processor. The 'max' CPU
> > model contains all the features of the cortex-a57 CPU (the old default), so
> > no regression should be possible.
> > 
> > A side effect is that, by default, the compiler will not receive a -mcpu
> > argument for compiling the code. The expectation is that this is fine,
> > since support for -mcpu=$PROCESSOR has only been added for arm64 in the
> > last commit.
> > 
> > The default for arm (cortex-a15) has been kept unchanged, because passing
> > --processor=max will cause compilation to break. If the user wants the qemu
> > CPU model to be 'max', the user will also have to supply a suitable compile
> > CPU target via --cflags=-mcpu=<cpu> configure option.
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  configure | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/configure b/configure
> > index 46964d36a7d8..3ab0ec208e10 100755
> > --- a/configure
> > +++ b/configure
> > @@ -14,7 +14,7 @@ function get_default_processor()
> >          default_processor="cortex-a15"
> >          ;;
> >      "arm64" | "aarch64")
> > -        default_processor="cortex-a57"
> > +        default_processor="max"
> >          ;;
> >      *)
> >          default_processor=$arch
> > -- 
> > 2.47.1
> >
> 
> Another reason to introduce a new parameter (qemu_cpu) is that we can also
> change arm32 to 'max', reducing divergence between arm32 and arm64.

That sounds very sensible, I'll do that for the next iteration.

Thanks,
Alex

Re: [kvm-unit-tests PATCH v1 1/5] configure: Document that the architecture name 'aarch64' is also supported

[Andrew Jones]

On Tue, Jan 14, 2025 at 05:03:20PM +0000, Alexandru Elisei wrote:
...
> diff --git a/configure b/configure
> index 86cf1da36467..1362b68dd68b 100755
> --- a/configure
> +++ b/configure
> @@ -15,8 +15,8 @@ objdump=objdump
>  readelf=readelf
>  ar=ar
>  addr2line=addr2line
> -arch=$(uname -m | sed -e 's/i.86/i386/;s/arm64/aarch64/;s/arm.*/arm/;s/ppc64.*/ppc64/')
> -host=$arch
> +host=$(uname -m | sed -e 's/i.86/i386/;s/arm64/aarch64/;s/arm.*/arm/;s/ppc64.*/ppc64/')
> +arch=$(echo $host | sed -e 's/aarch64/arm64/')

Sure, or avoid the second sed and just do

host=$(...)
arch=$host
[ "$arch" = "aarch64" ] && arch="arm64"

Thanks,
drew

Re: [kvm-unit-tests PATCH v1 2/5] configure: Display the default processor for arm and arm64

[Andrew Jones]

On Tue, Jan 14, 2025 at 05:17:28PM +0000, Alexandru Elisei wrote:
...
> > > +# $arch will have changed when cross-compiling.
> > > +[ -z "$processor" ] && processor=$(get_default_processor $arch)
> > 
> > The fact that $arch and $processor are wrong until they've had a chance to
> 
> $processor is never wrong. $processor is unset until either the user sets it
> with --processor, or until this line. This patch introduces $default_processor
> only for the purpose of having an accurate help text, it doesn't change when and
> how $processor is assigned.

I should have said "The fact that $arch and $default_processor are wrong..."

> 
> > be converted might be another reason for the $do_help idea. But it'll
> > always be fragile since another change that does some sort of conversion
> > could end up getting added after the '[ $do_help ] && usage' someday.
> 
> configure needs to distinguish between:
> 
> 1. The user not having specified --processor when doing ./configure.
> 2. The user having set --processor.
> 
> If 1, then kvm-unit-tests can use the default $processor value for $arch,
> which could have also been specified by the user.
> 
> If 2, then kvm-unit-tests should not touch $processor because that's what the
> user wants.
> 
> Do you see something wrong with that reasoning?

If we output $default_processor in usage() before it's had a chance to be
set correctly based on a given cross arch, then it won't display the
correct name.

> 
> Also, I don't understand why you say it's fragile, since configure doesn't

I wrote "it'll always be fragile" where 'it' refers to the most recent
object of my paragraph ("the $do_help idea"). But, TBH, I'm not sure
how important it is to get the help text accurate, so we can just not
care if we call usage() with the wrong strings sometimes.

Thanks,
drew

> touch $processor until this point (and unless the user sets it, of course).
> 
> Thanks,
> Alex
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [kvm-unit-tests PATCH v1 2/5] configure: Display the default processor for arm and arm64

[Alexandru Elisei]

Hi Drew,

On Tue, Jan 14, 2025 at 07:51:04PM +0100, Andrew Jones wrote:
> On Tue, Jan 14, 2025 at 05:17:28PM +0000, Alexandru Elisei wrote:
> ...
> > > > +# $arch will have changed when cross-compiling.
> > > > +[ -z "$processor" ] && processor=$(get_default_processor $arch)
> > > 
> > > The fact that $arch and $processor are wrong until they've had a chance to
> > 
> > $processor is never wrong. $processor is unset until either the user sets it
> > with --processor, or until this line. This patch introduces $default_processor
> > only for the purpose of having an accurate help text, it doesn't change when and
> > how $processor is assigned.
> 
> I should have said "The fact that $arch and $default_processor are wrong..."
> 
> > 
> > > be converted might be another reason for the $do_help idea. But it'll
> > > always be fragile since another change that does some sort of conversion
> > > could end up getting added after the '[ $do_help ] && usage' someday.
> > 
> > configure needs to distinguish between:
> > 
> > 1. The user not having specified --processor when doing ./configure.
> > 2. The user having set --processor.
> > 
> > If 1, then kvm-unit-tests can use the default $processor value for $arch,
> > which could have also been specified by the user.
> > 
> > If 2, then kvm-unit-tests should not touch $processor because that's what the
> > user wants.
> > 
> > Do you see something wrong with that reasoning?
> 
> If we output $default_processor in usage() before it's had a chance to be
> set correctly based on a given cross arch, then it won't display the
> correct name.
> 
> > 
> > Also, I don't understand why you say it's fragile, since configure doesn't
> 
> I wrote "it'll always be fragile" where 'it' refers to the most recent
> object of my paragraph ("the $do_help idea"). But, TBH, I'm not sure
> how important it is to get the help text accurate, so we can just not
> care if we call usage() with the wrong strings sometimes.

Got it now, thanks for explaining it.

My opinion is that a help text is there to help the user, and in my experience
an inaccurate help text can be very frustrating - think comments that say
one thing, and the code does something else.

How about this:

diff --git a/configure b/configure
index 3ab0ec208e10..5dbe189816b2 100755
--- a/configure
+++ b/configure
@@ -51,7 +51,6 @@ page_size=
 earlycon=
 efi=
 efi_direct=
-default_processor=$(get_default_processor $arch)

 # Enable -Werror by default for git repositories only (i.e. developer builds)
 if [ -e "$srcdir"/.git ]; then
@@ -61,13 +60,14 @@ else
 fi

 usage() {
+    [ -z "$processor" ] && processor=$(get_default_processor $arch)
     cat <<-EOF
        Usage: $0 [options]

        Options include:
            --arch=ARCH            architecture to compile for ($arch). ARCH can be one of:
                                   arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
-           --processor=PROCESSOR  processor to compile for ($default_processor). For arm and arm64, the
+           --processor=PROCESSOR  processor to compile for ($processor). For arm and arm64, the
                                   value 'max' is special and it will be passed directly to
                                   qemu, bypassing the compiler. In this case, --cflags can be
                                   used to compile for a specific processor.

Should be accurate enough, as far as I can tell. And I don't think there's
a need for $do_help: if the user does ./configure --help --arch=arm64, then
I think it's reasonable to expect that --help will be interpreted before
--arch is parsed.

Thanks,
Alex

Re: [kvm-unit-tests PATCH v1 1/5] configure: Document that the architecture name 'aarch64' is also supported

[Alexandru Elisei]

Hi Drew,

On Tue, Jan 14, 2025 at 07:39:49PM +0100, Andrew Jones wrote:
> On Tue, Jan 14, 2025 at 05:03:20PM +0000, Alexandru Elisei wrote:
> ...
> > diff --git a/configure b/configure
> > index 86cf1da36467..1362b68dd68b 100755
> > --- a/configure
> > +++ b/configure
> > @@ -15,8 +15,8 @@ objdump=objdump
> >  readelf=readelf
> >  ar=ar
> >  addr2line=addr2line
> > -arch=$(uname -m | sed -e 's/i.86/i386/;s/arm64/aarch64/;s/arm.*/arm/;s/ppc64.*/ppc64/')
> > -host=$arch
> > +host=$(uname -m | sed -e 's/i.86/i386/;s/arm64/aarch64/;s/arm.*/arm/;s/ppc64.*/ppc64/')
> > +arch=$(echo $host | sed -e 's/aarch64/arm64/')
> 
> Sure, or avoid the second sed and just do
> 
> host=$(...)
> arch=$host
> [ "$arch" = "aarch64" ] && arch="arm64"

Yep, thanks.

Alex

Re: [kvm-unit-tests PATCH v1 2/5] configure: Display the default processor for arm and arm64

[Andrew Jones]

On Wed, Jan 15, 2025 at 09:55:14AM +0000, Alexandru Elisei wrote:
> Hi Drew,
> 
> On Tue, Jan 14, 2025 at 07:51:04PM +0100, Andrew Jones wrote:
> > On Tue, Jan 14, 2025 at 05:17:28PM +0000, Alexandru Elisei wrote:
> > ...
> > > > > +# $arch will have changed when cross-compiling.
> > > > > +[ -z "$processor" ] && processor=$(get_default_processor $arch)
> > > > 
> > > > The fact that $arch and $processor are wrong until they've had a chance to
> > > 
> > > $processor is never wrong. $processor is unset until either the user sets it
> > > with --processor, or until this line. This patch introduces $default_processor
> > > only for the purpose of having an accurate help text, it doesn't change when and
> > > how $processor is assigned.
> > 
> > I should have said "The fact that $arch and $default_processor are wrong..."
> > 
> > > 
> > > > be converted might be another reason for the $do_help idea. But it'll
> > > > always be fragile since another change that does some sort of conversion
> > > > could end up getting added after the '[ $do_help ] && usage' someday.
> > > 
> > > configure needs to distinguish between:
> > > 
> > > 1. The user not having specified --processor when doing ./configure.
> > > 2. The user having set --processor.
> > > 
> > > If 1, then kvm-unit-tests can use the default $processor value for $arch,
> > > which could have also been specified by the user.
> > > 
> > > If 2, then kvm-unit-tests should not touch $processor because that's what the
> > > user wants.
> > > 
> > > Do you see something wrong with that reasoning?
> > 
> > If we output $default_processor in usage() before it's had a chance to be
> > set correctly based on a given cross arch, then it won't display the
> > correct name.
> > 
> > > 
> > > Also, I don't understand why you say it's fragile, since configure doesn't
> > 
> > I wrote "it'll always be fragile" where 'it' refers to the most recent
> > object of my paragraph ("the $do_help idea"). But, TBH, I'm not sure
> > how important it is to get the help text accurate, so we can just not
> > care if we call usage() with the wrong strings sometimes.
> 
> Got it now, thanks for explaining it.
> 
> My opinion is that a help text is there to help the user, and in my experience
> an inaccurate help text can be very frustrating - think comments that say
> one thing, and the code does something else.
> 
> How about this:
> 
> diff --git a/configure b/configure
> index 3ab0ec208e10..5dbe189816b2 100755
> --- a/configure
> +++ b/configure
> @@ -51,7 +51,6 @@ page_size=
>  earlycon=
>  efi=
>  efi_direct=
> -default_processor=$(get_default_processor $arch)
> 
>  # Enable -Werror by default for git repositories only (i.e. developer builds)
>  if [ -e "$srcdir"/.git ]; then
> @@ -61,13 +60,14 @@ else
>  fi
> 
>  usage() {
> +    [ -z "$processor" ] && processor=$(get_default_processor $arch)
>      cat <<-EOF
>         Usage: $0 [options]
> 
>         Options include:
>             --arch=ARCH            architecture to compile for ($arch). ARCH can be one of:
>                                    arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
> -           --processor=PROCESSOR  processor to compile for ($default_processor). For arm and arm64, the
> +           --processor=PROCESSOR  processor to compile for ($processor). For arm and arm64, the
>                                    value 'max' is special and it will be passed directly to
>                                    qemu, bypassing the compiler. In this case, --cflags can be
>                                    used to compile for a specific processor.
> 
> Should be accurate enough, as far as I can tell. And I don't think there's
> a need for $do_help: if the user does ./configure --help --arch=arm64, then
> I think it's reasonable to expect that --help will be interpreted before
> --arch is parsed.
>

Sounds good.

Thanks,
drew