[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: RE: [EXT] Re: [virtio-comment] [PATCH v9] virtio-net: Add support for the flexible driver notification structure.
>-----Original Message-----
>From: Michael S. Tsirkin <mst@redhat.com>
>Sent: Monday, 7 December, 2020 16:06
>To: Cornelia Huck <cohuck@redhat.com>
>Cc: Halil Pasic <pasic@linux.ibm.com>; Vitaly Mireyno <vmireyno@marvell.com>; virtio-
>comment@lists.oasis-open.org; Jason Wang <jasowang@redhat.com>; Ariel Elior
><aelior@marvell.com>
>Subject: [EXT] Re: [virtio-comment] [PATCH v9] virtio-net: Add support for the flexible driver
>notification structure.
>
>External Email
>
>----------------------------------------------------------------------
>On Tue, Nov 24, 2020 at 11:19:07AM +0100, Cornelia Huck wrote:
>> On Mon, 23 Nov 2020 17:16:42 +0100
>> Halil Pasic <pasic@linux.ibm.com> wrote:
>>
>> > On Thu, 5 Nov 2020 22:41:42 +0000
>> > Vitaly Mireyno <vmireyno@marvell.com> wrote:
>> >
>> > > When the driver is required to send an available buffer notification to the device, it sends the
>virtqueue number to be notified.
>> > > With this new feature, the device can optionally provide a per-virtqueue value for the driver to
>use in driver notifications, instead of the virtqueue number.
>> > > Some devices may benefit from this flexibility by providing, for example, an internal virtqueue
>identifier, or an internal offset related to the virtqueue number.
>> > >
>> > > Changes from v8:
>> > > * Incorporated comments for v8:
>> > > - moved the feature from a network device to a global section
>> > > - few minor changes
>> > >
>> > > Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
>> > > ---
>> > > content.tex | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>> > > 1 file changed, 45 insertions(+), 1 deletion(-)
>>
>> >
>> > Whith your patch applied on top of current master I get something
>> > like this
>> >
>> > """
>> > \subsubsection{Available Buffer Notifications}\label{sec:Virtio
>> > Transport Options / Virtio Over PCI Bus / PCI-specific
>> > Initialization And Device Operation / Available Buffer
>> > Notifications}
>> >
>> > When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the driver
>> > sends an available buffer notification to the device by writing the
>> > 16-bit virtqueue index of this virtqueue to the Queue Notify
>> > address.
>> >
>> > When VIRTIO_F_NOTIFICATION_DATA has been negotiated, the driver
>> > sends an available buffer notification to the device by writing the
>> > following 32-bit value to the Queue Notify address:
>> > \lstinputlisting{notifications-le.c}
>> >
>> > See \ref{sec:Virtqueues / Driver
>> > notifications}~\nameref{sec:Virtqueues / Driver notifications} for the definition of the components.
>> >
>> > See \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI
>> > Device Layout / Notification capability} for how to calculate the Queue Notify address.
>> >
>> > \drivernormative{\paragraph}{Available Buffer Notifications}{Virtio
>> > Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available
>Buffer Notifications} If VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated:
>> > \begin{itemize}
>> > \item If VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the
>> > driver MUST use the \field{queue_notify_data} value instead of the virtqueue index.
>> > \item If VIRTIO_F_NOTIFICATION_DATA has been negotiated, the driver
>> > MUST set the \field{vqn} field to the \field{queue_notify_data} value.
>> > \end{itemize}
>> >
>> > \subsubsection{Used Buffer Notifications}\label{sec:Virtio Transport
>> > Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Used Buffer
>Notifications} """
>> >
>> > The device normative section and the preceding text are IMHO in
>> > contradiction. The former says the driver sends a 16-bit virtqueue
>> > index unconditionally, the later says if if
>> > VIRTIO_F_NOTIFICATION_DATA has been negotiated then use
>> > queue_notify_data, instead (thus referencing the non-normative content).
>> >
>>
>> I'd probably reword the paragraph above to
>>
>> "When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the driver
>> sends an available buffer notification to the device by writing either
>> the 16-bit virtqueue index of this virtqueue or, if
>> VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated, the 16-bit queue
>> notification data to the Queue Notify address."
>>
>> > IMHO we need a normative section that fully describes all the
>> > available cases.
>> >
>> > I think we can do this on top.
>>
>> Agreed.
>
>Vitaly can you please propose a patch addressing all these comments?
>
Sure. I see that the patch was committed, so I'll work on a new one addressing these comments.
>
>> >
>> > Regards,
>> > Halil
>> >
>> > 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://urldefense.proofpoint.com/v2/url?u=https-3A__lists.oasis-2Do
>> > pen.org_archives_virtio-2Dcomment_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ
>> > &r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=Sdg9nY3DXvmW8GOOAnS
>> > c3bvhynULlXrU6VJr0PwpCQw&s=PUKZFs2yNUYr72olm1OXA3_bn6TJDPIPkctEwLQV_
>> > _c&e= Feedback License:
>> > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dope
>> > n.org_who_ipr_feedback-5Flicense.pdf&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xt
>> > fQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=Sdg9nY3DXvmW8GOOA
>> > nSc3bvhynULlXrU6VJr0PwpCQw&s=vdV5kbrxxGNM4ip1ZRHf-1ZYtlJo3iC40zyQHey
>> > tOUs&e= List Guidelines:
>> > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dope
>> > n.org_policies-2Dguidelines_mailing-2Dlists&d=DwIBAg&c=nKjWec2b6R0mO
>> > yPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=Sdg9nY3DXv
>> > mW8GOOAnSc3bvhynULlXrU6VJr0PwpCQw&s=53wcRwDYA794xlY_OV9nEQ4Pb9j5kXk4
>> > owwz4acRJlk&e=
>> > Committee:
>> > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dope
>> > n.org_committees_virtio_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW
>> > 52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=Sdg9nY3DXvmW8GOOAnSc3bvhynULl
>> > XrU6VJr0PwpCQw&s=j_NyouqpYmr-2MPBHQyQXFLZXj01PEcFqtoNnZiB0bY&e=
>> > Join OASIS:
>> > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dope
>> > n.org_join_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgF
>> > Rdcevq01tbLQAw4A_NO7xgI&m=Sdg9nY3DXvmW8GOOAnSc3bvhynULlXrU6VJr0PwpCQ
>> > w&s=4oVMylhMszp8y4x3fdF6X00K_r-3YIhPzz4cHLp5XdA&e=
>> >
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]