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

["Uminski, Piotr" <Piotr.Uminski@intel.com>]

> -----Original Message-----
> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> open.org> On Behalf Of Zhu, Lingshan
> Sent: Tuesday, August 15, 2023 1:30 PM
> To: Jason Wang <jasowang@redhat.com>
> Cc: mst@redhat.com; eperezma@redhat.com; cohuck@redhat.com; virtio-
> comment@lists.oasis-open.org; virtio-dev@lists.oasis-open.org
> Subject: [virtio-comment] Re: [RFC PATCH 4/5] virtqueue: constraints for
> virtqueue state
> 
> 
> 
> 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
It should be possible to set Virtqueue State in two states:
 - Before DRIVER_OK - as a part of the queue creation at target system
-  After SUSPEND - when the device was activated and then suspended
Setting state on active device can produce random results and should be forbidden.
We may allow to get Virtqueue State on active device, without SUSPEND. However, we should not mandate the device
to report correct value - on some implementations it may be impossible without suspend.
> >
> >> +
> >> +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
> >>
> 
> 
> 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/

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.