content: support SR-IOV

RFC In review Tiwei Bie 2 participants 5 messages May 18, 2018

lore

Share & tools

lore

Tiwei Bie

author

2018-05-18 07:39 UTC #0 lore raw

RFC 1/1 content: support SR-IOV

Reserve a feature bit for virtio devices which support SR-IOV.

Signed-off-by Tiwei Bie <[email protected]>

---
More details can be found from this thread:
https://patchwork.kernel.org/patch/10285541/

 content.tex | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

content.tex +12 -2

index 7a92cb1..62214fa 100644
@@ -95,10 +95,10 @@ Feature bits are allocated as follows:
 \begin{description}
 \item[0 to 23] Feature bits for the specific device type
 
-\item[24 to 33] Feature bits reserved for extensions to the queue and
+\item[24 to 36] Feature bits reserved for extensions to the queue and
   feature negotiation mechanisms
 
-\item[34 and above] Feature bits reserved for future extensions.
+\item[37 and above] Feature bits reserved for future extensions.
 \end{description}
 
 \begin{note}

@@ -5348,6 +5348,8 @@ Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Supp
   \item[VIRTIO_F_IN_ORDER(35)] This feature indicates
   that all buffers are used by the device in the same
   order in which they have been made available.
+  \item[VIRTIO_F_SR_IOV(36)] This feature indicates that
+  the device supports Single Root I/O Virtualization.
 \end{description}
 
 \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}

@@ -5363,6 +5365,10 @@ addresses to the device.
 
 A driver SHOULD accept VIRTIO_F_RING_PACKED if it is offered.
 
+A driver SHOULD accept VIRTIO_F_SR_IOV if it is offered.
+If VIRTIO_F_SR_IOV has been negotiated, a driver can
+access device's SR-IOV capability structure.
+
 \devicenormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
 
 A device MUST offer VIRTIO_F_VERSION_1.  A device MAY fail to operate further

@@ -5376,6 +5382,10 @@ accepted.
 If VIRTIO_F_IN_ORDER has been negotiated, a device MUST use
 buffers in the same order in which they have been available.
 
+A device SHOULD offer VIRTIO_F_SR_IOV if it presents a SR-IOV
+capability structure.  A device MAY fail to operate further
+if VIRTIO_F_SR_IOV is not accepted.
+
 \section{Legacy Interface: Reserved Feature Bits}\label{sec:Reserved Feature Bits / Legacy Interface: Reserved Feature Bits}
 
 Transitional devices MAY offer the following:

Signature · 6 lines

2.17.0


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Cornelia Huck

2018-05-18 08:14 UTC #1 lore raw

Quoted 1 line · On Fri, 18 May 2018 15:38:55 +0800 Tiwei Bie <[email protected]> wrote:

Reserve a feature bit for virtio devices which support SR-IOV.

Reserving a feature bit for this makes sense, but...

Quoted 28 lines

Signed-off-by Tiwei Bie <[email protected]>

---
More details can be found from this thread:
https://patchwork.kernel.org/patch/10285541/

 content.tex | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

content.tex +2 -2

index 7a92cb1..62214fa 100644
@@ -95,10 +95,10 @@ Feature bits are allocated as follows:
 \begin{description}
 \item[0 to 23] Feature bits for the specific device type
 
-\item[24 to 33] Feature bits reserved for extensions to the queue and
+\item[24 to 36] Feature bits reserved for extensions to the queue and
   feature negotiation mechanisms
 
-\item[34 and above] Feature bits reserved for future extensions.
+\item[37 and above] Feature bits reserved for future extensions.
 \end{description}
 
 \begin{note}

content.tex Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Supp · line 5352

Quoted 5 lines

content.tex +2 -0

@@ -5348,6 +5348,8 @@ Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Supp
   \item[VIRTIO_F_IN_ORDER(35)] This feature indicates
   that all buffers are used by the device in the same
   order in which they have been made available.
+  \item[VIRTIO_F_SR_IOV(36)] This feature indicates that
+  the device supports Single Root I/O Virtualization.

...shouldn't this mention 'PCI' somewhere? This feature does not make sense on any transport but PCI.

Quoted 15 lines

content.tex +4 -0

 \end{description}
 
 \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}

@@ -5363,6 +5365,10 @@ addresses to the device.
 
 A driver SHOULD accept VIRTIO_F_RING_PACKED if it is offered.
 
+A driver SHOULD accept VIRTIO_F_SR_IOV if it is offered.
+If VIRTIO_F_SR_IOV has been negotiated, a driver can
+access device's SR-IOV capability structure.
+
 \devicenormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
 
 A device MUST offer VIRTIO_F_VERSION_1.  A device MAY fail to operate further

content.tex accepted. · line 5391

Quoted 10 lines

content.tex +4 -0

@@ -5376,6 +5382,10 @@ accepted.
 If VIRTIO_F_IN_ORDER has been negotiated, a device MUST use
 buffers in the same order in which they have been available.
 
+A device SHOULD offer VIRTIO_F_SR_IOV if it presents a SR-IOV
+capability structure.  A device MAY fail to operate further
+if VIRTIO_F_SR_IOV is not accepted.
+
 \section{Legacy Interface: Reserved Feature Bits}\label{sec:Reserved Feature Bits / Legacy Interface: Reserved Feature Bits}
 
 Transitional devices MAY offer the following:

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Tiwei Bie

author

2018-05-18 08:42 UTC #2 lore raw

Quoted 33 lines · Cornelia Huck

Quoted 1 line · On Fri, 18 May 2018 15:38:55 +0800 Tiwei Bie <[email protected]> wrote:
Reserve a feature bit for virtio devices which support SR-IOV.
Reserving a feature bit for this makes sense, but...
Quoted 26 lines
Suggested-by Michael S. Tsirkin <[email protected]>
Signed-off-by Tiwei Bie <[email protected]>
---
More details can be found from this thread:
https://patchwork.kernel.org/patch/10285541/
 content.tex | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
content.tex +2 -2
index 7a92cb1..62214fa 100644
@@ -95,10 +95,10 @@ Feature bits are allocated as follows:
 \begin{description}
 \item[0 to 23] Feature bits for the specific device type
 
-\item[24 to 33] Feature bits reserved for extensions to the queue and
+\item[24 to 36] Feature bits reserved for extensions to the queue and
   feature negotiation mechanisms
 
-\item[34 and above] Feature bits reserved for future extensions.
+\item[37 and above] Feature bits reserved for future extensions.
 \end{description}
 

Quoted 10 lines

content.tex Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Supp · line 5352
Quoted 7 lines
content.tex +2 -0
 \begin{note}
@@ -5348,6 +5348,8 @@ Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Supp
   \item[VIRTIO_F_IN_ORDER(35)] This feature indicates
   that all buffers are used by the device in the same
   order in which they have been made available.
+  \item[VIRTIO_F_SR_IOV(36)] This feature indicates that
+  the device supports Single Root I/O Virtualization.
...shouldn't this mention 'PCI' somewhere? This feature does not make sense on any transport but PCI.

Yeah. When Michael suggested this feature, he also said:

https://patchwork.kernel.org/patch/10285541/
"""
It seems PCI specific so non pci transports would disable the feature for now.
"""

Based on the current description, transports other than PCI would have this feature bit disabled.

Do you think there is any possibility that other transports may reuse this feature bit to do something similar?

Or do you think we should make this feature bit PCI specific and don't leave any chance for other transports to reuse it, e.g. by naming it as VIRTIO_F_PCI_SR_IOV?

Or do you think it's OK to still name it as VIRTIO_F_SR_IOV, but add the word 'PCI' when talking about the capability, e.g.:

A device SHOULD offer VIRTIO_F_SR_IOV if it presents a PCI SR-IOV capability structure. A device MAY fail to operate further if VIRTIO_F_SR_IOV is not accepted.

Best regards,
Tiwei Bie

Quoted 1 line

[...]

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Cornelia Huck

2018-05-18 09:01 UTC #3 lore raw

Quoted 53 lines · On Fri, 18 May 2018 16:43:09 +0800 Tiwei Bie <[email protected]> wrote:

Quoted 33 lines · Cornelia Huck
Quoted 1 line · On Fri, 18 May 2018 15:38:55 +0800 Tiwei Bie <[email protected]> wrote:
Reserve a feature bit for virtio devices which support SR-IOV.
Reserving a feature bit for this makes sense, but...
Quoted 26 lines
Suggested-by Michael S. Tsirkin <[email protected]>
Signed-off-by Tiwei Bie <[email protected]>
---
More details can be found from this thread:
https://patchwork.kernel.org/patch/10285541/
 content.tex | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
content.tex +2 -2
index 7a92cb1..62214fa 100644
@@ -95,10 +95,10 @@ Feature bits are allocated as follows:
 \begin{description}
 \item[0 to 23] Feature bits for the specific device type
 
-\item[24 to 33] Feature bits reserved for extensions to the queue and
+\item[24 to 36] Feature bits reserved for extensions to the queue and
   feature negotiation mechanisms
 
-\item[34 and above] Feature bits reserved for future extensions.
+\item[37 and above] Feature bits reserved for future extensions.
 \end{description}
 
Quoted 10 lines
content.tex Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Supp · line 5352
Quoted 7 lines
content.tex +2 -0
 \begin{note}
@@ -5348,6 +5348,8 @@ Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Supp
   \item[VIRTIO_F_IN_ORDER(35)] This feature indicates
   that all buffers are used by the device in the same
   order in which they have been made available.
+  \item[VIRTIO_F_SR_IOV(36)] This feature indicates that
+  the device supports Single Root I/O Virtualization.  
...shouldn't this mention 'PCI' somewhere? This feature does not make sense on any transport but PCI.
Yeah. When Michael suggested this feature, he also said:
https://patchwork.kernel.org/patch/10285541/
"""
It seems PCI specific so non pci transports would disable the feature for now.
"""

Quoted 5 lines

Based on the current description, transports other than PCI would have this feature bit disabled.
Do you think there is any possibility that other transports may reuse this feature bit to do something similar?

I don't know about mmio (and I'm not sure how actively it is developed), but ccw is so different that I don't see how it could match to anything there.

Quoted 5 lines

Or do you think we should make this feature bit PCI specific and don't leave any chance for other transports to reuse it, e.g. by naming it as VIRTIO_F_PCI_SR_IOV?

Quoted 7 lines

Or do you think it's OK to still name it as VIRTIO_F_SR_IOV, but add the word 'PCI' when talking about the capability, e.g.:
A device SHOULD offer VIRTIO_F_SR_IOV if it presents a PCI SR-IOV capability structure. A device MAY fail to operate further if VIRTIO_F_SR_IOV is not accepted.

Adding 'PCI' to the description is probably best, I think.

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Tiwei Bie

author

2018-05-18 09:06 UTC #4 lore raw

Quoted 59 lines · Cornelia Huck

Quoted 56 lines · On Fri, 18 May 2018 16:43:09 +0800 Tiwei Bie <[email protected]> wrote:
Quoted 33 lines · Cornelia Huck
Quoted 1 line · On Fri, 18 May 2018 15:38:55 +0800 Tiwei Bie <[email protected]> wrote:
Reserve a feature bit for virtio devices which support SR-IOV.
Reserving a feature bit for this makes sense, but...
Quoted 26 lines
Suggested-by Michael S. Tsirkin <[email protected]>
Signed-off-by Tiwei Bie <[email protected]>
---
More details can be found from this thread:
https://patchwork.kernel.org/patch/10285541/
 content.tex | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
content.tex +2 -2
index 7a92cb1..62214fa 100644
@@ -95,10 +95,10 @@ Feature bits are allocated as follows:
 \begin{description}
 \item[0 to 23] Feature bits for the specific device type
 
-\item[24 to 33] Feature bits reserved for extensions to the queue and
+\item[24 to 36] Feature bits reserved for extensions to the queue and
   feature negotiation mechanisms
 
-\item[34 and above] Feature bits reserved for future extensions.
+\item[37 and above] Feature bits reserved for future extensions.
 \end{description}
 
Quoted 10 lines
content.tex Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Supp · line 5352
Quoted 7 lines
content.tex +2 -0
 \begin{note}
@@ -5348,6 +5348,8 @@ Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Supp
   \item[VIRTIO_F_IN_ORDER(35)] This feature indicates
   that all buffers are used by the device in the same
   order in which they have been made available.
+  \item[VIRTIO_F_SR_IOV(36)] This feature indicates that
+  the device supports Single Root I/O Virtualization.  
...shouldn't this mention 'PCI' somewhere? This feature does not make sense on any transport but PCI.
Yeah. When Michael suggested this feature, he also said:
https://patchwork.kernel.org/patch/10285541/
"""
It seems PCI specific so non pci transports would disable the feature for now.
"""
Based on the current description, transports other than PCI would have this feature bit disabled.

Quoted 6 lines

Quoted 2 lines
Do you think there is any possibility that other transports may reuse this feature bit to do something similar?
I don't know about mmio (and I'm not sure how actively it is developed), but ccw is so different that I don't see how it could match to anything there.

Got it. Thanks!

Quoted 10 lines

Quoted 10 lines
Or do you think we should make this feature bit PCI specific and don't leave any chance for other transports to reuse it, e.g. by naming it as VIRTIO_F_PCI_SR_IOV?
Or do you think it's OK to still name it as VIRTIO_F_SR_IOV, but add the word 'PCI' when talking about the capability, e.g.:

Quoted 5 lines

Quoted 3 lines
A device SHOULD offer VIRTIO_F_SR_IOV if it presents a PCI SR-IOV capability structure. A device MAY fail to operate further if VIRTIO_F_SR_IOV is not accepted.
Adding 'PCI' to the description is probably best, I think.

Got it. I'll do it in the next version. Thanks!

Best regards,
Tiwei Bie

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]