* [PATCH v9 0/3] KVM: selftests: arm64: Improve diagnostics from set_id_regs
@ 2026-11-03 18:19 Mark Brown
2026-11-03 18:19 ` [PATCH v9 1/3] KVM: selftests: arm64: Report set_id_reg reads of test registers as tests Mark Brown
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Mark Brown @ 2026-11-03 18:19 UTC (permalink / raw)
To: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Paolo Bonzini,
Shuah Khan, Oliver Upton
Cc: Ben Horgan, linux-arm-kernel, kvmarm, kvm, linux-kselftest,
linux-kernel, Mark Brown, Lorenzo Stoakes (ARM)
While debugging issues related to aarch64 only systems I ran into
speedbumps due to the lack of detail in the results reported when the
guest register read and reset value preservation tests were run, they
generated an immediately fatal assert without indicating which register
was being tested. Update these tests to non-fatally report a standard
kselftest result per register, making it much easier to see what the
problem being reported is.
A similar issue exists with the validation of the individual bitfields
in registers due to the use of immediately fatal asserts. Update those
asserts to be standard kselftest reports.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Changes in v9:
- Rebase onto v7.4-rc1.
- Link to v8: https://patch.msgid.link/20260929-kvm-arm64-set-id-regs-aarch64-v8-0-58aabe2c2fac@kernel.org
Changes in v8:
- Convert another assert.
- Link to v7: https://patch.msgid.link/20260901-kvm-arm64-set-id-regs-aarch64-v7-0-7b065b0bda9a@kernel.org
Changes in v7:
- Rebase onto v7.3-rc1.
- Link to v6: https://patch.msgid.link/20260719-kvm-arm64-set-id-regs-aarch64-v6-0-724287f5f108@kernel.org
Changes in v6:
- Rebase onto v7.2-rc3.
- Drop changes to handling of aarch32 related registers on 64 bit only
systems, an in kernel solution is preferred for those.
- Use a define to count the non-test_regs registers.
- Link to v5: https://patch.msgid.link/20260317-kvm-arm64-set-id-regs-aarch64-v5-0-a60f2b956e22@kernel.org
Changes in v5:
- Rebase onto v7.0-rc1.
- Link to v4: https://patch.msgid.link/20260106-kvm-arm64-set-id-regs-aarch64-v4-0-c7ef4551afb3@kernel.org
Changes in v4:
- Correct check for 32 bit ID registers.
- Link to v3: https://patch.msgid.link/20251219-kvm-arm64-set-id-regs-aarch64-v3-0-bfa474ec3218@kernel.org
Changes in v3:
- Rebase onto v6.19-rc1.
- Link to v2: https://patch.msgid.link/20251114-kvm-arm64-set-id-regs-aarch64-v2-0-672f214f41bf@kernel.org
Changes in v2:
- Add a fix for spurious failures with 64 bit only guests.
- Link to v1: https://patch.msgid.link/20251030-kvm-arm64-set-id-regs-aarch64-v1-0-96fe0d2b178e@kernel.org
---
Mark Brown (3):
KVM: selftests: arm64: Report set_id_reg reads of test registers as tests
KVM: selftests: arm64: Report register reset tests individually
KVM: selftests: arm64: Make set_id_regs bitfield validity checks non-fatal
tools/testing/selftests/kvm/arm64/set_id_regs.c | 130 ++++++++++++++++++------
1 file changed, 98 insertions(+), 32 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
base-tree: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci
change-id: 20251028-kvm-arm64-set-id-regs-aarch64-ebb77969401c
Best regards,
--
Mark Brown <broonie@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v9 1/3] KVM: selftests: arm64: Report set_id_reg reads of test registers as tests
2026-11-03 18:19 [PATCH v9 0/3] KVM: selftests: arm64: Improve diagnostics from set_id_regs Mark Brown
@ 2026-11-03 18:19 ` Mark Brown
2026-11-04 11:32 ` Sebastian Ott
2026-11-03 18:19 ` [PATCH v9 2/3] KVM: selftests: arm64: Report register reset tests individually Mark Brown
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Mark Brown @ 2026-11-03 18:19 UTC (permalink / raw)
To: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Paolo Bonzini,
Shuah Khan, Oliver Upton
Cc: Ben Horgan, linux-arm-kernel, kvmarm, kvm, linux-kselftest,
linux-kernel, Mark Brown, Lorenzo Stoakes (ARM)
Currently when we run guest code to validate that the values we wrote to
the registers are seen by the guest we assert that these values match using
a KVM selftests level assert, resulting in unclear diagnostics if the test
fails. Replace this assert with reporting a kselftest test per register.
In order to support getting the names of the registers we repaint the array
of ID_ registers to store the names and open code the rest.
Reviewed-by: Ben Horgan <ben.horgan@arm.com>
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
tools/testing/selftests/kvm/arm64/set_id_regs.c | 82 +++++++++++++++++++------
1 file changed, 63 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index 1d2cc25cb3f4..0e6fd7b57e52 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -44,6 +44,7 @@ struct reg_ftr_bits {
};
struct test_feature_reg {
+ const char *name;
u32 reg;
const struct reg_ftr_bits *ftr_bits;
};
@@ -228,30 +229,32 @@ static const struct reg_ftr_bits ftr_id_aa64zfr0_el1[] = {
#define TEST_REG(id, table) \
{ \
- .reg = id, \
+ .name = #id, \
+ .reg = SYS_ ## id, \
.ftr_bits = &((table)[0]), \
}
static struct test_feature_reg test_regs[] = {
- TEST_REG(SYS_ID_AA64DFR0_EL1, ftr_id_aa64dfr0_el1),
- TEST_REG(SYS_ID_DFR0_EL1, ftr_id_dfr0_el1),
- 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),
- TEST_REG(SYS_ID_AA64MMFR1_EL1, ftr_id_aa64mmfr1_el1),
- TEST_REG(SYS_ID_AA64MMFR2_EL1, ftr_id_aa64mmfr2_el1),
- TEST_REG(SYS_ID_AA64MMFR3_EL1, ftr_id_aa64mmfr3_el1),
- TEST_REG(SYS_ID_AA64ZFR0_EL1, ftr_id_aa64zfr0_el1),
+ TEST_REG(ID_AA64DFR0_EL1, ftr_id_aa64dfr0_el1),
+ TEST_REG(ID_DFR0_EL1, ftr_id_dfr0_el1),
+ TEST_REG(ID_AA64ISAR0_EL1, ftr_id_aa64isar0_el1),
+ TEST_REG(ID_AA64ISAR1_EL1, ftr_id_aa64isar1_el1),
+ TEST_REG(ID_AA64ISAR2_EL1, ftr_id_aa64isar2_el1),
+ TEST_REG(ID_AA64ISAR3_EL1, ftr_id_aa64isar3_el1),
+ TEST_REG(ID_AA64PFR0_EL1, ftr_id_aa64pfr0_el1),
+ TEST_REG(ID_AA64PFR1_EL1, ftr_id_aa64pfr1_el1),
+ TEST_REG(ID_AA64MMFR0_EL1, ftr_id_aa64mmfr0_el1),
+ TEST_REG(ID_AA64MMFR1_EL1, ftr_id_aa64mmfr1_el1),
+ TEST_REG(ID_AA64MMFR2_EL1, ftr_id_aa64mmfr2_el1),
+ TEST_REG(ID_AA64MMFR3_EL1, ftr_id_aa64mmfr3_el1),
+ TEST_REG(ID_AA64ZFR0_EL1, ftr_id_aa64zfr0_el1),
};
#define GUEST_REG_SYNC(id) GUEST_SYNC_ARGS(0, id, read_sysreg_s(id), 0, 0);
static void guest_code(void)
{
+ /* Registers in test_regs array */
GUEST_REG_SYNC(SYS_ID_AA64DFR0_EL1);
GUEST_REG_SYNC(SYS_ID_DFR0_EL1);
GUEST_REG_SYNC(SYS_ID_AA64ISAR0_EL1);
@@ -265,6 +268,8 @@ static void guest_code(void)
GUEST_REG_SYNC(SYS_ID_AA64MMFR2_EL1);
GUEST_REG_SYNC(SYS_ID_AA64MMFR3_EL1);
GUEST_REG_SYNC(SYS_ID_AA64ZFR0_EL1);
+
+ /* Additional registers counted in NUM_EXTRA_REGS */
GUEST_REG_SYNC(SYS_MPIDR_EL1);
GUEST_REG_SYNC(SYS_CLIDR_EL1);
GUEST_REG_SYNC(SYS_CTR_EL0);
@@ -275,6 +280,35 @@ static void guest_code(void)
GUEST_DONE();
}
+#define NUM_EXTRA_REGS 6
+#define GUEST_READ_TEST (ARRAY_SIZE(test_regs) + NUM_EXTRA_REGS)
+
+static const char *get_reg_name(u64 id)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(test_regs); i++)
+ if (test_regs[i].reg == id)
+ return test_regs[i].name;
+
+ switch (id) {
+ case SYS_MPIDR_EL1:
+ return "MPIDR_EL1";
+ case SYS_CLIDR_EL1:
+ return "CLIDR_EL1";
+ case SYS_CTR_EL0:
+ return "CTR_EL0";
+ case SYS_MIDR_EL1:
+ return "MIDR_EL1";
+ case SYS_REVIDR_EL1:
+ return "REVIDR_EL1";
+ case SYS_AIDR_EL1:
+ return "AIDR_EL1";
+ default:
+ TEST_FAIL("Unknown register");
+ }
+}
+
/* Return a safe value to a given ftr_bits an ftr value */
u64 get_safe_value(const struct reg_ftr_bits *ftr_bits, u64 ftr)
{
@@ -675,7 +709,8 @@ static void test_guest_reg_read(struct kvm_vcpu *vcpu)
struct ucall uc;
while (!done) {
- u64 val;
+ u64 reg_id, expected_val, guest_val;
+ bool match;
vcpu_run(vcpu);
@@ -684,11 +719,20 @@ static void test_guest_reg_read(struct kvm_vcpu *vcpu)
REPORT_GUEST_ASSERT(uc);
break;
case UCALL_SYNC:
- val = test_reg_vals[encoding_to_range_idx(uc.args[2])];
- val = reset_mutable_bits(uc.args[2], val);
+ expected_val = test_reg_vals[encoding_to_range_idx(uc.args[2])];
+ expected_val = reset_mutable_bits(uc.args[2], expected_val);
/* Make sure the written values are seen by guest */
- TEST_ASSERT_EQ(val, reset_mutable_bits(uc.args[2], uc.args[3]));
+ reg_id = uc.args[2];
+ guest_val = reset_mutable_bits(uc.args[2], uc.args[3]);
+
+ match = expected_val == guest_val;
+ if (!match)
+ ksft_print_msg("%lx != %lx\n",
+ expected_val, guest_val);
+ ksft_test_result(match,
+ "%s value seen in guest\n",
+ get_reg_name(reg_id));
break;
case UCALL_DONE:
done = true;
@@ -941,7 +985,7 @@ int main(void)
ksft_print_header();
- test_cnt = 4 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
+ test_cnt = 4 + MPAM_IDREG_TEST + MTE_IDREG_TEST + GUEST_READ_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.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v9 2/3] KVM: selftests: arm64: Report register reset tests individually
2026-11-03 18:19 [PATCH v9 0/3] KVM: selftests: arm64: Improve diagnostics from set_id_regs Mark Brown
2026-11-03 18:19 ` [PATCH v9 1/3] KVM: selftests: arm64: Report set_id_reg reads of test registers as tests Mark Brown
@ 2026-11-03 18:19 ` Mark Brown
2026-11-03 18:19 ` [PATCH v9 3/3] KVM: selftests: arm64: Make set_id_regs bitfield validity checks non-fatal Mark Brown
2026-11-04 11:37 ` [PATCH v9 0/3] KVM: selftests: arm64: Improve diagnostics from set_id_regs Sebastian Ott
3 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2026-11-03 18:19 UTC (permalink / raw)
To: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Paolo Bonzini,
Shuah Khan, Oliver Upton
Cc: Ben Horgan, linux-arm-kernel, kvmarm, kvm, linux-kselftest,
linux-kernel, Mark Brown, Lorenzo Stoakes (ARM)
set_id_regs tests that registers have their values preserved over reset.
Currently it reports all registers in a single test with an instantly fatal
assert which isn't great for diagnostics, it's hard to tell which register
failed or if it's just one register. Change this to report each register as
a separate test so that it's clear from the program output which registers
have problems.
Reviewed-by: Ben Horgan <ben.horgan@arm.com>
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
tools/testing/selftests/kvm/arm64/set_id_regs.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index 0e6fd7b57e52..37107eadd985 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -820,13 +820,21 @@ static void test_vcpu_non_ftr_id_regs(struct kvm_vcpu *vcpu)
static void test_assert_id_reg_unchanged(struct kvm_vcpu *vcpu, u32 encoding)
{
size_t idx = encoding_to_range_idx(encoding);
- u64 observed;
+ u64 observed, expected;
+ bool pass;
observed = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(encoding));
- TEST_ASSERT_EQ(reset_mutable_bits(encoding, test_reg_vals[idx]),
- reset_mutable_bits(encoding, observed));
+ observed = reset_mutable_bits(encoding, observed);
+ expected = reset_mutable_bits(encoding, test_reg_vals[idx]);
+ pass = expected == observed;
+ if (!pass)
+ ksft_print_msg("%lx != %lx\n", expected, observed);
+ ksft_test_result(pass, "%s unchanged by reset\n",
+ get_reg_name(encoding));
}
+#define ID_REG_RESET_UNCHANGED_TEST (ARRAY_SIZE(test_regs) + NUM_EXTRA_REGS)
+
static void test_reset_preserves_id_regs(struct kvm_vcpu *vcpu)
{
/*
@@ -844,8 +852,6 @@ static void test_reset_preserves_id_regs(struct kvm_vcpu *vcpu)
test_assert_id_reg_unchanged(vcpu, SYS_MIDR_EL1);
test_assert_id_reg_unchanged(vcpu, SYS_REVIDR_EL1);
test_assert_id_reg_unchanged(vcpu, SYS_AIDR_EL1);
-
- ksft_test_result_pass("%s\n", __func__);
}
/*
@@ -985,7 +991,8 @@ int main(void)
ksft_print_header();
- test_cnt = 4 + MPAM_IDREG_TEST + MTE_IDREG_TEST + GUEST_READ_TEST;
+ test_cnt = 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST + GUEST_READ_TEST +
+ ID_REG_RESET_UNCHANGED_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.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v9 3/3] KVM: selftests: arm64: Make set_id_regs bitfield validity checks non-fatal
2026-11-03 18:19 [PATCH v9 0/3] KVM: selftests: arm64: Improve diagnostics from set_id_regs Mark Brown
2026-11-03 18:19 ` [PATCH v9 1/3] KVM: selftests: arm64: Report set_id_reg reads of test registers as tests Mark Brown
2026-11-03 18:19 ` [PATCH v9 2/3] KVM: selftests: arm64: Report register reset tests individually Mark Brown
@ 2026-11-03 18:19 ` Mark Brown
2026-11-03 18:31 ` sashiko-bot
2026-11-04 11:37 ` [PATCH v9 0/3] KVM: selftests: arm64: Improve diagnostics from set_id_regs Sebastian Ott
3 siblings, 1 reply; 7+ messages in thread
From: Mark Brown @ 2026-11-03 18:19 UTC (permalink / raw)
To: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Paolo Bonzini,
Shuah Khan, Oliver Upton
Cc: Ben Horgan, linux-arm-kernel, kvmarm, kvm, linux-kselftest,
linux-kernel, Mark Brown, Lorenzo Stoakes (ARM)
Currently when set_id_regs encounters a problem checking validation of
writes to feature registers it uses an immediately fatal assert to report
the problem. This is not idiomatic for kselftest, and it is also not great
for usability. The affected bitfield is not clearly reported and further
tests do not have their results reported.
Switch to using standard kselftest result reporting for the asserts we
do, these are non-fatal asserts so allow the program to continue and the
test names include the affected field.
Reviewed-by: Ben Horgan <ben.horgan@arm.com>
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
tools/testing/selftests/kvm/arm64/set_id_regs.c | 31 ++++++++++++++++++-------
1 file changed, 23 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 37107eadd985..d7f8b8efe1e2 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -423,6 +423,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;
@@ -435,7 +436,10 @@ static u64 test_reg_set_success(struct kvm_vcpu *vcpu, u64 reg,
vcpu_set_reg(vcpu, reg, val);
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;
}
@@ -447,6 +451,7 @@ static void test_reg_set_fail(struct kvm_vcpu *vcpu, u64 reg,
u64 mask = ftr_bits->mask;
u64 val, old_val, ftr;
int r;
+ bool match;
val = vcpu_get_reg(vcpu, reg);
ftr = (val & mask) >> shift;
@@ -459,11 +464,19 @@ static void test_reg_set_fail(struct kvm_vcpu *vcpu, u64 reg,
val |= ftr;
r = __vcpu_set_reg(vcpu, reg, val);
- TEST_ASSERT(r < 0 && errno == EINVAL,
- "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno);
+ if (r >= 0 || errno != EINVAL) {
+ ksft_print_msg("Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d (%s)\n",
+ r, errno, strerror(errno));
+ ksft_test_result_fail("%s invalid write rejected\n",
+ ftr_bits->name);
+ return;
+ }
val = vcpu_get_reg(vcpu, reg);
- TEST_ASSERT_EQ(val, old_val);
+ match = val == old_val;
+ if (!match)
+ ksft_print_msg("%lx != %lx\n", val, old_val);
+ ksft_test_result(match, "%s invalid write rejected\n", ftr_bits->name);
}
static u64 test_reg_vals[KVM_ARM_FEATURE_ID_RANGE_SIZE];
@@ -503,7 +516,11 @@ static void test_vm_ftr_id_regs(struct kvm_vcpu *vcpu, bool aarch64_only)
for (int j = 0; ftr_bits[j].type != FTR_END; j++) {
/* Skip aarch32 reg on aarch64 only system, since they are RAZ/WI. */
if (aarch64_only && sys_reg_CRm(reg_id) < 4) {
- ksft_test_result_skip("%s on AARCH64 only system\n",
+ ksft_print_msg("%s on AARCH64 only system\n",
+ ftr_bits[j].name);
+ ksft_test_result_skip("%s invalid write rejected\n",
+ ftr_bits[j].name);
+ ksft_test_result_skip("%s valid write succeeded\n",
ftr_bits[j].name);
continue;
}
@@ -515,8 +532,6 @@ static void test_vm_ftr_id_regs(struct kvm_vcpu *vcpu, bool aarch64_only)
test_reg_vals[idx] = test_reg_set_success(vcpu, reg,
&ftr_bits[j]);
-
- ksft_test_result_pass("%s\n", ftr_bits[j].name);
}
}
}
@@ -995,7 +1010,7 @@ int main(void)
ID_REG_RESET_UNCHANGED_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++;
+ test_cnt += 2;
ksft_set_plan(test_cnt);
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v9 3/3] KVM: selftests: arm64: Make set_id_regs bitfield validity checks non-fatal
2026-11-03 18:19 ` [PATCH v9 3/3] KVM: selftests: arm64: Make set_id_regs bitfield validity checks non-fatal Mark Brown
@ 2026-11-03 18:31 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-11-03 18:31 UTC (permalink / raw)
To: Mark Brown; +Cc: kvmarm, kvm, Oliver Upton, Marc Zyngier
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] KVM: selftests: arm64: fatal vcpu_set_reg() in test_reg_set_success()
--
commit 528a199af5ef8c602f0139ccc2860e91e650c762
Author: Mark Brown <broonie@kernel.org>
KVM: selftests: arm64: Make set_id_regs bitfield validity checks non-fatal
This commit converts fatal assertions in set_id_regs feature register
validation checks into non-fatal kselftest result reports that include the
affected bitfield name and allow subsequent tests to continue running.
> diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
> index 37107eadd9854..d7f8b8efe1e28 100644
> --- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
> +++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
> @@ -423,6 +423,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;
> @@ -435,7 +436,10 @@ static u64 test_reg_set_success(struct kvm_vcpu *vcpu, u64 reg,
>
> vcpu_set_reg(vcpu, reg, val);
> 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);
[Severity: Low]
As reported by Sashiko on v8 [1], in test_reg_set_success() in
tools/testing/selftests/kvm/arm64/set_id_regs.c, vcpu_set_reg() is still used
to write the register value before checking whether the write succeeded.
In tools/testing/selftests/kvm/include/kvm_util.h, vcpu_set_reg() calls
vcpu_ioctl(), which triggers a fatal assertion if the KVM_SET_ONE_REG ioctl
returns a non-zero value:
tools/testing/selftests/kvm/include/kvm_util.h:vcpu_set_reg() {
struct kvm_one_reg reg = { .id = id, .addr = (u64)&val };
TEST_ASSERT(KVM_REG_SIZE(id) <= sizeof(val), "Reg %lx too big", id);
vcpu_ioctl(vcpu, KVM_SET_ONE_REG, ®);
}
When KVM rejects an ID register bitfield write in arch/arm64/kvm/sys_regs.c,
set_id_reg() returns -EINVAL from the ioctl rather than succeeding and
leaving the register unchanged:
arch/arm64/kvm/sys_regs.c:set_id_reg() {
...
ret = arm64_check_features(vcpu, rd, val);
if (!ret)
kvm_set_vm_id_reg(vcpu->kvm, id, val);
mutex_unlock(&vcpu->kvm->arch.config_lock);
if (ret == -E2BIG)
ret = -EINVAL;
return ret;
}
If KVM unexpectedly rejects a valid write during
main() -> test_vm_ftr_id_regs() -> test_reg_set_success(), wouldn't
vcpu_set_reg() abort the test binary immediately before reaching
ksft_test_result() to report ftr_bits->name and continue running the remaining
tests? Should test_reg_set_success() use __vcpu_set_reg() and handle a
non-zero return value non-fatally, similar to test_reg_set_fail()?
[1] https://lore.kernel.org/r/20260929163445.0D9331F000FF@smtp.kernel.org
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-kvm-arm64-set-id-regs-aarch64-v9-0-ab32bc79fa9c@kernel.org?part=3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v9 1/3] KVM: selftests: arm64: Report set_id_reg reads of test registers as tests
2026-11-03 18:19 ` [PATCH v9 1/3] KVM: selftests: arm64: Report set_id_reg reads of test registers as tests Mark Brown
@ 2026-11-04 11:32 ` Sebastian Ott
0 siblings, 0 replies; 7+ messages in thread
From: Sebastian Ott @ 2026-11-04 11:32 UTC (permalink / raw)
To: Mark Brown
Cc: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Paolo Bonzini,
Shuah Khan, Oliver Upton, Ben Horgan, linux-arm-kernel, kvmarm,
kvm, linux-kselftest, linux-kernel, Lorenzo Stoakes (ARM)
On Tue, 3 Nov 2026, Mark Brown wrote:
> Currently when we run guest code to validate that the values we wrote to
> the registers are seen by the guest we assert that these values match using
> a KVM selftests level assert, resulting in unclear diagnostics if the test
> fails. Replace this assert with reporting a kselftest test per register.
>
> In order to support getting the names of the registers we repaint the array
> of ID_ registers to store the names and open code the rest.
>
> Reviewed-by: Ben Horgan <ben.horgan@arm.com>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> Signed-off-by: Mark Brown <broonie@kernel.org>
[...]
> @@ -684,11 +719,20 @@ static void test_guest_reg_read(struct kvm_vcpu *vcpu)
> REPORT_GUEST_ASSERT(uc);
> break;
> case UCALL_SYNC:
> - val = test_reg_vals[encoding_to_range_idx(uc.args[2])];
> - val = reset_mutable_bits(uc.args[2], val);
> + expected_val = test_reg_vals[encoding_to_range_idx(uc.args[2])];
> + expected_val = reset_mutable_bits(uc.args[2], expected_val);
>
> /* Make sure the written values are seen by guest */
> - TEST_ASSERT_EQ(val, reset_mutable_bits(uc.args[2], uc.args[3]));
> + reg_id = uc.args[2];
^
Minor nit: this could be moved above and be used in 3 more places.
> + guest_val = reset_mutable_bits(uc.args[2], uc.args[3]);
> +
> + match = expected_val == guest_val;
> + if (!match)
> + ksft_print_msg("%lx != %lx\n",
> + expected_val, guest_val);
> + ksft_test_result(match,
> + "%s value seen in guest\n",
> + get_reg_name(reg_id));
> break;
> case UCALL_DONE:
> done = true;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v9 0/3] KVM: selftests: arm64: Improve diagnostics from set_id_regs
2026-11-03 18:19 [PATCH v9 0/3] KVM: selftests: arm64: Improve diagnostics from set_id_regs Mark Brown
` (2 preceding siblings ...)
2026-11-03 18:19 ` [PATCH v9 3/3] KVM: selftests: arm64: Make set_id_regs bitfield validity checks non-fatal Mark Brown
@ 2026-11-04 11:37 ` Sebastian Ott
3 siblings, 0 replies; 7+ messages in thread
From: Sebastian Ott @ 2026-11-04 11:37 UTC (permalink / raw)
To: Mark Brown
Cc: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Paolo Bonzini,
Shuah Khan, Oliver Upton, Ben Horgan, linux-arm-kernel, kvmarm,
kvm, linux-kselftest, linux-kernel, Lorenzo Stoakes (ARM)
On Tue, 3 Nov 2026, Mark Brown wrote:
> While debugging issues related to aarch64 only systems I ran into
> speedbumps due to the lack of detail in the results reported when the
> guest register read and reset value preservation tests were run, they
> generated an immediately fatal assert without indicating which register
> was being tested. Update these tests to non-fatally report a standard
> kselftest result per register, making it much easier to see what the
> problem being reported is.
>
> A similar issue exists with the validation of the individual bitfields
> in registers due to the use of immediately fatal asserts. Update those
> asserts to be standard kselftest reports.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
For the series:
Reviewed-by: Sebastian Ott <sebott@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-11-04 11:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 18:19 [PATCH v9 0/3] KVM: selftests: arm64: Improve diagnostics from set_id_regs Mark Brown
2026-11-03 18:19 ` [PATCH v9 1/3] KVM: selftests: arm64: Report set_id_reg reads of test registers as tests Mark Brown
2026-11-04 11:32 ` Sebastian Ott
2026-11-03 18:19 ` [PATCH v9 2/3] KVM: selftests: arm64: Report register reset tests individually Mark Brown
2026-11-03 18:19 ` [PATCH v9 3/3] KVM: selftests: arm64: Make set_id_regs bitfield validity checks non-fatal Mark Brown
2026-11-03 18:31 ` sashiko-bot
2026-11-04 11:37 ` [PATCH v9 0/3] KVM: selftests: arm64: Improve diagnostics from set_id_regs Sebastian Ott
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox