Hi,
On Tue, Mar 12, 2024 at 01:51:28PM +0000, David Woodhouse wrote:
> From: David Woodhouse <[email protected]>
>
> 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. There are other
features being done in userspace (e.g. vCPU hotplug) built on
intercepting hypercalls, this seems to be a reasonable candidate too.
[*] https://docs.kernel.org/virt/kvm/devices/vm.html#attribute-kvm-arm-vm-smccc-filter-w-o
--
Thanks,
Oliver
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 <[email protected]>
> >
> > 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?
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.
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.
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.
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 <[email protected]>
> > >
> > > 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
On Wed, 2024-03-13 at 12:42 -0700, Oliver Upton wrote:
> 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.
Right... so we'd intercept PSCI_FEATURES *just* to indicate support for
the one call we implement in userspace, and pass all other
PSCI_FEATURES calls through to the kernel to handle the others?
And then we'd implement SYSTEM_OFF2, hooking it up to do whatever our
existing KVM_EXIT_SYSTEM_EVENT handler *already* does for a standard
power-off, just with the extra flag to show it's a hibernate.
This concept does not fill me with joy.
> 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.
I'd be more concerned about supporting a draft ABI in a public cloud
provider, TBH. Having it as just one more downstream kernel patch,
posted upstream but not yet merged before the final publication of the
spec, would be the least of my worries.