[PATCH v2] kselftest: kvm/arm64: Skip tests if we can't create a vgic-v3

[Andrew Jones]

On Wed, Jan 26, 2022 at 02:29:14PM +0000, Mark Brown wrote:
> On Wed, Jan 26, 2022 at 03:17:41PM +0100, Andrew Jones wrote:
> > On Wed, Jan 26, 2022 at 01:53:19PM +0000, Mark Brown wrote:
> 
> > > -	vgic_v3_setup(vm, nr_vcpus, 64, GICD_BASE_GPA, GICR_BASE_GPA);
> > > +	ret = vgic_v3_setup(vm, nr_vcpus, 64, GICD_BASE_GPA, GICR_BASE_GPA);
> > > +	if (ret < 0) {
> > > +		pr_info("Failed to create vgic-v3, skipping\n");
> 
> > Please use 'print_skip', which appends ", skipping test" to keep the skip
> > messages consistent. Also, print_skip can't be disabled with -DQUIET like
> > pr_info.
> 
> I see.  It might be nice to convert these tests to use the ksft_
> stuff...

Indeed. I'll add that to my TODO.

> 
> > > -	/* Distributor setup */
> > > +	/* Distributor setup - test if it's possible then actually do it */
> > > +	gic_fd = kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V3, true);
> > > +	if (gic_fd != 0)
> > > +		return -1;
> > >  	gic_fd = kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V3, false);
> 
> > kvm selftests generally asserts on failure with the nonunderscore
> > prefixed KVM ioctl wrapper functions, which is why you appear to
> > be forced to do this nasty dance. However, kvm selftests usually
> > always also offers an underscore prefixed version of the KVM ioctl
> > wrapper function too for cases like these. So we can just do
> 
> >   if (_kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V3, false, &gic_fd) != 0)
> >           return -1;
> 
> And the _ version is OK to use in the vgic code?  The _ makes it look
> like it's internal only.

It's extra OK. Anything in lib/* or lib/*/* is internal to the lib.
However, it's even OK for a unit test to use the _ prefixed functions.
The _ isn't so much about being internal as it is about being a raw
version of the ioctl wrapper, which returns error codes, vs. the
asserting version of the wrapper which only returns results on success.

Thanks,
drew