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

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

On 8/16/2023 10:10 AM, Jason Wang wrote:
> On Tue, Aug 15, 2023 at 7:17 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>>
>>
>> On 8/15/2023 8:29 AM, Jason Wang wrote:
>>> On Mon, Aug 14, 2023 at 7:29 PM Zhu Lingshan <lingshan.zhu@intel.com> 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:
>>>> +\begin{itemize}
>>>> +\item Stop comsuming any descriptors
>>> Typo.
>> yes will fix
>>>> +\item Mark all finished descriptors as used and send used buffer notification to the driver
>>> What happens to the unfinished descriptors?
>> still in the descriptors table or considered as in-flight, we will post
>> a patch tracking in-flight descriptors.
> So I think we should either
>
> 1) add in-flight descriptors in this series
>
> or
>
> 2) force a flush
>
> To make sure the new proposed function is complete.
Yes, will fix in the next version: The device must wait for the 
descriptors and flush the buffers.
>
>>>> +\item Record Virtqueue State of each enabled virtqueue, see section \ref{sec:Virtqueues / Virtqueue State}
>>> This basically means those states are only available after suspending
>>> or not? It would be still useful for debugging if we allow it without
>>> suspending.
>> Yes, for now only allow to read/write the virtqueue state after setting
>> SUSPEND, this is to avoid race conditions with the device.
> For debugging, we don't need to care about those races. A lot of
> hardware allow to expose those via e.g ethtool.
How about we allow read vq state without SUSPEND and forbid write vq state?
>
>> Still can suspend then collect the idx for debugging?
> I mean for runtime debugging.
>
>>>> +\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}
>>> We probably need to define the "pause" here (e.g what happens to the
>>> inflight descriptors).
>> Shall we say: freeze both its data-plane and control-plane?
> I mean if we've defined "suspend" we can simply use "suspend" instead
> of "pause"?
This section defines SUSPEND behaviors, so actually SUSPEND is defined here.
so if we use suspend in the inner text, looks like a circular reasoning.

Thanks
>
> Thanks
>
>>> Thanks
>>>
>>>> +\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
>>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
>>> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>


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