Re: [PATCH v3] virtio-gpu: support blob alignment information

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

On Thu, Apr 10, 2025 at 10:34:56AM -0400, Sergio Lopez wrote:
> There's an increasing number of machines supporting multiple page sizes
> and, on these machines, the host and a guest can be running with
> different pages sizes. In addition to this, there might be physical
> devices that require to operate their memory at a specific granularity.
> 
> In these cases, if they are to use Shared Memory Regions, the device
> and the driver must operate with the same granularity, as otherwise
> the former might not be able to fulfill the requests sent by the
> latter.
> 
> For the GPU device, this has an impact on blob creation and mapping. To
> address the problem, allow the device to require certain alignment
> constrains for blob resources by extending the device configuration
> with the field "blob_alignment" and introducing the
> VIRTIO_GPU_F_BLOB_ALIGNMENT feature.
> 
> Signed-off-by: Sergio Lopez <slp@redhat.com>

Looks good to me now.

> ---
> 
> v3:
>  - Add driver conformance statements (thanks Michael).
>  - Avoid using "must" outside conformance statements (thanks Michael).
>  - Elaborate use case in commit description (thanks Michael).
>  - Drop "padding" field requirements (thanks Daniel).
> 
> v2:
>  - Clarify blob_alignment valid values (thanks Michael).
>  - Rephrase feature negotiation (thanks Michael).
> 
>  conformance.tex                  |  2 ++
>  device-types/gpu/description.tex | 28 ++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/conformance.tex b/conformance.tex
> index dc00e84..0aeb406 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -22,6 +22,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \ref{sec:Conformance / Driver Conformance / Entropy Driver Conformance},
>  \ref{sec:Conformance / Driver Conformance / Traditional Memory Balloon Driver Conformance},
>  \ref{sec:Conformance / Driver Conformance / SCSI Host Driver Conformance},
> +\ref{sec:Conformance / Driver Conformance / GPU Driver Conformance},
>  \ref{sec:Conformance / Driver Conformance / Input Driver Conformance},
>  \ref{sec:Conformance / Driver Conformance / Crypto Driver Conformance},
>  \ref{sec:Conformance / Driver Conformance / Socket Driver Conformance},
> @@ -140,6 +141,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \input{device-types/entropy/driver-conformance.tex}
>  \input{device-types/balloon/driver-conformance.tex}
>  \input{device-types/scsi/driver-conformance.tex}
> +\input{device-types/gpu/driver-conformance.tex}
>  \input{device-types/input/driver-conformance.tex}
>  \input{device-types/crypto/driver-conformance.tex}
>  \input{device-types/vsock/driver-conformance.tex}
> diff --git a/device-types/gpu/description.tex b/device-types/gpu/description.tex
> index 4435248..af54cf1 100644
> --- a/device-types/gpu/description.tex
> +++ b/device-types/gpu/description.tex
> @@ -37,6 +37,8 @@ \subsection{Feature bits}\label{sec:Device Types / GPU Device / Feature bits}
>    resources is supported.
>  \item[VIRTIO_GPU_F_CONTEXT_INIT (4)] multiple context types and
>    synchronization timelines supported.  Requires VIRTIO_GPU_F_VIRGL.
> +\item[VIRTIO_GPU_F_BLOB_ALIGNMENT (5)] configuration field
> +  \field{blob_alignment} is valid. Requires VIRTIO_GPU_F_RESOURCE_BLOB.
>  \end{description}
>  
>  \subsection{Device configuration layout}\label{sec:Device Types / GPU Device / Device configuration layout}
> @@ -52,6 +54,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / GPU Device / D
>          le32 events_clear;
>          le32 num_scanouts;
>          le32 num_capsets;
> +        le32 blob_alignment;
>  };
>  \end{lstlisting}
>  
> @@ -67,6 +70,9 @@ \subsubsection{Device configuration fields}
>    supported by the device.  Minimum value is 1, maximum value is 16.
>  \item[\field{num_capsets}] specifies the maximum number of capability
>    sets supported by the device.  The minimum value is zero.
> +\item[\field{blob_alignment}] specifies the minimal alignment, in bytes,
> +  required by the device for resource blobs.  The value is a power of
> +  two.  Minimum value is 1, maximum value is 4294967296.
>  \end{description}
>  
>  \subsubsection{Events}
> @@ -672,6 +678,9 @@ \subsubsection{Device Operation: controlq}\label{sec:Device Types / GPU Device /
>  resource. There is no restriction on the image/buffer view the driver
>  has on the blob resource.
>  
> +If VIRTIO_GPU_F_BLOB_ALIGNMENT has been negotiated, \field{size} is aligned
> +to \field{blob_alignment} as defined in the device configuration.
> +
>  \item[VIRTIO_GPU_CMD_SET_SCANOUT_BLOB] sets scanout parameters for a
>     blob resource. Request data is
>    \field{struct virtio_gpu_set_scanout_blob}. Response type is
> @@ -700,6 +709,14 @@ \subsubsection{Device Operation: controlq}\label{sec:Device Types / GPU Device /
>  
>  \end{description}
>  
> +\drivernormative{\paragraph}{Device Operation: controlq}{Device Types / GPU Device / Device Operation / Device Operation: controlq}
> +
> +\begin{itemize}
> +\item If VIRTIO_GPU_F_BLOB_ALIGNMENT has been negotiated, in
> +VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB requests \field{size} MUST be aligned
> +to \field{blob_alignment} as defined in the device configuration.
> +\end{itemize}
> +
>  \subsubsection{Device Operation: controlq (3d)}\label{sec:Device Types / GPU Device / Device Operation / Device Operation: controlq (3d)}
>  
>  These commands are supported by the device if the VIRTIO_GPU_F_VIRGL
> @@ -776,6 +793,9 @@ \subsubsection{Device Operation: controlq (3d)}\label{sec:Device Types / GPU Dev
>  };
>  \end{lstlisting}
>  
> +If VIRTIO_GPU_F_BLOB_ALIGNMENT has been negotiated, \field{offset} is
> +aligned to \field{blob_alignment} as defined in the device configuration.
> +
>  \item[VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB] unmaps a
>    host-only blob resource from the host visible memory region. Request data
>    is \field{struct virtio_gpu_resource_unmap_blob}.  Response type is
> @@ -793,6 +813,14 @@ \subsubsection{Device Operation: controlq (3d)}\label{sec:Device Types / GPU Dev
>  
>  \end{description}
>  
> +\drivernormative{\paragraph}{Device Operation: controlq (3d)}{Device Types / GPU Device / Device Operation / Device Operation: controlq (3d)}
> +
> +\begin{itemize}
> +\item If VIRTIO_GPU_F_BLOB_ALIGNMENT has been negotiated, in
> +VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB requests \field{offset} MUST be aligned to
> +\field{blob_alignment} as defined in the device configuration.
> +\end{itemize}
> +
>  \subsubsection{Device Operation: cursorq}\label{sec:Device Types / GPU Device / Device Operation / Device Operation: cursorq}
>  
>  Both cursorq commands use the same command struct.
> -- 
> 2.49.0