Re: [virtio-comment] [RFC] Define a low power state for devices

[David Stevens <stevensd@chromium.org>]

On Fri, Apr 14, 2023 at 12:57âAM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Apr 13, 2023 at 03:29:43PM +0900, David Stevens wrote:
> > On Fri, Apr 7, 2023 at 8:27âPM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Thu, Apr 06, 2023 at 04:30:02PM +0900, David Stevens wrote:
> > > > This RFC defines a low power state for virtio devices, to gives
> > > > drivers an option for power management besides simply resetting their
> > > > device.
> > > >
> > > > This patch is a draft aimed at starting a discussion, rather than being
> > > > a finalized patch.
> > > >
> > > > To provide some context on where this is coming from, I'm working on
> > > > reducing the power overhead of ARCVM (virtualized Android running on
> > > > ChromeOS). One of the big gaps in ARCVM power management is that it does
> > > > not implement Android's partial wake locks - i.e. the (virtualized) CPUs
> > > > are always on, even if the (virtualized) screen is off. This means we
> > > > can't force apps to stop running when they shouldn't be running, which
> > > > can lead to higher power consumption compared to the ChromeOS baseline.
> > > >
> > > > Partial wake locks are built on s2idle, but unfortunately the current
> > > > power management of virtio drivers does not let us use s2idle. The fact
> > > > that power management is based around resetting the virtio device means
> > > > that it doesn't work with stateful devices (e.g. virtio-fs). Even for
> > > > stateless devices, re-initializing all of the devices takes longer than
> > > > we would like, especially on lower end hardware.
> > > >
> > > > My rough idea for how to address this would be to make the existing
> > > > virtio power management targeted at S4 specifically (i.e. the freeze
> > > > device callback). For S0/S1/S3 (i.e. the suspend device callback), this
> > > > new lighter weight low power state would be used if available -
> > > > otherwise it would fall back to the existing S4 power management code.
> > > >
> > > > I have a very rough prototype that seems to work, and I haven't seen
> > > > anything that makes me think this approach is fundamentally unworkable.
> > > > That said, I would like to get feedback on the approach earlier rather
> > > > than later.
> > > > ---
> > > >  content.tex | 26 ++++++++++++++++++++++++++
> > > >  1 file changed, 26 insertions(+)
> > > >
> > > > diff --git a/content.tex b/content.tex
> > > > index cff548ab9675..01da6f62ef20 100644
> > > > --- a/content.tex
> > > > +++ b/content.tex
> > > > @@ -449,6 +449,28 @@ \section{Exporting Objects}\label{sec:Basic Facilities of a Virtio Device / Expo
> > > >  types. It is RECOMMENDED that devices generate version 4
> > > >  UUIDs as specified by \hyperref[intro:rfc4122]{[RFC4122]}.
> > > >
> > > > +\section{Low Power Mode}\label{sec:Basic Facilities of a Virtio Device / Low Power Mode}
> > > > +
> > > > +A virtio device can be put into a low power state when the
> > > > +VIRTIO_F_LOW_POWER bit is negotiated. How a driver puts a
> > > > +device into a low power state is transport specific.
> > > > +
> > > > +In general, a virtio device in a low power state should
> > > > +avoid initating any communication with the driver. However,
> > > > +certain device-specific functionality is exempt from this
> > > > +requirement. Such functionality is detailed in the device
> > > > +type specifications.
> > > > +
> > > > +% One example of such functionality would be allowing
> > > > +% the virtio-net device to wake up the guest to deliver
> > > > +% incoming network packets.
> > > > +
> > > > +While a virtio device is in a low power state, it should
> > > > +maintain any type specific state in such a way that it is
> > > > +able to immediately resume functioning upon leaving the low
> > > > +power state, without the need for any additional
> > > > +communication with the driver.
> > > > +
> > > >  \chapter{General Initialization And Device Operation}\label{sec:General Initialization And Device Operation}
> > > >
> > > >  We start with an overview of device initialization, then expand on the
> > > > @@ -803,6 +825,10 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> > > >    that the driver can reset a queue individually.
> > > >    See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}.
> > > >
> > > > +  \item[VIRTIO_F_LOW_POWER(41)] This feature indicates
> > > > +  that the driver can put the device into a low power mode.
> > > > +  See \ref{sec:Basic Facilities of a Virtio Device / Low Power Mode}.
> > > > +
> > > >  \end{description}
> > > >
> > > >  \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
> > >
> > > So what purpose does this flag serve exactly?  I guess transports also
> > > provide ways to enumerate supported power states, no?
> >
> > This is mostly here to parallel the VIRTIO_F_SR_IOV feature flag.
> > Generally speaking, it does seem redundant with the transport-specific
> > enumeration.
> >
> > The two potential uses I can think of would be to allow devices to
> > support transport level power management without supporting virtio
> > level power management (might apply to existing devices?) and to allow
> > devices to behave differently if they know that the driver doesn't
> > support virtio power management. But I don't know how useful these are
> > in practice.
> >
> > -David
>
> I'm a bit confused by all this. So there are actually two types of PM?

At least in the PCI case, there are two different specifications
involved (VIRTIO and PCI/PCIe), so I think it's fair to say there are
two different types of power management. If we want any specific
behavior when devices are in the "VIRTIO" low power mode, if we don't
have a feature bit somewhere, then we'd be mandating that feature for
any virtio device which implements PCI power management.

In particular, I would like to require that VIRTIO low power mode
preserves state, which is stricter than the PCI/PCIe spec. Reading the
spec again, keying on No_Soft_Reset = 1b might be sufficient, although
it's not completely clear to me whether No_Soft_Reset is a static
property of a given device or a property of a specific D3_Hot -> D0
transition. If it's a static property of the device, then I guess we
can probably avoid defining a top level feature bit in favor of device
type feature bits for any device-specific changes that we hash out
later.

> What does initiating communication involve? Is consuming buffers
> initiating communication? Sending interrupts?

I think I was coming at defining what low power state means from the
wrong way. Rather than defining what a device can't do, I think it's
easier to define what a device must do. How about this definition:

+ A driver MUST not interact with a device in low power mode device
+ in any way except for to take the device out of low power mode or to
+ handle wakeup events generated by the device. How wakeup events
+ are implemented is transport specific, and when a device should
+ generate wakeup events is device type specific.
+
+ A device in low power mode MUST maintain its state such that all driver
+ visible state after exiting low power mode exactly matches driver visible
+ state before entering low power mode. A device in low power mode
+ SHOULD minimize its resource usage, although what steps to take are
+ specific to a particular device implementation.

This modified definition would let us replace the part about
"initiating communication" in the PCI changes to just be

+ Wakeup events are implemented as PMEs.

-David