1. Patchew
  2. QEMU
  3. [PATCH v4 0/5] Virtio dmabuf improvements
  4. View patch

[PATCH v4 2/5] hw/virtio: document SharedObject structures

Albert Esteve posted 5 patches 8 months, 4 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Albert Esteve <aesteve@redhat.com>
[PATCH v4 2/5] hw/virtio: document SharedObject structures
Posted by Albert Esteve 8 months, 4 weeks ago 
Change VirtioSharedObject value type from
a generic pointer to a union storing the different
supported underlying types, which makes naming
less confusing.

With the update, use the chance to add kdoc
to both the SharedObjectType enum and
VirtioSharedObject struct.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 hw/display/virtio-dmabuf.c        |  8 ++++----
 include/hw/virtio/virtio-dmabuf.h | 25 ++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/hw/display/virtio-dmabuf.c b/hw/display/virtio-dmabuf.c
index 3dba4577ca..497cb6fa7c 100644
--- a/hw/display/virtio-dmabuf.c
+++ b/hw/display/virtio-dmabuf.c
@@ -57,7 +57,7 @@ bool virtio_add_dmabuf(QemuUUID *uuid, int udmabuf_fd)
     }
     vso = g_new(VirtioSharedObject, 1);
     vso->type = TYPE_DMABUF;
-    vso->value = GINT_TO_POINTER(udmabuf_fd);
+    vso->value.udma_buf = udmabuf_fd;
     result = virtio_add_resource(uuid, vso);
     if (!result) {
         g_free(vso);
@@ -75,7 +75,7 @@ bool virtio_add_vhost_device(QemuUUID *uuid, struct vhost_dev *dev)
     }
     vso = g_new(VirtioSharedObject, 1);
     vso->type = TYPE_VHOST_DEV;
-    vso->value = dev;
+    vso->value.dev = dev;
     result = virtio_add_resource(uuid, vso);
     if (!result) {
         g_free(vso);
@@ -114,7 +114,7 @@ int virtio_lookup_dmabuf(const QemuUUID *uuid)
         return -1;
     }
     assert(vso->type == TYPE_DMABUF);
-    return GPOINTER_TO_INT(vso->value);
+    return vso->value.udma_buf;
 }
 
 struct vhost_dev *virtio_lookup_vhost_device(const QemuUUID *uuid)
@@ -124,7 +124,7 @@ struct vhost_dev *virtio_lookup_vhost_device(const QemuUUID *uuid)
         return NULL;
     }
     assert(vso->type == TYPE_VHOST_DEV);
-    return (struct vhost_dev *) vso->value;
+    return (struct vhost_dev *) vso->value.dev;
 }
 
 SharedObjectType virtio_object_type(const QemuUUID *uuid)
diff --git a/include/hw/virtio/virtio-dmabuf.h b/include/hw/virtio/virtio-dmabuf.h
index 627c3b6db7..891a43162d 100644
--- a/include/hw/virtio/virtio-dmabuf.h
+++ b/include/hw/virtio/virtio-dmabuf.h
@@ -16,15 +16,38 @@
 #include "qemu/uuid.h"
 #include "vhost.h"
 
+/**
+ * SharedObjectType:
+ * 
+ * Identifies the type of the underlying type that the current lookup
+ * table entry is holding.
+ * 
+ * TYPE_INVALID: Invalid entry
+ * TYPE_DMABUF: Entry is a dmabuf file descriptor that can be directly
+ *              shared with the requestor
+ * TYPE_VHOST_DEV: Entry is a pointer to a vhost device that is holding
+ *                 the shared object.
+ */
 typedef enum SharedObjectType {
     TYPE_INVALID = 0,
     TYPE_DMABUF,
     TYPE_VHOST_DEV,
 } SharedObjectType;
 
+/**
+ * VirtioSharedObject:
+ * @type: Shared object type identifier
+ * @value: Union containing to the underlying type
+ * 
+ * The VirtioSharedObject object provides a way to distinguish,
+ * store, and handle the different types supported by the lookup table.
+ */
 typedef struct VirtioSharedObject {
     SharedObjectType type;
-    gpointer value;
+    union {
+        struct vhost_dev *dev;  /* TYPE_VHOST_DEV */
+        int udma_buf;           /* TYPE_DMABUF */
+    } value;
 } VirtioSharedObject;
 
 /**
-- 
2.43.1
Re: [PATCH v4 2/5] hw/virtio: document SharedObject structures
Posted by Manos Pitsidianakis 8 months, 3 weeks ago 
On Mon, 19 Feb 2024 16:34, Albert Esteve <aesteve@redhat.com> wrote:
>Change VirtioSharedObject value type from
>a generic pointer to a union storing the different
>supported underlying types, which makes naming
>less confusing.
>
>With the update, use the chance to add kdoc
>to both the SharedObjectType enum and
>VirtioSharedObject struct.
>
>Signed-off-by: Albert Esteve <aesteve@redhat.com>
>---
> hw/display/virtio-dmabuf.c        |  8 ++++----
> include/hw/virtio/virtio-dmabuf.h | 25 ++++++++++++++++++++++++-
> 2 files changed, 28 insertions(+), 5 deletions(-)
>
>diff --git a/hw/display/virtio-dmabuf.c b/hw/display/virtio-dmabuf.c
>index 3dba4577ca..497cb6fa7c 100644
>--- a/hw/display/virtio-dmabuf.c
>+++ b/hw/display/virtio-dmabuf.c
>@@ -57,7 +57,7 @@ bool virtio_add_dmabuf(QemuUUID *uuid, int udmabuf_fd)
>     }
>     vso = g_new(VirtioSharedObject, 1);
>     vso->type = TYPE_DMABUF;
>-    vso->value = GINT_TO_POINTER(udmabuf_fd);
>+    vso->value.udma_buf = udmabuf_fd;
>     result = virtio_add_resource(uuid, vso);
>     if (!result) {
>         g_free(vso);
>@@ -75,7 +75,7 @@ bool virtio_add_vhost_device(QemuUUID *uuid, struct vhost_dev *dev)
>     }
>     vso = g_new(VirtioSharedObject, 1);
>     vso->type = TYPE_VHOST_DEV;
>-    vso->value = dev;
>+    vso->value.dev = dev;
>     result = virtio_add_resource(uuid, vso);
>     if (!result) {
>         g_free(vso);
>@@ -114,7 +114,7 @@ int virtio_lookup_dmabuf(const QemuUUID *uuid)
>         return -1;
>     }
>     assert(vso->type == TYPE_DMABUF);
>-    return GPOINTER_TO_INT(vso->value);
>+    return vso->value.udma_buf;
> }
> 
> struct vhost_dev *virtio_lookup_vhost_device(const QemuUUID *uuid)
>@@ -124,7 +124,7 @@ struct vhost_dev *virtio_lookup_vhost_device(const QemuUUID *uuid)
>         return NULL;
>     }
>     assert(vso->type == TYPE_VHOST_DEV);
>-    return (struct vhost_dev *) vso->value;
>+    return (struct vhost_dev *) vso->value.dev;

Is the casting still required?


> }
> 
> SharedObjectType virtio_object_type(const QemuUUID *uuid)
>diff --git a/include/hw/virtio/virtio-dmabuf.h b/include/hw/virtio/virtio-dmabuf.h
>index 627c3b6db7..891a43162d 100644
>--- a/include/hw/virtio/virtio-dmabuf.h
>+++ b/include/hw/virtio/virtio-dmabuf.h
>@@ -16,15 +16,38 @@
> #include "qemu/uuid.h"
> #include "vhost.h"
> 
>+/**
>+ * SharedObjectType:
>+ *
>+ * Identifies the type of the underlying type that the current lookup
>+ * table entry is holding.
>+ * 
>+ * TYPE_INVALID: Invalid entry
>+ * TYPE_DMABUF: Entry is a dmabuf file descriptor that can be directly
>+ *              shared with the requestor
>+ * TYPE_VHOST_DEV: Entry is a pointer to a vhost device that is holding
>+ *                 the shared object.


nit:

+ * TYPE_INVALID:   Invalid entry.
+ * TYPE_DMABUF:    Entry is a dmabuf file descriptor that can be
+ *                 directly shared with the requestor.
+ * TYPE_VHOST_DEV: Entry is a pointer to a vhost device that is holding
+ *                 the shared object.


>+ */
> typedef enum SharedObjectType {
>     TYPE_INVALID = 0,
>     TYPE_DMABUF,
>     TYPE_VHOST_DEV,
> } SharedObjectType;
> 
>+/**
>+ * VirtioSharedObject:
>+ * @type: Shared object type identifier
>+ * @value: Union containing to the underlying type
>+ * 
>+ * The VirtioSharedObject object provides a way to distinguish,
>+ * store, and handle the different types supported by the lookup table.
>+ */
> typedef struct VirtioSharedObject {
>     SharedObjectType type;
>-    gpointer value;
>+    union {
>+        struct vhost_dev *dev;  /* TYPE_VHOST_DEV */
>+        int udma_buf;           /* TYPE_DMABUF */
>+    } value;
> } VirtioSharedObject;
> 
> /**
>-- 
>2.43.1
>
>
Re: [PATCH v4 2/5] hw/virtio: document SharedObject structures
Posted by Albert Esteve 8 months, 1 week ago 
On Tue, Feb 20, 2024 at 11:34 AM Manos Pitsidianakis <
manos.pitsidianakis@linaro.org> wrote:

> On Mon, 19 Feb 2024 16:34, Albert Esteve <aesteve@redhat.com> wrote:
> >Change VirtioSharedObject value type from
> >a generic pointer to a union storing the different
> >supported underlying types, which makes naming
> >less confusing.
> >
> >With the update, use the chance to add kdoc
> >to both the SharedObjectType enum and
> >VirtioSharedObject struct.
> >
> >Signed-off-by: Albert Esteve <aesteve@redhat.com>
> >---
> > hw/display/virtio-dmabuf.c        |  8 ++++----
> > include/hw/virtio/virtio-dmabuf.h | 25 ++++++++++++++++++++++++-
> > 2 files changed, 28 insertions(+), 5 deletions(-)
> >
> >diff --git a/hw/display/virtio-dmabuf.c b/hw/display/virtio-dmabuf.c
> >index 3dba4577ca..497cb6fa7c 100644
> >--- a/hw/display/virtio-dmabuf.c
> >+++ b/hw/display/virtio-dmabuf.c
> >@@ -57,7 +57,7 @@ bool virtio_add_dmabuf(QemuUUID *uuid, int udmabuf_fd)
> >     }
> >     vso = g_new(VirtioSharedObject, 1);
> >     vso->type = TYPE_DMABUF;
> >-    vso->value = GINT_TO_POINTER(udmabuf_fd);
> >+    vso->value.udma_buf = udmabuf_fd;
> >     result = virtio_add_resource(uuid, vso);
> >     if (!result) {
> >         g_free(vso);
> >@@ -75,7 +75,7 @@ bool virtio_add_vhost_device(QemuUUID *uuid, struct
> vhost_dev *dev)
> >     }
> >     vso = g_new(VirtioSharedObject, 1);
> >     vso->type = TYPE_VHOST_DEV;
> >-    vso->value = dev;
> >+    vso->value.dev = dev;
> >     result = virtio_add_resource(uuid, vso);
> >     if (!result) {
> >         g_free(vso);
> >@@ -114,7 +114,7 @@ int virtio_lookup_dmabuf(const QemuUUID *uuid)
> >         return -1;
> >     }
> >     assert(vso->type == TYPE_DMABUF);
> >-    return GPOINTER_TO_INT(vso->value);
> >+    return vso->value.udma_buf;
> > }
> >
> > struct vhost_dev *virtio_lookup_vhost_device(const QemuUUID *uuid)
> >@@ -124,7 +124,7 @@ struct vhost_dev *virtio_lookup_vhost_device(const
> QemuUUID *uuid)
> >         return NULL;
> >     }
> >     assert(vso->type == TYPE_VHOST_DEV);
> >-    return (struct vhost_dev *) vso->value;
> >+    return (struct vhost_dev *) vso->value.dev;
>
> Is the casting still required?
>
> You're right, probably not anymore. I'll remove it.


>
> > }
> >
> > SharedObjectType virtio_object_type(const QemuUUID *uuid)
> >diff --git a/include/hw/virtio/virtio-dmabuf.h
> b/include/hw/virtio/virtio-dmabuf.h
> >index 627c3b6db7..891a43162d 100644
> >--- a/include/hw/virtio/virtio-dmabuf.h
> >+++ b/include/hw/virtio/virtio-dmabuf.h
> >@@ -16,15 +16,38 @@
> > #include "qemu/uuid.h"
> > #include "vhost.h"
> >
> >+/**
> >+ * SharedObjectType:
> >+ *
> >+ * Identifies the type of the underlying type that the current lookup
> >+ * table entry is holding.
> >+ *
> >+ * TYPE_INVALID: Invalid entry
> >+ * TYPE_DMABUF: Entry is a dmabuf file descriptor that can be directly
> >+ *              shared with the requestor
> >+ * TYPE_VHOST_DEV: Entry is a pointer to a vhost device that is holding
> >+ *                 the shared object.
>
>
> nit:
>
> + * TYPE_INVALID:   Invalid entry.
> + * TYPE_DMABUF:    Entry is a dmabuf file descriptor that can be
> + *                 directly shared with the requestor.
> + * TYPE_VHOST_DEV: Entry is a pointer to a vhost device that is holding
> + *                 the shared object.
>
>
> >+ */
> > typedef enum SharedObjectType {
> >     TYPE_INVALID = 0,
> >     TYPE_DMABUF,
> >     TYPE_VHOST_DEV,
> > } SharedObjectType;
> >
> >+/**
> >+ * VirtioSharedObject:
> >+ * @type: Shared object type identifier
> >+ * @value: Union containing to the underlying type
> >+ *
> >+ * The VirtioSharedObject object provides a way to distinguish,
> >+ * store, and handle the different types supported by the lookup table.
> >+ */
> > typedef struct VirtioSharedObject {
> >     SharedObjectType type;
> >-    gpointer value;
> >+    union {
> >+        struct vhost_dev *dev;  /* TYPE_VHOST_DEV */
> >+        int udma_buf;           /* TYPE_DMABUF */
> >+    } value;
> > } VirtioSharedObject;
> >
> > /**
> >--
> >2.43.1
> >
> >
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.