[virtio-dev] Re: [RFC PATCH 4/5] virtqueue: constraints for virtqueue state

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

On 8/15/2023 8:34 AM, Jason Wang wrote:
> On Mon, Aug 14, 2023 at 7:29 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>> This commit specifies the constraints of the virtqueue state,
>> and the actions should be taken by the device when SUSPEND
>> and DRIVER_OK is set
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>> ---
>>   content.tex | 31 +++++++++++++++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>>
>> diff --git a/content.tex b/content.tex
>> index 43bd5de..f6ac581 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -587,6 +587,37 @@ \subsection{\field{Used State} Field}
>>
>>   See also \ref{sec:Packed Virtqueues / Driver and Device Ring Wrap Counters}.
>>
>> +\drivernormative{\subsection}{Virtqueue State}{Basic Facilities of a Virtio Device / Virtqueue State}
>> +
>> +If VIRTIO_F_QUEUE_STATE has been negotiated, the driver MUST set SUSPEND in \field{device status}
>> +first before getting or setting Virtqueue State of any virtqueues.
> I don't get why this is a must. It could be useful for debugging.
To avoid race conditions with the device and make the device 
implementation easier
>
>> +
>> +If VIRTIO_F_QUEUE_STATE has been negotiaged but VIRTIO_RING_F_PACKED not been negotiated,
> typo
yes
>
>> +the driver MUST NOT access \field{Used State} of any virtqueues, it should use the
>> +used index in the used ring.
>> +
>> +\devicenormative{\subsection}{Virtqueue State}{Basic Facilities of a Virtio Device / Virtqueue State}
>> +
>> +If VIRTIO_F_QUEUE_STATE has been negotiated but SUSPEND is not set in \field{device status},
>> +the device MUST ignore any accesses against Virtqueue State of any virtqueues.
> Btw, do we need to clarify the behavior of ring reset after suspending?
I think once suspended, the device should ignore resetting a queue
>
>> +
>> +When VIRTIO_F_QUEUE_STATE has been negotiated but VIRTIO_RING_F_PACKED is not,
>> +the device MUST ignore any accesses against \field{Used State}.
>> +
>> +If VIRTIO_F_QUEUE_STATE has been negotiaged, the device MUST reset
>> +the Virtqueue State of every virtqueue upon a reset.
> Need to define the meaning of "reset" this is important for packed virtqueue.
I will remove this as Stefan suggested.
>
>> +
>> +If VIRTIO_F_QUEUE_STATE and VIRTIO_RING_F_PACKED have been negotiaged, when SUSPEND is set,
>> +the device MUST record the Virtqueue State of every enabled virtqueue
>> +in \field{Available State} and \field{Used State} respectively,
>> +and correspondingly restore the Virtqueue State of every enabled virtqueue
>> +from \field{Avaiable State} and \field{Used State} when DRIVER_OK is set.
> We can just let the device report those states in any case then we
> don't need to care about those details, or did you see any blockers?
Agree, I will add the definition of used_state of splitted vq in the 
next version

Thanks
>
> Thanks
>
>> +
>> +If VIRTIO_F_QUEUE_STATE has been negotiated but VIRTIO_RING_F_PACKED has been not, when SUSPEND is set,
>> +the device MUST record the available state of every enabled virtqueue in \field{Available State},
>> +and restore the available state of every enabled virtqueue from \field{Avaiable State}
>> +when DRIVER_OK is set.
>> +
>>   \input{admin.tex}
>>
>>   \chapter{General Initialization And Device Operation}\label{sec:General Initialization And Device Operation}
>> --
>> 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