Re: [PATCH v8 9/9] selftests: KVM: Test vtimer offset reg in get-reg-list

From: Andrew Jones <drjones@redhat.com>
To: Oliver Upton <oupton@google.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu,
	Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>,
	Marc Zyngier <maz@kernel.org>, Peter Shier <pshier@google.com>,
	Jim Mattson <jmattson@google.com>,
	David Matlack <dmatlack@google.com>,
	Ricardo Koller <ricarkol@google.com>,
	Jing Zhang <jingzhangos@google.com>,
	Raghavendra Rao Anata <rananta@google.com>,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>
Subject: Re: [PATCH v8 9/9] selftests: KVM: Test vtimer offset reg in get-reg-list
Date: Mon, 20 Sep 2021 14:28:33 +0200	[thread overview]
Message-ID: <20210920122833.wlo7xj4ckq4upjch@gator> (raw)
In-Reply-To: <20210916181555.973085-10-oupton@google.com>

On Thu, Sep 16, 2021 at 06:15:55PM +0000, Oliver Upton wrote:
> Assert that KVM exposes KVM_REG_ARM_TIMER_OFFSET in the KVM_GET_REG_LIST
> ioctl when userspace buys in to the new behavior.
> 
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
>  .../selftests/kvm/aarch64/get-reg-list.c      | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> index cc898181faab..4f337d8b793a 100644
> --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> @@ -40,6 +40,7 @@ static __u64 *blessed_reg, blessed_n;
>  struct reg_sublist {
>  	const char *name;
>  	long capability;
> +	long enable_capability;

cap.cap is a __u32

>  	int feature;
>  	bool finalize;
>  	__u64 *regs;
> @@ -397,6 +398,19 @@ static void check_supported(struct vcpu_config *c)
>  	}
>  }
>  
> +static void enable_caps(struct kvm_vm *vm, struct vcpu_config *c)
> +{
> +	struct kvm_enable_cap cap = {0};
> +	struct reg_sublist *s;
> +
> +	for_each_sublist(c, s) {
> +		if (s->enable_capability) {
> +			cap.cap = s->enable_capability;
> +			vm_enable_cap(vm, &cap);
> +		}
> +	}
> +}
> +
>  static bool print_list;
>  static bool print_filtered;
>  static bool fixup_core_regs;
> @@ -412,6 +426,8 @@ static void run_test(struct vcpu_config *c)
>  	check_supported(c);
>  
>  	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
> +	enable_caps(vm, c);
> +
>  	prepare_vcpu_init(c, &init);
>  	aarch64_vcpu_add_default(vm, 0, &init, NULL);
>  	finalize_vcpu(vm, 0, c);
> @@ -1014,6 +1030,10 @@ static __u64 sve_rejects_set[] = {
>  	KVM_REG_ARM64_SVE_VLS,
>  };
>  
> +static __u64 vtimer_offset_regs[] = {
> +	KVM_REG_ARM_TIMER_OFFSET,
> +};
> +
>  #define BASE_SUBLIST \
>  	{ "base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), }
>  #define VREGS_SUBLIST \
> @@ -1025,6 +1045,10 @@ static __u64 sve_rejects_set[] = {
>  	{ "sve", .capability = KVM_CAP_ARM_SVE, .feature = KVM_ARM_VCPU_SVE, .finalize = true, \
>  	  .regs = sve_regs, .regs_n = ARRAY_SIZE(sve_regs), \
>  	  .rejects_set = sve_rejects_set, .rejects_set_n = ARRAY_SIZE(sve_rejects_set), }
> +#define VTIMER_OFFSET_SUBLIST \
> +	{ "vtimer_offset", .capability = KVM_CAP_ARM_VTIMER_OFFSET, \
> +	  .enable_capability = KVM_CAP_ARM_VTIMER_OFFSET, .regs = vtimer_offset_regs, \
> +	  .regs_n = ARRAY_SIZE(vtimer_offset_regs), }
>  
>  static struct vcpu_config vregs_config = {
>  	.sublists = {
> @@ -1041,6 +1065,14 @@ static struct vcpu_config vregs_pmu_config = {
>  	{0},
>  	},
>  };
> +static struct vcpu_config vregs_vtimer_config = {
> +	.sublists = {
> +	BASE_SUBLIST,
> +	VREGS_SUBLIST,
> +	VTIMER_OFFSET_SUBLIST,
> +	{0},
> +	},
> +};
>  static struct vcpu_config sve_config = {
>  	.sublists = {
>  	BASE_SUBLIST,
> @@ -1056,11 +1088,21 @@ static struct vcpu_config sve_pmu_config = {
>  	{0},
>  	},
>  };
> +static struct vcpu_config sve_vtimer_config = {
> +	.sublists = {
> +	BASE_SUBLIST,
> +	SVE_SUBLIST,
> +	VTIMER_OFFSET_SUBLIST,
> +	{0},
> +	},
> +};
>  
>  static struct vcpu_config *vcpu_configs[] = {
>  	&vregs_config,
>  	&vregs_pmu_config,
> +	&vregs_vtimer_config,
>  	&sve_config,
>  	&sve_pmu_config,
> +	&sve_vtimer_config,
>  };
>  static int vcpu_configs_n = ARRAY_SIZE(vcpu_configs);
> -- 
> 2.33.0.464.g1972c5931b-goog
>

Other than the enable_capability type nit

Reviewed-by: Andrew Jones <drjones@redhat.com>