On Sat, Oct 12, 2024 at 10:28:10AM +0100, David Woodhouse wrote:
On Tue, 2024-10-01 at 08:33 -0700, Oliver Upton wrote:
On Thu, Sep 26, 2024 at 07:37:59PM +0100, David Woodhouse wrote:
+ vm = setup_vm(guest_test_system_off2, &source, &target); + vcpu_get_reg(target, KVM_REG_ARM_PSCI_VERSION, &psci_version); + TEST_ASSERT(psci_version >= PSCI_VERSION(0, 2), + "Unexpected PSCI version %lu.%lu", + PSCI_VERSION_MAJOR(psci_version), + PSCI_VERSION_MINOR(psci_version));
+ if (psci_version < PSCI_VERSION(1,3)) + goto skip;
I'm not following this. Is there a particular reason why we'd want to skip for v1.2 and fail the test for anything less than that?
These tests unconditionally set KVM_ARM_VCPU_PSCI_0_2 in setup_vm(). Which is probably OK assuming support for that that predates KVM_CAP_ARM_SYSTEM_SUSPEND (which is already a TEST_REQUIRE() right at the start).
So the world is very broken if KVM actually starts a VM but the version isn't at least 0.2, and it seemed like it warranted an actual failure.
If we're looking at this from a testing lens then KVM coming up with any PSCI version other than KVM_ARM_PSCI_LATEST (i.e. v1.3) is a bug. So maybe we can tighten that assertion because...
Just do TEST_REQUIRE(psci_version >= PSCI_VERSION(1, 3)), it makes the requirements obvious in the case someone runs new selftests on an old kernel.
I don't think we want to put that in main() and skip the other checks that would run on earlier kernels.
Running KVM selftests on older kernels in a meaningful way isn't something we support. At all. An example of this is commit 8a53e1302133 ("KVM: selftests: Require KVM_CAP_USER_MEMORY2 for tests that create memslots"), which skips ~everything for kernels older than 6.8.
(Even if we had easy access to psci_version without actually running a test and starting a VM).
I could put it into host_test_system_off2() which runs last (and comment the invocations in main() to say that they're in increasing order of PSCI version) to accommodate such). But then it seems that I'd be the target of this comment in ksft_exit_skip()...
/* * FIXME: several tests misuse ksft_exit_skip so produce * something sensible if some tests have already been run * or a plan has been printed. Those tests should use * ksft_test_result_skip or ksft_exit_fail_msg instead. */I suspect the real answer here is that the individual tests here be calling ksft_test_result_pass(), and the system_off2 one should call ksft_test_result_skip() if it skips?
modulo a few one-offs, KVM selftests doesn't use the kselftest harness so it isn't subject to this comment. Since there's no test plan, we can skip at any time.
I'll add an explicit comment about the 0.2 check though, saying that it should never happen so we might as well have the ASSERT for it.
After looking at this again, I think we should do one of the following:
- TEST_REQUIRE() that the PSCI version is at least v1.3, making the dependency clear on older kernels.
- TEST_REQUIRE() for v1.3, which would provide better test coverage on upstream.