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

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