[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]