[virtio-dev] [PATCH 0/1] Allow "Vendor Specific" extension to virtio PCI capabilities.

[virtio-dev] [PATCH 0/1] Allow "Vendor Specific" extension to virtio PCI capabilities.

[Venu Busireddy]

Section 4.1.4 of the virtio specification, while describing the values
for cfg_type field of "virtio PCI capabilities" says, "Any other
value is reserved for future use."  Currently, values 1 through 5 are
defined. Every other value is reserved.

It would be useful if the virtio specification provides a virtio PCI
capability that is reserved for any vendor specific extension, thus
allowing the vendors to define any vendor specific extension to virtio
PCI capabilities.

That will be similar to what the PCI specification has done. While
reserving all Capability IDs from 12h through 0FFh, it also provides a
"Vendor Specific" capability ID (0x09), to allow the device vendors to
use the capability mechanism for vendor specific information.

There are alternative ways to achieve the same objective, but the
implementation is much cleaner with a new virtio PCI capability.


Venu Busireddy (1):
  Allow "Vendor Specific" extension to virtio PCI capabilities.

 content.tex | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH 1/1] Allow "Vendor Specific" extension to virtio PCI capabilities.

[Venu Busireddy]

Add VIRTIO_PCI_CAP_VENDOR_EXT_CFG to the virtio PCI capabilities to
allow vendors to define vendor specific extension.

Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com>
---
 content.tex | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/content.tex b/content.tex
index 7a92cb1..406bfe6 100644
--- a/content.tex
+++ b/content.tex
@@ -599,6 +599,8 @@ The fields are interpreted as follows:
 #define VIRTIO_PCI_CAP_DEVICE_CFG        4
 /* PCI configuration access */
 #define VIRTIO_PCI_CAP_PCI_CFG           5
+/* Vendor Specific Extension */
+#define VIRTIO_PCI_CAP_VENDOR_EXT_CFG    6
 \end{lstlisting}
 
         Any other value is reserved for future use.
@@ -997,6 +999,47 @@ address \field{cap.length} bytes within a BAR range
 specified by some other Virtio Structure PCI Capability
 of type other than \field{VIRTIO_PCI_CAP_PCI_CFG}.
 
+\subsubsection{Vendor Specific Extension capability}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Vendor Specific Extension capability}
+
+The VIRTIO_PCI_CAP_VENDOR_EXT_CFG capability
+provides means for vendors to define any vendor specific extension to the virtio specification.
+
+The capability is immediately followed by an arbitrary amount of data as below:
+
+\begin{lstlisting}
+struct virtio_pci_vendor_ext_cap {
+        struct virtio_pci_cap cap;
+        u8 vendor_specific_data[]; /* Vendor specific data. */
+};
+\end{lstlisting}
+
+The fields \field{cap.bar}, \field{cap.length}, \field{cap.offset} and
+\field{vendor_specific_data} are read-only for the driver.
+
+The specification does not impose any restrictions on the size or
+structure of vendor_specific_data[]. Vendors are free to declare this
+array as large as needed, as long as the combined size of all capabilities
+can be accommodated within the PCI configuration space.
+
+If there is enough room in the PCI configuration space to accommodate
+the vendor specific data, the fields \field{cap.bar}, \field{cap.offset}
+and \field{cap.length} should be set to 0.
+
+If there isn't enough room, some or all of the data can be presented
+in the BAR region, in which case the fields \field{cap.bar},
+\field{cap.offset} and \field{cap.length} should be set appropriately.
+
+In either case, the field \field{cap.cap_len} indicates the length of
+the data present in the configuration space itself.
+
+\devicenormative{\paragraph}{Vendor Specific Extension capability}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Vendor Specific Extension capability}
+
+The device may or may not present the VIRTIO_PCI_CAP_VENDOR_EXT_CFG capability.
+
+\drivernormative{\paragraph}{Vendor Specific Extension capability}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Vendor Specific Extension capability}
+
+The driver MUST NOT write to vendor_specific_data[] area or the BAR region.
+
 \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
 
 Transitional devices MUST present part of configuration

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH 1/1] Allow "Vendor Specific" extension to virtio PCI capabilities.

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1158 bytes --]

On Thu, May 17, 2018 at 09:06:04AM -0400, Venu Busireddy wrote:

Can you describe a use case where vendor-specific extensions make sense
as opposed to extending the VIRTIO specification?

This proposal only supports the PCI transport, so do extensions only
affect the PCI transport behavior or also other aspects of VIRTIO
(device behavior, vring, etc)?

> +\devicenormative{\paragraph}{Vendor Specific Extension capability}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Vendor Specific Extension capability}
> +
> +The device may or may not present the VIRTIO_PCI_CAP_VENDOR_EXT_CFG capability.

s/may or may not/MAY/

> +
> +\drivernormative{\paragraph}{Vendor Specific Extension capability}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Vendor Specific Extension capability}
> +
> +The driver MUST NOT write to vendor_specific_data[] area or the BAR region.

How does a driver identify vendor-specific extensions that it supports?
No Vendor ID is included in the capability, just an opaque blob, so how
are collisions avoided?

How does a driver behave upon encountering an unknown vendor-specific
extension?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [virtio-dev] [PATCH 1/1] Allow "Vendor Specific" extension to virtio PCI capabilities.

[Venu Busireddy]

On 2018-05-18 17:06:23 +0100, Stefan Hajnoczi wrote:
> On Thu, May 17, 2018 at 09:06:04AM -0400, Venu Busireddy wrote:
> 
> Can you describe a use case where vendor-specific extensions make sense
> as opposed to extending the VIRTIO specification?

Sometimes, qemu may need to group certain devices together. In our
problem scenario, qemu needed to tell the driver which virtio device is a
fallback device for a given vfio-pci device. If the virtio device's PCI
capabilities are extended, the new capability could be used to store a
unique ID (say, the bus:device:function) that identifies the vfio-pci
device. The driver can use that information when needed to pair the
virtio device with the vfio-pci device.

Extending the VIRTIO specification is also a good alternative, but
every time the vendor needs something new, the specification needs to
be changed again. A generic "vendor specific" mechanism alleviates that.

> This proposal only supports the PCI transport, so do extensions only
> affect the PCI transport behavior or also other aspects of VIRTIO
> (device behavior, vring, etc)?

The proposal is only for PCI transport. It does not affect other
transports.

> > +\devicenormative{\paragraph}{Vendor Specific Extension capability}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Vendor Specific Extension capability}
> > +
> > +The device may or may not present the VIRTIO_PCI_CAP_VENDOR_EXT_CFG capability.
> 
> s/may or may not/MAY/

Will change that, along with any other suggestions, in v2.

> > +
> > +\drivernormative{\paragraph}{Vendor Specific Extension capability}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Vendor Specific Extension capability}
> > +
> > +The driver MUST NOT write to vendor_specific_data[] area or the BAR region.
> 
> How does a driver identify vendor-specific extensions that it supports?
> No Vendor ID is included in the capability, just an opaque blob, so how
> are collisions avoided?

Sorry for not making this clear! Since the capability is vendor specific,
the driver also needs to have corresponding vendor specific changes. The
driver needs to look at the Subsystem ID and Subsystem Vendor ID of the
virtio device, and interpret the data associated with the new capability
only when the IDs match with the IDs built into the driver. Vendors are
free to implement their own versioning scheme into that data to maintain
backward/forward compatibility.

> How does a driver behave upon encountering an unknown vendor-specific
> extension?

Drivers should ignore the data associated with the new capability if
the Subsystem ID and Subsystem Vendor ID do not match with the IDs built
into the driver. And, the specification already mandates that the drivers
should ignore VIRTIO_PCI_CAP_VENDOR_EXT_CFG if they don't know about it.

Venu


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH 1/1] Allow "Vendor Specific" extension to virtio PCI capabilities.

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]

On Fri, May 18, 2018 at 04:43:36PM -0500, Venu Busireddy wrote:
> On 2018-05-18 17:06:23 +0100, Stefan Hajnoczi wrote:
> > On Thu, May 17, 2018 at 09:06:04AM -0400, Venu Busireddy wrote:
> > 
> > Can you describe a use case where vendor-specific extensions make sense
> > as opposed to extending the VIRTIO specification?
> 
> Sometimes, qemu may need to group certain devices together. In our
> problem scenario, qemu needed to tell the driver which virtio device is a
> fallback device for a given vfio-pci device. If the virtio device's PCI
> capabilities are extended, the new capability could be used to store a
> unique ID (say, the bus:device:function) that identifies the vfio-pci
> device. The driver can use that information when needed to pair the
> virtio device with the vfio-pci device.

This feature seems generic and could be in the VIRTIO spec proper.  What
is vendor-specific about it?

> Extending the VIRTIO specification is also a good alternative, but
> every time the vendor needs something new, the specification needs to
> be changed again. A generic "vendor specific" mechanism alleviates that.

If you plan to push Linux code changes upstream to implement this
feature then a vendor-specific extension is inappropriate and you should
extend the VIRTIO spec.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [virtio-dev] [PATCH 1/1] Allow "Vendor Specific" extension to virtio PCI capabilities.

[Venu Busireddy]

On 2018-05-22 11:03:37 +0100, Stefan Hajnoczi wrote:
> On Fri, May 18, 2018 at 04:43:36PM -0500, Venu Busireddy wrote:
> > On 2018-05-18 17:06:23 +0100, Stefan Hajnoczi wrote:
> > > On Thu, May 17, 2018 at 09:06:04AM -0400, Venu Busireddy wrote:
> > > 
> > > Can you describe a use case where vendor-specific extensions make sense
> > > as opposed to extending the VIRTIO specification?
> > 
> > Sometimes, qemu may need to group certain devices together. In our
> > problem scenario, qemu needed to tell the driver which virtio device is a
> > fallback device for a given vfio-pci device. If the virtio device's PCI
> > capabilities are extended, the new capability could be used to store a
> > unique ID (say, the bus:device:function) that identifies the vfio-pci
> > device. The driver can use that information when needed to pair the
> > virtio device with the vfio-pci device.
> 
> This feature seems generic and could be in the VIRTIO spec proper.  What
> is vendor-specific about it?
> 
> > Extending the VIRTIO specification is also a good alternative, but
> > every time the vendor needs something new, the specification needs to
> > be changed again. A generic "vendor specific" mechanism alleviates that.
> 
> If you plan to push Linux code changes upstream to implement this
> feature then a vendor-specific extension is inappropriate and you should
> extend the VIRTIO spec.

I will redo the patch to extend the virtio specification.

Since the new patch will be conceptually different, shall I create a
new thread, or should I post it as v2?

Venu


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH 1/1] Allow "Vendor Specific" extension to virtio PCI capabilities.

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1761 bytes --]

On Tue, May 22, 2018 at 08:48:58AM -0500, Venu Busireddy wrote:
> On 2018-05-22 11:03:37 +0100, Stefan Hajnoczi wrote:
> > On Fri, May 18, 2018 at 04:43:36PM -0500, Venu Busireddy wrote:
> > > On 2018-05-18 17:06:23 +0100, Stefan Hajnoczi wrote:
> > > > On Thu, May 17, 2018 at 09:06:04AM -0400, Venu Busireddy wrote:
> > > > 
> > > > Can you describe a use case where vendor-specific extensions make sense
> > > > as opposed to extending the VIRTIO specification?
> > > 
> > > Sometimes, qemu may need to group certain devices together. In our
> > > problem scenario, qemu needed to tell the driver which virtio device is a
> > > fallback device for a given vfio-pci device. If the virtio device's PCI
> > > capabilities are extended, the new capability could be used to store a
> > > unique ID (say, the bus:device:function) that identifies the vfio-pci
> > > device. The driver can use that information when needed to pair the
> > > virtio device with the vfio-pci device.
> > 
> > This feature seems generic and could be in the VIRTIO spec proper.  What
> > is vendor-specific about it?
> > 
> > > Extending the VIRTIO specification is also a good alternative, but
> > > every time the vendor needs something new, the specification needs to
> > > be changed again. A generic "vendor specific" mechanism alleviates that.
> > 
> > If you plan to push Linux code changes upstream to implement this
> > feature then a vendor-specific extension is inappropriate and you should
> > extend the VIRTIO spec.
> 
> I will redo the patch to extend the virtio specification.
> 
> Since the new patch will be conceptually different, shall I create a
> new thread, or should I post it as v2?

Please post a new patch.  Thanks!

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]