Re: [PATCH v2 03/10] selftests: kvm: s390: Add s390x ucontrol test suite with hpage test

[Christoph Schlameuss <schlameuss@linux.ibm.com>]

On Wed, 24 Jul 2024 16:52:00 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 7/23/24 11:31, Christoph Schlameuss wrote:
> > Add test suite to validate the s390x architecture specific ucontrol KVM
> > interface.
> > 
> > Make use of the selftest test harness.
> > 
> > * uc_cap_hpage testcase verifies that a ucontrol VM cannot be run with
> >    hugepages.
> > 
> > To allow testing of the ucontrol interface the kernel needs a
> > non-default config containing CONFIG_KVM_S390_UCONTROL.
> > This config needs to be set to built-in (y) as this cannot be built as
> > module.
> > 
> > Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > ---  
> 
> [...]
> 
> > +#include "kselftest_harness.h"
> > +#include "kvm_util.h"
> > +
> > +#include <linux/capability.h>
> > +#include <linux/sizes.h>
> > +
> > +#define SYS_ADMIN_CAP 0x200000  
> 
> This looked suspicious to me.
> Surely this would be available in some form in capability.h since 
> CAP_SYS_ADMIN is something that's regularly checked.
> 
> [...]
> 
> > +
> > +/* so directly declare capget to check caps without libcap */
> > +int capget(cap_user_header_t header, cap_user_data_t data);
> > +
> > +/**
> > + * In order to create user controlled virtual machines on S390,
> > + * check KVM_CAP_S390_UCONTROL and use the flag KVM_VM_S390_UCONTROL
> > + * as privileged user (SYS_ADMIN).
> > + */
> > +void require_ucontrol_admin(void)
> > +{
> > +	struct __user_cap_data_struct data[_LINUX_CAPABILITY_U32S_3];
> > +	struct __user_cap_header_struct hdr = {
> > +		.version = _LINUX_CAPABILITY_VERSION_3,
> > +	};
> > +	int rc;
> > +
> > +	rc = capget(&hdr, data);
> > +	TEST_ASSERT_EQ(0, rc);
> > +	TEST_REQUIRE((data->effective & SYS_ADMIN_CAP) > 0);  
> 
> And in fact capability.h does have defines which hide the magic constant:
> data->effective & CAP_TO_MASK(CAP_SYS_ADMIN)
> 

Yes, thank you, that is much better. Will use that instead and remove
the custom SYS_ADMIN_CAP define above.

> > +
> > +	TEST_REQUIRE(kvm_has_cap(KVM_CAP_S390_UCONTROL));
> > +}
> > +  
>