Re: [PATCH v4 2/6] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation

[Francesco Lavra <francescolavra.fl@gmail.com>]

On Tue, 2024-09-24 at 17:05 +0100, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> The PSCI v1.3 specification (alpha) adds support for a SYSTEM_OFF2

Can remove (alpha).

> function
> which is analogous to ACPI S4 state. This will allow hosting
> environments
> to determine that a guest is hibernated rather than just powered off,
> and
> ensure that they preserve the virtual environment appropriately to
> allow
> the guest to resume safely (or bump the hardware_signature in the
> FACS to
> trigger a clean reboot instead).
> 
> The beta version will be changed to say that PSCI_FEATURES returns a
> bit
> mask of the supported hibernate types, which is implemented here.

Since the final spec has been released, we can revise or remove the
above wording.

> Although this new feature is inflicted unconditionally on unexpecting
> userspace, it ought to be mostly OK because it still results in the
> same
> KVM_SYSTEM_EVENT_SHUTDOWN event, just with a new flag which hopefully
> won't cause userspace to get unhappy.
> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  Documentation/virt/kvm/api.rst    | 11 +++++++++
>  arch/arm64/include/uapi/asm/kvm.h |  6 +++++
>  arch/arm64/kvm/psci.c             | 37
> +++++++++++++++++++++++++++++++
>  3 files changed, 54 insertions(+)
> 
> diff --git a/Documentation/virt/kvm/api.rst
> b/Documentation/virt/kvm/api.rst
> index b3be87489108..2918898b7047 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6840,6 +6840,10 @@ the first `ndata` items (possibly zero) of the
> data array are valid.
>     the guest issued a SYSTEM_RESET2 call according to v1.1 of the
> PSCI
>     specification.
>  
> + - for arm64, data[0] is set to
> KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2
> +   if the guest issued a SYSTEM_OFF2 call according to v1.3 of the
> PSCI
> +   specification.
> +
>   - for RISC-V, data[0] is set to the value of the second argument of
> the
>     ``sbi_system_reset`` call.
>  
> @@ -6873,6 +6877,13 @@ either:
>   - Deny the guest request to suspend the VM. See ARM DEN0022D.b
> 5.19.2
>     "Caller responsibilities" for possible return values.
>  
> +Hibernation using the PSCI SYSTEM_OFF2 call is enabled when PSCI
> v1.3
> +is enabled. If a guest invokes the PSCI SYSTEM_OFF2 function, KVM
> will
> +exit to userspace with the KVM_SYSTEM_EVENT_SHUTDOWN event type and
> with
> +data[0] set to KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2. The only
> +supported hibernate type for the SYSTEM_OFF2 function is
> HIBERNATE_OFF
> +0x0).

The spec says that the HIBERNATE_OFF parameter value is 0x1, not 0x0
(which is kind of unfortunate because it doesn't match the
corresponding bit in the feature flags).
So, either the BIT(PSCI_1_3_HIBERNATE_TYPE_OFF) value should be used
for the SYSTEM_OFF2 functions in the code, or the definition should be
changed in the header file (unless the text in the spec is wrong).