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

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

[tstark]

From: Taylor Stark <tstark@microsoft.com>

Changes from v2 [1]:
 - 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/msg00145.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 |  1 +
 virtio-pmem.tex | 34 +++++++++++++++++++++++++++++-----
 2 files changed, 30 insertions(+), 5 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 v3 1/1] virtio-pmem: Support describing pmem as shared memory region

[tstark]

From: Taylor Stark <tstark@microsoft.com>

Update the virtio-pmem RFC 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 |  1 +
 virtio-pmem.tex | 34 +++++++++++++++++++++++++++++-----
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index 33fdac9..8e414eb 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -201,6 +201,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 
 \begin{itemize}
 \item \ref{devicenormative:Device Types / PMEM Device / Device Initialization}
+\item \ref{drivernormative:Device Types / PMEM Driver / Driver Initialization}
 \item \ref{drivernormative:Device Types / PMEM Driver / Driver Initialization / Direct access}
 \item \ref{drivernormative:Device Types / PMEM Driver / Driver Initialization / Virtio flush}
 \item \ref{drivernormative:Device Types / PMEM Driver / Driver Operation / Virtqueue command}
diff --git a/virtio-pmem.tex b/virtio-pmem.tex
index 04e07bb..ceb62db 100644
--- a/virtio-pmem.tex
+++ b/virtio-pmem.tex
@@ -19,7 +19,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}
 
@@ -38,11 +41,11 @@ \subsection{Device configuration layout}\label{sec:Device Types / PMEM Device /
 \subsection{Device Initialization}\label{sec:Device Types / PMEM Device / Device Initialization}
 
 Device hotplugs physical memory to guest address space. Persistent memory device
-is emulated with file backed memory at host side.
-
+is emulated with file backed memory at host side. The device indicates the guest
+physical address to the driver in one of two ways:
 \begin{enumerate}
-\item Guest vpmem start is read from \field{start}.
-\item Guest vpmem end is read from \field{size}.
+\item As a guest absolute address.
+\item As a shared memory region.
 \end{enumerate}
 
 \devicenormative{\subsubsection}{Device Initialization}{Device Types / PMEM Device / Device Initialization}
@@ -50,12 +53,33 @@ \subsection{Device Initialization}\label{sec:Device Types / PMEM Device / Device
 File backed memory SHOULD be memory mapped to guest address space with SHARED
 memory mapping.
 
+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.
+
 \subsection{Driver Initialization}\label{sec:Device Types / PMEM Driver / Driver Initialization}
 
 Driver hotplugs the physical memory and registers associated
 region with the pmem API. Also, configures a flush callback
 function with the corresponding region.
 
+\drivernormative{\subsubsection}{Driver Initialization}{Device Types / PMEM Driver / Driver Initialization}
+
+In order to discover the physical address ranges where the pmem was
+mapped:
+
+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}.
+
 \drivernormative{\subsubsection}{Driver Initialization: Filesystem direct access}{Device Types / PMEM Driver / Driver Initialization / Direct access}
 
 Driver SHOULD enable filesystem direct access operations for
-- 
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 v3 0/1] virtio-pmem: Support describing pmem as shared memory region

[Cornelia Huck]

On Wed, Jul 28 2021, tstark@linux.microsoft.com wrote:

> From: Taylor Stark <tstark@microsoft.com>
>
> Changes from v2 [1]:
>  - 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/msg00145.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 |  1 +
>  virtio-pmem.tex | 34 +++++++++++++++++++++++++++++-----
>  2 files changed, 30 insertions(+), 5 deletions(-)
>

Looks good to me now, but I'd also like a comment from someone who has
actually done some work in the area.

The main issue is to get the actual (base) pmem spec merged, but I see
that a patch has been posted, so we should be able to go from there.


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

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

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

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

[Taylor Stark]

> Looks good to me now, but I'd also like a comment from someone who has
> actually done some work in the area.

Thanks for the review. I think the authority on this area would be Pankaj,
who is also driving getting the base pmem spec merged. Pankaj, do you have
any thoughts/feedback?

> The main issue is to get the actual (base) pmem spec merged, but I see
> that a patch has been posted, so we should be able to go from there.

Sounds good. I'll keep tabs on getting the base pmem spec merged and circle
back once it is.

Thanks,
Taylor