[virtio] [PATCH] content: add vendor specific cfg type

[virtio] [PATCH] content: add vendor specific cfg type

[Michael S. Tsirkin]

Vendors might want to add their own capability
in the PCI capability list. However, Virtio already
uses the vendor specific capability ID (0x09)
for its own purposes.

Provide a structure for vendor specific extensions.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 content.tex | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/content.tex b/content.tex
index b1ea9b9..8a79b03 100644
--- a/content.tex
+++ b/content.tex
@@ -691,6 +691,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
 #define VIRTIO_PCI_CAP_PCI_CFG           5
 /* Shared memory region */
 #define VIRTIO_PCI_CAP_SHARED_MEMORY_CFG 8
+/* Vendor-specific data */
+#define VIRTIO_PCI_CAP_VENDOR_CFG        9
 \end{lstlisting}
 
         Any other value is reserved for future use.
@@ -1099,6 +1101,84 @@ \subsubsection{Shared memory capability}\label{sec:Virtio Transport Options / Vi
 
 The \field{cap.id} MUST be unique for any one device instance.
 
+\devicenormative{\paragraph}{Device-specific configuration}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Shared memory capability}
+
+The region defined by the combination of the \field {cap.offset},
+\field {cap.offset_hi}, and \field {cap.length}, \field
+{cap.length_hi} fields MUST be contained within the declared bar.
+
+The \field{cap.id} MUST be unique for any one device instance.
+
+\subsubsection{Vendor data capability}\label{sec:Virtio
+Transport Options / Virtio Over PCI Bus / PCI Device Layout /
+Vendor data capability}
+
+The optional Vendor data capability allows the device to present
+vendor-specific data to the driver, without
+conflicts, for debugging and/or reporting purposes,
+and without conflicting with standard functionality.
+
+This capability augments but does not replace the standard
+subsystem ID and subsystem vendor ID fields
+(offsets 0x2C and 0x2E in the PCI configuration space header)
+as specified by \hyperref[intro:PCI]{[PCI]}.
+
+Vendor data capability is enumerated on the PCI transport
+as a VIRTIO_PCI_CAP_VENDOR_CFG capability.
+
+The capability has the following structure:
+\begin{lstlisting}
+struct virtio_pci_vndr_data {
+        u8 cap_vndr;    /* Generic PCI field: PCI_CAP_ID_VNDR */
+        u8 cap_next;    /* Generic PCI field: next ptr. */
+        u8 cap_len;     /* Generic PCI field: capability length */
+        u8 cfg_type;    /* Identifies the structure. */
+        u16 vendor_id;  /* Identifies the vendor-specific format. */
+	/* For Vendor Definition */
+	/* Pads structure to a multiple of 4 bytes */
+	/* Reads must not have side effects */
+};
+\end{lstlisting}
+
+Where \field{vendor_id} identifies the PCI-SIG assigned Vendor ID
+as specified by \hyperref[intro:PCI]{[PCI]}.
+
+Note that the capability size is required to be a multiple of 4.
+
+To make it safe for a generic driver to access the capability,
+reads from this capability MUST NOT have any side effects.
+
+\devicenormative{\subsection}{Vendor data capability}{Virtio
+Transport Options / Virtio Over PCI Bus / PCI Device Layout /
+Vendor data capability}
+
+The device SHOULD present the PCI subsystem vendor ID matching
+the device vendor, at offset 0x2C in its PCI configuration space
+header.
+
+Devices CAN present \field{vendor_id} that does not match
+either the PCI Vendor ID or the PCI Subsystem Vendor ID.
+
+Devices CAN present multiple Vendor data capabilities with
+either different or identical \field{vendor_id} values.
+
+The value \field{vendor_id} MUST NOT equal 0x1AF4.
+
+The size of the Vendor data capability MUST be a multiple of 4 bytes.
+
+Reads of the Vendor data capability by the driver MUST NOT have any
+side effects.
+
+\drivernormative{\subsection}{Vendor data capability}{Virtio
+Transport Options / Virtio Over PCI Bus / PCI Device Layout /
+Vendor data capability}
+
+The driver SHOULD NOT use the Vendor data capability except
+for debugging and reporting purposes.
+
+The driver MUST qualify the \field{vendor_id} before
+interpreting or writing into the Vendor data capability.
+
 \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / PCI configuration access capability}
 
 The VIRTIO_PCI_CAP_PCI_CFG capability
-- 
MST


---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php

[virtio] Re: [PATCH] content: add vendor specific cfg type

[Michael S. Tsirkin]

On Mon, Oct 28, 2019 at 06:55:42AM -0400, Michael S. Tsirkin wrote:
> Vendors might want to add their own capability
> in the PCI capability list. However, Virtio already
> uses the vendor specific capability ID (0x09)
> for its own purposes.
> 
> Provide a structure for vendor specific extensions.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/62

> ---
>  content.tex | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index b1ea9b9..8a79b03 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -691,6 +691,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
>  #define VIRTIO_PCI_CAP_PCI_CFG           5
>  /* Shared memory region */
>  #define VIRTIO_PCI_CAP_SHARED_MEMORY_CFG 8
> +/* Vendor-specific data */
> +#define VIRTIO_PCI_CAP_VENDOR_CFG        9
>  \end{lstlisting}
>  
>          Any other value is reserved for future use.
> @@ -1099,6 +1101,84 @@ \subsubsection{Shared memory capability}\label{sec:Virtio Transport Options / Vi
>  
>  The \field{cap.id} MUST be unique for any one device instance.
>  
> +\devicenormative{\paragraph}{Device-specific configuration}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Shared memory capability}
> +
> +The region defined by the combination of the \field {cap.offset},
> +\field {cap.offset_hi}, and \field {cap.length}, \field
> +{cap.length_hi} fields MUST be contained within the declared bar.
> +
> +The \field{cap.id} MUST be unique for any one device instance.
> +
> +\subsubsection{Vendor data capability}\label{sec:Virtio
> +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> +Vendor data capability}
> +
> +The optional Vendor data capability allows the device to present
> +vendor-specific data to the driver, without
> +conflicts, for debugging and/or reporting purposes,
> +and without conflicting with standard functionality.
> +
> +This capability augments but does not replace the standard
> +subsystem ID and subsystem vendor ID fields
> +(offsets 0x2C and 0x2E in the PCI configuration space header)
> +as specified by \hyperref[intro:PCI]{[PCI]}.
> +
> +Vendor data capability is enumerated on the PCI transport
> +as a VIRTIO_PCI_CAP_VENDOR_CFG capability.
> +
> +The capability has the following structure:
> +\begin{lstlisting}
> +struct virtio_pci_vndr_data {
> +        u8 cap_vndr;    /* Generic PCI field: PCI_CAP_ID_VNDR */
> +        u8 cap_next;    /* Generic PCI field: next ptr. */
> +        u8 cap_len;     /* Generic PCI field: capability length */
> +        u8 cfg_type;    /* Identifies the structure. */
> +        u16 vendor_id;  /* Identifies the vendor-specific format. */
> +	/* For Vendor Definition */
> +	/* Pads structure to a multiple of 4 bytes */
> +	/* Reads must not have side effects */
> +};
> +\end{lstlisting}
> +
> +Where \field{vendor_id} identifies the PCI-SIG assigned Vendor ID
> +as specified by \hyperref[intro:PCI]{[PCI]}.
> +
> +Note that the capability size is required to be a multiple of 4.
> +
> +To make it safe for a generic driver to access the capability,
> +reads from this capability MUST NOT have any side effects.
> +
> +\devicenormative{\subsection}{Vendor data capability}{Virtio
> +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> +Vendor data capability}
> +
> +The device SHOULD present the PCI subsystem vendor ID matching
> +the device vendor, at offset 0x2C in its PCI configuration space
> +header.
> +
> +Devices CAN present \field{vendor_id} that does not match
> +either the PCI Vendor ID or the PCI Subsystem Vendor ID.
> +
> +Devices CAN present multiple Vendor data capabilities with
> +either different or identical \field{vendor_id} values.
> +
> +The value \field{vendor_id} MUST NOT equal 0x1AF4.
> +
> +The size of the Vendor data capability MUST be a multiple of 4 bytes.
> +
> +Reads of the Vendor data capability by the driver MUST NOT have any
> +side effects.
> +
> +\drivernormative{\subsection}{Vendor data capability}{Virtio
> +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> +Vendor data capability}
> +
> +The driver SHOULD NOT use the Vendor data capability except
> +for debugging and reporting purposes.
> +
> +The driver MUST qualify the \field{vendor_id} before
> +interpreting or writing into the Vendor data capability.
> +
>  \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / PCI configuration access capability}
>  
>  The VIRTIO_PCI_CAP_PCI_CFG capability
> -- 
> MST


---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that 
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php 

[virtio] Re: [virtio-dev] [PATCH] content: add vendor specific cfg type

[Jan Kiszka]

On 28.10.19 11:55, Michael S. Tsirkin wrote:
> Vendors might want to add their own capability
> in the PCI capability list. However, Virtio already
> uses the vendor specific capability ID (0x09)
> for its own purposes.

Did some vendor express that need, or do we only assume it so far? IOW: 
Do we know at least once concrete use case?

> 
> Provide a structure for vendor specific extensions.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   content.tex | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 80 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index b1ea9b9..8a79b03 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -691,6 +691,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
>   #define VIRTIO_PCI_CAP_PCI_CFG           5
>   /* Shared memory region */
>   #define VIRTIO_PCI_CAP_SHARED_MEMORY_CFG 8
> +/* Vendor-specific data */
> +#define VIRTIO_PCI_CAP_VENDOR_CFG        9
>   \end{lstlisting}
>   
>           Any other value is reserved for future use.
> @@ -1099,6 +1101,84 @@ \subsubsection{Shared memory capability}\label{sec:Virtio Transport Options / Vi
>   
>   The \field{cap.id} MUST be unique for any one device instance.
>   
> +\devicenormative{\paragraph}{Device-specific configuration}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Shared memory capability}
> +
> +The region defined by the combination of the \field {cap.offset},
> +\field {cap.offset_hi}, and \field {cap.length}, \field
> +{cap.length_hi} fields MUST be contained within the declared bar.
> +
> +The \field{cap.id} MUST be unique for any one device instance.
> +
> +\subsubsection{Vendor data capability}\label{sec:Virtio
> +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> +Vendor data capability}
> +
> +The optional Vendor data capability allows the device to present
> +vendor-specific data to the driver, without
> +conflicts, for debugging and/or reporting purposes,
> +and without conflicting with standard functionality.
> +
> +This capability augments but does not replace the standard
> +subsystem ID and subsystem vendor ID fields
> +(offsets 0x2C and 0x2E in the PCI configuration space header)
> +as specified by \hyperref[intro:PCI]{[PCI]}.
> +
> +Vendor data capability is enumerated on the PCI transport
> +as a VIRTIO_PCI_CAP_VENDOR_CFG capability.
> +
> +The capability has the following structure:
> +\begin{lstlisting}
> +struct virtio_pci_vndr_data {
> +        u8 cap_vndr;    /* Generic PCI field: PCI_CAP_ID_VNDR */
> +        u8 cap_next;    /* Generic PCI field: next ptr. */
> +        u8 cap_len;     /* Generic PCI field: capability length */
> +        u8 cfg_type;    /* Identifies the structure. */
> +        u16 vendor_id;  /* Identifies the vendor-specific format. */
> +	/* For Vendor Definition */
> +	/* Pads structure to a multiple of 4 bytes */
> +	/* Reads must not have side effects */
> +};
> +\end{lstlisting}
> +
> +Where \field{vendor_id} identifies the PCI-SIG assigned Vendor ID
> +as specified by \hyperref[intro:PCI]{[PCI]}.
> +
> +Note that the capability size is required to be a multiple of 4.
> +
> +To make it safe for a generic driver to access the capability,
> +reads from this capability MUST NOT have any side effects.
> +
> +\devicenormative{\subsection}{Vendor data capability}{Virtio
> +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> +Vendor data capability}
> +
> +The device SHOULD present the PCI subsystem vendor ID matching
> +the device vendor, at offset 0x2C in its PCI configuration space
> +header.
> +
> +Devices CAN present \field{vendor_id} that does not match
> +either the PCI Vendor ID or the PCI Subsystem Vendor ID.
> +
> +Devices CAN present multiple Vendor data capabilities with
> +either different or identical \field{vendor_id} values.
> +
> +The value \field{vendor_id} MUST NOT equal 0x1AF4.
> +
> +The size of the Vendor data capability MUST be a multiple of 4 bytes.
> +
> +Reads of the Vendor data capability by the driver MUST NOT have any
> +side effects.
> +
> +\drivernormative{\subsection}{Vendor data capability}{Virtio
> +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> +Vendor data capability}
> +
> +The driver SHOULD NOT use the Vendor data capability except
> +for debugging and reporting purposes.

Every vendor is free to mess up the device behavior by adding more to 
this than the spec defines. Still, I'm undecided if this shouldn't be 
stronger.

Jan

> +
> +The driver MUST qualify the \field{vendor_id} before
> +interpreting or writing into the Vendor data capability.
> +
>   \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / PCI configuration access capability}
>   
>   The VIRTIO_PCI_CAP_PCI_CFG capability
> 

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that 
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php 

[virtio] Re: [virtio-dev] [PATCH] content: add vendor specific cfg type

[Michael S. Tsirkin]

On Mon, Nov 25, 2019 at 08:45:52AM +0100, Jan Kiszka wrote:
> On 28.10.19 11:55, Michael S. Tsirkin wrote:
> > Vendors might want to add their own capability
> > in the PCI capability list. However, Virtio already
> > uses the vendor specific capability ID (0x09)
> > for its own purposes.
> 
> Did some vendor express that need, or do we only assume it so far? IOW: Do
> we know at least once concrete use case?

Good point, I should have added this in the log.

I know of a device that implements virtio and has a bug.

Device is a transitional one so can not have vendor specific
subsystem IDs (that violates the SHOULD below. Do we want to qualify
that this recommendation is for non-transitional devices?). While that
specific device can't be fixed like this on bare metal, it creates a consistent way
for future devices to trigger vendor/device specific quirks,
and it creates an option for the hypervisor to add the capability.

> > 
> > Provide a structure for vendor specific extensions.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >   content.tex | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 80 insertions(+)
> > 
> > diff --git a/content.tex b/content.tex
> > index b1ea9b9..8a79b03 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -691,6 +691,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
> >   #define VIRTIO_PCI_CAP_PCI_CFG           5
> >   /* Shared memory region */
> >   #define VIRTIO_PCI_CAP_SHARED_MEMORY_CFG 8
> > +/* Vendor-specific data */
> > +#define VIRTIO_PCI_CAP_VENDOR_CFG        9
> >   \end{lstlisting}
> >           Any other value is reserved for future use.
> > @@ -1099,6 +1101,84 @@ \subsubsection{Shared memory capability}\label{sec:Virtio Transport Options / Vi
> >   The \field{cap.id} MUST be unique for any one device instance.
> > +\devicenormative{\paragraph}{Device-specific configuration}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Shared memory capability}
> > +
> > +The region defined by the combination of the \field {cap.offset},
> > +\field {cap.offset_hi}, and \field {cap.length}, \field
> > +{cap.length_hi} fields MUST be contained within the declared bar.
> > +
> > +The \field{cap.id} MUST be unique for any one device instance.
> > +
> > +\subsubsection{Vendor data capability}\label{sec:Virtio
> > +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> > +Vendor data capability}
> > +
> > +The optional Vendor data capability allows the device to present
> > +vendor-specific data to the driver, without
> > +conflicts, for debugging and/or reporting purposes,
> > +and without conflicting with standard functionality.
> > +
> > +This capability augments but does not replace the standard
> > +subsystem ID and subsystem vendor ID fields
> > +(offsets 0x2C and 0x2E in the PCI configuration space header)
> > +as specified by \hyperref[intro:PCI]{[PCI]}.
> > +
> > +Vendor data capability is enumerated on the PCI transport
> > +as a VIRTIO_PCI_CAP_VENDOR_CFG capability.
> > +
> > +The capability has the following structure:
> > +\begin{lstlisting}
> > +struct virtio_pci_vndr_data {
> > +        u8 cap_vndr;    /* Generic PCI field: PCI_CAP_ID_VNDR */
> > +        u8 cap_next;    /* Generic PCI field: next ptr. */
> > +        u8 cap_len;     /* Generic PCI field: capability length */
> > +        u8 cfg_type;    /* Identifies the structure. */
> > +        u16 vendor_id;  /* Identifies the vendor-specific format. */
> > +	/* For Vendor Definition */
> > +	/* Pads structure to a multiple of 4 bytes */
> > +	/* Reads must not have side effects */
> > +};
> > +\end{lstlisting}
> > +
> > +Where \field{vendor_id} identifies the PCI-SIG assigned Vendor ID
> > +as specified by \hyperref[intro:PCI]{[PCI]}.
> > +
> > +Note that the capability size is required to be a multiple of 4.
> > +
> > +To make it safe for a generic driver to access the capability,
> > +reads from this capability MUST NOT have any side effects.
> > +
> > +\devicenormative{\subsection}{Vendor data capability}{Virtio
> > +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> > +Vendor data capability}
> > +
> > +The device SHOULD present the PCI subsystem vendor ID matching
> > +the device vendor, at offset 0x2C in its PCI configuration space
> > +header.
> > +
> > +Devices CAN present \field{vendor_id} that does not match
> > +either the PCI Vendor ID or the PCI Subsystem Vendor ID.
> > +
> > +Devices CAN present multiple Vendor data capabilities with
> > +either different or identical \field{vendor_id} values.
> > +
> > +The value \field{vendor_id} MUST NOT equal 0x1AF4.
> > +
> > +The size of the Vendor data capability MUST be a multiple of 4 bytes.
> > +
> > +Reads of the Vendor data capability by the driver MUST NOT have any
> > +side effects.
> > +
> > +\drivernormative{\subsection}{Vendor data capability}{Virtio
> > +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> > +Vendor data capability}
> > +
> > +The driver SHOULD NOT use the Vendor data capability except
> > +for debugging and reporting purposes.
> 
> Every vendor is free to mess up the device behavior by adding more to this
> than the spec defines. Still, I'm undecided if this shouldn't be stronger.
> 
> Jan
> 
> > +
> > +The driver MUST qualify the \field{vendor_id} before
> > +interpreting or writing into the Vendor data capability.
> > +
> >   \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / PCI configuration access capability}
> >   The VIRTIO_PCI_CAP_PCI_CFG capability
> > 
> 
> -- 
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux


---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that 
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php 

Re: [virtio] Re: [virtio-dev] [PATCH] content: add vendor specific cfg type

[Cornelia Huck]

On Mon, 25 Nov 2019 03:11:43 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Nov 25, 2019 at 08:45:52AM +0100, Jan Kiszka wrote:
> > On 28.10.19 11:55, Michael S. Tsirkin wrote:  
> > > Vendors might want to add their own capability
> > > in the PCI capability list. However, Virtio already
> > > uses the vendor specific capability ID (0x09)
> > > for its own purposes.  
> > 
> > Did some vendor express that need, or do we only assume it so far? IOW: Do
> > we know at least once concrete use case?  
> 
> Good point, I should have added this in the log.
> 
> I know of a device that implements virtio and has a bug.
> 
> Device is a transitional one so can not have vendor specific
> subsystem IDs (that violates the SHOULD below. Do we want to qualify
> that this recommendation is for non-transitional devices?). While that

What about adding some notes on top that vendor specific subsystem IDs
are not for transitional devices? That would also catch the case in the
other update.

> specific device can't be fixed like this on bare metal, it creates a consistent way
> for future devices to trigger vendor/device specific quirks,
> and it creates an option for the hypervisor to add the capability.
> 
> > > 
> > > Provide a structure for vendor specific extensions.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >   content.tex | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 80 insertions(+)

> > > +\devicenormative{\subsection}{Vendor data capability}{Virtio
> > > +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> > > +Vendor data capability}
> > > +
> > > +The device SHOULD present the PCI subsystem vendor ID matching
> > > +the device vendor, at offset 0x2C in its PCI configuration space
> > > +header.
> > > +
> > > +Devices CAN present \field{vendor_id} that does not match
> > > +either the PCI Vendor ID or the PCI Subsystem Vendor ID.
> > > +
> > > +Devices CAN present multiple Vendor data capabilities with
> > > +either different or identical \field{vendor_id} values.
> > > +
> > > +The value \field{vendor_id} MUST NOT equal 0x1AF4.
> > > +
> > > +The size of the Vendor data capability MUST be a multiple of 4 bytes.
> > > +
> > > +Reads of the Vendor data capability by the driver MUST NOT have any
> > > +side effects.


---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that 
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php 

Re: [virtio] Re: [virtio-dev] [PATCH] content: add vendor specific cfg type

[Michael S. Tsirkin]

On Wed, Nov 27, 2019 at 08:59:56AM +0100, Cornelia Huck wrote:
> On Mon, 25 Nov 2019 03:11:43 -0500
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Mon, Nov 25, 2019 at 08:45:52AM +0100, Jan Kiszka wrote:
> > > On 28.10.19 11:55, Michael S. Tsirkin wrote:  
> > > > Vendors might want to add their own capability
> > > > in the PCI capability list. However, Virtio already
> > > > uses the vendor specific capability ID (0x09)
> > > > for its own purposes.  
> > > 
> > > Did some vendor express that need, or do we only assume it so far? IOW: Do
> > > we know at least once concrete use case?  
> > 
> > Good point, I should have added this in the log.
> > 
> > I know of a device that implements virtio and has a bug.
> > 
> > Device is a transitional one so can not have vendor specific
> > subsystem IDs (that violates the SHOULD below. Do we want to qualify
> > that this recommendation is for non-transitional devices?). While that
> 
> What about adding some notes on top that vendor specific subsystem IDs
> are not for transitional devices? That would also catch the case in the
> other update.

I'm still trying to figure out a good wording though.
A full description would need to describe the legacy
tricks with subsystem device ID etc. That's a lot of
text. Certainly doable but
maybe I'll just drop the subsystem vendor ID part
for now, treat that as a different subject?

> > specific device can't be fixed like this on bare metal, it creates a consistent way
> > for future devices to trigger vendor/device specific quirks,
> > and it creates an option for the hypervisor to add the capability.
> > 
> > > > 
> > > > Provide a structure for vendor specific extensions.
> > > > 
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > >   content.tex | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >   1 file changed, 80 insertions(+)
> 
> > > > +\devicenormative{\subsection}{Vendor data capability}{Virtio
> > > > +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> > > > +Vendor data capability}
> > > > +
> > > > +The device SHOULD present the PCI subsystem vendor ID matching
> > > > +the device vendor, at offset 0x2C in its PCI configuration space
> > > > +header.
> > > > +
> > > > +Devices CAN present \field{vendor_id} that does not match
> > > > +either the PCI Vendor ID or the PCI Subsystem Vendor ID.
> > > > +
> > > > +Devices CAN present multiple Vendor data capabilities with
> > > > +either different or identical \field{vendor_id} values.
> > > > +
> > > > +The value \field{vendor_id} MUST NOT equal 0x1AF4.
> > > > +
> > > > +The size of the Vendor data capability MUST be a multiple of 4 bytes.
> > > > +
> > > > +Reads of the Vendor data capability by the driver MUST NOT have any
> > > > +side effects.


---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that 
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php 

Re: [virtio] Re: [virtio-dev] [PATCH] content: add vendor specific cfg type

[Michael S. Tsirkin]

On Wed, Nov 27, 2019 at 05:59:03AM -0500, Michael S. Tsirkin wrote:
> On Wed, Nov 27, 2019 at 08:59:56AM +0100, Cornelia Huck wrote:
> > On Mon, 25 Nov 2019 03:11:43 -0500
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > On Mon, Nov 25, 2019 at 08:45:52AM +0100, Jan Kiszka wrote:
> > > > On 28.10.19 11:55, Michael S. Tsirkin wrote:  
> > > > > Vendors might want to add their own capability
> > > > > in the PCI capability list. However, Virtio already
> > > > > uses the vendor specific capability ID (0x09)
> > > > > for its own purposes.  
> > > > 
> > > > Did some vendor express that need, or do we only assume it so far? IOW: Do
> > > > we know at least once concrete use case?  
> > > 
> > > Good point, I should have added this in the log.
> > > 
> > > I know of a device that implements virtio and has a bug.
> > > 
> > > Device is a transitional one so can not have vendor specific
> > > subsystem IDs (that violates the SHOULD below. Do we want to qualify
> > > that this recommendation is for non-transitional devices?). While that
> > 
> > What about adding some notes on top that vendor specific subsystem IDs
> > are not for transitional devices? That would also catch the case in the
> > other update.


So how about I just skip the following chunk when I commit?

	+The device SHOULD present the PCI subsystem vendor ID matching
	+the device vendor, at offset 0x2C in its PCI configuration space
	+header.

that seems to belong to the other ballot that I have withdrawn.

Minor enough that I don't feel we need to redo the whole ballot,
right?.

-- 
MST


---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that 
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php 

Re: [virtio] Re: [virtio-dev] [PATCH] content: add vendor specific cfg type

[Cornelia Huck]

On Wed, 27 Nov 2019 08:23:44 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Nov 27, 2019 at 05:59:03AM -0500, Michael S. Tsirkin wrote:
> > On Wed, Nov 27, 2019 at 08:59:56AM +0100, Cornelia Huck wrote:  
> > > On Mon, 25 Nov 2019 03:11:43 -0500
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >   
> > > > On Mon, Nov 25, 2019 at 08:45:52AM +0100, Jan Kiszka wrote:  
> > > > > On 28.10.19 11:55, Michael S. Tsirkin wrote:    
> > > > > > Vendors might want to add their own capability
> > > > > > in the PCI capability list. However, Virtio already
> > > > > > uses the vendor specific capability ID (0x09)
> > > > > > for its own purposes.    
> > > > > 
> > > > > Did some vendor express that need, or do we only assume it so far? IOW: Do
> > > > > we know at least once concrete use case?    
> > > > 
> > > > Good point, I should have added this in the log.
> > > > 
> > > > I know of a device that implements virtio and has a bug.
> > > > 
> > > > Device is a transitional one so can not have vendor specific
> > > > subsystem IDs (that violates the SHOULD below. Do we want to qualify
> > > > that this recommendation is for non-transitional devices?). While that  
> > > 
> > > What about adding some notes on top that vendor specific subsystem IDs
> > > are not for transitional devices? That would also catch the case in the
> > > other update.  
> 
> 
> So how about I just skip the following chunk when I commit?
> 
> 	+The device SHOULD present the PCI subsystem vendor ID matching
> 	+the device vendor, at offset 0x2C in its PCI configuration space
> 	+header.
> 
> that seems to belong to the other ballot that I have withdrawn.
> 
> Minor enough that I don't feel we need to redo the whole ballot,
> right?.
> 

I think that's fine.


---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that 
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php 

[virtio] Re: [virtio-comment] [PATCH] content: add vendor specific cfg type

[Michael S. Tsirkin]

On Wed, Nov 27, 2019 at 03:54:17PM +0100, Christophe de Dinechin wrote:
> 
> 
> > On 28 Oct 2019, at 11:55, Michael S. Tsirkin <mst@redhat.com> wrote:
> > 
> > Vendors might want to add their own capability
> > in the PCI capability list. However, Virtio already
> > uses the vendor specific capability ID (0x09)
> > for its own purposes.
> > 
> > Provide a structure for vendor specific extensions.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > content.tex | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 80 insertions(+)
> > 
> > diff --git a/content.tex b/content.tex
> > index b1ea9b9..8a79b03 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -691,6 +691,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
> > #define VIRTIO_PCI_CAP_PCI_CFG           5
> > /* Shared memory region */
> > #define VIRTIO_PCI_CAP_SHARED_MEMORY_CFG 8
> > +/* Vendor-specific data */
> > +#define VIRTIO_PCI_CAP_VENDOR_CFG        9
> > \end{lstlisting}
> > 
> >         Any other value is reserved for future use.
> > @@ -1099,6 +1101,84 @@ \subsubsection{Shared memory capability}\label{sec:Virtio Transport Options / Vi
> > 
> > The \field{cap.id} MUST be unique for any one device instance.
> > 
> > +\devicenormative{\paragraph}{Device-specific configuration}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Shared memory capability}
> > +
> > +The region defined by the combination of the \field {cap.offset},
> > +\field {cap.offset_hi}, and \field {cap.length}, \field
> > +{cap.length_hi} fields MUST be contained within the declared bar.
> 
> Uppercase BAR?

\field{bar} I think.

But really this is from "shared memory: Define PCI capability"
that I somehow failed to apply properly :(

> > +
> > +The \field{cap.id} MUST be unique for any one device instance.
> > +
> > +\subsubsection{Vendor data capability}\label{sec:Virtio
> > +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> > +Vendor data capability}
> > +
> > +The optional Vendor data capability allows the device to present
> > +vendor-specific data to the driver, without
> > +conflicts, for debugging and/or reporting purposes,
> > +and without conflicting with standard functionality.
> > +
> > +This capability augments but does not replace the standard
> > +subsystem ID and subsystem vendor ID fields
> > +(offsets 0x2C and 0x2E in the PCI configuration space header)
> > +as specified by \hyperref[intro:PCI]{[PCI]}.
> > +
> > +Vendor data capability is enumerated on the PCI transport
> > +as a VIRTIO_PCI_CAP_VENDOR_CFG capability.
> > +
> > +The capability has the following structure:
> > +\begin{lstlisting}
> > +struct virtio_pci_vndr_data {
> > +        u8 cap_vndr;    /* Generic PCI field: PCI_CAP_ID_VNDR */
> > +        u8 cap_next;    /* Generic PCI field: next ptr. */
> > +        u8 cap_len;     /* Generic PCI field: capability length */
> > +        u8 cfg_type;    /* Identifies the structure. */
> > +        u16 vendor_id;  /* Identifies the vendor-specific format. */
> > +	/* For Vendor Definition */
> > +	/* Pads structure to a multiple of 4 bytes */
> > +	/* Reads must not have side effects */
> > +};
> > +\end{lstlisting}
> > +
> > +Where \field{vendor_id} identifies the PCI-SIG assigned Vendor ID
> > +as specified by \hyperref[intro:PCI]{[PCI]}.
> > +
> > +Note that the capability size is required to be a multiple of 4.
> > +
> > +To make it safe for a generic driver to access the capability,
> > +reads from this capability MUST NOT have any side effects.
> 
> I think the intent is that this includes the capability payload, not
> just the generic fields. Worth spelling out?
> 
> > +
> > +\devicenormative{\subsection}{Vendor data capability}{Virtio
> > +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> > +Vendor data capability}
> > +
> > +The device SHOULD present the PCI subsystem vendor ID matching
> > +the device vendor, at offset 0x2C in its PCI configuration space
> > +header.
> > +
> > +Devices CAN present \field{vendor_id} that does not match
> > +either the PCI Vendor ID or the PCI Subsystem Vendor ID.
> > +
> > +Devices CAN present multiple Vendor data capabilities with
> > +either different or identical \field{vendor_id} values.
> > +
> > +The value \field{vendor_id} MUST NOT equal 0x1AF4.
> > +
> > +The size of the Vendor data capability MUST be a multiple of 4 bytes.
> > +
> > +Reads of the Vendor data capability by the driver MUST NOT have any
> > +side effects.
> > +
> > +\drivernormative{\subsection}{Vendor data capability}{Virtio
> > +Transport Options / Virtio Over PCI Bus / PCI Device Layout /
> > +Vendor data capability}
> > +
> > +The driver SHOULD NOT use the Vendor data capability except
> > +for debugging and reporting purposes.
> 
> What is the rationale for this restriction?
> 
> For example, would it be OK for a vendor to expose the presence
> of a particular firmware bugfix, so that the driver could disable
> some bug workaround? You seem to hint that this is the intent
> in a later response, but on initial reading, it was not clear to me
> that such usage would fit under “debugging” or “reporting”.
> 
> (I’m quite new here, so feel free to ignore if it’s clear to everybody else)
> 
> 
> > +
> > +The driver MUST qualify the \field{vendor_id} before
> > +interpreting or writing into the Vendor data capability.
> > +
> > \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / PCI configuration access capability}
> > 
> > The VIRTIO_PCI_CAP_PCI_CFG capability
> > -- 
> > MST
> > 
> > 
> > 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://lists.oasis-open.org/archives/virtio-comment/
> > Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> > List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> > Committee: https://www.oasis-open.org/committees/virtio/
> > Join OASIS: https://www.oasis-open.org/join/
> > 


---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that 
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php