Re: [PATCH v8 3/3] KVM: selftests: arm64: Make set_id_regs bitfield validity checks non-fatal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests: set_id_regs uses fatal vcpu_set_reg() for valid writes
--

commit d230119197ae572759175fc9da082ab1dada3cca
Author: Mark Brown <broonie@kernel.org>

KVM: selftests: arm64: Make set_id_regs bitfield validity checks non-fatal

This commit modifies the set_id_regs selftest to use standard kselftest
reporting instead of fatal assertions when validating writes to feature
registers. This allows the test program to continue executing and report
all affected bitfields even if some validation checks fail.

> diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
> index 3aa8886e8b70e..43dd9506036cf 100644
> --- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
> +++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
[ ... ]
> @@ -422,6 +422,7 @@ static u64 test_reg_set_success(struct kvm_vcpu *vcpu, u64 reg,
>  	u8 shift = ftr_bits->shift;
>  	u64 mask = ftr_bits->mask;
>  	u64 val, new_val, ftr;
> +	bool match;
>  
>  	val = vcpu_get_reg(vcpu, reg);
>  	ftr = (val & mask) >> shift;
> @@ -434,7 +435,10 @@ static u64 test_reg_set_success(struct kvm_vcpu *vcpu, u64 reg,
>  
>  	vcpu_set_reg(vcpu, reg, val);

[Severity: Low]
Does using vcpu_set_reg() here defeat the stated goal of making write
validation checks non-fatal? 

In the KVM selftests framework, vcpu_set_reg() contains a fatal assertion
that triggers if the underlying ioctl fails. If KVM unexpectedly rejects a
"valid" write by returning an error like -EINVAL, wouldn't this still fatally
abort the test suite instead of printing the affected bitfield and continuing?

Looking at test_reg_set_fail() further down in this patch, it explicitly
uses __vcpu_set_reg() and manually checks the return code and errno to
avoid fatal assertions on the ioctl. Should __vcpu_set_reg() be used here
in test_reg_set_success() as well?

>  	new_val = vcpu_get_reg(vcpu, reg);
> -	TEST_ASSERT_EQ(new_val, val);
> +	match = new_val == val;
> +	if (!match)
> +		ksft_print_msg("%lx != %lx\n", new_val, val);
> +	ksft_test_result(match, "%s valid write succeeded\n", ftr_bits->name);
>  
>  	return new_val;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-kvm-arm64-set-id-regs-aarch64-v8-0-58aabe2c2fac@kernel.org?part=3