Re: [PATCH v2 2/3] drm/virtio: honor blob_alignment requirements

[Dmitry Osipenko <dmitry.osipenko@collabora.com>]

On 7/13/26 14:19, Alyssa Ross wrote:
> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:
> 
>> On 7/1/26 14:10, Alyssa Ross wrote:
>>> On Tue, Apr 28, 2026 at 09:44:49PM +0200, Sergio Lopez wrote:
>>>> If VIRTIO_GPU_F_BLOB_ALIGNMENT has been negotiated, blob size must be
>>>> aligned to blob_alignment. Validate this in verify_blob() so that
>>>> invalid requests are rejected early.
>>>>
>>>> Signed-off-by: Sergio Lopez <slp@redhat.com>
>>>
>>> FYI: this change breaks crosvm, which is squatting the 5 and 6 values
>>> of VIRTIO_GPU_F_* with different meanings.  I've reported it as a
>>> crosvm bug, so hopefully it can be taken care of there.
>>>
>>> https://issuetracker.google.com/issues/529852979
>>>
>>>> ---
>>>>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>>>> index c33c057365f8..d0c4edf1eaf4 100644
>>>> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>>>> @@ -489,6 +489,11 @@ static int verify_blob(struct virtio_gpu_device *vgdev,
>>>>  	params->size = rc_blob->size;
>>>>  	params->blob = true;
>>>>  	params->blob_flags = rc_blob->blob_flags;
>>>> +
>>>> +	if (vgdev->has_blob_alignment &&
>>>> +	    !IS_ALIGNED(params->size, vgdev->blob_alignment))
>>>> +		return -EINVAL;
>>>> +
>>>>  	return 0;
>>>>  }
>>>>
>>>> --
>>>> 2.53.0
>>>>
>>
>> Thanks for the report. Indeed, crosvm will need to fix its experimental
>> caps. CI will likely run into this problem first once it will update
>> guest to 7.2+ kernel.
> 
> I sent
> https://chromium-review.googlesource.com/c/crosvm/crosvm/+/8064741 last
> week but still waiting to hear anything.  Do you have any insight into
> whether it's still used / being pursued?

The GUEST_HANDLE should be used by Android [1]. The FENCE_PASSING
shouldn't be used in production, though don't know for sure.

Crosvm should bump its experimental defines by +10 to prevent clash with
upstream in future.

[1]
https://github.com/google/gfxstream/blob/main/host/vulkan/vk_decoder_global_state.cpp#L107

-- 
Best regards,
Dmitry