[PATCH 0/3] set_id_regs cleanup

[PATCH 0/3] set_id_regs cleanup

[Ben Horgan]

Tidies up a few small things in set_id_regs. The removal of
ARM64_FEATURE_FIELD_BITS touches other code only so much as it removes
the define but set_id_regs.c is the only place its used.

Based on v6.18-rc1

Ben Horgan (3):
  KVM: arm64: selftests: Count test_guest_reg_read() as a test
  KVM: arm64: selftests: Remove ARM64_FEATURE_FIELD_BITS and its last
    user
  KVM: arm64: selftests: Consider all 7 possible levels of cache

 arch/arm64/include/asm/sysreg.h                 |  2 --
 tools/testing/selftests/kvm/arm64/set_id_regs.c | 14 ++++++++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH 1/3] KVM: arm64: selftests: Count test_guest_reg_read() as a test

[Ben Horgan]

The test test_guest_reg_read() is run without announcing its presence or
contributing to the test count. Rectify this.

Signed-off-by: Ben Horgan <ben.horgan@arm.com>
---
 tools/testing/selftests/kvm/arm64/set_id_regs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index 5e24f77868b5..6878ee23310e 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -655,6 +655,8 @@ static void test_guest_reg_read(struct kvm_vcpu *vcpu)
 			TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
 		}
 	}
+
+	ksft_test_result_pass("%s\n", __func__);
 }
 
 /* Politely lifted from arch/arm64/include/asm/cache.h */
@@ -786,7 +788,7 @@ int main(void)
 
 	ksft_print_header();
 
-	test_cnt = 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
+	test_cnt = 4 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
 	for (i = 0; i < ARRAY_SIZE(test_regs); i++)
 		for (j = 0; test_regs[i].ftr_bits[j].type != FTR_END; j++)
 			test_cnt++;
-- 
2.43.0

[PATCH 2/3] KVM: arm64: selftests: Remove ARM64_FEATURE_FIELD_BITS and its last user

[Ben Horgan]

ARM64_FEATURE_FIELD_BITS is set to 4 but not all ID register fields are 4
bits. See for instance ID_AA64SMFR0_EL1. The last user of this define,
ARM64_FEATURE_FIELD_BITS, is the set_id_regs selftest. Its logic assumes
the fields aren't a single bits; assert that's the case and stop using the
define. As there are no more users, ARM64_FEATURE_FIELD_BITS is removed.

Signed-off-by: Ben Horgan <ben.horgan@arm.com>
---
 arch/arm64/include/asm/sysreg.h                 | 2 --
 tools/testing/selftests/kvm/arm64/set_id_regs.c | 8 ++++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 6455db1b54fd..d9aa76d08e13 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -1129,8 +1129,6 @@
 #define gicr_insn(insn)			read_sysreg_s(GICV5_OP_GICR_##insn)
 #define gic_insn(v, insn)		write_sysreg_s(v, GICV5_OP_GIC_##insn)
 
-#define ARM64_FEATURE_FIELD_BITS	4
-
 #ifdef __ASSEMBLY__
 
 	.macro	mrs_s, rt, sreg
diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index 6878ee23310e..a0b81b627037 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -268,7 +268,9 @@ static void guest_code(void)
 /* Return a safe value to a given ftr_bits an ftr value */
 uint64_t get_safe_value(const struct reg_ftr_bits *ftr_bits, uint64_t ftr)
 {
-	uint64_t ftr_max = GENMASK_ULL(ARM64_FEATURE_FIELD_BITS - 1, 0);
+	uint64_t ftr_max = ftr_bits->mask >> ftr_bits->shift;
+
+	TEST_ASSERT(ftr_max > 1, "This test doesn't support single bit features");
 
 	if (ftr_bits->sign == FTR_UNSIGNED) {
 		switch (ftr_bits->type) {
@@ -320,7 +322,9 @@ uint64_t get_safe_value(const struct reg_ftr_bits *ftr_bits, uint64_t ftr)
 /* Return an invalid value to a given ftr_bits an ftr value */
 uint64_t get_invalid_value(const struct reg_ftr_bits *ftr_bits, uint64_t ftr)
 {
-	uint64_t ftr_max = GENMASK_ULL(ARM64_FEATURE_FIELD_BITS - 1, 0);
+	uint64_t ftr_max = ftr_bits->mask >> ftr_bits->shift;
+
+	TEST_ASSERT(ftr_max > 1, "This test doesn't support single bit features");
 
 	if (ftr_bits->sign == FTR_UNSIGNED) {
 		switch (ftr_bits->type) {
-- 
2.43.0

[PATCH 3/3] KVM: arm64: selftests: Consider all 7 possible levels of cache

[Ben Horgan]

In test_clidr() if an empty cache level is not found then the TEST_ASSERT
will not fire. Fix this by considering all 7 possible levels when iterating
through the hierarchy. Found by inspection.

Signed-off-by: Ben Horgan <ben.horgan@arm.com>
---
 tools/testing/selftests/kvm/arm64/set_id_regs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index a0b81b627037..731f8b23b1c5 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -678,7 +678,7 @@ static void test_clidr(struct kvm_vcpu *vcpu)
 	clidr = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CLIDR_EL1));
 
 	/* find the first empty level in the cache hierarchy */
-	for (level = 1; level < 7; level++) {
+	for (level = 1; level <= 7; level++) {
 		if (!CLIDR_CTYPE(clidr, level))
 			break;
 	}
-- 
2.43.0

Re: [PATCH 2/3] KVM: arm64: selftests: Remove ARM64_FEATURE_FIELD_BITS and its last user

[Oliver Upton]

Hi Ben,

On Tue, Oct 14, 2025 at 11:21:07AM +0100, Ben Horgan wrote:
> ARM64_FEATURE_FIELD_BITS is set to 4 but not all ID register fields are 4
> bits. See for instance ID_AA64SMFR0_EL1. The last user of this define,
> ARM64_FEATURE_FIELD_BITS, is the set_id_regs selftest. Its logic assumes
> the fields aren't a single bits; assert that's the case and stop using the
> define. As there are no more users, ARM64_FEATURE_FIELD_BITS is removed.
> 
> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
> ---
>  arch/arm64/include/asm/sysreg.h                 | 2 --
>  tools/testing/selftests/kvm/arm64/set_id_regs.c | 8 ++++++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 6455db1b54fd..d9aa76d08e13 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -1129,8 +1129,6 @@
>  #define gicr_insn(insn)			read_sysreg_s(GICV5_OP_GICR_##insn)
>  #define gic_insn(v, insn)		write_sysreg_s(v, GICV5_OP_GIC_##insn)
>  
> -#define ARM64_FEATURE_FIELD_BITS	4
> -
>  #ifdef __ASSEMBLY__
>  
>  	.macro	mrs_s, rt, sreg

You can send this diff as a separate patch. Selftests actually uses the
definition from tools/arch/arm64/include/asm/sysreg.h, so you'll want to
drop that definition along with the change to set_id_regs.

Thanks,
Oliver

Re: [PATCH 1/3] KVM: arm64: selftests: Count test_guest_reg_read() as a test

[Oliver Upton]

On Tue, Oct 14, 2025 at 11:21:06AM +0100, Ben Horgan wrote:
> The test test_guest_reg_read() is run without announcing its presence or
> contributing to the test count. Rectify this.
> 
> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
> ---
>  tools/testing/selftests/kvm/arm64/set_id_regs.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
> index 5e24f77868b5..6878ee23310e 100644
> --- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
> +++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
> @@ -655,6 +655,8 @@ static void test_guest_reg_read(struct kvm_vcpu *vcpu)
>  			TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
>  		}
>  	}
> +
> +	ksft_test_result_pass("%s\n", __func__);
>  }
>  
>  /* Politely lifted from arch/arm64/include/asm/cache.h */
> @@ -786,7 +788,7 @@ int main(void)
>  
>  	ksft_print_header();
>  
> -	test_cnt = 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
> +	test_cnt = 4 + MPAM_IDREG_TEST + MTE_IDREG_TEST;

TBH, I'd actually be in favor of a patch that removes usage of the
ksft_* harness from this test altogether. I don't think it adds much and
we use the KVM-specific test assertions anyway.

Thanks,
Oliver

Re: [PATCH 1/3] KVM: arm64: selftests: Count test_guest_reg_read() as a test

[Ben Horgan]

Hi Oliver,

On 10/29/25 20:48, Oliver Upton wrote:
> On Tue, Oct 14, 2025 at 11:21:06AM +0100, Ben Horgan wrote:
>> The test test_guest_reg_read() is run without announcing its presence or
>> contributing to the test count. Rectify this.
>>
>> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
>> ---
>>  tools/testing/selftests/kvm/arm64/set_id_regs.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
>> index 5e24f77868b5..6878ee23310e 100644
>> --- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
>> +++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
>> @@ -655,6 +655,8 @@ static void test_guest_reg_read(struct kvm_vcpu *vcpu)
>>  			TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
>>  		}
>>  	}
>> +
>> +	ksft_test_result_pass("%s\n", __func__);
>>  }
>>  
>>  /* Politely lifted from arch/arm64/include/asm/cache.h */
>> @@ -786,7 +788,7 @@ int main(void)
>>  
>>  	ksft_print_header();
>>  
>> -	test_cnt = 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
>> +	test_cnt = 4 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
> 
> TBH, I'd actually be in favor of a patch that removes usage of the
> ksft_* harness from this test altogether. I don't think it adds much and
> we use the KVM-specific test assertions anyway.

I'll give this a go for a respin unless anyone pipes up to say the
ksft_* usage in this test is helpful for them.

> 
> Thanks,
> Oliver

-- 
Thanks,

Ben

Re: [PATCH 2/3] KVM: arm64: selftests: Remove ARM64_FEATURE_FIELD_BITS and its last user

[Ben Horgan]

Hi Oliver,

On 10/29/25 20:45, Oliver Upton wrote:
> Hi Ben,
> 
> On Tue, Oct 14, 2025 at 11:21:07AM +0100, Ben Horgan wrote:
>> ARM64_FEATURE_FIELD_BITS is set to 4 but not all ID register fields are 4
>> bits. See for instance ID_AA64SMFR0_EL1. The last user of this define,
>> ARM64_FEATURE_FIELD_BITS, is the set_id_regs selftest. Its logic assumes
>> the fields aren't a single bits; assert that's the case and stop using the
>> define. As there are no more users, ARM64_FEATURE_FIELD_BITS is removed.
>>
>> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
>> ---
>>  arch/arm64/include/asm/sysreg.h                 | 2 --
>>  tools/testing/selftests/kvm/arm64/set_id_regs.c | 8 ++++++--
>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
>> index 6455db1b54fd..d9aa76d08e13 100644
>> --- a/arch/arm64/include/asm/sysreg.h
>> +++ b/arch/arm64/include/asm/sysreg.h
>> @@ -1129,8 +1129,6 @@
>>  #define gicr_insn(insn)			read_sysreg_s(GICV5_OP_GICR_##insn)
>>  #define gic_insn(v, insn)		write_sysreg_s(v, GICV5_OP_GIC_##insn)
>>  
>> -#define ARM64_FEATURE_FIELD_BITS	4
>> -
>>  #ifdef __ASSEMBLY__
>>  
>>  	.macro	mrs_s, rt, sreg
> 
> You can send this diff as a separate patch. Selftests actually uses the
> definition from tools/arch/arm64/include/asm/sysreg.h, so you'll want to
> drop that definition along with the change to set_id_regs.

Thanks for the pointers. I'll do as you suggest.

> 
> Thanks,
> Oliver
> 

-- 
Thanks,

Ben