* [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification
@ 2024-09-24 16:05 David Woodhouse
2024-09-24 16:05 ` [PATCH v4 2/6] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation David Woodhouse
` (7 more replies)
0 siblings, 8 replies; 16+ messages in thread
From: David Woodhouse @ 2024-09-24 16:05 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Marc Zyngier, Oliver Upton,
James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Lorenzo Pieralisi, Rafael J. Wysocki,
Pavel Machek, Len Brown, Shuah Khan, David Woodhouse, kvm,
linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-pm,
linux-kselftest
From: David Woodhouse <dwmw@amazon.co.uk>
The v1.3 PSCI spec (https://developer.arm.com/documentation/den0022) adds
SYSTEM_OFF2, CLEAN_INV_MEMREGION and CLEAN_INV_MEMREGION_ATTRIBUTES
functions. Add definitions for them and their parameters, along with the
new TIMEOUT, RATE_LIMITED and BUSY error values.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
include/uapi/linux/psci.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index 42a40ad3fb62..082ed689fdaf 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -59,6 +59,8 @@
#define PSCI_1_1_FN_SYSTEM_RESET2 PSCI_0_2_FN(18)
#define PSCI_1_1_FN_MEM_PROTECT PSCI_0_2_FN(19)
#define PSCI_1_1_FN_MEM_PROTECT_CHECK_RANGE PSCI_0_2_FN(20)
+#define PSCI_1_3_FN_SYSTEM_OFF2 PSCI_0_2_FN(21)
+#define PSCI_1_3_FN_CLEAN_INV_MEMREGION_ATTRIBUTES PSCI_0_2_FN(23)
#define PSCI_1_0_FN64_CPU_DEFAULT_SUSPEND PSCI_0_2_FN64(12)
#define PSCI_1_0_FN64_NODE_HW_STATE PSCI_0_2_FN64(13)
@@ -68,6 +70,8 @@
#define PSCI_1_1_FN64_SYSTEM_RESET2 PSCI_0_2_FN64(18)
#define PSCI_1_1_FN64_MEM_PROTECT_CHECK_RANGE PSCI_0_2_FN64(20)
+#define PSCI_1_3_FN64_SYSTEM_OFF2 PSCI_0_2_FN64(21)
+#define PSCI_1_3_FN64_CLEAN_INV_MEMREGION PSCI_0_2_FN64(22)
/* PSCI v0.2 power state encoding for CPU_SUSPEND function */
#define PSCI_0_2_POWER_STATE_ID_MASK 0xffff
@@ -100,6 +104,19 @@
#define PSCI_1_1_RESET_TYPE_SYSTEM_WARM_RESET 0
#define PSCI_1_1_RESET_TYPE_VENDOR_START 0x80000000U
+/* PSCI v1.3 hibernate type for SYSTEM_OFF2 */
+#define PSCI_1_3_HIBERNATE_TYPE_OFF 0
+
+/* PSCI v1.3 flags for CLEAN_INV_MEMREGION */
+#define PSCI_1_3_CLEAN_INV_MEMREGION_FLAG_DRY_RUN BIT(0)
+
+/* PSCI v1.3 attributes for CLEAN_INV_MEMREGION_ATTRIBUTES */
+#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_OP_TYPE 0
+#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_CPU_RDVZ 1
+#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_LATENCY 2
+#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_RATE_LIMIT 3
+#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_TIMEOUT 4
+
/* PSCI version decoding (independent of PSCI version) */
#define PSCI_VERSION_MAJOR_SHIFT 16
#define PSCI_VERSION_MINOR_MASK \
@@ -133,5 +150,8 @@
#define PSCI_RET_NOT_PRESENT -7
#define PSCI_RET_DISABLED -8
#define PSCI_RET_INVALID_ADDRESS -9
+#define PSCI_RET_TIMEOUT -10
+#define PSCI_RET_RATE_LIMITED -11
+#define PSCI_RET_BUSY -12
#endif /* _UAPI_LINUX_PSCI_H */
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 2/6] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation
2024-09-24 16:05 [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification David Woodhouse
@ 2024-09-24 16:05 ` David Woodhouse
2024-09-26 9:14 ` Francesco Lavra
2024-09-24 16:05 ` [PATCH v4 3/6] KVM: arm64: Add support for PSCI v1.2 and v1.3 David Woodhouse
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: David Woodhouse @ 2024-09-24 16:05 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Marc Zyngier, Oliver Upton,
James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Lorenzo Pieralisi, Rafael J. Wysocki,
Pavel Machek, Len Brown, Shuah Khan, David Woodhouse, kvm,
linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-pm,
linux-kselftest
From: David Woodhouse <dwmw@amazon.co.uk>
The PSCI v1.3 specification (alpha) adds support for a SYSTEM_OFF2 function
which is analogous to ACPI S4 state. This will allow hosting environments
to determine that a guest is hibernated rather than just powered off, and
ensure that they preserve the virtual environment appropriately to allow
the guest to resume safely (or bump the hardware_signature in the FACS to
trigger a clean reboot instead).
The beta version will be changed to say that PSCI_FEATURES returns a bit
mask of the supported hibernate types, which is implemented here.
Although this new feature is inflicted unconditionally on unexpecting
userspace, it ought to be mostly OK because it still results in the same
KVM_SYSTEM_EVENT_SHUTDOWN event, just with a new flag which hopefully
won't cause userspace to get unhappy.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
Documentation/virt/kvm/api.rst | 11 +++++++++
arch/arm64/include/uapi/asm/kvm.h | 6 +++++
arch/arm64/kvm/psci.c | 37 +++++++++++++++++++++++++++++++
3 files changed, 54 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index b3be87489108..2918898b7047 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6840,6 +6840,10 @@ the first `ndata` items (possibly zero) of the data array are valid.
the guest issued a SYSTEM_RESET2 call according to v1.1 of the PSCI
specification.
+ - for arm64, data[0] is set to KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2
+ if the guest issued a SYSTEM_OFF2 call according to v1.3 of the PSCI
+ specification.
+
- for RISC-V, data[0] is set to the value of the second argument of the
``sbi_system_reset`` call.
@@ -6873,6 +6877,13 @@ either:
- Deny the guest request to suspend the VM. See ARM DEN0022D.b 5.19.2
"Caller responsibilities" for possible return values.
+Hibernation using the PSCI SYSTEM_OFF2 call is enabled when PSCI v1.3
+is enabled. If a guest invokes the PSCI SYSTEM_OFF2 function, KVM will
+exit to userspace with the KVM_SYSTEM_EVENT_SHUTDOWN event type and with
+data[0] set to KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2. The only
+supported hibernate type for the SYSTEM_OFF2 function is HIBERNATE_OFF
+0x0).
+
::
/* KVM_EXIT_IOAPIC_EOI */
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 964df31da975..66736ff04011 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -484,6 +484,12 @@ enum {
*/
#define KVM_SYSTEM_EVENT_RESET_FLAG_PSCI_RESET2 (1ULL << 0)
+/*
+ * Shutdown caused by a PSCI v1.3 SYSTEM_OFF2 call.
+ * Valid only when the system event has a type of KVM_SYSTEM_EVENT_SHUTDOWN.
+ */
+#define KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2 (1ULL << 0)
+
/* run->fail_entry.hardware_entry_failure_reason codes. */
#define KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED (1ULL << 0)
diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c
index 1f69b667332b..fd0f82464f7d 100644
--- a/arch/arm64/kvm/psci.c
+++ b/arch/arm64/kvm/psci.c
@@ -194,6 +194,12 @@ static void kvm_psci_system_off(struct kvm_vcpu *vcpu)
kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_SHUTDOWN, 0);
}
+static void kvm_psci_system_off2(struct kvm_vcpu *vcpu)
+{
+ kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_SHUTDOWN,
+ KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2);
+}
+
static void kvm_psci_system_reset(struct kvm_vcpu *vcpu)
{
kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_RESET, 0);
@@ -358,6 +364,11 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor)
if (minor >= 1)
val = 0;
break;
+ case PSCI_1_3_FN_SYSTEM_OFF2:
+ case PSCI_1_3_FN64_SYSTEM_OFF2:
+ if (minor >= 3)
+ val = BIT(PSCI_1_3_HIBERNATE_TYPE_OFF);
+ break;
}
break;
case PSCI_1_0_FN_SYSTEM_SUSPEND:
@@ -392,6 +403,32 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor)
break;
}
break;
+ case PSCI_1_3_FN_SYSTEM_OFF2:
+ kvm_psci_narrow_to_32bit(vcpu);
+ fallthrough;
+ case PSCI_1_3_FN64_SYSTEM_OFF2:
+ if (minor < 3)
+ break;
+
+ arg = smccc_get_arg1(vcpu);
+ if (arg != PSCI_1_3_HIBERNATE_TYPE_OFF) {
+ val = PSCI_RET_INVALID_PARAMS;
+ break;
+ }
+ kvm_psci_system_off2(vcpu);
+ /*
+ * We shouldn't be going back to guest VCPU after
+ * receiving SYSTEM_OFF2 request.
+ *
+ * If user space accidentally/deliberately resumes
+ * guest VCPU after SYSTEM_OFF2 request then guest
+ * VCPU should see internal failure from PSCI return
+ * value. To achieve this, we preload r0 (or x0) with
+ * PSCI return value INTERNAL_FAILURE.
+ */
+ val = PSCI_RET_INTERNAL_FAILURE;
+ ret = 0;
+ break;
default:
return kvm_psci_0_2_call(vcpu);
}
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 3/6] KVM: arm64: Add support for PSCI v1.2 and v1.3
2024-09-24 16:05 [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification David Woodhouse
2024-09-24 16:05 ` [PATCH v4 2/6] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation David Woodhouse
@ 2024-09-24 16:05 ` David Woodhouse
2024-09-24 16:05 ` [PATCH v4 4/6] KVM: selftests: Add test for PSCI SYSTEM_OFF2 David Woodhouse
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2024-09-24 16:05 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Marc Zyngier, Oliver Upton,
James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Lorenzo Pieralisi, Rafael J. Wysocki,
Pavel Machek, Len Brown, Shuah Khan, David Woodhouse, kvm,
linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-pm,
linux-kselftest
From: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
arch/arm64/kvm/hypercalls.c | 2 ++
arch/arm64/kvm/psci.c | 6 +++++-
include/kvm/arm_psci.h | 4 +++-
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 5763d979d8ca..9c6267ca2b82 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -575,6 +575,8 @@ int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
case KVM_ARM_PSCI_0_2:
case KVM_ARM_PSCI_1_0:
case KVM_ARM_PSCI_1_1:
+ case KVM_ARM_PSCI_1_2:
+ case KVM_ARM_PSCI_1_3:
if (!wants_02)
return -EINVAL;
vcpu->kvm->arch.psci_version = val;
diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c
index fd0f82464f7d..5177dda5a411 100644
--- a/arch/arm64/kvm/psci.c
+++ b/arch/arm64/kvm/psci.c
@@ -328,7 +328,7 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor)
switch(psci_fn) {
case PSCI_0_2_FN_PSCI_VERSION:
- val = minor == 0 ? KVM_ARM_PSCI_1_0 : KVM_ARM_PSCI_1_1;
+ val = PSCI_VERSION(1, minor);
break;
case PSCI_1_0_FN_PSCI_FEATURES:
arg = smccc_get_arg1(vcpu);
@@ -486,6 +486,10 @@ int kvm_psci_call(struct kvm_vcpu *vcpu)
}
switch (version) {
+ case KVM_ARM_PSCI_1_3:
+ return kvm_psci_1_x_call(vcpu, 3);
+ case KVM_ARM_PSCI_1_2:
+ return kvm_psci_1_x_call(vcpu, 2);
case KVM_ARM_PSCI_1_1:
return kvm_psci_1_x_call(vcpu, 1);
case KVM_ARM_PSCI_1_0:
diff --git a/include/kvm/arm_psci.h b/include/kvm/arm_psci.h
index e8fb624013d1..cbaec804eb83 100644
--- a/include/kvm/arm_psci.h
+++ b/include/kvm/arm_psci.h
@@ -14,8 +14,10 @@
#define KVM_ARM_PSCI_0_2 PSCI_VERSION(0, 2)
#define KVM_ARM_PSCI_1_0 PSCI_VERSION(1, 0)
#define KVM_ARM_PSCI_1_1 PSCI_VERSION(1, 1)
+#define KVM_ARM_PSCI_1_2 PSCI_VERSION(1, 2)
+#define KVM_ARM_PSCI_1_3 PSCI_VERSION(1, 3)
-#define KVM_ARM_PSCI_LATEST KVM_ARM_PSCI_1_1
+#define KVM_ARM_PSCI_LATEST KVM_ARM_PSCI_1_3
static inline int kvm_psci_version(struct kvm_vcpu *vcpu)
{
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 4/6] KVM: selftests: Add test for PSCI SYSTEM_OFF2
2024-09-24 16:05 [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification David Woodhouse
2024-09-24 16:05 ` [PATCH v4 2/6] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation David Woodhouse
2024-09-24 16:05 ` [PATCH v4 3/6] KVM: arm64: Add support for PSCI v1.2 and v1.3 David Woodhouse
@ 2024-09-24 16:05 ` David Woodhouse
2024-09-24 16:05 ` [PATCH v4 5/6] KVM: arm64: nvhe: Pass through PSCI v1.3 SYSTEM_OFF2 call David Woodhouse
` (4 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2024-09-24 16:05 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Marc Zyngier, Oliver Upton,
James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Lorenzo Pieralisi, Rafael J. Wysocki,
Pavel Machek, Len Brown, Shuah Khan, David Woodhouse, kvm,
linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-pm,
linux-kselftest
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(+)
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;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 5/6] KVM: arm64: nvhe: Pass through PSCI v1.3 SYSTEM_OFF2 call
2024-09-24 16:05 [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification David Woodhouse
` (2 preceding siblings ...)
2024-09-24 16:05 ` [PATCH v4 4/6] KVM: selftests: Add test for PSCI SYSTEM_OFF2 David Woodhouse
@ 2024-09-24 16:05 ` David Woodhouse
2024-09-24 16:05 ` [PATCH v4 6/6] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate David Woodhouse
` (3 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2024-09-24 16:05 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Marc Zyngier, Oliver Upton,
James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Lorenzo Pieralisi, Rafael J. Wysocki,
Pavel Machek, Len Brown, Shuah Khan, David Woodhouse, kvm,
linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-pm,
linux-kselftest
From: David Woodhouse <dwmw@amazon.co.uk>
Pass through the SYSTEM_OFF2 function for hibernation, just like SYSTEM_OFF.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/kvm/hyp/nvhe/psci-relay.c b/arch/arm64/kvm/hyp/nvhe/psci-relay.c
index dfe8fe0f7eaf..9c2ce1e0e99a 100644
--- a/arch/arm64/kvm/hyp/nvhe/psci-relay.c
+++ b/arch/arm64/kvm/hyp/nvhe/psci-relay.c
@@ -265,6 +265,8 @@ static unsigned long psci_1_0_handler(u64 func_id, struct kvm_cpu_context *host_
case PSCI_1_0_FN_PSCI_FEATURES:
case PSCI_1_0_FN_SET_SUSPEND_MODE:
case PSCI_1_1_FN64_SYSTEM_RESET2:
+ case PSCI_1_3_FN_SYSTEM_OFF2:
+ case PSCI_1_3_FN64_SYSTEM_OFF2:
return psci_forward(host_ctxt);
case PSCI_1_0_FN64_SYSTEM_SUSPEND:
return psci_system_suspend(func_id, host_ctxt);
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 6/6] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate
2024-09-24 16:05 [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification David Woodhouse
` (3 preceding siblings ...)
2024-09-24 16:05 ` [PATCH v4 5/6] KVM: arm64: nvhe: Pass through PSCI v1.3 SYSTEM_OFF2 call David Woodhouse
@ 2024-09-24 16:05 ` David Woodhouse
2024-09-26 9:18 ` Francesco Lavra
2024-09-24 16:13 ` [PATCH v4 0/6] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation David Woodhouse
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: David Woodhouse @ 2024-09-24 16:05 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Marc Zyngier, Oliver Upton,
James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Lorenzo Pieralisi, Rafael J. Wysocki,
Pavel Machek, Len Brown, Shuah Khan, David Woodhouse, kvm,
linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-pm,
linux-kselftest
From: David Woodhouse <dwmw@amazon.co.uk>
The PSCI v1.3 specification (alpha) adds support for a SYSTEM_OFF2
function which is analogous to ACPI S4 state. This will allow hosting
environments to determine that a guest is hibernated rather than just
powered off, and handle that state appropriately on subsequent launches.
Since commit 60c0d45a7f7a ("efi/arm64: use UEFI for system reset and
poweroff") the EFI shutdown method is deliberately preferred over PSCI
or other methods. So register a SYS_OFF_MODE_POWER_OFF handler which
*only* handles the hibernation, leaving the original PSCI SYSTEM_OFF as
a last resort via the legacy pm_power_off function pointer.
The hibernation code already exports a system_entering_hibernation()
function which is be used by the higher-priority handler to check for
hibernation. That existing function just returns the value of a static
boolean variable from hibernate.c, which was previously only set in the
hibernation_platform_enter() code path. Set the same flag in the simpler
code path around the call to kernel_power_off() too.
An alternative way to hook SYSTEM_OFF2 into the hibernation code would
be to register a platform_hibernation_ops structure with an ->enter()
method which makes the new SYSTEM_OFF2 call. But that would have the
unwanted side-effect of making hibernation take a completely different
code path in hibernation_platform_enter(), invoking a lot of special dpm
callbacks.
Another option might be to add a new SYS_OFF_MODE_HIBERNATE mode, with
fallback to SYS_OFF_MODE_POWER_OFF. Or to use the sys_off_data to
indicate whether the power off is for hibernation.
But this version works and is relatively simple.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Acked-by: Rafael J. Wysocki <rafael@kernel.org>
---
drivers/firmware/psci/psci.c | 37 ++++++++++++++++++++++++++++++++++++
kernel/power/hibernate.c | 5 ++++-
2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index 2328ca58bba6..fbf09a51b817 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -78,6 +78,7 @@ struct psci_0_1_function_ids get_psci_0_1_function_ids(void)
static u32 psci_cpu_suspend_feature;
static bool psci_system_reset2_supported;
+static bool psci_system_off2_hibernate_supported;
static inline bool psci_has_ext_power_state(void)
{
@@ -333,6 +334,28 @@ static void psci_sys_poweroff(void)
invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
}
+#ifdef CONFIG_HIBERNATION
+static int psci_sys_hibernate(struct sys_off_data *data)
+{
+ if (system_entering_hibernation())
+ invoke_psci_fn(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2),
+ PSCI_1_3_HIBERNATE_TYPE_OFF, 0, 0);
+ return NOTIFY_DONE;
+}
+
+static int __init psci_hibernate_init(void)
+{
+ if (psci_system_off2_hibernate_supported) {
+ /* Higher priority than EFI shutdown, but only for hibernate */
+ register_sys_off_handler(SYS_OFF_MODE_POWER_OFF,
+ SYS_OFF_PRIO_FIRMWARE + 2,
+ psci_sys_hibernate, NULL);
+ }
+ return 0;
+}
+subsys_initcall(psci_hibernate_init);
+#endif
+
static int psci_features(u32 psci_func_id)
{
return invoke_psci_fn(PSCI_1_0_FN_PSCI_FEATURES,
@@ -364,6 +387,7 @@ static const struct {
PSCI_ID_NATIVE(1_1, SYSTEM_RESET2),
PSCI_ID(1_1, MEM_PROTECT),
PSCI_ID_NATIVE(1_1, MEM_PROTECT_CHECK_RANGE),
+ PSCI_ID_NATIVE(1_3, SYSTEM_OFF2),
};
static int psci_debugfs_read(struct seq_file *s, void *data)
@@ -525,6 +549,18 @@ static void __init psci_init_system_reset2(void)
psci_system_reset2_supported = true;
}
+static void __init psci_init_system_off2(void)
+{
+ int ret;
+
+ ret = psci_features(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2));
+ if (ret < 0)
+ return;
+
+ if (ret & BIT(PSCI_1_3_HIBERNATE_TYPE_OFF))
+ psci_system_off2_hibernate_supported = true;
+}
+
static void __init psci_init_system_suspend(void)
{
int ret;
@@ -655,6 +691,7 @@ static int __init psci_probe(void)
psci_init_cpu_suspend();
psci_init_system_suspend();
psci_init_system_reset2();
+ psci_init_system_off2();
kvm_init_hyp_services();
}
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index e35829d36039..1f87aa01ba44 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -685,8 +685,11 @@ static void power_down(void)
}
fallthrough;
case HIBERNATION_SHUTDOWN:
- if (kernel_can_power_off())
+ if (kernel_can_power_off()) {
+ entering_platform_hibernation = true;
kernel_power_off();
+ entering_platform_hibernation = false;
+ }
break;
}
kernel_halt();
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 0/6] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation
2024-09-24 16:05 [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification David Woodhouse
` (4 preceding siblings ...)
2024-09-24 16:05 ` [PATCH v4 6/6] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate David Woodhouse
@ 2024-09-24 16:13 ` David Woodhouse
2024-09-26 8:55 ` [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification Francesco Lavra
2024-09-26 9:56 ` Miguel Luis
7 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2024-09-24 16:13 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Marc Zyngier, Oliver Upton,
James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Lorenzo Pieralisi, Rafael J. Wysocki,
Pavel Machek, Len Brown, Shuah Khan, kvm, linux-doc, linux-kernel,
linux-arm-kernel, kvmarm, linux-pm, linux-kselftest
[-- Attachment #1: Type: text/plain, Size: 2832 bytes --]
(oops, missed --compose on that command line. You can have the cover
letter as a reply instead)
The PSCI v1.3 spec (https://developer.arm.com/documentation/den0022)
adds support for a SYSTEM_OFF2 function enabling a HIBERNATE_OFF state
which is analogous to ACPI S4. This will allow hosting environments to
determine that a guest is hibernated rather than just powered off, and
ensure that they preserve the virtual environment appropriately to
allow the guest to resume safely (or bump the hardware_signature in the
FACS to trigger a clean reboot instead).
This updates KVM to support advertising PSCI v1.3, and unconditionally
enables the SYSTEM_OFF2 support when PSCI v1.3 is enabled.
For the guest side, add a new SYS_OFF_MODE_POWER_OFF handler with higher
priority than the EFI one, but which *only* triggers when there's a
hibernation in progress. There are other ways to do this (see the commit
message for more details) but this seemed like the simplest.
Version 2 of the patch series splits out the psci.h definitions into a
separate commit (a dependency for both the guest and KVM side), and adds
definitions for the other new functions added in v1.3. It also moves the
pKVM psci-relay support to a separate commit; although in arch/arm64/kvm
that's actually about the *guest* side of SYSTEM_OFF2 (i.e. using it
from the host kernel, relayed through nVHE).
Version 3 dropped the KVM_CAP which allowed userspace to explicitly opt
in to the new feature like with SYSTEM_SUSPEND, and makes it depend only
on PSCI v1.3 being exposed to the guest.
Version 4 is no longer RFC, as the PSCI v1.3 spec is finally published.
Minor fixes from the last round of review, and an added KVM self test.
David Woodhouse (6):
firmware/psci: Add definitions for PSCI v1.3 specification
KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation
KVM: arm64: Add support for PSCI v1.2 and v1.3
KVM: selftests: Add test for PSCI SYSTEM_OFF2
KVM: arm64: nvhe: Pass through PSCI v1.3 SYSTEM_OFF2 call
arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate
Documentation/virt/kvm/api.rst | 11 +++++
arch/arm64/include/uapi/asm/kvm.h | 6 +++
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 2 +
arch/arm64/kvm/hypercalls.c | 2 +
arch/arm64/kvm/psci.c | 43 ++++++++++++++++-
drivers/firmware/psci/psci.c | 37 +++++++++++++++
include/kvm/arm_psci.h | 4 +-
include/uapi/linux/psci.h | 20 ++++++++
kernel/power/hibernate.c | 5 +-
tools/testing/selftests/kvm/aarch64/psci_test.c | 61 +++++++++++++++++++++++++
10 files changed, 188 insertions(+), 3 deletions(-)
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification
2024-09-24 16:05 [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification David Woodhouse
` (5 preceding siblings ...)
2024-09-24 16:13 ` [PATCH v4 0/6] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation David Woodhouse
@ 2024-09-26 8:55 ` Francesco Lavra
2024-09-26 9:59 ` David Woodhouse
2024-09-26 9:56 ` Miguel Luis
7 siblings, 1 reply; 16+ messages in thread
From: Francesco Lavra @ 2024-09-26 8:55 UTC (permalink / raw)
To: David Woodhouse, Paolo Bonzini, Jonathan Corbet, Marc Zyngier,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Pavel Machek, Len Brown, Shuah Khan,
David Woodhouse, kvm, linux-doc, linux-kernel, linux-arm-kernel,
kvmarm, linux-pm, linux-kselftest
On Tue, 2024-09-24 at 17:05 +0100, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The v1.3 PSCI spec (https://developer.arm.com/documentation/den0022)
> adds
> SYSTEM_OFF2, CLEAN_INV_MEMREGION and CLEAN_INV_MEMREGION_ATTRIBUTES
> functions. Add definitions for them and their parameters, along with
> the
> new TIMEOUT, RATE_LIMITED and BUSY error values.
The CLEAN_INV_MEMREGION and CLEAN_INV_MEMREGION_ATTRIBUTES
functions were added in the alpha release of the spec but have been
dropped in the beta release, and are not included in the final spec. So
IMO the uapi header file should not contain these definitions.
The same goes for the TIMEOUT, RATE_LIMITED and BUSY error values.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 2/6] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation
2024-09-24 16:05 ` [PATCH v4 2/6] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation David Woodhouse
@ 2024-09-26 9:14 ` Francesco Lavra
0 siblings, 0 replies; 16+ messages in thread
From: Francesco Lavra @ 2024-09-26 9:14 UTC (permalink / raw)
To: David Woodhouse, Paolo Bonzini, Jonathan Corbet, Marc Zyngier,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Pavel Machek, Len Brown, Shuah Khan,
David Woodhouse, kvm, linux-doc, linux-kernel, linux-arm-kernel,
kvmarm, linux-pm, linux-kselftest
On Tue, 2024-09-24 at 17:05 +0100, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The PSCI v1.3 specification (alpha) adds support for a SYSTEM_OFF2
Can remove (alpha).
> function
> which is analogous to ACPI S4 state. This will allow hosting
> environments
> to determine that a guest is hibernated rather than just powered off,
> and
> ensure that they preserve the virtual environment appropriately to
> allow
> the guest to resume safely (or bump the hardware_signature in the
> FACS to
> trigger a clean reboot instead).
>
> The beta version will be changed to say that PSCI_FEATURES returns a
> bit
> mask of the supported hibernate types, which is implemented here.
Since the final spec has been released, we can revise or remove the
above wording.
> Although this new feature is inflicted unconditionally on unexpecting
> userspace, it ought to be mostly OK because it still results in the
> same
> KVM_SYSTEM_EVENT_SHUTDOWN event, just with a new flag which hopefully
> won't cause userspace to get unhappy.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
> Documentation/virt/kvm/api.rst | 11 +++++++++
> arch/arm64/include/uapi/asm/kvm.h | 6 +++++
> arch/arm64/kvm/psci.c | 37
> +++++++++++++++++++++++++++++++
> 3 files changed, 54 insertions(+)
>
> diff --git a/Documentation/virt/kvm/api.rst
> b/Documentation/virt/kvm/api.rst
> index b3be87489108..2918898b7047 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6840,6 +6840,10 @@ the first `ndata` items (possibly zero) of the
> data array are valid.
> the guest issued a SYSTEM_RESET2 call according to v1.1 of the
> PSCI
> specification.
>
> + - for arm64, data[0] is set to
> KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2
> + if the guest issued a SYSTEM_OFF2 call according to v1.3 of the
> PSCI
> + specification.
> +
> - for RISC-V, data[0] is set to the value of the second argument of
> the
> ``sbi_system_reset`` call.
>
> @@ -6873,6 +6877,13 @@ either:
> - Deny the guest request to suspend the VM. See ARM DEN0022D.b
> 5.19.2
> "Caller responsibilities" for possible return values.
>
> +Hibernation using the PSCI SYSTEM_OFF2 call is enabled when PSCI
> v1.3
> +is enabled. If a guest invokes the PSCI SYSTEM_OFF2 function, KVM
> will
> +exit to userspace with the KVM_SYSTEM_EVENT_SHUTDOWN event type and
> with
> +data[0] set to KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2. The only
> +supported hibernate type for the SYSTEM_OFF2 function is
> HIBERNATE_OFF
> +0x0).
The spec says that the HIBERNATE_OFF parameter value is 0x1, not 0x0
(which is kind of unfortunate because it doesn't match the
corresponding bit in the feature flags).
So, either the BIT(PSCI_1_3_HIBERNATE_TYPE_OFF) value should be used
for the SYSTEM_OFF2 functions in the code, or the definition should be
changed in the header file (unless the text in the spec is wrong).
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 6/6] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate
2024-09-24 16:05 ` [PATCH v4 6/6] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate David Woodhouse
@ 2024-09-26 9:18 ` Francesco Lavra
0 siblings, 0 replies; 16+ messages in thread
From: Francesco Lavra @ 2024-09-26 9:18 UTC (permalink / raw)
To: David Woodhouse, Paolo Bonzini, Jonathan Corbet, Marc Zyngier,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Pavel Machek, Len Brown, Shuah Khan,
David Woodhouse, kvm, linux-doc, linux-kernel, linux-arm-kernel,
kvmarm, linux-pm, linux-kselftest
On Tue, 2024-09-24 at 17:05 +0100, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The PSCI v1.3 specification (alpha) adds support for a SYSTEM_OFF2
Can remove (alpha).
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification
2024-09-24 16:05 [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification David Woodhouse
` (6 preceding siblings ...)
2024-09-26 8:55 ` [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification Francesco Lavra
@ 2024-09-26 9:56 ` Miguel Luis
2024-09-26 16:30 ` David Woodhouse
7 siblings, 1 reply; 16+ messages in thread
From: Miguel Luis @ 2024-09-26 9:56 UTC (permalink / raw)
To: David Woodhouse
Cc: Paolo Bonzini, Jonathan Corbet, Marc Zyngier, Oliver Upton,
James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Lorenzo Pieralisi, Rafael J. Wysocki,
Pavel Machek, Len Brown, Shuah Khan, David Woodhouse,
kvm@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
linux-pm@vger.kernel.org, linux-kselftest@vger.kernel.org
Hi David,
> On 24 Sep 2024, at 16:05, David Woodhouse <dwmw2@infradead.org> wrote:
>
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The v1.3 PSCI spec (https://developer.arm.com/documentation/den0022) adds
> SYSTEM_OFF2, CLEAN_INV_MEMREGION and CLEAN_INV_MEMREGION_ATTRIBUTES
> functions. Add definitions for them and their parameters, along with the
> new TIMEOUT, RATE_LIMITED and BUSY error values.
>
DEN0022F REL superseded DEN0022F ALP1 which doesn’t describe CLEAN_INV_MEMREGION
or CLEAN_INV_MEMREGION_ATTRIBUTES. Defining those at another time shouldn’t be a
blocker for the rest of this patchset.
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
> include/uapi/linux/psci.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
> index 42a40ad3fb62..082ed689fdaf 100644
> --- a/include/uapi/linux/psci.h
> +++ b/include/uapi/linux/psci.h
> @@ -59,6 +59,8 @@
> #define PSCI_1_1_FN_SYSTEM_RESET2 PSCI_0_2_FN(18)
> #define PSCI_1_1_FN_MEM_PROTECT PSCI_0_2_FN(19)
> #define PSCI_1_1_FN_MEM_PROTECT_CHECK_RANGE PSCI_0_2_FN(20)
> +#define PSCI_1_3_FN_SYSTEM_OFF2 PSCI_0_2_FN(21)
> +#define PSCI_1_3_FN_CLEAN_INV_MEMREGION_ATTRIBUTES PSCI_0_2_FN(23)
>
> #define PSCI_1_0_FN64_CPU_DEFAULT_SUSPEND PSCI_0_2_FN64(12)
> #define PSCI_1_0_FN64_NODE_HW_STATE PSCI_0_2_FN64(13)
> @@ -68,6 +70,8 @@
>
> #define PSCI_1_1_FN64_SYSTEM_RESET2 PSCI_0_2_FN64(18)
> #define PSCI_1_1_FN64_MEM_PROTECT_CHECK_RANGE PSCI_0_2_FN64(20)
> +#define PSCI_1_3_FN64_SYSTEM_OFF2 PSCI_0_2_FN64(21)
> +#define PSCI_1_3_FN64_CLEAN_INV_MEMREGION PSCI_0_2_FN64(22)
>
> /* PSCI v0.2 power state encoding for CPU_SUSPEND function */
> #define PSCI_0_2_POWER_STATE_ID_MASK 0xffff
> @@ -100,6 +104,19 @@
> #define PSCI_1_1_RESET_TYPE_SYSTEM_WARM_RESET 0
> #define PSCI_1_1_RESET_TYPE_VENDOR_START 0x80000000U
>
> +/* PSCI v1.3 hibernate type for SYSTEM_OFF2 */
> +#define PSCI_1_3_HIBERNATE_TYPE_OFF 0
Should it be 1 as hibernate type?
> +
> +/* PSCI v1.3 flags for CLEAN_INV_MEMREGION */
> +#define PSCI_1_3_CLEAN_INV_MEMREGION_FLAG_DRY_RUN BIT(0)
> +
> +/* PSCI v1.3 attributes for CLEAN_INV_MEMREGION_ATTRIBUTES */
> +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_OP_TYPE 0
> +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_CPU_RDVZ 1
> +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_LATENCY 2
> +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_RATE_LIMIT 3
> +#define PSCI_1_3_CLEAN_INV_MEMREGION_ATTR_TIMEOUT 4
> +
> /* PSCI version decoding (independent of PSCI version) */
> #define PSCI_VERSION_MAJOR_SHIFT 16
> #define PSCI_VERSION_MINOR_MASK \
> @@ -133,5 +150,8 @@
> #define PSCI_RET_NOT_PRESENT -7
> #define PSCI_RET_DISABLED -8
> #define PSCI_RET_INVALID_ADDRESS -9
> +#define PSCI_RET_TIMEOUT -10
> +#define PSCI_RET_RATE_LIMITED -11
> +#define PSCI_RET_BUSY -12
>
Thanks
Miguel
> #endif /* _UAPI_LINUX_PSCI_H */
> --
> 2.44.0
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification
2024-09-26 8:55 ` [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification Francesco Lavra
@ 2024-09-26 9:59 ` David Woodhouse
0 siblings, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2024-09-26 9:59 UTC (permalink / raw)
To: Francesco Lavra, Paolo Bonzini, Jonathan Corbet, Marc Zyngier,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Pavel Machek, Len Brown, Shuah Khan, kvm,
linux-doc, linux-kernel, linux-arm-kernel, kvmarm, linux-pm,
linux-kselftest
[-- Attachment #1: Type: text/plain, Size: 843 bytes --]
On Thu, 2024-09-26 at 10:55 +0200, Francesco Lavra wrote:
> On Tue, 2024-09-24 at 17:05 +0100, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@amazon.co.uk>
> >
> > The v1.3 PSCI spec (https://developer.arm.com/documentation/den0022)
> > adds
> > SYSTEM_OFF2, CLEAN_INV_MEMREGION and CLEAN_INV_MEMREGION_ATTRIBUTES
> > functions. Add definitions for them and their parameters, along with
> > the
> > new TIMEOUT, RATE_LIMITED and BUSY error values.
>
> The CLEAN_INV_MEMREGION and CLEAN_INV_MEMREGION_ATTRIBUTES
> functions were added in the alpha release of the spec but have been
> dropped in the beta release, and are not included in the final spec. So
> IMO the uapi header file should not contain these definitions.
> The same goes for the TIMEOUT, RATE_LIMITED and BUSY error values.
Thanks. I'll drop those.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification
2024-09-26 9:56 ` Miguel Luis
@ 2024-09-26 16:30 ` David Woodhouse
2024-09-27 12:43 ` Miguel Luis
0 siblings, 1 reply; 16+ messages in thread
From: David Woodhouse @ 2024-09-26 16:30 UTC (permalink / raw)
To: Miguel Luis, Souvik Chakravarty
Cc: Paolo Bonzini, Jonathan Corbet, Marc Zyngier, Oliver Upton,
James Morse, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Lorenzo Pieralisi, Rafael J. Wysocki,
Pavel Machek, Len Brown, Shuah Khan, kvm@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
linux-pm@vger.kernel.org, linux-kselftest@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1851 bytes --]
On Thu, 2024-09-26 at 09:56 +0000, Miguel Luis wrote:
>
> > +/* PSCI v1.3 hibernate type for SYSTEM_OFF2 */
> > +#define PSCI_1_3_HIBERNATE_TYPE_OFF 0
>
> Should it be 1 as hibernate type?
It is in discovery, as BIT(PSCI_1_3_HIBERNATE_TYPE_OFF) == 1<<0 == 1.
But using a bitmask was only supposed to be for the discovery with
PSCI_FEATURES, as that has to advertise all the available hibernation
types.
The actual SYSTEM_OFF2 call was supposed to just take the numeric value
as an argument, since obviously *that* one isn't a bitmask.
Except... I see that now the spec has finally been updated, it seems to
say that 0x1 is the value to pass to the SYSTEM_OFF2 call for
HIBERNATE_OFF, not 0x0. Which doesn't seem to make much sense, and I
don't recall it being what we discussed. Souvik, what happened there?
My understanding was that for each supported hibernation type #n, for
which HIBERERNATE_OFF is zero), the PSCI_FEATURES query would include
the bit (1<<n) to indicate that it is supported, and then the actual
SYSTEM_OFF2 call parameter would be (n) itself, precisely as
implemented here.
But the spec now seems to say that HIBERNATE_OFF is advertised as
(1<<0) in PSCI_FEATURES, but invoked with the value (1).
Is it too late to fix?
If it isn't just a thinko, what is the intent in the current spec?
If we have new hibernate types such that
#define PSCI_1_3_HIBERNATE_TYPE_OFF 0
#define PSCI_1_3_HIBERNATE_TYPE_FOO 1
#define PSCI_1_3_HIBERNATE_TYPE_BAR 2
It seems obvious that the PSCI_FEATURES response will contain (1<<0),
(1<<1) and (1<<2) for them respectively, but what is supposed to be
passed to the actual SYSTEM_OFF2 call? Is it always just going to be
(PSCI_1_3_HIBERNATE_TYPE_xxx + 1)?
I think we should just fix §5.1.10 to report that 0x0 is HIBERNATE_OFF,
yes?
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification
2024-09-26 16:30 ` David Woodhouse
@ 2024-09-27 12:43 ` Miguel Luis
2024-09-27 13:20 ` David Woodhouse
2024-09-27 14:24 ` David Woodhouse
0 siblings, 2 replies; 16+ messages in thread
From: Miguel Luis @ 2024-09-27 12:43 UTC (permalink / raw)
To: David Woodhouse
Cc: Souvik Chakravarty, Paolo Bonzini, Jonathan Corbet, Marc Zyngier,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Pavel Machek, Len Brown, Shuah Khan,
kvm@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
linux-pm@vger.kernel.org, linux-kselftest@vger.kernel.org
Hi David,
> On 26 Sep 2024, at 16:30, David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Thu, 2024-09-26 at 09:56 +0000, Miguel Luis wrote:
>>
>>> +/* PSCI v1.3 hibernate type for SYSTEM_OFF2 */
>>> +#define PSCI_1_3_HIBERNATE_TYPE_OFF 0
>>
>> Should it be 1 as hibernate type?
>
> It is in discovery, as BIT(PSCI_1_3_HIBERNATE_TYPE_OFF) == 1<<0 == 1.
>
Now I see the definition for PSCI_1_3_HIBERNATE_TYPE_OFF was misleading for me
when BIT(PSCI_1_3_HIBERNATE_TYPE_OFF) works for both discovery and as argument
for SYSTEM_OFF2.
The common factor being the bit offset in the bitmap for SYSTEM_OFF2 discovery
and argument to call SYSTEM_OFF2 as well. Would it be clearer something like:
#define PSCI_1_3_HIBERNATE_TYPE_OFF BIT(0)
Assuming future definitions would keep the same common factor can be helpful, however
please let me know whether I am missing something.
Thanks,
Miguel
> But using a bitmask was only supposed to be for the discovery with
> PSCI_FEATURES, as that has to advertise all the available hibernation
> types.
>
> The actual SYSTEM_OFF2 call was supposed to just take the numeric value
> as an argument, since obviously *that* one isn't a bitmask.
>
> Except... I see that now the spec has finally been updated, it seems to
> say that 0x1 is the value to pass to the SYSTEM_OFF2 call for
> HIBERNATE_OFF, not 0x0. Which doesn't seem to make much sense, and I
> don't recall it being what we discussed. Souvik, what happened there?
>
> My understanding was that for each supported hibernation type #n, for
> which HIBERERNATE_OFF is zero), the PSCI_FEATURES query would include
> the bit (1<<n) to indicate that it is supported, and then the actual
> SYSTEM_OFF2 call parameter would be (n) itself, precisely as
> implemented here.
>
> But the spec now seems to say that HIBERNATE_OFF is advertised as
> (1<<0) in PSCI_FEATURES, but invoked with the value (1).
>
> Is it too late to fix?
>
> If it isn't just a thinko, what is the intent in the current spec?
>
> If we have new hibernate types such that
>
> #define PSCI_1_3_HIBERNATE_TYPE_OFF 0
> #define PSCI_1_3_HIBERNATE_TYPE_FOO 1
> #define PSCI_1_3_HIBERNATE_TYPE_BAR 2
>
> It seems obvious that the PSCI_FEATURES response will contain (1<<0),
> (1<<1) and (1<<2) for them respectively, but what is supposed to be
> passed to the actual SYSTEM_OFF2 call? Is it always just going to be
> (PSCI_1_3_HIBERNATE_TYPE_xxx + 1)?
>
> I think we should just fix §5.1.10 to report that 0x0 is HIBERNATE_OFF,
> yes?
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification
2024-09-27 12:43 ` Miguel Luis
@ 2024-09-27 13:20 ` David Woodhouse
2024-09-27 14:24 ` David Woodhouse
1 sibling, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2024-09-27 13:20 UTC (permalink / raw)
To: Miguel Luis
Cc: Souvik Chakravarty, Paolo Bonzini, Jonathan Corbet, Marc Zyngier,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Pavel Machek, Len Brown, Shuah Khan,
kvm@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
linux-pm@vger.kernel.org, linux-kselftest@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 2511 bytes --]
On Fri, 2024-09-27 at 12:43 +0000, Miguel Luis wrote:
> Hi David,
>
> > On 26 Sep 2024, at 16:30, David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > On Thu, 2024-09-26 at 09:56 +0000, Miguel Luis wrote:
> > >
> > > > +/* PSCI v1.3 hibernate type for SYSTEM_OFF2 */
> > > > +#define PSCI_1_3_HIBERNATE_TYPE_OFF 0
> > >
> > > Should it be 1 as hibernate type?
> >
> > It is in discovery, as BIT(PSCI_1_3_HIBERNATE_TYPE_OFF) == 1<<0 == 1.
> >
>
> Now I see the definition for PSCI_1_3_HIBERNATE_TYPE_OFF was misleading for me
> when BIT(PSCI_1_3_HIBERNATE_TYPE_OFF) works for both discovery and as argument
> for SYSTEM_OFF2.
That *wasn't* the intent, as I understood it.
An early version of the spec just returned PSCI_1_3_HIBERNATE_TYPE_OFF
(zero) for discovery and also used it as the argument for SYSTEM_OFF2.
Obviously that doesn't allow for supporting any other types (at least,
not unless an implementation had to support *all* types up to the one
it advertises). So for *discovery* it was changed to a bitmap,
returning BIT(PSCI_1_3_HIBERNATE_TYPE_OFF), and explicitly documented
as "this field is a bitmap".
We discussed that, and settled on the changes, and I had completely
failed to spot that the beta spec then also quietly changed the actual
argument to SYSTEM_OFF2 from 0x0 to 0x1 for HIBERNATE_OFF too. I do not
recall that change ever being discussed, so thanks for catching it!
> The common factor being the bit offset in the bitmap for SYSTEM_OFF2 discovery
> and argument to call SYSTEM_OFF2 as well. Would it be clearer something like:
>
> #define PSCI_1_3_HIBERNATE_TYPE_OFF BIT(0)
>
> Assuming future definitions would keep the same common factor can be helpful, however
> please let me know whether I am missing something.
Right. If the spec is going to stay as it is, then just defining it as
BIT(0) probably makes sense.
In practice, as I said, it doesn't make a lot of difference because the
KVM code handling SYSTEM_OFF2 doesn't even look at the argument. It
just sets a flag to let userspace know it was a SYSTEM_OFF2 call
instead of SYSTEM_OFF. Precisely the same way that SYSTEM_RESET2 is
handled.
If userspace wants to know the precise argument, I think it's supposed
to go digging in the registers for itself? And the only implementation
in existence that I know of doesn't bother; it treats *all* SYSTEM_OFF2
calls just the same, regardless of the argument. Since there is only
one possibility anyway.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification
2024-09-27 12:43 ` Miguel Luis
2024-09-27 13:20 ` David Woodhouse
@ 2024-09-27 14:24 ` David Woodhouse
1 sibling, 0 replies; 16+ messages in thread
From: David Woodhouse @ 2024-09-27 14:24 UTC (permalink / raw)
To: Miguel Luis
Cc: Souvik Chakravarty, Paolo Bonzini, Jonathan Corbet, Marc Zyngier,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Pavel Machek, Len Brown, Shuah Khan,
kvm@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
linux-pm@vger.kernel.org, linux-kselftest@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]
On Fri, 2024-09-27 at 12:43 +0000, Miguel Luis wrote:
>
> The common factor being the bit offset in the bitmap for SYSTEM_OFF2 discovery
> and argument to call SYSTEM_OFF2 as well. Would it be clearer something like:
>
> #define PSCI_1_3_HIBERNATE_TYPE_OFF BIT(0)
I've updated the tree at
https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/psci-hibernate
to do it that way.
As I did so, I realised that KVM *does* care about the argument to
SYSTEM_OFF2. This is a straight copy of the SYSTEM_RESET2 handling.
Although it doesn't pass the argument up to userspace (presumably
userspace is expected to look at the registers if it cares), it *does*
check it's within the range of permitted values and return
PSCI_RET_INVALID_PARAMS if not.
I've changed that check to:
arg = smccc_get_arg1(vcpu);
/*
* Permit zero to mean HIBERNATE_OFF as well as the bitmap
* form which was introduced in PSCI v1.3 beta.
*/
if (arg && arg != PSCI_1_3_HIBERNATE_TYPE_OFF) {
val = PSCI_RET_INVALID_PARAMS;
break;
}
kvm_psci_system_off2(vcpu);
On the guest side, I've changed the invocation to:
static int psci_sys_hibernate(struct sys_off_data *data)
{
/*
* Zero is an acceptable alternative to PSCI_1_3_HIBERNATE_TYPE_OFF
* and is supported by hypervisors implementing an earlier version
* of the pSCI v1.3 spec.
*/
if (system_entering_hibernation())
invoke_psci_fn(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2),
0 /*PSCI_1_3_HIBERNATE_TYPE_OFF*/, 0, 0);
return NOTIFY_DONE;
}
I'm going to do some careful interop testing with existing and new
hypervisors before reposting this version.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-09-27 14:24 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-24 16:05 [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification David Woodhouse
2024-09-24 16:05 ` [PATCH v4 2/6] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation David Woodhouse
2024-09-26 9:14 ` Francesco Lavra
2024-09-24 16:05 ` [PATCH v4 3/6] KVM: arm64: Add support for PSCI v1.2 and v1.3 David Woodhouse
2024-09-24 16:05 ` [PATCH v4 4/6] KVM: selftests: Add test for PSCI SYSTEM_OFF2 David Woodhouse
2024-09-24 16:05 ` [PATCH v4 5/6] KVM: arm64: nvhe: Pass through PSCI v1.3 SYSTEM_OFF2 call David Woodhouse
2024-09-24 16:05 ` [PATCH v4 6/6] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate David Woodhouse
2024-09-26 9:18 ` Francesco Lavra
2024-09-24 16:13 ` [PATCH v4 0/6] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation David Woodhouse
2024-09-26 8:55 ` [PATCH v4 1/6] firmware/psci: Add definitions for PSCI v1.3 specification Francesco Lavra
2024-09-26 9:59 ` David Woodhouse
2024-09-26 9:56 ` Miguel Luis
2024-09-26 16:30 ` David Woodhouse
2024-09-27 12:43 ` Miguel Luis
2024-09-27 13:20 ` David Woodhouse
2024-09-27 14:24 ` David Woodhouse
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).