* [kvm-unit-tests PATCH 0/2] arm: 32 bit tests improvements
@ 2022-03-09 16:21 ` Alexandru Elisei
0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2022-03-09 16:21 UTC (permalink / raw)
To: drjones, kvm, kvmarm, pbonzini, thuth
First patch is to allow the 32 bit tests to run under kvmtool; second patch
fixes running the 32 bit tests on an arm64 machine with KVM. Both patches
came out of a discussion on the list [1].
[1] https://www.spinics.net/lists/kvm/msg267391.html
Alexandru Elisei (1):
arm: Change text base address for 32 bit tests when running under
kvmtool
Andrew Jones (1):
arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
arm/Makefile.arm | 6 ++++++
arm/run | 5 +++++
2 files changed, 11 insertions(+)
--
2.35.1
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 16+ messages in thread
* [kvm-unit-tests PATCH 0/2] arm: 32 bit tests improvements
@ 2022-03-09 16:21 ` Alexandru Elisei
0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2022-03-09 16:21 UTC (permalink / raw)
To: drjones, kvm, kvmarm, pbonzini, thuth
First patch is to allow the 32 bit tests to run under kvmtool; second patch
fixes running the 32 bit tests on an arm64 machine with KVM. Both patches
came out of a discussion on the list [1].
[1] https://www.spinics.net/lists/kvm/msg267391.html
Alexandru Elisei (1):
arm: Change text base address for 32 bit tests when running under
kvmtool
Andrew Jones (1):
arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
arm/Makefile.arm | 6 ++++++
arm/run | 5 +++++
2 files changed, 11 insertions(+)
--
2.35.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [kvm-unit-tests PATCH 1/2] arm: Change text base address for 32 bit tests when running under kvmtool
2022-03-09 16:21 ` Alexandru Elisei
@ 2022-03-09 16:21 ` Alexandru Elisei
-1 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2022-03-09 16:21 UTC (permalink / raw)
To: drjones, kvm, kvmarm, pbonzini, thuth
The 32 bit tests do not have relocation support and rely on the build
system to set the text base address to 0x4001_0000, which is the memory
location where the test is placed by qemu. However, kvmtool loads a payload
at a different address, 0x8000_8000, when loading a test with --kernel.
When using --firmware, the default is 0x8000_0000, but that can be changed
with the --firmware-address comand line option.
When 32 bit tests are configured to run under kvmtool, set the text base
address to 0x8000_8000.
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
arm/Makefile.arm | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arm/Makefile.arm b/arm/Makefile.arm
index 3a4cc6b26234..01fd4c7bb6e2 100644
--- a/arm/Makefile.arm
+++ b/arm/Makefile.arm
@@ -14,7 +14,13 @@ CFLAGS += $(machine)
CFLAGS += -mcpu=$(PROCESSOR)
CFLAGS += -mno-unaligned-access
+ifeq ($(TARGET),qemu)
arch_LDFLAGS = -Ttext=40010000
+else ifeq ($(TARGET),kvmtool)
+arch_LDFLAGS = -Ttext=80008000
+else
+$(error Unknown target $(TARGET))
+endif
define arch_elf_check =
endef
--
2.35.1
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [kvm-unit-tests PATCH 1/2] arm: Change text base address for 32 bit tests when running under kvmtool
@ 2022-03-09 16:21 ` Alexandru Elisei
0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2022-03-09 16:21 UTC (permalink / raw)
To: drjones, kvm, kvmarm, pbonzini, thuth
The 32 bit tests do not have relocation support and rely on the build
system to set the text base address to 0x4001_0000, which is the memory
location where the test is placed by qemu. However, kvmtool loads a payload
at a different address, 0x8000_8000, when loading a test with --kernel.
When using --firmware, the default is 0x8000_0000, but that can be changed
with the --firmware-address comand line option.
When 32 bit tests are configured to run under kvmtool, set the text base
address to 0x8000_8000.
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
arm/Makefile.arm | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arm/Makefile.arm b/arm/Makefile.arm
index 3a4cc6b26234..01fd4c7bb6e2 100644
--- a/arm/Makefile.arm
+++ b/arm/Makefile.arm
@@ -14,7 +14,13 @@ CFLAGS += $(machine)
CFLAGS += -mcpu=$(PROCESSOR)
CFLAGS += -mno-unaligned-access
+ifeq ($(TARGET),qemu)
arch_LDFLAGS = -Ttext=40010000
+else ifeq ($(TARGET),kvmtool)
+arch_LDFLAGS = -Ttext=80008000
+else
+$(error Unknown target $(TARGET))
+endif
define arch_elf_check =
endef
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [kvm-unit-tests PATCH 2/2] arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
2022-03-09 16:21 ` Alexandru Elisei
@ 2022-03-09 16:21 ` Alexandru Elisei
-1 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2022-03-09 16:21 UTC (permalink / raw)
To: drjones, kvm, kvmarm, pbonzini, thuth
From: Andrew Jones <drjones@redhat.com>
KVM on arm64 can create 32 bit and 64 bit VMs. kvm-unit-tests tries to
take advantage of this by setting the aarch64=off -cpu option. However,
get_qemu_accelerator() isn't aware that KVM on arm64 can run both types
of VMs and it selects qemu-system-arm instead of qemu-system-aarch64.
This leads to an error in premature_failure() and the test is marked as
skipped:
$ ./run_tests.sh selftest-setup
SKIP selftest-setup (qemu-system-arm: -accel kvm: invalid accelerator kvm)
Fix this by setting QEMU to the correct qemu binary before calling
get_qemu_accelerator().
Signed-off-by: Andrew Jones <drjones@redhat.com>
[ Alex E: Added commit message, changed the logic to make it clearer ]
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
arm/run | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arm/run b/arm/run
index 2153bd320751..5fe0a45c4820 100755
--- a/arm/run
+++ b/arm/run
@@ -13,6 +13,11 @@ processor="$PROCESSOR"
ACCEL=$(get_qemu_accelerator) ||
exit $?
+# KVM for arm64 can create a VM in either aarch32 or aarch64 modes.
+if [ "$ACCEL" = kvm ] && [ -z "$QEMU" ] && [ "$HOST" = "aarch64" ]; then
+ QEMU=qemu-system-aarch64
+fi
+
qemu=$(search_qemu_binary) ||
exit $?
--
2.35.1
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [kvm-unit-tests PATCH 2/2] arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
@ 2022-03-09 16:21 ` Alexandru Elisei
0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2022-03-09 16:21 UTC (permalink / raw)
To: drjones, kvm, kvmarm, pbonzini, thuth
From: Andrew Jones <drjones@redhat.com>
KVM on arm64 can create 32 bit and 64 bit VMs. kvm-unit-tests tries to
take advantage of this by setting the aarch64=off -cpu option. However,
get_qemu_accelerator() isn't aware that KVM on arm64 can run both types
of VMs and it selects qemu-system-arm instead of qemu-system-aarch64.
This leads to an error in premature_failure() and the test is marked as
skipped:
$ ./run_tests.sh selftest-setup
SKIP selftest-setup (qemu-system-arm: -accel kvm: invalid accelerator kvm)
Fix this by setting QEMU to the correct qemu binary before calling
get_qemu_accelerator().
Signed-off-by: Andrew Jones <drjones@redhat.com>
[ Alex E: Added commit message, changed the logic to make it clearer ]
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
arm/run | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arm/run b/arm/run
index 2153bd320751..5fe0a45c4820 100755
--- a/arm/run
+++ b/arm/run
@@ -13,6 +13,11 @@ processor="$PROCESSOR"
ACCEL=$(get_qemu_accelerator) ||
exit $?
+# KVM for arm64 can create a VM in either aarch32 or aarch64 modes.
+if [ "$ACCEL" = kvm ] && [ -z "$QEMU" ] && [ "$HOST" = "aarch64" ]; then
+ QEMU=qemu-system-aarch64
+fi
+
qemu=$(search_qemu_binary) ||
exit $?
--
2.35.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
2022-03-09 16:21 ` Alexandru Elisei
@ 2022-03-09 16:58 ` Andrew Jones
-1 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2022-03-09 16:58 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: pbonzini, thuth, kvmarm, kvm
On Wed, Mar 09, 2022 at 04:21:17PM +0000, Alexandru Elisei wrote:
> From: Andrew Jones <drjones@redhat.com>
>
> KVM on arm64 can create 32 bit and 64 bit VMs. kvm-unit-tests tries to
> take advantage of this by setting the aarch64=off -cpu option. However,
> get_qemu_accelerator() isn't aware that KVM on arm64 can run both types
> of VMs and it selects qemu-system-arm instead of qemu-system-aarch64.
> This leads to an error in premature_failure() and the test is marked as
> skipped:
>
> $ ./run_tests.sh selftest-setup
> SKIP selftest-setup (qemu-system-arm: -accel kvm: invalid accelerator kvm)
>
> Fix this by setting QEMU to the correct qemu binary before calling
> get_qemu_accelerator().
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> [ Alex E: Added commit message, changed the logic to make it clearer ]
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> arm/run | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arm/run b/arm/run
> index 2153bd320751..5fe0a45c4820 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -13,6 +13,11 @@ processor="$PROCESSOR"
> ACCEL=$(get_qemu_accelerator) ||
> exit $?
>
> +# KVM for arm64 can create a VM in either aarch32 or aarch64 modes.
> +if [ "$ACCEL" = kvm ] && [ -z "$QEMU" ] && [ "$HOST" = "aarch64" ]; then
> + QEMU=qemu-system-aarch64
> +fi
> +
> qemu=$(search_qemu_binary) ||
> exit $?
>
> --
> 2.35.1
>
So there's a bug with this patch which was also present in the patch I
proposed. By setting $QEMU before we call search_qemu_binary() we may
force a "A QEMU binary was not found." failure even though a perfectly
good 'qemu-kvm' binary is present.
I'll try to come up with something better.
Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
@ 2022-03-09 16:58 ` Andrew Jones
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2022-03-09 16:58 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: kvm, kvmarm, pbonzini, thuth
On Wed, Mar 09, 2022 at 04:21:17PM +0000, Alexandru Elisei wrote:
> From: Andrew Jones <drjones@redhat.com>
>
> KVM on arm64 can create 32 bit and 64 bit VMs. kvm-unit-tests tries to
> take advantage of this by setting the aarch64=off -cpu option. However,
> get_qemu_accelerator() isn't aware that KVM on arm64 can run both types
> of VMs and it selects qemu-system-arm instead of qemu-system-aarch64.
> This leads to an error in premature_failure() and the test is marked as
> skipped:
>
> $ ./run_tests.sh selftest-setup
> SKIP selftest-setup (qemu-system-arm: -accel kvm: invalid accelerator kvm)
>
> Fix this by setting QEMU to the correct qemu binary before calling
> get_qemu_accelerator().
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> [ Alex E: Added commit message, changed the logic to make it clearer ]
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> arm/run | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arm/run b/arm/run
> index 2153bd320751..5fe0a45c4820 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -13,6 +13,11 @@ processor="$PROCESSOR"
> ACCEL=$(get_qemu_accelerator) ||
> exit $?
>
> +# KVM for arm64 can create a VM in either aarch32 or aarch64 modes.
> +if [ "$ACCEL" = kvm ] && [ -z "$QEMU" ] && [ "$HOST" = "aarch64" ]; then
> + QEMU=qemu-system-aarch64
> +fi
> +
> qemu=$(search_qemu_binary) ||
> exit $?
>
> --
> 2.35.1
>
So there's a bug with this patch which was also present in the patch I
proposed. By setting $QEMU before we call search_qemu_binary() we may
force a "A QEMU binary was not found." failure even though a perfectly
good 'qemu-kvm' binary is present.
I'll try to come up with something better.
Thanks,
drew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] arm: Change text base address for 32 bit tests when running under kvmtool
2022-03-09 16:21 ` Alexandru Elisei
@ 2022-03-09 17:01 ` Andrew Jones
-1 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2022-03-09 17:01 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: pbonzini, thuth, kvmarm, kvm
On Wed, Mar 09, 2022 at 04:21:16PM +0000, Alexandru Elisei wrote:
> The 32 bit tests do not have relocation support and rely on the build
> system to set the text base address to 0x4001_0000, which is the memory
> location where the test is placed by qemu. However, kvmtool loads a payload
> at a different address, 0x8000_8000, when loading a test with --kernel.
> When using --firmware, the default is 0x8000_0000, but that can be changed
> with the --firmware-address comand line option.
>
> When 32 bit tests are configured to run under kvmtool, set the text base
> address to 0x8000_8000.
>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> arm/Makefile.arm | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index 3a4cc6b26234..01fd4c7bb6e2 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -14,7 +14,13 @@ CFLAGS += $(machine)
> CFLAGS += -mcpu=$(PROCESSOR)
> CFLAGS += -mno-unaligned-access
>
> +ifeq ($(TARGET),qemu)
> arch_LDFLAGS = -Ttext=40010000
> +else ifeq ($(TARGET),kvmtool)
> +arch_LDFLAGS = -Ttext=80008000
> +else
> +$(error Unknown target $(TARGET))
> +endif
>
> define arch_elf_check =
> endef
> --
> 2.35.1
>
Applied to arm/queue,
https://gitlab.com/rhdrjones/kvm-unit-tests/-/commits/arm/queue
Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] arm: Change text base address for 32 bit tests when running under kvmtool
@ 2022-03-09 17:01 ` Andrew Jones
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2022-03-09 17:01 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: kvm, kvmarm, pbonzini, thuth
On Wed, Mar 09, 2022 at 04:21:16PM +0000, Alexandru Elisei wrote:
> The 32 bit tests do not have relocation support and rely on the build
> system to set the text base address to 0x4001_0000, which is the memory
> location where the test is placed by qemu. However, kvmtool loads a payload
> at a different address, 0x8000_8000, when loading a test with --kernel.
> When using --firmware, the default is 0x8000_0000, but that can be changed
> with the --firmware-address comand line option.
>
> When 32 bit tests are configured to run under kvmtool, set the text base
> address to 0x8000_8000.
>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> arm/Makefile.arm | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index 3a4cc6b26234..01fd4c7bb6e2 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -14,7 +14,13 @@ CFLAGS += $(machine)
> CFLAGS += -mcpu=$(PROCESSOR)
> CFLAGS += -mno-unaligned-access
>
> +ifeq ($(TARGET),qemu)
> arch_LDFLAGS = -Ttext=40010000
> +else ifeq ($(TARGET),kvmtool)
> +arch_LDFLAGS = -Ttext=80008000
> +else
> +$(error Unknown target $(TARGET))
> +endif
>
> define arch_elf_check =
> endef
> --
> 2.35.1
>
Applied to arm/queue,
https://gitlab.com/rhdrjones/kvm-unit-tests/-/commits/arm/queue
Thanks,
drew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
2022-03-09 16:58 ` Andrew Jones
@ 2022-03-09 17:12 ` Alexandru Elisei
-1 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2022-03-09 17:12 UTC (permalink / raw)
To: Andrew Jones; +Cc: pbonzini, thuth, kvmarm, kvm
Hi,
On Wed, Mar 09, 2022 at 05:58:12PM +0100, Andrew Jones wrote:
> On Wed, Mar 09, 2022 at 04:21:17PM +0000, Alexandru Elisei wrote:
> > From: Andrew Jones <drjones@redhat.com>
> >
> > KVM on arm64 can create 32 bit and 64 bit VMs. kvm-unit-tests tries to
> > take advantage of this by setting the aarch64=off -cpu option. However,
> > get_qemu_accelerator() isn't aware that KVM on arm64 can run both types
> > of VMs and it selects qemu-system-arm instead of qemu-system-aarch64.
> > This leads to an error in premature_failure() and the test is marked as
> > skipped:
> >
> > $ ./run_tests.sh selftest-setup
> > SKIP selftest-setup (qemu-system-arm: -accel kvm: invalid accelerator kvm)
> >
> > Fix this by setting QEMU to the correct qemu binary before calling
> > get_qemu_accelerator().
> >
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > [ Alex E: Added commit message, changed the logic to make it clearer ]
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> > arm/run | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arm/run b/arm/run
> > index 2153bd320751..5fe0a45c4820 100755
> > --- a/arm/run
> > +++ b/arm/run
> > @@ -13,6 +13,11 @@ processor="$PROCESSOR"
> > ACCEL=$(get_qemu_accelerator) ||
> > exit $?
> >
> > +# KVM for arm64 can create a VM in either aarch32 or aarch64 modes.
> > +if [ "$ACCEL" = kvm ] && [ -z "$QEMU" ] && [ "$HOST" = "aarch64" ]; then
> > + QEMU=qemu-system-aarch64
> > +fi
> > +
> > qemu=$(search_qemu_binary) ||
> > exit $?
> >
> > --
> > 2.35.1
> >
>
> So there's a bug with this patch which was also present in the patch I
> proposed. By setting $QEMU before we call search_qemu_binary() we may
> force a "A QEMU binary was not found." failure even though a perfectly
> good 'qemu-kvm' binary is present.
I noticed that search_qemu_binary() tries to search for both
qemu-system-ARCH_NAME and qemu-kvm, and I first thought that qemu-kvm is a
legacy name for qemu-system-ARCH_NAME.
I just did some googling, and I think it's actually how certain distros (like
SLES) package qemu-system-ARCH_NAME, is that correct?
If that is so, one idea I toyed with (for something else) is to move the error
messages from search_qemu_binary() to the call sites, that way arm/run can first
try to find qemu-system-aarch64, then fallback to qemu-kvm, and only after both
aren't found exit with an error. Just a suggestion, in case you find it useful.
Thanks,
Alex
>
> I'll try to come up with something better.
>
> Thanks,
> drew
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
@ 2022-03-09 17:12 ` Alexandru Elisei
0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2022-03-09 17:12 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, kvmarm, pbonzini, thuth
Hi,
On Wed, Mar 09, 2022 at 05:58:12PM +0100, Andrew Jones wrote:
> On Wed, Mar 09, 2022 at 04:21:17PM +0000, Alexandru Elisei wrote:
> > From: Andrew Jones <drjones@redhat.com>
> >
> > KVM on arm64 can create 32 bit and 64 bit VMs. kvm-unit-tests tries to
> > take advantage of this by setting the aarch64=off -cpu option. However,
> > get_qemu_accelerator() isn't aware that KVM on arm64 can run both types
> > of VMs and it selects qemu-system-arm instead of qemu-system-aarch64.
> > This leads to an error in premature_failure() and the test is marked as
> > skipped:
> >
> > $ ./run_tests.sh selftest-setup
> > SKIP selftest-setup (qemu-system-arm: -accel kvm: invalid accelerator kvm)
> >
> > Fix this by setting QEMU to the correct qemu binary before calling
> > get_qemu_accelerator().
> >
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > [ Alex E: Added commit message, changed the logic to make it clearer ]
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> > arm/run | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arm/run b/arm/run
> > index 2153bd320751..5fe0a45c4820 100755
> > --- a/arm/run
> > +++ b/arm/run
> > @@ -13,6 +13,11 @@ processor="$PROCESSOR"
> > ACCEL=$(get_qemu_accelerator) ||
> > exit $?
> >
> > +# KVM for arm64 can create a VM in either aarch32 or aarch64 modes.
> > +if [ "$ACCEL" = kvm ] && [ -z "$QEMU" ] && [ "$HOST" = "aarch64" ]; then
> > + QEMU=qemu-system-aarch64
> > +fi
> > +
> > qemu=$(search_qemu_binary) ||
> > exit $?
> >
> > --
> > 2.35.1
> >
>
> So there's a bug with this patch which was also present in the patch I
> proposed. By setting $QEMU before we call search_qemu_binary() we may
> force a "A QEMU binary was not found." failure even though a perfectly
> good 'qemu-kvm' binary is present.
I noticed that search_qemu_binary() tries to search for both
qemu-system-ARCH_NAME and qemu-kvm, and I first thought that qemu-kvm is a
legacy name for qemu-system-ARCH_NAME.
I just did some googling, and I think it's actually how certain distros (like
SLES) package qemu-system-ARCH_NAME, is that correct?
If that is so, one idea I toyed with (for something else) is to move the error
messages from search_qemu_binary() to the call sites, that way arm/run can first
try to find qemu-system-aarch64, then fallback to qemu-kvm, and only after both
aren't found exit with an error. Just a suggestion, in case you find it useful.
Thanks,
Alex
>
> I'll try to come up with something better.
>
> Thanks,
> drew
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
2022-03-09 17:12 ` Alexandru Elisei
@ 2022-03-10 6:59 ` Andrew Jones
-1 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2022-03-10 6:59 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: kvm, kvmarm, pbonzini, thuth
On Wed, Mar 09, 2022 at 05:12:05PM +0000, Alexandru Elisei wrote:
> Hi,
>
> On Wed, Mar 09, 2022 at 05:58:12PM +0100, Andrew Jones wrote:
> > On Wed, Mar 09, 2022 at 04:21:17PM +0000, Alexandru Elisei wrote:
> > > From: Andrew Jones <drjones@redhat.com>
> > >
> > > KVM on arm64 can create 32 bit and 64 bit VMs. kvm-unit-tests tries to
> > > take advantage of this by setting the aarch64=off -cpu option. However,
> > > get_qemu_accelerator() isn't aware that KVM on arm64 can run both types
> > > of VMs and it selects qemu-system-arm instead of qemu-system-aarch64.
> > > This leads to an error in premature_failure() and the test is marked as
> > > skipped:
> > >
> > > $ ./run_tests.sh selftest-setup
> > > SKIP selftest-setup (qemu-system-arm: -accel kvm: invalid accelerator kvm)
> > >
> > > Fix this by setting QEMU to the correct qemu binary before calling
> > > get_qemu_accelerator().
> > >
> > > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > > [ Alex E: Added commit message, changed the logic to make it clearer ]
> > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > ---
> > > arm/run | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/arm/run b/arm/run
> > > index 2153bd320751..5fe0a45c4820 100755
> > > --- a/arm/run
> > > +++ b/arm/run
> > > @@ -13,6 +13,11 @@ processor="$PROCESSOR"
> > > ACCEL=$(get_qemu_accelerator) ||
> > > exit $?
> > >
> > > +# KVM for arm64 can create a VM in either aarch32 or aarch64 modes.
> > > +if [ "$ACCEL" = kvm ] && [ -z "$QEMU" ] && [ "$HOST" = "aarch64" ]; then
> > > + QEMU=qemu-system-aarch64
> > > +fi
> > > +
> > > qemu=$(search_qemu_binary) ||
> > > exit $?
> > >
> > > --
> > > 2.35.1
> > >
> >
> > So there's a bug with this patch which was also present in the patch I
> > proposed. By setting $QEMU before we call search_qemu_binary() we may
> > force a "A QEMU binary was not found." failure even though a perfectly
> > good 'qemu-kvm' binary is present.
>
> I noticed that search_qemu_binary() tries to search for both
> qemu-system-ARCH_NAME and qemu-kvm, and I first thought that qemu-kvm is a
> legacy name for qemu-system-ARCH_NAME.
>
> I just did some googling, and I think it's actually how certain distros (like
> SLES) package qemu-system-ARCH_NAME, is that correct?
Right
>
> If that is so, one idea I toyed with (for something else) is to move the error
> messages from search_qemu_binary() to the call sites, that way arm/run can first
> try to find qemu-system-aarch64, then fallback to qemu-kvm, and only after both
> aren't found exit with an error. Just a suggestion, in case you find it useful.
We don't have to move the error messages, even if we want to use
search_qemu_binary() as a silent check. We can just call it with
a &>/dev/null and then check its return code. I still need to
allocate some time to think more about this though.
Thanks,
drew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
@ 2022-03-10 6:59 ` Andrew Jones
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2022-03-10 6:59 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: pbonzini, thuth, kvmarm, kvm
On Wed, Mar 09, 2022 at 05:12:05PM +0000, Alexandru Elisei wrote:
> Hi,
>
> On Wed, Mar 09, 2022 at 05:58:12PM +0100, Andrew Jones wrote:
> > On Wed, Mar 09, 2022 at 04:21:17PM +0000, Alexandru Elisei wrote:
> > > From: Andrew Jones <drjones@redhat.com>
> > >
> > > KVM on arm64 can create 32 bit and 64 bit VMs. kvm-unit-tests tries to
> > > take advantage of this by setting the aarch64=off -cpu option. However,
> > > get_qemu_accelerator() isn't aware that KVM on arm64 can run both types
> > > of VMs and it selects qemu-system-arm instead of qemu-system-aarch64.
> > > This leads to an error in premature_failure() and the test is marked as
> > > skipped:
> > >
> > > $ ./run_tests.sh selftest-setup
> > > SKIP selftest-setup (qemu-system-arm: -accel kvm: invalid accelerator kvm)
> > >
> > > Fix this by setting QEMU to the correct qemu binary before calling
> > > get_qemu_accelerator().
> > >
> > > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > > [ Alex E: Added commit message, changed the logic to make it clearer ]
> > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > ---
> > > arm/run | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/arm/run b/arm/run
> > > index 2153bd320751..5fe0a45c4820 100755
> > > --- a/arm/run
> > > +++ b/arm/run
> > > @@ -13,6 +13,11 @@ processor="$PROCESSOR"
> > > ACCEL=$(get_qemu_accelerator) ||
> > > exit $?
> > >
> > > +# KVM for arm64 can create a VM in either aarch32 or aarch64 modes.
> > > +if [ "$ACCEL" = kvm ] && [ -z "$QEMU" ] && [ "$HOST" = "aarch64" ]; then
> > > + QEMU=qemu-system-aarch64
> > > +fi
> > > +
> > > qemu=$(search_qemu_binary) ||
> > > exit $?
> > >
> > > --
> > > 2.35.1
> > >
> >
> > So there's a bug with this patch which was also present in the patch I
> > proposed. By setting $QEMU before we call search_qemu_binary() we may
> > force a "A QEMU binary was not found." failure even though a perfectly
> > good 'qemu-kvm' binary is present.
>
> I noticed that search_qemu_binary() tries to search for both
> qemu-system-ARCH_NAME and qemu-kvm, and I first thought that qemu-kvm is a
> legacy name for qemu-system-ARCH_NAME.
>
> I just did some googling, and I think it's actually how certain distros (like
> SLES) package qemu-system-ARCH_NAME, is that correct?
Right
>
> If that is so, one idea I toyed with (for something else) is to move the error
> messages from search_qemu_binary() to the call sites, that way arm/run can first
> try to find qemu-system-aarch64, then fallback to qemu-kvm, and only after both
> aren't found exit with an error. Just a suggestion, in case you find it useful.
We don't have to move the error messages, even if we want to use
search_qemu_binary() as a silent check. We can just call it with
a &>/dev/null and then check its return code. I still need to
allocate some time to think more about this though.
Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
2022-03-10 6:59 ` Andrew Jones
@ 2022-03-10 9:28 ` Alexandru Elisei
-1 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2022-03-10 9:28 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, kvmarm, pbonzini, thuth
Hi,
On Thu, Mar 10, 2022 at 07:59:41AM +0100, Andrew Jones wrote:
> On Wed, Mar 09, 2022 at 05:12:05PM +0000, Alexandru Elisei wrote:
> > Hi,
> >
> > On Wed, Mar 09, 2022 at 05:58:12PM +0100, Andrew Jones wrote:
> > > On Wed, Mar 09, 2022 at 04:21:17PM +0000, Alexandru Elisei wrote:
> > > > From: Andrew Jones <drjones@redhat.com>
> > > >
> > > > KVM on arm64 can create 32 bit and 64 bit VMs. kvm-unit-tests tries to
> > > > take advantage of this by setting the aarch64=off -cpu option. However,
> > > > get_qemu_accelerator() isn't aware that KVM on arm64 can run both types
> > > > of VMs and it selects qemu-system-arm instead of qemu-system-aarch64.
> > > > This leads to an error in premature_failure() and the test is marked as
> > > > skipped:
> > > >
> > > > $ ./run_tests.sh selftest-setup
> > > > SKIP selftest-setup (qemu-system-arm: -accel kvm: invalid accelerator kvm)
> > > >
> > > > Fix this by setting QEMU to the correct qemu binary before calling
> > > > get_qemu_accelerator().
> > > >
> > > > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > > > [ Alex E: Added commit message, changed the logic to make it clearer ]
> > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > > ---
> > > > arm/run | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/arm/run b/arm/run
> > > > index 2153bd320751..5fe0a45c4820 100755
> > > > --- a/arm/run
> > > > +++ b/arm/run
> > > > @@ -13,6 +13,11 @@ processor="$PROCESSOR"
> > > > ACCEL=$(get_qemu_accelerator) ||
> > > > exit $?
> > > >
> > > > +# KVM for arm64 can create a VM in either aarch32 or aarch64 modes.
> > > > +if [ "$ACCEL" = kvm ] && [ -z "$QEMU" ] && [ "$HOST" = "aarch64" ]; then
> > > > + QEMU=qemu-system-aarch64
> > > > +fi
> > > > +
> > > > qemu=$(search_qemu_binary) ||
> > > > exit $?
> > > >
> > > > --
> > > > 2.35.1
> > > >
> > >
> > > So there's a bug with this patch which was also present in the patch I
> > > proposed. By setting $QEMU before we call search_qemu_binary() we may
> > > force a "A QEMU binary was not found." failure even though a perfectly
> > > good 'qemu-kvm' binary is present.
> >
> > I noticed that search_qemu_binary() tries to search for both
> > qemu-system-ARCH_NAME and qemu-kvm, and I first thought that qemu-kvm is a
> > legacy name for qemu-system-ARCH_NAME.
> >
> > I just did some googling, and I think it's actually how certain distros (like
> > SLES) package qemu-system-ARCH_NAME, is that correct?
>
> Right
Thanks for confirming it!
>
> >
> > If that is so, one idea I toyed with (for something else) is to move the error
> > messages from search_qemu_binary() to the call sites, that way arm/run can first
> > try to find qemu-system-aarch64, then fallback to qemu-kvm, and only after both
> > aren't found exit with an error. Just a suggestion, in case you find it useful.
>
> We don't have to move the error messages, even if we want to use
> search_qemu_binary() as a silent check. We can just call it with
> a &>/dev/null and then check its return code. I still need to
> allocate some time to think more about this though.
Sure, I'll review and test whatever you come up with.
Thanks,
Alex
>
> Thanks,
> drew
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64
@ 2022-03-10 9:28 ` Alexandru Elisei
0 siblings, 0 replies; 16+ messages in thread
From: Alexandru Elisei @ 2022-03-10 9:28 UTC (permalink / raw)
To: Andrew Jones; +Cc: pbonzini, thuth, kvmarm, kvm
Hi,
On Thu, Mar 10, 2022 at 07:59:41AM +0100, Andrew Jones wrote:
> On Wed, Mar 09, 2022 at 05:12:05PM +0000, Alexandru Elisei wrote:
> > Hi,
> >
> > On Wed, Mar 09, 2022 at 05:58:12PM +0100, Andrew Jones wrote:
> > > On Wed, Mar 09, 2022 at 04:21:17PM +0000, Alexandru Elisei wrote:
> > > > From: Andrew Jones <drjones@redhat.com>
> > > >
> > > > KVM on arm64 can create 32 bit and 64 bit VMs. kvm-unit-tests tries to
> > > > take advantage of this by setting the aarch64=off -cpu option. However,
> > > > get_qemu_accelerator() isn't aware that KVM on arm64 can run both types
> > > > of VMs and it selects qemu-system-arm instead of qemu-system-aarch64.
> > > > This leads to an error in premature_failure() and the test is marked as
> > > > skipped:
> > > >
> > > > $ ./run_tests.sh selftest-setup
> > > > SKIP selftest-setup (qemu-system-arm: -accel kvm: invalid accelerator kvm)
> > > >
> > > > Fix this by setting QEMU to the correct qemu binary before calling
> > > > get_qemu_accelerator().
> > > >
> > > > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > > > [ Alex E: Added commit message, changed the logic to make it clearer ]
> > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > > ---
> > > > arm/run | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/arm/run b/arm/run
> > > > index 2153bd320751..5fe0a45c4820 100755
> > > > --- a/arm/run
> > > > +++ b/arm/run
> > > > @@ -13,6 +13,11 @@ processor="$PROCESSOR"
> > > > ACCEL=$(get_qemu_accelerator) ||
> > > > exit $?
> > > >
> > > > +# KVM for arm64 can create a VM in either aarch32 or aarch64 modes.
> > > > +if [ "$ACCEL" = kvm ] && [ -z "$QEMU" ] && [ "$HOST" = "aarch64" ]; then
> > > > + QEMU=qemu-system-aarch64
> > > > +fi
> > > > +
> > > > qemu=$(search_qemu_binary) ||
> > > > exit $?
> > > >
> > > > --
> > > > 2.35.1
> > > >
> > >
> > > So there's a bug with this patch which was also present in the patch I
> > > proposed. By setting $QEMU before we call search_qemu_binary() we may
> > > force a "A QEMU binary was not found." failure even though a perfectly
> > > good 'qemu-kvm' binary is present.
> >
> > I noticed that search_qemu_binary() tries to search for both
> > qemu-system-ARCH_NAME and qemu-kvm, and I first thought that qemu-kvm is a
> > legacy name for qemu-system-ARCH_NAME.
> >
> > I just did some googling, and I think it's actually how certain distros (like
> > SLES) package qemu-system-ARCH_NAME, is that correct?
>
> Right
Thanks for confirming it!
>
> >
> > If that is so, one idea I toyed with (for something else) is to move the error
> > messages from search_qemu_binary() to the call sites, that way arm/run can first
> > try to find qemu-system-aarch64, then fallback to qemu-kvm, and only after both
> > aren't found exit with an error. Just a suggestion, in case you find it useful.
>
> We don't have to move the error messages, even if we want to use
> search_qemu_binary() as a silent check. We can just call it with
> a &>/dev/null and then check its return code. I still need to
> allocate some time to think more about this though.
Sure, I'll review and test whatever you come up with.
Thanks,
Alex
>
> Thanks,
> drew
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2022-03-10 9:28 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-09 16:21 [kvm-unit-tests PATCH 0/2] arm: 32 bit tests improvements Alexandru Elisei
2022-03-09 16:21 ` Alexandru Elisei
2022-03-09 16:21 ` [kvm-unit-tests PATCH 1/2] arm: Change text base address for 32 bit tests when running under kvmtool Alexandru Elisei
2022-03-09 16:21 ` Alexandru Elisei
2022-03-09 17:01 ` Andrew Jones
2022-03-09 17:01 ` Andrew Jones
2022-03-09 16:21 ` [kvm-unit-tests PATCH 2/2] arm/run: Fix using qemu-system-aarch64 to run aarch32 tests on aarch64 Alexandru Elisei
2022-03-09 16:21 ` Alexandru Elisei
2022-03-09 16:58 ` Andrew Jones
2022-03-09 16:58 ` Andrew Jones
2022-03-09 17:12 ` Alexandru Elisei
2022-03-09 17:12 ` Alexandru Elisei
2022-03-10 6:59 ` Andrew Jones
2022-03-10 6:59 ` Andrew Jones
2022-03-10 9:28 ` Alexandru Elisei
2022-03-10 9:28 ` Alexandru Elisei
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.