Re: [PATCH] KVM: selftests: arm64: Correct feature test for S1PIE in get-reg-list

[Marc Zyngier <maz@kernel.org>]

On Wed, 31 Jul 2024 17:21:13 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> The ID register for S1PIE is ID_AA64MMFR3_EL1.S1PIE which is bits 11:8 but
> get-reg-list uses a shift of 4, checking SCTLRX instead. Use a shift of 8
> instead.
> 
> Fixes: 5f0419a0083b ("KVM: selftests: get-reg-list: add Permission Indirection registers")
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  tools/testing/selftests/kvm/aarch64/get-reg-list.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> index 709d7d721760..4abebde78187 100644
> --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> @@ -32,13 +32,13 @@ static struct feature_id_reg feat_id_regs[] = {
>  	{
>  		ARM64_SYS_REG(3, 0, 10, 2, 2),	/* PIRE0_EL1 */
>  		ARM64_SYS_REG(3, 0, 0, 7, 3),	/* ID_AA64MMFR3_EL1 */
> -		4,
> +		8,
>  		1
>  	},
>  	{
>  		ARM64_SYS_REG(3, 0, 10, 2, 3),	/* PIR_EL1 */
>  		ARM64_SYS_REG(3, 0, 0, 7, 3),	/* ID_AA64MMFR3_EL1 */
> -		4,
> +		8,
>  		1
>  	}
>  };

Thanks for spotting this. However, we are fixing it in a very backward
way.

Can we please switch all this stuff to symbolic naming instead of
magic numbers? Given how much effort is going into the "automated
generation" thing, it is mind-boggling that the tests still rely on
handcrafted numbers. We just end-up with two different sets of bugs.

At the moment, the level of confidence I have in this stuff is
sub-zero.

	M.

-- 
Without deviation from the norm, progress is not possible.