Re: [RFC PATCH v9 4/4] drm/virtio: wire blob ioctl creation to userptr objects

["Huang, Honglei" <honghuan@amd.com>]

On 9/27/2026 9:04 AM, Akihiko Odaki wrote:
> On 2026/09/24 18:55, Honglei Huang wrote:
>> Integrate userptr into the blob resource creation ioctl.
>>
>> - A non-zero userptr selects virtio_gpu_userptr_create()
>> - Reject userptr unless blob_mem is VIRTGPU_BLOB_MEM_GUEST
>> - Reject VIRTGPU_BLOB_FLAG_USE_READONLY unless the device
>>    advertised VIRTIO_GPU_F_BLOB_READONLY
>> - Advertise VIRTGPU_PARAM_USERPTR and VIRTGPU_PARAM_BLOB_READONLY
>> - Add VIRTIO_GPU_F_BLOB_READONLY to the driver's features[] array
>>
>> virtio_has_feature() routes device-specific feature bits through
>> virtio_check_driver_offered_feature(), which calls BUG() when the
>> driver queries a bit it never declared in features[]. The
>> VIRTIO_GPU_F_BLOB_READONLY probe added to virtio_gpu_init() hit
>> exactly that: every probe crashed regardless of host support because
>> the bit was missing from features[]. Registering it there fixes the
>> crash.
>>
>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>> ---
>>   drivers/gpu/drm/virtio/virtgpu_debugfs.c |  1 +
>>   drivers/gpu/drm/virtio/virtgpu_drv.c     |  1 +
>>   drivers/gpu/drm/virtio/virtgpu_ioctl.c   | 35 ++++++++++++++++++++----
>>   drivers/gpu/drm/virtio/virtgpu_kms.c     |  8 ++++--
>>   4 files changed, 38 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_debugfs.c b/drivers/gpu/ 
>> drm/virtio/virtgpu_debugfs.c
>> index 3a68a16b58..b8b9b40584 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_debugfs.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_debugfs.c
>> @@ -55,6 +55,7 @@ static int virtio_gpu_features(struct seq_file *m, 
>> void *data)
>>                   vgdev->has_resource_assign_uuid);
>>       virtio_gpu_add_bool(m, "blob resources", vgdev->has_resource_blob);
>> +    virtio_gpu_add_bool(m, "blob readonly", vgdev->has_blob_readonly);
>>       virtio_gpu_add_bool(m, "context init", vgdev->has_context_init);
>>       virtio_gpu_add_int(m, "cap sets", vgdev->num_capsets);
>>       virtio_gpu_add_int(m, "scanouts", vgdev->num_scanouts);
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/ 
>> virtio/virtgpu_drv.c
>> index 2aaa7cb080..f1edd20c37 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
>> @@ -182,6 +182,7 @@ static unsigned int features[] = {
>>       VIRTIO_GPU_F_RESOURCE_BLOB,
>>       VIRTIO_GPU_F_CONTEXT_INIT,
>>       VIRTIO_GPU_F_BLOB_ALIGNMENT,
>> +    VIRTIO_GPU_F_BLOB_READONLY,
>>   };
>>   #ifdef CONFIG_PM_SLEEP
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/ 
>> virtio/virtgpu_ioctl.c
>> index 3d8e4ccdb7..3dc058e50e 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>> @@ -36,7 +36,10 @@
>>   #define VIRTGPU_BLOB_FLAG_USE_MASK (VIRTGPU_BLOB_FLAG_USE_MAPPABLE | \
>>                       VIRTGPU_BLOB_FLAG_USE_SHAREABLE | \
>> -                    VIRTGPU_BLOB_FLAG_USE_CROSS_DEVICE)
>> +                    VIRTGPU_BLOB_FLAG_USE_CROSS_DEVICE | \
>> +                    VIRTGPU_BLOB_FLAG_USE_READONLY)
>> +
>> +#define VIRTGPU_BLOB_HINT_MASK DRM_VIRTGPU_BLOB_FLAG_HINT_DEFER_MAPPING
>>   /* Must be called with &virtio_gpu_fpriv.struct_mutex held. */
>>   static void virtio_gpu_create_context_locked(struct 
>> virtio_gpu_device *vgdev,
>> @@ -122,6 +125,12 @@ static int virtio_gpu_getparam_ioctl(struct 
>> drm_device *dev, void *data,
>>               return -ENOENT;
>>           value = vgdev->blob_alignment;
>>           break;
>> +    case VIRTGPU_PARAM_USERPTR:
>> +        value = 1;
>> +        break;
>> +    case VIRTGPU_PARAM_BLOB_READONLY:
>> +        value = vgdev->has_blob_readonly ? 1 : 0;
>> +        break;
>>       default:
>>           return -EINVAL;
>>       }
>> @@ -453,11 +462,23 @@ static int verify_blob(struct virtio_gpu_device 
>> *vgdev,
>>       if (rc_blob->blob_flags & ~VIRTGPU_BLOB_FLAG_USE_MASK)
>>           return -EINVAL;
>> +    if (rc_blob->blob_hints & ~VIRTGPU_BLOB_HINT_MASK)
>> +        return -EINVAL;
>> +
>>       if (rc_blob->blob_flags & VIRTGPU_BLOB_FLAG_USE_CROSS_DEVICE) {
>>           if (!vgdev->has_resource_assign_uuid)
>>               return -EINVAL;
>>       }
>> +    if (rc_blob->blob_flags & VIRTGPU_BLOB_FLAG_USE_READONLY) {
>> +        if (!vgdev->has_blob_readonly)
>> +            return -EINVAL;
>> +    }
>> +
>> +    if (rc_blob->userptr &&
>> +        rc_blob->blob_mem != VIRTGPU_BLOB_MEM_GUEST)
>> +        return -EINVAL;
>> +
>>       switch (rc_blob->blob_mem) {
>>       case VIRTGPU_BLOB_MEM_GUEST:
>>           *guest_blob = true;
>> @@ -495,6 +516,7 @@ static int verify_blob(struct virtio_gpu_device 
>> *vgdev,
>>       params->blob = true;
>>       params->blob_flags = rc_blob->blob_flags;
>>       params->blob_hints = rc_blob->blob_hints;
>> +    params->userptr = rc_blob->userptr;
>>       if (vgdev->has_blob_alignment &&
>>           !IS_ALIGNED(params->size, vgdev->blob_alignment))
>> @@ -518,9 +540,10 @@ static int 
>> virtio_gpu_resource_create_blob_ioctl(struct drm_device *dev,
>>       struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
>>       struct drm_virtgpu_resource_create_blob *rc_blob = data;
>> -    if (verify_blob(vgdev, vfpriv, &params, rc_blob,
>> -            &guest_blob, &host3d_blob))
>> -        return -EINVAL;
>> +    ret = verify_blob(vgdev, vfpriv, &params, rc_blob,
>> +              &guest_blob, &host3d_blob);
>> +    if (ret)
>> +        return ret;
>>       if (vgdev->has_virgl_3d)
>>           virtio_gpu_create_context(dev, file);
>> @@ -538,7 +561,9 @@ static int 
>> virtio_gpu_resource_create_blob_ioctl(struct drm_device *dev,
>>                         vfpriv->ctx_id, NULL, NULL);
>>       }
>> -    if (guest_blob)
>> +    if (guest_blob && params.userptr)
>> +        ret = virtio_gpu_userptr_create(vgdev, file, &params, &bo);
> 
> After userptr creation has pinned pages, charged pinned_vm, queued 
> resource creation, and registered the object for restore, the [handle- 
> creation failure path calls drm_gem_object_release() directly. The UUID- 
> assignment failure path does likewise.
> 
> That function releases GEM internals; it does not invoke the 
> object’s .free callback. The userptr pages, DMA mapping, memlock charge, 
> and host resource therefore remain unreleased, with no userspace handle 
> available to close them.
> 
> These error paths predate the series, but this patch routes long-term 
> userptr pins through them.

Will fix it in next version.

Regards,
Honglei

> 
> Regards,
> Akihiko Odaki
> 
>> +    else if (guest_blob)
>>           ret = virtio_gpu_object_create(vgdev, &params, &bo, NULL);
>>       else if (!guest_blob && host3d_blob)
>>           ret = virtio_gpu_vram_create(vgdev, &params, &bo);
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/ 
>> virtio/virtgpu_kms.c
>> index 1d4d3bf46a..06c2bded49 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
>> @@ -249,15 +249,19 @@ int virtio_gpu_init(struct virtio_device *vdev, 
>> struct drm_device *dev)
>>           vgdev->blob_alignment = blob_alignment;
>>       }
>> +    if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_BLOB_READONLY))
>> +        vgdev->has_blob_readonly = true;
>> +
>>       DRM_INFO("features: %cvirgl %cedid %cresource_blob %chost_visible",
>>            vgdev->has_virgl_3d    ? '+' : '-',
>>            vgdev->has_edid        ? '+' : '-',
>>            vgdev->has_resource_blob ? '+' : '-',
>>            vgdev->has_host_visible ? '+' : '-');
>> -    DRM_INFO("features: %ccontext_init %cblob_alignment\n",
>> +    DRM_INFO("features: %ccontext_init %cblob_alignment 
>> %cblob_readonly\n",
>>            vgdev->has_context_init ? '+' : '-',
>> -         vgdev->has_blob_alignment ? '+' : '-');
>> +         vgdev->has_blob_alignment ? '+' : '-',
>> +         vgdev->has_blob_readonly ? '+' : '-');
>>       ret = virtio_gpu_find_vqs(vgdev);
>>       if (ret) {
>