[virtio-comment] [PATCH v4 0/1] virtio-pmem: Support describing pmem as shared memory region

[virtio-comment] [PATCH v4 0/1] virtio-pmem: Support describing pmem as shared memory region

[tstark]

From: Taylor Stark <tstark@microsoft.com>

Changes from v3 [1]:
 - Rebased onto master now that the base virtio-pmem spec has been merged.

Changes from v2:
 - Incorporated suggestions from Cornelia Huck on rewording driver initialization.

Changes from v1:
 - Added in a feature bit (VIRTIO_PMEM_F_SHMEM_REGION) for controlling how the
   device indicates the guest physical address ranges to the driver. This feature
   directly affects control flow of the driver, since it seemed weird to have
   the driver indicate support for shared memory regions, and then needing
   to include an enum (or similar) informing the driver how the device
   indicated guest physical address ranges. If devices want to indicate the
   ranges as guest absolute addresses, they can skip negotiating the feature.
 - The linux driver implementation has been updated and tested, but I'm holding
   off on posting the patches to get some feedback on the new approach.
 - Moved some changes to proper subsections (normative subsections).

[1] https://lists.oasis-open.org/archives/virtio-comment/202107/msg00169.html

---

This patch updates the virtio-pmem spec to add support for describing the pmem
region as a shared memory region. This is required to support virtio-pmem in
Hyper-V, since Hyper-V only allows PCI devices to operate on memory ranges
defined via BARs. When using the virtio PCI transport, shared memory regions
are described via PCI BARs.

As virtio-pmem hasn't been added to the virtio spec yet (see this issue [1]),
this patch is based off the RFC spec [2]. The linux driver implementation has
been posted at [3].

[1] https://github.com/oasis-tcs/virtio-spec/issues/78
[2] https://lists.oasis-open.org/archives/virtio-dev/201903/msg00083.html
[3] https://lore.kernel.org/nvdimm/20210715223505.GA29329@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net


Taylor Stark (1):
  virtio-pmem: Support describing pmem as shared memory region

 conformance.tex | 14 ++++++++++++--
 virtio-pmem.tex | 34 +++++++++++++++++++++++++++++-----
 2 files changed, 41 insertions(+), 7 deletions(-)

-- 
2.31.0.vfs.0.1


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] [PATCH v4 1/1] virtio-pmem: Support describing pmem as shared memory region

[tstark]

From: Taylor Stark <tstark@microsoft.com>

Update the virtio-pmem spec to add support for describing the pmem region as a
shared memory window. This is required to support virtio-pmem in Hyper-V, since
Hyper-V only allows PCI devices to operate on memory ranges defined via BARs.
When using the virtio PCI transport, shared memory regions are described via
PCI BARs.

Signed-off-by: Taylor Stark <tstark@microsoft.com>
---
 conformance.tex | 14 ++++++++++++--
 virtio-pmem.tex | 34 +++++++++++++++++++++++++++++-----
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index 80547db..d9f2b45 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -31,8 +31,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \ref{sec:Conformance / Driver Conformance / Sound Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
-\ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance} or
-\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance}.
+\ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
+\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
+\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
 
     \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
   \end{itemize}
@@ -314,6 +315,14 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{drivernormative:Device Types / GPIO Device / eventq Operation}
 \end{itemize}
 
+\conformance{\subsection}{PMEM Driver Conformance}\label{sec:Conformance / Driver Conformance / PMEM Driver Conformance}
+
+A PMEM driver MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{drivernormative:Device Types / PMEM Device / Device Initialization}
+\end{itemize}
+
 \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
 
 A device MUST conform to the following normative statements:
@@ -578,6 +587,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 A PMEM device MUST conform to the following normative statements:
 
 \begin{itemize}
+\item \ref{devicenormative:Device Types / PMEM Device / Device Initialization}
 \item \ref{devicenormative:Device Types / PMEM Device / Device Operation / Virtqueue flush}
 \item \ref{devicenormative:Device Types / PMEM Device / Device Operation / Virtqueue return}
 \end{itemize}
diff --git a/virtio-pmem.tex b/virtio-pmem.tex
index 93ab3c1..eb89f55 100644
--- a/virtio-pmem.tex
+++ b/virtio-pmem.tex
@@ -24,7 +24,10 @@ \subsection{Virtqueues}\label{sec:Device Types / PMEM Device / Virtqueues}
 
 \subsection{Feature bits}\label{sec:Device Types / PMEM Device / Feature bits}
 
-There are currently no feature bits defined for this device.
+\begin{description}
+\item[VIRTIO_PMEM_F_SHMEM_REGION (0)] The guest physical address range will be
+indicated as a shared memory region.
+\end{description}
 
 \subsection{Device configuration layout}\label{sec:Device Types / PMEM Device / Device configuration layout}
 
@@ -41,17 +44,38 @@ \subsection{Device configuration layout}\label{sec:Device Types / PMEM Device /
 \item[\field{size}] contains the length of this address range.
 \end{description}
 
+\subsection{Device Initialization}\label{sec:Device Types / PMEM Device / Device Initialization}
+
+The device indicates the guest physical address to the driver in one of two ways:
 \begin{enumerate}
-\item Driver vpmem start is read from \field{start}.
-\item Driver vpmem end is read from \field{size}.
+\item As a guest absolute address, using virtio_pmem_config.
+\item As a shared memory region.
 \end{enumerate}
 
-\subsection{Driver Initialization}\label{sec:Device Types / PMEM Driver / Driver Initialization}
-
 The driver determines the start address and size of the persistent memory region in preparation for reading or writing data.
 
 The driver initializes req_vq in preparation for making flush requests.
 
+\devicenormative{\subsubsection}{Device Initialization}{Device Types / PMEM Device / Device Initialization}
+
+If VIRTIO_PMEM_F_SHMEM_REGION has been negotiated, the device MUST indicate the
+guest physical address as a shared memory region. The device MUST use shared
+memory region ID 0. The device SHOULD set \field{start} and \field{size} to zero.
+
+If VIRTIO_PMEM_F_SHMEM_REGION has not been negotiated, the device MUST indicate
+the guest physical address as a guest absolute address. The device MUST set
+\field{start} to the absolute address and \field{size} to the size of the
+address range, in bytes.
+
+\drivernormative{\subsubsection}{Device Initialization}{Device Types / PMEM Device / Device Initialization}
+
+If VIRTIO_PMEM_F_SHMEM_REGION has been negotiated, the driver MUST query
+shared memory ID 0 for the physical address ranges, and MUST NOT use
+\field{start} or \field{stop}.
+
+If VIRTIO_PMEM_F_SHMEM_REGION has not been negotiated, the driver MUST read the
+physical address ranges from \field{start} and \field{stop}.
+
 \subsection{Driver Operations}\label{sec:Device Types / PMEM Driver / Driver Operation / Request Queues}
 
 Requests have the following format:
-- 
2.31.0.vfs.0.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/

Re: [virtio-comment] [PATCH v4 1/1] virtio-pmem: Support describing pmem as shared memory region

[Cornelia Huck]

On Fri, Nov 05 2021, tstark@linux.microsoft.com wrote:

> From: Taylor Stark <tstark@microsoft.com>
>
> Update the virtio-pmem spec to add support for describing the pmem region as a
> shared memory window. This is required to support virtio-pmem in Hyper-V, since
> Hyper-V only allows PCI devices to operate on memory ranges defined via BARs.
> When using the virtio PCI transport, shared memory regions are described via
> PCI BARs.
>
> Signed-off-by: Taylor Stark <tstark@microsoft.com>
> ---
>  conformance.tex | 14 ++++++++++++--
>  virtio-pmem.tex | 34 +++++++++++++++++++++++++++++-----
>  2 files changed, 41 insertions(+), 7 deletions(-)

(...)

> @@ -41,17 +44,38 @@ \subsection{Device configuration layout}\label{sec:Device Types / PMEM Device /
>  \item[\field{size}] contains the length of this address range.
>  \end{description}

I wonder whether we should also amend the description of the config
space fields to "contains (...), if VIRTIO_PMEM_F_SHMEM_REGION has not
been negotiated". But maybe that is not really needed.

>  
> +\subsection{Device Initialization}\label{sec:Device Types / PMEM Device / Device Initialization}
> +
> +The device indicates the guest physical address to the driver in one of two ways:
>  \begin{enumerate}
> -\item Driver vpmem start is read from \field{start}.
> -\item Driver vpmem end is read from \field{size}.
> +\item As a guest absolute address, using virtio_pmem_config.
> +\item As a shared memory region.
>  \end{enumerate}

Otherwise, looks good to me.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] [PATCH v4 1/1] virtio-pmem: Support describing pmem as shared memory region

[Taylor Stark]

On Mon, Nov 08, 2021 at 01:54:27PM +0100, Cornelia Huck wrote:
> On Fri, Nov 05 2021, tstark@linux.microsoft.com wrote:
> 
> > From: Taylor Stark <tstark@microsoft.com>
> >
> > Update the virtio-pmem spec to add support for describing the pmem region as a
> > shared memory window. This is required to support virtio-pmem in Hyper-V, since
> > Hyper-V only allows PCI devices to operate on memory ranges defined via BARs.
> > When using the virtio PCI transport, shared memory regions are described via
> > PCI BARs.
> >
> > Signed-off-by: Taylor Stark <tstark@microsoft.com>
> > ---
> >  conformance.tex | 14 ++++++++++++--
> >  virtio-pmem.tex | 34 +++++++++++++++++++++++++++++-----
> >  2 files changed, 41 insertions(+), 7 deletions(-)
> 
> (...)
> 
> > @@ -41,17 +44,38 @@ \subsection{Device configuration layout}\label{sec:Device Types / PMEM Device /
> >  \item[\field{size}] contains the length of this address range.
> >  \end{description}
> 
> I wonder whether we should also amend the description of the config
> space fields to "contains (...), if VIRTIO_PMEM_F_SHMEM_REGION has not
> been negotiated". But maybe that is not really needed.

Sounds reasonable to me. I'll send an update with the changes.