Re: [PATCH v3 1/3] shared-mem: introduce page alignment restrictions

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Mar 31, 2025 at 05:37:09PM -0400, Sergio Lopez wrote:
> Add a subsection for page alignment restrictions and introduce the
> VIRTIO_F_SHM_PAGE_SIZE feature bit.
> 
> Signed-off-by: Sergio Lopez <slp@redhat.com>
> ---
>  content.tex    | 10 ++++++++--
>  shared-mem.tex |  7 +++++++
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/content.tex b/content.tex
> index c17ffa6..c8826a2 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -99,10 +99,10 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
>  \begin{description}
>  \item[0 to 23, and 50 to 127] Feature bits for the specific device type
>  
> -\item[24 to 41] Feature bits reserved for extensions to the queue and
> +\item[24 to 42] Feature bits reserved for extensions to the queue and
>    feature negotiation mechanisms, see \ref{sec:Reserved Feature Bits}
>  
> -\item[42 to 49, and 128 and above] Feature bits reserved for future extensions.
> +\item[43 to 49, and 128 and above] Feature bits reserved for future extensions.
>  \end{description}
>  
>  \begin{note}
> @@ -872,6 +872,12 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>  	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits} for
>  	handling features reserved for future use.
>  
> +  \item[VIRTIO_F_SHM_PAGE_SIZE(42)] This feature indicates that the device
> +  transport provides information about the supported page size and expects the
> +  driver to honor the alignment restrictions derived from it when requesting
> +  mappings on the corresponding shared memory region.
> +  See also \ref {sec:Basic Facilities of a Virtio Device / Shared Memory Regions / Page alignment restrictions}.
> +
>  \end{description}
>  
>  \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
> diff --git a/shared-mem.tex b/shared-mem.tex
> index 6e6f6c4..dd90cb7 100644
> --- a/shared-mem.tex
> +++ b/shared-mem.tex
> @@ -34,6 +34,13 @@ \subsection{Addressing within regions}\label{sec:Basic Facilities of a Virtio De
>  The \field{shmid} may be explicit or may be inferred from the
>  context of the reference.
>  
> +\subsection{Page alignment restrictions}\label{sec:Basic Facilities of a Virtio Device / Shared Memory Regions / Page alignment restrictions}
> +
> +If VIRTIO_F_SHM_PAGE_SIZE has been negotiated, when requesting
> +the device to map memory into a shared memory region, the driver

the shared memory region - it is this specific one.

> +MUST obtain the page size information from the transport

avoid MUST. Also not "from the transport". In a transport-specific way.

> and honor
> +the page alignment constrains derived from that page size.

Also explain here it is only supported for pci for now, to make
sure people do not go looking for it on other transports.

> +
>  \devicenormative{\subsection}{Shared Memory Regions}{Basic Facilities of a Virtio
>  Device / Shared Memory Regions}
>  Shared memory regions MUST NOT expose shared memory regions which



I have a problem with this wording in that "map" and "request mapping"
is actually a device type specific thing. Someone reading this
paragraph will not know what this term means. Note also that the
term "map" has two meanings, e.g. device-types/fs/description.tex
says 
	In cases where data transfer is
	undesirable, the device can map file contents into the DAX window shared memory
	region. 
but also
	Drivers map this shared
	memory region with writeback caching as if it were regular RAM. 

I am guessing the "request mapping" refers to the 1st but not the 2nd
type of map, but neither mentions "request mapping".

My suggestions, but really just suggestions:

- update device specific text to mention alignment where appropriate,
  using MUST there.

- Remove the MUST here, it is not good to have MUST outside non
conformance sections anyway.

- Avoid saying map since that is not
a generic term.
Just say generally
"information about the supported page size" and put all
the detail in device specific sections.
Also "device expects" is really a weird way to put things.
device does abc. driver does abc. that is all.

E.g. 
	When VIRTIO_F_SHM_PAGE_SIZE is negotiated, driver aligns
	device requests
	using shared memory regions on the supported page size of each
	memory region, in a device-specific way. The supported
	page size is retrieved in a transport-specific way. This feature
	is only supported for the PCI transport (see ref to the relevant section).


And add device specific text where it applies.


> -- 
> 2.49.0