On Tue, Feb 20, 2024 at 11:34 AM Manos Pitsidianakis <
manos.pitsidiana...@linaro.org> wrote:
> On Mon, 19 Feb 2024 16:34, Albert Esteve <aest...@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 <aest...@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
> >
> >
>
>
