<snip>
+ create = g_malloc0(sizeof(*create) +
+ res->iov_cnt * sizeof (struct udmabuf_create_item));
+ if (!create)
+ return NULL;
Pointless allocation check (g_malloc will abort if it failed to allocate)
[Kasireddy, Vivek] Ok, will remove it in v2.
+
+ create->count = res->iov_cnt;
+ create->flags = UDMABUF_FLAGS_CLOEXEC;
+ for (i = 0; i < res->iov_cnt; i++) {
+ rb = qemu_ram_block_from_host(res->iov[i].iov_base, false, &offset);
+ if (!rb || rb->fd < 0) {
+ g_free(create);
+ return NULL;
+ }
+
+ create->list[i].memfd = rb->fd;
+ create->list[i].__pad = 0;
+ create->list[i].offset = offset;
+ create->list[i].size = res->iov[i].iov_len;
+ }
+
+ dmabuf_fd = ioctl(udmabuf_fd, UDMABUF_CREATE_LIST, create);
+ if (dmabuf_fd < 0) {
It would be useful to print the error with errno.
[Kasireddy, Vivek] Sure.
+ g_free(create);
(we now like new code to use g_auto)
+ return NULL;
+ }
+
+ /* FIXME: We should get the modifier and format info with blob resources */
+ modifier_hi = fourcc_mod_code(INTEL, I915_FORMAT_MOD_X_TILED) >> 32;
+ modifier_lo = fourcc_mod_code(INTEL,I915_FORMAT_MOD_X_TILED) & 0xFFFFFFFF;
I have no idea if this format is going to work with various drivers and matches the underlying memory representation.
[Kasireddy, Vivek] No, it won’t work with other drivers. The modifier information should
be obtained from the Guest that allocates the buffers. I just added it as a temporary hack
for testing with a Intel GPU.
+ modifier = ((uint64_t)modifier_hi << 32) | modifier_lo;
+
+ dmabuf = g_new0(VGPUDMABuf, 1);
+ dmabuf->dmabuf_id = ids++;
+ dmabuf->buf.width = res->width;
+ dmabuf->buf.height = res->height;
+ dmabuf->buf.stride = pixman_image_get_stride(res->image);
+ dmabuf->buf.fourcc = DRM_FORMAT_XRGB8888;
+ dmabuf->buf.modifier = modifier;
+ dmabuf->buf.fd = dmabuf_fd;
+
+ QTAILQ_INSERT_HEAD(&g->dmabuf.bufs, dmabuf, next);
+ g_free(create);
+
+ return dmabuf;
+}
+ udmabuf_fd = open("/dev/udmabuf", O_RDWR);
If udmabuf_fd is already opened, this will leak fds.
[Kasireddy, Vivek] Yup, will take care of it in v2.
+ if (udmabuf_fd < 0) {
+ error_setg_errno(errp, errno,
+ "udmabuf: failed to open vhost device");
+ return;
The fallback path should keep working
[Kasireddy, Vivek] Makes sense; will fix it in v2.
It would be worth doing a similar change in vhost-user-gpu.
[Kasireddy, Vivek] Sure, will look into it after understanding the deltas between
vhost-user and the in-qemu implementations.
Thanks,
Vivek