Re: [virtio-dev] [PATCH RESEND] virtio-pmem: PMEM device spec

[Stefan Hajnoczi <stefanha@redhat.com>]

On Wed, Aug 04, 2021 at 03:58:49PM -0700, Taylor Stark wrote:
> On Wed, Aug 04, 2021 at 01:33:05PM +0100, Stefan Hajnoczi wrote:
> > On Wed, Aug 04, 2021 at 01:11:52PM +0200, David Hildenbrand wrote:
> > > On 04.08.21 13:07, Stefan Hajnoczi wrote:
> > > > On Wed, Jul 28, 2021 at 05:04:35PM +0200, Pankaj Gupta wrote:
> > > > > +\subsection{Device configuration layout}\label{sec:Device Types / PMEM Device / Device configuration layout}
> > > > > +
> > > > > +\begin{lstlisting}
> > > > > +struct virtio_pmem_config {
> > > > > +	le64 start;
> > > > > +	le64 size;
> > > > > +};
> > > > > +\end{lstlisting}
> > > > > +
> > > > > +\begin{description}
> > > > > +\item[\field{start}] contains the start address from the guest physical address range
> > > > > +to be hotplugged into the guest address space using the pmem API.
> > > > > +
> > > > > +\item[\field{size}] contains the length of this address range.
> > > > > +\end{description}
> > > > 
> > > > Please use a Shared Memory Region instead. This is a relatively new
> > > > addition to the VIRTIO device model that seems like a good fit for this
> > > > device:
> > > > https://github.com/oasis-tcs/virtio-spec/blob/master/shared-mem.tex
> > > 
> > > Note that virtio-pmem is already upstream in QEMU and in Linux using this
> > > model. There is a proposal to optionally use shared memory for exposing the
> > > area, to be unlocked with a feature bit.
> > 
> > Please incorporate the proposal into this patch. A feature bit is a good
> > way of handling this case.
> > 
> > Although the difference between configuration space parameters and a
> > shared memory region might seem trivial, I think using the shared memory
> > region is worthwhile because it makes the memory region a first-class
> > concept in the VIRTIO device model that is supported by the driver and
> > device emulation infrastructure.
> > 
> > Stefan
> 
> For what it's worth, here's the proposal to add support for shared memory regions:
> https://lists.oasis-open.org/archives/virtio-comment/202107/msg00169.html
> 
> The original plan was to have the base spec go in first, and then consider the
> proposal. Do you think it's better to include the shared memory region proposal
> from the start?

Talyor and Pankaj,
I hadn't seen Taylor's patch. Since it's small and has already had some
review it can probably be merged quickly after the main virtio-pmem spec
change.

Feel free to keep it as a separate series!

Stefan

Attachment: signature.asc
Description: PGP signature