On 12/21/23 13:00, Julia Zhang wrote: > From: Daniel Stone <daniels@xxxxxxxxxxxxx> > > Add a new ioctl to allow the guest VM to discover how the guest > actually allocated the underlying buffer, which allows buffers to > be used for GL<->Vulkan interop and through standard window systems. > It's also a step towards properly supporting modifiers in the guest. > > Signed-off-by: Daniel Stone <daniels@xxxxxxxxxxxxx> > Co-developed-by: Julia Zhang <julia.zhang@xxxxxxx> # support query > stride before it's created > Signed-off-by: Julia Zhang <julia.zhang@xxxxxxx> > --- > drivers/gpu/drm/virtio/virtgpu_drv.c | 1 + > drivers/gpu/drm/virtio/virtgpu_drv.h | 22 ++++++++- > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 66 ++++++++++++++++++++++++++ > drivers/gpu/drm/virtio/virtgpu_kms.c | 8 +++- > drivers/gpu/drm/virtio/virtgpu_vq.c | 63 ++++++++++++++++++++++++ > include/uapi/drm/virtgpu_drm.h | 21 ++++++++ > include/uapi/linux/virtio_gpu.h | 30 ++++++++++++ > 7 files changed, 208 insertions(+), 3 deletions(-) ... > +static int virtio_gpu_resource_query_layout_ioctl(struct drm_device *dev, > + void *data, > + struct drm_file *file) > +{ > + struct drm_virtgpu_resource_query_layout *args = data; > + struct virtio_gpu_device *vgdev = dev->dev_private; > + struct drm_gem_object *obj = NULL; > + struct virtio_gpu_object *bo = NULL; > + struct virtio_gpu_query_info bo_info = {0}; > + int ret = 0; > + int i; > + > + if (!vgdev->has_resource_query_layout) { > + DRM_ERROR("failing: no RQL on host\n"); Please remove this message > + return -EINVAL; return -ENOSYS > + } > + > + if (args->handle > 0) { > + obj = drm_gem_object_lookup(file, args->handle); > + if (obj == NULL) { > + DRM_ERROR("invalid handle 0x%x\n", args->handle); > + return -ENOENT; > + } > + bo = gem_to_virtio_gpu_obj(obj); > + } > + > + ret = virtio_gpu_cmd_get_resource_layout(vgdev, &bo_info, args->width, > + args->height, args->format, > + args->bind, bo ? bo->hw_res_handle : 0); What this special hw_res_handle=0 is doing? Why is it needed? > + if (ret) > + goto out; > + > + ret = wait_event_timeout(vgdev->resp_wq, > + atomic_read(&bo_info.is_valid), > + 5 * HZ); > + if (!ret) > + goto out; > + > +valid: > + smp_rmb(); > + WARN_ON(atomic_read(&bo_info.is_valid)); Please remove this WARN_ON and fix the kernelbot report > + args->num_planes = bo_info.num_planes; > + args->modifier = bo_info.modifier; > + for (i = 0; i < args->num_planes; i++) { > + args->planes[i].offset = bo_info.planes[i].offset; > + args->planes[i].stride = bo_info.planes[i].stride; > + } > + for (; i < VIRTIO_GPU_MAX_RESOURCE_PLANES; i++) { > + args->planes[i].offset = 0; > + args->planes[i].stride = 0; > + } > + ret = 0; ret is already 0 here > +out: > + if (obj) > + drm_gem_object_put(obj); > + return ret; > +} ... > diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h > index f556fde07b76..547575232376 100644 > --- a/include/uapi/linux/virtio_gpu.h > +++ b/include/uapi/linux/virtio_gpu.h > @@ -65,6 +65,11 @@ > */ > #define VIRTIO_GPU_F_CONTEXT_INIT 4 > > +/* > + * VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT > + */ > +#define VIRTIO_GPU_F_RESOURCE_QUERY_LAYOUT 5 > + > enum virtio_gpu_ctrl_type { > VIRTIO_GPU_UNDEFINED = 0, > > @@ -95,6 +100,7 @@ enum virtio_gpu_ctrl_type { > VIRTIO_GPU_CMD_SUBMIT_3D, > VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB, > VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB, > + VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT, > > /* cursor commands */ > VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300, > @@ -108,6 +114,7 @@ enum virtio_gpu_ctrl_type { > VIRTIO_GPU_RESP_OK_EDID, > VIRTIO_GPU_RESP_OK_RESOURCE_UUID, > VIRTIO_GPU_RESP_OK_MAP_INFO, > + VIRTIO_GPU_RESP_OK_RESOURCE_LAYOUT, > > /* error responses */ > VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200, > @@ -453,4 +460,27 @@ struct virtio_gpu_resource_unmap_blob { > __le32 padding; > }; > > +/* VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT */ > +struct virtio_gpu_resource_query_layout { > + struct virtio_gpu_ctrl_hdr hdr; > + __le32 resource_id; > + __le32 width; > + __le32 height; > + __le32 format; > + __le32 bind; 64b pad missing > +}; > + > + > +/* VIRTIO_GPU_RESP_OK_RESOURCE_LAYOUT */ > +#define VIRTIO_GPU_RES_MAX_PLANES 4 > +struct virtio_gpu_resp_resource_layout { > + struct virtio_gpu_ctrl_hdr hdr; > + __le64 modifier; > + __le32 num_planes; > + struct virtio_gpu_resource_plane { > + __le64 offset; > + __le32 stride; > + } planes[VIRTIO_GPU_RES_MAX_PLANES]; > +}; Virto-spec changes should have a corresponding doc update in [1]. [1] https://github.com/oasis-tcs/virtio-spec/blob/master/device-types/gpu/description.tex -- Best regards, Dmitry