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

[David Stevens <stevensd@chromium.org>]

On Thu, Apr 6, 2023 at 9:35âPM Stefan Hajnoczi <stefanha@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(+)
>
> I'm curious to see how this maps to virtio-pci and the underlying PCI
> power management features. Do you have a VIRTIO PCI transport spec
> proposal you can share?

Here's a proposal for the change to the VIRTIO PCI transport spec. In
my testing, this is sufficient for things to work with Linux's
existing PCI power management functionality. Although I'm far from an
expert in this area, so I definitely could be missing something.

diff --git a/transport-pci.tex b/transport-pci.tex
index 5d984673cd60..a2e78bd118f2 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -1146,3 +1146,15 @@ \subsubsection{Driver Handling
Interrupts}\label{sec:Virtio Transport Options /
         re-examine the configuration space to see what changed.
     \end{itemize}
 \end{itemize}
+
+\devicenormative{\paragraph}{Notification of Device Configuration
Changes}{Virtio Transport Options / Virtio Over PCI Bus / PCI-specific
Initialization And Device Operation / Low Power Mode}
+
+Low power mode corresponds to the device power management state
+D3\textsubscript{Hot}. A device offering VIRTIO_F_LOW_POWER MUST
+present the PCI Power Management Capability. Such a device MUST
+NOT perform an internal reset when transitioning from
+D3\textsubscript{Hot} to D0.
+
+When a device in a low power state initiates communication with
+the driver, it MUST generate a PME via the transport appropriate
+mechanism.

-David