OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: Re: [PATCH] virtio: introduce VIRTIO_F_RING_RESET for reset queue


On Thu, Sep 16, 2021 at 3:53 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> This patch allows the driver to reset a queue individually.
>
> This is very common on general network equipment. By disabling a queue,
> you can quickly reclaim the buffer currently on the queue. If necessary,
> we can reinitialize the queue separately.
>
> For example, when virtio-net implements support for AF_XDP, we need to
> disable a queue to release all the original buffers when AF_XDP setup.
> And quickly release all the AF_XDP buffers that have been placed in the
> queue when AF_XDP exits.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>  content.tex | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/content.tex b/content.tex
> index 3aeb4a4..235cbf9 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -1001,7 +1001,13 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>  After writing 0 to \field{device_status}, the driver MUST wait for a read of
>  \field{device_status} to return 0 before reinitializing the device.
>
> -The driver MUST NOT write a 0 to \field{queue_enable}.
> +If the VIRTIO_F_RING_RESET feature bit is not negotiated,
> +the driver MUST NOT write a 0 to \field{queue_enable}.

I guess what you actually mean is "write a 1"?

> +
> +Otherwise, if the VIRTIO_F_RING_RESET feature bit is negotiated,
> +the driver can stop using the queue by writing a 0 to \field{queue_enable} and

I believe it's actually preventing the device from using the queue.

> +MUST read the value back to ensure synchronization. Then optionally re-enable
> +the queue.

See above, write 1 should enable the queue not zero?

>
>  \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
>
> @@ -1964,9 +1970,15 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
>  \field{QueueNum}, \field{QueueDescLow}, \field{QueueDescHigh},
>  \field{QueueDriverLow}, \field{QueueDriverHigh}, \field{QueueDeviceLow}, \field{QueueDeviceHigh}.
>
> -To stop using the queue the driver MUST write zero (0x0) to this
> +If the VIRTIO_F_RING_RESET feature bit is not negotiated,
> +to stop using the queue the driver MUST write zero (0x0) to this
>  \field{QueueReady} and MUST read the value back to ensure
> -synchronization.
> +synchronization. But the queue cannot be re-enabled.
> +
> +Otherwise, if the VIRTIO_F_RING_RESET feature bit is negotiated,
> +the driver can stop using the queue by writing a 0 to \field{QueueReady} and
> +MUST read the value back to ensure synchronization. Then optionally re-enable
> +the queue.
>
>  The driver MUST ignore undefined bits in \field{InterruptStatus}.
>
> @@ -6673,6 +6685,20 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>    transport specific.
>    For more details about driver notifications over PCI see \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}.
>
> +  \item[VIRTIO_F_RING_RESET(40)] This feature indicates
> +  that the driver can reset a queue individually.
> +
> +  "reset a queue" is divided into two parts. We can stop a queue first, and then
> +  re-enable the queue. The re-enable part is optional.
> +
> +  In the process of independently stopping a queue, we MUST first notify the
> +  device and confirm that we have successfully stopped a queue. Then we destroy
> +  the state and memory of the queue.

I think we need to define the "stop" here. E.g queue_enable has the
following description:

"The driver uses this to selectively prevent the device from executing
requests from this virtqueue"

And as we discussed, if we "stop" and "re-enable" does the queue
continue from the previous state?

> +
> +  Re-enable the queue, just like the process of a queue during the reset process.
> +  Reallocate memory and set the state.

Who will do the reallocation and set? Is it the driver or the device?

> For example, the avail index MUST be the
> +  same as the reset process, set to 0.

I guess the "avail index" should be the one stored in the device, not
the one in the memory. We then need to clarify this.

I was thinking we probably need to clarify the queue state [1] and I
wonder if it's better to introduce a new field like "queue_reset"
instead of reusing queue_enable.

(Not a native speaker but using queue_enble feels like the queue can
resume with the previous states).

Thanks

[1] https://markmail.org/message/os2hoamjd6theoje

> +
>  \end{description}
>
>  \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
> --
> 2.31.0
>



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]