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.
Version 5 drops some of the changes which didn't make it to the final v1.3 spec, and cleans up a couple of places which still referred to it as 'alpha' or 'beta'. It also temporarily drops the guest-side patch to invoke SYSTEM_OFF2 for hibernation, pending confirmation that the final PSCI v1.3 spec just has a typo where it changed to saying that 0x1 should be passed to mean HIBERNATE_OFF, even though it's advertised as bit 0. That can be sent under separate cover, and perhaps should have been anyway. The change in question doesn't matter for any of the KVM patches, because we just treat SYSTEM_OFF2 like the existing SYSTEM_RESET2, setting a flag to indicate that it was a SYSTEM_OFF2 call, but not actually caring about the argument; that's for userspace to worry about.
Version 6 is updated to revision F.b of the specification, allowing both 0x0 and 0x1 to indicate HIBERNATE_OFF in the SYSTEM_OFF2 call, as well as disallowing anything but zero in the second argument. The test is expanded to cover those variants, and to require PSCI v1.3 instead of skipping on older kernels. The final guest-side patch in the series is reinstated, using zero as the argument for compatibility with existing hypervisors in the field as permitted by revision F.b. allows for zero as well as 0x1 for HIBERNATE_OFF to accommodate the change in the final version of the spec, and expands the test to cover both forms as well as checking that a non-zero second argument is correctly rejected.
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 | 50 ++++++++++++- drivers/firmware/psci/psci.c | 42 +++++++++++ include/kvm/arm_psci.h | 4 +- include/uapi/linux/psci.h | 5 ++ kernel/power/hibernate.c | 5 +- tools/testing/selftests/kvm/aarch64/psci_test.c | 93 +++++++++++++++++++++++++ 10 files changed, 217 insertions(+), 3 deletions(-)
From: David Woodhouse dwmw@amazon.co.uk
The v1.3 PSCI spec (https://developer.arm.com/documentation/den0022) adds the SYSTEM_OFF2 function. Add definitions for it and its hibernation type parameter.
Signed-off-by: David Woodhouse dwmw@amazon.co.uk --- include/uapi/linux/psci.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h index 42a40ad3fb62..81759ff385e6 100644 --- a/include/uapi/linux/psci.h +++ b/include/uapi/linux/psci.h @@ -59,6 +59,7 @@ #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_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 +69,7 @@
#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)
/* PSCI v0.2 power state encoding for CPU_SUSPEND function */ #define PSCI_0_2_POWER_STATE_ID_MASK 0xffff @@ -100,6 +102,9 @@ #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_OFF_TYPE_HIBERNATE_OFF BIT(0) + /* PSCI version decoding (independent of PSCI version) */ #define PSCI_VERSION_MAJOR_SHIFT 16 #define PSCI_VERSION_MINOR_MASK \
On 19 Oct 2024, at 17:15, 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 the SYSTEM_OFF2 function. Add definitions for it and its hibernation type parameter.
Signed-off-by: David Woodhouse dwmw@amazon.co.uk
include/uapi/linux/psci.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h index 42a40ad3fb62..81759ff385e6 100644 --- a/include/uapi/linux/psci.h +++ b/include/uapi/linux/psci.h @@ -59,6 +59,7 @@ #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_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 +69,7 @@
#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)
/* PSCI v0.2 power state encoding for CPU_SUSPEND function */ #define PSCI_0_2_POWER_STATE_ID_MASK 0xffff @@ -100,6 +102,9 @@ #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_OFF_TYPE_HIBERNATE_OFF BIT(0)
Reviewed-by: Miguel Luis miguel.luis@oracle.com
/* PSCI version decoding (independent of PSCI version) */ #define PSCI_VERSION_MAJOR_SHIFT 16
#define PSCI_VERSION_MINOR_MASK \
2.44.0
From: David Woodhouse dwmw@amazon.co.uk
The PSCI v1.3 specification 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).
This feature is safe to enable unconditionally (in a subsequent commit) because it is exposed to userspace through the existing KVM_SYSTEM_EVENT_SHUTDOWN event, just with an additional flag which userspace can use to know that the instance intended hibernation instead of a plain power-off.
As with SYSTEM_RESET2, there is only one type available (in this case HIBERNATE_OFF), and it is not explicitly reported to userspace through the event; userspace can get it from the registers if it cares).
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 | 44 +++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index e32471977d0a..1ec076d806e6 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -6855,6 +6855,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.
@@ -6888,6 +6892,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..df834f2e928e 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 = PSCI_1_3_OFF_TYPE_HIBERNATE_OFF; + break; } break; case PSCI_1_0_FN_SYSTEM_SUSPEND: @@ -392,6 +403,39 @@ 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); + /* + * PSCI v1.3 issue F.b requires that zero be accepted to mean + * HIBERNATE_OFF (in line with pre-publication versions of the + * spec, and thus some actual implementations in the wild). + * The second argument must be zero. + */ + if ((arg && arg != PSCI_1_3_OFF_TYPE_HIBERNATE_OFF) || + smccc_get_arg2(vcpu) != 0) { + 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); }
Hi David,
On 19 Oct 2024, at 17:15, David Woodhouse dwmw2@infradead.org wrote:
From: David Woodhouse dwmw@amazon.co.uk
The PSCI v1.3 specification 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).
This feature is safe to enable unconditionally (in a subsequent commit) because it is exposed to userspace through the existing KVM_SYSTEM_EVENT_SHUTDOWN event, just with an additional flag which userspace can use to know that the instance intended hibernation instead of a plain power-off.
As with SYSTEM_RESET2, there is only one type available (in this case HIBERNATE_OFF), and it is not explicitly reported to userspace through the event; userspace can get it from the registers if it cares).
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 | 44 +++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index e32471977d0a..1ec076d806e6 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -6855,6 +6855,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.
@@ -6888,6 +6892,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).
I don’t think that ‘0x0’ adds something to what’s already explained before, IMO.
::
/* 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..df834f2e928e 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 = PSCI_1_3_OFF_TYPE_HIBERNATE_OFF;
- break;
} break; case PSCI_1_0_FN_SYSTEM_SUSPEND: @@ -392,6 +403,39 @@ 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);
- /*
- PSCI v1.3 issue F.b requires that zero be accepted to mean
- HIBERNATE_OFF (in line with pre-publication versions of the
- spec, and thus some actual implementations in the wild).
- The second argument must be zero.
- */
- if ((arg && arg != PSCI_1_3_OFF_TYPE_HIBERNATE_OFF) ||
- smccc_get_arg2(vcpu) != 0) {
- 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;
Other than that it looks good to me:
Reviewed-by: Miguel Luis miguel.luis@oracle.com
Thanks, Miguel
default: return kvm_psci_0_2_call(vcpu); } -- 2.44.0
From: David Woodhouse dwmw@amazon.co.uk
As with PSCI v1.1 in commit 512865d83fd9 ("KVM: arm64: Bump guest PSCI version to 1.1"), expose v1.3 to the guest by default. The SYSTEM_OFF2 call which is exposed by doing so is compatible for userspace because it's just a new flag in the event that KVM raises, in precisely the same way that SYSTEM_RESET2 was compatible when v1.1 was enabled by default.
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 df834f2e928e..6c24a9252fa3 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); @@ -493,6 +493,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) {
On 19 Oct 2024, at 17:15, David Woodhouse dwmw2@infradead.org wrote:
From: David Woodhouse dwmw@amazon.co.uk
As with PSCI v1.1 in commit 512865d83fd9 ("KVM: arm64: Bump guest PSCI version to 1.1"), expose v1.3 to the guest by default. The SYSTEM_OFF2 call which is exposed by doing so is compatible for userspace because it's just a new flag in the event that KVM raises, in precisely the same way that SYSTEM_RESET2 was compatible when v1.1 was enabled by default.
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 df834f2e928e..6c24a9252fa3 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); @@ -493,6 +493,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
Reviewed-by: Miguel Luis miguel.luis@oracle.com
static inline int kvm_psci_version(struct kvm_vcpu *vcpu) { -- 2.44.0
From: David Woodhouse dwmw@amazon.co.uk
Signed-off-by: David Woodhouse dwmw@amazon.co.uk --- .../testing/selftests/kvm/aarch64/psci_test.c | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+)
diff --git a/tools/testing/selftests/kvm/aarch64/psci_test.c b/tools/testing/selftests/kvm/aarch64/psci_test.c index 61731a950def..1eabbef99dbc 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, uint64_t cookie) +{ + struct arm_smccc_res res; + + smccc_hvc(PSCI_1_3_FN64_SYSTEM_OFF2, type, cookie, 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,95 @@ 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) & + PSCI_1_3_OFF_TYPE_HIBERNATE_OFF); + GUEST_ASSERT(psci_features(PSCI_1_3_FN64_SYSTEM_OFF2) & + PSCI_1_3_OFF_TYPE_HIBERNATE_OFF); + + /* With non-zero 'cookie' field, it should fail */ + ret = psci_system_off2(PSCI_1_3_OFF_TYPE_HIBERNATE_OFF, 1); + GUEST_ASSERT(ret == PSCI_RET_INVALID_PARAMS); + + /* + * This would normally never return, so KVM sets the return value + * to PSCI_RET_INTERNAL_FAILURE. The test case *does* return, so + * that it can test both values for HIBERNATE_OFF. + */ + ret = psci_system_off2(PSCI_1_3_OFF_TYPE_HIBERNATE_OFF, 0); + GUEST_ASSERT(ret == PSCI_RET_INTERNAL_FAILURE); + + /* + * Revision F.b of the PSCI v1.3 specification documents zero as an + * alias for HIBERNATE_OFF, since that's the value used in earlier + * revisions of the spec and some implementations in the field. + */ + ret = psci_system_off2(0, 1); + GUEST_ASSERT(ret == PSCI_RET_INVALID_PARAMS); + + ret = psci_system_off2(0, 0); + /* Unless this test is extended, the guest never gets here. */ + GUEST_ASSERT(ret == PSCI_RET_INTERNAL_FAILURE); + + GUEST_DONE(); +} + +static void host_test_system_off2(void) +{ + struct kvm_vcpu *source, *target; + struct kvm_mp_state mps; + uint64_t psci_version = 0; + int nr_shutdowns = 0; + struct kvm_run *run; + struct ucall uc; + + 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(1, 3), + "Unexpected PSCI version %lu.%lu", + PSCI_VERSION_MAJOR(psci_version), + PSCI_VERSION_MINOR(psci_version)); + + vcpu_power_off(target); + run = source->run; + + enter_guest(source); + while (run->exit_reason == 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); + + nr_shutdowns++; + + /* Restart the vCPU */ + mps.mp_state = KVM_MP_STATE_RUNNABLE; + vcpu_mp_state_set(source, &mps); + + enter_guest(source); + } + + TEST_ASSERT(get_ucall(source, &uc) == UCALL_DONE, "Guest did not exit cleanly"); + TEST_ASSERT(nr_shutdowns == 2, "Two shutdown events were expected, but saw %d", nr_shutdowns); +} + 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; }
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);
On 19 Oct 2024, at 17:15, David Woodhouse dwmw2@infradead.org wrote:
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:
Reviewed-by: Miguel Luis miguel.luis@oracle.com
return psci_forward(host_ctxt); case PSCI_1_0_FN64_SYSTEM_SUSPEND: return psci_system_suspend(func_id, host_ctxt); -- 2.44.0
From: David Woodhouse dwmw@amazon.co.uk
The PSCI v1.3 specification 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 | 42 ++++++++++++++++++++++++++++++++++++ kernel/power/hibernate.c | 5 ++++- 2 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c index 2328ca58bba6..8809455a61a6 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,33 @@ 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) +{ + /* + * Zero is an acceptable alternative to PSCI_1_3_OFF_TYPE_HIBERNATE_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_OFF_TYPE_HIBERNATE_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 +392,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 +554,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 & PSCI_1_3_OFF_TYPE_HIBERNATE_OFF) + psci_system_off2_hibernate_supported = true; +} + static void __init psci_init_system_suspend(void) { int ret; @@ -655,6 +696,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();
Hi David,
On 19 Oct 2024, at 17:15, David Woodhouse dwmw2@infradead.org wrote:
From: David Woodhouse dwmw@amazon.co.uk
The PSCI v1.3 specification 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 | 42 ++++++++++++++++++++++++++++++++++++ kernel/power/hibernate.c | 5 ++++- 2 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c index 2328ca58bba6..8809455a61a6 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,33 @@ 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) +{
- /*
- Zero is an acceptable alternative to PSCI_1_3_OFF_TYPE_HIBERNATE_OFF
- and is supported by hypervisors implementing an earlier version
- of the pSCI v1.3 spec.
s/pSCI/PSCI
- */
- if (system_entering_hibernation())
- invoke_psci_fn(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2),
0 /*PSCI_1_3_OFF_TYPE_HIBERNATE_OFF*/, 0, 0);
Perhaps spec. F.b. could be accommodated by first invoking SYSTEM_OFF2 with PSCI_1_3_OFF_TYPE_HIBERNATE_OFF and checking its return value in case of a fallback to an invocation with 0x0 ?
Thanks. Miguel
- 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 +392,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 +554,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 & PSCI_1_3_OFF_TYPE_HIBERNATE_OFF)
- psci_system_off2_hibernate_supported = true;
+}
static void __init psci_init_system_suspend(void) { int ret; @@ -655,6 +696,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
On 24 October 2024 14:54:41 CEST, Miguel Luis miguel.luis@oracle.com wrote:
Perhaps spec. F.b. could be accommodated by first invoking SYSTEM_OFF2 with PSCI_1_3_OFF_TYPE_HIBERNATE_OFF and checking its return value in case of a fallback to an invocation with 0x0 ?
I wasn't aware there was any point. Are there any hypervisors which actually implemented it that way? Amazon Linux and Ubuntu guests already just use zero.
We could add it later if such a hypervisor (now in violation of F.b) turns up, I suppose?
Hi,
On Thu, Oct 24, 2024 at 03:48:26PM +0200, David Woodhouse wrote:
On 24 October 2024 14:54:41 CEST, Miguel Luis miguel.luis@oracle.com wrote:
Perhaps spec. F.b. could be accommodated by first invoking SYSTEM_OFF2 with PSCI_1_3_OFF_TYPE_HIBERNATE_OFF and checking its return value in case of a fallback to an invocation with 0x0 ?
This already complies with F.b.
The PSCI implementation is required to accept either 0 or 1 for HIBERNATE_OFF. Using 0 seems like a good choice for compatibility since ...
I wasn't aware there was any point. Are there any hypervisors which actually implemented it that way? Amazon Linux and Ubuntu guests already just use zero.
We could add it later if such a hypervisor (now in violation of F.b) turns up, I suppose?
IIUC, you're really wanting to 0x0 because there are hypervisors out there that violate the final spec and *only* accept this value.
That's perfectly fine, but it'd help avoid confusion if the supporting comment was a bit more direct:
/* * If no hibernate type is specified SYSTEM_OFF2 defaults to * selecting HIBERNATE_OFF. * * There are hypervisors in the wild that violate the spec and * reject calls that explicitly provide a hibernate type. For * compatibility with these nonstandard implementations, pass 0 * as the type. */ if (system_entering_hibernation()) invoke_psci_fn(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2), 0 , 0, 0);
Thoughts?
On 24 October 2024 17:44:49 CEST, Oliver Upton oliver.upton@linux.dev wrote:
Hi,
On Thu, Oct 24, 2024 at 03:48:26PM +0200, David Woodhouse wrote:
On 24 October 2024 14:54:41 CEST, Miguel Luis miguel.luis@oracle.com wrote:
Perhaps spec. F.b. could be accommodated by first invoking SYSTEM_OFF2 with PSCI_1_3_OFF_TYPE_HIBERNATE_OFF and checking its return value in case of a fallback to an invocation with 0x0 ?
This already complies with F.b.
The PSCI implementation is required to accept either 0 or 1 for HIBERNATE_OFF. Using 0 seems like a good choice for compatibility since ...
I wasn't aware there was any point. Are there any hypervisors which actually implemented it that way? Amazon Linux and Ubuntu guests already just use zero.
We could add it later if such a hypervisor (now in violation of F.b) turns up, I suppose?
IIUC, you're really wanting to 0x0 because there are hypervisors out there that violate the final spec and *only* accept this value.
That's perfectly fine, but it'd help avoid confusion if the supporting comment was a bit more direct:
/* * If no hibernate type is specified SYSTEM_OFF2 defaults to * selecting HIBERNATE_OFF. * * There are hypervisors in the wild that violate the spec and * reject calls that explicitly provide a hibernate type. For * compatibility with these nonstandard implementations, pass 0 * as the type. */ if (system_entering_hibernation()) invoke_psci_fn(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2), 0 , 0, 0);
By the time this makes it into released versions of the guest Linux kernel, that comment won't be true any more.
On Thu, Oct 24, 2024 at 05:56:09PM +0200, David Woodhouse wrote:
On 24 October 2024 17:44:49 CEST, Oliver Upton oliver.upton@linux.dev wrote:
IIUC, you're really wanting to 0x0 because there are hypervisors out there that violate the final spec and *only* accept this value.
That's perfectly fine, but it'd help avoid confusion if the supporting comment was a bit more direct:
/* * If no hibernate type is specified SYSTEM_OFF2 defaults to * selecting HIBERNATE_OFF. * * There are hypervisors in the wild that violate the spec and * reject calls that explicitly provide a hibernate type. For * compatibility with these nonstandard implementations, pass 0 * as the type. */ if (system_entering_hibernation()) invoke_psci_fn(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2), 0 , 0, 0);
By the time this makes it into released versions of the guest Linux kernel, that comment won't be true any more.
Then does it even matter? What is the problem you're trying to solve with using a particular value for the hibernate type?
Either the goal of this is to make the PSCI client code compatible with your hypervisor today (and any other implementation based on 'F ALP1') or we don't care and go with whatever value we want.
Even if the comment eventually becomes stale, there is a ton of value in documenting the exact implementation decision being made.
On 24 October 2024 21:57:38 CEST, Oliver Upton oliver.upton@linux.dev wrote:
On Thu, Oct 24, 2024 at 05:56:09PM +0200, David Woodhouse wrote:
On 24 October 2024 17:44:49 CEST, Oliver Upton oliver.upton@linux.dev wrote:
IIUC, you're really wanting to 0x0 because there are hypervisors out there that violate the final spec and *only* accept this value.
That's perfectly fine, but it'd help avoid confusion if the supporting comment was a bit more direct:
/* * If no hibernate type is specified SYSTEM_OFF2 defaults to * selecting HIBERNATE_OFF. * * There are hypervisors in the wild that violate the spec and * reject calls that explicitly provide a hibernate type. For * compatibility with these nonstandard implementations, pass 0 * as the type. */ if (system_entering_hibernation()) invoke_psci_fn(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2), 0 , 0, 0);
By the time this makes it into released versions of the guest Linux kernel, that comment won't be true any more.
Then does it even matter? What is the problem you're trying to solve with using a particular value for the hibernate type?
Either the goal of this is to make the PSCI client code compatible with your hypervisor today (and any other implementation based on 'F ALP1') or we don't care and go with whatever value we want.
Even if the comment eventually becomes stale, there is a ton of value in documenting the exact implementation decision being made.
Eventually it won't matter and we can go with whatever value we want. But yes, the goal is to be compatible with the hypervisor *today* until it catches up the changes to the final versions of the spec. I didn't spend much time overthinking the comment. What was it....
/* * Zero is an acceptable alternative to PSCI_1_3_OFF_TYPE_HIBERNATE_OFF * and is supported by hypervisors implementing an earlier version * of the pSCI v1.3 spec. */
That seems to cover it just fine, I think.
On Fri, Oct 25, 2024 at 08:13:03AM +0200, David Woodhouse wrote:
On 24 October 2024 21:57:38 CEST, Oliver Upton oliver.upton@linux.dev wrote:
On Thu, Oct 24, 2024 at 05:56:09PM +0200, David Woodhouse wrote:
On 24 October 2024 17:44:49 CEST, Oliver Upton oliver.upton@linux.dev wrote:
IIUC, you're really wanting to 0x0 because there are hypervisors out there that violate the final spec and *only* accept this value.
That's perfectly fine, but it'd help avoid confusion if the supporting comment was a bit more direct:
/* * If no hibernate type is specified SYSTEM_OFF2 defaults to * selecting HIBERNATE_OFF. * * There are hypervisors in the wild that violate the spec and * reject calls that explicitly provide a hibernate type. For * compatibility with these nonstandard implementations, pass 0 * as the type. */ if (system_entering_hibernation()) invoke_psci_fn(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2), 0 , 0, 0);
By the time this makes it into released versions of the guest Linux kernel, that comment won't be true any more.
Then does it even matter? What is the problem you're trying to solve with using a particular value for the hibernate type?
Either the goal of this is to make the PSCI client code compatible with your hypervisor today (and any other implementation based on 'F ALP1') or we don't care and go with whatever value we want.
Even if the comment eventually becomes stale, there is a ton of value in documenting the exact implementation decision being made.
Eventually it won't matter and we can go with whatever value we want. But yes, the goal is to be compatible with the hypervisor *today* until it catches up the changes to the final versions of the spec. I didn't spend much time overthinking the comment. What was it....
/* * Zero is an acceptable alternative to PSCI_1_3_OFF_TYPE_HIBERNATE_OFF * and is supported by hypervisors implementing an earlier version * of the pSCI v1.3 spec. */
That seems to cover it just fine, I think.
No. You're leaving the work for the reader to:
(1) Figure out what you're talking about (2) Go dig up an "earlier version" of the spec (3) Realise that it means certain hypervisors only take 0x0 as an argument
If you speak *directly* about the problem you're trying to address then reviewers are less likely to get hung up on what you're trying to do.
I'm genuinely at a loss for why you would want to present this as an "acceptable alterantive" rather than a functional requirement for this driver to run on your hypervisor.
On Fri, 2024-10-25 at 13:40 -0700, Oliver Upton wrote:
No. You're leaving the work for the reader to:
(1) Figure out what you're talking about (2) Go dig up an "earlier version" of the spec (3) Realise that it means certain hypervisors only take 0x0 as an argument
No. There's no need to dig up an 'earlier version' of the spec. The current F.b release says, "if the value of this parameter is 0x0, the implementation defaults to selecting HIBERNATE_OFF". That's what makes it an acceptable alternative.
If you speak *directly* about the problem you're trying to address then reviewers are less likely to get hung up on what you're trying to do.
The "problem" this comment addresses is a reader who looks at this code and wonders why it uses zero instead of HIBERNATE_OFF.
Firstly, that reader needs to spot the text, in the *current* version of the spec as cited above, which makes it clear that it's a perfectly acceptable alternative.
Secondly, that reader needs to know why we chose zero over HIBERNATE_OFF, which is also explained fairly succinctly: because it's compatible with hypervisors implementing an earlier version of the spec.
I'm genuinely at a loss for why you would want to present this as an "acceptable alterantive" rather than a functional requirement for this driver to run on your hypervisor.
Because "my" hypervisor is a live product which actually gets security fixes and updates. If it wasn't for the silly season of "thou shalt not ship *anything* except security fixes and stuff that's going to be announced at a big conference at the end of the month", the change to make it accept the new value of HIBERNATE_OFF would already have been deployed.
And *even* with the silly season delaying non-critical hypervisor changes, your suggested wording will *still* probably not be true by the time the comment is included in an actual release of the Linux kernel.
But honestly, it isn't a hill I'm prepared to die on. If you insist on changing the comment to your 'There are hypervisors in the wile that violate the spec...' version, by all means go ahead and do so. I'll follow up with a patch to correct it in a few weeks when it becomes obsolete.
On Sat, Oct 19, 2024 at 06:15:47PM +0100, David Woodhouse wrote:
From: David Woodhouse dwmw@amazon.co.uk
The PSCI v1.3 specification 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
Acked-by: Catalin Marinas catalin.marinas@arm.com
On Sat, Oct 19, 2024 at 06:15:47PM +0100, David Woodhouse wrote:
[...]
+#ifdef CONFIG_HIBERNATION +static int psci_sys_hibernate(struct sys_off_data *data) +{
- /*
* Zero is an acceptable alternative to PSCI_1_3_OFF_TYPE_HIBERNATE_OFF* and is supported by hypervisors implementing an earlier version* of the pSCI v1.3 spec.*/
It is obvious but with this patch applied a host kernel would start executing SYSTEM_OFF2 too if supported in firmware to hibernate, it is not a hypervisor only code path.
Related to that: is it now always safe to override
commit 60c0d45a7f7a ("efi/arm64: use UEFI for system reset and poweroff")
for hibernation ? It is not very clear to me why overriding PSCI for poweroff was the right thing to do - tried to follow that patch history but the question remains (it is related to UpdateCapsule() but I don't know how that applies to the hibernation use case).
As far as type == 0 is concerned:
I don't think the issue here is really PSCI 1.3 ALP1 vs PSCI 1.3 Issue F.b, by reading the PSCI 1.3 Issue F.b specs (note (e) page 96) it looks like there was a (superseded) PSCI 1.3 Issue F (september 2024 - superseded in October 2024 - just reading the specs timeline) that allowed an implementation to return INVALID_PARAMETERS if type == 0 - there should be no firwmare out there that followed it - it was short lived.
Lorenzo
- if (system_entering_hibernation())
invoke_psci_fn(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2),0 /*PSCI_1_3_OFF_TYPE_HIBERNATE_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 +392,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 +554,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 & PSCI_1_3_OFF_TYPE_HIBERNATE_OFF)
psci_system_off2_hibernate_supported = true;+}
static void __init psci_init_system_suspend(void) { int ret; @@ -655,6 +696,7 @@ static int __init psci_probe(void) psci_init_cpu_suspend(); psci_init_system_suspend(); psci_init_system_reset2();
psci_init_system_off2();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;}-- 2.44.0
[+Ard, Sami, for EFI]
On Thu, Oct 31, 2024 at 06:55:43PM +0100, Lorenzo Pieralisi wrote:
On Sat, Oct 19, 2024 at 06:15:47PM +0100, David Woodhouse wrote:
[...]
+#ifdef CONFIG_HIBERNATION +static int psci_sys_hibernate(struct sys_off_data *data) +{
- /*
* Zero is an acceptable alternative to PSCI_1_3_OFF_TYPE_HIBERNATE_OFF* and is supported by hypervisors implementing an earlier version* of the pSCI v1.3 spec.*/It is obvious but with this patch applied a host kernel would start executing SYSTEM_OFF2 too if supported in firmware to hibernate, it is not a hypervisor only code path.
Related to that: is it now always safe to override
commit 60c0d45a7f7a ("efi/arm64: use UEFI for system reset and poweroff")
for hibernation ? It is not very clear to me why overriding PSCI for poweroff was the right thing to do - tried to follow that patch history but the question remains (it is related to UpdateCapsule() but I don't know how that applies to the hibernation use case).
RFC: It is unclear to me what happens in current mainline if we try to hibernate with EFI runtime services enabled and a capsule update pending (we issue EFI ResetSystem(EFI_RESET_SHUTDOWN,..) which might not be compatible with the reset required by the pending capsule update request) what happens in this case I don't know but at least the choice is all contained in EFI firmware.
Then if in the same scenario now we are switching to PSCI SYSTEM_OFF2 for the hibernate reset I suspect that what happens to the in-flight capsule update requests strictly depends on what "reset" PSCI SYSTEM_OFF2 will end up doing ?
I think this is just a corner case and it is unlikely it has been ever tested (is it even possible ? Looking at EFI folks) - it would be good to clarify it at least to make sure we understand this code path.
Thanks, Lorenzo
On Fri, 1 Nov 2024 at 18:49, Lorenzo Pieralisi lpieralisi@kernel.org wrote:
[+Ard, Sami, for EFI]
On Thu, Oct 31, 2024 at 06:55:43PM +0100, Lorenzo Pieralisi wrote:
On Sat, Oct 19, 2024 at 06:15:47PM +0100, David Woodhouse wrote:
[...]
+#ifdef CONFIG_HIBERNATION +static int psci_sys_hibernate(struct sys_off_data *data) +{
- /*
- Zero is an acceptable alternative to PSCI_1_3_OFF_TYPE_HIBERNATE_OFF
- and is supported by hypervisors implementing an earlier version
- of the pSCI v1.3 spec.
- */
It is obvious but with this patch applied a host kernel would start executing SYSTEM_OFF2 too if supported in firmware to hibernate, it is not a hypervisor only code path.
Related to that: is it now always safe to override
commit 60c0d45a7f7a ("efi/arm64: use UEFI for system reset and poweroff")
for hibernation ? It is not very clear to me why overriding PSCI for poweroff was the right thing to do - tried to follow that patch history but the question remains (it is related to UpdateCapsule() but I don't know how that applies to the hibernation use case).
RFC: It is unclear to me what happens in current mainline if we try to hibernate with EFI runtime services enabled and a capsule update pending (we issue EFI ResetSystem(EFI_RESET_SHUTDOWN,..) which might not be compatible with the reset required by the pending capsule update request) what happens in this case I don't know but at least the choice is all contained in EFI firmware.
Then if in the same scenario now we are switching to PSCI SYSTEM_OFF2 for the hibernate reset I suspect that what happens to the in-flight capsule update requests strictly depends on what "reset" PSCI SYSTEM_OFF2 will end up doing ?
I think this is just a corner case and it is unlikely it has been ever tested (is it even possible ? Looking at EFI folks) - it would be good to clarify it at least to make sure we understand this code path.
I'm not aware of any OS that actually uses capsule update at runtime (both Windows and Linux queue up the capsule and call the UpdateCapsule() runtime service at boot time after a reboot).
So it is unlikely that this would break anything, and I'd actually be inclined to disable capsule update at runtime altogether.
I will also note that hibernation with EFI is flaky in general, given that EFI memory regions may move around
On Mon, Nov 04, 2024 at 02:54:12PM +0100, Ard Biesheuvel wrote:
On Fri, 1 Nov 2024 at 18:49, Lorenzo Pieralisi lpieralisi@kernel.org wrote:
[+Ard, Sami, for EFI]
On Thu, Oct 31, 2024 at 06:55:43PM +0100, Lorenzo Pieralisi wrote:
On Sat, Oct 19, 2024 at 06:15:47PM +0100, David Woodhouse wrote:
[...]
+#ifdef CONFIG_HIBERNATION +static int psci_sys_hibernate(struct sys_off_data *data) +{
- /*
- Zero is an acceptable alternative to PSCI_1_3_OFF_TYPE_HIBERNATE_OFF
- and is supported by hypervisors implementing an earlier version
- of the pSCI v1.3 spec.
- */
It is obvious but with this patch applied a host kernel would start executing SYSTEM_OFF2 too if supported in firmware to hibernate, it is not a hypervisor only code path.
Related to that: is it now always safe to override
commit 60c0d45a7f7a ("efi/arm64: use UEFI for system reset and poweroff")
for hibernation ? It is not very clear to me why overriding PSCI for poweroff was the right thing to do - tried to follow that patch history but the question remains (it is related to UpdateCapsule() but I don't know how that applies to the hibernation use case).
RFC: It is unclear to me what happens in current mainline if we try to hibernate with EFI runtime services enabled and a capsule update pending (we issue EFI ResetSystem(EFI_RESET_SHUTDOWN,..) which might not be compatible with the reset required by the pending capsule update request) what happens in this case I don't know but at least the choice is all contained in EFI firmware.
Then if in the same scenario now we are switching to PSCI SYSTEM_OFF2 for the hibernate reset I suspect that what happens to the in-flight capsule update requests strictly depends on what "reset" PSCI SYSTEM_OFF2 will end up doing ?
I think this is just a corner case and it is unlikely it has been ever tested (is it even possible ? Looking at EFI folks) - it would be good to clarify it at least to make sure we understand this code path.
I'm not aware of any OS that actually uses capsule update at runtime (both Windows and Linux queue up the capsule and call the UpdateCapsule() runtime service at boot time after a reboot).
So it is unlikely that this would break anything, and I'd actually be inclined to disable capsule update at runtime altogether.
I will also note that hibernation with EFI is flaky in general, given that EFI memory regions may move around
Thank you for chiming in, I think we are OK (I don't think this patch will create more issues than the ones that are already there for hibernate anyway) - the reasoning behind the change is in the commit logs.
Lorenzo
On Sat, 19 Oct 2024 18:15:41 +0100, David Woodhouse wrote:
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).
[...]
I grabbed the KVM portions of this series, as they look ready to go. Happy to take the last one through kvmarm tree w/ acks, and can toss it on top.
Applied to kvmarm/next, thanks!
[1/6] firmware/psci: Add definitions for PSCI v1.3 specification https://git.kernel.org/kvmarm/kvmarm/c/2f2d46959808 [2/6] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation https://git.kernel.org/kvmarm/kvmarm/c/97413cea1c48 [3/6] KVM: arm64: Add support for PSCI v1.2 and v1.3 https://git.kernel.org/kvmarm/kvmarm/c/8be82d536a9f [4/6] KVM: selftests: Add test for PSCI SYSTEM_OFF2 https://git.kernel.org/kvmarm/kvmarm/c/72be5aa6be4a [5/6] KVM: arm64: nvhe: Pass through PSCI v1.3 SYSTEM_OFF2 call https://git.kernel.org/kvmarm/kvmarm/c/94f985c39a1e
-- Best, Oliver
On Fri, Oct 25, 2024 at 10:12:41PM +0000, Oliver Upton wrote:
On Sat, 19 Oct 2024 18:15:41 +0100, David Woodhouse wrote:
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).
[...]
I grabbed the KVM portions of this series, as they look ready to go. Happy to take the last one through kvmarm tree w/ acks, and can toss it on top.
Happy for you to take the last patch as well through the KVM tree. Feel free to adjust the code comment as you see fit.
On Thu, 2024-10-31 at 12:15 +0000, Catalin Marinas wrote:
On Fri, Oct 25, 2024 at 10:12:41PM +0000, Oliver Upton wrote:
On Sat, 19 Oct 2024 18:15:41 +0100, David Woodhouse wrote:
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).
[...]
I grabbed the KVM portions of this series, as they look ready to go. Happy to take the last one through kvmarm tree w/ acks, and can toss it on top.
Happy for you to take the last patch as well through the KVM tree. Feel free to adjust the code comment as you see fit.
Thanks. that's probably the best option, as it depends on the definitions in psci.h.
On Sat, 19 Oct 2024 18:15:41 +0100, David Woodhouse wrote:
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).
[...]
Thanks Catalin for the ack, as promised:
Applied to kvmarm/next, thanks!
[6/6] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate https://git.kernel.org/kvmarm/kvmarm/c/3e251afaec9a
-- Best, Oliver
On Thu, 2024-10-31 at 17:56 +0000, Oliver Upton wrote:
On Sat, 19 Oct 2024 18:15:41 +0100, David Woodhouse wrote:
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).
[...]
Thanks Catalin for the ack, as promised:
Applied to kvmarm/next, thanks!
[6/6] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate https://git.kernel.org/kvmarm/kvmarm/c/3e251afaec9a
Thank you.
linux-kselftest-mirror@lists.linaro.org
-
Ard Biesheuvel -
Catalin Marinas -
David Woodhouse -
Lorenzo Pieralisi -
Miguel Luis -
Oliver Upton