This patchset implements the current proposal for virtio cross-device resource sharing [1], with minor changes based on recent comments. It is expected that this will be used to import virtio resources into the virtio-video driver currently under discussion [2].
This patchset adds a new hook to dma-buf, for querying the dma-buf's underlying virtio UUID. This hook is then plumbed through DRM PRIME buffers, and finally implemented in virtgpu.
[1] https://markmail.org/thread/jsaoqy7phrqdcpqu [2] https://markmail.org/thread/p5d3k566srtdtute
v1 -> v2 changes: - Move get_uuid callback into main dma-buf ops (instead of placing it in a new flavor of dma-buf). - Rename new virtio commands and feature flag, and pull uapi changes into their own patch.
David Stevens (4): dma-buf: add support for virtio exported objects drm/prime: add support for virtio exported objects virtio-gpu: add VIRTIO_GPU_F_RESOURCE_UUID feature drm/virtio: Support virtgpu exported resources
drivers/dma-buf/dma-buf.c | 14 ++++++ drivers/gpu/drm/drm_prime.c | 27 +++++++++++ drivers/gpu/drm/virtio/virtgpu_drv.c | 3 ++ drivers/gpu/drm/virtio/virtgpu_drv.h | 19 ++++++++ drivers/gpu/drm/virtio/virtgpu_kms.c | 4 ++ drivers/gpu/drm/virtio/virtgpu_prime.c | 48 ++++++++++++++++++-- drivers/gpu/drm/virtio/virtgpu_vq.c | 62 ++++++++++++++++++++++++++ include/drm/drm_drv.h | 15 +++++++ include/linux/dma-buf.h | 22 +++++++++ include/uapi/linux/virtio_gpu.h | 19 ++++++++ 10 files changed, 230 insertions(+), 3 deletions(-)
This change adds a new dma-buf operation that allows dma-bufs to be used by virtio drivers to share exported objects. The new operation allows the importing driver to query the exporting driver for the UUID which identifies the underlying exported object.
Signed-off-by: David Stevens stevensd@chromium.org --- drivers/dma-buf/dma-buf.c | 14 ++++++++++++++ include/linux/dma-buf.h | 22 ++++++++++++++++++++++ 2 files changed, 36 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index d4097856c86b..a04632284ec2 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1158,6 +1158,20 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr) } EXPORT_SYMBOL_GPL(dma_buf_vunmap);
+#ifdef CONFIG_VIRTIO +int dma_buf_get_uuid(struct dma_buf *dmabuf, uuid_t *uuid) +{ + if (WARN_ON(!dmabuf) || !uuid) + return -EINVAL; + + if (!dmabuf->ops->get_uuid) + return -ENODEV; + + return dmabuf->ops->get_uuid(dmabuf, uuid); +} +EXPORT_SYMBOL_GPL(dma_buf_get_uuid); +#endif + #ifdef CONFIG_DEBUG_FS static int dma_buf_debug_show(struct seq_file *s, void *unused) { diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index abf5459a5b9d..f5fecf8abe6a 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -251,6 +251,23 @@ struct dma_buf_ops {
void *(*vmap)(struct dma_buf *); void (*vunmap)(struct dma_buf *, void *vaddr); + +#ifdef CONFIG_VIRTIO + /** + * @get_uuid + * + * This is called by dma_buf_get_uuid to get the UUID which identifies + * the buffer to virtio devices. + * + * This callback is optional. + * + * Returns: + * + * 0 on success or a negative error code on failure. On success uuid + * will be populated with the buffer's UUID. + */ + int (*get_uuid)(struct dma_buf *dmabuf, uuid_t *uuid); +#endif };
/** @@ -444,4 +461,9 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *, unsigned long); void *dma_buf_vmap(struct dma_buf *); void dma_buf_vunmap(struct dma_buf *, void *vaddr); + +#ifdef CONFIG_VIRTIO +int dma_buf_get_uuid(struct dma_buf *dmabuf, uuid_t *uuid); +#endif + #endif /* __DMA_BUF_H__ */
This change exposes dma-buf's get_uuid callback to PRIME drivers.
Signed-off-by: David Stevens stevensd@chromium.org --- drivers/gpu/drm/drm_prime.c | 27 +++++++++++++++++++++++++++ include/drm/drm_drv.h | 15 +++++++++++++++ 2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 86d9b0e45c8c..fc6e932a4fa6 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -779,6 +779,30 @@ int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma) } EXPORT_SYMBOL(drm_gem_dmabuf_mmap);
+#ifdef CONFIG_VIRTIO +/** + * drm_gem_dmabuf_get_uuid - dma_buf get_uuid implementation for GEM + * @dma_buf: buffer to query + * @uuid: uuid outparam + * + * Queries the buffer's virtio UUID. This can be used as the + * &dma_buf_ops.get_uuid callback. Calls into &drm_driver.gem_prime_get_uuid. + * + * Returns 0 on success or a negative error code on failure. + */ +int drm_gem_dmabuf_get_uuid(struct dma_buf *dma_buf, uuid_t *uuid) +{ + struct drm_gem_object *obj = dma_buf->priv; + struct drm_device *dev = obj->dev; + + if (!dev->driver->gem_prime_get_uuid) + return -ENODEV; + + return dev->driver->gem_prime_get_uuid(obj, uuid); +} +EXPORT_SYMBOL(drm_gem_dmabuf_get_uuid); +#endif + static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { .cache_sgt_mapping = true, .attach = drm_gem_map_attach, @@ -789,6 +813,9 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { .mmap = drm_gem_dmabuf_mmap, .vmap = drm_gem_dmabuf_vmap, .vunmap = drm_gem_dmabuf_vunmap, +#ifdef CONFIG_VIRTIO + .get_uuid = drm_gem_dmabuf_get_uuid, +#endif };
/** diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 77685ed7aa65..3cbe9aa6b44a 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -32,6 +32,10 @@
#include <drm/drm_device.h>
+#ifdef CONFIG_VIRTIO +#include <linux/uuid.h> +#endif + struct drm_file; struct drm_gem_object; struct drm_master; @@ -639,6 +643,17 @@ struct drm_driver { int (*gem_prime_mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
+#ifdef CONFIG_VIRTIO + /** + * @gem_prime_get_uuid + * + * get_uuid hook for GEM drivers. Retrieves the virtio uuid of the + * given GEM buffer. + */ + int (*gem_prime_get_uuid)(struct drm_gem_object *obj, + uuid_t *uuid); +#endif + /** * @dumb_create: *
This feature allows the guest to request a UUID from the host for a particular virtio_gpu resource. The UUID can then be shared with other virtio devices, to allow the other host devices to access the virtio_gpu's corresponding host resource.
Signed-off-by: David Stevens stevensd@chromium.org --- include/uapi/linux/virtio_gpu.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h index 0c85914d9369..9721d58b4d58 100644 --- a/include/uapi/linux/virtio_gpu.h +++ b/include/uapi/linux/virtio_gpu.h @@ -50,6 +50,10 @@ * VIRTIO_GPU_CMD_GET_EDID */ #define VIRTIO_GPU_F_EDID 1 +/* + * VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID + */ +#define VIRTIO_GPU_F_RESOURCE_UUID 2
enum virtio_gpu_ctrl_type { VIRTIO_GPU_UNDEFINED = 0, @@ -66,6 +70,7 @@ enum virtio_gpu_ctrl_type { VIRTIO_GPU_CMD_GET_CAPSET_INFO, VIRTIO_GPU_CMD_GET_CAPSET, VIRTIO_GPU_CMD_GET_EDID, + VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID,
/* 3d commands */ VIRTIO_GPU_CMD_CTX_CREATE = 0x0200, @@ -87,6 +92,7 @@ enum virtio_gpu_ctrl_type { VIRTIO_GPU_RESP_OK_CAPSET_INFO, VIRTIO_GPU_RESP_OK_CAPSET, VIRTIO_GPU_RESP_OK_EDID, + VIRTIO_GPU_RESP_OK_RESOURCE_UUID,
/* error responses */ VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200, @@ -340,4 +346,17 @@ enum virtio_gpu_formats { VIRTIO_GPU_FORMAT_R8G8B8X8_UNORM = 134, };
+/* VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID */ +struct virtio_gpu_resource_assign_uuid { + struct virtio_gpu_ctrl_hdr hdr; + __le32 resource_id; + __le32 padding; +}; + +/* VIRTIO_GPU_RESP_OK_RESOURCE_UUID */ +struct virtio_gpu_resp_resource_uuid { + struct virtio_gpu_ctrl_hdr hdr; + __u8 uuid[16]; +}; + #endif
Add support for UUID-based resource sharing mechanism to virtgpu. This implements the new virtgpu commands and hooks them up to dma-buf's get_uuid callback.
Signed-off-by: David Stevens stevensd@chromium.org --- drivers/gpu/drm/virtio/virtgpu_drv.c | 3 ++ drivers/gpu/drm/virtio/virtgpu_drv.h | 19 ++++++++ drivers/gpu/drm/virtio/virtgpu_kms.c | 4 ++ drivers/gpu/drm/virtio/virtgpu_prime.c | 48 ++++++++++++++++++-- drivers/gpu/drm/virtio/virtgpu_vq.c | 62 ++++++++++++++++++++++++++ 5 files changed, 133 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c index ab4bed78e656..776e6667042e 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.c +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c @@ -165,6 +165,7 @@ static unsigned int features[] = { VIRTIO_GPU_F_VIRGL, #endif VIRTIO_GPU_F_EDID, + VIRTIO_GPU_F_RESOURCE_UUID, }; static struct virtio_driver virtio_gpu_driver = { .feature_table = features, @@ -202,7 +203,9 @@ static struct drm_driver driver = { .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, .gem_prime_mmap = drm_gem_prime_mmap, + .gem_prime_export = virtgpu_gem_prime_export, .gem_prime_import_sg_table = virtgpu_gem_prime_import_sg_table, + .gem_prime_get_uuid = virtgpu_gem_prime_get_uuid,
.gem_create_object = virtio_gpu_create_object, .fops = &virtio_gpu_driver_fops, diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index af9403e1cf78..4be84de73d86 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -49,6 +49,11 @@ #define DRIVER_MINOR 1 #define DRIVER_PATCHLEVEL 0
+#define UUID_NOT_INITIALIZED 0 +#define UUID_INITIALIZING 1 +#define UUID_INITIALIZED 2 +#define UUID_INITIALIZATION_FAILED 3 + struct virtio_gpu_object_params { uint32_t format; uint32_t width; @@ -75,6 +80,9 @@ struct virtio_gpu_object {
bool dumb; bool created; + + int uuid_state; + uuid_t uuid; }; #define gem_to_virtio_gpu_obj(gobj) \ container_of((gobj), struct virtio_gpu_object, base.base) @@ -196,6 +204,7 @@ struct virtio_gpu_device { bool has_virgl_3d; bool has_edid; bool has_indirect; + bool has_resource_assign_uuid;
struct work_struct config_changed_work;
@@ -206,6 +215,8 @@ struct virtio_gpu_device { struct virtio_gpu_drv_capset *capsets; uint32_t num_capsets; struct list_head cap_cache; + + spinlock_t resource_export_lock; };
struct virtio_gpu_fpriv { @@ -338,6 +349,10 @@ void virtio_gpu_dequeue_fence_func(struct work_struct *work); void virtio_gpu_disable_notify(struct virtio_gpu_device *vgdev); void virtio_gpu_enable_notify(struct virtio_gpu_device *vgdev);
+int +virtio_gpu_cmd_resource_assign_uuid(struct virtio_gpu_device *vgdev, + struct virtio_gpu_object *bo); + /* virtio_gpu_display.c */ void virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev); void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev); @@ -366,6 +381,10 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, struct virtio_gpu_object **bo_ptr, struct virtio_gpu_fence *fence); /* virtgpu_prime.c */ +struct dma_buf *virtgpu_gem_prime_export(struct drm_gem_object *obj, + int flags); +int virtgpu_gem_prime_get_uuid(struct drm_gem_object *obj, + uuid_t *uuid); struct drm_gem_object *virtgpu_gem_prime_import_sg_table( struct drm_device *dev, struct dma_buf_attachment *attach, struct sg_table *sgt); diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 4009c2f97d08..5a2aeb6d2f35 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -134,6 +134,7 @@ int virtio_gpu_init(struct drm_device *dev) vgdev->dev = dev->dev;
spin_lock_init(&vgdev->display_info_lock); + spin_lock_init(&vgdev->resource_export_lock); ida_init(&vgdev->ctx_id_ida); ida_init(&vgdev->resource_ida); init_waitqueue_head(&vgdev->resp_wq); @@ -162,6 +163,9 @@ int virtio_gpu_init(struct drm_device *dev) if (virtio_has_feature(vgdev->vdev, VIRTIO_RING_F_INDIRECT_DESC)) { vgdev->has_indirect = true; } + if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_RESOURCE_UUID)) { + vgdev->has_resource_assign_uuid = true; + }
DRM_INFO("features: %cvirgl %cedid\n", vgdev->has_virgl_3d ? '+' : '-', diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c index 050d24c39a8f..12ceda34b4f9 100644 --- a/drivers/gpu/drm/virtio/virtgpu_prime.c +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c @@ -26,9 +26,51 @@
#include "virtgpu_drv.h"
-/* Empty Implementations as there should not be any other driver for a virtual - * device that might share buffers with virtgpu - */ +int virtgpu_gem_prime_get_uuid(struct drm_gem_object *obj, + uuid_t *uuid) +{ + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); + struct virtio_gpu_device *vgdev = obj->dev->dev_private; + + // The state should have changed when the buffer was exported. + WARN_ON(bo->uuid_state == UUID_NOT_INITIALIZED); + + wait_event(vgdev->resp_wq, bo->uuid_state != UUID_INITIALIZING); + if (bo->uuid_state != UUID_INITIALIZED) + return -ENODEV; + + uuid_copy(uuid, &bo->uuid); + + return 0; +} + +struct dma_buf *virtgpu_gem_prime_export(struct drm_gem_object *obj, + int flags) +{ + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); + struct virtio_gpu_device *vgdev = obj->dev->dev_private; + bool needs_init = false; + int ret = 0; + + if (vgdev->has_resource_assign_uuid) { + spin_lock(&vgdev->resource_export_lock); + if (bo->uuid_state == UUID_NOT_INITIALIZED) { + bo->uuid_state = UUID_INITIALIZING; + needs_init = true; + } + spin_unlock(&vgdev->resource_export_lock); + + if (needs_init) { + ret = virtio_gpu_cmd_resource_assign_uuid(vgdev, bo); + if (ret) + return ERR_PTR(ret); + } + } else { + bo->uuid_state = UUID_INITIALIZATION_FAILED; + } + + return drm_gem_prime_export(obj, flags); +}
struct drm_gem_object *virtgpu_gem_prime_import_sg_table( struct drm_device *dev, struct dma_buf_attachment *attach, diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index cfe9c54f87a3..e692098fc573 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -1111,3 +1111,65 @@ void virtio_gpu_cursor_ping(struct virtio_gpu_device *vgdev, memcpy(cur_p, &output->cursor, sizeof(output->cursor)); virtio_gpu_queue_cursor(vgdev, vbuf); } + +static void virtio_gpu_cmd_resource_uuid_cb(struct virtio_gpu_device *vgdev, + struct virtio_gpu_vbuffer *vbuf) +{ + struct virtio_gpu_resp_resource_uuid *resp = + (struct virtio_gpu_resp_resource_uuid *)vbuf->resp_buf; + struct virtio_gpu_object *obj = + (struct virtio_gpu_object *)vbuf->data_buf; + uint32_t resp_type = le32_to_cpu(resp->hdr.type); + + /* + * Keeps the data_buf, which points to this virtio_gpu_object, from + * getting kfree'd after this cb returns. + */ + vbuf->data_buf = NULL; + + spin_lock(&vgdev->resource_export_lock); + WARN_ON(obj->uuid_state != UUID_INITIALIZING); + + if (resp_type == VIRTIO_GPU_RESP_OK_RESOURCE_UUID && + obj->uuid_state == UUID_INITIALIZING) { + memcpy(&obj->uuid.b, resp->uuid, sizeof(obj->uuid.b)); + obj->uuid_state = UUID_INITIALIZED; + } else { + obj->uuid_state = UUID_INITIALIZATION_FAILED; + } + spin_unlock(&vgdev->resource_export_lock); + + drm_gem_object_put_unlocked(&obj->base.base); + wake_up_all(&vgdev->resp_wq); +} + +int +virtio_gpu_cmd_resource_assign_uuid(struct virtio_gpu_device *vgdev, + struct virtio_gpu_object *bo) +{ + struct virtio_gpu_resource_assign_uuid *cmd_p; + struct virtio_gpu_vbuffer *vbuf; + struct virtio_gpu_resp_resource_uuid *resp_buf; + + resp_buf = kzalloc(sizeof(*resp_buf), GFP_KERNEL); + if (!resp_buf) { + spin_lock(&vgdev->resource_export_lock); + bo->uuid_state = UUID_INITIALIZATION_FAILED; + spin_unlock(&vgdev->resource_export_lock); + return -ENOMEM; + } + + cmd_p = virtio_gpu_alloc_cmd_resp(vgdev, + virtio_gpu_cmd_resource_uuid_cb, &vbuf, sizeof(*cmd_p), + sizeof(struct virtio_gpu_resp_resource_uuid), resp_buf); + memset(cmd_p, 0, sizeof(*cmd_p)); + + cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID); + cmd_p->resource_id = cpu_to_le32(bo->hw_res_handle); + + /* Reuse the data_buf pointer for the object pointer. */ + vbuf->data_buf = bo; + drm_gem_object_get(&bo->base.base); + virtio_gpu_queue_ctrl_buffer(vgdev, vbuf); + return 0; +}
On Mon, Mar 2, 2020 at 4:15 AM David Stevens stevensd@chromium.org wrote:
cmd_p->hdr.ctx_id =
Before this completion of this hypercall, this resource can be considered context local, while afterward it can be considered "exported".
vbuf->objs?
Maybe I'm misunderstanding render contexts, but exporting a resource doesn't seem related to render contexts. The other resource management operations (e.g. creation, attaching a backing) don't take render contexts, and exporting a resource seems like the same sort of operation. It's not clear to me why exporting a resource would affect what render contexts a resource has been attached to, nor why the render contexts a resource has been attached to would affect exporting the resource. Also, from an implementation perspective, I don't see any struct virtio_gpu_fpriv to get the ctx_id from.
-David
On Tue, Mar 03, 2020 at 11:42:22AM +0900, David Stevens wrote:
It isn't indeed. Binding resources to contexts might need dma-buf imports/exports on the host side, but that is another story and not related to dma-buf exports inside the guest.
cheers, Gerd
Hi,
You can submit a fenced command, then wait on the fence here. Removes the need for UUID_INITIALIZING.
Also note that this function will be called only once, on the first export. When exporting the same object again drm will simply reuse the existing dmabuf. You can drop UUID_NOT_INITIALIZED and needs_init.
So you are left with only two uuid_state states. You could turn uuid into a pointer, so it gets only allocated when needed. Also uuid == NULL can be used for "uuid not available" then.
cheers, Gerd
On Wed, Mar 4, 2020 at 5:01 PM Gerd Hoffmann kraxel@redhat.com wrote:
Synchronously waiting is simper, but only doing the wait when trying to use the UUID can help to hide the latency of the virito commands. That can save quite a bit of time when setting up multiple buffers for a graphics pipeline, which I think is worthwhile.
-David
dri-devel@lists.freedesktop.org
-
David Stevens -
Gerd Hoffmann -
Gurchetan Singh