Re: [PATCH v4 1/1] Add suspend support for virtio PCI devices

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Feb 16, 2024 at 05:24:32PM +0900, David Stevens wrote:
> Add a virtio power management PCI capability to allow drivers to suspend
> virtio PCI devices. This allows drivers to suspend devices at the virtio
> level before suspending them at the PCI transport layer. This allows
> drivers to do a two phase suspend, which prevents notifications from
> being ignored or lost if interrupts are reconfigured at the PCI
> transport layer immediately before or after the device is put into the
> PCI PM D3 low power state.
> ---
>  transport-pci.tex | 51 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/transport-pci.tex b/transport-pci.tex
> index a5c6719ea871..ce77708a9b69 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -188,6 +188,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
>  #define VIRTIO_PCI_CAP_SHARED_MEMORY_CFG 8
>  /* Vendor-specific data */
>  #define VIRTIO_PCI_CAP_VENDOR_CFG        9
> +/* Power management configuration */
> +#define VIRTIO_PCI_CAP_PM_CFG            10
>  \end{lstlisting}
>  
>          Any other value is reserved for future use.
> @@ -804,6 +806,55 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
>  specified by some other Virtio Structure PCI Capability
>  of type other than \field{VIRTIO_PCI_CAP_PCI_CFG}.
>  
> +\subsubsection{Power management capability}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Power management capability}
> +
> +The VIRTIO_PCI_CAP_PM_CFG capability refers to a single byte. The
> +driver can write to the byte to set the power state of the device,
> +and it can read from the byte to get the current power state of the
> +device.
> +
> +The valid power states are:
> +
> +\begin{lstlisting}
> +/* Device is operating normally */
> +#define VIRTIO_PM_STATE_ACTIVE    0
> +/* Device operation is suspended */
> +#define VIRTIO_PM_STATE_SUSPENDED 1
> +\end{lstlisting}
> +
> +The device power state has no effect when \field{device status} does
> +not have the DRIVER_OK bit set or does have the DEVICE_NEEDS_RESET bit
> +set.


Given this is only after DRIVER_OK, wouldn't a feature bit + status bit
make more sense? This will make it transport-independent and simplify
discovery.

> +\devicenormative{\paragraph}{Power management capability}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Power management capability}
> +
> +A device MUST maintain its state while suspended such that all driver
> +visible state after resuming exactly matches driver visible state
> +before suspending.
> +
> +A device MUST NOT send notifications while suspended.
> +
> +A device MAY operate on any buffers in its virtqueue while suspended.

How is this reconsiled with state matching exactly? buffers are
driver-visible ...

> +A device MUST set its power state to VIRTIO_PM_STATE_ACTIVE on reset.
> +
> +A device SHOULD take steps to minimize its resource consumption while
> +suspended, although what this involves is specific to the particular
> +device implementation.
> +
> +\drivernormative{\paragraph}{Power management capability}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Power management capability}
> +
> +A driver MUST NOT access a suspended device's BARs corresponding to
> +any virtio structures, except for the power management byte.
> +
> +A driver MAY suspend a device that has buffers in one of its
> +virtqueues, but it MUST NOT modify any such buffers while the device
> +is suspended.
> +
> +A driver MUST read from the power management byte after writing to the
> +byte to verify that the device successfully entered the target power
> +state.

Verify how? By checking the value returned? And what should it do with the value 
does not match?


> +
>  \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
>  
>  Transitional devices MUST present part of configuration
> -- 
> 2.44.0.rc0.258.g7320e95886-goog