RE: [EXTERNAL] Re: [PATCH] VIRTIO_F_USED_EVENT_AUTO_DISABLE: add new used buffer notification suppression mechanism

Vamsi Krishna Attunuru
2024-07-02T05:31:01Z

>-----Original Message-----
>From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>Sent: Tuesday, July 2, 2024 6:45 AM
>To: Xiaoguang Wang <lege.wang@jaguarmicro.com>
>Cc: Vamsi Krishna Attunuru <vattunuru@marvell.com>; Nithin Kumar
>Dabilpuram <ndabilpuram@marvell.com>; parav@nvidia.com;
>mst@redhat.com; leo.liu@jaguarmicro.com; angus.chen@jaguarmicro.com;
>Xiaoguang Wang <lege.wang@jaguarmicro.com>; virtio-
>comment@lists.linux.dev
>Subject: [EXTERNAL] Re: [PATCH] VIRTIO_F_USED_EVENT_AUTO_DISABLE:
>add new used buffer notification suppression mechanism
>
>On Mon, 1 Jul 2024 11: 44: 35 +0800, Xiaoguang Wang
><lege. wang@ jaguarmicro. com> wrote: > Currently for the PCI transport,
>used buffer notification suppression > informations are located in guest
>memory, for example, pvirtq_event_suppress
>On Mon, 1 Jul 2024 11:44:35 +0800, Xiaoguang Wang
><lege.wang@jaguarmicro.com> wrote:
>> Currently for the PCI transport, used buffer notification suppression
>> informations are located in guest memory, for example,
>> pvirtq_event_suppress for packed ring and virtq_avail for split ring,
>> then PCI devices simulated by hypervisors, such as QEMU, can vist
>> these suppression informations directly before issuing used event, the
>> only overheads may be just proper memory barriers, but for some
>> hardware PCI devices offered by DPU(Data Processing Unit), DPU would
>> need to submit DMAs to copy guest suppression informations to DPU
>> internal memory, then check whether used event should be notified by
>> examing this internal memory, which is obvious expensive, and this DMA
>> operation is needed every time DPU attempts to issue used event, DPU
>> may also choose to add a hardware component to polling suppression
>> informations by submit DMAs continuously to get newest used event
>suppression infos, but it's also expensive.
>>
>> To help improve this sitiuations, add a new feature:
>> VIRTIO_F_USED_EVENT_AUTO_DISABLE. When this feature bit is
>negotiated,
>> devices will place new notification area for used buffer suppression
>
>
>How about reuse the doorbells, like other devices?
>
>We can do some refactors for the doorbell? Let it support some new flags.
>
I think refactoring or overloading the current doorbell region would introduce synchronization issues on the device side.
Anyways this proposal places the new notification region next to the doorbell region only right.

>Thanks.
>
>
>> notification in PCI bar specfied by virtio_pci_notify_cap, also for
>> better used buffer notification coalescing, the device will disable
>> later used buffer notifications automatically after sending one used
>> buffer notification to driver, and after the driver finishes handling
>> current used buffer event, it needs to notify the device to enable
>> used buffer notification again by writing above new notification area
>> in PCI bar. Device will receive this notification reliably, and device
>> knows that it can issue used event now, but it may choose appropriate
>> timing to send used events according to its notification coalescing
>> policy , now it also does not need to DMA guest memory to obtain
>notification suppression.
>>
>> Signed-off-by: Xiaoguang Wang <lege.wang@jaguarmicro.com>
>> ---
>> content.tex | 38 ++++++++++++++++++++++++++++++++++++++
>> transport-pci.tex | 26 +++++++++++++++++++++++++-
>> 2 files changed, 63 insertions(+), 1 deletion(-)
>>
>> diff --git a/content.tex b/content.tex index 0a62dce..84505da 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -448,6 +448,7 @@ \section{Driver Notifications} \label{sec:Basic
>> Facilities of a Virtio Device / The driver is sometimes required to
>> send an available buffer notification to the device.
>>
>> +\subsection{Available Buffer Notifications} \label{sec:Basic
>> +Facilities of a Virtio Device / Driver notifications / Available
>> +Buffer Notifications}
>> When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, this
>> notification contains either a virtqueue index if
>> VIRTIO_F_NOTIF_CONFIG_DATA is not negotiated or device supplied
>> virtqueue @@ -488,6 +489,35 @@ \section{Driver Notifications}
>> \label{sec:Basic Facilities of a Virtio Device / has been negotiated,
>> these notifications would then have identical \field{next_off} and
>\field{next_wrap} values.
>>
>> +\subsection{Used Buffer Event Enable Notifications} \label{sec:Basic
>> +Facilities of a Virtio Device / Driver notifications / Used Buffer Event Enable
>Notifications} This kind of notification is only necessary when
>VIRTIO_F_USED_EVENT_AUTO_DISABLE has been negotiated.
>> +When this feature bit is negotiated, the device will disable used
>> +buffer notification automatically after sending one used buffer
>> +notification to the driver, and after the driver finishes handling
>> +current used buffer event, it needs to notify the device to enable used
>buffer notification again.
>> +
>> +\begin{description}
>> +\item [vq_index or vq_notif_config_data] Either virtqueue index or device
>> + supplied queue notification config data corresponding to a virtqueue.
>> +\item [used_off] Offset
>> + within the ring where the latest used ring entry the driver has
>processed.
>> + Without VIRTIO_F_RING_PACKED this refers to the 14 least significant
>bits of the
>> + offset (in units of descriptor entries) within the descriptor ring where
>the latest used
>> + ring entry the driver has processed. With VIRTIO_F_RING_PACKED this
>refers to the offset
>> + (in units of descriptor entries) within the descriptor ring where the
>latest used ring
>> + entry the driver has processed.
>> +\item [used_wrap] Wrap Counter.
>> + With VIRTIO_F_RING_PACKED this is the wrap counter
>> + referring to the latest used descriptor where the driver has processed.
>> + Without VIRTIO_F_RING_PACKED this refers to the most significant
>bit(bit 14) of the
>> + offset (in units of descriptor entries) within the descriptor ring where
>the latest used ring
>> + entry the driver has processed..
>> +\item [enable_notify] Whether to enable used buffer notification.
>> + The driver may benefit from just informing device the latest used ring
>entry
>> + it has processed, but not enabling used buffer notification. This field
>only
>> + occupies one bit, if set to 1, it will enable used buffer notification,
>otherwise not.
>> +\end{description}

Once driver sets enable_notify to '1', it's not supposed to update the Offset field unless
device clears/disables the notifications right. It's good to document the sequence that driver
is supposed to follow for updating offset & enable_notify fields.

>> +
>> \input{shared-mem.tex}
>>
>> \section{Exporting Objects}\label{sec:Basic Facilities of a Virtio
>> Device / Exporting Objects} @@ -872,6 +902,14 @@ \chapter{Reserved
>Feature Bits}\label{sec:Reserved Feature Bits}
>> \ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits}
>for
>> handling features reserved for future use.
>>
>> + \item[VIRTIO_F_USED_EVENT_AUTO_DISABLE(42)] This feature indicates
>> + that the device will disable used buffer notification automatically after
>sending one used buffer notification.
>> + Note: when VIRTIO_F_RING_PACKED has not been negotiated, the
>> + maximum queue size MUST NOT be greater than 16384, when
>> + VIRTIO_F_RING_PACKED has been negotiated, the maximum queue size
>MUST NOT be greater than 16383.
>> +
>> + See \ref{sec:Basic Facilities of a Virtio Device / Driver
>> + notifications / Used Buffer Event Enable Notifications}
>> +
>> \end{description}
>>
>> \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature
>> Bits} diff --git a/transport-pci.tex b/transport-pci.tex index
>> a5c6719..62ba9dc 100644
>> --- a/transport-pci.tex
>> +++ b/transport-pci.tex
>> @@ -551,6 +551,13 @@ \subsubsection{Notification structure
>layout}\label{sec:Virtio Transport Options
>> cap.offset + queue_notify_off * notify_off_multiplier
>> \end{lstlisting}
>>
>> +If the VIRTIO_F_USED_EVENT_AUTO_DISABLE feature has been
>negotiated,
>> +\field{notify_off_multiplier} is also combined with the
>> +\field{queue_notify_off} to derive the Queue Notify address for enabling
>used buffer notifications within a BAR for a virtqueue, but add a 4 bytes
>offset:
>> +\begin{lstlisting}
>> + cap.offset + queue_notify_off * notify_off_multiplier + 4
>> +\end{lstlisting}
>> +
>> The \field{cap.offset} and \field{notify_off_multiplier} are taken
>> from the notification capability structure above, and the
>> \field{queue_notify_off} is taken from the common configuration
>structure.
>> @@ -563,7 +570,7 @@ \subsubsection{Notification structure
>> layout}\label{sec:Virtio Transport Options
>> \devicenormative{\paragraph}{Notification capability}{Virtio Transport
>Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability} The
>device MUST present at least one notification capability.
>>
>> -For devices not offering VIRTIO_F_NOTIFICATION_DATA:
>> +For devices not offering VIRTIO_F_NOTIFICATION_DATA or
>VIRTIO_F_USED_EVENT_AUTO_DISABLE:
>>
>> The \field{cap.offset} MUST be 2-byte aligned.
>>
>> @@ -596,6 +603,23 @@ \subsubsection{Notification structure
>> layout}\label{sec:Virtio Transport Options cap.length >=
>> queue_notify_off * notify_off_multiplier + 4 \end{lstlisting}
>>
>> +For devices offering VIRTIO_F_USED_EVENT_AUTO_DISABLE:
>> +
>> +The device MUST either present \field{notify_off_multiplier} as a
>> +number that is a power of 2 that is also a multiple 4, or present
>> +\field{notify_off_multiplier} as 0.
>> +
>> +The \field{cap.offset} MUST be 4-byte aligned.
>> +
>> +The value \field{cap.length} presented by the device MUST be at least
>> +8 and MUST be large enough to support queue notification offsets for
>> +all supported queues in all possible configurations.
>> +
>> +For all queues, the value \field{cap.length} presented by the device MUST
>satisfy:
>> +\begin{lstlisting}
>> +cap.length >= queue_notify_off * notify_off_multiplier + 8
>> +\end{lstlisting}
>> +
>> \subsubsection{ISR status capability}\label{sec:Virtio Transport
>> Options / Virtio Over PCI Bus / PCI Device Layout / ISR status
>> capability}
>>
>> The VIRTIO_PCI_CAP_ISR_CFG capability
>> --
>> 2.34.1
>>
>>