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

[Taylor Stark <tstark@linux.microsoft.com>]

On Tue, Jul 27, 2021 at 05:06:43PM +0200, Cornelia Huck wrote:
> On Mon, Jul 26 2021, tstark@linux.microsoft.com wrote:
> 
> > @@ -50,12 +53,29 @@ \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}
> > +
> > +The driver SHOULD query the physical address ranges where the pmem was mapped.
> 
> Isn't the driver quite useless if it doesn't discover where the pmem can
> be found? I'm not sure we need that sentence.

It's about as useful as a paperweight if it doesn't discover where the pmem
was mapped :) Agreed, this isn't necessary.
 
> > +If VIRTIO_PMEM_F_SHMEM_REGION has been negotiated, the driver SHOULD query
> > +shared memory ID 0 for the physical address ranges and MUST NOT use \field{start}
> > +or \field{size}. Else, the driver SHOULD read the physical address ranges from
> > +\field{start} and \field{size}.
> 
> 'SHOULD' is probably not the right way to specify this. What about the
> following:
> 
> "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}."

Sounds great to me. Will update in v3. Thanks again for the advice.

Thanks,
Taylor