OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-comment message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]

Subject: Re: [PATCH V2 2/6] virtio: introduce SUSPEND bit in device status



On 11/3/2023 11:54 PM, Parav Pandit wrote:

From: Zhu, Lingshan <lingshan.zhu@intel.com>
Sent: Friday, November 3, 2023 8:25 PM

On 11/3/2023 7:35 PM, Parav Pandit wrote:

From: Zhu Lingshan <lingshan.zhu@intel.com>
Sent: Friday, November 3, 2023 4:05 PM

This patch introduces a new status bit in the device status: SUSPEND.

This SUSPEND bit can be used by the driver to suspend a device, in
order to stabilize the device states and virtqueue states.

Its main use case is live migration.

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>

You constantly complained that whatever was proposed using admin

commands method in [1] must work for passthrough and non-passthrough.

And halfway in the discussion you propose a method after learning all the

limitations of in-band, you propose a solution only works for non-passthrough
mode.

You asked someone to have comprehensive proposal and when it comes to

you following it, you just donât.
not sure what you are talking about.

And have most shallow commit message to not even mention it.

Please be consistent in design approach.
And if you donât want to be, stop asking others.

this SUSPEND/RESUME doesn't change since the RFC series, how can it not be
inconsistent???

This is not the way TC collaboration works.
I probably shouldnât even expect this from you.

Your proposal does not cover both the use cases of passthrough and non-passthrough.
Yet you kept demanding them for others.
This is just wrong.

I am aware that both models as technical pros and cons.
Why this doesn't work? the device status byte has been working for many years, 
and do you know when guest freeze, the hypervisor owns the device????



[1]
https://lists.oasis-open.org/archives/virtio-comment/202310/msg00472.h
tml

Please don't be so emotional and please be professional.

Why this solution can not work for pass-through? Do you know the device
ownership will be transferred to the hypervisor when guest suspended in live
migration?

I explained 5 reasons why it does not work in previous reply.

As the word indicates "live migration", the hypervisor needs to access the device when it is "live" (not just after).
Hence, passthrough mode must be able to capture the state of the device and dirty pages database when its live.
(and after the source is suspended).

No, the hypervisor should only collect dirty pages when the device alive.
As you can see, the dirty page tracking facility has a PASID for isolation. But still, the 
question is, we should better use platform dirty page tracking
Then suspend the device after guest freeze, to stabilize the device status, then read the status. 

How can you say this does not work???



Signed-off-by: Eugenio PÃrez <eperezma@redhat.com>
---
   content.tex | 36 ++++++++++++++++++++++++++++++++++--
   1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/content.tex b/content.tex index 76813b5..bcc9d4b 100644
--- a/content.tex
+++ b/content.tex
@@ -49,6 +49,10 @@ \section{\field{Device Status}
Field}\label{sec:Basic Facilities of a Virtio Dev

   \item[DEVICE_NEEDS_RESET (64)] Indicates that the device has

experienced

     an error from which it can't recover.
+
+\item[SUSPEND (16)] When VIRTIO_F_SUSPEND is negotiated, indicates
+that the
+  device has been suspended by the driver.
+
   \end{description}

   The \field{device status} field starts out as 0, and is
reinitialized to 0 by @@ -
73,6 +77,10 @@ \section{\field{Device Status} Field}\label{sec:Basic
Facilities of a Virtio Dev  recover by issuing a reset.
   \end{note}

+The driver SHOULD NOT set SUSPEND if FEATURES_OK is not set.
+
+When setting SUSPEND, the driver MUST re-read \field{device status}
+to ensure
the SUSPEND bit is set.
+
   \devicenormative{\subsection}{Device Status Field}{Basic Facilities
of a Virtio Device / Device Status Field}

   The device MUST NOT consume buffers or send any used buffer @@
-82,6
+90,26 @@ \section{\field{Device Status} Field}\label{sec:Basic
+Facilities of a
Virtio Dev  that a reset is needed.  If DRIVER_OK is set, after it
sets DEVICE_NEEDS_RESET, the device  MUST send a device configuration
change notification to the driver.

+The device MUST ignore SUSPEND if FEATURES_OK is not set.
+
+The device MUST ignore SUSPEND if VIRTIO_F_SUSPEND is not negotiated.
+
+The device SHOULD allow settings to \field{device status} even when
+SUSPEND
is set.
+
+If VIRTIO_F_SUSPEND is negotiated and SUSPEND is set, the device
+SHOULD clear SUSPEND and resumes operation upon DRIVER_OK.
+
+If VIRTIO_F_SUSPEND is negotiated, when the driver sets SUSPEND, the
+device SHOULD perform the following actions before presenting
+SUSPEND bit
in the \field{device status}:
+
+\begin{itemize}
+\item Stop consuming buffers of any virtqueues and mark all finished
descritors as used.
+\item Wait until all descriptors that being processed to finish and
+mark them
as used.
+\item Flush all used buffer and send used buffer notifications to the driver.
+\item Record Virtqueue State of each enabled virtqueue, see section
+\ref{sec:Virtqueues / Virtqueue State} \item Pause its operation
+except \field{device status} and preserve configurations in its
+Device Configuration Space, see \ref{sec:Basic Facilities of a
+Virtio Device / Device Configuration Space} \end{itemize}
+
   \section{Feature Bits}\label{sec:Basic Facilities of a Virtio
Device / Feature Bits}

   Each virtio device offers all the features it understands.  During
@@ -99,10
+127,10 @@ \section{Feature Bits}\label{sec:Basic Facilities of a
+Virtio Device /
Feature B  \begin{description}
   \item[0 to 23, and 50 to 127] Feature bits for the specific device
type

-\item[24 to 42] Feature bits reserved for extensions to the queue
and
+\item[24 to 43] Feature bits reserved for extensions to the queue
+and
     feature negotiation mechanisms

-\item[43 to 49, and 128 and above] Feature bits reserved for future

extensions.

+\item[44 to 49, and 128 and above] Feature bits reserved for future
extensions.
   \end{description}

   \begin{note}
@@ -875,6 +903,10 @@ \chapter{Reserved Feature
Bits}\label{sec:Reserved Feature Bits}
     \item[VIRTIO_F_QUEUE_STATE(42)] This feature indicates that the
device allows the driver
     to access its internal virtqueue state.

+  \item[VIRTIO_F_SUSPEND(43)] This feature indicates that the driver can
+   SUSPEND the device.
+   See \ref{sec:Basic Facilities of a Virtio Device / Device Status Field}.
+
   \end{description}

   \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature
Bits}
--
2.35.3

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]