All of lore.kernel.org
 help / color / mirror / Atom feed
* [virtio-comment] [PATCH v4 0/3] Large shared memory regions
@ 2019-03-20 12:07 Dr. David Alan Gilbert (git)
  2019-03-20 12:07 ` [virtio-comment] [PATCH v4 1/3] shared memory: Define " Dr. David Alan Gilbert (git)
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-03-20 12:07 UTC (permalink / raw)
  To: virtio-dev, virtio-comment; +Cc: stefanha, cohuck

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 v4 has one small tweak to the addressing comment in
patch 1, based partially on Stefan's feedback to v3.

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/


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [virtio-comment] [PATCH v4 1/3] shared memory: Define shared memory regions
  2019-03-20 12:07 [virtio-comment] [PATCH v4 0/3] Large shared memory regions Dr. David Alan Gilbert (git)
@ 2019-03-20 12:07 ` Dr. David Alan Gilbert (git)
  2019-06-17 18:30   ` Michael S. Tsirkin
  2019-03-20 12:07 ` [virtio-comment] [PATCH v4 2/3] shared memory: Define PCI capability Dr. David Alan Gilbert (git)
  2019-03-20 12:07 ` [virtio-comment] [PATCH v4 3/3] shared memory: Define mmio registers Dr. David Alan Gilbert (git)
  2 siblings, 1 reply; 17+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-03-20 12:07 UTC (permalink / raw)
  To: virtio-dev, virtio-comment; +Cc: stefanha, cohuck

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  | 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 ede0ef6..8cd1a38 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..6dc506a
--- /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 }
+
+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.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/


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [virtio-comment] [PATCH v4 2/3] shared memory: Define PCI capability
  2019-03-20 12:07 [virtio-comment] [PATCH v4 0/3] Large shared memory regions Dr. David Alan Gilbert (git)
  2019-03-20 12:07 ` [virtio-comment] [PATCH v4 1/3] shared memory: Define " Dr. David Alan Gilbert (git)
@ 2019-03-20 12:07 ` Dr. David Alan Gilbert (git)
  2019-06-17 18:36   ` [virtio-comment] Re: [virtio-dev] " Michael S. Tsirkin
  2019-03-20 12:07 ` [virtio-comment] [PATCH v4 3/3] shared memory: Define mmio registers Dr. David Alan Gilbert (git)
  2 siblings, 1 reply; 17+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-03-20 12:07 UTC (permalink / raw)
  To: virtio-dev, virtio-comment; +Cc: stefanha, cohuck

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 8cd1a38..ddbf949 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/


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [virtio-comment] [PATCH v4 3/3] shared memory: Define mmio registers
  2019-03-20 12:07 [virtio-comment] [PATCH v4 0/3] Large shared memory regions Dr. David Alan Gilbert (git)
  2019-03-20 12:07 ` [virtio-comment] [PATCH v4 1/3] shared memory: Define " Dr. David Alan Gilbert (git)
  2019-03-20 12:07 ` [virtio-comment] [PATCH v4 2/3] shared memory: Define PCI capability Dr. David Alan Gilbert (git)
@ 2019-03-20 12:07 ` Dr. David Alan Gilbert (git)
  2 siblings, 0 replies; 17+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-03-20 12:07 UTC (permalink / raw)
  To: virtio-dev, virtio-comment; +Cc: stefanha, cohuck

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 ddbf949..f2195be 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/


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [virtio-comment] [PATCH v4 1/3] shared memory: Define shared memory regions
  2019-03-20 12:07 ` [virtio-comment] [PATCH v4 1/3] shared memory: Define " Dr. David Alan Gilbert (git)
@ 2019-06-17 18:30   ` Michael S. Tsirkin
  2019-06-19 16:06     ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2019-06-17 18:30 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: virtio-dev, virtio-comment, stefanha, cohuck

On Wed, Mar 20, 2019 at 12:07:38PM +0000, Dr. David Alan Gilbert (git) 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  | 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 ede0ef6..8cd1a38 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..6dc506a
> --- /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.

Please rewrite host->device, guest->driver

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

I'm not sure what does above mean in context of virtio.
Is this an example?
So maybe:
	For example, with device being part of a
	hypervisor and driver being part of a VM guest
	etc etc

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

Let's just say 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.

regions must not expose regions. confused.
typo?

Also, what does stream data mean?


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

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/


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [virtio-comment] Re: [virtio-dev] [PATCH v4 2/3] shared memory: Define PCI capability
  2019-03-20 12:07 ` [virtio-comment] [PATCH v4 2/3] shared memory: Define PCI capability Dr. David Alan Gilbert (git)
@ 2019-06-17 18:36   ` Michael S. Tsirkin
  2019-06-17 18:41     ` Michael S. Tsirkin
  2019-06-19 17:10     ` Dr. David Alan Gilbert
  0 siblings, 2 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2019-06-17 18:36 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: virtio-dev, virtio-comment, stefanha, cohuck

On Wed, Mar 20, 2019 at 12:07:39PM +0000, Dr. David Alan Gilbert (git) 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>
> 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 8cd1a38..ddbf949 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;

Pls pad, pci capabilities must be a multiple of 4 bytes.
BTW virtio_pci_cap has 3 bytes of padding.
We can stick the id there maybe?

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

I didn't realize we will ever need so much space.  I think I prefer
moving offset_hi/length_hi to a general virtio_pci_cap64 that others can
reuse.


> +
> +While most capabilities can not be repeated,

I'd drop this part.

> 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 

let's do \field{cap.length} \field{cap.offset} etc.

>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
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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/


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [virtio-comment] Re: [virtio-dev] [PATCH v4 2/3] shared memory: Define PCI capability
  2019-06-17 18:36   ` [virtio-comment] Re: [virtio-dev] " Michael S. Tsirkin
@ 2019-06-17 18:41     ` Michael S. Tsirkin
  2019-06-19 17:13       ` Dr. David Alan Gilbert
  2019-06-19 17:10     ` Dr. David Alan Gilbert
  1 sibling, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2019-06-17 18:41 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: virtio-dev, virtio-comment, stefanha, cohuck

On Mon, Jun 17, 2019 at 02:36:43PM -0400, Michael S. Tsirkin wrote:
> On Wed, Mar 20, 2019 at 12:07:39PM +0000, Dr. David Alan Gilbert (git) 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>
> > 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 8cd1a38..ddbf949 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;
> 
> Pls pad, pci capabilities must be a multiple of 4 bytes.
> BTW virtio_pci_cap has 3 bytes of padding.
> We can stick the id there maybe?
> 
> > +};
> > +\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.
> 
> I didn't realize we will ever need so much space.  I think I prefer
> moving offset_hi/length_hi to a general virtio_pci_cap64 that others can
> reuse.

And maybe we should set a bit in some other reserved byte
so people can generally use 64 bit offsets/lengths?

> 
> > +
> > +While most capabilities can not be repeated,
> 
> I'd drop this part.
> 
> > 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 
> 
> let's do \field{cap.length} \field{cap.offset} etc.
> 
> >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
> > 
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> 
> 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/

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/


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [virtio-comment] [PATCH v4 1/3] shared memory: Define shared memory regions
  2019-06-17 18:30   ` Michael S. Tsirkin
@ 2019-06-19 16:06     ` Dr. David Alan Gilbert
  2019-06-19 16:24       ` Michael S. Tsirkin
  2019-06-21 20:42       ` Michael S. Tsirkin
  0 siblings, 2 replies; 17+ messages in thread
From: Dr. David Alan Gilbert @ 2019-06-19 16:06 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtio-dev, virtio-comment, stefanha, cohuck

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Wed, Mar 20, 2019 at 12:07:38PM +0000, Dr. David Alan Gilbert (git) 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  | 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 ede0ef6..8cd1a38 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..6dc506a
> > --- /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.
> 
> Please rewrite host->device, guest->driver

Done.

> > +
> > +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.
> 
> I'm not sure what does above mean in context of virtio.
> Is this an example?
> So maybe:
> 	For example, with device being part of a
> 	hypervisor and driver being part of a VM guest
> 	etc etc

It means the memory can be allocated by another library in the host
and then passed into the guest; the alternative is where the guest
gives a pointer into the guests own 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.
> 
> Let's just say in a transport-specific way.

Done.

> > +
> > +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.
> 
> regions must not expose regions. confused.
> typo?
> 
> Also, what does stream data mean?

I'm trying to find a way to say that people shouldn't side-step virtio
queues by putting everything in a big blob of shared memory.

Better wording welcome.

Dave

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [virtio-comment] [PATCH v4 1/3] shared memory: Define shared memory regions
  2019-06-19 16:06     ` Dr. David Alan Gilbert
@ 2019-06-19 16:24       ` Michael S. Tsirkin
  2019-06-21 20:42       ` Michael S. Tsirkin
  1 sibling, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2019-06-19 16:24 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: virtio-dev, virtio-comment, stefanha, cohuck

On Wed, Jun 19, 2019 at 05:06:04PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Wed, Mar 20, 2019 at 12:07:38PM +0000, Dr. David Alan Gilbert (git) 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  | 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 ede0ef6..8cd1a38 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..6dc506a
> > > --- /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.
> > 
> > Please rewrite host->device, guest->driver
> 
> Done.
> 
> > > +
> > > +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.
> > 
> > I'm not sure what does above mean in context of virtio.
> > Is this an example?
> > So maybe:
> > 	For example, with device being part of a
> > 	hypervisor and driver being part of a VM guest
> > 	etc etc
> 
> It means the memory can be allocated by another library in the host
> and then passed into the guest; the alternative is where the guest
> gives a pointer into the guests own RAM.

But we don't know driver is guest and device is host.

This is why I suggested that it's an example:
for example if driver is in guest and device is in host software
then another host library can map it.


> > > +
> > > +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.
> > 
> > Let's just say in a transport-specific way.
> 
> Done.
> 
> > > +
> > > +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.
> > 
> > regions must not expose regions. confused.
> > typo?
> > 
> > Also, what does stream data mean?
> 
> I'm trying to find a way to say that people shouldn't side-step virtio
> queues by putting everything in a big blob of shared memory.
> 
> Better wording welcome.
> 
> Dave

I think writes and read to these regions must not have
side effects. Is this what you are trying to say?

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [virtio-comment] Re: [virtio-dev] [PATCH v4 2/3] shared memory: Define PCI capability
  2019-06-17 18:36   ` [virtio-comment] Re: [virtio-dev] " Michael S. Tsirkin
  2019-06-17 18:41     ` Michael S. Tsirkin
@ 2019-06-19 17:10     ` Dr. David Alan Gilbert
  2019-06-20  5:43       ` Michael S. Tsirkin
  2019-06-21 20:39       ` Michael S. Tsirkin
  1 sibling, 2 replies; 17+ messages in thread
From: Dr. David Alan Gilbert @ 2019-06-19 17:10 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtio-dev, virtio-comment, stefanha, cohuck

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Wed, Mar 20, 2019 at 12:07:39PM +0000, Dr. David Alan Gilbert (git) 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>
> > 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 8cd1a38..ddbf949 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;
> 
> Pls pad, pci capabilities must be a multiple of 4 bytes.
> BTW virtio_pci_cap has 3 bytes of padding.
> We can stick the id there maybe?

Do you mean changing the def of virtio_pci_cap to: ?
struct virtio_pci_cap {
        __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. */
        __u8 bar;               /* Where to find it. */
        __u8 id;                /* Unique ID for this region */
        __u8 padding[2];        /* Pad to full dword. */
        __le32 offset;          /* Offset within bar. */
        __le32 length;          /* Length of the structure, in bytes. */
};

> > +};
> > +\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.
> 
> I didn't realize we will ever need so much space.  I think I prefer
> moving offset_hi/length_hi to a general virtio_pci_cap64 that others can
> reuse.

Would we then just rename virtio_pci_shm_cap to virtio_pci_cap64 after
moving the id to virito_pci_cap?

> 
> 
> > +
> > +While most capabilities can not be repeated,
> 
> I'd drop this part.

Done.

> > 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 
> 
> let's do \field{cap.length} \field{cap.offset} etc.

I've done that as:
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.

is that right, but the base and hi are in separate structures.

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [virtio-comment] Re: [virtio-dev] [PATCH v4 2/3] shared memory: Define PCI capability
  2019-06-17 18:41     ` Michael S. Tsirkin
@ 2019-06-19 17:13       ` Dr. David Alan Gilbert
  2019-06-20  5:45         ` Michael S. Tsirkin
  2019-06-21 20:38         ` [virtio-comment] Re: [virtio-dev] " Michael S. Tsirkin
  0 siblings, 2 replies; 17+ messages in thread
From: Dr. David Alan Gilbert @ 2019-06-19 17:13 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtio-dev, virtio-comment, stefanha, cohuck

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Mon, Jun 17, 2019 at 02:36:43PM -0400, Michael S. Tsirkin wrote:
> > On Wed, Mar 20, 2019 at 12:07:39PM +0000, Dr. David Alan Gilbert (git) 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>
> > > 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 8cd1a38..ddbf949 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;
> > 
> > Pls pad, pci capabilities must be a multiple of 4 bytes.
> > BTW virtio_pci_cap has 3 bytes of padding.
> > We can stick the id there maybe?
> > 
> > > +};
> > > +\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.
> > 
> > I didn't realize we will ever need so much space.  I think I prefer
> > moving offset_hi/length_hi to a general virtio_pci_cap64 that others can
> > reuse.
> 
> And maybe we should set a bit in some other reserved byte
> so people can generally use 64 bit offsets/lengths?

Is it actually any cleaner? I don't know - where would you put that bit?

Dave

> > 
> > > +
> > > +While most capabilities can not be repeated,
> > 
> > I'd drop this part.
> > 
> > > 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 
> > 
> > let's do \field{cap.length} \field{cap.offset} etc.
> > 
> > >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
> > > 
> > > 
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > 
> > 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/
--
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/


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [virtio-comment] Re: [virtio-dev] [PATCH v4 2/3] shared memory: Define PCI capability
  2019-06-19 17:10     ` Dr. David Alan Gilbert
@ 2019-06-20  5:43       ` Michael S. Tsirkin
  2019-06-21 20:39       ` Michael S. Tsirkin
  1 sibling, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2019-06-20  5:43 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: virtio-dev, virtio-comment, stefanha, cohuck

On Wed, Jun 19, 2019 at 06:10:45PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Wed, Mar 20, 2019 at 12:07:39PM +0000, Dr. David Alan Gilbert (git) 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>
> > > 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 8cd1a38..ddbf949 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;
> > 
> > Pls pad, pci capabilities must be a multiple of 4 bytes.
> > BTW virtio_pci_cap has 3 bytes of padding.
> > We can stick the id there maybe?
> 
> Do you mean changing the def of virtio_pci_cap to: ?
> struct virtio_pci_cap {
>         __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. */
>         __u8 bar;               /* Where to find it. */
>         __u8 id;                /* Unique ID for this region */
>         __u8 padding[2];        /* Pad to full dword. */
>         __le32 offset;          /* Offset within bar. */
>         __le32 length;          /* Length of the structure, in bytes. */
> };

Something like this, yes.

> > > +};
> > > +\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.
> > 
> > I didn't realize we will ever need so much space.  I think I prefer
> > moving offset_hi/length_hi to a general virtio_pci_cap64 that others can
> > reuse.
> 
> Would we then just rename virtio_pci_shm_cap to virtio_pci_cap64 after
> moving the id to virito_pci_cap?

Right except we then need to document that only shm uses cap64, others
use cap. And I guess for forward compatibility set a bit somewhere - within bar maybe?

Overall I'm no longer sure it's a good idea. you decide.

> > 
> > 
> > > +
> > > +While most capabilities can not be repeated,
> > 
> > I'd drop this part.
> 
> Done.
> 
> > > 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 
> > 
> > let's do \field{cap.length} \field{cap.offset} etc.
> 
> I've done that as:
> 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.
> 
> is that right, but the base and hi are in separate structures.


ok

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [virtio-comment] Re: [virtio-dev] [PATCH v4 2/3] shared memory: Define PCI capability
  2019-06-19 17:13       ` Dr. David Alan Gilbert
@ 2019-06-20  5:45         ` Michael S. Tsirkin
  2019-06-21 20:38         ` [virtio-comment] Re: [virtio-dev] " Michael S. Tsirkin
  1 sibling, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2019-06-20  5:45 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: virtio-dev, virtio-comment, stefanha, cohuck

On Wed, Jun 19, 2019 at 06:13:17PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Mon, Jun 17, 2019 at 02:36:43PM -0400, Michael S. Tsirkin wrote:
> > > On Wed, Mar 20, 2019 at 12:07:39PM +0000, Dr. David Alan Gilbert (git) 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>
> > > > 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 8cd1a38..ddbf949 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;
> > > 
> > > Pls pad, pci capabilities must be a multiple of 4 bytes.
> > > BTW virtio_pci_cap has 3 bytes of padding.
> > > We can stick the id there maybe?
> > > 
> > > > +};
> > > > +\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.
> > > 
> > > I didn't realize we will ever need so much space.  I think I prefer
> > > moving offset_hi/length_hi to a general virtio_pci_cap64 that others can
> > > reuse.
> > 
> > And maybe we should set a bit in some other reserved byte
> > so people can generally use 64 bit offsets/lengths?
> 
> Is it actually any cleaner? I don't know - where would you put that bit?
> 
> Dave

We'd put it either in bar or cfg type.

No, I'm not sure it's strictly required.
It saves a bit of config space but increases
complexity for guests ....

> > > 
> > > > +
> > > > +While most capabilities can not be repeated,
> > > 
> > > I'd drop this part.
> > > 
> > > > 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 
> > > 
> > > let's do \field{cap.length} \field{cap.offset} etc.
> > > 
> > > >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
> > > > 
> > > > 
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > > 
> > > 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/
> --
> 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/


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [virtio-dev] [PATCH v4 2/3] shared memory: Define PCI capability
  2019-06-19 17:13       ` Dr. David Alan Gilbert
  2019-06-20  5:45         ` Michael S. Tsirkin
@ 2019-06-21 20:38         ` Michael S. Tsirkin
  1 sibling, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2019-06-21 20:38 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: virtio-dev, virtio-comment, stefanha, cohuck

On Wed, Jun 19, 2019 at 06:13:17PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Mon, Jun 17, 2019 at 02:36:43PM -0400, Michael S. Tsirkin wrote:
> > > On Wed, Mar 20, 2019 at 12:07:39PM +0000, Dr. David Alan Gilbert (git) 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>
> > > > 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 8cd1a38..ddbf949 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;
> > > 
> > > Pls pad, pci capabilities must be a multiple of 4 bytes.
> > > BTW virtio_pci_cap has 3 bytes of padding.
> > > We can stick the id there maybe?
> > > 
> > > > +};
> > > > +\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.
> > > 
> > > I didn't realize we will ever need so much space.  I think I prefer
> > > moving offset_hi/length_hi to a general virtio_pci_cap64 that others can
> > > reuse.
> > 
> > And maybe we should set a bit in some other reserved byte
> > so people can generally use 64 bit offsets/lengths?
> 
> Is it actually any cleaner?
> 
> Dave

I am just saying this: let's assume some other config type will also
want to use 64 bit offsets down the road. If we do it will be nice
to be able, down the line, to write
if (bit set)
	64 bit
else
	32 bit

without special-casing this config type.


> I don't know - where would you put that bit?

options include cfg type and bar.
I think I prefer bar since their number is definitely limited by pci
spec.

> > > 
> > > > +
> > > > +While most capabilities can not be repeated,
> > > 
> > > I'd drop this part.
> > > 
> > > > 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 
> > > 
> > > let's do \field{cap.length} \field{cap.offset} etc.
> > > 
> > > >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
> > > > 
> > > > 
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > > 
> > > 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/
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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/


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [virtio-comment] Re: [virtio-dev] [PATCH v4 2/3] shared memory: Define PCI capability
  2019-06-19 17:10     ` Dr. David Alan Gilbert
  2019-06-20  5:43       ` Michael S. Tsirkin
@ 2019-06-21 20:39       ` Michael S. Tsirkin
  1 sibling, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2019-06-21 20:39 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: virtio-dev, virtio-comment, stefanha, cohuck

On Wed, Jun 19, 2019 at 06:10:45PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Wed, Mar 20, 2019 at 12:07:39PM +0000, Dr. David Alan Gilbert (git) 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>
> > > 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 8cd1a38..ddbf949 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;
> > 
> > Pls pad, pci capabilities must be a multiple of 4 bytes.
> > BTW virtio_pci_cap has 3 bytes of padding.
> > We can stick the id there maybe?
> 
> Do you mean changing the def of virtio_pci_cap to: ?
> struct virtio_pci_cap {
>         __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. */
>         __u8 bar;               /* Where to find it. */
>         __u8 id;                /* Unique ID for this region */
>         __u8 padding[2];        /* Pad to full dword. */
>         __le32 offset;          /* Offset within bar. */
>         __le32 length;          /* Length of the structure, in bytes. */
> };
> 
> > > +};
> > > +\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.
> > 
> > I didn't realize we will ever need so much space.  I think I prefer
> > moving offset_hi/length_hi to a general virtio_pci_cap64 that others can
> > reuse.
> 
> Would we then just rename virtio_pci_shm_cap to virtio_pci_cap64 after
> moving the id to virito_pci_cap?

yes. and for simplicity we can for now specify that only shmem can be 64 bit.

> > 
> > 
> > > +
> > > +While most capabilities can not be repeated,
> > 
> > I'd drop this part.
> 
> Done.
> 
> > > 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 
> > 
> > let's do \field{cap.length} \field{cap.offset} etc.
> 
> I've done that as:
> 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.
> 
> is that right, but the base and hi are in separate structures.
> 
> > >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
> > > 
> > > 
> > > ---------------------------------------------------------------------
> > > 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
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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/


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [virtio-comment] [PATCH v4 1/3] shared memory: Define shared memory regions
  2019-06-19 16:06     ` Dr. David Alan Gilbert
  2019-06-19 16:24       ` Michael S. Tsirkin
@ 2019-06-21 20:42       ` Michael S. Tsirkin
  2019-06-27 16:35         ` [virtio-comment] Re: [virtio-dev] " Dr. David Alan Gilbert
  1 sibling, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2019-06-21 20:42 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: virtio-dev, virtio-comment, stefanha, cohuck

On Wed, Jun 19, 2019 at 05:06:04PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Wed, Mar 20, 2019 at 12:07:38PM +0000, Dr. David Alan Gilbert (git) 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  | 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 ede0ef6..8cd1a38 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..6dc506a
> > > --- /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.
> > 
> > Please rewrite host->device, guest->driver
> 
> Done.
> 
> > > +
> > > +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.
> > 
> > I'm not sure what does above mean in context of virtio.
> > Is this an example?
> > So maybe:
> > 	For example, with device being part of a
> > 	hypervisor and driver being part of a VM guest
> > 	etc etc
> 
> It means the memory can be allocated by another library in the host
> and then passed into the guest; the alternative is where the guest
> gives a pointer into the guests own RAM.

OK so let's say this like this: memory is allocated
by the device as opposed to by driver.
As such this allows configurations where access to guest
memory is limited. For example and so .


> > > +
> > > +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.
> > 
> > Let's just say in a transport-specific way.
> 
> Done.
> 
> > > +
> > > +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.
> > 
> > regions must not expose regions. confused.
> > typo?
> > 
> > Also, what does stream data mean?
> 
> I'm trying to find a way to say that people shouldn't side-step virtio
> queues by putting everything in a big blob of shared memory.
> 
> Better wording welcome.
> 
> Dave

The way to improve wording would be by specifying what are
the disadvantages of storing data in shared memory,
and when it should be avoided.


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

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/


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] [PATCH v4 1/3] shared memory: Define shared memory regions
  2019-06-21 20:42       ` Michael S. Tsirkin
@ 2019-06-27 16:35         ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 17+ messages in thread
From: Dr. David Alan Gilbert @ 2019-06-27 16:35 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtio-dev, virtio-comment, stefanha, cohuck

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Wed, Jun 19, 2019 at 05:06:04PM +0100, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > On Wed, Mar 20, 2019 at 12:07:38PM +0000, Dr. David Alan Gilbert (git) 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  | 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 ede0ef6..8cd1a38 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..6dc506a
> > > > --- /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.
> > > 
> > > Please rewrite host->device, guest->driver
> > 
> > Done.
> > 
> > > > +
> > > > +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.
> > > 
> > > I'm not sure what does above mean in context of virtio.
> > > Is this an example?
> > > So maybe:
> > > 	For example, with device being part of a
> > > 	hypervisor and driver being part of a VM guest
> > > 	etc etc
> > 
> > It means the memory can be allocated by another library in the host
> > and then passed into the guest; the alternative is where the guest
> > gives a pointer into the guests own RAM.
> 
> OK so let's say this like this: memory is allocated
> by the device as opposed to by driver.
> As such this allows configurations where access to guest
> memory is limited. For example and so .

How about:

  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.

Dave
> 
> > > > +
> > > > +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.
> > > 
> > > Let's just say in a transport-specific way.
> > 
> > Done.
> > 
> > > > +
> > > > +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.
> > > 
> > > regions must not expose regions. confused.
> > > typo?
> > > 
> > > Also, what does stream data mean?
> > 
> > I'm trying to find a way to say that people shouldn't side-step virtio
> > queues by putting everything in a big blob of shared memory.
> > 
> > Better wording welcome.
> > 
> > Dave
> 
> The way to improve wording would be by specifying what are
> the disadvantages of storing data in shared memory,
> and when it should be avoided.
> 
> 
> > > 
> > > > +
> > > > -- 
> > > > 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/
> > --
> > 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/
> 
> ---------------------------------------------------------------------
> 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/


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-06-27 16:35 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-20 12:07 [virtio-comment] [PATCH v4 0/3] Large shared memory regions Dr. David Alan Gilbert (git)
2019-03-20 12:07 ` [virtio-comment] [PATCH v4 1/3] shared memory: Define " Dr. David Alan Gilbert (git)
2019-06-17 18:30   ` Michael S. Tsirkin
2019-06-19 16:06     ` Dr. David Alan Gilbert
2019-06-19 16:24       ` Michael S. Tsirkin
2019-06-21 20:42       ` Michael S. Tsirkin
2019-06-27 16:35         ` [virtio-comment] Re: [virtio-dev] " Dr. David Alan Gilbert
2019-03-20 12:07 ` [virtio-comment] [PATCH v4 2/3] shared memory: Define PCI capability Dr. David Alan Gilbert (git)
2019-06-17 18:36   ` [virtio-comment] Re: [virtio-dev] " Michael S. Tsirkin
2019-06-17 18:41     ` Michael S. Tsirkin
2019-06-19 17:13       ` Dr. David Alan Gilbert
2019-06-20  5:45         ` Michael S. Tsirkin
2019-06-21 20:38         ` [virtio-comment] Re: [virtio-dev] " Michael S. Tsirkin
2019-06-19 17:10     ` Dr. David Alan Gilbert
2019-06-20  5:43       ` Michael S. Tsirkin
2019-06-21 20:39       ` Michael S. Tsirkin
2019-03-20 12:07 ` [virtio-comment] [PATCH v4 3/3] shared memory: Define mmio registers Dr. David Alan Gilbert (git)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.