[virtio-dev] Re: [virtio-comment] [RFC PATCH 3/5] virtio: The actions by the device upon SUSPEND

["Zhu, Lingshan" <lingshan.zhu@intel.com>]

On 8/15/2023 8:33 PM, Stefan Hajnoczi wrote:
> On Tue, Aug 15, 2023 at 07:07:42PM +0800, Zhu, Lingshan wrote:
>>
>> On 8/14/2023 11:00 PM, Stefan Hajnoczi wrote:
>>> On Tue, Aug 15, 2023 at 03:29:02AM +0800, Zhu Lingshan wrote:
>>>> This commit specifies the actions to be taken by the device upon
>>>> SUSPEND.
>>>>
>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>>> Signed-off-by: Eugenio PÃrez <eperezma@redhat.com>
>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>> ---
>>>>    content.tex | 9 +++++++++
>>>>    1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/content.tex b/content.tex
>>>> index 074f43e..43bd5de 100644
>>>> --- a/content.tex
>>>> +++ b/content.tex
>>>> @@ -96,6 +96,15 @@ \section{\field{Device Status} Field}\label{sec:Basic Facilities of a Virtio Dev
>>>>    If VIRTIO_F_SUSPEND is negotiated and SUSPEND is set, the device MUST clear SUSPEND
>>>>    and resumes operation upon DRIVER_OK.
>>>> +If VIRTIO_F_SUSPEND is negotiated, when SUSPEND is set, the device MUST perform the following operations:
>>> "when SUSPEND is set" is ambigious. It could mean when the driver writes
>>> the Device Status Field, when the device transitions to SUSPEND, or
>>> something else. Maybe "before the device reports the SUSPEND bit set in
>>> the Device Status Field"?
>> Nice catch! will fix: when the driver sets SUSPEND, before the device
>> reports the
>> SUSPEND bit set in the \field{device status}, the device MUST perform the
>> following actions.
>>>> +\begin{itemize}
>>>> +\item Stop comsuming any descriptors
>>> "consuming"
>> yes
>>> It may be more consistent to talk about virtqueue buffers (i.e.
>>> requests) rather than descriptors here.
>> yes
>>>> +\item Mark all finished descriptors as used and send used buffer notification to the driver
>>> The device has to complete everything that is in flight, just completing
>>> "finished" stuff is not enough. Does "finished descriptors" really mean
>>> "in-flight virtqueue buffers"?
>> A patch of tracking in-flight descriptors is planned. Here I expect
>> "finished" mean "done" buffers, transmitted and received ACK.
> Yes, I don't mean tracking in-flight virtqueue buffers, I mean waiting
> until they are marked as used. I think "finished" is unclear and the
> text should explain that the device waits for in-flight virtqueue
> buffers (in the future this could be relaxed with in-flight tracking
> functionality).
Sure, I can add this in the next version: the device MUST wait until all 
descriptors that being processed to finish and mark them as used

And we can replace it with tracking in-flight descriptors in the future

Thanks
>
>>> "send a used buffer notification" or "send used buffer notifications"
>> Yes will fix
>>>> +\item Record Virtqueue State of each enabled virtqueue, see section \ref{sec:Virtqueues / Virtqueue State}
>>> Does "Record" mean that Virtqueue State fields can only be accessed by
>>> the driver while device is paused (they may be outdated or invalid while
>>> the device is unpaused)?
>> Yes, to avoid race with the device and make device implementation easier,
>> this is also mentioned in Patch 4.
>>>> +\item Pause its operation and preserve all configurations in its Device Configuration Space, see \ref{sec:Basic Facilities of a Virtio Device / Device Configuration Space}
>>> What does "preserve" mean? Does it mean the Device Configuration Space
>>> is not allowed to change during SUSPEND?
>> Yes, once the device presents SUSPEND in its device status, it should not
>> change any
>> fields in its Device Configuration Space.
>>
>> Thanks for your review
>>>> +\item Present SUSPEND in \field{device status}
>>>> +\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
>>>> -- 
>>>> 2.35.3
>>>>
>>>>
>>>> This publicly archived list offers a means to provide input to the
>>>> OASIS Virtual I/O Device (VIRTIO) TC.
>>>>
>>>> In order to verify user consent to the Feedback License terms and
>>>> to minimize spam in the list archive, subscription is required
>>>> before posting.
>>>>
>>>> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
>>>> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
>>>> List help: virtio-comment-help@lists.oasis-open.org
>>>> List archive: https://lists.oasis-open.org/archives/virtio-comment/
>>>> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
>>>> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
>>>> Committee: https://www.oasis-open.org/committees/virtio/
>>>> Join OASIS: https://www.oasis-open.org/join/
>>>>
>>
>> This publicly archived list offers a means to provide input to the
>> OASIS Virtual I/O Device (VIRTIO) TC.
>>
>> In order to verify user consent to the Feedback License terms and
>> to minimize spam in the list archive, subscription is required
>> before posting.
>>
>> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
>> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
>> List help: virtio-comment-help@lists.oasis-open.org
>> List archive: https://lists.oasis-open.org/archives/virtio-comment/
>> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
>> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
>> Committee: https://www.oasis-open.org/committees/virtio/
>> Join OASIS: https://www.oasis-open.org/join/
>>


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org