All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] selftests: KVM: Gracefully handle missing vCPU features
@ 2021-08-18 21:29 ` Oliver Upton
  0 siblings, 0 replies; 8+ messages in thread
From: Oliver Upton @ 2021-08-18 21:29 UTC (permalink / raw)
  To: kvm, kvmarm
  Cc: Marc Zyngier, Peter Shier, Ricardo Koller, Jing Zhang,
	Raghavendra Rao Anata, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Andrew Jones, Oliver Upton

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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH] selftests: KVM: Gracefully handle missing vCPU features
@ 2021-08-18 21:29 ` Oliver Upton
  0 siblings, 0 replies; 8+ 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] 8+ messages in thread

* Re: [PATCH] selftests: KVM: Gracefully handle missing vCPU features
  2021-08-18 21:29 ` Oliver Upton
@ 2021-08-19 10:32   ` Andrew Jones
  -1 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2021-08-19 10:32 UTC (permalink / raw)
  To: Oliver Upton
  Cc: kvm, kvmarm, Marc Zyngier, Peter Shier, Ricardo Koller,
	Jing Zhang, Raghavendra Rao Anata, James Morse, Alexandru Elisei,
	Suzuki K Poulose

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: KVM: Gracefully handle missing vCPU features
@ 2021-08-19 10:32   ` Andrew Jones
  0 siblings, 0 replies; 8+ 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] 8+ messages in thread

* Re: [PATCH] selftests: KVM: Gracefully handle missing vCPU features
  2021-08-18 21:29 ` Oliver Upton
@ 2021-09-21 18:00   ` Paolo Bonzini
  -1 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2021-09-21 18:00 UTC (permalink / raw)
  To: Oliver Upton, kvm, kvmarm
  Cc: Marc Zyngier, Peter Shier, Ricardo Koller, Jing Zhang,
	Raghavendra Rao Anata, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Andrew Jones

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: KVM: Gracefully handle missing vCPU features
@ 2021-09-21 18:00   ` Paolo Bonzini
  0 siblings, 0 replies; 8+ 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] 8+ 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
  -1 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2021-09-21 18:15 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Oliver Upton, kvm, kvmarm, Marc Zyngier, Peter Shier,
	Ricardo Koller, Jing Zhang, Raghavendra Rao Anata, James Morse,
	Alexandru Elisei, Suzuki K Poulose

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] selftests: KVM: Gracefully handle missing vCPU features
@ 2021-09-21 18:15     ` Andrew Jones
  0 siblings, 0 replies; 8+ 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] 8+ messages in thread

end of thread, other threads:[~2021-09-21 18:15 UTC | newest]

Thread overview: 8+ 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-18 21:29 ` Oliver Upton
2021-08-19 10:32 ` Andrew Jones
2021-08-19 10:32   ` Andrew Jones
2021-09-21 18:00 ` Paolo Bonzini
2021-09-21 18:00   ` Paolo Bonzini
2021-09-21 18:15   ` Andrew Jones
2021-09-21 18:15     ` Andrew Jones

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.