On Tue, Sep 01, 2026 at 07:50:02PM +0100, Mark Brown wrote:
> 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.

Sounds reasonable.

>
> Reviewed-by: Ben Horgan <[email protected]>
> Signed-off-by: Mark Brown <[email protected]>

All LGTM so:

Reviewed-by: Lorenzo Stoakes (ARM) <[email protected]>

> ---
>  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 db6414a93ad3..3aa8886e8b70 100644
> --- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
> +++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
> @@ -819,13 +819,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));

Ah yeah -> individual tests.

>  }
>
> +#define ID_REG_RESET_UNCHANGED_TEST (ARRAY_SIZE(test_regs) + NUM_EXTRA_REGS)

Yeah can see from original test_reset_preserves_id_regs() that this is correct.

> +
>  static void test_reset_preserves_id_regs(struct kvm_vcpu *vcpu)
>  {
>       /*
> @@ -843,8 +851,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__);
>  }
>
>  int main(void)
> @@ -872,7 +878,8 @@ int main(void)
>
>       ksft_print_header();
>
> -     test_cnt = 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST + GUEST_READ_TEST;
> +     test_cnt = 2 + 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
>
>

--
Cheers, Lorenzo

Reply via email to