Re: [PATCH 0/6] KVM: arm64: Implement PSCI SYSTEM_SUSPEND support

From: Marc Zyngier <maz@kernel.org>
To: Oliver Upton <oupton@google.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu,
	Peter Shier <pshier@google.com>,
	Ricardo Koller <ricarkol@google.com>,
	Jing Zhang <jingzhangos@google.com>,
	Raghavendra Rao Anata <rananta@google.com>,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <Alexandru.Elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Andrew Jones <drjones@redhat.com>
Subject: Re: [PATCH 0/6] KVM: arm64: Implement PSCI SYSTEM_SUSPEND support
Date: Thu, 26 Aug 2021 11:51:00 +0100	[thread overview]
Message-ID: <8735qwpjhn.wl-maz@kernel.org> (raw)
In-Reply-To: <CAOQ_QsgaACbcW276TAqrmq2E5ne-C2JiEDVGjVXg9-WRds8ZQA@mail.gmail.com>

On Sun, 22 Aug 2021 20:56:13 +0100,
Oliver Upton <oupton@google.com> wrote:
> 
> Marc,
> 
> On Thu, Aug 19, 2021 at 3:36 PM Oliver Upton <oupton@google.com> wrote:
> >
> > Certain VMMs/operators may wish to give their guests the ability to
> > initiate a system suspend that could result in the VM being saved to
> > persistent storage to be resumed at a later time. The PSCI v1.0
> > specification describes an SMC, SYSTEM_SUSPEND, that allows a kernel to
> > request a system suspend. This call is optional for v1.0, and KVM
> > elected to not support the call in its v1.0 implementation.
> >
> > This series adds support for the SYSTEM_SUSPEND PSCI call to KVM/arm64.
> > Since this is a system-scoped event, KVM cannot quiesce the VM on its
> > own. We add a new system exit type in this series to clue in userspace
> > that a suspend was requested. Per the KVM_EXIT_SYSTEM_EVENT ABI, a VMM
> > that doesn't care about this event can simply resume the guest without
> > issue (we set up the calling vCPU to come out of reset correctly on next
> > KVM_RUN).
> >
> > Patch 1 is unrelated, and is a fix for "KVM: arm64: Enforce reserved
> > bits for PSCI target affinities" on the kvmarm/next branch. Nothing
> > particularly hairy, just an unused param.
> 
> Title line may not have been clear on this series, Patch 1 is a fix
> for the PSCI CPU_ON series that's in kvmarm/next to suppress a
> compiler warning.

I'm not getting this warning. What are you compiling with? In general,
the compiler should shout about unused function parameters.

> 
> > Patch 2 simplifies the function to check if KVM allows a particular PSCI
> > function. We can generally disallow any PSCI function that sets the
> > SMC64 bit in the PSCI function ID.
> >
> > Patch 3 wraps up the PSCI reset logic used for CPU_ON, which will be
> > needed later to queue up a reset on the vCPU that requested the system
> > suspend.
> >
> > Patch 4 brings in the new UAPI and PSCI call, guarded behind a VM
> > capability for backwards compatibility.
> >
> > Patch 5 is indirectly related to this series, and avoids compiler
> > reordering on PSCI calls in the selftest introduced by "selftests: KVM:
> > Introduce psci_cpu_on_test".
> 
> This too is a fix for the PSCI CPU_ON series. Just wanted to raise it
> to your attention beyond the new feature I'm working on here.

I'm not sure this actually need fixing. The dependencies on the input
and output will effectively prevent such reordering. That will
definitely be a good cleanup though, but maybe not worth taking out of
this series.

Thanks,

	M. (trying to find excuses to close the tree quickly).

-- 
Without deviation from the norm, progress is not possible.