Re: [RFC PATCH 1/2] KVM: arm64: Add PSCI SYSTEM_OFF2 function for hibernation

[Oliver Upton <oliver.upton@linux.dev>]

On Wed, Mar 13, 2024 at 12:53:45PM +0000, David Woodhouse wrote:
> On Tue, 2024-03-12 at 08:47 -0700, Oliver Upton wrote:
> > Hi,
> > 
> > On Tue, Mar 12, 2024 at 01:51:28PM +0000, David Woodhouse 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.
> > 
> > Have you considered doing the PSCI implementation in userspace? The
> > SMCCC filter [*] was added for this exact purpose. 
> > 
> 
> For the purpose of deprecating the in-KVM PSCI implementation and
> reimplementing it in VMMs? So we're never going to add new features and
> versions to the kernel PSCI?

I'm not against the idea of adding features to the in-kernel PSCI
implementation when it has a clear reason to live in the kernel. For
this hypercall in particular the actual implementation lives in
userspace, the KVM code is just boilerplate for migration / UAPI
compatibility.

> If that's the case then I suppose I can send the patch to clearly
> document the in-KVM PSCI as deprecated, and do it that way.

There probably isn't an awful lot to be gained from documenting the UAPI
as deprecated, we will never actually get to delete it.

> But to answer your question directly, no I hadn't considered that. I
> was just following the existing precedent of adding new optional PSCI
> features like SYSTEM_RESET2.

Understandable. And the infrastructure I'm recommending didn't exist
around the time of THE SYSTEM_RESET2 addition.

> I didn't think that the addition of SYSTEM_OFF2 in precisely the same
> fashion would be the straw that broke the camel's back, and pushed us
> to reimplement it in userspace instead.

I do not believe using the SMCCC filter to take SYSTEM_OFF2 to userspace
would necessitate a _full_ userspace reimplementation. You're free to
leave the default handler in place for functions you don't care about.
Forwarding PSCI_VERSION, PSCI_FEATURES, and SYSTEM_OFF2 would be sufficient
to get this off the ground, and the VMM can still advertise the rest of
the hypercalls implemented by KVM.

That might get you where you want to go a bit faster, since it'd avoid
any concerns about implementing a draft ABI in the kernel.

-- 
Thanks,
Oliver