mbox series

[v5,0/5] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation

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

Message

David Woodhouse Sept. 26, 2024, 6:37 p.m. UTC
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.

David Woodhouse (5):
      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

 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 ++++++++++++++++-
 include/kvm/arm_psci.h                          |  4 +-
 include/uapi/linux/psci.h                       |  5 ++
 tools/testing/selftests/kvm/aarch64/psci_test.c | 61 +++++++++++++++++++++++++
 8 files changed, 132 insertions(+), 2 deletions(-)

Comments

Oliver Upton Oct. 1, 2024, 3:35 p.m. UTC | #1
On Thu, Sep 26, 2024 at 07:37:58PM +0100, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>

Please, add changelogs to your patches.

What we really need here is the detail on *why* we can just bump the
PSCI version like this, i.e. no new required ABI. On top of that, you
could mention that KVM has made the implementation choice to provide
SYSTEM_OFF2 unconditionally in its PSCIv1.3 implementation.

> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  arch/arm64/kvm/hypercalls.c | 2 ++
>  arch/arm64/kvm/psci.c       | 6 +++++-
>  include/kvm/arm_psci.h      | 4 +++-
>  3 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index 5763d979d8ca..9c6267ca2b82 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -575,6 +575,8 @@ int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>  		case KVM_ARM_PSCI_0_2:
>  		case KVM_ARM_PSCI_1_0:
>  		case KVM_ARM_PSCI_1_1:
> +		case KVM_ARM_PSCI_1_2:
> +		case KVM_ARM_PSCI_1_3:
>  			if (!wants_02)
>  				return -EINVAL;
>  			vcpu->kvm->arch.psci_version = val;
> diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c
> index fd0f82464f7d..5177dda5a411 100644
> --- a/arch/arm64/kvm/psci.c
> +++ b/arch/arm64/kvm/psci.c
> @@ -328,7 +328,7 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor)
>  
>  	switch(psci_fn) {
>  	case PSCI_0_2_FN_PSCI_VERSION:
> -		val = minor == 0 ? KVM_ARM_PSCI_1_0 : KVM_ARM_PSCI_1_1;
> +		val = PSCI_VERSION(1, minor);
>  		break;
>  	case PSCI_1_0_FN_PSCI_FEATURES:
>  		arg = smccc_get_arg1(vcpu);
> @@ -486,6 +486,10 @@ int kvm_psci_call(struct kvm_vcpu *vcpu)
>  	}
>  
>  	switch (version) {
> +	case KVM_ARM_PSCI_1_3:
> +		return kvm_psci_1_x_call(vcpu, 3);
> +	case KVM_ARM_PSCI_1_2:
> +		return kvm_psci_1_x_call(vcpu, 2);
>  	case KVM_ARM_PSCI_1_1:
>  		return kvm_psci_1_x_call(vcpu, 1);
>  	case KVM_ARM_PSCI_1_0:
> diff --git a/include/kvm/arm_psci.h b/include/kvm/arm_psci.h
> index e8fb624013d1..cbaec804eb83 100644
> --- a/include/kvm/arm_psci.h
> +++ b/include/kvm/arm_psci.h
> @@ -14,8 +14,10 @@
>  #define KVM_ARM_PSCI_0_2	PSCI_VERSION(0, 2)
>  #define KVM_ARM_PSCI_1_0	PSCI_VERSION(1, 0)
>  #define KVM_ARM_PSCI_1_1	PSCI_VERSION(1, 1)
> +#define KVM_ARM_PSCI_1_2	PSCI_VERSION(1, 2)
> +#define KVM_ARM_PSCI_1_3	PSCI_VERSION(1, 3)
>  
> -#define KVM_ARM_PSCI_LATEST	KVM_ARM_PSCI_1_1
> +#define KVM_ARM_PSCI_LATEST	KVM_ARM_PSCI_1_3
>  
>  static inline int kvm_psci_version(struct kvm_vcpu *vcpu)
>  {
> -- 
> 2.44.0
>