On Tue, 2024-03-12 at 15:36 +0000, Marc Zyngier wrote:
> On Tue, 12 Mar 2024 13:51:28 +0000,
> David Woodhouse <dwmw2@xxxxxxxxxxxxx> wrote:
> >
> > +Hibernation using the PSCI SYSTEM_OFF2 call is enabled with the
> > +KVM_CAP_ARM_SYSTEM_OFF2 VM capability. If a guest invokes the PSCI
>
> Checking that PSCI 1.3 is enabled for the guest should be enough, no?
> I don't think providing yet another level of optionally brings us
> much, other than complexity.
This is just following what we already do for SYSTEM_RESET2. Regardless
of the PSCI version, these calls are *optional*. Shouldn't exposing
them to the guest be a deliberate choice on the part of the userspace
VMM?
I was originally thinking of a KVM_CAP with a bitmask of the optional
features to be enabled (and which would return the bitmask of supported
features). But that isn't how it was already being done, so I just
followed the existing precedent.
> > --- a/arch/arm64/kvm/hyp/nvhe/psci-relay.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/psci-relay.c
> > @@ -264,6 +264,8 @@ static unsigned long psci_1_0_handler(u64 func_id, struct kvm_cpu_context *host_
> > switch (func_id) {
> > case PSCI_1_0_FN_PSCI_FEATURES:
> > case PSCI_1_0_FN_SET_SUSPEND_MODE:
> > + case PSCI_1_3_FN_SYSTEM_OFF2:
> > + case PSCI_1_3_FN64_SYSTEM_OFF2:
>
> nit: order by version number.
Ack.
> > @@ -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 (test_bit(KVM_ARCH_FLAG_SYSTEM_OFF2_ENABLED, &kvm->arch.flags))
> > + val = 1UL << PSCI_1_3_HIBERNATE_TYPE_OFF;
> > + break;
>
> Testing the PSCI version should be enough (minor >= 3). Same thing
> goes the the capability: checking that the host supports 1.3 should be
> enough.
Wouldn't that mean we should implement *all* the new functions which
are optional in v1.3? I really think the opt-in should be per feature,
for the optional ones.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
