The set_id_regs selftest lacks coverag for ID_AA64ISR3_EL1 which has several features exposed to KVM guests in it. Add coverage, and while we're here adjust the test to improve maintainability a bit.
The test will fail without the recently applied change adding FEAT_LSFE:
https://lore.kernel.org/r/175829303126.1764550.939188785634158487.b4-ty@kern...
Signed-off-by: Mark Brown broonie@kernel.org --- Mark Brown (2): KVM: arm64: selftests: Remove a duplicate register listing in set_id_regs KVM: arm64: selftests: Cover ID_AA64ISAR3_EL1 in set_id_regs
tools/testing/selftests/kvm/arm64/set_id_regs.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) --- base-commit: 5db15c998c390efbe5c82f6cda77cb896a3a6a3e change-id: 20250919-kvm-arm64-id-aa64isar3-el1-f0bd8ab3d36b
Best regards, -- Mark Brown broonie@kernel.org
Currently we list the main set of registers with bits we test three times, once in the test_regs array which is used at runtime, once in the guest code and once in a list of ARRAY_SIZE() operations we use to tell kselftest how many tests we plan to execute. This is needlessly fiddly, when adding new registers as the test_cnt calculation is formatted with two registers per line. Instead count the number of bitfields in the register arrays at runtime.
The existing code subtracts ARRAY_SIZE(test_regs) from the number of tests to account for the terminating FTR_REG_END entries in the per register arrays, the new code accounts for this when enumerating.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/kvm/arm64/set_id_regs.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c index d3bf9204409c..bfb70926272d 100644 --- a/tools/testing/selftests/kvm/arm64/set_id_regs.c +++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c @@ -763,7 +763,7 @@ int main(void) struct kvm_vm *vm; bool aarch64_only; uint64_t val, el0; - int test_cnt; + int test_cnt, i, j;
TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_SUPPORTED_REG_MASK_RANGES)); TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_WRITABLE_IMP_ID_REGS)); @@ -779,13 +779,10 @@ int main(void)
ksft_print_header();
- test_cnt = ARRAY_SIZE(ftr_id_aa64dfr0_el1) + ARRAY_SIZE(ftr_id_dfr0_el1) + - ARRAY_SIZE(ftr_id_aa64isar0_el1) + ARRAY_SIZE(ftr_id_aa64isar1_el1) + - ARRAY_SIZE(ftr_id_aa64isar2_el1) + ARRAY_SIZE(ftr_id_aa64pfr0_el1) + - ARRAY_SIZE(ftr_id_aa64pfr1_el1) + ARRAY_SIZE(ftr_id_aa64mmfr0_el1) + - ARRAY_SIZE(ftr_id_aa64mmfr1_el1) + ARRAY_SIZE(ftr_id_aa64mmfr2_el1) + - ARRAY_SIZE(ftr_id_aa64mmfr3_el1) + ARRAY_SIZE(ftr_id_aa64zfr0_el1) - - ARRAY_SIZE(test_regs) + 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST; + test_cnt = 3 + 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++;
ksft_set_plan(test_cnt);
We have a couple of writable bitfields in ID_AA64ISAR3_EL1 but the set_id_regs selftest does not cover this register at all, add coverage.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/kvm/arm64/set_id_regs.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c index bfb70926272d..c7c38b1a1f10 100644 --- a/tools/testing/selftests/kvm/arm64/set_id_regs.c +++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c @@ -125,6 +125,13 @@ static const struct reg_ftr_bits ftr_id_aa64isar2_el1[] = { REG_FTR_END, };
+static const struct reg_ftr_bits ftr_id_aa64isar3_el1[] = { + REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64ISAR3_EL1, FPRCVT, 0), + REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64ISAR3_EL1, LSFE, 0), + REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64ISAR3_EL1, FAMINMAX, 0), + REG_FTR_END, +}; + static const struct reg_ftr_bits ftr_id_aa64pfr0_el1[] = { REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, CSV3, 0), REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, CSV2, 0), @@ -221,6 +228,7 @@ static struct test_feature_reg test_regs[] = { TEST_REG(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0_el1), TEST_REG(SYS_ID_AA64ISAR1_EL1, ftr_id_aa64isar1_el1), TEST_REG(SYS_ID_AA64ISAR2_EL1, ftr_id_aa64isar2_el1), + TEST_REG(SYS_ID_AA64ISAR3_EL1, ftr_id_aa64isar3_el1), TEST_REG(SYS_ID_AA64PFR0_EL1, ftr_id_aa64pfr0_el1), TEST_REG(SYS_ID_AA64PFR1_EL1, ftr_id_aa64pfr1_el1), TEST_REG(SYS_ID_AA64MMFR0_EL1, ftr_id_aa64mmfr0_el1), @@ -239,6 +247,7 @@ static void guest_code(void) GUEST_REG_SYNC(SYS_ID_AA64ISAR0_EL1); GUEST_REG_SYNC(SYS_ID_AA64ISAR1_EL1); GUEST_REG_SYNC(SYS_ID_AA64ISAR2_EL1); + GUEST_REG_SYNC(SYS_ID_AA64ISAR3_EL1); GUEST_REG_SYNC(SYS_ID_AA64PFR0_EL1); GUEST_REG_SYNC(SYS_ID_AA64MMFR0_EL1); GUEST_REG_SYNC(SYS_ID_AA64MMFR1_EL1);
On 2025/9/21 03:52, Mark Brown wrote:
We have a couple of writable bitfields in ID_AA64ISAR3_EL1 but the set_id_regs selftest does not cover this register at all, add coverage.
Signed-off-by: Mark Brown broonie@kernel.org
tools/testing/selftests/kvm/arm64/set_id_regs.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c index bfb70926272d..c7c38b1a1f10 100644 --- a/tools/testing/selftests/kvm/arm64/set_id_regs.c +++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c @@ -125,6 +125,13 @@ static const struct reg_ftr_bits ftr_id_aa64isar2_el1[] = { REG_FTR_END, }; +static const struct reg_ftr_bits ftr_id_aa64isar3_el1[] = {
- REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64ISAR3_EL1, FPRCVT, 0),
- REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64ISAR3_EL1, LSFE, 0),
- REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64ISAR3_EL1, FAMINMAX, 0),
- REG_FTR_END,
+};
static const struct reg_ftr_bits ftr_id_aa64pfr0_el1[] = { REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, CSV3, 0), REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64PFR0_EL1, CSV2, 0), @@ -221,6 +228,7 @@ static struct test_feature_reg test_regs[] = { TEST_REG(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0_el1), TEST_REG(SYS_ID_AA64ISAR1_EL1, ftr_id_aa64isar1_el1), TEST_REG(SYS_ID_AA64ISAR2_EL1, ftr_id_aa64isar2_el1),
- TEST_REG(SYS_ID_AA64ISAR3_EL1, ftr_id_aa64isar3_el1), TEST_REG(SYS_ID_AA64PFR0_EL1, ftr_id_aa64pfr0_el1), TEST_REG(SYS_ID_AA64PFR1_EL1, ftr_id_aa64pfr1_el1), TEST_REG(SYS_ID_AA64MMFR0_EL1, ftr_id_aa64mmfr0_el1),
@@ -239,6 +247,7 @@ static void guest_code(void) GUEST_REG_SYNC(SYS_ID_AA64ISAR0_EL1); GUEST_REG_SYNC(SYS_ID_AA64ISAR1_EL1); GUEST_REG_SYNC(SYS_ID_AA64ISAR2_EL1);
- GUEST_REG_SYNC(SYS_ID_AA64ISAR3_EL1); GUEST_REG_SYNC(SYS_ID_AA64PFR0_EL1); GUEST_REG_SYNC(SYS_ID_AA64MMFR0_EL1); GUEST_REG_SYNC(SYS_ID_AA64MMFR1_EL1);
Not related to this patch but seems that we forgot to sync several registers (ID_AA64PFR1, MPIDR, CLIDR) in guest to make sure the guest had seen the written value.
Zenghui
On Sat, Sep 20, 2025 at 08:51:58PM +0100, Mark Brown wrote:
The set_id_regs selftest lacks coverag for ID_AA64ISR3_EL1 which has several features exposed to KVM guests in it. Add coverage, and while we're here adjust the test to improve maintainability a bit.
The test will fail without the recently applied change adding FEAT_LSFE:
https://lore.kernel.org/r/175829303126.1764550.939188785634158487.b4-ty@kern...
Signed-off-by: Mark Brown broonie@kernel.org
Mark Brown (2): KVM: arm64: selftests: Remove a duplicate register listing in set_id_regs KVM: arm64: selftests: Cover ID_AA64ISAR3_EL1 in set_id_regs
Reviewed-by: Oliver Upton oliver.upton@linux.dev
Thanks, Oliver
On Sat, 20 Sep 2025 20:51:58 +0100, Mark Brown broonie@kernel.org wrote:
The set_id_regs selftest lacks coverag for ID_AA64ISR3_EL1 which has several features exposed to KVM guests in it. Add coverage, and while we're here adjust the test to improve maintainability a bit.
The test will fail without the recently applied change adding FEAT_LSFE:
https://lore.kernel.org/r/175829303126.1764550.939188785634158487.b4-ty@kern...
Signed-off-by: Mark Brown broonie@kernel.org
Mark Brown (2): KVM: arm64: selftests: Remove a duplicate register listing in set_id_regs KVM: arm64: selftests: Cover ID_AA64ISAR3_EL1 in set_id_regs
tools/testing/selftests/kvm/arm64/set_id_regs.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)
base-commit: 5db15c998c390efbe5c82f6cda77cb896a3a6a3e
$ git show 5db15c998c390efbe5c82f6cda77cb896a3a6a3e fatal: bad object 5db15c998c390efbe5c82f6cda77cb896a3a6a3e
What's the point of indicating a base commit if that's not something I can find? You should never base any work on top of something that isn't a stable tag.
M.
On Wed, Sep 24, 2025 at 07:29:49PM +0100, Marc Zyngier wrote:
Mark Brown broonie@kernel.org wrote:
base-commit: 5db15c998c390efbe5c82f6cda77cb896a3a6a3e
$ git show 5db15c998c390efbe5c82f6cda77cb896a3a6a3e fatal: bad object 5db15c998c390efbe5c82f6cda77cb896a3a6a3e
What's the point of indicating a base commit if that's not something I can find? You should never base any work on top of something that isn't a stable tag.
This was due to you having applied the LSFE patch but that application not yet being visible to me at the time I did the rebase (I've noticed occasional slow mirroring on kernel.org recently, possibly it was that), I needed to base on the already applied patch so that I could check that the test binary worked properly.
On Sat, 20 Sep 2025 20:51:58 +0100, Mark Brown wrote:
The set_id_regs selftest lacks coverag for ID_AA64ISR3_EL1 which has several features exposed to KVM guests in it. Add coverage, and while we're here adjust the test to improve maintainability a bit.
The test will fail without the recently applied change adding FEAT_LSFE:
https://lore.kernel.org/r/175829303126.1764550.939188785634158487.b4-ty@kern...
[...]
Applied to next, thanks!
[1/2] KVM: arm64: selftests: Remove a duplicate register listing in set_id_regs commit: 5a070fc376babc7efdc8288b97431e43e18f4646 [2/2] KVM: arm64: selftests: Cover ID_AA64ISAR3_EL1 in set_id_regs commit: b02a2c060b657296c080cff1b54ee4e9e650811c
Cheers,
M.
linux-kselftest-mirror@lists.linaro.org
-
Marc Zyngier -
Mark Brown -
Oliver Upton -
Zenghui Yu