[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] [PATCH v3] virtio-net: Add support for the flexible driver notification structure
>-----Original Message-----
>From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-open.org> On Behalf Of Jason
>Wang
>Sent: Thursday, 27 February, 2020 12:21
>To: Michael S. Tsirkin <mst@redhat.com>
>Cc: Vitaly Mireyno <vmireyno@marvell.com>; virtio-comment@lists.oasis-open.org; Ariel Elior
><aelior@marvell.com>
>Subject: [EXT] Re: [virtio-comment] [PATCH v3] virtio-net: Add support for the flexible driver
>notification structure
>
>External Email
>
>----------------------------------------------------------------------
>
>On 2020/2/27 äå6:11, Michael S. Tsirkin wrote:
>> On Thu, Feb 27, 2020 at 06:05:30PM +0800, Jason Wang wrote:
>>> On 2020/2/27 äå1:10, Michael S. Tsirkin wrote:
>>>> On Wed, Feb 26, 2020 at 02:54:36PM +0000, Vitaly Mireyno wrote:
>>>>>> -----Original Message-----
>>>>>> From: Jason Wang <jasowang@redhat.com>
>>>>>> Sent: Wednesday, 26 February, 2020 5:13
>>>>>> To: Vitaly Mireyno <vmireyno@marvell.com>;
>>>>>> virtio-comment@lists.oasis-open.org
>>>>>> Cc: Michael S. Tsirkin <mst@redhat.com>; Ariel Elior
>>>>>> <aelior@marvell.com>
>>>>>> Subject: [EXT] Re: [virtio-comment] [PATCH v3] virtio-net: Add
>>>>>> support for the flexible driver notification structure
>>>>>>
>>>>>> ------------------------------------------------------------------
>>>>>> ----
>>>>>>
>>>>>> On 2020/2/26 äå12:30, Vitaly Mireyno wrote:
>>>>>>> Currently, the driver notification (available buffer notification) has a fixed structure.
>>>>>>> If VIRTIO_F_NOTIFICATION_DATA has been negotiated, it includes: vqn, next_off and
>next_wrap.
>>>>>>> If notify_off_multiplier > 0, the VQ number can be derived by the
>>>>>>> device from the Queue Notify
>>>>>> address, so vqn may be redundant.
>>>>>>> Some devices benefit from receiving an additional data with
>>>>>>> driver notifications. This data can
>>>>>> optionally replace the vqn field in the driver notification structure.
>>>>>>> In its simplest form, it would be sufficient for this data to be a per-device constant value.
>>>>>>>
>>>>>>> Changes from v2 - Defined a new feature flag instead of a PCI-specific flag.
>>>>>>>
>>>>>>> Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
>>>>>>> ---
>>>>>>> content.tex | 34 ++++++++++++++++++++++++++++++++++
>>>>>>> 1 file changed, 34 insertions(+)
>>>>>>>
>>>>>>> diff --git a/content.tex b/content.tex index b91a132..5223d5c
>>>>>>> 100644
>>>>>>> --- a/content.tex
>>>>>>> +++ b/content.tex
>>>>>>> @@ -965,6 +965,8 @@ \subsubsection{Notification structure
>>>>>>> layout}\label{sec:Virtio Transport
>>>>>> Options
>>>>>>> struct virtio_pci_notify_cap {
>>>>>>> struct virtio_pci_cap cap;
>>>>>>> le32 notify_off_multiplier; /* Multiplier for
>>>>>>> queue_notify_off. */
>>>>>>> + le16 notify_data; /* Data to be placed in the vqn field */
>>>>>>> + le16 padding; /* Pad to a dword */
>>>>>>> };
>>>>>>> \end{lstlisting}
>>>>>>>
>>>>>>> @@ -984,6 +986,21 @@ \subsubsection{Notification structure
>>>>>>> layout}\label{sec:Virtio Transport
>>>>>> Options
>>>>>>> the same Queue Notify address for all queues.
>>>>>>> \end{note}
>>>>>>>
>>>>>>> +\field{notify_data} is the data that the driver will set in the
>>>>>>> +\field{vqn} field in the available buffer notification, if
>>>>>>> +VIRTIO_NET_F_NOTIF_EXTRA_DATA has been negotiated.
>>>>>>> +
>>>>>>> +\begin{note}
>>>>>>> +If \field{notify_off_multiplier} > 0, the virtqueue number can
>>>>>>> +potentially be derived by the device from the Queue Notify
>>>>>>> +address, so \field{vqn} may be redundant. Some devices benefit
>>>>>>> +from receiving the additional data with driver notifications. An
>>>>>>> +example could be a hardware device implementing multiple
>>>>>>> +protocols (with virtio being one of them), so extra notification
>>>>>>> +data could serve as a notification type indication or a protocol
>>>>>> indication.
>>>>>>> +Another example could be using shared hardware memory space for
>>>>>>> +driver notifications for multiple virtio devices in a trusted environment.
>>>>>>> +\end{note}
>>>>>>> +
>>>>>>> \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.
>>>>>>>
>>>>>>> @@ -1020,6 +1037,10 @@ \subsubsection{Notification structure
>>>>>>> layout}\label{sec:Virtio Transport
>>>>>> Options
>>>>>>> cap.length >= queue_notify_off * notify_off_multiplier + 4
>>>>>>> \end{lstlisting}
>>>>>>>
>>>>>>> +If VIRTIO_NET_F_NOTIF_EXTRA_DATA has been negotiated, the device
>>>>>>> +MUST set \field{notify_data} to a valid value, and SHOULD set
>>>>>>> +\field{notify_off_multiplier} > 0.
>>>>>>> +
>>>>>>> \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 @@ -1519,6 +1540,14 @@
>>>>>>> \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Option
>>>>>>> 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} The driver SHOULD accept the
>>>>>> VIRTIO_NET_F_NOTIF_EXTRA_DATA feature if it has been offered.
>>>>>>
>>>>>>
>>>>>> I think we need use "MUST" here?
>>>>>>
>>>>> Wouldnât it break the existing drivers then?
>>>> I agree, we can't declare existing drivers non-conformant by fiat.
>>>> Certainly not for a boutique feature like this.
>>>
>>> I may miss something but I think device won't work correctly if this
>>> feature is not negotiated?
>>>
>>> Thanks
>> That would depend on the device. Device can always fail FEATURES_OK if
>> it doesn't support existing drivers.
>> I really need to address
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_oasis-
>> 2Dtcs_virtio-2Dspec_issues_71&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ
>> 2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=nz5vSwBxhEZ3_S38Fikj0WPa4nyI
>> FyJOn1tT22DHP5A&s=C6M9AZj_BRJc7VUaltzvCiSh2SnKMte3l1lLjY5THw4&e=
>> so people know what to expect ...
>
>
>That's my understanding as well.
>
>E.g for IOMMU_PLATFORM, device can still work if it the feature is not negotiated so we use "SHOULD"
>in the spec.
>
>But for this feature, there's no way except for failing the negotiation, so "MUST" seems correct.
>
If I understand correctly, if we'll define it as "MUST", all the existing drivers will become non-conformant, no?
If we'll define it as "SHOULD", existing drivers will still be conformant, and will just not work with the specific device that needs this feature.
Also, even basic feature like VIRTIO_F_RING_PACKED is mentioned as "SHOULD": "A driver SHOULD accept VIRTIO_F_RING_PACKED if it is offered."
What do you think?
>Thanks
>
>
>>
>>>>
>>>>>> Other looks good to me.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>>
>>>>>>> +
>>>>>>> +If VIRTIO_NET_F_NOTIF_EXTRA_DATA has been negotiated, the driver
>>>>>>> +MUST set the \field{vqn} field of the available buffer
>>>>>>> +notification structure to the \field{notify_data} value.
>>>>>>> +
>>>>>>> \subsubsection{Used Buffer Notifications}\label{sec:Virtio
>>>>>>> Transport Options / Virtio Over PCI Bus / PCI-specific
>>>>>>> Initialization And Device Operation / Used Buffer Notifications}
>>>>>>>
>>>>>>> If a used buffer notification is necessary for a virtqueue, the device would typically act as
>follows:
>>>>>>> @@ -2895,6 +2924,10 @@ \subsection{Feature bits}\label{sec:Device
>>>>>>> Types / Network Device /
>>>>>> Feature bits
>>>>>>> \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>>>>>>> channel.
>>>>>>>
>>>>>>> +\item[VIRTIO_NET_F_NOTIF_EXTRA_DATA(57)] Driver provides an extra data with
>>>>>>> + available buffer notifications, to aid in notification processing by the
>>>>>>> + device.
>>>>>>> +
>>>>>>> \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
>>>>>>> value. Device benefits from knowing the exact header length.
>>>>>>>
>>>>>>> @@ -2934,6 +2967,7 @@ \subsubsection{Feature bit
>>>>>>> requirements}\label{sec:Device Types /
>>>>>> Network Device
>>>>>>> \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
>>>>>>> \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4
>>>>>>> or
>>>>>> VIRTIO_NET_F_HOST_TSO6.
>>>>>>> \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
>>>>>>> +\item[VIRTIO_NET_F_NOTIF_EXTRA_DATA] Requires VIRTIO_F_NOTIFICATION_DATA.
>>>>>>> \end{description}
>>>>>>>
>>>>>>> \subsubsection{Legacy Interface: Feature
>>>>>>> bits}\label{sec:Device Types / Network Device / Feature bits /
>>>>>>> Legacy Interface: Feature bits}
>>>>>>> --
>>>>>>>
>>>>>>> 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-
>>>>>>> 2Dope
>>>>>>> n.org_archives_virtio-2Dcomment_&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtf
>>>>>>> Q&r=l
>>>>>>> DHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=94BRvyqIARqVJ-yydsIq
>>>>>>> G25_z
>>>>>>> AUHjeENww-NxeMs_s8&s=fspKS4uA_CyM-S4HrR6GdJbvttjkX5iCygZvcybe_qs&
>>>>>>> e=
>>>>>>> Feedback License:
>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>>>>>>> org_who_ipr_feedback-5Flicense.pdf&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7x
>>>>>>> tfQ&r
>>>>>>> =lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=94BRvyqIARqVJ-yyds
>>>>>>> IqG25
>>>>>>> _zAUHjeENww-NxeMs_s8&s=sskV_Q0nV6odmu9tb3_QO4sJgxQQfA7SDNkDbhCPgZ
>>>>>>> 0&e=
>>>>>>> List Guidelines:
>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>>>>>>> org_policies-2Dguidelines_mailing-2Dlists&d=DwIFaQ&c=nKjWec2b6R0m
>>>>>>> OyPaz
>>>>>>> 7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=94BRvyqIARq
>>>>>>> VJ-yy
>>>>>>> dsIqG25_zAUHjeENww-NxeMs_s8&s=0FDuoPUSirLgEwBf7ATkyVMaalFeB6N0-vh
>>>>>>> pOnkB
>>>>>>> vWY&e=
>>>>>>> Committee:
>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>>>>>>> org_committees_virtio_&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2F
>>>>>>> W52oJ
>>>>>>> 3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=94BRvyqIARqVJ-yydsIqG25_zAUHje
>>>>>>> ENww- NxeMs_s8&s=raC15yZMnr9y9Iv5_woorP7Bq2OQiO-uybPhgqzNtlE&e=
>>>>>>> Join OASIS:
>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>>>>>>> org_join_&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArg
>>>>>>> FRdce
>>>>>>> vq01tbLQAw4A_NO7xgI&m=94BRvyqIARqVJ-yydsIqG25_zAUHjeENww-NxeMs_s8
>>>>>>> &s=Gw VYWNwNZr7MFb5jJ7acLULJni1uMlwLgWBgiTSrp74&e=
>>>>>>>
>>>> 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=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ
>>>> &r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=nz5vSwBxhEZ3_S38Fik
>>>> j0WPa4nyIFyJOn1tT22DHP5A&s=lu2_ycYan1SMgGPt6rPWbYTJz-KKXvx8tY7FXvHok
>>>> ok&e= Feedback License:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dope
>>>> n.org_who_ipr_feedback-5Flicense.pdf&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xt
>>>> fQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=nz5vSwBxhEZ3_S38F
>>>> ikj0WPa4nyIFyJOn1tT22DHP5A&s=cRti96FQZ3brRot2AiK7oLvI0o3xnFszgolfmA3
>>>> PL_A&e= List Guidelines:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dope
>>>> n.org_policies-2Dguidelines_mailing-2Dlists&d=DwIFaQ&c=nKjWec2b6R0mO
>>>> yPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=nz5vSwBxhE
>>>> Z3_S38Fikj0WPa4nyIFyJOn1tT22DHP5A&s=a1mAWhOj9wMRvIbKTnAR6DKkhX9XJc5W
>>>> TEK_7xtRj7w&e=
>>>> Committee:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dope
>>>> n.org_committees_virtio_&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW
>>>> 52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=nz5vSwBxhEZ3_S38Fikj0WPa4nyIF
>>>> yJOn1tT22DHP5A&s=9ViH9CRdsIVpN4dHeKF6hAfgYxw4W36A8eWqSlsta4o&e=
>>>> Join OASIS:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dope
>>>> n.org_join_&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgF
>>>> Rdcevq01tbLQAw4A_NO7xgI&m=nz5vSwBxhEZ3_S38Fikj0WPa4nyIFyJOn1tT22DHP5
>>>> A&s=ZkxpzujHsaR-YJs_--CK0J52uGrM2CogpGD5XowEgrM&e=
>>>>
>
>
>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-
>2Dopen.org_archives_virtio-
>2Dcomment_&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4
>A_NO7xgI&m=nz5vSwBxhEZ3_S38Fikj0WPa4nyIFyJOn1tT22DHP5A&s=lu2_ycYan1SMgGPt6rPWbYTJz-
>KKXvx8tY7FXvHokok&e=
>Feedback License: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_who_ipr_feedback-
>5Flicense.pdf&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4
>A_NO7xgI&m=nz5vSwBxhEZ3_S38Fikj0WPa4nyIFyJOn1tT22DHP5A&s=cRti96FQZ3brRot2AiK7oLvI0o3xn
>FszgolfmA3PL_A&e=
>List Guidelines: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_policies-2Dguidelines_mailing-
>2Dlists&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7
>xgI&m=nz5vSwBxhEZ3_S38Fikj0WPa4nyIFyJOn1tT22DHP5A&s=a1mAWhOj9wMRvIbKTnAR6DKkhX9XJc
>5WTEK_7xtRj7w&e=
>Committee: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_committees_virtio_&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgF
>Rdcevq01tbLQAw4A_NO7xgI&m=nz5vSwBxhEZ3_S38Fikj0WPa4nyIFyJOn1tT22DHP5A&s=9ViH9CRdsIV
>pN4dHeKF6hAfgYxw4W36A8eWqSlsta4o&e=
>Join OASIS: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_join_&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQ
>Aw4A_NO7xgI&m=nz5vSwBxhEZ3_S38Fikj0WPa4nyIFyJOn1tT22DHP5A&s=ZkxpzujHsaR-YJs_--
>CK0J52uGrM2CogpGD5XowEgrM&e=
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]