[RFC PATCH v3 0/5] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation

[RFC PATCH v3 0/5] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation

[David Woodhouse]

The PSCI v1.3 spec (https://developer.arm.com/documentation/den0022,
currently in Alpha state, hence 'RFC') 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 now,
KVM defaults to PSCI v1.2 unless explicitly requested.

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.

David Woodhouse (5):
      firmware/psci: Add definitions for PSCI v1.3 specification (ALPHA)
      KVM: arm64: Add support for PSCI v1.2 and v1.3
      KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation
      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         | 35 +++++++++++++++++++++++++++++
 include/kvm/arm_psci.h               |  4 +++-
 include/uapi/linux/psci.h            | 20 +++++++++++++++++
 kernel/power/hibernate.c             |  5 ++++-
 9 files changed, 125 insertions(+), 3 deletions(-)

[RFC PATCH v3 1/5] firmware/psci: Add definitions for PSCI v1.3 specification (ALPHA)

[David Woodhouse]

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

[RFC PATCH v3 2/5] KVM: arm64: Add support for PSCI v1.2 and v1.3

[David Woodhouse]

From: David Woodhouse <dwmw@amazon.co.uk>

Since the v1.3 specification is still in Alpha, only default to v1.2
unless userspace explicitly requests v1.3 for now.

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 1f69b667332b..f689ef3f2f10 100644
--- a/arch/arm64/kvm/psci.c
+++ b/arch/arm64/kvm/psci.c
@@ -322,7 +322,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);
@@ -449,6 +449,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..ebd7d9a12790 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_2 /* v1.3 is still Alpha */
 
 static inline int kvm_psci_version(struct kvm_vcpu *vcpu)
 {
-- 
2.44.0

Re: [RFC PATCH v3 2/5] KVM: arm64: Add support for PSCI v1.2 and v1.3

[Oliver Upton]

On Tue, Mar 19, 2024 at 12:59:03PM +0000, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Since the v1.3 specification is still in Alpha, only default to v1.2
> unless userspace explicitly requests v1.3 for now.

The ABI is final the moment we take this, alpha or not. Let's just
advertise v1.3 from the start and only apply the series when things are
ready to go.

-- 
Thanks,
Oliver

Re: [RFC PATCH v3 2/5] KVM: arm64: Add support for PSCI v1.2 and v1.3

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 488 bytes --]

On Tue, 2024-03-19 at 08:42 -0700, Oliver Upton wrote:
> On Tue, Mar 19, 2024 at 12:59:03PM +0000, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > Since the v1.3 specification is still in Alpha, only default to v1.2
> > unless userspace explicitly requests v1.3 for now.
> 
> The ABI is final the moment we take this, alpha or not. Let's just
> advertise v1.3 from the start and only apply the series when things are
> ready to go.

Makes sense.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [RFC PATCH v3 2/5] KVM: arm64: Add support for PSCI v1.2 and v1.3

[Marc Zyngier]

On Tue, 19 Mar 2024 12:59:03 +0000,
David Woodhouse <dwmw2@infradead.org> wrote:
> 
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Since the v1.3 specification is still in Alpha, only default to v1.2
> unless userspace explicitly requests v1.3 for now.
> 
> 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 1f69b667332b..f689ef3f2f10 100644
> --- a/arch/arm64/kvm/psci.c
> +++ b/arch/arm64/kvm/psci.c
> @@ -322,7 +322,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);
> @@ -449,6 +449,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..ebd7d9a12790 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_2 /* v1.3 is still Alpha */
>  
>  static inline int kvm_psci_version(struct kvm_vcpu *vcpu)
>  {

Consider making the visibility of v1.2/1.3 to userspace and guest the
last patch in the series, so that there is no transient support for
some oddball PSCI version with no feature (keeps bisection clean).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [RFC PATCH v3 2/5] KVM: arm64: Add support for PSCI v1.2 and v1.3

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

On Fri, 2024-03-22 at 16:05 +0000, Marc Zyngier wrote:
> 
> Consider making the visibility of v1.2/1.3 to userspace and guest the
> last patch in the series, so that there is no transient support for
> some oddball PSCI version with no feature (keeps bisection clean).

Ack. I think I can just reorder the patches and that will Just Work,
and the check for 'minor >= 3' will be tautologically false until the
later patch which adds v1.3 support for real. Will do that and test
that it does indeed build that way.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

[RFC PATCH v3 3/5] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation

[David Woodhouse]

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 0b5a33ee71ee..ba4ddb13e253 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6761,6 +6761,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.
 
@@ -6794,6 +6798,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 f689ef3f2f10..7acf07900c08 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);
@@ -353,6 +359,11 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor)
 			if (test_bit(KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED, &kvm->arch.flags))
 				val = 0;
 			break;
+		case PSCI_1_3_FN_SYSTEM_OFF2:
+		case PSCI_1_3_FN64_SYSTEM_OFF2:
+			if (minor >= 3)
+				val = 1UL << PSCI_1_3_HIBERNATE_TYPE_OFF;
+			break;
 		case PSCI_1_1_FN_SYSTEM_RESET2:
 		case PSCI_1_1_FN64_SYSTEM_RESET2:
 			if (minor >= 1)
@@ -374,6 +385,32 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor)
 			return 0;
 		}
 		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;
 	case PSCI_1_1_FN_SYSTEM_RESET2:
 		kvm_psci_narrow_to_32bit(vcpu);
 		fallthrough;
-- 
2.44.0

Re: [RFC PATCH v3 3/5] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation

[Marc Zyngier]

On Tue, 19 Mar 2024 12:59:04 +0000,
David Woodhouse <dwmw2@infradead.org> wrote:
> 
> 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 0b5a33ee71ee..ba4ddb13e253 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6761,6 +6761,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.
>  
> @@ -6794,6 +6798,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 f689ef3f2f10..7acf07900c08 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);
> @@ -353,6 +359,11 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor)
>  			if (test_bit(KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED, &kvm->arch.flags))
>  				val = 0;
>  			break;
> +		case PSCI_1_3_FN_SYSTEM_OFF2:
> +		case PSCI_1_3_FN64_SYSTEM_OFF2:
> +			if (minor >= 3)
> +				val = 1UL << PSCI_1_3_HIBERNATE_TYPE_OFF;
> +			break;
>  		case PSCI_1_1_FN_SYSTEM_RESET2:
>  		case PSCI_1_1_FN64_SYSTEM_RESET2:

nit: please keep the switch ordered by version number.

>  			if (minor >= 1)
> @@ -374,6 +385,32 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor)
>  			return 0;
>  		}
>  		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;
>  	case PSCI_1_1_FN_SYSTEM_RESET2:
>  		kvm_psci_narrow_to_32bit(vcpu);
>  		fallthrough;

Same thing here.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

[RFC PATCH v3 4/5] KVM: arm64: nvhe: Pass through PSCI v1.3 SYSTEM_OFF2 call

[David Woodhouse]

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 d57bcb6ab94d..76c7643e7eff 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

[RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate

[David Woodhouse]

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>
---
 drivers/firmware/psci/psci.c | 35 +++++++++++++++++++++++++++++++++++
 kernel/power/hibernate.c     |  5 ++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index d9629ff87861..69d2f6969438 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_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_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)
@@ -523,6 +547,16 @@ 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 != PSCI_RET_NOT_SUPPORTED)
+		psci_system_off2_supported = true;
+}
+
 static void __init psci_init_system_suspend(void)
 {
 	int ret;
@@ -653,6 +687,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 4b0b7cf2e019..ac87b3cb670c 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -676,8 +676,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

Re: [RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate

[Marc Zyngier]

On Tue, 19 Mar 2024 12:59:06 +0000,
David Woodhouse <dwmw2@infradead.org> wrote:

[...]

> +static void __init psci_init_system_off2(void)
> +{
> +	int ret;
> +
> +	ret = psci_features(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2));
> +
> +	if (ret != PSCI_RET_NOT_SUPPORTED)
> +		psci_system_off2_supported = true;

It'd be worth considering the (slightly broken) case where SYSTEM_OFF2
is supported, but HIBERNATE_OFF is not set in the response, as the
spec doesn't say that this bit is mandatory (it seems legal to
implement SYSTEM_OFF2 without any hibernate type, making it similar to
SYSTEM_OFF).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]

On Fri, 2024-03-22 at 16:02 +0000, Marc Zyngier wrote:
> On Tue, 19 Mar 2024 12:59:06 +0000,
> David Woodhouse <dwmw2@infradead.org> wrote:
> 
> [...]
> 
> > +static void __init psci_init_system_off2(void)
> > +{
> > +       int ret;
> > +
> > +       ret = psci_features(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2));
> > +
> > +       if (ret != PSCI_RET_NOT_SUPPORTED)
> > +               psci_system_off2_supported = true;
> 
> It'd be worth considering the (slightly broken) case where SYSTEM_OFF2
> is supported, but HIBERNATE_OFF is not set in the response, as the
> spec doesn't say that this bit is mandatory (it seems legal to
> implement SYSTEM_OFF2 without any hibernate type, making it similar to
> SYSTEM_OFF).

Such is not my understanding. If SYSTEM_OFF2 is supported, then
HIBERNATE_OFF *is* mandatory.

The next update to the spec is turning the PSCI_FEATURES response into
a *bitmap* of the available features, and I believe it will mandate
that bit zero is set.

And if for whatever reason that SYSTEM_OFF2/HIBERNATE_OFF call
*doesn't* work, Linux will end up doing a 'real' poweroff, first
through EFI and then finally as a last resort with a PSCI SYSTEM_OFF.
So it would be OK to have false positives in the detection.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate

[Marc Zyngier]

On Fri, 22 Mar 2024 16:12:44 +0000,
David Woodhouse <dwmw2@infradead.org> wrote:
> 
> On Fri, 2024-03-22 at 16:02 +0000, Marc Zyngier wrote:
> > On Tue, 19 Mar 2024 12:59:06 +0000,
> > David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > [...]
> > 
> > > +static void __init psci_init_system_off2(void)
> > > +{
> > > +       int ret;
> > > +
> > > +       ret = psci_features(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2));
> > > +
> > > +       if (ret != PSCI_RET_NOT_SUPPORTED)
> > > +               psci_system_off2_supported = true;
> > 
> > It'd be worth considering the (slightly broken) case where SYSTEM_OFF2
> > is supported, but HIBERNATE_OFF is not set in the response, as the
> > spec doesn't say that this bit is mandatory (it seems legal to
> > implement SYSTEM_OFF2 without any hibernate type, making it similar to
> > SYSTEM_OFF).
> 
> Such is not my understanding. If SYSTEM_OFF2 is supported, then
> HIBERNATE_OFF *is* mandatory.
> 
> The next update to the spec is turning the PSCI_FEATURES response into
> a *bitmap* of the available features, and I believe it will mandate
> that bit zero is set.

The bitmap is already present in the current Alpha spec:

<quote>
5.16.2 Implementation responsibilities

[...]

Bits[31] Reserved, must be zero.

Bits[30:0] Hibernate types supported.
	- 0x0 - HIBERNATE_OFF

All other values are reserved for future use.
</quote>

and doesn't say (yet) that HIBERNATE_OFF is mandatory. Furthermore,

<quote>
5.11.2 Caller responsibilities

The calling OS uses the PSCI_FEATURES API, with the SYSTEM_OFF2
function ID, to discover whether the function is present:

- If the function is implemented, PSCI_FEATURES returns the hibernate
  types supported.

- If the function is not implemented, PSCI_FEATURES returns
  NOT_SUPPORTED.
</quote>

which doesn't say anything about which hibernate type must be
implemented. Which makes sense, as I expect it to, in the fine ARM
tradition, grow things such as "HIBERNATE_WITH_ROT13_ENCRYPTION" and
even "HIBERNATE_WITH_ERRATA_XYZ", because firmware is where people
dump their crap. And we will need some special handling for these
tasty variants.

> And if for whatever reason that SYSTEM_OFF2/HIBERNATE_OFF call
> *doesn't* work, Linux will end up doing a 'real' poweroff, first
> through EFI and then finally as a last resort with a PSCI SYSTEM_OFF.
> So it would be OK to have false positives in the detection.

I agree that nothing really breaks, but I also hold the view that
broken firmware implementations should be given the finger, specially
given that you have done this work *ahead* of the spec. I would really
like this to fail immediately on these and not even try to suspend.

With that in mind, if doesn't really matter whether HIBERNATE_OFF is
mandatory or not. We really should check for it and pretend it doesn't
exist if the correct flag isn't set.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

On Fri, 2024-03-22 at 16:37 +0000, Marc Zyngier wrote:
> 
> I agree that nothing really breaks, but I also hold the view that
> broken firmware implementations should be given the finger, specially
> given that you have done this work *ahead* of the spec. I would really
> like this to fail immediately on these and not even try to suspend.
> 
> With that in mind, if doesn't really matter whether HIBERNATE_OFF is
> mandatory or not. We really should check for it and pretend it doesn't
> exist if the correct flag isn't set.

Ack.

I'll rename that variable to 'psci_system_off2_hibernate_supported' then.

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 & (1 << PSCI_1_3_HIBERNATE_TYPE_OFF))
		psci_system_off2_hibernate_supported = true;
}


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate

[Sudeep Holla]

On Fri, Mar 22, 2024 at 04:55:04PM +0000, David Woodhouse wrote:
> On Fri, 2024-03-22 at 16:37 +0000, Marc Zyngier wrote:
> > 
> > I agree that nothing really breaks, but I also hold the view that
> > broken firmware implementations should be given the finger, specially
> > given that you have done this work *ahead* of the spec. I would really
> > like this to fail immediately on these and not even try to suspend.
> > 
> > With that in mind, if doesn't really matter whether HIBERNATE_OFF is
> > mandatory or not. We really should check for it and pretend it doesn't
> > exist if the correct flag isn't set.
> 
> Ack.
> 
> I'll rename that variable to 'psci_system_off2_hibernate_supported' then.
> 
> 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 & (1 << PSCI_1_3_HIBERNATE_TYPE_OFF))
> 		psci_system_off2_hibernate_supported = true;
>

Ah OK, you have already agreed to do this, please ignore my response then.

--
Regards,
Sudeep

Re: [RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate

[Sudeep Holla]

On Fri, Mar 22, 2024 at 04:37:19PM +0000, Marc Zyngier wrote:
> On Fri, 22 Mar 2024 16:12:44 +0000,
> David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > On Fri, 2024-03-22 at 16:02 +0000, Marc Zyngier wrote:
> > > On Tue, 19 Mar 2024 12:59:06 +0000,
> > > David Woodhouse <dwmw2@infradead.org> wrote:
> > > 
> > > [...]
> > > 
> > > > +static void __init psci_init_system_off2(void)
> > > > +{
> > > > +       int ret;
> > > > +
> > > > +       ret = psci_features(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2));
> > > > +
> > > > +       if (ret != PSCI_RET_NOT_SUPPORTED)
> > > > +               psci_system_off2_supported = true;
> > > 
> > > It'd be worth considering the (slightly broken) case where SYSTEM_OFF2
> > > is supported, but HIBERNATE_OFF is not set in the response, as the
> > > spec doesn't say that this bit is mandatory (it seems legal to
> > > implement SYSTEM_OFF2 without any hibernate type, making it similar to
> > > SYSTEM_OFF).
> > 
> > Such is not my understanding. If SYSTEM_OFF2 is supported, then
> > HIBERNATE_OFF *is* mandatory.
> > 
> > The next update to the spec is turning the PSCI_FEATURES response into
> > a *bitmap* of the available features, and I believe it will mandate
> > that bit zero is set.

Correct, but we add a extra check as well to be sure even if it is mandated
unless the spec relaxes in a way that psci_features(SYSTEM_OFF2) need not
return the mandatory types in the bitmask which I doubt.

Something like:
	if (ret != PSCI_RET_NOT_SUPPORTED &&
		(ret & BIT(PSCI_1_3_HIBERNATE_TYPE_OFF)))
		psci_system_off2_supported = true;

This will ensure the firmware will not randomly set bit[0]=0 if in the
future it support some newer types as well.

I understand the kernel is not conformance test for the spec but in
practice especially for such features and PSCI spec in particular, kernel
has become defacto conformance for firmware developers which is sad.
It some feature works in the kernel, the firmware is assumed to be
conformant to the spec w.r.t the feature.

--
Regards,
Sudeep

Re: [RFC PATCH v3 0/5] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation

[Oliver Upton]

On Tue, Mar 19, 2024 at 12:59:01PM +0000, David Woodhouse wrote:
> David Woodhouse (5):
>       firmware/psci: Add definitions for PSCI v1.3 specification (ALPHA)
>       KVM: arm64: Add support for PSCI v1.2 and v1.3
>       KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation
>       KVM: arm64: nvhe: Pass through PSCI v1.3 SYSTEM_OFF2 call
>       arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate

If we're going down the route of having this PSCI call live in KVM, it
really deserves a test. I think you can just pile on the existing
psci_test selftest.

-- 
Thanks,
Oliver

Re: [RFC PATCH v3 0/5] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 3354 bytes --]

On Tue, 2024-03-19 at 08:27 -0700, Oliver Upton wrote:
> If we're going down the route of having this PSCI call live in KVM, it
> really deserves a test. I think you can just pile on the existing
> psci_test selftest.

Added to
https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/psci-hibernate
for next time.

From 8c72a78e6179bc8970edc66a85ab6bee26f581fb Mon Sep 17 00:00:00 2001
From: David Woodhouse <dwmw@amazon.co.uk>
Date: Tue, 19 Mar 2024 17:07:46 +0000
Subject: [PATCH 4/8] KVM: selftests: Add test for PSCI SYSTEM_OFF2

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 9b004905d1d3..1c1cf1580d70 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) &
+		     (1UL << PSCI_1_3_HIBERNATE_TYPE_OFF));
+	GUEST_ASSERT(psci_features(PSCI_1_3_FN64_SYSTEM_OFF2) &
+		     (1UL << 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.34.1




[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [RFC PATCH v3 0/5] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation

[Oliver Upton]

On Tue, Mar 19, 2024 at 05:14:42PM +0000, David Woodhouse wrote:
> On Tue, 2024-03-19 at 08:27 -0700, Oliver Upton wrote:
> > If we're going down the route of having this PSCI call live in KVM, it
> > really deserves a test. I think you can just pile on the existing
> > psci_test selftest.
> 
> Added to
> https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/psci-hibernate
> for next time.
> 
> From 8c72a78e6179bc8970edc66a85ab6bee26f581fb Mon Sep 17 00:00:00 2001
> From: David Woodhouse <dwmw@amazon.co.uk>
> Date: Tue, 19 Mar 2024 17:07:46 +0000
> Subject: [PATCH 4/8] KVM: selftests: Add test for PSCI SYSTEM_OFF2
> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>

Looks good, thanks!

-- 
Thanks,
Oliver

Re: [RFC PATCH v3 0/5] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]

On Tue, 2024-03-19 at 12:41 -0700, Oliver Upton wrote:
> On Tue, Mar 19, 2024 at 05:14:42PM +0000, David Woodhouse wrote:
> > On Tue, 2024-03-19 at 08:27 -0700, Oliver Upton wrote:
> > > If we're going down the route of having this PSCI call live in KVM, it
> > > really deserves a test. I think you can just pile on the existing
> > > psci_test selftest.
> > 
> > Added to
> > https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/psci-hibernate
> > for next time.
> > 
> > From 8c72a78e6179bc8970edc66a85ab6bee26f581fb Mon Sep 17 00:00:00 2001
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > Date: Tue, 19 Mar 2024 17:07:46 +0000
> > Subject: [PATCH 4/8] KVM: selftests: Add test for PSCI SYSTEM_OFF2
> > 
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> 
> Looks good, thanks!

Thanks.

Marc, I think I've also addressed your feedback? Is there anything else
to do other than wait for the spec to be published?

Shall I post a v4 with PSCI v1.3 as default and the self-test? Would
you apply that into a branch ready for merging when the spec is ready,
or should I just wait and repost it all then?


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [RFC PATCH v3 0/5] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation

[Marc Zyngier]

On Fri, 22 Mar 2024 10:17:58 +0000,
David Woodhouse <dwmw2@infradead.org> wrote:
> 
> [1  <text/plain; UTF-8 (quoted-printable)>]
> On Tue, 2024-03-19 at 12:41 -0700, Oliver Upton wrote:
> > On Tue, Mar 19, 2024 at 05:14:42PM +0000, David Woodhouse wrote:
> > > On Tue, 2024-03-19 at 08:27 -0700, Oliver Upton wrote:
> > > > If we're going down the route of having this PSCI call live in KVM, it
> > > > really deserves a test. I think you can just pile on the existing
> > > > psci_test selftest.
> > > 
> > > Added to
> > > https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/psci-hibernate
> > > for next time.
> > > 
> > > From 8c72a78e6179bc8970edc66a85ab6bee26f581fb Mon Sep 17 00:00:00 2001
> > > From: David Woodhouse <dwmw@amazon.co.uk>
> > > Date: Tue, 19 Mar 2024 17:07:46 +0000
> > > Subject: [PATCH 4/8] KVM: selftests: Add test for PSCI SYSTEM_OFF2
> > > 
> > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > Looks good, thanks!
> 
> Thanks.
> 
> Marc, I think I've also addressed your feedback? Is there anything else
> to do other than wait for the spec to be published?

Other than the couple of minor nits I mentioned in replies to the
individual patches, this looks good to me.

> Shall I post a v4 with PSCI v1.3 as default and the self-test? Would
> you apply that into a branch ready for merging when the spec is ready,
> or should I just wait and repost it all then?

I think this can wait for the final spec. I assume that you are
directly tracking this anyway, so we don't need to poll for the spec
update.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [RFC PATCH v3 0/5] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]

On Fri, 2024-03-22 at 16:09 +0000, Marc Zyngier wrote:
> 
> > Marc, I think I've also addressed your feedback? Is there anything else
> > to do other than wait for the spec to be published?
> 
> Other than the couple of minor nits I mentioned in replies to the
> individual patches, this looks good to me.

I believe I've handled all that. And also Sudeep's implicit nudge to
use BIT() instead of manually shifting (1<<PSCI_1_3_HIBERNATE_TYPE_OFF).

Rebased onto 6.8 and pushed to
https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/psci-hibernate-6.8

> > Shall I post a v4 with PSCI v1.3 as default and the self-test? Would
> > you apply that into a branch ready for merging when the spec is ready,
> > or should I just wait and repost it all then?
> 
> I think this can wait for the final spec. I assume that you are
> directly tracking this anyway, so we don't need to poll for the spec
> update.

Indeed, will post again when the spec is published. Thanks.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]