2020-02-19 08:08:35

by David Stevens

[permalink] [raw]
Subject: [PATCH 0/2] Support virtio cross-device resources

This patchset implements the current proposal for virtio cross-device
resource sharing [1], with minor changes based on recent comments (i.e.
renumbering the new virtio gpu command and adding a feature flag).

The patchset adds a new flavor of dma-bufs that supports querying the
underlying virtio object UUID, as well as adding support for exporting
resources from virtgpu. It is expected that this will be used to import
virtgpu resources into the virtio-video driver currently under
discussion [2].

[1] https://markmail.org/thread/jsaoqy7phrqdcpqu
[2] https://markmail.org/thread/p5d3k566srtdtute

David Stevens (2):
virtio: add dma-buf support for exported objects
drm/virtio: Support virtgpu exported resources

drivers/gpu/drm/virtio/virtgpu_drv.c | 3 +
drivers/gpu/drm/virtio/virtgpu_drv.h | 21 +++++
drivers/gpu/drm/virtio/virtgpu_kms.c | 4 +
drivers/gpu/drm/virtio/virtgpu_prime.c | 109 ++++++++++++++++++++++++-
drivers/gpu/drm/virtio/virtgpu_vq.c | 58 +++++++++++++
drivers/virtio/Makefile | 2 +-
drivers/virtio/virtio.c | 6 ++
drivers/virtio/virtio_dma_buf.c | 97 ++++++++++++++++++++++
include/linux/virtio.h | 1 +
include/linux/virtio_dma_buf.h | 62 ++++++++++++++
include/uapi/linux/virtio_gpu.h | 19 +++++
11 files changed, 378 insertions(+), 4 deletions(-)
create mode 100644 drivers/virtio/virtio_dma_buf.c
create mode 100644 include/linux/virtio_dma_buf.h

--
2.25.0.265.gbab2e86ba0-goog


2020-02-19 08:08:59

by David Stevens

[permalink] [raw]
Subject: [PATCH 2/2] drm/virtio: Support virtgpu exported resources

Add support for exported resources to virtgpu. This includes adding
support for the new virtgpu command as well as well as switching from
regular prime dma-bufs to virtio dma-bufs.

Signed-off-by: David Stevens <[email protected]>
---
drivers/gpu/drm/virtio/virtgpu_drv.c | 3 +
drivers/gpu/drm/virtio/virtgpu_drv.h | 21 +++++
drivers/gpu/drm/virtio/virtgpu_kms.c | 4 +
drivers/gpu/drm/virtio/virtgpu_prime.c | 109 ++++++++++++++++++++++++-
drivers/gpu/drm/virtio/virtgpu_vq.c | 58 +++++++++++++
include/uapi/linux/virtio_gpu.h | 19 +++++
6 files changed, 211 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index ab4bed78e656..538ed981fea9 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_CROSS_DEVICE,
};
static struct virtio_driver virtio_gpu_driver = {
.feature_table = features,
@@ -201,6 +202,8 @@ static struct drm_driver driver = {
#endif
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
+ .gem_prime_export = virtgpu_gem_prime_export,
+ .gem_prime_import = virtgpu_gem_prime_import,
.gem_prime_mmap = drm_gem_prime_mmap,
.gem_prime_import_sg_table = virtgpu_gem_prime_import_sg_table,

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index af9403e1cf78..0d272fc26bf2 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -27,6 +27,7 @@
#define VIRTIO_DRV_H

#include <linux/virtio.h>
+#include <linux/virtio_dma_buf.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_config.h>
#include <linux/virtio_gpu.h>
@@ -49,6 +50,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 +81,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 +205,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 +216,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 +350,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 +382,11 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object **bo_ptr,
struct virtio_gpu_fence *fence);
/* virtgpu_prime.c */
+extern const struct virtio_dma_buf_ops virtgpu_dmabuf_ops;
+struct dma_buf *virtgpu_gem_prime_export(struct drm_gem_object *obj,
+ int flags);
+struct drm_gem_object *virtgpu_gem_prime_import(struct drm_device *dev,
+ struct dma_buf *buf);
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..be9719fb457b 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);
@@ -159,6 +160,9 @@ int virtio_gpu_init(struct drm_device *dev)
if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_EDID)) {
vgdev->has_edid = true;
}
+ if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_CROSS_DEVICE)) {
+ vgdev->has_resource_assign_uuid = true;
+ }
if (virtio_has_feature(vgdev->vdev, VIRTIO_RING_F_INDIRECT_DESC)) {
vgdev->has_indirect = true;
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c
index 050d24c39a8f..667cd4e45bfd 100644
--- a/drivers/gpu/drm/virtio/virtgpu_prime.c
+++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
@@ -22,13 +22,116 @@
* Authors: Andreas Pokorny
*/

+#include <drm/drm_drv.h>
#include <drm/drm_prime.h>

+#include <linux/dma-buf.h>
+#include <linux/virtio_dma_buf.h>
+
#include "virtgpu_drv.h"

-/* Empty Implementations as there should not be any other driver for a virtual
- * device that might share buffers with virtgpu
- */
+static int virtgpu_virtio_get_uuid(struct dma_buf *buf,
+ uuid_t *uuid)
+{
+ struct drm_gem_object *obj = buf->priv;
+ struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
+ struct virtio_gpu_device *vgdev = obj->dev->dev_private;
+
+ if (bo->uuid_state == UUID_NOT_INITIALIZED)
+ return -ENODEV;
+
+ wait_event(vgdev->resp_wq, bo->uuid_state != UUID_INITIALIZING);
+ if (bo->uuid_state == UUID_INITIALIZATION_FAILED)
+ return -ENODEV;
+
+ uuid_copy(uuid, &bo->uuid);
+
+ return 0;
+}
+
+int virtgpu_virtio_attach(struct dma_buf *buf,
+ struct dma_buf_attachment *attach)
+{
+ struct drm_gem_object *obj = buf->priv;
+ 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)
+ return 0;
+
+ 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);
+ return ret;
+}
+
+const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = {
+ .ops = {
+ .cache_sgt_mapping = true,
+ .attach = virtio_dma_buf_attach,
+ .detach = drm_gem_map_detach,
+ .map_dma_buf = drm_gem_map_dma_buf,
+ .unmap_dma_buf = drm_gem_unmap_dma_buf,
+ .release = drm_gem_dmabuf_release,
+ .mmap = drm_gem_dmabuf_mmap,
+ .vmap = drm_gem_dmabuf_vmap,
+ .vunmap = drm_gem_dmabuf_vunmap,
+ },
+ .virtio_attach = virtgpu_virtio_attach,
+ .device_attach = drm_gem_map_attach,
+ .get_uuid = virtgpu_virtio_get_uuid,
+};
+
+struct dma_buf *virtgpu_gem_prime_export(struct drm_gem_object *obj,
+ int flags)
+{
+ struct dma_buf *buf;
+ struct drm_device *dev = obj->dev;
+ DEFINE_VIRTIO_DMA_BUF_EXPORT_INFO(exp_info);
+
+ exp_info.ops = &virtgpu_dmabuf_ops;
+ exp_info.size = obj->size;
+ exp_info.flags = flags;
+ exp_info.priv = obj;
+ exp_info.resv = obj->resv;
+
+ buf = virtio_dma_buf_export(&exp_info);
+ if (IS_ERR(buf))
+ return buf;
+
+ drm_dev_get(dev);
+ drm_gem_object_get(obj);
+
+ return buf;
+}
+
+struct drm_gem_object *virtgpu_gem_prime_import(struct drm_device *dev,
+ struct dma_buf *buf)
+{
+ struct drm_gem_object *obj;
+
+ if (buf->ops == &virtgpu_dmabuf_ops.ops) {
+ obj = buf->priv;
+ if (obj->dev == dev) {
+ /*
+ * Importing dmabuf exported from our own gem increases
+ * refcount on gem itself instead of f_count of dmabuf.
+ */
+ drm_gem_object_get(obj);
+ return obj;
+ }
+ }
+
+ return drm_gem_prime_import(dev, buf);
+}

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..e9e7b1f885f6 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -1111,3 +1111,61 @@ 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_assign_uuid_cb(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_vbuffer *vbuf)
+{
+ struct virtio_gpu_resp_resource_assign_uuid *resp =
+ (struct virtio_gpu_resp_resource_assign_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_ASSIGN_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_assign_uuid *resp_buf;
+
+ resp_buf = kzalloc(sizeof(*resp_buf), GFP_KERNEL);
+ if (!resp_buf)
+ return -ENOMEM;
+
+ cmd_p = virtio_gpu_alloc_cmd_resp(vgdev,
+ virtio_gpu_cmd_resource_assign_uuid_cb, &vbuf, sizeof(*cmd_p),
+ sizeof(struct virtio_gpu_resp_resource_assign_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;
+}
diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
index 0c85914d9369..9c428ef03060 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_CROSS_DEVICE 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_ASSIGN_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_ASSIGN_UUID */
+struct virtio_gpu_resp_resource_assign_uuid {
+ struct virtio_gpu_ctrl_hdr hdr;
+ __u8 uuid[16];
+};
+
#endif
--
2.25.0.265.gbab2e86ba0-goog

2020-02-19 08:09:42

by David Stevens

[permalink] [raw]
Subject: [PATCH 1/2] virtio: add dma-buf support for exported objects

This change adds a new flavor of dma-bufs that can be used by virtio
drivers to share exported objects. A virtio dma-buf can be queried by
virtio drivers to obtain the UUID which identifies the underlying
exported object.

Signed-off-by: David Stevens <[email protected]>
---
drivers/virtio/Makefile | 2 +-
drivers/virtio/virtio.c | 6 ++
drivers/virtio/virtio_dma_buf.c | 97 +++++++++++++++++++++++++++++++++
include/linux/virtio.h | 1 +
include/linux/virtio_dma_buf.h | 62 +++++++++++++++++++++
5 files changed, 167 insertions(+), 1 deletion(-)
create mode 100644 drivers/virtio/virtio_dma_buf.c
create mode 100644 include/linux/virtio_dma_buf.h

diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index 3a2b5c5dcf46..0ad30f4cdb9f 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o
+obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o virtio_dma_buf.o
obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o
obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index a977e32a88f2..5d46f0ded92d 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -357,6 +357,12 @@ int register_virtio_device(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(register_virtio_device);

+bool is_virtio_device(struct device *dev)
+{
+ return dev->bus == &virtio_bus;
+}
+EXPORT_SYMBOL_GPL(is_virtio_device);
+
void unregister_virtio_device(struct virtio_device *dev)
{
int index = dev->index; /* save for after device release */
diff --git a/drivers/virtio/virtio_dma_buf.c b/drivers/virtio/virtio_dma_buf.c
new file mode 100644
index 000000000000..1f7e2d50b189
--- /dev/null
+++ b/drivers/virtio/virtio_dma_buf.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * dma-bufs for virtio exported objects
+ *
+ * Copyright (C) 2020 Google, Inc.
+ */
+
+#include <linux/virtio_dma_buf.h>
+
+/**
+ * virtio_dma_buf_export - Creates a new dma-buf for a virtio exported object
+ *
+ * This wraps dma_buf_export() to allow virtio drivers to create a dma-buf
+ * for an virtio exported object that can be queried by other virtio drivers
+ * for the object's UUID.
+ */
+struct dma_buf *virtio_dma_buf_export(
+ const struct virtio_dma_buf_export_info *virtio_exp_info)
+{
+ struct dma_buf_export_info exp_info;
+
+ if (!virtio_exp_info->ops
+ || virtio_exp_info->ops->ops.attach != &virtio_dma_buf_attach
+ || !virtio_exp_info->ops->get_uuid) {
+ return ERR_PTR(-EINVAL);
+ }
+
+ exp_info.exp_name = virtio_exp_info->exp_name;
+ exp_info.owner = virtio_exp_info->owner;
+ exp_info.ops = &virtio_exp_info->ops->ops;
+ exp_info.size = virtio_exp_info->size;
+ exp_info.flags = virtio_exp_info->flags;
+ exp_info.resv = virtio_exp_info->resv;
+ exp_info.priv = virtio_exp_info->priv;
+ BUILD_BUG_ON(sizeof(struct virtio_dma_buf_export_info)
+ != sizeof(struct dma_buf_export_info));
+
+ return dma_buf_export(&exp_info);
+}
+EXPORT_SYMBOL(virtio_dma_buf_export);
+
+/**
+ * virtio_dma_buf_attach - mandatory attach callback for virtio dma-bufs
+ */
+int virtio_dma_buf_attach(struct dma_buf *dma_buf,
+ struct dma_buf_attachment *attach)
+{
+ int ret;
+ const struct virtio_dma_buf_ops *ops = container_of(
+ dma_buf->ops, const struct virtio_dma_buf_ops, ops);
+
+ if (ops->virtio_attach && is_virtio_device(attach->dev)) {
+ ret = ops->virtio_attach(dma_buf, attach);
+ if (ret)
+ return ret;
+ }
+
+ if (ops->device_attach) {
+ ret = ops->device_attach(dma_buf, attach);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+EXPORT_SYMBOL(virtio_dma_buf_attach);
+
+/**
+ * is_virtio_dma_buf - returns true if the given dma-buf is a virtio dma-buf
+ * @dma_buf: buffer to query
+ */
+bool is_virtio_dma_buf(struct dma_buf *dma_buf)
+{
+ return dma_buf->ops->attach == &virtio_dma_buf_attach;
+}
+EXPORT_SYMBOL(is_virtio_dma_buf);
+
+/**
+ * get_virtio_dma_buf_uuid - gets the uuid for the dma-buf's exported object
+ * @dma_buf: [in] buffer to query
+ * @uuid: [out] the uuid
+ *
+ * This function should only be called on dma_bufs attached to a virtio device.
+ *
+ * Returns: 0 on success, negative on failure.
+ */
+int get_virtio_dma_buf_uuid(struct dma_buf *dma_buf,
+ uuid_t *uuid)
+{
+ const struct virtio_dma_buf_ops *ops = container_of(
+ dma_buf->ops, const struct virtio_dma_buf_ops, ops);
+
+ if (!is_virtio_dma_buf(dma_buf))
+ return -EINVAL;
+
+ return ops->get_uuid(dma_buf, uuid);
+}
+EXPORT_SYMBOL(get_virtio_dma_buf_uuid);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 15f906e4a748..9397e25616c4 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -128,6 +128,7 @@ static inline struct virtio_device *dev_to_virtio(struct device *_dev)
void virtio_add_status(struct virtio_device *dev, unsigned int status);
int register_virtio_device(struct virtio_device *dev);
void unregister_virtio_device(struct virtio_device *dev);
+bool is_virtio_device(struct device *dev);

void virtio_break_device(struct virtio_device *dev);

diff --git a/include/linux/virtio_dma_buf.h b/include/linux/virtio_dma_buf.h
new file mode 100644
index 000000000000..d841a9115876
--- /dev/null
+++ b/include/linux/virtio_dma_buf.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * dma-bufs for virtio exported objects
+ *
+ * Copyright (C) 2020 Google, Inc.
+ */
+
+#ifndef _LINUX_VIRTIO_DMA_BUF_H
+#define _LINUX_VIRTIO_DMA_BUF_H
+
+#include <linux/dma-buf.h>
+#include <linux/uuid.h>
+#include <linux/virtio.h>
+
+/**
+ * struct virtio_dma_buf_ops - operations possible on exported object dma-buf
+ * @ops: the base dma_buf_ops. ops.attach MUST be virtio_dma_buf_attach.
+ * @virtio_attach: [optional] callback invoked by virtio_dma_buf_attach if
+ * the attaching device is a virtio device.
+ * @device_attach: [optional] callback invoked by virtio_dma_buf_attach during
+ * all attach operations.
+ * @get_uid: [required] callback to get the uuid of the exported object.
+ */
+struct virtio_dma_buf_ops {
+ struct dma_buf_ops ops;
+ int (*virtio_attach)(struct dma_buf *dma_buf,
+ struct dma_buf_attachment *attach);
+ int (*device_attach)(struct dma_buf *dma_buf,
+ struct dma_buf_attachment *attach);
+ int (*get_uuid)(struct dma_buf *dma_buf, uuid_t *uuid);
+};
+
+/**
+ * struct virtio_dma_buf_export_info - see struct dma_buf_export_info
+ */
+struct virtio_dma_buf_export_info {
+ const char *exp_name;
+ struct module *owner;
+ const struct virtio_dma_buf_ops *ops;
+ size_t size;
+ int flags;
+ struct dma_resv *resv;
+ void *priv;
+};
+
+/**
+ * DEFINE_VIRTIO_DMA_BUF_EXPORT_INFO - helper macro for exporters
+ */
+#define DEFINE_VIRTIO_DMA_BUF_EXPORT_INFO(name) \
+ struct virtio_dma_buf_export_info name = { \
+ .exp_name = KBUILD_MODNAME, \
+ .owner = THIS_MODULE }
+
+int virtio_dma_buf_attach(struct dma_buf *dma_buf,
+ struct dma_buf_attachment *attach);
+
+struct dma_buf *virtio_dma_buf_export(
+ const struct virtio_dma_buf_export_info *virtio_exp_info);
+bool is_virtio_dma_buf(struct dma_buf *dma_buf);
+int get_virtio_dma_buf_uuid(struct dma_buf *dma_buf, uuid_t *uuid);
+
+#endif /* _LINUX_VIRTIO_DMA_BUF_H */
--
2.25.0.265.gbab2e86ba0-goog

2020-02-25 06:12:25

by Gerd Hoffmann

[permalink] [raw]
Subject: Re: [PATCH 1/2] virtio: add dma-buf support for exported objects

On Wed, Feb 19, 2020 at 05:06:36PM +0900, David Stevens wrote:
> This change adds a new flavor of dma-bufs that can be used by virtio
> drivers to share exported objects. A virtio dma-buf can be queried by
> virtio drivers to obtain the UUID which identifies the underlying
> exported object.

That duplicates a bunch of code from dma-buf.c in virtio_dma_buf.c.

How about dma_buf_{get,set}_uuid, simliar to dma_buf_set_name?

cheers,
Gerd

2020-02-25 06:16:36

by Gerd Hoffmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] drm/virtio: Support virtgpu exported resources

Hi,

> +struct dma_buf *virtgpu_gem_prime_export(struct drm_gem_object *obj,
> + int flags)
> +{
[ ... ]
> +}
> +
> +struct drm_gem_object *virtgpu_gem_prime_import(struct drm_device *dev,
> + struct dma_buf *buf)
> +{
[ ... ]
> +}

More code duplication.

> diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
> index 0c85914d9369..9c428ef03060 100644
> --- a/include/uapi/linux/virtio_gpu.h
> +++ b/include/uapi/linux/virtio_gpu.h

API change should go to a separate patch.

> +/*
> + * VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID
> + */
> +#define VIRTIO_GPU_F_CROSS_DEVICE 2

Hmm, how about VIRTIO_GPU_F_RESOURCE_UUID ?

> @@ -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_ASSIGN_UUID,

The "assign" doesn't make sense in the reply. I'd name that
VIRTIO_GPU_RESP_OK_RESOURCE_UUID or just VIRTIO_GPU_RESP_OK_UUID,

> +/* VIRTIO_GPU_RESP_OK_RESOURCE_ASSIGN_UUID */
> +struct virtio_gpu_resp_resource_assign_uuid {
> + struct virtio_gpu_ctrl_hdr hdr;
> + __u8 uuid[16];
> +};

Same here.

cheers,
Gerd

2020-02-26 03:58:17

by David Stevens

[permalink] [raw]
Subject: Re: [virtio-dev] Re: [PATCH 1/2] virtio: add dma-buf support for exported objects

On Tue, Feb 25, 2020 at 3:10 PM Gerd Hoffmann <[email protected]> wrote:
>
> On Wed, Feb 19, 2020 at 05:06:36PM +0900, David Stevens wrote:
> > This change adds a new flavor of dma-bufs that can be used by virtio
> > drivers to share exported objects. A virtio dma-buf can be queried by
> > virtio drivers to obtain the UUID which identifies the underlying
> > exported object.
>
> That duplicates a bunch of code from dma-buf.c in virtio_dma_buf.c.
>
> How about dma_buf_{get,set}_uuid, simliar to dma_buf_set_name?

While I'm not opposed to such an API, I'm also hesitant to make
changes to the dma-buf API for a single use case.

As for the duplicated code around virtio_dma_buf_export_info, it can
be removed by sacrificing a little bit of type safety, if that's
preferable.

-David

2020-02-26 16:17:24

by Gerd Hoffmann

[permalink] [raw]
Subject: Re: [virtio-dev] Re: [PATCH 1/2] virtio: add dma-buf support for exported objects

On Wed, Feb 26, 2020 at 12:56:58PM +0900, David Stevens wrote:
> On Tue, Feb 25, 2020 at 3:10 PM Gerd Hoffmann <[email protected]> wrote:
> >
> > How about dma_buf_{get,set}_uuid, simliar to dma_buf_set_name?
>
> While I'm not opposed to such an API, I'm also hesitant to make
> changes to the dma-buf API for a single use case.

See virtio-wayland discussion. I expect we will see more cases show up.
Maybe this should even go one level up, to struct file.

cheers,
Gerd