Re: [PATCH] virtio-mem: introduce VIRTIO_MEM_F_PERSISTENT_SUSPEND

[Matias Ezequiel Vara Larsen <mvaralar@redhat.com>]

On Mon, Oct 07, 2024 at 10:17:23AM +0200, David Hildenbrand wrote:
> On 09.08.24 16:03, Juraj Marcin wrote:
> > Before, the behavior while suspending to a deep sleep state and waking
> > up was not specified. For example, in x86 QEMU VM all devices receive a
> > reset request during wake-up. This would lead to unplugging of all the
> > plugged memory blocks. Due to this, suspending is disallowed in the
> > Linux Kernel, when plugged memory is present and
> > VIRTIO_MEM_F_PERSISTENT_SUSPEND feature flag is not advertised by the
> > virtio-mem device.
> > 
> > This new flag should signal to the guest driver, that the device can
> > correctly suspend to a deep sleep state and then wake up without
> > disrupting the plugged memory blocks. This feature flag is already
> > supported in the Linux Kernel [1] and should be soon also supported in
> > QEMU [2].
> > 
> > [1]: https://lore.kernel.org/all/20240318120645.105664-1-david@redhat.com/
> > [2]: https://lore.kernel.org/all/20240806160756.182524-1-jmarcin@redhat.com/
> > 
> > Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
> > ---
> >   device-types/mem/description.tex | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> > 
> > diff --git a/device-types/mem/description.tex b/device-types/mem/description.tex
> > index 0a6d558..5bff24c 100644
> > --- a/device-types/mem/description.tex
> > +++ b/device-types/mem/description.tex
> > @@ -60,6 +60,8 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Device / Feature bits}
> >   access unplugged memory. \footnote{On platforms with memory properties that
> >   might get modified implicitly on memory access, this feature is expected to
> >   be offered by the device.}
> > +\item[VIRTIO_MEM_F_PERSISTENT_SUSPEND (2)] The driver can allow the guest
> > +system to enter suspended state (deep sleep, suspend-to-RAM).
> >   \end{description}
> >   \subsection{Device configuration layout}\label{sec:Device Types / Memory Device / Device configuration layout}
> > @@ -174,6 +176,9 @@ \subsection{Device Initialization}\label{Device Types / Memory Device / Device I
> >   The device MUST NOT modify memory or memory properties of plugged memory
> >   blocks during device reset.
> > +The device SHOULD offer VIRTIO_MEM_F_PERSISTENT_SUSPEND if the platform
> > +supports suspending (deep sleep, suspend-to-RAM) with plugged memory blocks.
> > +
> >   \subsection{Device Operation}\label{sec:Device Types / Memory Device / Device Operation}
> >   The device notifies the driver about the amount of memory the device wants
> > @@ -268,6 +273,9 @@ \subsection{Device Operation}\label{sec:Device Types / Memory Device / Device Op
> >   memory blocks to unplug them, or it would not be able to make use of
> >   unplugged memory blocks after plugging them (e.g., alignment).
> > +If VIRTIO_MEM_F_PERSISTENT_SUSPEND has not been negotiated, the driver MUST NOT
> > +allow the guest system to enter a suspended state (deep sleep, suspend-to-RAM).

I think `guest system` could be simply `guest` but I do not have a
strong opinion about it.

> > +
> >   \devicenormative{\subsubsection}{Device Operation}{Device Types / Memory Device / Device Operation}
> >   The device MUST provide the exact same memory properties with the exact same
> > @@ -293,6 +301,10 @@ \subsection{Device Operation}\label{sec:Device Types / Memory Device / Device Op
> >   The device SHOULD unplug all memory blocks during system resets.
> > +If VIRTIO_MEM_F_PERSISTENT_SUSPEND has been negotiated, the device MUST NOT not
> > +change the state of memory blocks when suspending or when waking up from
> > +suspended state (deep sleep, suspend-to-RAM).
> > +
> >   \subsubsection{PLUG request}\label{sec:Device Types / Memory Device / Device Operation / PLUG request}
> >   Request to plug consecutive memory blocks that are currently unplugged.
> 
> The kernel and QEMU part is upstream. A second pair of eyes would be great
> :)
> 

Reviewed-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>

Matias