* [PATCH] selftests: KVM: Gracefully handle missing vCPU features
@ 2021-08-18 21:29 Oliver Upton
2021-08-19 10:32 ` Andrew Jones
2021-09-21 18:00 ` Paolo Bonzini
0 siblings, 2 replies; 4+ messages in thread
From: Oliver Upton @ 2021-08-18 21:29 UTC (permalink / raw)
To: kvm, kvmarm; +Cc: Marc Zyngier, Peter Shier, Raghavendra Rao Anata
An error of ENOENT for the KVM_ARM_VCPU_INIT ioctl indicates that one of
the requested feature flags is not supported by the kernel/hardware.
Detect the case when KVM doesn't support the requested features and skip
the test rather than failing it.
Cc: Andrew Jones <drjones@redhat.com>
Signed-off-by: Oliver Upton <oupton@google.com>
---
Applies to 5.14-rc6. Tested by running all selftests on an Ampere Mt.
Jade system.
.../testing/selftests/kvm/lib/aarch64/processor.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index 632b74d6b3ca..b1064a0c5e62 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -216,6 +216,7 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *ini
{
struct kvm_vcpu_init default_init = { .target = -1, };
uint64_t sctlr_el1, tcr_el1;
+ int ret;
if (!init)
init = &default_init;
@@ -226,7 +227,19 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *ini
init->target = preferred.target;
}
- vcpu_ioctl(vm, vcpuid, KVM_ARM_VCPU_INIT, init);
+ ret = _vcpu_ioctl(vm, vcpuid, KVM_ARM_VCPU_INIT, init);
+
+ /*
+ * Missing kernel feature support should result in skipping the test,
+ * not failing it.
+ */
+ if (ret && errno == ENOENT) {
+ print_skip("requested vCPU features not supported; skipping test.");
+ exit(KSFT_SKIP);
+ }
+
+ TEST_ASSERT(!ret, "KVM_ARM_VCPU_INIT failed, rc: %i errno: %i (%s)",
+ ret, errno, strerror(errno));
/*
* Enable FP/ASIMD to avoid trapping when accessing Q0-Q15
--
2.33.0.rc1.237.g0d66db33f3-goog
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] selftests: KVM: Gracefully handle missing vCPU features
2021-08-18 21:29 [PATCH] selftests: KVM: Gracefully handle missing vCPU features Oliver Upton
@ 2021-08-19 10:32 ` Andrew Jones
2021-09-21 18:00 ` Paolo Bonzini
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Jones @ 2021-08-19 10:32 UTC (permalink / raw)
To: Oliver Upton
Cc: kvm, Marc Zyngier, Peter Shier, Raghavendra Rao Anata, kvmarm
On Wed, Aug 18, 2021 at 09:29:40PM +0000, Oliver Upton wrote:
> An error of ENOENT for the KVM_ARM_VCPU_INIT ioctl indicates that one of
> the requested feature flags is not supported by the kernel/hardware.
> Detect the case when KVM doesn't support the requested features and skip
> the test rather than failing it.
>
> Cc: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
> Applies to 5.14-rc6. Tested by running all selftests on an Ampere Mt.
> Jade system.
>
> .../testing/selftests/kvm/lib/aarch64/processor.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> index 632b74d6b3ca..b1064a0c5e62 100644
> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> @@ -216,6 +216,7 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *ini
> {
> struct kvm_vcpu_init default_init = { .target = -1, };
> uint64_t sctlr_el1, tcr_el1;
> + int ret;
>
> if (!init)
> init = &default_init;
> @@ -226,7 +227,19 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *ini
> init->target = preferred.target;
> }
>
> - vcpu_ioctl(vm, vcpuid, KVM_ARM_VCPU_INIT, init);
> + ret = _vcpu_ioctl(vm, vcpuid, KVM_ARM_VCPU_INIT, init);
> +
> + /*
> + * Missing kernel feature support should result in skipping the test,
> + * not failing it.
> + */
> + if (ret && errno == ENOENT) {
> + print_skip("requested vCPU features not supported; skipping test.");
", skipping test" will already be appended by print_skip().
> + exit(KSFT_SKIP);
> + }
> +
> + TEST_ASSERT(!ret, "KVM_ARM_VCPU_INIT failed, rc: %i errno: %i (%s)",
> + ret, errno, strerror(errno));
>
> /*
> * Enable FP/ASIMD to avoid trapping when accessing Q0-Q15
> --
> 2.33.0.rc1.237.g0d66db33f3-goog
>
I think I'd rather try to keep exit(KSFT_SKIP)'s out of the lib code. It'd
be better if the test gets to decide whether to skip or not. How about
moving this check+skip into the test instead?
Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] selftests: KVM: Gracefully handle missing vCPU features
2021-08-18 21:29 [PATCH] selftests: KVM: Gracefully handle missing vCPU features Oliver Upton
2021-08-19 10:32 ` Andrew Jones
@ 2021-09-21 18:00 ` Paolo Bonzini
2021-09-21 18:15 ` Andrew Jones
1 sibling, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2021-09-21 18:00 UTC (permalink / raw)
To: Oliver Upton, kvm, kvmarm; +Cc: Marc Zyngier, Peter Shier
On 18/08/21 23:29, Oliver Upton wrote:
> An error of ENOENT for the KVM_ARM_VCPU_INIT ioctl indicates that one of
> the requested feature flags is not supported by the kernel/hardware.
> Detect the case when KVM doesn't support the requested features and skip
> the test rather than failing it.
>
> Cc: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
> Applies to 5.14-rc6. Tested by running all selftests on an Ampere Mt.
> Jade system.
>
> .../testing/selftests/kvm/lib/aarch64/processor.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> index 632b74d6b3ca..b1064a0c5e62 100644
> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> @@ -216,6 +216,7 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *ini
> {
> struct kvm_vcpu_init default_init = { .target = -1, };
> uint64_t sctlr_el1, tcr_el1;
> + int ret;
>
> if (!init)
> init = &default_init;
> @@ -226,7 +227,19 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *ini
> init->target = preferred.target;
> }
>
> - vcpu_ioctl(vm, vcpuid, KVM_ARM_VCPU_INIT, init);
> + ret = _vcpu_ioctl(vm, vcpuid, KVM_ARM_VCPU_INIT, init);
> +
> + /*
> + * Missing kernel feature support should result in skipping the test,
> + * not failing it.
> + */
> + if (ret && errno == ENOENT) {
> + print_skip("requested vCPU features not supported; skipping test.");
> + exit(KSFT_SKIP);
> + }
> +
> + TEST_ASSERT(!ret, "KVM_ARM_VCPU_INIT failed, rc: %i errno: %i (%s)",
> + ret, errno, strerror(errno));
>
> /*
> * Enable FP/ASIMD to avoid trapping when accessing Q0-Q15
>
Queued, thanks.
Paolo
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] selftests: KVM: Gracefully handle missing vCPU features
2021-09-21 18:00 ` Paolo Bonzini
@ 2021-09-21 18:15 ` Andrew Jones
0 siblings, 0 replies; 4+ messages in thread
From: Andrew Jones @ 2021-09-21 18:15 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Marc Zyngier, Peter Shier, kvmarm
On Tue, Sep 21, 2021 at 08:00:02PM +0200, Paolo Bonzini wrote:
> On 18/08/21 23:29, Oliver Upton wrote:
> > An error of ENOENT for the KVM_ARM_VCPU_INIT ioctl indicates that one of
> > the requested feature flags is not supported by the kernel/hardware.
> > Detect the case when KVM doesn't support the requested features and skip
> > the test rather than failing it.
> >
> > Cc: Andrew Jones <drjones@redhat.com>
> > Signed-off-by: Oliver Upton <oupton@google.com>
> > ---
> > Applies to 5.14-rc6. Tested by running all selftests on an Ampere Mt.
> > Jade system.
> >
> > .../testing/selftests/kvm/lib/aarch64/processor.c | 15 ++++++++++++++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > index 632b74d6b3ca..b1064a0c5e62 100644
> > --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > @@ -216,6 +216,7 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *ini
> > {
> > struct kvm_vcpu_init default_init = { .target = -1, };
> > uint64_t sctlr_el1, tcr_el1;
> > + int ret;
> > if (!init)
> > init = &default_init;
> > @@ -226,7 +227,19 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *ini
> > init->target = preferred.target;
> > }
> > - vcpu_ioctl(vm, vcpuid, KVM_ARM_VCPU_INIT, init);
> > + ret = _vcpu_ioctl(vm, vcpuid, KVM_ARM_VCPU_INIT, init);
> > +
> > + /*
> > + * Missing kernel feature support should result in skipping the test,
> > + * not failing it.
> > + */
> > + if (ret && errno == ENOENT) {
> > + print_skip("requested vCPU features not supported; skipping test.");
> > + exit(KSFT_SKIP);
> > + }
> > +
> > + TEST_ASSERT(!ret, "KVM_ARM_VCPU_INIT failed, rc: %i errno: %i (%s)",
> > + ret, errno, strerror(errno));
> > /*
> > * Enable FP/ASIMD to avoid trapping when accessing Q0-Q15
> >
>
> Queued, thanks.
>
I'd rather we don't queue this. It'd be better, IMO, for the unit test to
probe for features and then skip the test, if that's what it wants to do,
when they're not present. I'd rather not have test skipping decisions
made in the library code, which may not be what the unit test developer
expects. Anyway, the 'skipping test' substring would be printed twice with
this patch.
Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-21 18:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18 21:29 [PATCH] selftests: KVM: Gracefully handle missing vCPU features Oliver Upton
2021-08-19 10:32 ` Andrew Jones
2021-09-21 18:00 ` Paolo Bonzini
2021-09-21 18:15 ` Andrew Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).