[PATCH] virtio-gpu: support blob alignment information

[PATCH] virtio-gpu: support blob alignment information

[Sergio Lopez]

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>
---
 device-types/gpu/description.tex | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/device-types/gpu/description.tex b/device-types/gpu/description.tex
index 4435248..8e5258d 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,8 @@ \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 byte alignment required by
+  the device for resource blobs.
 \end{description}
 
 \subsubsection{Events}
@@ -672,6 +677,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 is set, \field{size} must be 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
@@ -776,6 +784,10 @@ \subsubsection{Device Operation: controlq (3d)}\label{sec:Device Types / GPU Dev
 };
 \end{lstlisting}
 
+If VIRTIO_GPU_F_BLOB_ALIGNMENT is set, \field{padding} and \field{offset}
+must be 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
-- 
2.49.0

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

[Michael S. Tsirkin]

On Tue, Apr 01, 2025 at 01:42:03PM -0400, Sergio Lopez wrote:
> 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>
> ---
>  device-types/gpu/description.tex | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/device-types/gpu/description.tex b/device-types/gpu/description.tex
> index 4435248..8e5258d 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,8 @@ \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 byte alignment required by
> +  the device for resource blobs.

byte alignment means no alignment restrictions.
I think "alignment, in bytes"?
I also think you mean the minimal alignment.

Also please specify the legal values, like other fields do.
Is 2G alignment always going to be sufficient, in your opinion?

>  \end{description}
>  
>  \subsubsection{Events}
> @@ -672,6 +677,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 is set, \field{size} must be aligned to
> +\field{blob_alignment} as defined in the device configuration.
> +

set where? you mean has been negotiated?

>  \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
> @@ -776,6 +784,10 @@ \subsubsection{Device Operation: controlq (3d)}\label{sec:Device Types / GPU Dev
>  };
>  \end{lstlisting}
>  
> +If VIRTIO_GPU_F_BLOB_ALIGNMENT is set,

same

> \field{padding} and \field{offset}
> +must be 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
> -- 
> 2.49.0

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

[Sergio Lopez Pascual]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Tue, Apr 01, 2025 at 01:42:03PM -0400, Sergio Lopez wrote:
>> 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>
>> ---
>>  device-types/gpu/description.tex | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/device-types/gpu/description.tex b/device-types/gpu/description.tex
>> index 4435248..8e5258d 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,8 @@ \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 byte alignment required by
>> +  the device for resource blobs.
>
> byte alignment means no alignment restrictions.
> I think "alignment, in bytes"?
> I also think you mean the minimal alignment.

OK, I'll replace it with "(...) minimal alignment, in bytes (...)"

> Also please specify the legal values, like other fields do.
> Is 2G alignment always going to be sufficient, in your opinion?

We know it must be a power of two, and the minimum value is 1 (2^0), but
I'm not sure what to specify as maximum value. Should we state that is
4294967296 (2^32) even if we know it doesn't make sense in practice?

>>  \end{description}
>>
>>  \subsubsection{Events}
>> @@ -672,6 +677,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 is set, \field{size} must be aligned to
>> +\field{blob_alignment} as defined in the device configuration.
>> +
>
> set where? you mean has been negotiated?

Here I was just mimicking the style from the previous paragraph:

---
If VIRTIO_GPU_F_VIRGL is set, both VIRTIO_GPU_CMD_TRANSFER_TO_HOST_3D
and VIRTIO_GPU_CMD_TRANSFER_FROM_HOST_3D may be used to update the
resource. There is no restriction on the image/buffer view the driver
has on the blob resource.
---

Thanks!
Sergio.