diff mbox series

[v5,4/5] KVM: selftests: Add test for PSCI SYSTEM_OFF2

Message ID 20240926184546.833516-5-dwmw2@infradead.org
State New
Headers show
Series Add PSCI v1.3 SYSTEM_OFF2 support for hibernation | expand

Commit Message

David Woodhouse Sept. 26, 2024, 6:37 p.m. UTC
From: David Woodhouse <dwmw@amazon.co.uk>

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 .../testing/selftests/kvm/aarch64/psci_test.c | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)

Comments

Oliver Upton Oct. 1, 2024, 3:33 p.m. UTC | #1
On Thu, Sep 26, 2024 at 07:37:59PM +0100, David Woodhouse wrote:
> +static void guest_test_system_off2(void)
> +{
> +	uint64_t ret;
> +
> +	/* assert that SYSTEM_OFF2 is discoverable */
> +	GUEST_ASSERT(psci_features(PSCI_1_3_FN_SYSTEM_OFF2) &
> +		     BIT(PSCI_1_3_HIBERNATE_TYPE_OFF));
> +	GUEST_ASSERT(psci_features(PSCI_1_3_FN64_SYSTEM_OFF2) &
> +		     BIT(PSCI_1_3_HIBERNATE_TYPE_OFF));
> +

Can you also assert that the guest gets INVALID_PARAMETERS if it sets
arg1 or arg2 to a reserved value?

> +	ret = psci_system_off2(PSCI_1_3_HIBERNATE_TYPE_OFF);
> +	GUEST_SYNC(ret);
> +}
> +
> +static void host_test_system_off2(void)
> +{
> +	struct kvm_vcpu *source, *target;
> +	uint64_t psci_version = 0;
> +	struct kvm_run *run;
> +	struct kvm_vm *vm;
> +
> +	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?

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.
Sean Christopherson Oct. 17, 2024, 12:23 a.m. UTC | #2
On Tue, Oct 15, 2024, Oliver Upton wrote:
> On Sat, Oct 12, 2024 at 10:28:10AM +0100, David Woodhouse wrote:
> > 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.

FWIW, I have some ideas on how to use the nicer pieces of kselftest harness, if
anyone is looking for a project.  :-)

https://lore.kernel.org/all/ZjUwqEXPA5QVItyX@google.com
David Woodhouse Oct. 19, 2024, 5:14 p.m. UTC | #3
On Tue, 2024-10-15 at 10:16 -0700, Oliver Upton wrote:
> 
> > 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.
> 
> Sorry, I meant TEST_ASSERT() here.

Ack. I'll do the latter.
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/aarch64/psci_test.c b/tools/testing/selftests/kvm/aarch64/psci_test.c
index 61731a950def..b7e37956aecf 100644
--- a/tools/testing/selftests/kvm/aarch64/psci_test.c
+++ b/tools/testing/selftests/kvm/aarch64/psci_test.c
@@ -54,6 +54,15 @@  static uint64_t psci_system_suspend(uint64_t entry_addr, uint64_t context_id)
 	return res.a0;
 }
 
+static uint64_t psci_system_off2(uint64_t type)
+{
+	struct arm_smccc_res res;
+
+	smccc_hvc(PSCI_1_3_FN64_SYSTEM_OFF2, type, 0, 0, 0, 0, 0, 0, &res);
+
+	return res.a0;
+}
+
 static uint64_t psci_features(uint32_t func_id)
 {
 	struct arm_smccc_res res;
@@ -188,11 +197,63 @@  static void host_test_system_suspend(void)
 	kvm_vm_free(vm);
 }
 
+static void guest_test_system_off2(void)
+{
+	uint64_t ret;
+
+	/* assert that SYSTEM_OFF2 is discoverable */
+	GUEST_ASSERT(psci_features(PSCI_1_3_FN_SYSTEM_OFF2) &
+		     BIT(PSCI_1_3_HIBERNATE_TYPE_OFF));
+	GUEST_ASSERT(psci_features(PSCI_1_3_FN64_SYSTEM_OFF2) &
+		     BIT(PSCI_1_3_HIBERNATE_TYPE_OFF));
+
+	ret = psci_system_off2(PSCI_1_3_HIBERNATE_TYPE_OFF);
+	GUEST_SYNC(ret);
+}
+
+static void host_test_system_off2(void)
+{
+	struct kvm_vcpu *source, *target;
+	uint64_t psci_version = 0;
+	struct kvm_run *run;
+	struct kvm_vm *vm;
+
+	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;
+
+	vcpu_power_off(target);
+	run = source->run;
+
+	enter_guest(source);
+
+	TEST_ASSERT_KVM_EXIT_REASON(source, KVM_EXIT_SYSTEM_EVENT);
+	TEST_ASSERT(run->system_event.type == KVM_SYSTEM_EVENT_SHUTDOWN,
+		    "Unhandled system event: %u (expected: %u)",
+		    run->system_event.type, KVM_SYSTEM_EVENT_SHUTDOWN);
+	TEST_ASSERT(run->system_event.ndata >= 1,
+		    "Unexpected amount of system event data: %u (expected, >= 1)",
+		    run->system_event.ndata);
+	TEST_ASSERT(run->system_event.data[0] & KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2,
+		    "PSCI_OFF2 flag not set. Flags %llu (expected %llu)",
+		    run->system_event.data[0], KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2);
+
+ skip:
+	kvm_vm_free(vm);
+}
+
 int main(void)
 {
 	TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_SYSTEM_SUSPEND));
 
 	host_test_cpu_on();
 	host_test_system_suspend();
+	host_test_system_off2();
 	return 0;
 }