* [PATCH v3 0/4] Virtio shared dma-buf
@ 2023-05-24 9:13 Albert Esteve
2023-05-24 9:13 ` [PATCH v3 1/4] uuid: add hash_func and equal_func Albert Esteve
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: Albert Esteve @ 2023-05-24 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, Albert Esteve, marcandre.lureau, Michael S. Tsirkin,
cohuck, Fam Zheng
v1 link -> https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg00598.html
v2 link -> https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04530.html
v2 -> v3:
- Change UUID hash function strategy to djb
- Add qemu_uuid_is_equal wrapper
This patch covers the required steps to add support for virtio cross-device resource sharing[1],
which support is already available in the kernel.
The main usecase will be sharing dma buffers from virtio-gpu devices (as the exporter
-see VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID in [2]), to virtio-video (under discussion)
devices (as the buffer-user or importer). Therefore, even though virtio specs talk about
resources or objects[3], this patch adds the infrastructure with dma-bufs in mind.
Note that virtio specs let the devices themselves define what a vitio object is.
These are the main parts that are covered in the patch:
- Add hash_func and key_equal_func to uuid
- Shared resources table, to hold all resources that can be shared in the host and their assigned UUID
- Internal shared table API for virtio devices to add, lookup and remove resources
- Unit test to verify the API.
- New message to the vhost-user protocol to allow backend to interact with the shared
table API through the control socket
Applies cleanly to 1c12355
[1] - https://lwn.net/Articles/828988/
[2] - https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-3730006
[3] - https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-10500011
Albert Esteve (4):
uuid: add hash_func and equal_func
virtio-dmabuf: introduce virtio-dmabuf
vhost-user: add shared_object msg
vhost-user: refactor send_resp code
MAINTAINERS | 7 ++
docs/interop/vhost-user.rst | 15 +++
hw/display/meson.build | 1 +
hw/display/virtio-dmabuf.c | 90 +++++++++++++++++
hw/virtio/vhost-user.c | 90 ++++++++++++++---
include/hw/virtio/virtio-dmabuf.h | 59 ++++++++++++
include/qemu/uuid.h | 2 +
subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++
subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++
tests/unit/meson.build | 1 +
tests/unit/test-uuid.c | 27 ++++++
tests/unit/test-virtio-dmabuf.c | 112 ++++++++++++++++++++++
util/uuid.c | 14 +++
13 files changed, 549 insertions(+), 13 deletions(-)
create mode 100644 hw/display/virtio-dmabuf.c
create mode 100644 include/hw/virtio/virtio-dmabuf.h
create mode 100644 tests/unit/test-virtio-dmabuf.c
--
2.40.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 1/4] uuid: add hash_func and equal_func
2023-05-24 9:13 [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
@ 2023-05-24 9:13 ` Albert Esteve
2023-05-24 9:13 ` [PATCH v3 2/4] virtio-dmabuf: introduce virtio-dmabuf Albert Esteve
` (4 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Albert Esteve @ 2023-05-24 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, Albert Esteve, marcandre.lureau, Michael S. Tsirkin,
cohuck, Fam Zheng
Add hash and an equal function to uuid module.
Add a couple simple unit tests for new functions,
checking collisions for similar UUIDs in the case
of the hash function, and comparing generated UUIDs
for the equal function.
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
include/qemu/uuid.h | 2 ++
tests/unit/test-uuid.c | 27 +++++++++++++++++++++++++++
util/uuid.c | 14 ++++++++++++++
3 files changed, 43 insertions(+)
diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
index dc40ee1fc9..e24a1099e4 100644
--- a/include/qemu/uuid.h
+++ b/include/qemu/uuid.h
@@ -96,4 +96,6 @@ int qemu_uuid_parse(const char *str, QemuUUID *uuid);
QemuUUID qemu_uuid_bswap(QemuUUID uuid);
+uint32_t qemu_uuid_hash(const void *uuid);
+
#endif
diff --git a/tests/unit/test-uuid.c b/tests/unit/test-uuid.c
index c111de5fc1..aedc125ae9 100644
--- a/tests/unit/test-uuid.c
+++ b/tests/unit/test-uuid.c
@@ -171,6 +171,32 @@ static void test_uuid_unparse_strdup(void)
}
}
+static void test_uuid_hash(void)
+{
+ QemuUUID uuid;
+ int i;
+
+ for (i = 0; i < 100; i++) {
+ qemu_uuid_generate(&uuid);
+ /* Obtain the UUID hash */
+ uint32_t hash_a = qemu_uuid_hash(&uuid);
+ int data_idx = g_random_int_range(0, 15);
+ /* Change a single random byte of the UUID */
+ if (uuid.data[data_idx] < 0xFF) {
+ uuid.data[data_idx]++;
+ } else {
+ uuid.data[data_idx]--;
+ }
+ /* Obtain the UUID hash again */
+ uint32_t hash_b = qemu_uuid_hash(&uuid);
+ /*
+ * Both hashes shall be different (avoid collision)
+ * for any change in the UUID fields
+ */
+ g_assert_cmpint(hash_a, !=, hash_b);
+ }
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -179,6 +205,7 @@ int main(int argc, char **argv)
g_test_add_func("/uuid/parse", test_uuid_parse);
g_test_add_func("/uuid/unparse", test_uuid_unparse);
g_test_add_func("/uuid/unparse_strdup", test_uuid_unparse_strdup);
+ g_test_add_func("/uuid/hash", test_uuid_hash);
return g_test_run();
}
diff --git a/util/uuid.c b/util/uuid.c
index b1108dde78..64eaf2e208 100644
--- a/util/uuid.c
+++ b/util/uuid.c
@@ -116,3 +116,17 @@ QemuUUID qemu_uuid_bswap(QemuUUID uuid)
bswap16s(&uuid.fields.time_high_and_version);
return uuid;
}
+
+uint32_t qemu_uuid_hash(const void *uuid)
+{
+ QemuUUID *qid = (QemuUUID *) uuid;
+ uint32_t h = 5381;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(qid->data); i++) {
+ h = (h << 5) + h + qid->data[i];
+ }
+
+ return h;
+}
+
--
2.40.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 2/4] virtio-dmabuf: introduce virtio-dmabuf
2023-05-24 9:13 [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
2023-05-24 9:13 ` [PATCH v3 1/4] uuid: add hash_func and equal_func Albert Esteve
@ 2023-05-24 9:13 ` Albert Esteve
2023-05-24 9:13 ` [PATCH v3 3/4] vhost-user: add shared_object msg Albert Esteve
` (3 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Albert Esteve @ 2023-05-24 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, Albert Esteve, marcandre.lureau, Michael S. Tsirkin,
cohuck, Fam Zheng
This API manages objects (in this iteration,
dmabuf fds) that can be shared along different
virtio devices.
The API allows the different devices to add,
remove and/or retrieve the objects by simply
invoking the public functions that reside in the
virtio-dmabuf file.
Suggested-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
MAINTAINERS | 7 ++
hw/display/meson.build | 1 +
hw/display/virtio-dmabuf.c | 90 ++++++++++++++++++++++++
include/hw/virtio/virtio-dmabuf.h | 59 ++++++++++++++++
tests/unit/meson.build | 1 +
tests/unit/test-virtio-dmabuf.c | 112 ++++++++++++++++++++++++++++++
6 files changed, 270 insertions(+)
create mode 100644 hw/display/virtio-dmabuf.c
create mode 100644 include/hw/virtio/virtio-dmabuf.h
create mode 100644 tests/unit/test-virtio-dmabuf.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 1c93ab0ee5..3a497e9cd6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2145,6 +2145,13 @@ T: git https://gitlab.com/cohuck/qemu.git s390-next
T: git https://github.com/borntraeger/qemu.git s390-next
L: qemu-s390x@nongnu.org
+virtio-dmabuf
+M: Albert Esteve <aesteve@redhat.com>
+S: Supported
+F: hw/display/virtio-dmabuf.c
+F: include/hw/virtio/virtio-dmabuf.h
+F: tests/unit/test-virtio-dmabuf.c
+
virtiofs
M: Stefan Hajnoczi <stefanha@redhat.com>
S: Supported
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 17165bd536..62a27395c0 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -37,6 +37,7 @@ softmmu_ss.add(when: 'CONFIG_MACFB', if_true: files('macfb.c'))
softmmu_ss.add(when: 'CONFIG_NEXTCUBE', if_true: files('next-fb.c'))
softmmu_ss.add(when: 'CONFIG_VGA', if_true: files('vga.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO', if_true: files('virtio-dmabuf.c'))
if (config_all_devices.has_key('CONFIG_VGA_CIRRUS') or
config_all_devices.has_key('CONFIG_VGA_PCI') or
diff --git a/hw/display/virtio-dmabuf.c b/hw/display/virtio-dmabuf.c
new file mode 100644
index 0000000000..7dba0b2c71
--- /dev/null
+++ b/hw/display/virtio-dmabuf.c
@@ -0,0 +1,90 @@
+/*
+ * Virtio Shared dma-buf
+ *
+ * Copyright Red Hat, Inc. 2023
+ *
+ * Authors:
+ * Albert Esteve <aesteve@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "hw/virtio/virtio-dmabuf.h"
+
+
+static GMutex lock;
+static GHashTable *resource_uuids;
+
+/*
+ * uuid_equal_func: wrapper for UUID is_equal function to
+ * satisfy g_hash_table_new expected parameters signatures.
+ */
+static int uuid_equal_func(const void *lhv, const void *rhv)
+{
+ return qemu_uuid_is_equal(lhv, rhv);
+}
+
+static bool virtio_add_resource(QemuUUID *uuid, gpointer value)
+{
+ assert(resource_uuids != NULL);
+ if (g_hash_table_lookup(resource_uuids, uuid) != NULL) {
+ return false;
+ }
+
+ return g_hash_table_insert(resource_uuids, uuid, value);
+}
+
+static gpointer virtio_lookup_resource(const QemuUUID *uuid)
+{
+ if (resource_uuids == NULL) {
+ return NULL;
+ }
+
+ return g_hash_table_lookup(resource_uuids, uuid);
+}
+
+bool virtio_add_dmabuf(QemuUUID *uuid, int udmabuf_fd)
+{
+ bool result;
+ if (udmabuf_fd < 0) {
+ return false;
+ }
+ g_mutex_lock(&lock);
+ if (resource_uuids == NULL) {
+ resource_uuids = g_hash_table_new(qemu_uuid_hash, uuid_equal_func);
+ }
+ result = virtio_add_resource(uuid, GINT_TO_POINTER(udmabuf_fd));
+ g_mutex_unlock(&lock);
+
+ return result;
+}
+
+bool virtio_remove_resource(const QemuUUID *uuid)
+{
+ bool result;
+ g_mutex_lock(&lock);
+ result = g_hash_table_remove(resource_uuids, uuid);
+ g_mutex_unlock(&lock);
+
+ return result;
+}
+
+int virtio_lookup_dmabuf(const QemuUUID *uuid)
+{
+ g_mutex_lock(&lock);
+ gpointer lookup_res = virtio_lookup_resource(uuid);
+ g_mutex_unlock(&lock);
+ if (lookup_res == NULL) {
+ return -1;
+ }
+
+ return GPOINTER_TO_INT(lookup_res);
+}
+
+void virtio_free_resources(void)
+{
+ g_hash_table_destroy(resource_uuids);
+ /* Reference count shall be 0 after the implicit unref on destroy */
+ resource_uuids = NULL;
+}
diff --git a/include/hw/virtio/virtio-dmabuf.h b/include/hw/virtio/virtio-dmabuf.h
new file mode 100644
index 0000000000..4fdd394c4b
--- /dev/null
+++ b/include/hw/virtio/virtio-dmabuf.h
@@ -0,0 +1,59 @@
+/*
+ * Virtio Shared dma-buf
+ *
+ * Copyright Red Hat, Inc. 2023
+ *
+ * Authors:
+ * Albert Esteve <aesteve@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef VIRTIO_DMABUF_H
+#define VIRTIO_DMABUF_H
+
+#include "qemu/osdep.h"
+
+#include <glib.h>
+#include "qemu/uuid.h"
+
+/**
+ * virtio_add_dmabuf() - Add a new dma-buf resource to the lookup table
+ * @uuid: new resource's UUID
+ * @dmabuf_fd: the dma-buf descriptor that will be stored and shared with
+ * other virtio devices. The caller retains ownership over the
+ * descriptor and its lifecycle.
+ *
+ * Note: @dmabuf_fd must be a valid (non-negative) file descriptor.
+ *
+ * Return: true if the UUID did not exist and the resource has been added,
+ * false if another resource with the same UUID already existed.
+ * Note that if it finds a repeated UUID, the resource is not inserted in
+ * the lookup table.
+ */
+bool virtio_add_dmabuf(QemuUUID *uuid, int dmabuf_fd);
+
+/**
+ * virtio_remove_resource() - Removes a resource from the lookup table
+ * @uuid: resource's UUID
+ *
+ * Return: true if the UUID has been found and removed from the lookup table.
+ */
+bool virtio_remove_resource(const QemuUUID *uuid);
+
+/**
+ * virtio_lookup_dmabuf() - Looks for a dma-buf resource in the lookup table
+ * @uuid: resource's UUID
+ *
+ * Return: the dma-buf file descriptor integer, or -1 if the key is not found.
+ */
+int virtio_lookup_dmabuf(const QemuUUID *uuid);
+
+/**
+ * virtio_free_resources() - Destroys all keys and values of the shared
+ * resources lookup table, and frees them
+ */
+void virtio_free_resources(void);
+
+#endif /* VIRTIO_DMABUF_H */
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 3a6314269b..22b8140c04 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -50,6 +50,7 @@ tests = {
'test-qapi-util': [],
'test-interval-tree': [],
'test-xs-node': [qom],
+ 'test-virtio-dmabuf': [meson.project_source_root() / 'hw/display/virtio-dmabuf.c'],
}
if have_system or have_tools
diff --git a/tests/unit/test-virtio-dmabuf.c b/tests/unit/test-virtio-dmabuf.c
new file mode 100644
index 0000000000..53436aa93d
--- /dev/null
+++ b/tests/unit/test-virtio-dmabuf.c
@@ -0,0 +1,112 @@
+/*
+ * QEMU tests for shared dma-buf API
+ *
+ * Copyright (c) 2023 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/virtio/virtio-dmabuf.h"
+
+
+static void test_add_remove_resources(void)
+{
+ QemuUUID uuid;
+ int i, dmabuf_fd;
+
+ for (i = 0; i < 100; ++i) {
+ qemu_uuid_generate(&uuid);
+ dmabuf_fd = g_random_int_range(3, 500);
+ /* Add a new resource */
+ g_assert(virtio_add_dmabuf(&uuid, dmabuf_fd));
+ g_assert_cmpint(virtio_lookup_dmabuf(&uuid), ==, dmabuf_fd);
+ /* Remove the resource */
+ g_assert(virtio_remove_resource(&uuid));
+ /* Resource is not found anymore */
+ g_assert_cmpint(virtio_lookup_dmabuf(&uuid), ==, -1);
+ }
+}
+
+static void test_remove_invalid_resource(void)
+{
+ QemuUUID uuid;
+ int i;
+
+ for (i = 0; i < 20; ++i) {
+ qemu_uuid_generate(&uuid);
+ g_assert_cmpint(virtio_lookup_dmabuf(&uuid), ==, -1);
+ /* Removing a resource that does not exist returns false */
+ g_assert_false(virtio_remove_resource(&uuid));
+ }
+}
+
+static void test_add_invalid_resource(void)
+{
+ QemuUUID uuid;
+ int i, dmabuf_fd = -2, alt_dmabuf = 2;
+
+ for (i = 0; i < 20; ++i) {
+ qemu_uuid_generate(&uuid);
+ /* Add a new resource with invalid (negative) resource fd */
+ g_assert_false(virtio_add_dmabuf(&uuid, dmabuf_fd));
+ /* Resource is not found */
+ g_assert_cmpint(virtio_lookup_dmabuf(&uuid), ==, -1);
+ }
+
+ for (i = 0; i < 20; ++i) {
+ /* Add a valid resource */
+ qemu_uuid_generate(&uuid);
+ dmabuf_fd = g_random_int_range(3, 500);
+ g_assert(virtio_add_dmabuf(&uuid, dmabuf_fd));
+ g_assert_cmpint(virtio_lookup_dmabuf(&uuid), ==, dmabuf_fd);
+ /* Add a new resource with repeated uuid returns false */
+ g_assert_false(virtio_add_dmabuf(&uuid, alt_dmabuf));
+ /* The value for the uuid key is not replaced */
+ g_assert_cmpint(virtio_lookup_dmabuf(&uuid), ==, dmabuf_fd);
+ }
+}
+
+static void test_free_resources(void)
+{
+ QemuUUID uuids[20];
+ int i, dmabuf_fd;
+
+ for (i = 0; i < ARRAY_SIZE(uuids); ++i) {
+ qemu_uuid_generate(&uuids[i]);
+ dmabuf_fd = g_random_int_range(3, 500);
+ g_assert(virtio_add_dmabuf(&uuids[i], dmabuf_fd));
+ g_assert_cmpint(virtio_lookup_dmabuf(&uuids[i]), ==, dmabuf_fd);
+ }
+ virtio_free_resources();
+ for (i = 0; i < ARRAY_SIZE(uuids); ++i) {
+ /* None of the resources is found after free'd */
+ g_assert_cmpint(virtio_lookup_dmabuf(&uuids[i]), ==, -1);
+ }
+
+}
+
+int main(int argc, char **argv)
+{
+ g_test_init(&argc, &argv, NULL);
+ g_test_add_func("/virtio-dmabuf/add_rm_res", test_add_remove_resources);
+ g_test_add_func("/virtio-dmabuf/rm_invalid_res",
+ test_remove_invalid_resource);
+ g_test_add_func("/virtio-dmabuf/add_invalid_res",
+ test_add_invalid_resource);
+ g_test_add_func("/virtio-dmabuf/free_res", test_free_resources);
+
+ return g_test_run();
+}
--
2.40.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 3/4] vhost-user: add shared_object msg
2023-05-24 9:13 [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
2023-05-24 9:13 ` [PATCH v3 1/4] uuid: add hash_func and equal_func Albert Esteve
2023-05-24 9:13 ` [PATCH v3 2/4] virtio-dmabuf: introduce virtio-dmabuf Albert Esteve
@ 2023-05-24 9:13 ` Albert Esteve
2023-06-22 19:57 ` Michael S. Tsirkin
` (3 more replies)
2023-05-24 9:13 ` [PATCH v3 4/4] vhost-user: refactor send_resp code Albert Esteve
` (2 subsequent siblings)
5 siblings, 4 replies; 19+ messages in thread
From: Albert Esteve @ 2023-05-24 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, Albert Esteve, marcandre.lureau, Michael S. Tsirkin,
cohuck, Fam Zheng
Add new vhost-user protocol message
`VHOST_USER_BACKEND_SHARED_OBJECT`. This new
message is sent from vhost-user back-ends
to interact with the virtio-dmabuf table
in order to add, remove, or lookup for
virtio dma-buf shared objects.
The action taken in the front-end depends
on the type stored in the payload struct.
In the libvhost-user library add helper
functions to allow sending messages to
interact with the virtio shared objects
hash table.
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
docs/interop/vhost-user.rst | 15 ++++
hw/virtio/vhost-user.c | 68 ++++++++++++++++++
subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++++++++
subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++++++
4 files changed, 227 insertions(+)
diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 5a070adbc1..d3d8db41e5 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -1528,6 +1528,21 @@ is sent by the front-end.
The state.num field is currently reserved and must be set to 0.
+``VHOST_USER_BACKEND_SHARED_OBJECT``
+ :id: 6
+ :equivalent ioctl: N/A
+ :request payload: ``struct VhostUserShared``
+ :reply payload: ``struct VhostUserShared`` (only for ``LOOKUP`` requests)
+
+ Backends that need to interact with the virtio-dmabuf shared table API
+ can send this message. The operation is determined by the ``type`` member
+ of the payload struct. The valid values for the operation type are
+ ``VHOST_SHARED_OBJECT_*`` members, i.e., ``ADD``, ``LOOKUP``, and ``REMOVE``.
+ ``LOOKUP`` operations require the ``VHOST_USER_NEED_REPLY_MASK`` flag to be
+ set by the back-end, and the front-end will then send the dma-buf fd as
+ a response if the UUID matches an object in the table, or a negative value
+ otherwise.
+
.. _reply_ack:
VHOST_USER_PROTOCOL_F_REPLY_ACK
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 74a2a28663..5ac5f0eafd 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -10,6 +10,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
+#include "hw/virtio/virtio-dmabuf.h"
#include "hw/virtio/vhost.h"
#include "hw/virtio/vhost-user.h"
#include "hw/virtio/vhost-backend.h"
@@ -20,6 +21,7 @@
#include "sysemu/kvm.h"
#include "qemu/error-report.h"
#include "qemu/main-loop.h"
+#include "qemu/uuid.h"
#include "qemu/sockets.h"
#include "sysemu/runstate.h"
#include "sysemu/cryptodev.h"
@@ -128,6 +130,7 @@ typedef enum VhostUserSlaveRequest {
VHOST_USER_BACKEND_IOTLB_MSG = 1,
VHOST_USER_BACKEND_CONFIG_CHANGE_MSG = 2,
VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
+ VHOST_USER_BACKEND_SHARED_OBJECT = 6,
VHOST_USER_BACKEND_MAX
} VhostUserSlaveRequest;
@@ -190,6 +193,18 @@ typedef struct VhostUserInflight {
uint16_t queue_size;
} VhostUserInflight;
+typedef enum VhostUserSharedType {
+ VHOST_SHARED_OBJECT_ADD = 0,
+ VHOST_SHARED_OBJECT_LOOKUP,
+ VHOST_SHARED_OBJECT_REMOVE,
+} VhostUserSharedType;
+
+typedef struct VhostUserShared {
+ unsigned char uuid[16];
+ VhostUserSharedType type;
+ int dmabuf_fd;
+} VhostUserShared;
+
typedef struct {
VhostUserRequest request;
@@ -214,6 +229,7 @@ typedef union {
VhostUserCryptoSession session;
VhostUserVringArea area;
VhostUserInflight inflight;
+ VhostUserShared object;
} VhostUserPayload;
typedef struct VhostUserMsg {
@@ -1582,6 +1598,52 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
return 0;
}
+static int vhost_user_backend_handle_shared_object(VhostUserShared *object)
+{
+ QemuUUID uuid;
+ memcpy(uuid.data, object->uuid, sizeof(object->uuid));
+
+ switch (object->type) {
+ case VHOST_SHARED_OBJECT_ADD:
+ return virtio_add_dmabuf(&uuid, object->dmabuf_fd);
+ case VHOST_SHARED_OBJECT_LOOKUP:
+ object->dmabuf_fd = virtio_lookup_dmabuf(&uuid);
+ if (object->dmabuf_fd < 0) {
+ return object->dmabuf_fd;
+ }
+ break;
+ case VHOST_SHARED_OBJECT_REMOVE:
+ return virtio_remove_resource(&uuid);
+ }
+
+ return 0;
+}
+
+static bool
+vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
+ VhostUserPayload *payload)
+{
+ Error *local_err = NULL;
+ struct iovec iov[2];
+ if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
+ hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
+ hdr->flags |= VHOST_USER_REPLY_MASK;
+
+ hdr->size = sizeof(payload->object);
+
+ iov[0].iov_base = hdr;
+ iov[0].iov_len = VHOST_USER_HDR_SIZE;
+ iov[1].iov_base = payload;
+ iov[1].iov_len = hdr->size;
+
+ if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
+ error_report_err(local_err);
+ return false;
+ }
+ }
+ return true;
+}
+
static void close_slave_channel(struct vhost_user *u)
{
g_source_destroy(u->slave_src);
@@ -1639,6 +1701,12 @@ static gboolean slave_read(QIOChannel *ioc, GIOCondition condition,
ret = vhost_user_slave_handle_vring_host_notifier(dev, &payload.area,
fd ? fd[0] : -1);
break;
+ case VHOST_USER_BACKEND_SHARED_OBJECT:
+ ret = vhost_user_backend_handle_shared_object(&payload.object);
+ if (!vhost_user_backend_send_dmabuf_fd(ioc, &hdr, &payload)) {
+ goto err;
+ }
+ break;
default:
error_report("Received unexpected msg type: %d.", hdr.request);
ret = -EINVAL;
diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 8fb61e2df2..27f16d292a 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -1403,6 +1403,94 @@ bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
return vu_process_message_reply(dev, &vmsg);
}
+bool
+vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int *dmabuf_fd)
+{
+ bool result = false;
+ VhostUserMsg msg_reply;
+ VhostUserMsg msg = {
+ .request = VHOST_USER_BACKEND_SHARED_OBJECT,
+ .size = sizeof(msg.payload.object),
+ .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
+ .payload.object = {
+ .type = VHOST_SHARED_OBJECT_LOOKUP,
+ },
+ };
+
+ memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
+
+ pthread_mutex_lock(&dev->slave_mutex);
+ if (!vu_message_write(dev, dev->slave_fd, &msg)) {
+ goto out;
+ }
+
+ if (!vu_message_read_default(dev, dev->slave_fd, &msg_reply)) {
+ goto out;
+ }
+
+ if (msg_reply.request != msg.request) {
+ DPRINT("Received unexpected msg type. Expected %d, received %d",
+ msg.request, msg_reply.request);
+ goto out;
+ }
+
+ *dmabuf_fd = msg_reply.payload.object.dmabuf_fd;
+ result = *dmabuf_fd > 0;
+out:
+ pthread_mutex_unlock(&dev->slave_mutex);
+
+ return result;
+}
+
+static bool
+vu_send_message(VuDev *dev, VhostUserMsg *vmsg)
+{
+ bool result = false;
+ pthread_mutex_lock(&dev->slave_mutex);
+ if (!vu_message_write(dev, dev->slave_fd, vmsg)) {
+ goto out;
+ }
+
+ result = true;
+out:
+ pthread_mutex_unlock(&dev->slave_mutex);
+
+ return result;
+}
+
+bool
+vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int dmabuf_fd)
+{
+ VhostUserMsg msg = {
+ .request = VHOST_USER_BACKEND_SHARED_OBJECT,
+ .size = sizeof(msg.payload.object),
+ .flags = VHOST_USER_VERSION,
+ .payload.object = {
+ .dmabuf_fd = dmabuf_fd,
+ .type = VHOST_SHARED_OBJECT_ADD,
+ },
+ };
+ memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
+
+ return vu_send_message(dev, &msg);
+}
+
+bool
+vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
+{
+ VhostUserMsg msg = {
+ .request = VHOST_USER_BACKEND_SHARED_OBJECT,
+ .size = sizeof(msg.payload.object),
+ .flags = VHOST_USER_VERSION,
+ .payload.object = {
+ .type = VHOST_SHARED_OBJECT_REMOVE,
+ },
+ };
+ memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
+
+ return vu_send_message(dev, &msg);
+}
+
static bool
vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
{
diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
index 49208cceaa..a43d115bd7 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -119,6 +119,7 @@ typedef enum VhostUserSlaveRequest {
VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
VHOST_USER_BACKEND_VRING_CALL = 4,
VHOST_USER_BACKEND_VRING_ERR = 5,
+ VHOST_USER_BACKEND_SHARED_OBJECT = 6,
VHOST_USER_BACKEND_MAX
} VhostUserSlaveRequest;
@@ -172,6 +173,20 @@ typedef struct VhostUserInflight {
uint16_t queue_size;
} VhostUserInflight;
+typedef enum VhostUserSharedType {
+ VHOST_SHARED_OBJECT_ADD = 0,
+ VHOST_SHARED_OBJECT_LOOKUP,
+ VHOST_SHARED_OBJECT_REMOVE,
+} VhostUserSharedType;
+
+#define UUID_LEN 16
+
+typedef struct VhostUserShared {
+ unsigned char uuid[UUID_LEN];
+ VhostUserSharedType type;
+ int dmabuf_fd;
+} VhostUserShared;
+
#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
# define VU_PACKED __attribute__((gcc_struct, packed))
#else
@@ -199,6 +214,7 @@ typedef struct VhostUserMsg {
VhostUserConfig config;
VhostUserVringArea area;
VhostUserInflight inflight;
+ VhostUserShared object;
} payload;
int fds[VHOST_MEMORY_BASELINE_NREGIONS];
@@ -539,6 +555,46 @@ void vu_set_queue_handler(VuDev *dev, VuVirtq *vq,
bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
int size, int offset);
+/**
+ * vu_get_shared_object:
+ * @dev: a VuDev context
+ * @uuid: UUID of the shared object
+ * @dmabuf_fd: output dma-buf file descriptor
+ *
+ * Lookup for a virtio shared object (i.e., dma-buf fd) associated with the
+ * received UUID. Result, if found, is stored in the dmabuf_fd argument.
+ *
+ * Returns: whether the virtio object was found.
+ */
+bool vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
+ int *dmabuf_fd);
+
+/**
+ * vu_add_shared_object:
+ * @dev: a VuDev context
+ * @uuid: UUID of the shared object
+ * @dmabuf_fd: output dma-buf file descriptor
+ *
+ * Stores a new shared object (i.e., dma-buf fd) in the hash table, and
+ * associates it with the received UUID.
+ *
+ * Returns: TRUE on success, FALSE on failure.
+ */
+bool vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
+ int dmabuf_fd);
+
+/**
+ * vu_rm_shared_object:
+ * @dev: a VuDev context
+ * @uuid: UUID of the shared object
+ *
+ * Removes a shared object (i.e., dma-buf fd) associated with the
+ * received UUID from the hash table.
+ *
+ * Returns: TRUE on success, FALSE on failure.
+ */
+bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
+
/**
* vu_queue_set_notification:
* @dev: a VuDev context
--
2.40.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 4/4] vhost-user: refactor send_resp code
2023-05-24 9:13 [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
` (2 preceding siblings ...)
2023-05-24 9:13 ` [PATCH v3 3/4] vhost-user: add shared_object msg Albert Esteve
@ 2023-05-24 9:13 ` Albert Esteve
2023-06-23 6:48 ` Michael S. Tsirkin
2023-06-21 8:20 ` [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
2023-06-23 6:49 ` Michael S. Tsirkin
5 siblings, 1 reply; 19+ messages in thread
From: Albert Esteve @ 2023-05-24 9:13 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, Albert Esteve, marcandre.lureau, Michael S. Tsirkin,
cohuck, Fam Zheng
Refactor code to send response message so that
all common parts both for the common REPLY_ACK
case, and other data responses, can call it and
avoid code repetition.
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
hw/virtio/vhost-user.c | 52 +++++++++++++++++++-----------------------
1 file changed, 24 insertions(+), 28 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 5ac5f0eafd..b888f2c177 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1619,28 +1619,36 @@ static int vhost_user_backend_handle_shared_object(VhostUserShared *object)
return 0;
}
-static bool
-vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
- VhostUserPayload *payload)
+static bool vhost_user_send_resp(QIOChannel *ioc, VhostUserHeader *hdr,
+ VhostUserPayload *payload)
{
Error *local_err = NULL;
struct iovec iov[2];
- if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
- hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
- hdr->flags |= VHOST_USER_REPLY_MASK;
+ hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
+ hdr->flags |= VHOST_USER_REPLY_MASK;
- hdr->size = sizeof(payload->object);
+ iov[0].iov_base = hdr;
+ iov[0].iov_len = VHOST_USER_HDR_SIZE;
+ iov[1].iov_base = payload;
+ iov[1].iov_len = hdr->size;
+
+ if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
+ error_report_err(local_err);
+ return false;
+ }
- iov[0].iov_base = hdr;
- iov[0].iov_len = VHOST_USER_HDR_SIZE;
- iov[1].iov_base = payload;
- iov[1].iov_len = hdr->size;
+ return true;
+}
- if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
- error_report_err(local_err);
- return false;
- }
+static bool
+vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
+ VhostUserPayload *payload)
+{
+ if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
+ hdr->size = sizeof(payload->object);
+ return vhost_user_send_resp(ioc, hdr, payload);
}
+
return true;
}
@@ -1717,22 +1725,10 @@ static gboolean slave_read(QIOChannel *ioc, GIOCondition condition,
* directly in their request handlers.
*/
if (hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
- struct iovec iovec[2];
-
-
- hdr.flags &= ~VHOST_USER_NEED_REPLY_MASK;
- hdr.flags |= VHOST_USER_REPLY_MASK;
-
payload.u64 = !!ret;
hdr.size = sizeof(payload.u64);
- iovec[0].iov_base = &hdr;
- iovec[0].iov_len = VHOST_USER_HDR_SIZE;
- iovec[1].iov_base = &payload;
- iovec[1].iov_len = hdr.size;
-
- if (qio_channel_writev_all(ioc, iovec, ARRAY_SIZE(iovec), &local_err)) {
- error_report_err(local_err);
+ if (!vhost_user_send_resp(ioc, &hdr, &payload)) {
goto err;
}
}
--
2.40.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 0/4] Virtio shared dma-buf
2023-05-24 9:13 [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
` (3 preceding siblings ...)
2023-05-24 9:13 ` [PATCH v3 4/4] vhost-user: refactor send_resp code Albert Esteve
@ 2023-06-21 8:20 ` Albert Esteve
2023-06-21 20:25 ` Michael S. Tsirkin
2023-06-22 19:53 ` Michael S. Tsirkin
2023-06-23 6:49 ` Michael S. Tsirkin
5 siblings, 2 replies; 19+ messages in thread
From: Albert Esteve @ 2023-06-21 8:20 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, marcandre.lureau, Michael S. Tsirkin, cohuck, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 3177 bytes --]
Hi!
It has been a month since I sent this patch, so I'll give it a bump to get
some attention back.
@mst and @Fam any comments? What would be the next steps to take to move
this forward?
BR,
Albert
On Wed, May 24, 2023 at 11:13 AM Albert Esteve <aesteve@redhat.com> wrote:
> v1 link ->
> https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg00598.html
> v2 link ->
> https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04530.html
> v2 -> v3:
> - Change UUID hash function strategy to djb
> - Add qemu_uuid_is_equal wrapper
>
> This patch covers the required steps to add support for virtio
> cross-device resource sharing[1],
> which support is already available in the kernel.
>
> The main usecase will be sharing dma buffers from virtio-gpu devices (as
> the exporter
> -see VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID in [2]), to virtio-video (under
> discussion)
> devices (as the buffer-user or importer). Therefore, even though virtio
> specs talk about
> resources or objects[3], this patch adds the infrastructure with dma-bufs
> in mind.
> Note that virtio specs let the devices themselves define what a vitio
> object is.
>
> These are the main parts that are covered in the patch:
>
> - Add hash_func and key_equal_func to uuid
> - Shared resources table, to hold all resources that can be shared in the
> host and their assigned UUID
> - Internal shared table API for virtio devices to add, lookup and remove
> resources
> - Unit test to verify the API.
> - New message to the vhost-user protocol to allow backend to interact with
> the shared
> table API through the control socket
>
> Applies cleanly to 1c12355
>
> [1] - https://lwn.net/Articles/828988/
> [2] -
> https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-3730006
> [3] -
> https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-10500011
>
> Albert Esteve (4):
> uuid: add hash_func and equal_func
> virtio-dmabuf: introduce virtio-dmabuf
> vhost-user: add shared_object msg
> vhost-user: refactor send_resp code
>
> MAINTAINERS | 7 ++
> docs/interop/vhost-user.rst | 15 +++
> hw/display/meson.build | 1 +
> hw/display/virtio-dmabuf.c | 90 +++++++++++++++++
> hw/virtio/vhost-user.c | 90 ++++++++++++++---
> include/hw/virtio/virtio-dmabuf.h | 59 ++++++++++++
> include/qemu/uuid.h | 2 +
> subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++
> subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++
> tests/unit/meson.build | 1 +
> tests/unit/test-uuid.c | 27 ++++++
> tests/unit/test-virtio-dmabuf.c | 112 ++++++++++++++++++++++
> util/uuid.c | 14 +++
> 13 files changed, 549 insertions(+), 13 deletions(-)
> create mode 100644 hw/display/virtio-dmabuf.c
> create mode 100644 include/hw/virtio/virtio-dmabuf.h
> create mode 100644 tests/unit/test-virtio-dmabuf.c
>
> --
> 2.40.0
>
>
[-- Attachment #2: Type: text/html, Size: 4459 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 0/4] Virtio shared dma-buf
2023-06-21 8:20 ` [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
@ 2023-06-21 20:25 ` Michael S. Tsirkin
2023-06-22 19:53 ` Michael S. Tsirkin
1 sibling, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2023-06-21 20:25 UTC (permalink / raw)
To: Albert Esteve; +Cc: qemu-devel, kraxel, marcandre.lureau, cohuck, Fam Zheng
On Wed, Jun 21, 2023 at 10:20:25AM +0200, Albert Esteve wrote:
> Hi!
>
> It has been a month since I sent this patch, so I'll give it a bump to get some
> attention back.
>
> @mst and @Fam any comments? What would be the next steps to take to move this
> forward?
>
> BR,
> Albert
>
>
I'd really want help from Gerd here. Don't know enough about dmabuf.
Gerd any comments or I'll just merge?
--
MST
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 0/4] Virtio shared dma-buf
2023-06-21 8:20 ` [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
2023-06-21 20:25 ` Michael S. Tsirkin
@ 2023-06-22 19:53 ` Michael S. Tsirkin
1 sibling, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2023-06-22 19:53 UTC (permalink / raw)
To: Albert Esteve; +Cc: qemu-devel, kraxel, marcandre.lureau, cohuck, Fam Zheng
On Wed, Jun 21, 2023 at 10:20:25AM +0200, Albert Esteve wrote:
> Hi!
>
> It has been a month since I sent this patch, so I'll give it a bump to get some
> attention back.
>
> @mst and @Fam any comments? What would be the next steps to take to move this
> forward?
>
> BR,
> Albert
No one seems to be worried by this patchset so I queued it.
--
MST
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/4] vhost-user: add shared_object msg
2023-05-24 9:13 ` [PATCH v3 3/4] vhost-user: add shared_object msg Albert Esteve
@ 2023-06-22 19:57 ` Michael S. Tsirkin
2023-06-22 20:18 ` Marc-André Lureau
` (2 subsequent siblings)
3 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2023-06-22 19:57 UTC (permalink / raw)
To: Albert Esteve; +Cc: qemu-devel, kraxel, marcandre.lureau, cohuck, Fam Zheng
On Wed, May 24, 2023 at 11:13:32AM +0200, Albert Esteve wrote:
> Add new vhost-user protocol message
> `VHOST_USER_BACKEND_SHARED_OBJECT`. This new
> message is sent from vhost-user back-ends
> to interact with the virtio-dmabuf table
> in order to add, remove, or lookup for
> virtio dma-buf shared objects.
>
> The action taken in the front-end depends
> on the type stored in the payload struct.
>
> In the libvhost-user library add helper
> functions to allow sending messages to
> interact with the virtio shared objects
> hash table.
>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
> docs/interop/vhost-user.rst | 15 ++++
> hw/virtio/vhost-user.c | 68 ++++++++++++++++++
> subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++++++++
> subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++++++
> 4 files changed, 227 insertions(+)
>
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index 5a070adbc1..d3d8db41e5 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -1528,6 +1528,21 @@ is sent by the front-end.
>
> The state.num field is currently reserved and must be set to 0.
>
> +``VHOST_USER_BACKEND_SHARED_OBJECT``
> + :id: 6
> + :equivalent ioctl: N/A
> + :request payload: ``struct VhostUserShared``
> + :reply payload: ``struct VhostUserShared`` (only for ``LOOKUP`` requests)
> +
> + Backends that need to interact with the virtio-dmabuf shared table API
> + can send this message. The operation is determined by the ``type`` member
> + of the payload struct. The valid values for the operation type are
> + ``VHOST_SHARED_OBJECT_*`` members, i.e., ``ADD``, ``LOOKUP``, and ``REMOVE``.
> + ``LOOKUP`` operations require the ``VHOST_USER_NEED_REPLY_MASK`` flag to be
> + set by the back-end, and the front-end will then send the dma-buf fd as
> + a response if the UUID matches an object in the table, or a negative value
> + otherwise.
> +
> .. _reply_ack:
>
> VHOST_USER_PROTOCOL_F_REPLY_ACK
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 74a2a28663..5ac5f0eafd 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -10,6 +10,7 @@
>
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> +#include "hw/virtio/virtio-dmabuf.h"
> #include "hw/virtio/vhost.h"
> #include "hw/virtio/vhost-user.h"
> #include "hw/virtio/vhost-backend.h"
> @@ -20,6 +21,7 @@
> #include "sysemu/kvm.h"
> #include "qemu/error-report.h"
> #include "qemu/main-loop.h"
> +#include "qemu/uuid.h"
> #include "qemu/sockets.h"
> #include "sysemu/runstate.h"
> #include "sysemu/cryptodev.h"
> @@ -128,6 +130,7 @@ typedef enum VhostUserSlaveRequest {
> VHOST_USER_BACKEND_IOTLB_MSG = 1,
> VHOST_USER_BACKEND_CONFIG_CHANGE_MSG = 2,
> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> VHOST_USER_BACKEND_MAX
> } VhostUserSlaveRequest;
>
> @@ -190,6 +193,18 @@ typedef struct VhostUserInflight {
> uint16_t queue_size;
> } VhostUserInflight;
>
> +typedef enum VhostUserSharedType {
> + VHOST_SHARED_OBJECT_ADD = 0,
> + VHOST_SHARED_OBJECT_LOOKUP,
> + VHOST_SHARED_OBJECT_REMOVE,
> +} VhostUserSharedType;
> +
> +typedef struct VhostUserShared {
> + unsigned char uuid[16];
> + VhostUserSharedType type;
> + int dmabuf_fd;
> +} VhostUserShared;
> +
> typedef struct {
> VhostUserRequest request;
>
> @@ -214,6 +229,7 @@ typedef union {
> VhostUserCryptoSession session;
> VhostUserVringArea area;
> VhostUserInflight inflight;
> + VhostUserShared object;
> } VhostUserPayload;
>
> typedef struct VhostUserMsg {
> @@ -1582,6 +1598,52 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
> return 0;
> }
>
> +static int vhost_user_backend_handle_shared_object(VhostUserShared *object)
> +{
> + QemuUUID uuid;
> + memcpy(uuid.data, object->uuid, sizeof(object->uuid));
> +
> + switch (object->type) {
> + case VHOST_SHARED_OBJECT_ADD:
> + return virtio_add_dmabuf(&uuid, object->dmabuf_fd);
> + case VHOST_SHARED_OBJECT_LOOKUP:
> + object->dmabuf_fd = virtio_lookup_dmabuf(&uuid);
> + if (object->dmabuf_fd < 0) {
> + return object->dmabuf_fd;
> + }
> + break;
> + case VHOST_SHARED_OBJECT_REMOVE:
> + return virtio_remove_resource(&uuid);
> + }
> +
> + return 0;
> +}
> +
> +static bool
> +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> + VhostUserPayload *payload)
> +{
> + Error *local_err = NULL;
> + struct iovec iov[2];
> + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> + hdr->flags |= VHOST_USER_REPLY_MASK;
> +
> + hdr->size = sizeof(payload->object);
> +
> + iov[0].iov_base = hdr;
> + iov[0].iov_len = VHOST_USER_HDR_SIZE;
> + iov[1].iov_base = payload;
> + iov[1].iov_len = hdr->size;
> +
> + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
> + error_report_err(local_err);
> + return false;
> + }
> + }
> + return true;
> +}
> +
> static void close_slave_channel(struct vhost_user *u)
> {
> g_source_destroy(u->slave_src);
> @@ -1639,6 +1701,12 @@ static gboolean slave_read(QIOChannel *ioc, GIOCondition condition,
> ret = vhost_user_slave_handle_vring_host_notifier(dev, &payload.area,
> fd ? fd[0] : -1);
> break;
> + case VHOST_USER_BACKEND_SHARED_OBJECT:
> + ret = vhost_user_backend_handle_shared_object(&payload.object);
> + if (!vhost_user_backend_send_dmabuf_fd(ioc, &hdr, &payload)) {
> + goto err;
> + }
> + break;
> default:
> error_report("Received unexpected msg type: %d.", hdr.request);
> ret = -EINVAL;
> diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
> index 8fb61e2df2..27f16d292a 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -1403,6 +1403,94 @@ bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
> return vu_process_message_reply(dev, &vmsg);
> }
>
> +bool
> +vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int *dmabuf_fd)
> +{
> + bool result = false;
> + VhostUserMsg msg_reply;
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
> + .payload.object = {
> + .type = VHOST_SHARED_OBJECT_LOOKUP,
> + },
> + };
> +
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + pthread_mutex_lock(&dev->slave_mutex);
> + if (!vu_message_write(dev, dev->slave_fd, &msg)) {
> + goto out;
> + }
> +
> + if (!vu_message_read_default(dev, dev->slave_fd, &msg_reply)) {
> + goto out;
> + }
> +
> + if (msg_reply.request != msg.request) {
> + DPRINT("Received unexpected msg type. Expected %d, received %d",
> + msg.request, msg_reply.request);
> + goto out;
> + }
> +
> + *dmabuf_fd = msg_reply.payload.object.dmabuf_fd;
> + result = *dmabuf_fd > 0;
> +out:
> + pthread_mutex_unlock(&dev->slave_mutex);
> +
> + return result;
> +}
> +
> +static bool
> +vu_send_message(VuDev *dev, VhostUserMsg *vmsg)
> +{
> + bool result = false;
> + pthread_mutex_lock(&dev->slave_mutex);
> + if (!vu_message_write(dev, dev->slave_fd, vmsg)) {
> + goto out;
> + }
> +
> + result = true;
> +out:
> + pthread_mutex_unlock(&dev->slave_mutex);
> +
> + return result;
> +}
> +
> +bool
> +vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int dmabuf_fd)
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION,
> + .payload.object = {
> + .dmabuf_fd = dmabuf_fd,
> + .type = VHOST_SHARED_OBJECT_ADD,
> + },
> + };
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + return vu_send_message(dev, &msg);
> +}
> +
> +bool
> +vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION,
> + .payload.object = {
> + .type = VHOST_SHARED_OBJECT_REMOVE,
> + },
> + };
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + return vu_send_message(dev, &msg);
> +}
> +
> static bool
> vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
> {
> diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
> index 49208cceaa..a43d115bd7 100644
> --- a/subprojects/libvhost-user/libvhost-user.h
> +++ b/subprojects/libvhost-user/libvhost-user.h
> @@ -119,6 +119,7 @@ typedef enum VhostUserSlaveRequest {
> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> VHOST_USER_BACKEND_VRING_CALL = 4,
> VHOST_USER_BACKEND_VRING_ERR = 5,
> + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> VHOST_USER_BACKEND_MAX
> } VhostUserSlaveRequest;
>
> @@ -172,6 +173,20 @@ typedef struct VhostUserInflight {
> uint16_t queue_size;
> } VhostUserInflight;
>
> +typedef enum VhostUserSharedType {
> + VHOST_SHARED_OBJECT_ADD = 0,
> + VHOST_SHARED_OBJECT_LOOKUP,
> + VHOST_SHARED_OBJECT_REMOVE,
> +} VhostUserSharedType;
> +
> +#define UUID_LEN 16
> +
> +typedef struct VhostUserShared {
> + unsigned char uuid[UUID_LEN];
> + VhostUserSharedType type;
> + int dmabuf_fd;
> +} VhostUserShared;
> +
> #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> # define VU_PACKED __attribute__((gcc_struct, packed))
> #else
> @@ -199,6 +214,7 @@ typedef struct VhostUserMsg {
> VhostUserConfig config;
> VhostUserVringArea area;
> VhostUserInflight inflight;
> + VhostUserShared object;
> } payload;
>
> int fds[VHOST_MEMORY_BASELINE_NREGIONS];
> @@ -539,6 +555,46 @@ void vu_set_queue_handler(VuDev *dev, VuVirtq *vq,
> bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
> int size, int offset);
>
> +/**
> + * vu_get_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + * @dmabuf_fd: output dma-buf file descriptor
> + *
> + * Lookup for a virtio shared object (i.e., dma-buf fd) associated with the
> + * received UUID. Result, if found, is stored in the dmabuf_fd argument.
> + *
> + * Returns: whether the virtio object was found.
> + */
> +bool vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> + int *dmabuf_fd);
> +
> +/**
> + * vu_add_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + * @dmabuf_fd: output dma-buf file descriptor
> + *
> + * Stores a new shared object (i.e., dma-buf fd) in the hash table, and
trailing whitespace. I fixed it up.
> + * associates it with the received UUID.
> + *
> + * Returns: TRUE on success, FALSE on failure.
> + */
> +bool vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> + int dmabuf_fd);
> +
> +/**
> + * vu_rm_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + *
> + * Removes a shared object (i.e., dma-buf fd) associated with the
> + * received UUID from the hash table.
> + *
> + * Returns: TRUE on success, FALSE on failure.
> + */
> +bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
> +
> /**
> * vu_queue_set_notification:
> * @dev: a VuDev context
> --
> 2.40.0
On Wed, May 24, 2023 at 11:13:33AM +0200, Albert Esteve wrote:
> Refactor code to send response message so that
> all common parts both for the common REPLY_ACK
> case, and other data responses, can call it and
> avoid code repetition.
>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
> hw/virtio/vhost-user.c | 52 +++++++++++++++++++-----------------------
> 1 file changed, 24 insertions(+), 28 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 5ac5f0eafd..b888f2c177 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1619,28 +1619,36 @@ static int vhost_user_backend_handle_shared_object(VhostUserShared *object)
> return 0;
> }
>
> -static bool
> -vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> - VhostUserPayload *payload)
> +static bool vhost_user_send_resp(QIOChannel *ioc, VhostUserHeader *hdr,
> + VhostUserPayload *payload)
> {
> Error *local_err = NULL;
> struct iovec iov[2];
> - if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> - hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> - hdr->flags |= VHOST_USER_REPLY_MASK;
> + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> + hdr->flags |= VHOST_USER_REPLY_MASK;
>
> - hdr->size = sizeof(payload->object);
> + iov[0].iov_base = hdr;
> + iov[0].iov_len = VHOST_USER_HDR_SIZE;
> + iov[1].iov_base = payload;
> + iov[1].iov_len = hdr->size;
> +
> + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
> + error_report_err(local_err);
> + return false;
> + }
>
> - iov[0].iov_base = hdr;
> - iov[0].iov_len = VHOST_USER_HDR_SIZE;
> - iov[1].iov_base = payload;
> - iov[1].iov_len = hdr->size;
> + return true;
> +}
>
> - if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
> - error_report_err(local_err);
> - return false;
> - }
> +static bool
> +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> + VhostUserPayload *payload)
> +{
> + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> + hdr->size = sizeof(payload->object);
> + return vhost_user_send_resp(ioc, hdr, payload);
> }
> +
> return true;
> }
>
> @@ -1717,22 +1725,10 @@ static gboolean slave_read(QIOChannel *ioc, GIOCondition condition,
> * directly in their request handlers.
> */
> if (hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
> - struct iovec iovec[2];
> -
> -
> - hdr.flags &= ~VHOST_USER_NEED_REPLY_MASK;
> - hdr.flags |= VHOST_USER_REPLY_MASK;
> -
> payload.u64 = !!ret;
> hdr.size = sizeof(payload.u64);
>
> - iovec[0].iov_base = &hdr;
> - iovec[0].iov_len = VHOST_USER_HDR_SIZE;
> - iovec[1].iov_base = &payload;
> - iovec[1].iov_len = hdr.size;
> -
> - if (qio_channel_writev_all(ioc, iovec, ARRAY_SIZE(iovec), &local_err)) {
> - error_report_err(local_err);
> + if (!vhost_user_send_resp(ioc, &hdr, &payload)) {
> goto err;
> }
> }
> --
> 2.40.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/4] vhost-user: add shared_object msg
2023-05-24 9:13 ` [PATCH v3 3/4] vhost-user: add shared_object msg Albert Esteve
2023-06-22 19:57 ` Michael S. Tsirkin
@ 2023-06-22 20:18 ` Marc-André Lureau
2023-06-23 7:45 ` Albert Esteve
2023-06-23 6:45 ` Michael S. Tsirkin
2023-06-23 8:14 ` Michael S. Tsirkin
3 siblings, 1 reply; 19+ messages in thread
From: Marc-André Lureau @ 2023-06-22 20:18 UTC (permalink / raw)
To: Albert Esteve, Michael S. Tsirkin; +Cc: qemu-devel, kraxel, cohuck, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 12705 bytes --]
Hi
On Wed, May 24, 2023 at 11:13 AM Albert Esteve <aesteve@redhat.com> wrote:
> Add new vhost-user protocol message
> `VHOST_USER_BACKEND_SHARED_OBJECT`. This new
> message is sent from vhost-user back-ends
> to interact with the virtio-dmabuf table
> in order to add, remove, or lookup for
> virtio dma-buf shared objects.
>
> The action taken in the front-end depends
> on the type stored in the payload struct.
>
> In the libvhost-user library add helper
> functions to allow sending messages to
> interact with the virtio shared objects
> hash table.
>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
> docs/interop/vhost-user.rst | 15 ++++
> hw/virtio/vhost-user.c | 68 ++++++++++++++++++
> subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++++++++
> subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++++++
> 4 files changed, 227 insertions(+)
>
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index 5a070adbc1..d3d8db41e5 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -1528,6 +1528,21 @@ is sent by the front-end.
>
> The state.num field is currently reserved and must be set to 0.
>
> +``VHOST_USER_BACKEND_SHARED_OBJECT``
> + :id: 6
> + :equivalent ioctl: N/A
> + :request payload: ``struct VhostUserShared``
> + :reply payload: ``struct VhostUserShared`` (only for ``LOOKUP``
> requests)
>
only for LOOKUP, ahah...
> +
> + Backends that need to interact with the virtio-dmabuf shared table API
> + can send this message. The operation is determined by the ``type``
> member
> + of the payload struct. The valid values for the operation type are
> + ``VHOST_SHARED_OBJECT_*`` members, i.e., ``ADD``, ``LOOKUP``, and
> ``REMOVE``.
>
...why not use specific messages instead of this extra "type"?
> + ``LOOKUP`` operations require the ``VHOST_USER_NEED_REPLY_MASK`` flag
> to be
> + set by the back-end, and the front-end will then send the dma-buf fd as
> + a response if the UUID matches an object in the table, or a negative
> value
> + otherwise.
>
This new message(s) should be initially negotiated with a protocol feature
flag.
> +
> .. _reply_ack:
>
> VHOST_USER_PROTOCOL_F_REPLY_ACK
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 74a2a28663..5ac5f0eafd 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -10,6 +10,7 @@
>
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> +#include "hw/virtio/virtio-dmabuf.h"
> #include "hw/virtio/vhost.h"
> #include "hw/virtio/vhost-user.h"
> #include "hw/virtio/vhost-backend.h"
> @@ -20,6 +21,7 @@
> #include "sysemu/kvm.h"
> #include "qemu/error-report.h"
> #include "qemu/main-loop.h"
> +#include "qemu/uuid.h"
> #include "qemu/sockets.h"
> #include "sysemu/runstate.h"
> #include "sysemu/cryptodev.h"
> @@ -128,6 +130,7 @@ typedef enum VhostUserSlaveRequest {
> VHOST_USER_BACKEND_IOTLB_MSG = 1,
> VHOST_USER_BACKEND_CONFIG_CHANGE_MSG = 2,
> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> VHOST_USER_BACKEND_MAX
> } VhostUserSlaveRequest;
>
> @@ -190,6 +193,18 @@ typedef struct VhostUserInflight {
> uint16_t queue_size;
> } VhostUserInflight;
>
> +typedef enum VhostUserSharedType {
> + VHOST_SHARED_OBJECT_ADD = 0,
> + VHOST_SHARED_OBJECT_LOOKUP,
> + VHOST_SHARED_OBJECT_REMOVE,
> +} VhostUserSharedType;
> +
> +typedef struct VhostUserShared {
> + unsigned char uuid[16];
> + VhostUserSharedType type;
> + int dmabuf_fd;
> +} VhostUserShared;
> +
> typedef struct {
> VhostUserRequest request;
>
> @@ -214,6 +229,7 @@ typedef union {
> VhostUserCryptoSession session;
> VhostUserVringArea area;
> VhostUserInflight inflight;
> + VhostUserShared object;
> } VhostUserPayload;
>
> typedef struct VhostUserMsg {
> @@ -1582,6 +1598,52 @@ static int
> vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
> return 0;
> }
>
> +static int vhost_user_backend_handle_shared_object(VhostUserShared
> *object)
> +{
> + QemuUUID uuid;
> + memcpy(uuid.data, object->uuid, sizeof(object->uuid));
> +
> + switch (object->type) {
> + case VHOST_SHARED_OBJECT_ADD:
> + return virtio_add_dmabuf(&uuid, object->dmabuf_fd);
> + case VHOST_SHARED_OBJECT_LOOKUP:
> + object->dmabuf_fd = virtio_lookup_dmabuf(&uuid);
> + if (object->dmabuf_fd < 0) {
> + return object->dmabuf_fd;
> + }
> + break;
> + case VHOST_SHARED_OBJECT_REMOVE:
> + return virtio_remove_resource(&uuid);
> + }
> +
> + return 0;
> +}
> +
> +static bool
> +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> + VhostUserPayload *payload)
> +{
> + Error *local_err = NULL;
> + struct iovec iov[2];
> + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> + hdr->flags |= VHOST_USER_REPLY_MASK;
> +
> + hdr->size = sizeof(payload->object);
> +
> + iov[0].iov_base = hdr;
> + iov[0].iov_len = VHOST_USER_HDR_SIZE;
> + iov[1].iov_base = payload;
> + iov[1].iov_len = hdr->size;
> +
> + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov),
> &local_err)) {
> + error_report_err(local_err);
> + return false;
> + }
> + }
> + return true;
> +}
> +
> static void close_slave_channel(struct vhost_user *u)
> {
> g_source_destroy(u->slave_src);
> @@ -1639,6 +1701,12 @@ static gboolean slave_read(QIOChannel *ioc,
> GIOCondition condition,
> ret = vhost_user_slave_handle_vring_host_notifier(dev,
> &payload.area,
> fd ? fd[0] :
> -1);
> break;
> + case VHOST_USER_BACKEND_SHARED_OBJECT:
> + ret = vhost_user_backend_handle_shared_object(&payload.object);
> + if (!vhost_user_backend_send_dmabuf_fd(ioc, &hdr, &payload)) {
> + goto err;
> + }
> + break;
> default:
> error_report("Received unexpected msg type: %d.", hdr.request);
> ret = -EINVAL;
> diff --git a/subprojects/libvhost-user/libvhost-user.c
> b/subprojects/libvhost-user/libvhost-user.c
> index 8fb61e2df2..27f16d292a 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -1403,6 +1403,94 @@ bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq
> *vq, int fd,
> return vu_process_message_reply(dev, &vmsg);
> }
>
> +bool
> +vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
> *dmabuf_fd)
> +{
> + bool result = false;
> + VhostUserMsg msg_reply;
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
> + .payload.object = {
> + .type = VHOST_SHARED_OBJECT_LOOKUP,
> + },
> + };
> +
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + pthread_mutex_lock(&dev->slave_mutex);
> + if (!vu_message_write(dev, dev->slave_fd, &msg)) {
> + goto out;
> + }
> +
> + if (!vu_message_read_default(dev, dev->slave_fd, &msg_reply)) {
> + goto out;
> + }
> +
> + if (msg_reply.request != msg.request) {
> + DPRINT("Received unexpected msg type. Expected %d, received %d",
> + msg.request, msg_reply.request);
> + goto out;
> + }
> +
> + *dmabuf_fd = msg_reply.payload.object.dmabuf_fd;
> + result = *dmabuf_fd > 0;
> +out:
> + pthread_mutex_unlock(&dev->slave_mutex);
> +
> + return result;
> +}
> +
> +static bool
> +vu_send_message(VuDev *dev, VhostUserMsg *vmsg)
> +{
> + bool result = false;
> + pthread_mutex_lock(&dev->slave_mutex);
> + if (!vu_message_write(dev, dev->slave_fd, vmsg)) {
> + goto out;
> + }
> +
> + result = true;
> +out:
> + pthread_mutex_unlock(&dev->slave_mutex);
> +
> + return result;
> +}
> +
> +bool
> +vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
> dmabuf_fd)
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION,
> + .payload.object = {
> + .dmabuf_fd = dmabuf_fd,
> + .type = VHOST_SHARED_OBJECT_ADD,
> + },
> + };
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + return vu_send_message(dev, &msg);
> +}
> +
> +bool
> +vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION,
> + .payload.object = {
> + .type = VHOST_SHARED_OBJECT_REMOVE,
> + },
> + };
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + return vu_send_message(dev, &msg);
> +}
> +
> static bool
> vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
> {
> diff --git a/subprojects/libvhost-user/libvhost-user.h
> b/subprojects/libvhost-user/libvhost-user.h
> index 49208cceaa..a43d115bd7 100644
> --- a/subprojects/libvhost-user/libvhost-user.h
> +++ b/subprojects/libvhost-user/libvhost-user.h
> @@ -119,6 +119,7 @@ typedef enum VhostUserSlaveRequest {
> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> VHOST_USER_BACKEND_VRING_CALL = 4,
> VHOST_USER_BACKEND_VRING_ERR = 5,
> + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> VHOST_USER_BACKEND_MAX
> } VhostUserSlaveRequest;
>
> @@ -172,6 +173,20 @@ typedef struct VhostUserInflight {
> uint16_t queue_size;
> } VhostUserInflight;
>
> +typedef enum VhostUserSharedType {
> + VHOST_SHARED_OBJECT_ADD = 0,
> + VHOST_SHARED_OBJECT_LOOKUP,
> + VHOST_SHARED_OBJECT_REMOVE,
> +} VhostUserSharedType;
> +
> +#define UUID_LEN 16
> +
> +typedef struct VhostUserShared {
> + unsigned char uuid[UUID_LEN];
> + VhostUserSharedType type;
> + int dmabuf_fd;
> +} VhostUserShared;
> +
> #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> # define VU_PACKED __attribute__((gcc_struct, packed))
> #else
> @@ -199,6 +214,7 @@ typedef struct VhostUserMsg {
> VhostUserConfig config;
> VhostUserVringArea area;
> VhostUserInflight inflight;
> + VhostUserShared object;
> } payload;
>
> int fds[VHOST_MEMORY_BASELINE_NREGIONS];
> @@ -539,6 +555,46 @@ void vu_set_queue_handler(VuDev *dev, VuVirtq *vq,
> bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
> int size, int offset);
>
> +/**
> + * vu_get_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + * @dmabuf_fd: output dma-buf file descriptor
> + *
> + * Lookup for a virtio shared object (i.e., dma-buf fd) associated with
> the
> + * received UUID. Result, if found, is stored in the dmabuf_fd argument.
> + *
> + * Returns: whether the virtio object was found.
> + */
> +bool vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> + int *dmabuf_fd);
> +
> +/**
> + * vu_add_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + * @dmabuf_fd: output dma-buf file descriptor
> + *
> + * Stores a new shared object (i.e., dma-buf fd) in the hash table, and
> + * associates it with the received UUID.
> + *
> + * Returns: TRUE on success, FALSE on failure.
> + */
> +bool vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> + int dmabuf_fd);
> +
> +/**
> + * vu_rm_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + *
> + * Removes a shared object (i.e., dma-buf fd) associated with the
> + * received UUID from the hash table.
> + *
> + * Returns: TRUE on success, FALSE on failure.
> + */
> +bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
> +
> /**
> * vu_queue_set_notification:
> * @dev: a VuDev context
> --
> 2.40.0
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 15285 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/4] vhost-user: add shared_object msg
2023-05-24 9:13 ` [PATCH v3 3/4] vhost-user: add shared_object msg Albert Esteve
2023-06-22 19:57 ` Michael S. Tsirkin
2023-06-22 20:18 ` Marc-André Lureau
@ 2023-06-23 6:45 ` Michael S. Tsirkin
2023-06-23 7:19 ` Albert Esteve
2023-06-23 13:46 ` Albert Esteve
2023-06-23 8:14 ` Michael S. Tsirkin
3 siblings, 2 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2023-06-23 6:45 UTC (permalink / raw)
To: Albert Esteve; +Cc: qemu-devel, kraxel, marcandre.lureau, cohuck, Fam Zheng
On Wed, May 24, 2023 at 11:13:32AM +0200, Albert Esteve wrote:
> Add new vhost-user protocol message
> `VHOST_USER_BACKEND_SHARED_OBJECT`. This new
> message is sent from vhost-user back-ends
> to interact with the virtio-dmabuf table
> in order to add, remove, or lookup for
> virtio dma-buf shared objects.
>
> The action taken in the front-end depends
> on the type stored in the payload struct.
>
> In the libvhost-user library add helper
> functions to allow sending messages to
> interact with the virtio shared objects
> hash table.
>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
> docs/interop/vhost-user.rst | 15 ++++
> hw/virtio/vhost-user.c | 68 ++++++++++++++++++
> subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++++++++
> subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++++++
> 4 files changed, 227 insertions(+)
>
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index 5a070adbc1..d3d8db41e5 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -1528,6 +1528,21 @@ is sent by the front-end.
>
> The state.num field is currently reserved and must be set to 0.
>
> +``VHOST_USER_BACKEND_SHARED_OBJECT``
> + :id: 6
> + :equivalent ioctl: N/A
> + :request payload: ``struct VhostUserShared``
> + :reply payload: ``struct VhostUserShared`` (only for ``LOOKUP`` requests)
> +
> + Backends that need to interact with the virtio-dmabuf shared table API
> + can send this message. The operation is determined by the ``type`` member
> + of the payload struct. The valid values for the operation type are
> + ``VHOST_SHARED_OBJECT_*`` members, i.e., ``ADD``, ``LOOKUP``, and ``REMOVE``.
> + ``LOOKUP`` operations require the ``VHOST_USER_NEED_REPLY_MASK`` flag to be
> + set by the back-end, and the front-end will then send the dma-buf fd as
> + a response if the UUID matches an object in the table, or a negative value
> + otherwise.
> +
> .. _reply_ack:
>
> VHOST_USER_PROTOCOL_F_REPLY_ACK
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 74a2a28663..5ac5f0eafd 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -10,6 +10,7 @@
>
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> +#include "hw/virtio/virtio-dmabuf.h"
> #include "hw/virtio/vhost.h"
> #include "hw/virtio/vhost-user.h"
> #include "hw/virtio/vhost-backend.h"
> @@ -20,6 +21,7 @@
> #include "sysemu/kvm.h"
> #include "qemu/error-report.h"
> #include "qemu/main-loop.h"
> +#include "qemu/uuid.h"
> #include "qemu/sockets.h"
> #include "sysemu/runstate.h"
> #include "sysemu/cryptodev.h"
> @@ -128,6 +130,7 @@ typedef enum VhostUserSlaveRequest {
> VHOST_USER_BACKEND_IOTLB_MSG = 1,
> VHOST_USER_BACKEND_CONFIG_CHANGE_MSG = 2,
> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> VHOST_USER_BACKEND_MAX
> } VhostUserSlaveRequest;
>
> @@ -190,6 +193,18 @@ typedef struct VhostUserInflight {
> uint16_t queue_size;
> } VhostUserInflight;
>
> +typedef enum VhostUserSharedType {
> + VHOST_SHARED_OBJECT_ADD = 0,
> + VHOST_SHARED_OBJECT_LOOKUP,
> + VHOST_SHARED_OBJECT_REMOVE,
> +} VhostUserSharedType;
> +
> +typedef struct VhostUserShared {
> + unsigned char uuid[16];
> + VhostUserSharedType type;
> + int dmabuf_fd;
> +} VhostUserShared;
> +
> typedef struct {
> VhostUserRequest request;
>
> @@ -214,6 +229,7 @@ typedef union {
> VhostUserCryptoSession session;
> VhostUserVringArea area;
> VhostUserInflight inflight;
> + VhostUserShared object;
> } VhostUserPayload;
>
> typedef struct VhostUserMsg {
> @@ -1582,6 +1598,52 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
> return 0;
> }
>
> +static int vhost_user_backend_handle_shared_object(VhostUserShared *object)
> +{
> + QemuUUID uuid;
Can we initialize it here? uuid = { .data = object->uuid } ?
Also, pls put space after variable declaration.
> + memcpy(uuid.data, object->uuid, sizeof(object->uuid));
> +
> + switch (object->type) {
> + case VHOST_SHARED_OBJECT_ADD:
> + return virtio_add_dmabuf(&uuid, object->dmabuf_fd);
> + case VHOST_SHARED_OBJECT_LOOKUP:
> + object->dmabuf_fd = virtio_lookup_dmabuf(&uuid);
> + if (object->dmabuf_fd < 0) {
> + return object->dmabuf_fd;
> + }
> + break;
> + case VHOST_SHARED_OBJECT_REMOVE:
> + return virtio_remove_resource(&uuid);
> + }
> +
I couldn't figure out why, but if I commit this then run checkpatch,
like this
./scripts/checkpatch.pl HEAD~1..HEAD
then it is unhappy about the : in case. Any idea why?
> + return 0;
> +}
> +
> +static bool
> +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> + VhostUserPayload *payload)
> +{
> + Error *local_err = NULL;
> + struct iovec iov[2];
> + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> + hdr->flags |= VHOST_USER_REPLY_MASK;
> +
> + hdr->size = sizeof(payload->object);
> +
> + iov[0].iov_base = hdr;
> + iov[0].iov_len = VHOST_USER_HDR_SIZE;
> + iov[1].iov_base = payload;
> + iov[1].iov_len = hdr->size;
> +
> + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
> + error_report_err(local_err);
> + return false;
> + }
> + }
> + return true;
> +}
> +
> static void close_slave_channel(struct vhost_user *u)
> {
> g_source_destroy(u->slave_src);
> @@ -1639,6 +1701,12 @@ static gboolean slave_read(QIOChannel *ioc, GIOCondition condition,
> ret = vhost_user_slave_handle_vring_host_notifier(dev, &payload.area,
> fd ? fd[0] : -1);
> break;
> + case VHOST_USER_BACKEND_SHARED_OBJECT:
> + ret = vhost_user_backend_handle_shared_object(&payload.object);
> + if (!vhost_user_backend_send_dmabuf_fd(ioc, &hdr, &payload)) {
> + goto err;
> + }
> + break;
> default:
> error_report("Received unexpected msg type: %d.", hdr.request);
> ret = -EINVAL;
> diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
> index 8fb61e2df2..27f16d292a 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -1403,6 +1403,94 @@ bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
> return vu_process_message_reply(dev, &vmsg);
> }
>
> +bool
> +vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int *dmabuf_fd)
> +{
> + bool result = false;
> + VhostUserMsg msg_reply;
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
> + .payload.object = {
> + .type = VHOST_SHARED_OBJECT_LOOKUP,
> + },
> + };
> +
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + pthread_mutex_lock(&dev->slave_mutex);
> + if (!vu_message_write(dev, dev->slave_fd, &msg)) {
> + goto out;
> + }
> +
> + if (!vu_message_read_default(dev, dev->slave_fd, &msg_reply)) {
> + goto out;
> + }
> +
> + if (msg_reply.request != msg.request) {
> + DPRINT("Received unexpected msg type. Expected %d, received %d",
> + msg.request, msg_reply.request);
> + goto out;
> + }
> +
> + *dmabuf_fd = msg_reply.payload.object.dmabuf_fd;
> + result = *dmabuf_fd > 0;
> +out:
> + pthread_mutex_unlock(&dev->slave_mutex);
> +
> + return result;
> +}
> +
> +static bool
> +vu_send_message(VuDev *dev, VhostUserMsg *vmsg)
> +{
> + bool result = false;
> + pthread_mutex_lock(&dev->slave_mutex);
> + if (!vu_message_write(dev, dev->slave_fd, vmsg)) {
> + goto out;
> + }
> +
> + result = true;
> +out:
> + pthread_mutex_unlock(&dev->slave_mutex);
> +
> + return result;
> +}
> +
> +bool
> +vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int dmabuf_fd)
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION,
> + .payload.object = {
> + .dmabuf_fd = dmabuf_fd,
> + .type = VHOST_SHARED_OBJECT_ADD,
> + },
> + };
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + return vu_send_message(dev, &msg);
> +}
> +
> +bool
> +vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION,
> + .payload.object = {
> + .type = VHOST_SHARED_OBJECT_REMOVE,
> + },
> + };
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + return vu_send_message(dev, &msg);
> +}
> +
> static bool
> vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
> {
> diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
> index 49208cceaa..a43d115bd7 100644
> --- a/subprojects/libvhost-user/libvhost-user.h
> +++ b/subprojects/libvhost-user/libvhost-user.h
> @@ -119,6 +119,7 @@ typedef enum VhostUserSlaveRequest {
> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> VHOST_USER_BACKEND_VRING_CALL = 4,
> VHOST_USER_BACKEND_VRING_ERR = 5,
> + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> VHOST_USER_BACKEND_MAX
> } VhostUserSlaveRequest;
>
> @@ -172,6 +173,20 @@ typedef struct VhostUserInflight {
> uint16_t queue_size;
> } VhostUserInflight;
>
> +typedef enum VhostUserSharedType {
> + VHOST_SHARED_OBJECT_ADD = 0,
> + VHOST_SHARED_OBJECT_LOOKUP,
> + VHOST_SHARED_OBJECT_REMOVE,
> +} VhostUserSharedType;
> +
> +#define UUID_LEN 16
> +
> +typedef struct VhostUserShared {
> + unsigned char uuid[UUID_LEN];
> + VhostUserSharedType type;
> + int dmabuf_fd;
> +} VhostUserShared;
> +
> #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> # define VU_PACKED __attribute__((gcc_struct, packed))
> #else
> @@ -199,6 +214,7 @@ typedef struct VhostUserMsg {
> VhostUserConfig config;
> VhostUserVringArea area;
> VhostUserInflight inflight;
> + VhostUserShared object;
> } payload;
>
> int fds[VHOST_MEMORY_BASELINE_NREGIONS];
> @@ -539,6 +555,46 @@ void vu_set_queue_handler(VuDev *dev, VuVirtq *vq,
> bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
> int size, int offset);
>
> +/**
> + * vu_get_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + * @dmabuf_fd: output dma-buf file descriptor
> + *
> + * Lookup for a virtio shared object (i.e., dma-buf fd) associated with the
> + * received UUID. Result, if found, is stored in the dmabuf_fd argument.
> + *
> + * Returns: whether the virtio object was found.
> + */
> +bool vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> + int *dmabuf_fd);
> +
> +/**
> + * vu_add_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + * @dmabuf_fd: output dma-buf file descriptor
> + *
> + * Stores a new shared object (i.e., dma-buf fd) in the hash table, and
> + * associates it with the received UUID.
> + *
> + * Returns: TRUE on success, FALSE on failure.
> + */
> +bool vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> + int dmabuf_fd);
> +
> +/**
> + * vu_rm_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + *
> + * Removes a shared object (i.e., dma-buf fd) associated with the
> + * received UUID from the hash table.
> + *
> + * Returns: TRUE on success, FALSE on failure.
> + */
> +bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
> +
> /**
> * vu_queue_set_notification:
> * @dev: a VuDev context
> --
> 2.40.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 4/4] vhost-user: refactor send_resp code
2023-05-24 9:13 ` [PATCH v3 4/4] vhost-user: refactor send_resp code Albert Esteve
@ 2023-06-23 6:48 ` Michael S. Tsirkin
0 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2023-06-23 6:48 UTC (permalink / raw)
To: Albert Esteve; +Cc: qemu-devel, kraxel, marcandre.lureau, cohuck, Fam Zheng
On Wed, May 24, 2023 at 11:13:33AM +0200, Albert Esteve wrote:
> Refactor code to send response message so that
> all common parts both for the common REPLY_ACK
> case, and other data responses, can call it and
> avoid code repetition.
>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
> hw/virtio/vhost-user.c | 52 +++++++++++++++++++-----------------------
> 1 file changed, 24 insertions(+), 28 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 5ac5f0eafd..b888f2c177 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1619,28 +1619,36 @@ static int vhost_user_backend_handle_shared_object(VhostUserShared *object)
> return 0;
> }
>
> -static bool
> -vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> - VhostUserPayload *payload)
> +static bool vhost_user_send_resp(QIOChannel *ioc, VhostUserHeader *hdr,
> + VhostUserPayload *payload)
> {
> Error *local_err = NULL;
> struct iovec iov[2];
As long as you are refactoring, please add an empty line here
after variable declaration.
Also, can't we initialize it here?
struct iovec iov[] = {
{ .iov_base = hdr ....},
{ .iov_base = payload }
};
will also avoid the need for explicit size.
> - if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> - hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> - hdr->flags |= VHOST_USER_REPLY_MASK;
> + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> + hdr->flags |= VHOST_USER_REPLY_MASK;
>
> - hdr->size = sizeof(payload->object);
> + iov[0].iov_base = hdr;
> + iov[0].iov_len = VHOST_USER_HDR_SIZE;
> + iov[1].iov_base = payload;
> + iov[1].iov_len = hdr->size;
> +
> + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
> + error_report_err(local_err);
> + return false;
> + }
>
> - iov[0].iov_base = hdr;
> - iov[0].iov_len = VHOST_USER_HDR_SIZE;
> - iov[1].iov_base = payload;
> - iov[1].iov_len = hdr->size;
> + return true;
> +}
>
> - if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
> - error_report_err(local_err);
> - return false;
> - }
> +static bool
> +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> + VhostUserPayload *payload)
> +{
> + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> + hdr->size = sizeof(payload->object);
> + return vhost_user_send_resp(ioc, hdr, payload);
> }
> +
> return true;
> }
>
> @@ -1717,22 +1725,10 @@ static gboolean slave_read(QIOChannel *ioc, GIOCondition condition,
> * directly in their request handlers.
> */
> if (hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
> - struct iovec iovec[2];
> -
> -
> - hdr.flags &= ~VHOST_USER_NEED_REPLY_MASK;
> - hdr.flags |= VHOST_USER_REPLY_MASK;
> -
> payload.u64 = !!ret;
> hdr.size = sizeof(payload.u64);
>
> - iovec[0].iov_base = &hdr;
> - iovec[0].iov_len = VHOST_USER_HDR_SIZE;
> - iovec[1].iov_base = &payload;
> - iovec[1].iov_len = hdr.size;
> -
> - if (qio_channel_writev_all(ioc, iovec, ARRAY_SIZE(iovec), &local_err)) {
> - error_report_err(local_err);
> + if (!vhost_user_send_resp(ioc, &hdr, &payload)) {
> goto err;
> }
> }
> --
> 2.40.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 0/4] Virtio shared dma-buf
2023-05-24 9:13 [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
` (4 preceding siblings ...)
2023-06-21 8:20 ` [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
@ 2023-06-23 6:49 ` Michael S. Tsirkin
5 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2023-06-23 6:49 UTC (permalink / raw)
To: Albert Esteve; +Cc: qemu-devel, kraxel, marcandre.lureau, cohuck, Fam Zheng
On Wed, May 24, 2023 at 11:13:29AM +0200, Albert Esteve wrote:
> v1 link -> https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg00598.html
> v2 link -> https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04530.html
> v2 -> v3:
> - Change UUID hash function strategy to djb
> - Add qemu_uuid_is_equal wrapper
Posted some minor comments. Pls address and I'll merge.
> This patch covers the required steps to add support for virtio cross-device resource sharing[1],
> which support is already available in the kernel.
>
> The main usecase will be sharing dma buffers from virtio-gpu devices (as the exporter
> -see VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID in [2]), to virtio-video (under discussion)
> devices (as the buffer-user or importer). Therefore, even though virtio specs talk about
> resources or objects[3], this patch adds the infrastructure with dma-bufs in mind.
> Note that virtio specs let the devices themselves define what a vitio object is.
>
> These are the main parts that are covered in the patch:
>
> - Add hash_func and key_equal_func to uuid
> - Shared resources table, to hold all resources that can be shared in the host and their assigned UUID
> - Internal shared table API for virtio devices to add, lookup and remove resources
> - Unit test to verify the API.
> - New message to the vhost-user protocol to allow backend to interact with the shared
> table API through the control socket
>
> Applies cleanly to 1c12355
>
> [1] - https://lwn.net/Articles/828988/
> [2] - https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-3730006
> [3] - https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-10500011
>
> Albert Esteve (4):
> uuid: add hash_func and equal_func
> virtio-dmabuf: introduce virtio-dmabuf
> vhost-user: add shared_object msg
> vhost-user: refactor send_resp code
>
> MAINTAINERS | 7 ++
> docs/interop/vhost-user.rst | 15 +++
> hw/display/meson.build | 1 +
> hw/display/virtio-dmabuf.c | 90 +++++++++++++++++
> hw/virtio/vhost-user.c | 90 ++++++++++++++---
> include/hw/virtio/virtio-dmabuf.h | 59 ++++++++++++
> include/qemu/uuid.h | 2 +
> subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++
> subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++
> tests/unit/meson.build | 1 +
> tests/unit/test-uuid.c | 27 ++++++
> tests/unit/test-virtio-dmabuf.c | 112 ++++++++++++++++++++++
> util/uuid.c | 14 +++
> 13 files changed, 549 insertions(+), 13 deletions(-)
> create mode 100644 hw/display/virtio-dmabuf.c
> create mode 100644 include/hw/virtio/virtio-dmabuf.h
> create mode 100644 tests/unit/test-virtio-dmabuf.c
>
> --
> 2.40.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/4] vhost-user: add shared_object msg
2023-06-23 6:45 ` Michael S. Tsirkin
@ 2023-06-23 7:19 ` Albert Esteve
2023-06-23 7:32 ` Michael S. Tsirkin
2023-06-23 13:46 ` Albert Esteve
1 sibling, 1 reply; 19+ messages in thread
From: Albert Esteve @ 2023-06-23 7:19 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, kraxel, marcandre.lureau, cohuck, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 13634 bytes --]
On Fri, Jun 23, 2023 at 8:45 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, May 24, 2023 at 11:13:32AM +0200, Albert Esteve wrote:
> > Add new vhost-user protocol message
> > `VHOST_USER_BACKEND_SHARED_OBJECT`. This new
> > message is sent from vhost-user back-ends
> > to interact with the virtio-dmabuf table
> > in order to add, remove, or lookup for
> > virtio dma-buf shared objects.
> >
> > The action taken in the front-end depends
> > on the type stored in the payload struct.
> >
> > In the libvhost-user library add helper
> > functions to allow sending messages to
> > interact with the virtio shared objects
> > hash table.
> >
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> > docs/interop/vhost-user.rst | 15 ++++
> > hw/virtio/vhost-user.c | 68 ++++++++++++++++++
> > subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++++++++
> > subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++++++
> > 4 files changed, 227 insertions(+)
> >
> > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> > index 5a070adbc1..d3d8db41e5 100644
> > --- a/docs/interop/vhost-user.rst
> > +++ b/docs/interop/vhost-user.rst
> > @@ -1528,6 +1528,21 @@ is sent by the front-end.
> >
> > The state.num field is currently reserved and must be set to 0.
> >
> > +``VHOST_USER_BACKEND_SHARED_OBJECT``
> > + :id: 6
> > + :equivalent ioctl: N/A
> > + :request payload: ``struct VhostUserShared``
> > + :reply payload: ``struct VhostUserShared`` (only for ``LOOKUP``
> requests)
> > +
> > + Backends that need to interact with the virtio-dmabuf shared table API
> > + can send this message. The operation is determined by the ``type``
> member
> > + of the payload struct. The valid values for the operation type are
> > + ``VHOST_SHARED_OBJECT_*`` members, i.e., ``ADD``, ``LOOKUP``, and
> ``REMOVE``.
> > + ``LOOKUP`` operations require the ``VHOST_USER_NEED_REPLY_MASK`` flag
> to be
> > + set by the back-end, and the front-end will then send the dma-buf fd
> as
> > + a response if the UUID matches an object in the table, or a negative
> value
> > + otherwise.
> > +
> > .. _reply_ack:
> >
> > VHOST_USER_PROTOCOL_F_REPLY_ACK
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 74a2a28663..5ac5f0eafd 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -10,6 +10,7 @@
> >
> > #include "qemu/osdep.h"
> > #include "qapi/error.h"
> > +#include "hw/virtio/virtio-dmabuf.h"
> > #include "hw/virtio/vhost.h"
> > #include "hw/virtio/vhost-user.h"
> > #include "hw/virtio/vhost-backend.h"
> > @@ -20,6 +21,7 @@
> > #include "sysemu/kvm.h"
> > #include "qemu/error-report.h"
> > #include "qemu/main-loop.h"
> > +#include "qemu/uuid.h"
> > #include "qemu/sockets.h"
> > #include "sysemu/runstate.h"
> > #include "sysemu/cryptodev.h"
> > @@ -128,6 +130,7 @@ typedef enum VhostUserSlaveRequest {
> > VHOST_USER_BACKEND_IOTLB_MSG = 1,
> > VHOST_USER_BACKEND_CONFIG_CHANGE_MSG = 2,
> > VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> > + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> > VHOST_USER_BACKEND_MAX
> > } VhostUserSlaveRequest;
> >
> > @@ -190,6 +193,18 @@ typedef struct VhostUserInflight {
> > uint16_t queue_size;
> > } VhostUserInflight;
> >
> > +typedef enum VhostUserSharedType {
> > + VHOST_SHARED_OBJECT_ADD = 0,
> > + VHOST_SHARED_OBJECT_LOOKUP,
> > + VHOST_SHARED_OBJECT_REMOVE,
> > +} VhostUserSharedType;
> > +
> > +typedef struct VhostUserShared {
> > + unsigned char uuid[16];
> > + VhostUserSharedType type;
> > + int dmabuf_fd;
> > +} VhostUserShared;
> > +
> > typedef struct {
> > VhostUserRequest request;
> >
> > @@ -214,6 +229,7 @@ typedef union {
> > VhostUserCryptoSession session;
> > VhostUserVringArea area;
> > VhostUserInflight inflight;
> > + VhostUserShared object;
> > } VhostUserPayload;
> >
> > typedef struct VhostUserMsg {
> > @@ -1582,6 +1598,52 @@ static int
> vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
> > return 0;
> > }
> >
> > +static int vhost_user_backend_handle_shared_object(VhostUserShared
> *object)
> > +{
> > + QemuUUID uuid;
>
> Can we initialize it here? uuid = { .data = object->uuid } ?
>
> Also, pls put space after variable declaration.
>
> > + memcpy(uuid.data, object->uuid, sizeof(object->uuid));
> > +
> > + switch (object->type) {
> > + case VHOST_SHARED_OBJECT_ADD:
> > + return virtio_add_dmabuf(&uuid, object->dmabuf_fd);
> > + case VHOST_SHARED_OBJECT_LOOKUP:
> > + object->dmabuf_fd = virtio_lookup_dmabuf(&uuid);
> > + if (object->dmabuf_fd < 0) {
> > + return object->dmabuf_fd;
> > + }
> > + break;
> > + case VHOST_SHARED_OBJECT_REMOVE:
> > + return virtio_remove_resource(&uuid);
> > + }
> > +
>
> I couldn't figure out why, but if I commit this then run checkpatch,
> like this
> ./scripts/checkpatch.pl HEAD~1..HEAD
>
> then it is unhappy about the : in case. Any idea why?
>
I don't see any errors / warnings. What do you get?
>
> > + return 0;
> > +}
> > +
> > +static bool
> > +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> > + VhostUserPayload *payload)
> > +{
> > + Error *local_err = NULL;
> > + struct iovec iov[2];
> > + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> > + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> > + hdr->flags |= VHOST_USER_REPLY_MASK;
> > +
> > + hdr->size = sizeof(payload->object);
> > +
> > + iov[0].iov_base = hdr;
> > + iov[0].iov_len = VHOST_USER_HDR_SIZE;
> > + iov[1].iov_base = payload;
> > + iov[1].iov_len = hdr->size;
> > +
> > + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov),
> &local_err)) {
> > + error_report_err(local_err);
> > + return false;
> > + }
> > + }
> > + return true;
> > +}
> > +
> > static void close_slave_channel(struct vhost_user *u)
> > {
> > g_source_destroy(u->slave_src);
> > @@ -1639,6 +1701,12 @@ static gboolean slave_read(QIOChannel *ioc,
> GIOCondition condition,
> > ret = vhost_user_slave_handle_vring_host_notifier(dev,
> &payload.area,
> > fd ? fd[0] :
> -1);
> > break;
> > + case VHOST_USER_BACKEND_SHARED_OBJECT:
> > + ret = vhost_user_backend_handle_shared_object(&payload.object);
> > + if (!vhost_user_backend_send_dmabuf_fd(ioc, &hdr, &payload)) {
> > + goto err;
> > + }
> > + break;
> > default:
> > error_report("Received unexpected msg type: %d.", hdr.request);
> > ret = -EINVAL;
> > diff --git a/subprojects/libvhost-user/libvhost-user.c
> b/subprojects/libvhost-user/libvhost-user.c
> > index 8fb61e2df2..27f16d292a 100644
> > --- a/subprojects/libvhost-user/libvhost-user.c
> > +++ b/subprojects/libvhost-user/libvhost-user.c
> > @@ -1403,6 +1403,94 @@ bool vu_set_queue_host_notifier(VuDev *dev,
> VuVirtq *vq, int fd,
> > return vu_process_message_reply(dev, &vmsg);
> > }
> >
> > +bool
> > +vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
> *dmabuf_fd)
> > +{
> > + bool result = false;
> > + VhostUserMsg msg_reply;
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
> > + .payload.object = {
> > + .type = VHOST_SHARED_OBJECT_LOOKUP,
> > + },
> > + };
> > +
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + pthread_mutex_lock(&dev->slave_mutex);
> > + if (!vu_message_write(dev, dev->slave_fd, &msg)) {
> > + goto out;
> > + }
> > +
> > + if (!vu_message_read_default(dev, dev->slave_fd, &msg_reply)) {
> > + goto out;
> > + }
> > +
> > + if (msg_reply.request != msg.request) {
> > + DPRINT("Received unexpected msg type. Expected %d, received %d",
> > + msg.request, msg_reply.request);
> > + goto out;
> > + }
> > +
> > + *dmabuf_fd = msg_reply.payload.object.dmabuf_fd;
> > + result = *dmabuf_fd > 0;
> > +out:
> > + pthread_mutex_unlock(&dev->slave_mutex);
> > +
> > + return result;
> > +}
> > +
> > +static bool
> > +vu_send_message(VuDev *dev, VhostUserMsg *vmsg)
> > +{
> > + bool result = false;
> > + pthread_mutex_lock(&dev->slave_mutex);
> > + if (!vu_message_write(dev, dev->slave_fd, vmsg)) {
> > + goto out;
> > + }
> > +
> > + result = true;
> > +out:
> > + pthread_mutex_unlock(&dev->slave_mutex);
> > +
> > + return result;
> > +}
> > +
> > +bool
> > +vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
> dmabuf_fd)
> > +{
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION,
> > + .payload.object = {
> > + .dmabuf_fd = dmabuf_fd,
> > + .type = VHOST_SHARED_OBJECT_ADD,
> > + },
> > + };
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + return vu_send_message(dev, &msg);
> > +}
> > +
> > +bool
> > +vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
> > +{
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION,
> > + .payload.object = {
> > + .type = VHOST_SHARED_OBJECT_REMOVE,
> > + },
> > + };
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + return vu_send_message(dev, &msg);
> > +}
> > +
> > static bool
> > vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
> > {
> > diff --git a/subprojects/libvhost-user/libvhost-user.h
> b/subprojects/libvhost-user/libvhost-user.h
> > index 49208cceaa..a43d115bd7 100644
> > --- a/subprojects/libvhost-user/libvhost-user.h
> > +++ b/subprojects/libvhost-user/libvhost-user.h
> > @@ -119,6 +119,7 @@ typedef enum VhostUserSlaveRequest {
> > VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> > VHOST_USER_BACKEND_VRING_CALL = 4,
> > VHOST_USER_BACKEND_VRING_ERR = 5,
> > + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> > VHOST_USER_BACKEND_MAX
> > } VhostUserSlaveRequest;
> >
> > @@ -172,6 +173,20 @@ typedef struct VhostUserInflight {
> > uint16_t queue_size;
> > } VhostUserInflight;
> >
> > +typedef enum VhostUserSharedType {
> > + VHOST_SHARED_OBJECT_ADD = 0,
> > + VHOST_SHARED_OBJECT_LOOKUP,
> > + VHOST_SHARED_OBJECT_REMOVE,
> > +} VhostUserSharedType;
> > +
> > +#define UUID_LEN 16
> > +
> > +typedef struct VhostUserShared {
> > + unsigned char uuid[UUID_LEN];
> > + VhostUserSharedType type;
> > + int dmabuf_fd;
> > +} VhostUserShared;
> > +
> > #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> > # define VU_PACKED __attribute__((gcc_struct, packed))
> > #else
> > @@ -199,6 +214,7 @@ typedef struct VhostUserMsg {
> > VhostUserConfig config;
> > VhostUserVringArea area;
> > VhostUserInflight inflight;
> > + VhostUserShared object;
> > } payload;
> >
> > int fds[VHOST_MEMORY_BASELINE_NREGIONS];
> > @@ -539,6 +555,46 @@ void vu_set_queue_handler(VuDev *dev, VuVirtq *vq,
> > bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
> > int size, int offset);
> >
> > +/**
> > + * vu_get_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + * @dmabuf_fd: output dma-buf file descriptor
> > + *
> > + * Lookup for a virtio shared object (i.e., dma-buf fd) associated with
> the
> > + * received UUID. Result, if found, is stored in the dmabuf_fd argument.
> > + *
> > + * Returns: whether the virtio object was found.
> > + */
> > +bool vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> > + int *dmabuf_fd);
> > +
> > +/**
> > + * vu_add_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + * @dmabuf_fd: output dma-buf file descriptor
> > + *
> > + * Stores a new shared object (i.e., dma-buf fd) in the hash table, and
> > + * associates it with the received UUID.
> > + *
> > + * Returns: TRUE on success, FALSE on failure.
> > + */
> > +bool vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> > + int dmabuf_fd);
> > +
> > +/**
> > + * vu_rm_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + *
> > + * Removes a shared object (i.e., dma-buf fd) associated with the
> > + * received UUID from the hash table.
> > + *
> > + * Returns: TRUE on success, FALSE on failure.
> > + */
> > +bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
> > +
> > /**
> > * vu_queue_set_notification:
> > * @dev: a VuDev context
> > --
> > 2.40.0
>
>
[-- Attachment #2: Type: text/html, Size: 17030 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/4] vhost-user: add shared_object msg
2023-06-23 7:19 ` Albert Esteve
@ 2023-06-23 7:32 ` Michael S. Tsirkin
0 siblings, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2023-06-23 7:32 UTC (permalink / raw)
To: Albert Esteve
Cc: qemu-devel, kraxel, marcandre.lureau, cohuck, Fam Zheng, pbonzini
On Fri, Jun 23, 2023 at 09:19:29AM +0200, Albert Esteve wrote:
>
>
> On Fri, Jun 23, 2023 at 8:45 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, May 24, 2023 at 11:13:32AM +0200, Albert Esteve wrote:
> > Add new vhost-user protocol message
> > `VHOST_USER_BACKEND_SHARED_OBJECT`. This new
> > message is sent from vhost-user back-ends
> > to interact with the virtio-dmabuf table
> > in order to add, remove, or lookup for
> > virtio dma-buf shared objects.
> >
> > The action taken in the front-end depends
> > on the type stored in the payload struct.
> >
> > In the libvhost-user library add helper
> > functions to allow sending messages to
> > interact with the virtio shared objects
> > hash table.
> >
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> > docs/interop/vhost-user.rst | 15 ++++
> > hw/virtio/vhost-user.c | 68 ++++++++++++++++++
> > subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++++++++
> > subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++++++
> > 4 files changed, 227 insertions(+)
> >
> > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> > index 5a070adbc1..d3d8db41e5 100644
> > --- a/docs/interop/vhost-user.rst
> > +++ b/docs/interop/vhost-user.rst
> > @@ -1528,6 +1528,21 @@ is sent by the front-end.
> >
> > The state.num field is currently reserved and must be set to 0.
> >
> > +``VHOST_USER_BACKEND_SHARED_OBJECT``
> > + :id: 6
> > + :equivalent ioctl: N/A
> > + :request payload: ``struct VhostUserShared``
> > + :reply payload: ``struct VhostUserShared`` (only for ``LOOKUP``
> requests)
> > +
> > + Backends that need to interact with the virtio-dmabuf shared table API
> > + can send this message. The operation is determined by the ``type``
> member
> > + of the payload struct. The valid values for the operation type are
> > + ``VHOST_SHARED_OBJECT_*`` members, i.e., ``ADD``, ``LOOKUP``, and
> ``REMOVE``.
> > + ``LOOKUP`` operations require the ``VHOST_USER_NEED_REPLY_MASK`` flag
> to be
> > + set by the back-end, and the front-end will then send the dma-buf fd
> as
> > + a response if the UUID matches an object in the table, or a negative
> value
> > + otherwise.
> > +
> > .. _reply_ack:
> >
> > VHOST_USER_PROTOCOL_F_REPLY_ACK
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 74a2a28663..5ac5f0eafd 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -10,6 +10,7 @@
> >
> > #include "qemu/osdep.h"
> > #include "qapi/error.h"
> > +#include "hw/virtio/virtio-dmabuf.h"
> > #include "hw/virtio/vhost.h"
> > #include "hw/virtio/vhost-user.h"
> > #include "hw/virtio/vhost-backend.h"
> > @@ -20,6 +21,7 @@
> > #include "sysemu/kvm.h"
> > #include "qemu/error-report.h"
> > #include "qemu/main-loop.h"
> > +#include "qemu/uuid.h"
> > #include "qemu/sockets.h"
> > #include "sysemu/runstate.h"
> > #include "sysemu/cryptodev.h"
> > @@ -128,6 +130,7 @@ typedef enum VhostUserSlaveRequest {
> > VHOST_USER_BACKEND_IOTLB_MSG = 1,
> > VHOST_USER_BACKEND_CONFIG_CHANGE_MSG = 2,
> > VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> > + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> > VHOST_USER_BACKEND_MAX
> > } VhostUserSlaveRequest;
> >
> > @@ -190,6 +193,18 @@ typedef struct VhostUserInflight {
> > uint16_t queue_size;
> > } VhostUserInflight;
> >
> > +typedef enum VhostUserSharedType {
> > + VHOST_SHARED_OBJECT_ADD = 0,
> > + VHOST_SHARED_OBJECT_LOOKUP,
> > + VHOST_SHARED_OBJECT_REMOVE,
> > +} VhostUserSharedType;
> > +
> > +typedef struct VhostUserShared {
> > + unsigned char uuid[16];
> > + VhostUserSharedType type;
> > + int dmabuf_fd;
> > +} VhostUserShared;
> > +
> > typedef struct {
> > VhostUserRequest request;
> >
> > @@ -214,6 +229,7 @@ typedef union {
> > VhostUserCryptoSession session;
> > VhostUserVringArea area;
> > VhostUserInflight inflight;
> > + VhostUserShared object;
> > } VhostUserPayload;
> >
> > typedef struct VhostUserMsg {
> > @@ -1582,6 +1598,52 @@ static int
> vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
> > return 0;
> > }
> >
> > +static int vhost_user_backend_handle_shared_object(VhostUserShared
> *object)
> > +{
> > + QemuUUID uuid;
>
> Can we initialize it here? uuid = { .data = object->uuid } ?
>
> Also, pls put space after variable declaration.
>
> > + memcpy(uuid.data, object->uuid, sizeof(object->uuid));
> > +
> > + switch (object->type) {
> > + case VHOST_SHARED_OBJECT_ADD:
> > + return virtio_add_dmabuf(&uuid, object->dmabuf_fd);
> > + case VHOST_SHARED_OBJECT_LOOKUP:
> > + object->dmabuf_fd = virtio_lookup_dmabuf(&uuid);
> > + if (object->dmabuf_fd < 0) {
> > + return object->dmabuf_fd;
> > + }
> > + break;
> > + case VHOST_SHARED_OBJECT_REMOVE:
> > + return virtio_remove_resource(&uuid);
> > + }
> > +
>
> I couldn't figure out why, but if I commit this then run checkpatch,
> like this
> ./scripts/checkpatch.pl HEAD~1..HEAD
>
> then it is unhappy about the : in case. Any idea why?
>
>
> I don't see any errors / warnings. What do you get?
15/90 Checking commit 6cd801b0656b (vhost-user: add shared_object msg)
ERROR: spaces required around that ':' (ctx:VxE)
#187: FILE: hw/virtio/vhost-user.c:1619:
+ case VHOST_SHARED_OBJECT_ADD:
^
ERROR: spaces required around that ':' (ctx:VxE)
#189: FILE: hw/virtio/vhost-user.c:1621:
+ case VHOST_SHARED_OBJECT_LOOKUP:
^
ERROR: spaces required around that ':' (ctx:VxE)
#195: FILE: hw/virtio/vhost-user.c:1627:
+ case VHOST_SHARED_OBJECT_REMOVE:
^
ERROR: spaces required around that ':' (ctx:VxE)
#234: FILE: hw/virtio/vhost-user.c:1716:
+ case VHOST_USER_BACKEND_SHARED_OBJECT:
^
total: 4 errors, 0 warnings, 305 lines checked
which is wrong.
>
> > + return 0;
> > +}
> > +
> > +static bool
> > +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> > + VhostUserPayload *payload)
> > +{
> > + Error *local_err = NULL;
> > + struct iovec iov[2];
> > + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> > + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> > + hdr->flags |= VHOST_USER_REPLY_MASK;
> > +
> > + hdr->size = sizeof(payload->object);
> > +
> > + iov[0].iov_base = hdr;
> > + iov[0].iov_len = VHOST_USER_HDR_SIZE;
> > + iov[1].iov_base = payload;
> > + iov[1].iov_len = hdr->size;
> > +
> > + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &
> local_err)) {
> > + error_report_err(local_err);
> > + return false;
> > + }
> > + }
> > + return true;
> > +}
> > +
> > static void close_slave_channel(struct vhost_user *u)
> > {
> > g_source_destroy(u->slave_src);
> > @@ -1639,6 +1701,12 @@ static gboolean slave_read(QIOChannel *ioc,
> GIOCondition condition,
> > ret = vhost_user_slave_handle_vring_host_notifier(dev, &
> payload.area,
> > fd ? fd[0] :
> -1);
> > break;
> > + case VHOST_USER_BACKEND_SHARED_OBJECT:
> > + ret = vhost_user_backend_handle_shared_object(&payload.object);
> > + if (!vhost_user_backend_send_dmabuf_fd(ioc, &hdr, &payload)) {
> > + goto err;
> > + }
> > + break;
> > default:
> > error_report("Received unexpected msg type: %d.", hdr.request);
> > ret = -EINVAL;
> > diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/
> libvhost-user/libvhost-user.c
> > index 8fb61e2df2..27f16d292a 100644
> > --- a/subprojects/libvhost-user/libvhost-user.c
> > +++ b/subprojects/libvhost-user/libvhost-user.c
> > @@ -1403,6 +1403,94 @@ bool vu_set_queue_host_notifier(VuDev *dev,
> VuVirtq *vq, int fd,
> > return vu_process_message_reply(dev, &vmsg);
> > }
> >
> > +bool
> > +vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
> *dmabuf_fd)
> > +{
> > + bool result = false;
> > + VhostUserMsg msg_reply;
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
> > + .payload.object = {
> > + .type = VHOST_SHARED_OBJECT_LOOKUP,
> > + },
> > + };
> > +
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + pthread_mutex_lock(&dev->slave_mutex);
> > + if (!vu_message_write(dev, dev->slave_fd, &msg)) {
> > + goto out;
> > + }
> > +
> > + if (!vu_message_read_default(dev, dev->slave_fd, &msg_reply)) {
> > + goto out;
> > + }
> > +
> > + if (msg_reply.request != msg.request) {
> > + DPRINT("Received unexpected msg type. Expected %d, received %d",
> > + msg.request, msg_reply.request);
> > + goto out;
> > + }
> > +
> > + *dmabuf_fd = msg_reply.payload.object.dmabuf_fd;
> > + result = *dmabuf_fd > 0;
> > +out:
> > + pthread_mutex_unlock(&dev->slave_mutex);
> > +
> > + return result;
> > +}
> > +
> > +static bool
> > +vu_send_message(VuDev *dev, VhostUserMsg *vmsg)
> > +{
> > + bool result = false;
> > + pthread_mutex_lock(&dev->slave_mutex);
> > + if (!vu_message_write(dev, dev->slave_fd, vmsg)) {
> > + goto out;
> > + }
> > +
> > + result = true;
> > +out:
> > + pthread_mutex_unlock(&dev->slave_mutex);
> > +
> > + return result;
> > +}
> > +
> > +bool
> > +vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
> dmabuf_fd)
> > +{
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION,
> > + .payload.object = {
> > + .dmabuf_fd = dmabuf_fd,
> > + .type = VHOST_SHARED_OBJECT_ADD,
> > + },
> > + };
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + return vu_send_message(dev, &msg);
> > +}
> > +
> > +bool
> > +vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
> > +{
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION,
> > + .payload.object = {
> > + .type = VHOST_SHARED_OBJECT_REMOVE,
> > + },
> > + };
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + return vu_send_message(dev, &msg);
> > +}
> > +
> > static bool
> > vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
> > {
> > diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/
> libvhost-user/libvhost-user.h
> > index 49208cceaa..a43d115bd7 100644
> > --- a/subprojects/libvhost-user/libvhost-user.h
> > +++ b/subprojects/libvhost-user/libvhost-user.h
> > @@ -119,6 +119,7 @@ typedef enum VhostUserSlaveRequest {
> > VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> > VHOST_USER_BACKEND_VRING_CALL = 4,
> > VHOST_USER_BACKEND_VRING_ERR = 5,
> > + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> > VHOST_USER_BACKEND_MAX
> > } VhostUserSlaveRequest;
> >
> > @@ -172,6 +173,20 @@ typedef struct VhostUserInflight {
> > uint16_t queue_size;
> > } VhostUserInflight;
> >
> > +typedef enum VhostUserSharedType {
> > + VHOST_SHARED_OBJECT_ADD = 0,
> > + VHOST_SHARED_OBJECT_LOOKUP,
> > + VHOST_SHARED_OBJECT_REMOVE,
> > +} VhostUserSharedType;
> > +
> > +#define UUID_LEN 16
> > +
> > +typedef struct VhostUserShared {
> > + unsigned char uuid[UUID_LEN];
> > + VhostUserSharedType type;
> > + int dmabuf_fd;
> > +} VhostUserShared;
> > +
> > #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> > # define VU_PACKED __attribute__((gcc_struct, packed))
> > #else
> > @@ -199,6 +214,7 @@ typedef struct VhostUserMsg {
> > VhostUserConfig config;
> > VhostUserVringArea area;
> > VhostUserInflight inflight;
> > + VhostUserShared object;
> > } payload;
> >
> > int fds[VHOST_MEMORY_BASELINE_NREGIONS];
> > @@ -539,6 +555,46 @@ void vu_set_queue_handler(VuDev *dev, VuVirtq *vq,
> > bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
> > int size, int offset);
> >
> > +/**
> > + * vu_get_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + * @dmabuf_fd: output dma-buf file descriptor
> > + *
> > + * Lookup for a virtio shared object (i.e., dma-buf fd) associated with
> the
> > + * received UUID. Result, if found, is stored in the dmabuf_fd argument.
> > + *
> > + * Returns: whether the virtio object was found.
> > + */
> > +bool vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> > + int *dmabuf_fd);
> > +
> > +/**
> > + * vu_add_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + * @dmabuf_fd: output dma-buf file descriptor
> > + *
> > + * Stores a new shared object (i.e., dma-buf fd) in the hash table, and
> > + * associates it with the received UUID.
> > + *
> > + * Returns: TRUE on success, FALSE on failure.
> > + */
> > +bool vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> > + int dmabuf_fd);
> > +
> > +/**
> > + * vu_rm_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + *
> > + * Removes a shared object (i.e., dma-buf fd) associated with the
> > + * received UUID from the hash table.
> > + *
> > + * Returns: TRUE on success, FALSE on failure.
> > + */
> > +bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
> > +
> > /**
> > * vu_queue_set_notification:
> > * @dev: a VuDev context
> > --
> > 2.40.0
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/4] vhost-user: add shared_object msg
2023-06-22 20:18 ` Marc-André Lureau
@ 2023-06-23 7:45 ` Albert Esteve
0 siblings, 0 replies; 19+ messages in thread
From: Albert Esteve @ 2023-06-23 7:45 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Michael S. Tsirkin, qemu-devel, kraxel, cohuck, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 14028 bytes --]
On Thu, Jun 22, 2023 at 10:19 PM Marc-André Lureau <
marcandre.lureau@gmail.com> wrote:
> Hi
>
> On Wed, May 24, 2023 at 11:13 AM Albert Esteve <aesteve@redhat.com> wrote:
>
>> Add new vhost-user protocol message
>> `VHOST_USER_BACKEND_SHARED_OBJECT`. This new
>> message is sent from vhost-user back-ends
>> to interact with the virtio-dmabuf table
>> in order to add, remove, or lookup for
>> virtio dma-buf shared objects.
>>
>> The action taken in the front-end depends
>> on the type stored in the payload struct.
>>
>> In the libvhost-user library add helper
>> functions to allow sending messages to
>> interact with the virtio shared objects
>> hash table.
>>
>> Signed-off-by: Albert Esteve <aesteve@redhat.com>
>> ---
>> docs/interop/vhost-user.rst | 15 ++++
>> hw/virtio/vhost-user.c | 68 ++++++++++++++++++
>> subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++++++++
>> subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++++++
>> 4 files changed, 227 insertions(+)
>>
>> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
>> index 5a070adbc1..d3d8db41e5 100644
>> --- a/docs/interop/vhost-user.rst
>> +++ b/docs/interop/vhost-user.rst
>> @@ -1528,6 +1528,21 @@ is sent by the front-end.
>>
>> The state.num field is currently reserved and must be set to 0.
>>
>> +``VHOST_USER_BACKEND_SHARED_OBJECT``
>> + :id: 6
>> + :equivalent ioctl: N/A
>> + :request payload: ``struct VhostUserShared``
>> + :reply payload: ``struct VhostUserShared`` (only for ``LOOKUP``
>> requests)
>>
>
> only for LOOKUP, ahah...
>
I wasn't sure how to highlight that the reply was not required in all
cases...
>
>
>> +
>> + Backends that need to interact with the virtio-dmabuf shared table API
>> + can send this message. The operation is determined by the ``type``
>> member
>> + of the payload struct. The valid values for the operation type are
>> + ``VHOST_SHARED_OBJECT_*`` members, i.e., ``ADD``, ``LOOKUP``, and
>> ``REMOVE``.
>>
>
> ...why not use specific messages instead of this extra "type"?
>
I wouldn't mind having specific messages for each type.
I initially opted for having it one message to avoid having the code that
handles the
message scattered. The logic of each operation is simple and this way we
have a single
handler method for all of them.
Also, there are currently only a few slave requests available, so adding 3
new requests felt
like a higher "overhead".
But as I said, I don't have a strong opinion. Should I change it?
>
>
>
>> + ``LOOKUP`` operations require the ``VHOST_USER_NEED_REPLY_MASK`` flag
>> to be
>> + set by the back-end, and the front-end will then send the dma-buf fd as
>> + a response if the UUID matches an object in the table, or a negative
>> value
>> + otherwise.
>>
>
> This new message(s) should be initially negotiated with a protocol feature
> flag.
>
Ok. Similar to `VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS` and
`VHOST_USER_BACKEND_VRING_CALL`, where it checks the feature with
`vu_has_protocol_feature`?
The name could be `VHOST_USER_PROTOCOL_F_SHARED_OBJECT` (?).
>
>
>> +
>> .. _reply_ack:
>>
>> VHOST_USER_PROTOCOL_F_REPLY_ACK
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index 74a2a28663..5ac5f0eafd 100644
>> --- a/hw/virtio/vhost-user.c
>> +++ b/hw/virtio/vhost-user.c
>> @@ -10,6 +10,7 @@
>>
>> #include "qemu/osdep.h"
>> #include "qapi/error.h"
>> +#include "hw/virtio/virtio-dmabuf.h"
>> #include "hw/virtio/vhost.h"
>> #include "hw/virtio/vhost-user.h"
>> #include "hw/virtio/vhost-backend.h"
>> @@ -20,6 +21,7 @@
>> #include "sysemu/kvm.h"
>> #include "qemu/error-report.h"
>> #include "qemu/main-loop.h"
>> +#include "qemu/uuid.h"
>> #include "qemu/sockets.h"
>> #include "sysemu/runstate.h"
>> #include "sysemu/cryptodev.h"
>> @@ -128,6 +130,7 @@ typedef enum VhostUserSlaveRequest {
>> VHOST_USER_BACKEND_IOTLB_MSG = 1,
>> VHOST_USER_BACKEND_CONFIG_CHANGE_MSG = 2,
>> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
>> + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
>> VHOST_USER_BACKEND_MAX
>> } VhostUserSlaveRequest;
>>
>> @@ -190,6 +193,18 @@ typedef struct VhostUserInflight {
>> uint16_t queue_size;
>> } VhostUserInflight;
>>
>> +typedef enum VhostUserSharedType {
>> + VHOST_SHARED_OBJECT_ADD = 0,
>> + VHOST_SHARED_OBJECT_LOOKUP,
>> + VHOST_SHARED_OBJECT_REMOVE,
>> +} VhostUserSharedType;
>> +
>> +typedef struct VhostUserShared {
>> + unsigned char uuid[16];
>> + VhostUserSharedType type;
>> + int dmabuf_fd;
>> +} VhostUserShared;
>> +
>> typedef struct {
>> VhostUserRequest request;
>>
>> @@ -214,6 +229,7 @@ typedef union {
>> VhostUserCryptoSession session;
>> VhostUserVringArea area;
>> VhostUserInflight inflight;
>> + VhostUserShared object;
>> } VhostUserPayload;
>>
>> typedef struct VhostUserMsg {
>> @@ -1582,6 +1598,52 @@ static int
>> vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
>> return 0;
>> }
>>
>> +static int vhost_user_backend_handle_shared_object(VhostUserShared
>> *object)
>> +{
>> + QemuUUID uuid;
>> + memcpy(uuid.data, object->uuid, sizeof(object->uuid));
>> +
>> + switch (object->type) {
>> + case VHOST_SHARED_OBJECT_ADD:
>> + return virtio_add_dmabuf(&uuid, object->dmabuf_fd);
>> + case VHOST_SHARED_OBJECT_LOOKUP:
>> + object->dmabuf_fd = virtio_lookup_dmabuf(&uuid);
>> + if (object->dmabuf_fd < 0) {
>> + return object->dmabuf_fd;
>> + }
>> + break;
>> + case VHOST_SHARED_OBJECT_REMOVE:
>> + return virtio_remove_resource(&uuid);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static bool
>> +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
>> + VhostUserPayload *payload)
>> +{
>> + Error *local_err = NULL;
>> + struct iovec iov[2];
>> + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
>> + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
>> + hdr->flags |= VHOST_USER_REPLY_MASK;
>> +
>> + hdr->size = sizeof(payload->object);
>> +
>> + iov[0].iov_base = hdr;
>> + iov[0].iov_len = VHOST_USER_HDR_SIZE;
>> + iov[1].iov_base = payload;
>> + iov[1].iov_len = hdr->size;
>> +
>> + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov),
>> &local_err)) {
>> + error_report_err(local_err);
>> + return false;
>> + }
>> + }
>> + return true;
>> +}
>> +
>> static void close_slave_channel(struct vhost_user *u)
>> {
>> g_source_destroy(u->slave_src);
>> @@ -1639,6 +1701,12 @@ static gboolean slave_read(QIOChannel *ioc,
>> GIOCondition condition,
>> ret = vhost_user_slave_handle_vring_host_notifier(dev,
>> &payload.area,
>> fd ? fd[0] :
>> -1);
>> break;
>> + case VHOST_USER_BACKEND_SHARED_OBJECT:
>> + ret = vhost_user_backend_handle_shared_object(&payload.object);
>> + if (!vhost_user_backend_send_dmabuf_fd(ioc, &hdr, &payload)) {
>> + goto err;
>> + }
>> + break;
>> default:
>> error_report("Received unexpected msg type: %d.", hdr.request);
>> ret = -EINVAL;
>> diff --git a/subprojects/libvhost-user/libvhost-user.c
>> b/subprojects/libvhost-user/libvhost-user.c
>> index 8fb61e2df2..27f16d292a 100644
>> --- a/subprojects/libvhost-user/libvhost-user.c
>> +++ b/subprojects/libvhost-user/libvhost-user.c
>> @@ -1403,6 +1403,94 @@ bool vu_set_queue_host_notifier(VuDev *dev,
>> VuVirtq *vq, int fd,
>> return vu_process_message_reply(dev, &vmsg);
>> }
>>
>> +bool
>> +vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
>> *dmabuf_fd)
>> +{
>> + bool result = false;
>> + VhostUserMsg msg_reply;
>> + VhostUserMsg msg = {
>> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
>> + .size = sizeof(msg.payload.object),
>> + .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
>> + .payload.object = {
>> + .type = VHOST_SHARED_OBJECT_LOOKUP,
>> + },
>> + };
>> +
>> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
>> +
>> + pthread_mutex_lock(&dev->slave_mutex);
>> + if (!vu_message_write(dev, dev->slave_fd, &msg)) {
>> + goto out;
>> + }
>> +
>> + if (!vu_message_read_default(dev, dev->slave_fd, &msg_reply)) {
>> + goto out;
>> + }
>> +
>> + if (msg_reply.request != msg.request) {
>> + DPRINT("Received unexpected msg type. Expected %d, received %d",
>> + msg.request, msg_reply.request);
>> + goto out;
>> + }
>> +
>> + *dmabuf_fd = msg_reply.payload.object.dmabuf_fd;
>> + result = *dmabuf_fd > 0;
>> +out:
>> + pthread_mutex_unlock(&dev->slave_mutex);
>> +
>> + return result;
>> +}
>> +
>> +static bool
>> +vu_send_message(VuDev *dev, VhostUserMsg *vmsg)
>> +{
>> + bool result = false;
>> + pthread_mutex_lock(&dev->slave_mutex);
>> + if (!vu_message_write(dev, dev->slave_fd, vmsg)) {
>> + goto out;
>> + }
>> +
>> + result = true;
>> +out:
>> + pthread_mutex_unlock(&dev->slave_mutex);
>> +
>> + return result;
>> +}
>> +
>> +bool
>> +vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
>> dmabuf_fd)
>> +{
>> + VhostUserMsg msg = {
>> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
>> + .size = sizeof(msg.payload.object),
>> + .flags = VHOST_USER_VERSION,
>> + .payload.object = {
>> + .dmabuf_fd = dmabuf_fd,
>> + .type = VHOST_SHARED_OBJECT_ADD,
>> + },
>> + };
>> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
>> +
>> + return vu_send_message(dev, &msg);
>> +}
>> +
>> +bool
>> +vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
>> +{
>> + VhostUserMsg msg = {
>> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
>> + .size = sizeof(msg.payload.object),
>> + .flags = VHOST_USER_VERSION,
>> + .payload.object = {
>> + .type = VHOST_SHARED_OBJECT_REMOVE,
>> + },
>> + };
>> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
>> +
>> + return vu_send_message(dev, &msg);
>> +}
>> +
>> static bool
>> vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
>> {
>> diff --git a/subprojects/libvhost-user/libvhost-user.h
>> b/subprojects/libvhost-user/libvhost-user.h
>> index 49208cceaa..a43d115bd7 100644
>> --- a/subprojects/libvhost-user/libvhost-user.h
>> +++ b/subprojects/libvhost-user/libvhost-user.h
>> @@ -119,6 +119,7 @@ typedef enum VhostUserSlaveRequest {
>> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
>> VHOST_USER_BACKEND_VRING_CALL = 4,
>> VHOST_USER_BACKEND_VRING_ERR = 5,
>> + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
>> VHOST_USER_BACKEND_MAX
>> } VhostUserSlaveRequest;
>>
>> @@ -172,6 +173,20 @@ typedef struct VhostUserInflight {
>> uint16_t queue_size;
>> } VhostUserInflight;
>>
>> +typedef enum VhostUserSharedType {
>> + VHOST_SHARED_OBJECT_ADD = 0,
>> + VHOST_SHARED_OBJECT_LOOKUP,
>> + VHOST_SHARED_OBJECT_REMOVE,
>> +} VhostUserSharedType;
>> +
>> +#define UUID_LEN 16
>> +
>> +typedef struct VhostUserShared {
>> + unsigned char uuid[UUID_LEN];
>> + VhostUserSharedType type;
>> + int dmabuf_fd;
>> +} VhostUserShared;
>> +
>> #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
>> # define VU_PACKED __attribute__((gcc_struct, packed))
>> #else
>> @@ -199,6 +214,7 @@ typedef struct VhostUserMsg {
>> VhostUserConfig config;
>> VhostUserVringArea area;
>> VhostUserInflight inflight;
>> + VhostUserShared object;
>> } payload;
>>
>> int fds[VHOST_MEMORY_BASELINE_NREGIONS];
>> @@ -539,6 +555,46 @@ void vu_set_queue_handler(VuDev *dev, VuVirtq *vq,
>> bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
>> int size, int offset);
>>
>> +/**
>> + * vu_get_shared_object:
>> + * @dev: a VuDev context
>> + * @uuid: UUID of the shared object
>> + * @dmabuf_fd: output dma-buf file descriptor
>> + *
>> + * Lookup for a virtio shared object (i.e., dma-buf fd) associated with
>> the
>> + * received UUID. Result, if found, is stored in the dmabuf_fd argument.
>> + *
>> + * Returns: whether the virtio object was found.
>> + */
>> +bool vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
>> + int *dmabuf_fd);
>> +
>> +/**
>> + * vu_add_shared_object:
>> + * @dev: a VuDev context
>> + * @uuid: UUID of the shared object
>> + * @dmabuf_fd: output dma-buf file descriptor
>> + *
>> + * Stores a new shared object (i.e., dma-buf fd) in the hash table, and
>> + * associates it with the received UUID.
>> + *
>> + * Returns: TRUE on success, FALSE on failure.
>> + */
>> +bool vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
>> + int dmabuf_fd);
>> +
>> +/**
>> + * vu_rm_shared_object:
>> + * @dev: a VuDev context
>> + * @uuid: UUID of the shared object
>> + *
>> + * Removes a shared object (i.e., dma-buf fd) associated with the
>> + * received UUID from the hash table.
>> + *
>> + * Returns: TRUE on success, FALSE on failure.
>> + */
>> +bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
>> +
>> /**
>> * vu_queue_set_notification:
>> * @dev: a VuDev context
>> --
>> 2.40.0
>>
>>
>
> --
> Marc-André Lureau
>
Thanks!
Albert Esteve
[-- Attachment #2: Type: text/html, Size: 17330 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/4] vhost-user: add shared_object msg
2023-05-24 9:13 ` [PATCH v3 3/4] vhost-user: add shared_object msg Albert Esteve
` (2 preceding siblings ...)
2023-06-23 6:45 ` Michael S. Tsirkin
@ 2023-06-23 8:14 ` Michael S. Tsirkin
2023-06-23 12:08 ` Albert Esteve
3 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2023-06-23 8:14 UTC (permalink / raw)
To: Albert Esteve; +Cc: qemu-devel, kraxel, marcandre.lureau, cohuck, Fam Zheng
On Wed, May 24, 2023 at 11:13:32AM +0200, Albert Esteve wrote:
> Add new vhost-user protocol message
> `VHOST_USER_BACKEND_SHARED_OBJECT`. This new
> message is sent from vhost-user back-ends
> to interact with the virtio-dmabuf table
> in order to add, remove, or lookup for
> virtio dma-buf shared objects.
>
> The action taken in the front-end depends
> on the type stored in the payload struct.
>
> In the libvhost-user library add helper
> functions to allow sending messages to
> interact with the virtio shared objects
> hash table.
>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
> docs/interop/vhost-user.rst | 15 ++++
> hw/virtio/vhost-user.c | 68 ++++++++++++++++++
> subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++++++++
> subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++++++
> 4 files changed, 227 insertions(+)
>
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index 5a070adbc1..d3d8db41e5 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -1528,6 +1528,21 @@ is sent by the front-end.
>
> The state.num field is currently reserved and must be set to 0.
>
> +``VHOST_USER_BACKEND_SHARED_OBJECT``
> + :id: 6
> + :equivalent ioctl: N/A
> + :request payload: ``struct VhostUserShared``
> + :reply payload: ``struct VhostUserShared`` (only for ``LOOKUP`` requests)
> +
> + Backends that need to interact with the virtio-dmabuf shared table API
> + can send this message. The operation is determined by the ``type`` member
> + of the payload struct. The valid values for the operation type are
> + ``VHOST_SHARED_OBJECT_*`` members, i.e., ``ADD``, ``LOOKUP``, and ``REMOVE``.
> + ``LOOKUP`` operations require the ``VHOST_USER_NEED_REPLY_MASK`` flag to be
> + set by the back-end, and the front-end will then send the dma-buf fd as
> + a response if the UUID matches an object in the table, or a negative value
> + otherwise.
> +
> .. _reply_ack:
If you are reusing same message then I would maybe just make all types
the same - if there's a reply bit backend can reply back with the same
message - might be good for debugging etc.
Also it's not detailed here but I am guessing LOOKUP at least
does not send an fd and gets an fd back? Maybe LOOKUP
at least actually should be a separate message.
> VHOST_USER_PROTOCOL_F_REPLY_ACK
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 74a2a28663..5ac5f0eafd 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -10,6 +10,7 @@
>
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> +#include "hw/virtio/virtio-dmabuf.h"
> #include "hw/virtio/vhost.h"
> #include "hw/virtio/vhost-user.h"
> #include "hw/virtio/vhost-backend.h"
> @@ -20,6 +21,7 @@
> #include "sysemu/kvm.h"
> #include "qemu/error-report.h"
> #include "qemu/main-loop.h"
> +#include "qemu/uuid.h"
> #include "qemu/sockets.h"
> #include "sysemu/runstate.h"
> #include "sysemu/cryptodev.h"
> @@ -128,6 +130,7 @@ typedef enum VhostUserSlaveRequest {
> VHOST_USER_BACKEND_IOTLB_MSG = 1,
> VHOST_USER_BACKEND_CONFIG_CHANGE_MSG = 2,
> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> VHOST_USER_BACKEND_MAX
> } VhostUserSlaveRequest;
>
> @@ -190,6 +193,18 @@ typedef struct VhostUserInflight {
> uint16_t queue_size;
> } VhostUserInflight;
>
> +typedef enum VhostUserSharedType {
> + VHOST_SHARED_OBJECT_ADD = 0,
> + VHOST_SHARED_OBJECT_LOOKUP,
> + VHOST_SHARED_OBJECT_REMOVE,
> +} VhostUserSharedType;
> +
> +typedef struct VhostUserShared {
> + unsigned char uuid[16];
> + VhostUserSharedType type;
> + int dmabuf_fd;
> +} VhostUserShared;
BTW all fields in structs that go on the wire should be fixed size.
> +
> typedef struct {
> VhostUserRequest request;
>
> @@ -214,6 +229,7 @@ typedef union {
> VhostUserCryptoSession session;
> VhostUserVringArea area;
> VhostUserInflight inflight;
> + VhostUserShared object;
> } VhostUserPayload;
>
> typedef struct VhostUserMsg {
> @@ -1582,6 +1598,52 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
> return 0;
> }
>
> +static int vhost_user_backend_handle_shared_object(VhostUserShared *object)
> +{
> + QemuUUID uuid;
> + memcpy(uuid.data, object->uuid, sizeof(object->uuid));
> +
> + switch (object->type) {
> + case VHOST_SHARED_OBJECT_ADD:
> + return virtio_add_dmabuf(&uuid, object->dmabuf_fd);
> + case VHOST_SHARED_OBJECT_LOOKUP:
> + object->dmabuf_fd = virtio_lookup_dmabuf(&uuid);
> + if (object->dmabuf_fd < 0) {
> + return object->dmabuf_fd;
> + }
> + break;
> + case VHOST_SHARED_OBJECT_REMOVE:
> + return virtio_remove_resource(&uuid);
> + }
> +
> + return 0;
> +}
> +
> +static bool
> +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> + VhostUserPayload *payload)
> +{
> + Error *local_err = NULL;
> + struct iovec iov[2];
> + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> + hdr->flags |= VHOST_USER_REPLY_MASK;
> +
> + hdr->size = sizeof(payload->object);
> +
> + iov[0].iov_base = hdr;
> + iov[0].iov_len = VHOST_USER_HDR_SIZE;
> + iov[1].iov_base = payload;
> + iov[1].iov_len = hdr->size;
> +
> + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov), &local_err)) {
> + error_report_err(local_err);
> + return false;
> + }
> + }
> + return true;
> +}
> +
> static void close_slave_channel(struct vhost_user *u)
> {
> g_source_destroy(u->slave_src);
> @@ -1639,6 +1701,12 @@ static gboolean slave_read(QIOChannel *ioc, GIOCondition condition,
> ret = vhost_user_slave_handle_vring_host_notifier(dev, &payload.area,
> fd ? fd[0] : -1);
> break;
> + case VHOST_USER_BACKEND_SHARED_OBJECT:
> + ret = vhost_user_backend_handle_shared_object(&payload.object);
> + if (!vhost_user_backend_send_dmabuf_fd(ioc, &hdr, &payload)) {
> + goto err;
> + }
> + break;
> default:
> error_report("Received unexpected msg type: %d.", hdr.request);
> ret = -EINVAL;
> diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
> index 8fb61e2df2..27f16d292a 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -1403,6 +1403,94 @@ bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
> return vu_process_message_reply(dev, &vmsg);
> }
>
> +bool
> +vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int *dmabuf_fd)
> +{
> + bool result = false;
> + VhostUserMsg msg_reply;
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
> + .payload.object = {
> + .type = VHOST_SHARED_OBJECT_LOOKUP,
> + },
> + };
> +
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + pthread_mutex_lock(&dev->slave_mutex);
> + if (!vu_message_write(dev, dev->slave_fd, &msg)) {
> + goto out;
> + }
> +
> + if (!vu_message_read_default(dev, dev->slave_fd, &msg_reply)) {
> + goto out;
> + }
> +
> + if (msg_reply.request != msg.request) {
> + DPRINT("Received unexpected msg type. Expected %d, received %d",
> + msg.request, msg_reply.request);
> + goto out;
> + }
> +
> + *dmabuf_fd = msg_reply.payload.object.dmabuf_fd;
> + result = *dmabuf_fd > 0;
> +out:
> + pthread_mutex_unlock(&dev->slave_mutex);
> +
> + return result;
> +}
> +
> +static bool
> +vu_send_message(VuDev *dev, VhostUserMsg *vmsg)
> +{
> + bool result = false;
> + pthread_mutex_lock(&dev->slave_mutex);
> + if (!vu_message_write(dev, dev->slave_fd, vmsg)) {
> + goto out;
> + }
> +
> + result = true;
> +out:
> + pthread_mutex_unlock(&dev->slave_mutex);
> +
> + return result;
> +}
> +
> +bool
> +vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int dmabuf_fd)
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION,
> + .payload.object = {
> + .dmabuf_fd = dmabuf_fd,
> + .type = VHOST_SHARED_OBJECT_ADD,
> + },
> + };
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + return vu_send_message(dev, &msg);
> +}
> +
> +bool
> +vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> + .size = sizeof(msg.payload.object),
> + .flags = VHOST_USER_VERSION,
> + .payload.object = {
> + .type = VHOST_SHARED_OBJECT_REMOVE,
> + },
> + };
> + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> +
> + return vu_send_message(dev, &msg);
> +}
> +
> static bool
> vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
> {
> diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
> index 49208cceaa..a43d115bd7 100644
> --- a/subprojects/libvhost-user/libvhost-user.h
> +++ b/subprojects/libvhost-user/libvhost-user.h
> @@ -119,6 +119,7 @@ typedef enum VhostUserSlaveRequest {
> VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> VHOST_USER_BACKEND_VRING_CALL = 4,
> VHOST_USER_BACKEND_VRING_ERR = 5,
> + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> VHOST_USER_BACKEND_MAX
> } VhostUserSlaveRequest;
>
> @@ -172,6 +173,20 @@ typedef struct VhostUserInflight {
> uint16_t queue_size;
> } VhostUserInflight;
>
> +typedef enum VhostUserSharedType {
> + VHOST_SHARED_OBJECT_ADD = 0,
> + VHOST_SHARED_OBJECT_LOOKUP,
> + VHOST_SHARED_OBJECT_REMOVE,
> +} VhostUserSharedType;
> +
> +#define UUID_LEN 16
> +
> +typedef struct VhostUserShared {
> + unsigned char uuid[UUID_LEN];
> + VhostUserSharedType type;
> + int dmabuf_fd;
> +} VhostUserShared;
> +
> #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> # define VU_PACKED __attribute__((gcc_struct, packed))
> #else
> @@ -199,6 +214,7 @@ typedef struct VhostUserMsg {
> VhostUserConfig config;
> VhostUserVringArea area;
> VhostUserInflight inflight;
> + VhostUserShared object;
> } payload;
>
> int fds[VHOST_MEMORY_BASELINE_NREGIONS];
> @@ -539,6 +555,46 @@ void vu_set_queue_handler(VuDev *dev, VuVirtq *vq,
> bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
> int size, int offset);
>
> +/**
> + * vu_get_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + * @dmabuf_fd: output dma-buf file descriptor
> + *
> + * Lookup for a virtio shared object (i.e., dma-buf fd) associated with the
> + * received UUID. Result, if found, is stored in the dmabuf_fd argument.
> + *
> + * Returns: whether the virtio object was found.
> + */
> +bool vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> + int *dmabuf_fd);
> +
> +/**
> + * vu_add_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + * @dmabuf_fd: output dma-buf file descriptor
> + *
> + * Stores a new shared object (i.e., dma-buf fd) in the hash table, and
> + * associates it with the received UUID.
> + *
> + * Returns: TRUE on success, FALSE on failure.
> + */
> +bool vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> + int dmabuf_fd);
> +
> +/**
> + * vu_rm_shared_object:
> + * @dev: a VuDev context
> + * @uuid: UUID of the shared object
> + *
> + * Removes a shared object (i.e., dma-buf fd) associated with the
> + * received UUID from the hash table.
> + *
> + * Returns: TRUE on success, FALSE on failure.
> + */
> +bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
> +
> /**
> * vu_queue_set_notification:
> * @dev: a VuDev context
> --
> 2.40.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/4] vhost-user: add shared_object msg
2023-06-23 8:14 ` Michael S. Tsirkin
@ 2023-06-23 12:08 ` Albert Esteve
0 siblings, 0 replies; 19+ messages in thread
From: Albert Esteve @ 2023-06-23 12:08 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, kraxel, marcandre.lureau, cohuck, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 14253 bytes --]
On Fri, Jun 23, 2023 at 10:14 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, May 24, 2023 at 11:13:32AM +0200, Albert Esteve wrote:
> > Add new vhost-user protocol message
> > `VHOST_USER_BACKEND_SHARED_OBJECT`. This new
> > message is sent from vhost-user back-ends
> > to interact with the virtio-dmabuf table
> > in order to add, remove, or lookup for
> > virtio dma-buf shared objects.
> >
> > The action taken in the front-end depends
> > on the type stored in the payload struct.
> >
> > In the libvhost-user library add helper
> > functions to allow sending messages to
> > interact with the virtio shared objects
> > hash table.
> >
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> > docs/interop/vhost-user.rst | 15 ++++
> > hw/virtio/vhost-user.c | 68 ++++++++++++++++++
> > subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++++++++
> > subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++++++
> > 4 files changed, 227 insertions(+)
> >
> > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> > index 5a070adbc1..d3d8db41e5 100644
> > --- a/docs/interop/vhost-user.rst
> > +++ b/docs/interop/vhost-user.rst
> > @@ -1528,6 +1528,21 @@ is sent by the front-end.
> >
> > The state.num field is currently reserved and must be set to 0.
> >
> > +``VHOST_USER_BACKEND_SHARED_OBJECT``
> > + :id: 6
> > + :equivalent ioctl: N/A
> > + :request payload: ``struct VhostUserShared``
> > + :reply payload: ``struct VhostUserShared`` (only for ``LOOKUP``
> requests)
> > +
> > + Backends that need to interact with the virtio-dmabuf shared table API
> > + can send this message. The operation is determined by the ``type``
> member
> > + of the payload struct. The valid values for the operation type are
> > + ``VHOST_SHARED_OBJECT_*`` members, i.e., ``ADD``, ``LOOKUP``, and
> ``REMOVE``.
> > + ``LOOKUP`` operations require the ``VHOST_USER_NEED_REPLY_MASK`` flag
> to be
> > + set by the back-end, and the front-end will then send the dma-buf fd
> as
> > + a response if the UUID matches an object in the table, or a negative
> value
> > + otherwise.
> > +
> > .. _reply_ack:
>
> If you are reusing same message then I would maybe just make all types
> the same - if there's a reply bit backend can reply back with the same
> message - might be good for debugging etc.
>
> Also it's not detailed here but I am guessing LOOKUP at least
> does not send an fd and gets an fd back? Maybe LOOKUP
> at least actually should be a separate message.
>
Lookup sends a UUID and gets a fd back.
Is a good point, having consistent behaviour within
the same type (or message).
Maybe splitting all types into messages as Marc suggested is best after all.
I can do that and then add a feature bit to negotiate.
>
>
> > VHOST_USER_PROTOCOL_F_REPLY_ACK
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 74a2a28663..5ac5f0eafd 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -10,6 +10,7 @@
> >
> > #include "qemu/osdep.h"
> > #include "qapi/error.h"
> > +#include "hw/virtio/virtio-dmabuf.h"
> > #include "hw/virtio/vhost.h"
> > #include "hw/virtio/vhost-user.h"
> > #include "hw/virtio/vhost-backend.h"
> > @@ -20,6 +21,7 @@
> > #include "sysemu/kvm.h"
> > #include "qemu/error-report.h"
> > #include "qemu/main-loop.h"
> > +#include "qemu/uuid.h"
> > #include "qemu/sockets.h"
> > #include "sysemu/runstate.h"
> > #include "sysemu/cryptodev.h"
> > @@ -128,6 +130,7 @@ typedef enum VhostUserSlaveRequest {
> > VHOST_USER_BACKEND_IOTLB_MSG = 1,
> > VHOST_USER_BACKEND_CONFIG_CHANGE_MSG = 2,
> > VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> > + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> > VHOST_USER_BACKEND_MAX
> > } VhostUserSlaveRequest;
> >
> > @@ -190,6 +193,18 @@ typedef struct VhostUserInflight {
> > uint16_t queue_size;
> > } VhostUserInflight;
> >
> > +typedef enum VhostUserSharedType {
> > + VHOST_SHARED_OBJECT_ADD = 0,
> > + VHOST_SHARED_OBJECT_LOOKUP,
> > + VHOST_SHARED_OBJECT_REMOVE,
> > +} VhostUserSharedType;
> > +
> > +typedef struct VhostUserShared {
> > + unsigned char uuid[16];
> > + VhostUserSharedType type;
> > + int dmabuf_fd;
> > +} VhostUserShared;
>
> BTW all fields in structs that go on the wire should be fixed size.
>
You mean the fd? I may actually be doing it wrong now that I checked...
I think this probably should go into the `fds` field of the VhostUserMsg
struct.
Let me see if I can rework it a bit for the next drop, so that it does not
have
to be in the payload struct.
> > +
> > typedef struct {
> > VhostUserRequest request;
> >
> > @@ -214,6 +229,7 @@ typedef union {
> > VhostUserCryptoSession session;
> > VhostUserVringArea area;
> > VhostUserInflight inflight;
> > + VhostUserShared object;
> > } VhostUserPayload;
> >
> > typedef struct VhostUserMsg {
> > @@ -1582,6 +1598,52 @@ static int
> vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
> > return 0;
> > }
> >
> > +static int vhost_user_backend_handle_shared_object(VhostUserShared
> *object)
> > +{
> > + QemuUUID uuid;
> > + memcpy(uuid.data, object->uuid, sizeof(object->uuid));
> > +
> > + switch (object->type) {
> > + case VHOST_SHARED_OBJECT_ADD:
> > + return virtio_add_dmabuf(&uuid, object->dmabuf_fd);
> > + case VHOST_SHARED_OBJECT_LOOKUP:
> > + object->dmabuf_fd = virtio_lookup_dmabuf(&uuid);
> > + if (object->dmabuf_fd < 0) {
> > + return object->dmabuf_fd;
> > + }
> > + break;
> > + case VHOST_SHARED_OBJECT_REMOVE:
> > + return virtio_remove_resource(&uuid);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static bool
> > +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> > + VhostUserPayload *payload)
> > +{
> > + Error *local_err = NULL;
> > + struct iovec iov[2];
> > + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> > + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> > + hdr->flags |= VHOST_USER_REPLY_MASK;
> > +
> > + hdr->size = sizeof(payload->object);
> > +
> > + iov[0].iov_base = hdr;
> > + iov[0].iov_len = VHOST_USER_HDR_SIZE;
> > + iov[1].iov_base = payload;
> > + iov[1].iov_len = hdr->size;
> > +
> > + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov),
> &local_err)) {
> > + error_report_err(local_err);
> > + return false;
> > + }
> > + }
> > + return true;
> > +}
> > +
> > static void close_slave_channel(struct vhost_user *u)
> > {
> > g_source_destroy(u->slave_src);
> > @@ -1639,6 +1701,12 @@ static gboolean slave_read(QIOChannel *ioc,
> GIOCondition condition,
> > ret = vhost_user_slave_handle_vring_host_notifier(dev,
> &payload.area,
> > fd ? fd[0] :
> -1);
> > break;
> > + case VHOST_USER_BACKEND_SHARED_OBJECT:
> > + ret = vhost_user_backend_handle_shared_object(&payload.object);
> > + if (!vhost_user_backend_send_dmabuf_fd(ioc, &hdr, &payload)) {
> > + goto err;
> > + }
> > + break;
> > default:
> > error_report("Received unexpected msg type: %d.", hdr.request);
> > ret = -EINVAL;
> > diff --git a/subprojects/libvhost-user/libvhost-user.c
> b/subprojects/libvhost-user/libvhost-user.c
> > index 8fb61e2df2..27f16d292a 100644
> > --- a/subprojects/libvhost-user/libvhost-user.c
> > +++ b/subprojects/libvhost-user/libvhost-user.c
> > @@ -1403,6 +1403,94 @@ bool vu_set_queue_host_notifier(VuDev *dev,
> VuVirtq *vq, int fd,
> > return vu_process_message_reply(dev, &vmsg);
> > }
> >
> > +bool
> > +vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
> *dmabuf_fd)
> > +{
> > + bool result = false;
> > + VhostUserMsg msg_reply;
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
> > + .payload.object = {
> > + .type = VHOST_SHARED_OBJECT_LOOKUP,
> > + },
> > + };
> > +
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + pthread_mutex_lock(&dev->slave_mutex);
> > + if (!vu_message_write(dev, dev->slave_fd, &msg)) {
> > + goto out;
> > + }
> > +
> > + if (!vu_message_read_default(dev, dev->slave_fd, &msg_reply)) {
> > + goto out;
> > + }
> > +
> > + if (msg_reply.request != msg.request) {
> > + DPRINT("Received unexpected msg type. Expected %d, received %d",
> > + msg.request, msg_reply.request);
> > + goto out;
> > + }
> > +
> > + *dmabuf_fd = msg_reply.payload.object.dmabuf_fd;
> > + result = *dmabuf_fd > 0;
> > +out:
> > + pthread_mutex_unlock(&dev->slave_mutex);
> > +
> > + return result;
> > +}
> > +
> > +static bool
> > +vu_send_message(VuDev *dev, VhostUserMsg *vmsg)
> > +{
> > + bool result = false;
> > + pthread_mutex_lock(&dev->slave_mutex);
> > + if (!vu_message_write(dev, dev->slave_fd, vmsg)) {
> > + goto out;
> > + }
> > +
> > + result = true;
> > +out:
> > + pthread_mutex_unlock(&dev->slave_mutex);
> > +
> > + return result;
> > +}
> > +
> > +bool
> > +vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
> dmabuf_fd)
> > +{
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION,
> > + .payload.object = {
> > + .dmabuf_fd = dmabuf_fd,
> > + .type = VHOST_SHARED_OBJECT_ADD,
> > + },
> > + };
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + return vu_send_message(dev, &msg);
> > +}
> > +
> > +bool
> > +vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
> > +{
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION,
> > + .payload.object = {
> > + .type = VHOST_SHARED_OBJECT_REMOVE,
> > + },
> > + };
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + return vu_send_message(dev, &msg);
> > +}
> > +
> > static bool
> > vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
> > {
> > diff --git a/subprojects/libvhost-user/libvhost-user.h
> b/subprojects/libvhost-user/libvhost-user.h
> > index 49208cceaa..a43d115bd7 100644
> > --- a/subprojects/libvhost-user/libvhost-user.h
> > +++ b/subprojects/libvhost-user/libvhost-user.h
> > @@ -119,6 +119,7 @@ typedef enum VhostUserSlaveRequest {
> > VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> > VHOST_USER_BACKEND_VRING_CALL = 4,
> > VHOST_USER_BACKEND_VRING_ERR = 5,
> > + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> > VHOST_USER_BACKEND_MAX
> > } VhostUserSlaveRequest;
> >
> > @@ -172,6 +173,20 @@ typedef struct VhostUserInflight {
> > uint16_t queue_size;
> > } VhostUserInflight;
> >
> > +typedef enum VhostUserSharedType {
> > + VHOST_SHARED_OBJECT_ADD = 0,
> > + VHOST_SHARED_OBJECT_LOOKUP,
> > + VHOST_SHARED_OBJECT_REMOVE,
> > +} VhostUserSharedType;
> > +
> > +#define UUID_LEN 16
> > +
> > +typedef struct VhostUserShared {
> > + unsigned char uuid[UUID_LEN];
> > + VhostUserSharedType type;
> > + int dmabuf_fd;
> > +} VhostUserShared;
> > +
> > #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> > # define VU_PACKED __attribute__((gcc_struct, packed))
> > #else
> > @@ -199,6 +214,7 @@ typedef struct VhostUserMsg {
> > VhostUserConfig config;
> > VhostUserVringArea area;
> > VhostUserInflight inflight;
> > + VhostUserShared object;
> > } payload;
> >
> > int fds[VHOST_MEMORY_BASELINE_NREGIONS];
> > @@ -539,6 +555,46 @@ void vu_set_queue_handler(VuDev *dev, VuVirtq *vq,
> > bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
> > int size, int offset);
> >
> > +/**
> > + * vu_get_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + * @dmabuf_fd: output dma-buf file descriptor
> > + *
> > + * Lookup for a virtio shared object (i.e., dma-buf fd) associated with
> the
> > + * received UUID. Result, if found, is stored in the dmabuf_fd argument.
> > + *
> > + * Returns: whether the virtio object was found.
> > + */
> > +bool vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> > + int *dmabuf_fd);
> > +
> > +/**
> > + * vu_add_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + * @dmabuf_fd: output dma-buf file descriptor
> > + *
> > + * Stores a new shared object (i.e., dma-buf fd) in the hash table, and
> > + * associates it with the received UUID.
> > + *
> > + * Returns: TRUE on success, FALSE on failure.
> > + */
> > +bool vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> > + int dmabuf_fd);
> > +
> > +/**
> > + * vu_rm_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + *
> > + * Removes a shared object (i.e., dma-buf fd) associated with the
> > + * received UUID from the hash table.
> > + *
> > + * Returns: TRUE on success, FALSE on failure.
> > + */
> > +bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
> > +
> > /**
> > * vu_queue_set_notification:
> > * @dev: a VuDev context
> > --
> > 2.40.0
>
>
[-- Attachment #2: Type: text/html, Size: 17838 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/4] vhost-user: add shared_object msg
2023-06-23 6:45 ` Michael S. Tsirkin
2023-06-23 7:19 ` Albert Esteve
@ 2023-06-23 13:46 ` Albert Esteve
1 sibling, 0 replies; 19+ messages in thread
From: Albert Esteve @ 2023-06-23 13:46 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, kraxel, marcandre.lureau, cohuck, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 13708 bytes --]
On Fri, Jun 23, 2023 at 8:45 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, May 24, 2023 at 11:13:32AM +0200, Albert Esteve wrote:
> > Add new vhost-user protocol message
> > `VHOST_USER_BACKEND_SHARED_OBJECT`. This new
> > message is sent from vhost-user back-ends
> > to interact with the virtio-dmabuf table
> > in order to add, remove, or lookup for
> > virtio dma-buf shared objects.
> >
> > The action taken in the front-end depends
> > on the type stored in the payload struct.
> >
> > In the libvhost-user library add helper
> > functions to allow sending messages to
> > interact with the virtio shared objects
> > hash table.
> >
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> > docs/interop/vhost-user.rst | 15 ++++
> > hw/virtio/vhost-user.c | 68 ++++++++++++++++++
> > subprojects/libvhost-user/libvhost-user.c | 88 +++++++++++++++++++++++
> > subprojects/libvhost-user/libvhost-user.h | 56 +++++++++++++++
> > 4 files changed, 227 insertions(+)
> >
> > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> > index 5a070adbc1..d3d8db41e5 100644
> > --- a/docs/interop/vhost-user.rst
> > +++ b/docs/interop/vhost-user.rst
> > @@ -1528,6 +1528,21 @@ is sent by the front-end.
> >
> > The state.num field is currently reserved and must be set to 0.
> >
> > +``VHOST_USER_BACKEND_SHARED_OBJECT``
> > + :id: 6
> > + :equivalent ioctl: N/A
> > + :request payload: ``struct VhostUserShared``
> > + :reply payload: ``struct VhostUserShared`` (only for ``LOOKUP``
> requests)
> > +
> > + Backends that need to interact with the virtio-dmabuf shared table API
> > + can send this message. The operation is determined by the ``type``
> member
> > + of the payload struct. The valid values for the operation type are
> > + ``VHOST_SHARED_OBJECT_*`` members, i.e., ``ADD``, ``LOOKUP``, and
> ``REMOVE``.
> > + ``LOOKUP`` operations require the ``VHOST_USER_NEED_REPLY_MASK`` flag
> to be
> > + set by the back-end, and the front-end will then send the dma-buf fd
> as
> > + a response if the UUID matches an object in the table, or a negative
> value
> > + otherwise.
> > +
> > .. _reply_ack:
> >
> > VHOST_USER_PROTOCOL_F_REPLY_ACK
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 74a2a28663..5ac5f0eafd 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -10,6 +10,7 @@
> >
> > #include "qemu/osdep.h"
> > #include "qapi/error.h"
> > +#include "hw/virtio/virtio-dmabuf.h"
> > #include "hw/virtio/vhost.h"
> > #include "hw/virtio/vhost-user.h"
> > #include "hw/virtio/vhost-backend.h"
> > @@ -20,6 +21,7 @@
> > #include "sysemu/kvm.h"
> > #include "qemu/error-report.h"
> > #include "qemu/main-loop.h"
> > +#include "qemu/uuid.h"
> > #include "qemu/sockets.h"
> > #include "sysemu/runstate.h"
> > #include "sysemu/cryptodev.h"
> > @@ -128,6 +130,7 @@ typedef enum VhostUserSlaveRequest {
> > VHOST_USER_BACKEND_IOTLB_MSG = 1,
> > VHOST_USER_BACKEND_CONFIG_CHANGE_MSG = 2,
> > VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> > + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> > VHOST_USER_BACKEND_MAX
> > } VhostUserSlaveRequest;
> >
> > @@ -190,6 +193,18 @@ typedef struct VhostUserInflight {
> > uint16_t queue_size;
> > } VhostUserInflight;
> >
> > +typedef enum VhostUserSharedType {
> > + VHOST_SHARED_OBJECT_ADD = 0,
> > + VHOST_SHARED_OBJECT_LOOKUP,
> > + VHOST_SHARED_OBJECT_REMOVE,
> > +} VhostUserSharedType;
> > +
> > +typedef struct VhostUserShared {
> > + unsigned char uuid[16];
> > + VhostUserSharedType type;
> > + int dmabuf_fd;
> > +} VhostUserShared;
> > +
> > typedef struct {
> > VhostUserRequest request;
> >
> > @@ -214,6 +229,7 @@ typedef union {
> > VhostUserCryptoSession session;
> > VhostUserVringArea area;
> > VhostUserInflight inflight;
> > + VhostUserShared object;
> > } VhostUserPayload;
> >
> > typedef struct VhostUserMsg {
> > @@ -1582,6 +1598,52 @@ static int
> vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
> > return 0;
> > }
> >
> > +static int vhost_user_backend_handle_shared_object(VhostUserShared
> *object)
> > +{
> > + QemuUUID uuid;
>
> Can we initialize it here? uuid = { .data = object->uuid } ?
>
> Also, pls put space after variable declaration.
>
At this point, they are still just char arrays, so I need the memcpy.
But I will add the space after variable declaration.
>
> > + memcpy(uuid.data, object->uuid, sizeof(object->uuid));
> > +
> > + switch (object->type) {
> > + case VHOST_SHARED_OBJECT_ADD:
> > + return virtio_add_dmabuf(&uuid, object->dmabuf_fd);
> > + case VHOST_SHARED_OBJECT_LOOKUP:
> > + object->dmabuf_fd = virtio_lookup_dmabuf(&uuid);
> > + if (object->dmabuf_fd < 0) {
> > + return object->dmabuf_fd;
> > + }
> > + break;
> > + case VHOST_SHARED_OBJECT_REMOVE:
> > + return virtio_remove_resource(&uuid);
> > + }
> > +
>
> I couldn't figure out why, but if I commit this then run checkpatch,
> like this
> ./scripts/checkpatch.pl HEAD~1..HEAD
>
> then it is unhappy about the : in case. Any idea why?
>
> > + return 0;
> > +}
> > +
> > +static bool
> > +vhost_user_backend_send_dmabuf_fd(QIOChannel *ioc, VhostUserHeader *hdr,
> > + VhostUserPayload *payload)
> > +{
> > + Error *local_err = NULL;
> > + struct iovec iov[2];
> > + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> > + hdr->flags &= ~VHOST_USER_NEED_REPLY_MASK;
> > + hdr->flags |= VHOST_USER_REPLY_MASK;
> > +
> > + hdr->size = sizeof(payload->object);
> > +
> > + iov[0].iov_base = hdr;
> > + iov[0].iov_len = VHOST_USER_HDR_SIZE;
> > + iov[1].iov_base = payload;
> > + iov[1].iov_len = hdr->size;
> > +
> > + if (qio_channel_writev_all(ioc, iov, ARRAY_SIZE(iov),
> &local_err)) {
> > + error_report_err(local_err);
> > + return false;
> > + }
> > + }
> > + return true;
> > +}
> > +
> > static void close_slave_channel(struct vhost_user *u)
> > {
> > g_source_destroy(u->slave_src);
> > @@ -1639,6 +1701,12 @@ static gboolean slave_read(QIOChannel *ioc,
> GIOCondition condition,
> > ret = vhost_user_slave_handle_vring_host_notifier(dev,
> &payload.area,
> > fd ? fd[0] :
> -1);
> > break;
> > + case VHOST_USER_BACKEND_SHARED_OBJECT:
> > + ret = vhost_user_backend_handle_shared_object(&payload.object);
> > + if (!vhost_user_backend_send_dmabuf_fd(ioc, &hdr, &payload)) {
> > + goto err;
> > + }
> > + break;
> > default:
> > error_report("Received unexpected msg type: %d.", hdr.request);
> > ret = -EINVAL;
> > diff --git a/subprojects/libvhost-user/libvhost-user.c
> b/subprojects/libvhost-user/libvhost-user.c
> > index 8fb61e2df2..27f16d292a 100644
> > --- a/subprojects/libvhost-user/libvhost-user.c
> > +++ b/subprojects/libvhost-user/libvhost-user.c
> > @@ -1403,6 +1403,94 @@ bool vu_set_queue_host_notifier(VuDev *dev,
> VuVirtq *vq, int fd,
> > return vu_process_message_reply(dev, &vmsg);
> > }
> >
> > +bool
> > +vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
> *dmabuf_fd)
> > +{
> > + bool result = false;
> > + VhostUserMsg msg_reply;
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
> > + .payload.object = {
> > + .type = VHOST_SHARED_OBJECT_LOOKUP,
> > + },
> > + };
> > +
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + pthread_mutex_lock(&dev->slave_mutex);
> > + if (!vu_message_write(dev, dev->slave_fd, &msg)) {
> > + goto out;
> > + }
> > +
> > + if (!vu_message_read_default(dev, dev->slave_fd, &msg_reply)) {
> > + goto out;
> > + }
> > +
> > + if (msg_reply.request != msg.request) {
> > + DPRINT("Received unexpected msg type. Expected %d, received %d",
> > + msg.request, msg_reply.request);
> > + goto out;
> > + }
> > +
> > + *dmabuf_fd = msg_reply.payload.object.dmabuf_fd;
> > + result = *dmabuf_fd > 0;
> > +out:
> > + pthread_mutex_unlock(&dev->slave_mutex);
> > +
> > + return result;
> > +}
> > +
> > +static bool
> > +vu_send_message(VuDev *dev, VhostUserMsg *vmsg)
> > +{
> > + bool result = false;
> > + pthread_mutex_lock(&dev->slave_mutex);
> > + if (!vu_message_write(dev, dev->slave_fd, vmsg)) {
> > + goto out;
> > + }
> > +
> > + result = true;
> > +out:
> > + pthread_mutex_unlock(&dev->slave_mutex);
> > +
> > + return result;
> > +}
> > +
> > +bool
> > +vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN], int
> dmabuf_fd)
> > +{
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION,
> > + .payload.object = {
> > + .dmabuf_fd = dmabuf_fd,
> > + .type = VHOST_SHARED_OBJECT_ADD,
> > + },
> > + };
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + return vu_send_message(dev, &msg);
> > +}
> > +
> > +bool
> > +vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN])
> > +{
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_BACKEND_SHARED_OBJECT,
> > + .size = sizeof(msg.payload.object),
> > + .flags = VHOST_USER_VERSION,
> > + .payload.object = {
> > + .type = VHOST_SHARED_OBJECT_REMOVE,
> > + },
> > + };
> > + memcpy(msg.payload.object.uuid, uuid, sizeof(uuid[0]) * UUID_LEN);
> > +
> > + return vu_send_message(dev, &msg);
> > +}
> > +
> > static bool
> > vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
> > {
> > diff --git a/subprojects/libvhost-user/libvhost-user.h
> b/subprojects/libvhost-user/libvhost-user.h
> > index 49208cceaa..a43d115bd7 100644
> > --- a/subprojects/libvhost-user/libvhost-user.h
> > +++ b/subprojects/libvhost-user/libvhost-user.h
> > @@ -119,6 +119,7 @@ typedef enum VhostUserSlaveRequest {
> > VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG = 3,
> > VHOST_USER_BACKEND_VRING_CALL = 4,
> > VHOST_USER_BACKEND_VRING_ERR = 5,
> > + VHOST_USER_BACKEND_SHARED_OBJECT = 6,
> > VHOST_USER_BACKEND_MAX
> > } VhostUserSlaveRequest;
> >
> > @@ -172,6 +173,20 @@ typedef struct VhostUserInflight {
> > uint16_t queue_size;
> > } VhostUserInflight;
> >
> > +typedef enum VhostUserSharedType {
> > + VHOST_SHARED_OBJECT_ADD = 0,
> > + VHOST_SHARED_OBJECT_LOOKUP,
> > + VHOST_SHARED_OBJECT_REMOVE,
> > +} VhostUserSharedType;
> > +
> > +#define UUID_LEN 16
> > +
> > +typedef struct VhostUserShared {
> > + unsigned char uuid[UUID_LEN];
> > + VhostUserSharedType type;
> > + int dmabuf_fd;
> > +} VhostUserShared;
> > +
> > #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
> > # define VU_PACKED __attribute__((gcc_struct, packed))
> > #else
> > @@ -199,6 +214,7 @@ typedef struct VhostUserMsg {
> > VhostUserConfig config;
> > VhostUserVringArea area;
> > VhostUserInflight inflight;
> > + VhostUserShared object;
> > } payload;
> >
> > int fds[VHOST_MEMORY_BASELINE_NREGIONS];
> > @@ -539,6 +555,46 @@ void vu_set_queue_handler(VuDev *dev, VuVirtq *vq,
> > bool vu_set_queue_host_notifier(VuDev *dev, VuVirtq *vq, int fd,
> > int size, int offset);
> >
> > +/**
> > + * vu_get_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + * @dmabuf_fd: output dma-buf file descriptor
> > + *
> > + * Lookup for a virtio shared object (i.e., dma-buf fd) associated with
> the
> > + * received UUID. Result, if found, is stored in the dmabuf_fd argument.
> > + *
> > + * Returns: whether the virtio object was found.
> > + */
> > +bool vu_get_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> > + int *dmabuf_fd);
> > +
> > +/**
> > + * vu_add_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + * @dmabuf_fd: output dma-buf file descriptor
> > + *
> > + * Stores a new shared object (i.e., dma-buf fd) in the hash table, and
> > + * associates it with the received UUID.
> > + *
> > + * Returns: TRUE on success, FALSE on failure.
> > + */
> > +bool vu_add_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN],
> > + int dmabuf_fd);
> > +
> > +/**
> > + * vu_rm_shared_object:
> > + * @dev: a VuDev context
> > + * @uuid: UUID of the shared object
> > + *
> > + * Removes a shared object (i.e., dma-buf fd) associated with the
> > + * received UUID from the hash table.
> > + *
> > + * Returns: TRUE on success, FALSE on failure.
> > + */
> > +bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
> > +
> > /**
> > * vu_queue_set_notification:
> > * @dev: a VuDev context
> > --
> > 2.40.0
>
>
[-- Attachment #2: Type: text/html, Size: 17257 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2023-06-23 13:47 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-24 9:13 [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
2023-05-24 9:13 ` [PATCH v3 1/4] uuid: add hash_func and equal_func Albert Esteve
2023-05-24 9:13 ` [PATCH v3 2/4] virtio-dmabuf: introduce virtio-dmabuf Albert Esteve
2023-05-24 9:13 ` [PATCH v3 3/4] vhost-user: add shared_object msg Albert Esteve
2023-06-22 19:57 ` Michael S. Tsirkin
2023-06-22 20:18 ` Marc-André Lureau
2023-06-23 7:45 ` Albert Esteve
2023-06-23 6:45 ` Michael S. Tsirkin
2023-06-23 7:19 ` Albert Esteve
2023-06-23 7:32 ` Michael S. Tsirkin
2023-06-23 13:46 ` Albert Esteve
2023-06-23 8:14 ` Michael S. Tsirkin
2023-06-23 12:08 ` Albert Esteve
2023-05-24 9:13 ` [PATCH v3 4/4] vhost-user: refactor send_resp code Albert Esteve
2023-06-23 6:48 ` Michael S. Tsirkin
2023-06-21 8:20 ` [PATCH v3 0/4] Virtio shared dma-buf Albert Esteve
2023-06-21 20:25 ` Michael S. Tsirkin
2023-06-22 19:53 ` Michael S. Tsirkin
2023-06-23 6:49 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).