[virtio-comment] [PATCH v5 0/5] Large shared memory regions

[virtio-comment] [PATCH v5 0/5] Large shared memory regions

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

  This series formalises the idea of shared memory regions I'd
previously discussed and is intended for use with virtio-fs.

I've split it into five parts:

  a) A general definition of the idea

  b) An addition of an 'id' field to allow multiple distinct
     capabilities of the same type.

  c) An addition of a 64bit capability type.

  d) A definition on virtio-pci using the layout previously
discussed.

  e) A definition on virtio-mmio.

v5:
  Based on Michael's comments, the 'id' field is now moved into
the main virtio_pci_cap, and the 64 bit cpaaibility is now a more
general type.
  Minor other rewordings.

Dr. David Alan Gilbert (5):
  shared memory: Define shared memory regions
  pci: Define id field
  pci: Define virtio_pci_cap64
  shared memory: Define PCI capability
  shared memory: Define mmio registers

 conformance.tex |  1 +
 content.tex     | 77 ++++++++++++++++++++++++++++++++++++++++++++++++-
 shared-mem.tex  | 42 +++++++++++++++++++++++++++
 3 files changed, 119 insertions(+), 1 deletion(-)
 create mode 100644 shared-mem.tex

-- 
2.21.0


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/

[virtio-comment] [PATCH v5 1/5] shared memory: Define shared memory regions

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Define the requirements and idea behind shared memory regions.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 conformance.tex |  1 +
 content.tex     |  2 ++
 shared-mem.tex  | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+)
 create mode 100644 shared-mem.tex

diff --git a/conformance.tex b/conformance.tex
index 42f702a..4524237 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -197,6 +197,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Available Buffer Notification Suppression}
+\item \ref{devicenormative:Basic Facilities of a Virtio Device / Shared Memory Regions}
 \item \ref{devicenormative:Reserved Feature Bits}
 \end{itemize}
 
diff --git a/content.tex b/content.tex
index 8f0498e..6433226 100644
--- a/content.tex
+++ b/content.tex
@@ -371,6 +371,8 @@ \subsection{Driver notifications} \label{sec:Virtqueues / Driver notifications}
 has been negotiated, these notifications would then have
 identical \field{next_off} and \field{next_wrap} values.
 
+\input{shared-mem.tex}
+
 \chapter{General Initialization And Device Operation}\label{sec:General Initialization And Device Operation}
 
 We start with an overview of device initialization, then expand on the
diff --git a/shared-mem.tex b/shared-mem.tex
new file mode 100644
index 0000000..6e6f6c4
--- /dev/null
+++ b/shared-mem.tex
@@ -0,0 +1,42 @@
+\section{Shared Memory Regions}\label{sec:Basic Facilities of a Virtio Device / Shared Memory Regions}
+
+Shared memory regions are an additional facility
+available to devices that need a region of memory that's
+continuously shared between the device and the driver, rather
+than passed between them in the way virtqueue elements are.
+
+Example uses include shared caches and version pools for versioned
+data structures.
+
+The memory region is allocated by the device and presented to the
+driver.  Where the device is implemented in software on a host,
+this arrangement allows the memory region to be allocated by
+a library on the host, which the device may not have full control
+over.
+
+A device may have multiple shared memory regions associated with
+it.  Each region has a \field{shmid} to identify it, the meaning
+of which is device-specific.
+
+Enumeration and location of shared memory regions is performed
+in a transport-specific way.
+
+Memory consistency rules vary depending on the region and the
+device and they will be specified as required by each device.
+
+\subsection{Addressing within regions}\label{sec:Basic Facilities of a Virtio Device / Shared Memory Regions / Addressing within regions }
+
+References into shared memory regions are represented as offsets from
+the beginning of the region instead of absolute memory addresses.
+Offsets are used both for references between structures stored
+within shared memory and for requests placed in virtqueues that
+refer to shared memory.
+The \field{shmid} may be explicit or may be inferred from the
+context of the reference.
+
+\devicenormative{\subsection}{Shared Memory Regions}{Basic Facilities of a Virtio
+Device / Shared Memory Regions}
+Shared memory regions MUST NOT expose shared memory regions which
+are used to control the operation of the device, nor to stream
+data.
+
-- 
2.21.0


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/

[virtio-comment] [PATCH v5 2/5] pci: Define id field

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

For the virtio-fs device we require multiple large shared memory
regions.  Differentiate these by an 'id' field in the base capability.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 content.tex | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/content.tex b/content.tex
index 6433226..41926c0 100644
--- a/content.tex
+++ b/content.tex
@@ -651,7 +651,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
         u8 cap_len;     /* Generic PCI field: capability length */
         u8 cfg_type;    /* Identifies the structure. */
         u8 bar;         /* Where to find it. */
-        u8 padding[3];  /* Pad to full dword. */
+        u8 id;          /* Multiple capabilities of the same type */
+        u8 padding[2];  /* Pad to full dword. */
         le32 offset;    /* Offset within bar. */
         le32 length;    /* Length of the structure, in bytes. */
 };
@@ -716,6 +717,11 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
 
         Any other value is reserved for future use.
 
+\item[\field{id}]
+        Multiple capabilities of the same type can exist as long
+        as they each have a unique \field{id}.  The specific
+        meaning of the field is different for each device type.
+
 \item[\field{offset}]
         indicates where the structure begins relative to the base address associated
         with the BAR.  The alignment requirements of \field{offset} are indicated
-- 
2.21.0


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/

[virtio-comment] [PATCH v5 3/5] pci: Define virtio_pci_cap64

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Define 'virtio_pci_cap64' to allow capabilities to describe
memory regions larger than, or with an offset larger than 4GiB.

This will be used by the shared memory region capability.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 content.tex | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/content.tex b/content.tex
index 41926c0..63e9179 100644
--- a/content.tex
+++ b/content.tex
@@ -744,6 +744,23 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
         \end{note}
 \end{description}
 
+A variant of this type, struct virtio_pci_cap64, is defined for
+those capaibilites that require offsets or lengths larger than
+4GiB:
+
+\begin{lstlisting}
+struct virtio_pci_cap64 {
+        struct virtio_pci_cap cap;
+        u32 offset_hi;
+        u32 length_hi;
+};
+\end{lstlisting}
+
+Given that the \field{cap.length} and \field{cap.offset} fields
+are only 32 bit, the additional \field{offset_hi} and \field {length_hi}
+fields provide the most significant 32 bits of a total 64 bit offset and
+length within the bar specified by \field{cap.bar}.
+
 \drivernormative{\subsubsection}{Virtio Structure PCI Capabilities}{Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities}
 
 The driver MUST ignore any vendor-specific capability structure which has
-- 
2.21.0


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/

[virtio-comment] [PATCH v5 4/5] shared memory: Define PCI capability

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Define the PCI capability used for enumerating shared memory regions.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 content.tex | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/content.tex b/content.tex
index 63e9179..fcf0d81 100644
--- a/content.tex
+++ b/content.tex
@@ -689,6 +689,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
 #define VIRTIO_PCI_CAP_DEVICE_CFG        4
 /* PCI configuration access */
 #define VIRTIO_PCI_CAP_PCI_CFG           5
+/* Shared memory region */
+#define VIRTIO_PCI_CAP_SHARED_MEMORY_CFG 8
 \end{lstlisting}
 
         Any other value is reserved for future use.
@@ -1075,6 +1077,24 @@ \subsubsection{Device-specific configuration}\label{sec:Virtio Transport Options
 
 The \field{offset} for the device-specific configuration MUST be 4-byte aligned.
 
+\subsubsection{Shared memory capability}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Shared memory capability}
+
+Shared memory regions \ref{sec:Basic Facilities of a Virtio
+Device / Shared Memory Regions} are enumerated on the PCI transport
+as a sequence of VIRTIO_PCI_CAP_SHARED_MEMORY_CFG capabilities, one per region.
+
+The capability is defined by a struct virtio_pci_cap64 and
+utilises the \field{cap.id} to allow multiple shared memory
+regions per device.
+
+\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{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
-- 
2.21.0


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/

[virtio-comment] [PATCH v5 5/5] shared memory: Define mmio registers

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Define an MMIO interface to discover and map shared
memory regions.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 content.tex | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/content.tex b/content.tex
index fcf0d81..588b299 100644
--- a/content.tex
+++ b/content.tex
@@ -1732,6 +1732,33 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
     selected by writing to \field{QueueSel}.
   }
   \hline 
+  \mmioreg{SHMSel}{Shared memory id}{0x0ac}{W}{%
+    Writing to this register selects the shared memory region \ref{sec:Basic Facilities of a Virtio Device / Shared Memory Regions}
+    following operations on \field{SHMLenLow}, \field{SHMLenHigh},
+    \field{SHMBaseLow} and \field{SHMBaseHigh} apply to.
+  }
+  \hline 
+  \mmiodreg{SHMLenLow}{SHMLenHigh}{Shared memory region 64 bit long length}{0x0b0}{0x0xb4}{R}{%
+    These registers return the length of the shared memory
+    region in bytes, as defined by the device for the region selected by
+    the \field{SHMSel} register.  The lower 32 bits of the length
+    are read from \field{SHMLenLow} and the higher 32 bits from
+    \field{SHMLenHigh}.  Reading from a non-existent
+    region (i.e. where the ID written to \field{SHMSel} is unused)
+    results in a length of -1.
+  }
+  \hline 
+  \mmiodreg{SHMBaseLow}{SHMBaseHigh}{Shared memory region 64 bit long physical address}{0x0b8}{0x0xbc}{R}{%
+    The driver reads these registers to discover the base address
+    of the region in physical address space.  This address is
+    chosen by the device (or other part of the VMM).
+    The lower 32 bits of the address are read from \field{SHMBaseLow}
+    with the higher 32 bits from \field{SHMBaseHigh}.  Reading
+    from a non-existent region (i.e. where the ID written to
+    \field{SHMSel} is unused) results in a base address of
+    0xffffffffffffffff.
+  }
+  \hline 
   \mmioreg{ConfigGeneration}{Configuration atomicity value}{0x0fc}{R}{
     Reading from this register returns a value describing a version of the device-specific configuration space (see \field{Config}).
     The driver can then access the configuration space and, when finished, read \field{ConfigGeneration} again.
@@ -1821,6 +1848,9 @@ \subsubsection{Device Initialization}\label{sec:Virtio Transport Options / Virti
 and if its value is zero (0x0) MUST abort initialization and
 MUST NOT access any other register.
 
+Drivers not expecting shared memory MUST NOT use the shared
+memory registers.
+
 Further initialization MUST follow the procedure described in
 \ref{sec:General Initialization And Device Operation / Device Initialization}~\nameref{sec:General Initialization And Device Operation / Device Initialization}.
 
-- 
2.21.0


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/

[virtio-comment] Re: [PATCH v5 2/5] pci: Define id field

[Cornelia Huck]

On Thu, 27 Jun 2019 20:28:27 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> For the virtio-fs device we require multiple large shared memory
> regions.  Differentiate these by an 'id' field in the base capability.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  content.tex | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/content.tex b/content.tex
> index 6433226..41926c0 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -651,7 +651,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
>          u8 cap_len;     /* Generic PCI field: capability length */
>          u8 cfg_type;    /* Identifies the structure. */
>          u8 bar;         /* Where to find it. */
> -        u8 padding[3];  /* Pad to full dword. */
> +        u8 id;          /* Multiple capabilities of the same type */
> +        u8 padding[2];  /* Pad to full dword. */
>          le32 offset;    /* Offset within bar. */
>          le32 length;    /* Length of the structure, in bytes. */
>  };
> @@ -716,6 +717,11 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
>  
>          Any other value is reserved for future use.
>  
> +\item[\field{id}]
> +        Multiple capabilities of the same type can exist as long
> +        as they each have a unique \field{id}.  The specific

The requirement for id is new, isn't it? Shouldn't it rather be
optional?

> +        meaning of the field is different for each device type.
> +
>  \item[\field{offset}]
>          indicates where the structure begins relative to the base address associated
>          with the BAR.  The alignment requirements of \field{offset} are indicated

The current specification for cfg_type defines a kind of hierarchy for
capabilities of the same type (first one is preferred). We probably
need to be more explicit how id may interact with that (even if it is
device type specific).

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/

[virtio-comment] Re: [PATCH v5 3/5] pci: Define virtio_pci_cap64

[Cornelia Huck]

On Thu, 27 Jun 2019 20:28:28 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Define 'virtio_pci_cap64' to allow capabilities to describe
> memory regions larger than, or with an offset larger than 4GiB.
> 
> This will be used by the shared memory region capability.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  content.tex | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

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/

[virtio-comment] Re: [virtio-dev] Re: [PATCH v5 2/5] pci: Define id field

[Dr. David Alan Gilbert]

* Cornelia Huck (cohuck@redhat.com) wrote:
> On Thu, 27 Jun 2019 20:28:27 +0100
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> 
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > For the virtio-fs device we require multiple large shared memory
> > regions.  Differentiate these by an 'id' field in the base capability.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  content.tex | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/content.tex b/content.tex
> > index 6433226..41926c0 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -651,7 +651,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
> >          u8 cap_len;     /* Generic PCI field: capability length */
> >          u8 cfg_type;    /* Identifies the structure. */
> >          u8 bar;         /* Where to find it. */
> > -        u8 padding[3];  /* Pad to full dword. */
> > +        u8 id;          /* Multiple capabilities of the same type */
> > +        u8 padding[2];  /* Pad to full dword. */
> >          le32 offset;    /* Offset within bar. */
> >          le32 length;    /* Length of the structure, in bytes. */
> >  };
> > @@ -716,6 +717,11 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
> >  
> >          Any other value is reserved for future use.
> >  
> > +\item[\field{id}]
> > +        Multiple capabilities of the same type can exist as long
> > +        as they each have a unique \field{id}.  The specific
> 
> The requirement for id is new, isn't it? Shouldn't it rather be
> optional?

Yes it is; this comes from moving it from the new structure we had it in
the previous version, into the main _cap.

Can I just say that only some devices use it, or do I need something
more?   Can I assume that 'id' is 0 for existing devices or is it
undefined?

> > +        meaning of the field is different for each device type.
> > +
> >  \item[\field{offset}]
> >          indicates where the structure begins relative to the base address associated
> >          with the BAR.  The alignment requirements of \field{offset} are indicated
> 
> The current specification for cfg_type defines a kind of hierarchy for
> capabilities of the same type (first one is preferred). We probably
> need to be more explicit how id may interact with that (even if it is
> device type specific).

One way I was thinking was to say that the hierarchy now applies to
pairs of {cfg_type, id} rather than just cfg_type; but that depends on
the answer to the previous question to avoid breaking any existing
hierarchies.

Dave



> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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/

[virtio-comment] Re: [virtio-dev] Re: [PATCH v5 2/5] pci: Define id field

[Cornelia Huck]

On Fri, 28 Jun 2019 14:36:14 +0100
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * Cornelia Huck (cohuck@redhat.com) wrote:
> > On Thu, 27 Jun 2019 20:28:27 +0100
> > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> >   
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > 
> > > For the virtio-fs device we require multiple large shared memory
> > > regions.  Differentiate these by an 'id' field in the base capability.
> > > 
> > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > ---
> > >  content.tex | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/content.tex b/content.tex
> > > index 6433226..41926c0 100644
> > > --- a/content.tex
> > > +++ b/content.tex
> > > @@ -651,7 +651,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
> > >          u8 cap_len;     /* Generic PCI field: capability length */
> > >          u8 cfg_type;    /* Identifies the structure. */
> > >          u8 bar;         /* Where to find it. */
> > > -        u8 padding[3];  /* Pad to full dword. */
> > > +        u8 id;          /* Multiple capabilities of the same type */
> > > +        u8 padding[2];  /* Pad to full dword. */
> > >          le32 offset;    /* Offset within bar. */
> > >          le32 length;    /* Length of the structure, in bytes. */
> > >  };
> > > @@ -716,6 +717,11 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
> > >  
> > >          Any other value is reserved for future use.
> > >  
> > > +\item[\field{id}]
> > > +        Multiple capabilities of the same type can exist as long
> > > +        as they each have a unique \field{id}.  The specific  
> > 
> > The requirement for id is new, isn't it? Shouldn't it rather be
> > optional?  
> 
> Yes it is; this comes from moving it from the new structure we had it in
> the previous version, into the main _cap.
> 
> Can I just say that only some devices use it, or do I need something
> more?   Can I assume that 'id' is 0 for existing devices or is it
> undefined?

Maybe something like the following?

"Used by some device types to uniquely identify multiple capabilities
of a certain type. If the device type does not specify the meaning of
this field, its contents are undefined."

We should not make any assumptions here, I think.

> 
> > > +        meaning of the field is different for each device type.
> > > +
> > >  \item[\field{offset}]
> > >          indicates where the structure begins relative to the base address associated
> > >          with the BAR.  The alignment requirements of \field{offset} are indicated  
> > 
> > The current specification for cfg_type defines a kind of hierarchy for
> > capabilities of the same type (first one is preferred). We probably
> > need to be more explicit how id may interact with that (even if it is
> > device type specific).  
> 
> One way I was thinking was to say that the hierarchy now applies to
> pairs of {cfg_type, id} rather than just cfg_type; but that depends on
> the answer to the previous question to avoid breaking any existing
> hierarchies.

If we make the meaning of id device type specific, we also need to make
its effects on the ordering device type specific.

What about keeping the hierarchy, but specifying that a device type may
override it if it specifies usage of the id field? Each device type
using id would need to explicitly specify the ordering in that case.

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/

[virtio-comment] Re: [PATCH v5 1/5] shared memory: Define shared memory regions

[Cornelia Huck]

On Thu, 27 Jun 2019 20:28:26 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Define the requirements and idea behind shared memory regions.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  conformance.tex |  1 +
>  content.tex     |  2 ++
>  shared-mem.tex  | 42 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 45 insertions(+)
>  create mode 100644 shared-mem.tex

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

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/

[virtio-comment] Re: [PATCH v5 4/5] shared memory: Define PCI capability

[Cornelia Huck]

On Thu, 27 Jun 2019 20:28:29 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Define the PCI capability used for enumerating shared memory regions.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  content.tex | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index 63e9179..fcf0d81 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -689,6 +689,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
>  #define VIRTIO_PCI_CAP_DEVICE_CFG        4
>  /* PCI configuration access */
>  #define VIRTIO_PCI_CAP_PCI_CFG           5
> +/* Shared memory region */
> +#define VIRTIO_PCI_CAP_SHARED_MEMORY_CFG 8
>  \end{lstlisting}
>  
>          Any other value is reserved for future use.
> @@ -1075,6 +1077,24 @@ \subsubsection{Device-specific configuration}\label{sec:Virtio Transport Options
>  
>  The \field{offset} for the device-specific configuration MUST be 4-byte aligned.
>  
> +\subsubsection{Shared memory capability}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Shared memory capability}
> +
> +Shared memory regions \ref{sec:Basic Facilities of a Virtio
> +Device / Shared Memory Regions} are enumerated on the PCI transport
> +as a sequence of VIRTIO_PCI_CAP_SHARED_MEMORY_CFG capabilities, one per region.
> +
> +The capability is defined by a struct virtio_pci_cap64 and
> +utilises the \field{cap.id} to allow multiple shared memory
> +regions per device.

Maybe add a sentence here

"The identifier in \field{cap.id} does not denote a certain order of
preference; it is only used to uniquely identify a region."

(as per discussion of patch 2)

> +
> +\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{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

Else, looks good to me.

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/

[virtio-comment] Re: [PATCH v5 5/5] shared memory: Define mmio registers

[Cornelia Huck]

On Thu, 27 Jun 2019 20:28:30 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Define an MMIO interface to discover and map shared
> memory regions.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  content.tex | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

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/

[virtio-comment] Re: [virtio-dev] Re: [PATCH v5 2/5] pci: Define id field

[Dr. David Alan Gilbert]

* Cornelia Huck (cohuck@redhat.com) wrote:
> On Fri, 28 Jun 2019 14:36:14 +0100
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> 
> > * Cornelia Huck (cohuck@redhat.com) wrote:
> > > On Thu, 27 Jun 2019 20:28:27 +0100
> > > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> > >   
> > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > > 
> > > > For the virtio-fs device we require multiple large shared memory
> > > > regions.  Differentiate these by an 'id' field in the base capability.
> > > > 
> > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > > ---
> > > >  content.tex | 8 +++++++-
> > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/content.tex b/content.tex
> > > > index 6433226..41926c0 100644
> > > > --- a/content.tex
> > > > +++ b/content.tex
> > > > @@ -651,7 +651,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
> > > >          u8 cap_len;     /* Generic PCI field: capability length */
> > > >          u8 cfg_type;    /* Identifies the structure. */
> > > >          u8 bar;         /* Where to find it. */
> > > > -        u8 padding[3];  /* Pad to full dword. */
> > > > +        u8 id;          /* Multiple capabilities of the same type */
> > > > +        u8 padding[2];  /* Pad to full dword. */
> > > >          le32 offset;    /* Offset within bar. */
> > > >          le32 length;    /* Length of the structure, in bytes. */
> > > >  };
> > > > @@ -716,6 +717,11 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
> > > >  
> > > >          Any other value is reserved for future use.
> > > >  
> > > > +\item[\field{id}]
> > > > +        Multiple capabilities of the same type can exist as long
> > > > +        as they each have a unique \field{id}.  The specific  
> > > 
> > > The requirement for id is new, isn't it? Shouldn't it rather be
> > > optional?  
> > 
> > Yes it is; this comes from moving it from the new structure we had it in
> > the previous version, into the main _cap.
> > 
> > Can I just say that only some devices use it, or do I need something
> > more?   Can I assume that 'id' is 0 for existing devices or is it
> > undefined?
> 
> Maybe something like the following?
> 
> "Used by some device types to uniquely identify multiple capabilities
> of a certain type. If the device type does not specify the meaning of
> this field, its contents are undefined."
> 

Thanks; slotted that in.

> We should not make any assumptions here, I think.

> > 
> > > > +        meaning of the field is different for each device type.
> > > > +
> > > >  \item[\field{offset}]
> > > >          indicates where the structure begins relative to the base address associated
> > > >          with the BAR.  The alignment requirements of \field{offset} are indicated  
> > > 
> > > The current specification for cfg_type defines a kind of hierarchy for
> > > capabilities of the same type (first one is preferred). We probably
> > > need to be more explicit how id may interact with that (even if it is
> > > device type specific).  
> > 
> > One way I was thinking was to say that the hierarchy now applies to
> > pairs of {cfg_type, id} rather than just cfg_type; but that depends on
> > the answer to the previous question to avoid breaking any existing
> > hierarchies.
> 
> If we make the meaning of id device type specific, we also need to make
> its effects on the ordering device type specific.
> 
> What about keeping the hierarchy, but specifying that a device type may
> override it if it specifies usage of the id field? Each device type
> using id would need to explicitly specify the ordering in that case.

OK, I've gone with:
         The device MAY offer more than one structure of any type - this makes it
         possible for the device to expose multiple interfaces to drivers.  The order of
         the capabilities in the capability list specifies the order of preference
-        suggested by the device.
+        suggested by the device.  A device may specify that this ordering mechanism be
+        overridden by the use of the \field{id} field.
         \begin{note}

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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/

[virtio-comment] Re: [PATCH v5 4/5] shared memory: Define PCI capability

[Dr. David Alan Gilbert]

* Cornelia Huck (cohuck@redhat.com) wrote:
> On Thu, 27 Jun 2019 20:28:29 +0100
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> 
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Define the PCI capability used for enumerating shared memory regions.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  content.tex | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/content.tex b/content.tex
> > index 63e9179..fcf0d81 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -689,6 +689,8 @@ \subsection{Virtio Structure PCI Capabilities}\label{sec:Virtio Transport Option
> >  #define VIRTIO_PCI_CAP_DEVICE_CFG        4
> >  /* PCI configuration access */
> >  #define VIRTIO_PCI_CAP_PCI_CFG           5
> > +/* Shared memory region */
> > +#define VIRTIO_PCI_CAP_SHARED_MEMORY_CFG 8
> >  \end{lstlisting}
> >  
> >          Any other value is reserved for future use.
> > @@ -1075,6 +1077,24 @@ \subsubsection{Device-specific configuration}\label{sec:Virtio Transport Options
> >  
> >  The \field{offset} for the device-specific configuration MUST be 4-byte aligned.
> >  
> > +\subsubsection{Shared memory capability}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Shared memory capability}
> > +
> > +Shared memory regions \ref{sec:Basic Facilities of a Virtio
> > +Device / Shared Memory Regions} are enumerated on the PCI transport
> > +as a sequence of VIRTIO_PCI_CAP_SHARED_MEMORY_CFG capabilities, one per region.
> > +
> > +The capability is defined by a struct virtio_pci_cap64 and
> > +utilises the \field{cap.id} to allow multiple shared memory
> > +regions per device.
> 
> Maybe add a sentence here
> 
> "The identifier in \field{cap.id} does not denote a certain order of
> preference; it is only used to uniquely identify a region."
> 
> (as per discussion of patch 2)

Added.

> > +
> > +\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{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
> 
> Else, looks good to me.
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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/