[virtio-comment] [PATCH v3 0/3] Large shared memory regions

[virtio-comment] [PATCH v3 0/3] 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 three parts:

  a) A general definition of the idea

  b) A definition on virtio-pci using the layout previously
discussed; this still uses the list of capabilities.

  c) A definition on virtio-mmio.

This v3 addresses most feedback on v2.  The MMIO registers have
changed a bit based on feedback (in particular from Sebastien
who has been trying it out).

Dave

Dr. David Alan Gilbert (3):
  shared memory: Define shared memory regions
  shared memory: Define PCI capability
  shared memory: Define mmio registers

 conformance.tex |  1 +
 content.tex     | 69 +++++++++++++++++++++++++++++++++++++++++++++++++
 shared-mem.tex  | 40 ++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 shared-mem.tex

-- 
2.20.1


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 v3 1/3] 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>
---
 conformance.tex |  1 +
 content.tex     |  2 ++
 shared-mem.tex  | 40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+)
 create mode 100644 shared-mem.tex

diff --git a/conformance.tex b/conformance.tex
index ad7e82e..f6c0d50 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -195,6 +195,7 @@ A device MUST conform to the following normative statements:
 \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 836ee52..3dd504c 100644
--- a/content.tex
+++ b/content.tex
@@ -371,6 +371,8 @@ making any more buffers available. When VIRTIO_F_NOTIFICATION_DATA
 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..86b0050
--- /dev/null
+++ b/shared-mem.tex
@@ -0,0 +1,40 @@
+\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 host and the guest, rather
+than passed between them in the way virtqueue elements are.
+
+Example uses include shared caches and version pools for versioned
+data structures.
+
+The region is chosen by the host and presented to the guest, as
+such it is useful in situations where the memory is accessed on
+the host by other libraries that can't safely access guest RAM.
+
+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
+using a transport-specific data structure and mechanism.
+
+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 }
+
+Commands sent over the virtqueues may refer to data within the
+shared memory regions, for example a command may be used by a
+driver to cause a device to add or remove a mapping within
+a region.  When referring to data, the addresses will normally be
+offsets within a particular region rather than absolute host or
+guest addresses.  The \field{shmid} may be explicit or may be
+inferred from the command type.
+
+\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.20.1


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 v3 2/3] 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>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 content.tex | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/content.tex b/content.tex
index 3dd504c..9838208 100644
--- a/content.tex
+++ b/content.tex
@@ -688,6 +688,8 @@ The fields are interpreted as follows:
 #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.
@@ -1052,6 +1054,41 @@ any device type which has a device-specific configuration.
 
 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 immediately followed by an additional field like so:
+
+\begin{lstlisting}
+struct virtio_pci_shm_cap {
+        struct virtio_pci_cap cap;
+        u32 offset_hi;
+        u32 length_hi;
+        u8  id;
+};
+\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}.  \field{id}
+uniquely identifies the given shared memory region using a device
+specific identifier.
+
+While most capabilities can not be repeated, multiple
+VIRTIO_PCI_CAP_SHARED_MEMORY_CFG capabilities are allowed as long
+as the \field{id} is unique.
+
+\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 offsets and length
+fields MUST be contained within the declared bar.
+
+The \field{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.20.1


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 v3 3/3] 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>
---
 content.tex | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/content.tex b/content.tex
index 9838208..7696337 100644
--- a/content.tex
+++ b/content.tex
@@ -1726,6 +1726,33 @@ All register values are organized as Little Endian.
     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.
@@ -1815,6 +1842,9 @@ If both values are valid, it MUST read \field{DeviceID}
 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.20.1


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 v3 1/3] shared memory: Define shared memory regions

[Cornelia Huck]

On Mon,  4 Mar 2019 13:25:29 +0000
"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>
> ---
>  conformance.tex |  1 +
>  content.tex     |  2 ++
>  shared-mem.tex  | 40 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 43 insertions(+)
>  create mode 100644 shared-mem.tex
> 
(...)
> diff --git a/shared-mem.tex b/shared-mem.tex
> new file mode 100644
> index 0000000..86b0050
> --- /dev/null
> +++ b/shared-mem.tex
> @@ -0,0 +1,40 @@
> +\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 host and the guest, rather
> +than passed between them in the way virtqueue elements are.
> +
> +Example uses include shared caches and version pools for versioned
> +data structures.
> +
> +The region is chosen by the host and presented to the guest, as
> +such it is useful in situations where the memory is accessed on
> +the host by other libraries that can't safely access guest RAM.
> +
> +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
> +using a transport-specific data structure and mechanism.
> +
> +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 }
> +
> +Commands sent over the virtqueues may refer to data within the
> +shared memory regions, for example a command may be used by a
> +driver to cause a device to add or remove a mapping within
> +a region.  When referring to data, the addresses will normally be
> +offsets within a particular region rather than absolute host or
> +guest addresses.  The \field{shmid} may be explicit or may be
> +inferred from the command type.

It's probably reasonable to frame it like that instead of making it a
normative statement.

Maybe "the addresses are expected to be offsets"? It would feel odd if
most devices handled it like that and then an oddball device came
along...

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

Otherwise, 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/

Re: [virtio-comment] [PATCH v3 1/3] shared memory: Define shared memory regions

[Stefan Hajnoczi]

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

On Mon, Mar 04, 2019 at 01:25:29PM +0000, Dr. David Alan Gilbert (git) wrote:
> +\subsection{Addressing within regions}\label{sec:Basic Facilities of a Virtio Device / Shared Memory Regions / Addressing within regions }
> +
> +Commands sent over the virtqueues may refer to data within the
> +shared memory regions, for example a command may be used by a
> +driver to cause a device to add or remove a mapping within
> +a region.  When referring to data, the addresses will normally be
> +offsets within a particular region rather than absolute host or
> +guest addresses.  The \field{shmid} may be explicit or may be
> +inferred from the command type.

This sounds like normative statements.  They belong in a driver or
device normative section.

Otherwise:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

[virtio-comment] Re: [PATCH v3 2/3] shared memory: Define PCI capability

[Stefan Hajnoczi]

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

On Mon, Mar 04, 2019 at 01:25:30PM +0000, Dr. David Alan Gilbert (git) wrote:
> +\begin{lstlisting}
> +struct virtio_pci_shm_cap {
> +        struct virtio_pci_cap cap;
> +        u32 offset_hi;
> +        u32 length_hi;
> +        u8  id;
> +};
> +\end{lstlisting}

Are there any alignment requirements for PCI capability structures?
This struct ends with a u8, which might be a problem if 32-bit or 64-bit
alignemnt is required by something.

Otherwise:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

[virtio-comment] Re: [PATCH v3 3/3] shared memory: Define mmio registers

[Stefan Hajnoczi]

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

On Mon, Mar 04, 2019 at 01:25:31PM +0000, Dr. David Alan Gilbert (git) 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>
> ---
>  content.tex | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [virtio-comment] [PATCH v3 1/3] shared memory: Define shared memory regions

[Dr. David Alan Gilbert]

* Stefan Hajnoczi (stefanha@redhat.com) wrote:
> On Mon, Mar 04, 2019 at 01:25:29PM +0000, Dr. David Alan Gilbert (git) wrote:
> > +\subsection{Addressing within regions}\label{sec:Basic Facilities of a Virtio Device / Shared Memory Regions / Addressing within regions }
> > +
> > +Commands sent over the virtqueues may refer to data within the
> > +shared memory regions, for example a command may be used by a
> > +driver to cause a device to add or remove a mapping within
> > +a region.  When referring to data, the addresses will normally be
> > +offsets within a particular region rather than absolute host or
> > +guest addresses.  The \field{shmid} may be explicit or may be
> > +inferred from the command type.
> 
> This sounds like normative statements.  They belong in a driver or
> device normative section.

I think we were trying in a previous version to make it one; but the
problem is it's not really a statement about something a device or
driver must do; it's a statement advising someone writing the spec
for a particular device to make sure it's specified like that.

> Otherwise:
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Thanks.

Dave


--
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 v3 2/3] shared memory: Define PCI capability

[Dr. David Alan Gilbert]

* Stefan Hajnoczi (stefanha@redhat.com) wrote:
> On Mon, Mar 04, 2019 at 01:25:30PM +0000, Dr. David Alan Gilbert (git) wrote:
> > +\begin{lstlisting}
> > +struct virtio_pci_shm_cap {
> > +        struct virtio_pci_cap cap;
> > +        u32 offset_hi;
> > +        u32 length_hi;
> > +        u8  id;
> > +};
> > +\end{lstlisting}
> 
> Are there any alignment requirements for PCI capability structures?

The PCI spec says 'Each capability must be DWORD aligned. '

> This struct ends with a u8, which might be a problem if 32-bit or 64-bit
> alignemnt is required by something.

It doesn't seem to be unusual, for example the MSI capability structure
seems to be 10 or 14 bytes even with the DWORD alignement restriction
for the start of it.


> Otherwise:
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Thanks,

Dave


--
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/

Re: [virtio-comment] [PATCH v3 1/3] shared memory: Define shared memory regions

[Stefan Hajnoczi]

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

On Thu, Mar 07, 2019 at 05:53:48PM +0000, Dr. David Alan Gilbert wrote:
> * Stefan Hajnoczi (stefanha@redhat.com) wrote:
> > On Mon, Mar 04, 2019 at 01:25:29PM +0000, Dr. David Alan Gilbert (git) wrote:
> > > +\subsection{Addressing within regions}\label{sec:Basic Facilities of a Virtio Device / Shared Memory Regions / Addressing within regions }
> > > +
> > > +Commands sent over the virtqueues may refer to data within the
> > > +shared memory regions, for example a command may be used by a
> > > +driver to cause a device to add or remove a mapping within
> > > +a region.  When referring to data, the addresses will normally be
> > > +offsets within a particular region rather than absolute host or
> > > +guest addresses.  The \field{shmid} may be explicit or may be
> > > +inferred from the command type.
> > 
> > This sounds like normative statements.  They belong in a driver or
> > device normative section.
> 
> I think we were trying in a previous version to make it one; but the
> problem is it's not really a statement about something a device or
> driver must do; it's a statement advising someone writing the spec
> for a particular device to make sure it's specified like that.

I see.  An idea for phrasing this in descriptive language:

  References into shared memory regions are represented as offsets from
  the beginning of the region instead of absolute memory addresses since
  absolute memory addresses are transport- and/or
  architecture-dependent.  Offsets are used both for references between
  structures stored within shared memory and for requests placed in
  virtqueues that refer to shared memory.

Feel free to discard this if you prefer the current wording.

Stefan

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

[virtio-comment] Re: [PATCH v3 2/3] shared memory: Define PCI capability

[Stefan Hajnoczi]

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

On Thu, Mar 07, 2019 at 06:24:57PM +0000, Dr. David Alan Gilbert wrote:
> * Stefan Hajnoczi (stefanha@redhat.com) wrote:
> > On Mon, Mar 04, 2019 at 01:25:30PM +0000, Dr. David Alan Gilbert (git) wrote:
> > > +\begin{lstlisting}
> > > +struct virtio_pci_shm_cap {
> > > +        struct virtio_pci_cap cap;
> > > +        u32 offset_hi;
> > > +        u32 length_hi;
> > > +        u8  id;
> > > +};
> > > +\end{lstlisting}
> > 
> > Are there any alignment requirements for PCI capability structures?
> 
> The PCI spec says 'Each capability must be DWORD aligned. '
> 
> > This struct ends with a u8, which might be a problem if 32-bit or 64-bit
> > alignemnt is required by something.
> 
> It doesn't seem to be unusual, for example the MSI capability structure
> seems to be 10 or 14 bytes even with the DWORD alignement restriction
> for the start of it.

Sounds good.  Just wanted to check.

Stefan

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