Re: [PATCH] KVM: arm64: selftests: Check ID regs are immutable after a failed run

On Tue, 4 Aug 2026 at 19:14, Mark Brown <[email protected]> wrote:
...
> > +     if (!found) {
> > +             ksft_test_result_skip("%s (no immutable ID reg field to test)\n",
> > +                                   __func__);
> > +             kvm_vm_free(vm);
> > +             return;
> > +     }
>
> All test result reports for a given test should use the same string when
> reporting so that automation can figure out that results for a given
> test from different runs correspond to each other.  It's better to print
> a diagnostic message, then the ksft_test_result_() with the result for
> the framework.

Thanks, will fix in v2: the skip will report under the same name, with
the reason as a ksft_print_msg().

>
> > +     TEST_ASSERT(r < 0 && errno == EBUSY,
> > +                 "ID reg write after failed first run: r=%d errno=%d",
> > +                 r, errno);
> > +     TEST_ASSERT_EQ(vcpu_get_reg(vcpu, reg), val);
>
> > +     kvm_vm_free(vm);
> > +     ksft_test_result_pass("%s\n", __func__);
>
> Mixing TEST_ASSERT() and ksft_test_result_ in the same test program also
> interacts really poorly with automation, TEST_ASSERT() just kills the

Agreed, but it looks file-wide rather than specific to this test, so I
would rather leave it to your series than convert one function. Happy
to follow once that lands.

Cheers,
/fuad


> entire program so no result is reported for the actual failing test and
> any tests the program would attempt to run afterwards also vanish from
> the results.  If the test program is using the kselftest framework to
> report tests as this one is it should report the results via kselftest
> framework.  A better pattern would be something like:
>
>         if (r < 0 && errno == EBUSY) {
>                 ksft_print_msg("ID reg write after failed first run: r=%d errno=%d",
>                                 r, errno);
>                 pass = false;
>                 goto out;
>         }
>
> ...
>
>  out:
>         kvm_vm_free(vm);
>         ksft_test_result(pass, "%s\n", __func__);
>
> TEST_ASSERT() is a good fit for tests that are just one test case per
> program, then it plays nicely with tooling since the tooling just
> tracking the executable result and any output is diagnostic logging.