Re: [PATCH v4 2/5] hw/virtio: document SharedObject structures
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 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
> >---
> > 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
> >
> >
>
>
Re: [PATCH v4 2/5] hw/virtio: document SharedObject structures
On Mon, 19 Feb 2024 16:34, Albert Esteve 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
---
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
[PATCH v4 2/5] hw/virtio: document SharedObject structures
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
---
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
