[virtio-comment] [PATCH] virtio-rng: add a control queue

[virtio-comment] [PATCH] virtio-rng: add a control queue

[Laurent Vivier]

A control queue is needed to send command to ask the device to
release the request buffers if it cannot provide data.

Changes:

  5.4.2 Virtqueues
  0 requestq
  1 controlq
  controlq only exists if VIRTIO_RNG_F_CTRL_VQ set.

  5.4.3 Feature bits
  VIRTIO_RNG_F_CTRL_VQ (0)
  Device has a control queue

  5.4.5 Device Initialization
  Identify and initialize the request virtqueue.
  If the VIRTIO_RNG_F_CTRL_VQ feature bit is negotiated,
  identify the control virtqueue.

  5.4.6.3 Control Virtqueue
  The driver uses the control virtqueue (if VIRTIO_RNG_F_CTRL_VQ is negotiated)
  to send commands to control the device.
  All commands are of the following form:
  struct virtio_rng_ctrl_hdr {
         u8 cmd;
  };

  5.4.6.3.1 Request Virtqueue Flushing
  When the device is not able to provide enough entropy to the driver,
  the driver can be stuck. And for instance, it cannot be removed from the
  system to switch to another one. The flush command allows the driver to ask
  the device to release all the buffers sent to the device and to unlock the
  driver.
  #define VIRTIO_RNG_CMD_FLUSH 0

Bug-Link: https://github.com/oasis-tcs/virtio-spec/issues/83
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 content.tex | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/content.tex b/content.tex
index 91735e3eb018..e37078650be8 100644
--- a/content.tex
+++ b/content.tex
@@ -4952,10 +4952,16 @@ \subsection{Device ID}\label{sec:Device Types / Entropy Device / Device ID}
 \subsection{Virtqueues}\label{sec:Device Types / Entropy Device / Virtqueues}
 \begin{description}
 \item[0] requestq
+\item[1] controlq
 \end{description}
 
+controlq only exists if VIRTIO_RNG_F_CTRL_VQ set.
+
 \subsection{Feature bits}\label{sec:Device Types / Entropy Device / Feature bits}
-  None currently defined
+
+\begin{description}
+\item[VIRTIO_RNG_F_CTRL_VQ (0)] Device has a control queue
+\end{description}
 
 \subsection{Device configuration layout}\label{sec:Device Types / Entropy Device / Device configuration layout}
   None currently defined.
@@ -4963,7 +4969,11 @@ \subsection{Device configuration layout}\label{sec:Device Types / Entropy Device
 \subsection{Device Initialization}\label{sec:Device Types / Entropy Device / Device Initialization}
 
 \begin{enumerate}
-\item The virtqueue is initialized
+\item Identify and initialize the request virtqueue.
+
+\item If the VIRTIO_RNG_F_CTRL_VQ feature bit is negotiated,
+  identify the control virtqueue.
+
 \end{enumerate}
 
 \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device Operation}
@@ -4984,6 +4994,30 @@ \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O
 The device MUST place one or more random bytes into the buffer, but it
 MAY use less than the entire buffer length.
 
+\subsubsection{Control Virtqueue}\label{sec:Device Types / Entropy Device / Device Operation / Control Virtqueue}
+
+The driver uses the control virtqueue (if VIRTIO_RNG_F_CTRL_VQ is
+negotiated) to send commands to control the device.
+
+All commands are of the following form:
+
+\begin{lstlisting}
+struct virtio_rng_ctrl_hdr {
+       u8 cmd;
+};
+\end{lstlisting}
+
+\paragraph{Request Virtqueue Flushing}\label{sec:Device Types / Entropy Device / Device Operation / Control Virtqueue / Request Virtqueue Flushing}
+
+When the device is not able to provide enough entropy to the driver, the driver can be stuck.
+And for instance, it cannot be removed from the system to switch to another one.
+The flush command allows the driver to ask the device to release all the buffers sent to the
+device and to unlock the driver.
+
+\begin{lstlisting}
+#define VIRTIO_RNG_CMD_FLUSH 0
+\end{lstlisting}
+
 \section{Traditional Memory Balloon Device}\label{sec:Device Types / Memory Balloon Device}
 
 This is the traditional balloon device.  The device number 13 is
-- 
2.26.2


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/

Re: [virtio-comment] [PATCH] virtio-rng: add a control queue

[Cornelia Huck]

On Mon,  3 Aug 2020 15:24:31 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> A control queue is needed to send command to ask the device to
> release the request buffers if it cannot provide data.
> 
> Changes:
> 
>   5.4.2 Virtqueues
>   0 requestq
>   1 controlq
>   controlq only exists if VIRTIO_RNG_F_CTRL_VQ set.
> 
>   5.4.3 Feature bits
>   VIRTIO_RNG_F_CTRL_VQ (0)
>   Device has a control queue
> 
>   5.4.5 Device Initialization
>   Identify and initialize the request virtqueue.
>   If the VIRTIO_RNG_F_CTRL_VQ feature bit is negotiated,
>   identify the control virtqueue.
> 
>   5.4.6.3 Control Virtqueue
>   The driver uses the control virtqueue (if VIRTIO_RNG_F_CTRL_VQ is negotiated)
>   to send commands to control the device.
>   All commands are of the following form:
>   struct virtio_rng_ctrl_hdr {
>          u8 cmd;
>   };
> 
>   5.4.6.3.1 Request Virtqueue Flushing
>   When the device is not able to provide enough entropy to the driver,
>   the driver can be stuck. And for instance, it cannot be removed from the
>   system to switch to another one. The flush command allows the driver to ask
>   the device to release all the buffers sent to the device and to unlock the
>   driver.
>   #define VIRTIO_RNG_CMD_FLUSH 0
> 
> Bug-Link: https://github.com/oasis-tcs/virtio-spec/issues/83
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  content.tex | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)

(...)

> +\paragraph{Request Virtqueue Flushing}\label{sec:Device Types / Entropy Device / Device Operation / Control Virtqueue / Request Virtqueue Flushing}
> +
> +When the device is not able to provide enough entropy to the driver, the driver can be stuck.
> +And for instance, it cannot be removed from the system to switch to another one.

Maybe

"...the driver may be stuck waiting for the device returning buffers,
and cannot be removed..." ?

> +The flush command allows the driver to ask the device to release all the buffers sent to the
> +device and to unlock the driver.

What does "unlock the driver" mean?

You'll probably also want a device normative statement for that, something like

"When the driver sends a flush request, the device MUST release all
outstanding buffers."

> +
> +\begin{lstlisting}
> +#define VIRTIO_RNG_CMD_FLUSH 0
> +\end{lstlisting}

As we (currently) have only that one command, I think we need to
specify that (a) negotiation of the CTRL_VQ feature implies that the
flush command is available, and that (b) any other value of cmd should
(must?) be ignored by the device. (I'd assume that new commands would
require new feature bits.)

> +
>  \section{Traditional Memory Balloon Device}\label{sec:Device Types / Memory Balloon Device}
>  
>  This is the traditional balloon device.  The device number 13 is


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/

Re: [virtio-comment] [PATCH] virtio-rng: add a control queue

[Laurent Vivier]

On 03/08/2020 15:57, Cornelia Huck wrote:
> On Mon,  3 Aug 2020 15:24:31 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
>> A control queue is needed to send command to ask the device to
>> release the request buffers if it cannot provide data.
>>
>> Changes:
>>
>>   5.4.2 Virtqueues
>>   0 requestq
>>   1 controlq
>>   controlq only exists if VIRTIO_RNG_F_CTRL_VQ set.
>>
>>   5.4.3 Feature bits
>>   VIRTIO_RNG_F_CTRL_VQ (0)
>>   Device has a control queue
>>
>>   5.4.5 Device Initialization
>>   Identify and initialize the request virtqueue.
>>   If the VIRTIO_RNG_F_CTRL_VQ feature bit is negotiated,
>>   identify the control virtqueue.
>>
>>   5.4.6.3 Control Virtqueue
>>   The driver uses the control virtqueue (if VIRTIO_RNG_F_CTRL_VQ is negotiated)
>>   to send commands to control the device.
>>   All commands are of the following form:
>>   struct virtio_rng_ctrl_hdr {
>>          u8 cmd;
>>   };
>>
>>   5.4.6.3.1 Request Virtqueue Flushing
>>   When the device is not able to provide enough entropy to the driver,
>>   the driver can be stuck. And for instance, it cannot be removed from the
>>   system to switch to another one. The flush command allows the driver to ask
>>   the device to release all the buffers sent to the device and to unlock the
>>   driver.
>>   #define VIRTIO_RNG_CMD_FLUSH 0
>>
>> Bug-Link: https://github.com/oasis-tcs/virtio-spec/issues/83
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  content.tex | 38 ++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> (...)
> 
>> +\paragraph{Request Virtqueue Flushing}\label{sec:Device Types / Entropy Device / Device Operation / Control Virtqueue / Request Virtqueue Flushing}
>> +
>> +When the device is not able to provide enough entropy to the driver, the driver can be stuck.
>> +And for instance, it cannot be removed from the system to switch to another one.
> 
> Maybe
> 
> "...the driver may be stuck waiting for the device returning buffers,
> and cannot be removed..." ?

ok

> 
>> +The flush command allows the driver to ask the device to release all the buffers sent to the
>> +device and to unlock the driver.
> 
> What does "unlock the driver" mean?

In fact, this is very implementation specific: in current linux
implementation the driver is waiting on the data with read mutex locked.
And with this mutex locked you can't do anything else in the driver...

I will replace this by:
"This allows the driver to release resources allocated to receive data
from the device."

> 
> You'll probably also want a device normative statement for that, something like
> 
> "When the driver sends a flush request, the device MUST release all
> outstanding buffers."
> 
>> +
>> +\begin{lstlisting}
>> +#define VIRTIO_RNG_CMD_FLUSH 0
>> +\end{lstlisting}
> 
> As we (currently) have only that one command, I think we need to
> specify that (a) negotiation of the CTRL_VQ feature implies that the
> flush command is available, and that (b) any other value of cmd should
> (must?) be ignored by the device. (I'd assume that new commands would
> require new feature bits.)

I will add:

"The negociation of the CTRL_VQ feature implies that the flush command
is available."
"Any other value of cmd MUST be ignored by the device."

But where? In this section?

Thanks,
Laurent


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/

Re: [virtio-comment] [PATCH] virtio-rng: add a control queue

[Cornelia Huck]

On Mon, 3 Aug 2020 17:00:34 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 03/08/2020 15:57, Cornelia Huck wrote:
> > On Mon,  3 Aug 2020 15:24:31 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> A control queue is needed to send command to ask the device to
> >> release the request buffers if it cannot provide data.
> >>
> >> Changes:
> >>
> >>   5.4.2 Virtqueues
> >>   0 requestq
> >>   1 controlq
> >>   controlq only exists if VIRTIO_RNG_F_CTRL_VQ set.
> >>
> >>   5.4.3 Feature bits
> >>   VIRTIO_RNG_F_CTRL_VQ (0)
> >>   Device has a control queue
> >>
> >>   5.4.5 Device Initialization
> >>   Identify and initialize the request virtqueue.
> >>   If the VIRTIO_RNG_F_CTRL_VQ feature bit is negotiated,
> >>   identify the control virtqueue.
> >>
> >>   5.4.6.3 Control Virtqueue
> >>   The driver uses the control virtqueue (if VIRTIO_RNG_F_CTRL_VQ is negotiated)
> >>   to send commands to control the device.
> >>   All commands are of the following form:
> >>   struct virtio_rng_ctrl_hdr {
> >>          u8 cmd;
> >>   };
> >>
> >>   5.4.6.3.1 Request Virtqueue Flushing
> >>   When the device is not able to provide enough entropy to the driver,
> >>   the driver can be stuck. And for instance, it cannot be removed from the
> >>   system to switch to another one. The flush command allows the driver to ask
> >>   the device to release all the buffers sent to the device and to unlock the
> >>   driver.
> >>   #define VIRTIO_RNG_CMD_FLUSH 0
> >>
> >> Bug-Link: https://github.com/oasis-tcs/virtio-spec/issues/83
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >>  content.tex | 38 ++++++++++++++++++++++++++++++++++++--
> >>  1 file changed, 36 insertions(+), 2 deletions(-)  
> > 
> > (...)
> >   
> >> +\paragraph{Request Virtqueue Flushing}\label{sec:Device Types / Entropy Device / Device Operation / Control Virtqueue / Request Virtqueue Flushing}
> >> +
> >> +When the device is not able to provide enough entropy to the driver, the driver can be stuck.
> >> +And for instance, it cannot be removed from the system to switch to another one.  
> > 
> > Maybe
> > 
> > "...the driver may be stuck waiting for the device returning buffers,
> > and cannot be removed..." ?  
> 
> ok
> 
> >   
> >> +The flush command allows the driver to ask the device to release all the buffers sent to the
> >> +device and to unlock the driver.  
> > 
> > What does "unlock the driver" mean?  
> 
> In fact, this is very implementation specific: in current linux
> implementation the driver is waiting on the data with read mutex locked.
> And with this mutex locked you can't do anything else in the driver...
> 
> I will replace this by:
> "This allows the driver to release resources allocated to receive data
> from the device."

Sounds good.

> 
> > 
> > You'll probably also want a device normative statement for that, something like
> > 
> > "When the driver sends a flush request, the device MUST release all
> > outstanding buffers."
> >   
> >> +
> >> +\begin{lstlisting}
> >> +#define VIRTIO_RNG_CMD_FLUSH 0
> >> +\end{lstlisting}  
> > 
> > As we (currently) have only that one command, I think we need to
> > specify that (a) negotiation of the CTRL_VQ feature implies that the
> > flush command is available, and that (b) any other value of cmd should
> > (must?) be ignored by the device. (I'd assume that new commands would
> > require new feature bits.)  
> 
> I will add:
> 
> "The negociation of the CTRL_VQ feature implies that the flush command
> is available."

"If VIRTIO_RNG_F_CTRL_VQ is negotiated, the device MUST implement the
flush command." ?

> "Any other value of cmd MUST be ignored by the device."
> 
> But where? In this section?

I think you want a devicenormative paragraph in the control vq
subsection (also for the MUST statement above.) This needs to be
referenced by the conformance part of the spec.

> 
> Thanks,
> Laurent
> 


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/