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

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

On Tue, Apr 01, 2025 at 09:37:21AM +0000, Parav Pandit wrote:
> > From: Sergio Lopez Pascual <slp@redhat.com>
> > Sent: Tuesday, April 1, 2025 3:04 PM
> > 
> > Parav Pandit <parav@nvidia.com> writes:
> > 
> > >> From: Michael S. Tsirkin <mst@redhat.com>
> > >> Sent: Tuesday, April 1, 2025 2:45 PM
> > >>
> > >> On Mon, Mar 31, 2025 at 09:40:37AM -0700, Sergio Lopez Pascual wrote:
> > >> > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > >> >
> > >> > > On Mon, Mar 31, 2025 at 11:05:46AM -0400, Sergio Lopez wrote:
> > >> > >> Add a subsection for page alignment restrictions.
> > >> > >>
> > >> > >> Signed-off-by: Sergio Lopez <slp@redhat.com>
> > >> > >> ---
> > >> > >>  shared-mem.tex | 6 ++++++
> > >> > >>  1 file changed, 6 insertions(+)
> > >> > >>
> > >> > >> diff --git a/shared-mem.tex b/shared-mem.tex index
> > >> > >> 6e6f6c4..2021083
> > >> > >> 100644
> > >> > >> --- a/shared-mem.tex
> > >> > >> +++ b/shared-mem.tex
> > >> > >> @@ -34,6 +34,12 @@ \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}
> > >> > >> +
> > >> > >> +When requesting the device to map memory into a shared memory
> > >> > >> +region the driver MUST obtain the page size information from
> > >> > >> +the transport and honor the page alignment constrains derived
> > >> > >> +from that
> > >> page size.
> > >> > >> +
> > >> > >
> > >> > > will make existing drivers autmatically incompliant with new devices.
> > >> > >
> > >> > > we generally avoid things like this.
> > >> >
> > >> > This is the reason why in v1 this was gated behind
> > >> > VIRTIO_F_SHM_PAGE_SIZE. But from the discussion on that thread, I
> > >> > understood that, at least for the PCI transport, it was preferred
> > >> > to have the page_shift field unconditionally:
> > >> >
> > >> > https://lore.kernel.org/virtio-
> > >> comment/CY8PR12MB71950314C5683EB7CFF081
> > >> > 5BDCCA2@CY8PR12MB7195.namprd12.prod.outlook.com/
> > >> >
> > >> > Did I misunderstand the conclusions?
> > >> >
> > >> > Sergio.
> > >>
> > >> I think Parav missed this compatibility implication when he said:
> > >>
> > >> 	So to obtain a free information, feature bit is not necessary, this
> > >> can be self-
> > >> 	described in the pci capability itself.
> > >>
> > >> Parav, this is about the MUST requirement referring to honoring
> > >> alignment, not about obtaining the information which indeed can be done
> > without.
> > >>
> > > It can't be MUST requirement as it breaks the driver which will not negotiate
> > this feature bit.
> > > So my understanding is, the device indicates one or multiple alignments it
> > supports and driver can choose which alignment it wants to use to get better
> > performance.
> > 
> > Not exactly. The device indicates its supported page size (just one), which
> > implies the driver needs to honor the alignment restrictions derived from that
> > page size.
> > 
> A device may not know on which OS flavor or which cpu flavor it may run.
> So it is better for the device to expose multiple supported page sizes alignment and driver can choose one configured on the driver side.

I doubt it's a thing though. Device has minimal alignment it can support
and that's it.  What are "multiple page sizes" and why would device
care?


> Keep in mind that often device != host (device can be actual device too).
> 
> When the driver page size is bigger, it can always align to the smaller page size told by the device, no?

What if it's smaller?


> > Just to clarify, this is not a performance improvement. Today, there are
> > devices (i.e. virtio-gpu) that doesn't work when the host (device) is running
> > with on a system with a bigger page size than the guest (driver).
> > 
> > Thanks,
> > Sergio.
>