Discussion of the implementations of VIRTIO specification
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Christophe de Dinechin <cdupontd@redhat.com>
Cc: virtio@lists.oasis-open.org, virtio-dev@lists.oasis-open.org,
	virtio-comment@lists.oasis-open.org
Subject: [virtio] Re: [virtio-comment] [PATCH] content: add vendor specific cfg type
Date: Wed, 27 Nov 2019 16:17:29 -0500	[thread overview]
Message-ID: <20191127161428-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <F1C6DEC9-5E4D-464A-A0C3-D8E625D53F7E@redhat.com>

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 


      parent reply	other threads:[~2019-11-27 21:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-28 10:55 [virtio] [PATCH] content: add vendor specific cfg type Michael S. Tsirkin
2019-11-24 12:32 ` [virtio] " Michael S. Tsirkin
2019-11-25  7:45 ` [virtio] Re: [virtio-dev] " Jan Kiszka
2019-11-25  8:11   ` Michael S. Tsirkin
2019-11-27  7:59     ` Cornelia Huck
2019-11-27 10:59       ` Michael S. Tsirkin
2019-11-27 13:23         ` Michael S. Tsirkin
2019-11-27 14:43           ` Cornelia Huck
     [not found] ` <F1C6DEC9-5E4D-464A-A0C3-D8E625D53F7E@redhat.com>
2019-11-27 21:17   ` Michael S. Tsirkin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191127161428-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=cdupontd@redhat.com \
    --cc=virtio-comment@lists.oasis-open.org \
    --cc=virtio-dev@lists.oasis-open.org \
    --cc=virtio@lists.oasis-open.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox