[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] [PATCH] virtio-rng: add a control queue
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 >
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]