Re: [PATCH v8 1/4] drm/virtio-gpu: Add VIRTIO_GPU_CAPSET_ROCM capability

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

On 9/18/2026 7:27 PM, Akihiko Odaki wrote:
> On 2026/09/18 19:13, Huang, Honglei wrote:
>>
>>
>> This patch is for adding ROCm capset only, can be spited into another 
>> thread if it is needed.
>>
>> Regards,
>> Honglei
>>
>> On 9/18/2026 6:07 PM, sashiko-bot@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 2 potential 
>>> issue(s) to consider:
>>> - [Medium] virtio-gpu: premature UAPI exposure of unfinalized 
>>> capability ID
>>> - [Low] virtio-gpu: missing DRM UAPI alias for ROCM capability set
>>> -- 
>>>
>>> commit a394228d77eeafc9f1dcc36150380f8561f1ee49
>>> Author: Honglei Huang <honghuan@amd.com>
>>>
>>> drm/virtio-gpu: Add VIRTIO_GPU_CAPSET_ROCM capability
>>>
>>> This commit adds a new GPU capability set VIRTIO_GPU_CAPSET_ROCM with 
>>> ID 8
>>> to support ROCm compute workloads. The commit message notes that the
>>> matching virtio-gpu specification change is currently in progress.
>>>
>>>> diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/ 
>>>> virtio_gpu.h
>>>> index 4f530d90058c7..3d4dfadc9d789 100644
>>>> --- a/include/uapi/linux/virtio_gpu.h
>>>> +++ b/include/uapi/linux/virtio_gpu.h
>>>> @@ -321,6 +321,7 @@ struct virtio_gpu_cmd_submit {
>>>>   #define VIRTIO_GPU_CAPSET_VENUS 4
>>>>   #define VIRTIO_GPU_CAPSET_CROSS_DOMAIN 5
>>>>   #define VIRTIO_GPU_CAPSET_DRM 6
>>>> +#define VIRTIO_GPU_CAPSET_ROCM 8
>>>
>>> [Severity: Medium]
>>> Is it safe to expose this speculative capability ID to userspace 
>>> before the
>>> virtio-gpu specification is finalized?
>>>
>>> The commit message states the specification change for ID 8 is in 
>>> progress.
>>> Because the Linux UAPI is frozen once released, exposing this ID now 
>>> could
>>> permanently commit the kernel to ID 8 for ROCm. If the OASIS virtio
>>> technical committee assigns a different ID or rejects the proposal, 
>>> would
>>> this leave the Linux UAPI permanently incompatible with the standard?
> 
> It is actually somewhat common to introduce a capset before updating the 
> virtio specification, as capsets remain opaque to both the spec and the 
> kernel without strict interface stability enforcement (for example, 
> VIRTIO_GPU_CAPSET_DRM is still not in the spec). However, we must at 
> least wait until the virglrenderer developers agree to implement it.
> 
> An unfortunate complication is that gfxstream has already claimed 
> capsets 7–9 prior to updating the spec or kernel headers:
> 
> https://github.com/magma-gpu/rutabaga_gfx/blob/v0.1.85/ffi/src/include/ 
> rutabaga_gfx_ffi.h#L41-L43
> 
> You could target capset 10 instead, but I advise coordinating with the 
> VCL developers first to avoid further collisions.

Got it, will use 10 or 11 in next version.

> 
> Additionally, "[PATCH v8 2/4] drm/virtgpu api: add blob userptr 
> resource" poses a more critical issue: it adds 
> VIRTIO_GPU_F_BLOB_READONLY and VIRTIO_GPU_BLOB_FLAG_USE_READONLY, 
> directly altering semantics explicitly defined in the spec. We should 
> see at least a draft spec patch posted before moving forward, so this 
> series should remain an RFC until then.

Will change to RFC in next version.

the VIRTIO_GPU_F_BLOB_READONLY  flag is for ensure host side will handle
the readonly flag instead of ignoring that. Actually I think 
VIRTIO_GPU_BLOB_FLAG_USE_READONLY is enough, but AI review requests 
needs a VIRTIO_GPU_F_BLOB_READONLY  feature flag, maybe can remove it.


> 
>>>
>>> [Severity: Low]
>>> Also, should there be a corresponding DRM UAPI alias added for this 
>>> capability
>>> set?
>>>
>>> Previous capability sets have matching definitions in virtgpu_drm.h, 
>>> such as
>>> VIRTGPU_DRM_CAPSET_VENUS and VIRTGPU_DRM_CAPSET_DRM. Without adding a 
>>> similar
>>> VIRTGPU_DRM_CAPSET_ROCM alias, userspace DRM clients relying on 
>>> virtgpu_drm.h
>>> for the DRM_IOCTL_VIRTGPU_GET_CAPS ioctl will lack the necessary 
>>> constant to
>>> query ROCm capabilities natively.
> 
> As for Sashiko's suggestion regarding DRM UAPI aliases, while I am 
> unsure why those aliases were introduced initially, adding one here 
> makes sense for internal consistency.

Will add one.

Regards,
Honglei>
> Regards,
> Akihiko Odaki
> 
>>>
>>>>   /* VIRTIO_GPU_CMD_GET_CAPSET_INFO */
>>>>   struct virtio_gpu_get_capset_info {
>>>
>>
>