[PATCH v8 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum

Vivek Kasireddy posted 10 patches 2 days, 10 hours ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>
[PATCH v8 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum
Posted by Vivek Kasireddy 2 days, 10 hours ago
Make the error handling more robust in virtio_gpu_init_udmabuf()
by introducing 'Error **' parameter to capture errors and using an
enum from VFIO to categorize Guest and Host errors. This allows
for better error reporting and handling of errors from
virtio_gpu_create_udmabuf() and virtio_gpu_remap_dmabuf().

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Alex Williamson <alex@shazbot.org>
Cc: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu-dmabuf.c | 61 ++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
index 8d67ef7c2a..b2badfc1bd 100644
--- a/hw/display/virtio-gpu-dmabuf.c
+++ b/hw/display/virtio-gpu-dmabuf.c
@@ -18,6 +18,7 @@
 #include "ui/console.h"
 #include "hw/virtio/virtio-gpu.h"
 #include "hw/virtio/virtio-gpu-pixman.h"
+#include "hw/vfio/vfio-device.h"
 #include "trace.h"
 #include "system/ramblock.h"
 #include "system/hostmem.h"
@@ -27,16 +28,18 @@
 #include "standard-headers/linux/udmabuf.h"
 #include "standard-headers/drm/drm_fourcc.h"
 
-static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
+static int virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res,
+                                     Error **errp)
 {
     g_autofree struct udmabuf_create_list *list = NULL;
     RAMBlock *rb;
     ram_addr_t offset;
-    int udmabuf, i;
+    int udmabuf, i, fd;
 
     udmabuf = udmabuf_fd();
     if (udmabuf < 0) {
-        return;
+        error_setg(errp, "udmabuf device not available or enabled");
+        return VFIO_DMABUF_CREATE_HOST_ERROR;
     }
 
     list = g_malloc0(sizeof(struct udmabuf_create_list) +
@@ -45,7 +48,8 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
     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) {
-            return;
+            error_setg(errp, "IOV memory address incompatible with udmabuf ");
+            return VFIO_DMABUF_CREATE_GUEST_ERROR;
         }
 
         list->list[i].memfd  = rb->fd;
@@ -56,22 +60,30 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
     list->count = res->iov_cnt;
     list->flags = UDMABUF_FLAGS_CLOEXEC;
 
-    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
-    if (res->dmabuf_fd < 0) {
-        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
-                    strerror(errno));
+    fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
+    if (fd < 0) {
+        error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl failed");
+        if (errno == EINVAL || errno == EBADFD) {
+            return VFIO_DMABUF_CREATE_GUEST_ERROR;
+        }
+        return VFIO_DMABUF_CREATE_HOST_ERROR;
     }
+    return fd;
 }
 
-static void virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res)
+static bool virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res,
+                                    Error **errp)
 {
-    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
-                         MAP_SHARED, res->dmabuf_fd, 0);
-    if (res->remapped == MAP_FAILED) {
-        warn_report("%s: dmabuf mmap failed: %s", __func__,
-                    strerror(errno));
+    void *map;
+
+    map = mmap(NULL, res->blob_size, PROT_READ, MAP_SHARED, res->dmabuf_fd, 0);
+    if (map == MAP_FAILED) {
+        error_setg_errno(errp, errno, "dmabuf mmap failed");
         res->remapped = NULL;
+        return false;
     }
+    res->remapped = map;
+    return true;
 }
 
 static void virtio_gpu_destroy_dmabuf(struct virtio_gpu_simple_resource *res)
@@ -125,19 +137,32 @@ bool virtio_gpu_have_udmabuf(void)
 
 void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
 {
+    Error *local_err = NULL;
     void *pdata = NULL;
+    int ret;
 
     res->dmabuf_fd = -1;
     if (res->iov_cnt == 1 &&
         res->iov[0].iov_len < 4096) {
         pdata = res->iov[0].iov_base;
     } else {
-        virtio_gpu_create_udmabuf(res);
-        if (res->dmabuf_fd < 0) {
+        ret = virtio_gpu_create_udmabuf(res, &local_err);
+        if (ret < 0) {
+            error_prepend(&local_err, "Cannot create dmabuf: ");
+
+            if (ret == VFIO_DMABUF_CREATE_GUEST_ERROR) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "Cannot create dmabuf: incompatible memory\n");
+                error_free(local_err);
+                return;
+            }
+            error_report_err(local_err);
             return;
         }
-        virtio_gpu_remap_dmabuf(res);
-        if (!res->remapped) {
+
+        res->dmabuf_fd = ret;
+        if (!virtio_gpu_remap_dmabuf(res, &local_err)) {
+            error_report_err(local_err);
             return;
         }
         pdata = res->remapped;
-- 
2.53.0


Re: [PATCH v8 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum
Posted by Akihiko Odaki 2 days, 1 hour ago
On 2026/02/28 7:13, Vivek Kasireddy wrote:
> Make the error handling more robust in virtio_gpu_init_udmabuf()
> by introducing 'Error **' parameter to capture errors and using an
> enum from VFIO to categorize Guest and Host errors. This allows
> for better error reporting and handling of errors from
> virtio_gpu_create_udmabuf() and virtio_gpu_remap_dmabuf().
> 
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Cc: Alex Williamson <alex@shazbot.org>
> Cc: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   hw/display/virtio-gpu-dmabuf.c | 61 ++++++++++++++++++++++++----------
>   1 file changed, 43 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-dmabuf.c
> index 8d67ef7c2a..b2badfc1bd 100644
> --- a/hw/display/virtio-gpu-dmabuf.c
> +++ b/hw/display/virtio-gpu-dmabuf.c
> @@ -18,6 +18,7 @@
>   #include "ui/console.h"
>   #include "hw/virtio/virtio-gpu.h"
>   #include "hw/virtio/virtio-gpu-pixman.h"
> +#include "hw/vfio/vfio-device.h"
>   #include "trace.h"
>   #include "system/ramblock.h"
>   #include "system/hostmem.h"
> @@ -27,16 +28,18 @@
>   #include "standard-headers/linux/udmabuf.h"
>   #include "standard-headers/drm/drm_fourcc.h"
>   
> -static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
> +static int virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res,
> +                                     Error **errp)
>   {
>       g_autofree struct udmabuf_create_list *list = NULL;
>       RAMBlock *rb;
>       ram_addr_t offset;
> -    int udmabuf, i;
> +    int udmabuf, i, fd;
>   
>       udmabuf = udmabuf_fd();
>       if (udmabuf < 0) {
> -        return;
> +        error_setg(errp, "udmabuf device not available or enabled");
> +        return VFIO_DMABUF_CREATE_HOST_ERROR;

I failed to point out one thing for naming of the enum. The name should 
express if the error is caused by invalid iov or something else. Then 
virtio-gpu decides how to recover the error; it's virtio-gpu who knows 
the iov is from the guest and decides to report a guest error, not vfio. 
Otherwise, it reports a host error.

>       }
>   
>       list = g_malloc0(sizeof(struct udmabuf_create_list) +
> @@ -45,7 +48,8 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
>       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) {
> -            return;
> +            error_setg(errp, "IOV memory address incompatible with udmabuf ");
> +            return VFIO_DMABUF_CREATE_GUEST_ERROR;
>           }
>   
>           list->list[i].memfd  = rb->fd;
> @@ -56,22 +60,30 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
>       list->count = res->iov_cnt;
>       list->flags = UDMABUF_FLAGS_CLOEXEC;
>   
> -    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> -    if (res->dmabuf_fd < 0) {
> -        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
> -                    strerror(errno));
> +    fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> +    if (fd < 0) {
> +        error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl failed");
> +        if (errno == EINVAL || errno == EBADFD) {
> +            return VFIO_DMABUF_CREATE_GUEST_ERROR;
> +        }
> +        return VFIO_DMABUF_CREATE_HOST_ERROR;
>       }
> +    return fd;
>   }
>   
> -static void virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res)
> +static bool virtio_gpu_remap_dmabuf(struct virtio_gpu_simple_resource *res,
> +                                    Error **errp)
>   {
> -    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
> -                         MAP_SHARED, res->dmabuf_fd, 0);
> -    if (res->remapped == MAP_FAILED) {
> -        warn_report("%s: dmabuf mmap failed: %s", __func__,
> -                    strerror(errno));
> +    void *map;
> +
> +    map = mmap(NULL, res->blob_size, PROT_READ, MAP_SHARED, res->dmabuf_fd, 0);
> +    if (map == MAP_FAILED) {
> +        error_setg_errno(errp, errno, "dmabuf mmap failed");
>           res->remapped = NULL;
> +        return false;
>       }
> +    res->remapped = map;
> +    return true;
>   }
>   
>   static void virtio_gpu_destroy_dmabuf(struct virtio_gpu_simple_resource *res)
> @@ -125,19 +137,32 @@ bool virtio_gpu_have_udmabuf(void)
>   
>   void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
>   {
> +    Error *local_err = NULL;
>       void *pdata = NULL;
> +    int ret;
>   
>       res->dmabuf_fd = -1;
>       if (res->iov_cnt == 1 &&
>           res->iov[0].iov_len < 4096) {
>           pdata = res->iov[0].iov_base;
>       } else {
> -        virtio_gpu_create_udmabuf(res);
> -        if (res->dmabuf_fd < 0) {
> +        ret = virtio_gpu_create_udmabuf(res, &local_err);
> +        if (ret < 0) {
> +            error_prepend(&local_err, "Cannot create dmabuf: ");

This error_prepend() is redundant. The error messages 
virtio_gpu_create_udmabuf() emits already suggests they are problems 
with DMA-BUF.

Regards,
Akihiko Odaki

> +
> +            if (ret == VFIO_DMABUF_CREATE_GUEST_ERROR) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "Cannot create dmabuf: incompatible memory\n");
> +                error_free(local_err);
> +                return;
> +            }
> +            error_report_err(local_err);
>               return;
>           }
> -        virtio_gpu_remap_dmabuf(res);
> -        if (!res->remapped) {
> +
> +        res->dmabuf_fd = ret;
> +        if (!virtio_gpu_remap_dmabuf(res, &local_err)) {
> +            error_report_err(local_err);
>               return;
>           }
>           pdata = res->remapped;


RE: [PATCH v8 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum
Posted by Kasireddy, Vivek 2 hours ago
Hi Akihiko,

> Subject: Re: [PATCH v8 09/10] virtio-gpu-dmabuf: Improve error handling
> with 'Error **' and err enum
> 
> On 2026/02/28 7:13, Vivek Kasireddy wrote:
> > Make the error handling more robust in virtio_gpu_init_udmabuf()
> > by introducing 'Error **' parameter to capture errors and using an
> > enum from VFIO to categorize Guest and Host errors. This allows
> > for better error reporting and handling of errors from
> > virtio_gpu_create_udmabuf() and virtio_gpu_remap_dmabuf().
> >
> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Cc: Alex Bennée <alex.bennee@linaro.org>
> > Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> > Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > Cc: Alex Williamson <alex@shazbot.org>
> > Cc: Cédric Le Goater <clg@redhat.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >   hw/display/virtio-gpu-dmabuf.c | 61 ++++++++++++++++++++++++-------
> ---
> >   1 file changed, 43 insertions(+), 18 deletions(-)
> >
> > diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
> dmabuf.c
> > index 8d67ef7c2a..b2badfc1bd 100644
> > --- a/hw/display/virtio-gpu-dmabuf.c
> > +++ b/hw/display/virtio-gpu-dmabuf.c
> > @@ -18,6 +18,7 @@
> >   #include "ui/console.h"
> >   #include "hw/virtio/virtio-gpu.h"
> >   #include "hw/virtio/virtio-gpu-pixman.h"
> > +#include "hw/vfio/vfio-device.h"
> >   #include "trace.h"
> >   #include "system/ramblock.h"
> >   #include "system/hostmem.h"
> > @@ -27,16 +28,18 @@
> >   #include "standard-headers/linux/udmabuf.h"
> >   #include "standard-headers/drm/drm_fourcc.h"
> >
> > -static void virtio_gpu_create_udmabuf(struct
> virtio_gpu_simple_resource *res)
> > +static int virtio_gpu_create_udmabuf(struct
> virtio_gpu_simple_resource *res,
> > +                                     Error **errp)
> >   {
> >       g_autofree struct udmabuf_create_list *list = NULL;
> >       RAMBlock *rb;
> >       ram_addr_t offset;
> > -    int udmabuf, i;
> > +    int udmabuf, i, fd;
> >
> >       udmabuf = udmabuf_fd();
> >       if (udmabuf < 0) {
> > -        return;
> > +        error_setg(errp, "udmabuf device not available or enabled");
> > +        return VFIO_DMABUF_CREATE_HOST_ERROR;
> 
> I failed to point out one thing for naming of the enum. The name should
> express if the error is caused by invalid iov or something else. Then
> virtio-gpu decides how to recover the error; it's virtio-gpu who knows
> the iov is from the guest and decides to report a guest error, not vfio.
> Otherwise, it reports a host error.
So, it sounds like you are suggesting that there needs to be three enum
values:
enum {
    /* Guest is responsible for this error */
    VFIO_DMABUF_CREATE_GUEST_ERROR = -1,
    /* Guest is at fault as the IOVEC passed in is invalid */
    VFIO_DMABUF_CREATE_INVALID_IOV_ERROR = -2,
    /* Host is at fault for this error */
    VFIO_DMABUF_CREATE_HOST_ERROR = -3,
}

Or, please let me know in detail what changes you would like to see.

> 
> >       }
> >
> >       list = g_malloc0(sizeof(struct udmabuf_create_list) +
> > @@ -45,7 +48,8 @@ static void virtio_gpu_create_udmabuf(struct
> virtio_gpu_simple_resource *res)
> >       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) {
> > -            return;
> > +            error_setg(errp, "IOV memory address incompatible with
> udmabuf ");
> > +            return VFIO_DMABUF_CREATE_GUEST_ERROR;
> >           }
> >
> >           list->list[i].memfd  = rb->fd;
> > @@ -56,22 +60,30 @@ static void virtio_gpu_create_udmabuf(struct
> virtio_gpu_simple_resource *res)
> >       list->count = res->iov_cnt;
> >       list->flags = UDMABUF_FLAGS_CLOEXEC;
> >
> > -    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> > -    if (res->dmabuf_fd < 0) {
> > -        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
> > -                    strerror(errno));
> > +    fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
> > +    if (fd < 0) {
> > +        error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl
> failed");
> > +        if (errno == EINVAL || errno == EBADFD) {
> > +            return VFIO_DMABUF_CREATE_GUEST_ERROR;
> > +        }
> > +        return VFIO_DMABUF_CREATE_HOST_ERROR;
> >       }
> > +    return fd;
> >   }
> >
> > -static void virtio_gpu_remap_dmabuf(struct
> virtio_gpu_simple_resource *res)
> > +static bool virtio_gpu_remap_dmabuf(struct
> virtio_gpu_simple_resource *res,
> > +                                    Error **errp)
> >   {
> > -    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
> > -                         MAP_SHARED, res->dmabuf_fd, 0);
> > -    if (res->remapped == MAP_FAILED) {
> > -        warn_report("%s: dmabuf mmap failed: %s", __func__,
> > -                    strerror(errno));
> > +    void *map;
> > +
> > +    map = mmap(NULL, res->blob_size, PROT_READ, MAP_SHARED, res-
> >dmabuf_fd, 0);
> > +    if (map == MAP_FAILED) {
> > +        error_setg_errno(errp, errno, "dmabuf mmap failed");
> >           res->remapped = NULL;
> > +        return false;
> >       }
> > +    res->remapped = map;
> > +    return true;
> >   }
> >
> >   static void virtio_gpu_destroy_dmabuf(struct
> virtio_gpu_simple_resource *res)
> > @@ -125,19 +137,32 @@ bool virtio_gpu_have_udmabuf(void)
> >
> >   void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
> >   {
> > +    Error *local_err = NULL;
> >       void *pdata = NULL;
> > +    int ret;
> >
> >       res->dmabuf_fd = -1;
> >       if (res->iov_cnt == 1 &&
> >           res->iov[0].iov_len < 4096) {
> >           pdata = res->iov[0].iov_base;
> >       } else {
> > -        virtio_gpu_create_udmabuf(res);
> > -        if (res->dmabuf_fd < 0) {
> > +        ret = virtio_gpu_create_udmabuf(res, &local_err);
> > +        if (ret < 0) {
> > +            error_prepend(&local_err, "Cannot create dmabuf: ");
> 
> This error_prepend() is redundant. The error messages
> virtio_gpu_create_udmabuf() emits already suggests they are problems
> with DMA-BUF.
Ok, I'll drop the error_prepend().

Thanks,
Vivek

> 
> Regards,
> Akihiko Odaki
> 
> > +
> > +            if (ret == VFIO_DMABUF_CREATE_GUEST_ERROR) {
> > +                qemu_log_mask(LOG_GUEST_ERROR,
> > +                              "Cannot create dmabuf: incompatible memory\n");
> > +                error_free(local_err);
> > +                return;
> > +            }
> > +            error_report_err(local_err);
> >               return;
> >           }
> > -        virtio_gpu_remap_dmabuf(res);
> > -        if (!res->remapped) {
> > +
> > +        res->dmabuf_fd = ret;
> > +        if (!virtio_gpu_remap_dmabuf(res, &local_err)) {
> > +            error_report_err(local_err);
> >               return;
> >           }
> >           pdata = res->remapped;
Re: [PATCH v8 09/10] virtio-gpu-dmabuf: Improve error handling with 'Error **' and err enum
Posted by Akihiko Odaki 2 hours ago
On 2026/03/02 15:03, Kasireddy, Vivek wrote:
> Hi Akihiko,
> 
>> Subject: Re: [PATCH v8 09/10] virtio-gpu-dmabuf: Improve error handling
>> with 'Error **' and err enum
>>
>> On 2026/02/28 7:13, Vivek Kasireddy wrote:
>>> Make the error handling more robust in virtio_gpu_init_udmabuf()
>>> by introducing 'Error **' parameter to capture errors and using an
>>> enum from VFIO to categorize Guest and Host errors. This allows
>>> for better error reporting and handling of errors from
>>> virtio_gpu_create_udmabuf() and virtio_gpu_remap_dmabuf().
>>>
>>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Cc: Alex Bennée <alex.bennee@linaro.org>
>>> Cc: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>> Cc: Alex Williamson <alex@shazbot.org>
>>> Cc: Cédric Le Goater <clg@redhat.com>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> ---
>>>    hw/display/virtio-gpu-dmabuf.c | 61 ++++++++++++++++++++++++-------
>> ---
>>>    1 file changed, 43 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/hw/display/virtio-gpu-dmabuf.c b/hw/display/virtio-gpu-
>> dmabuf.c
>>> index 8d67ef7c2a..b2badfc1bd 100644
>>> --- a/hw/display/virtio-gpu-dmabuf.c
>>> +++ b/hw/display/virtio-gpu-dmabuf.c
>>> @@ -18,6 +18,7 @@
>>>    #include "ui/console.h"
>>>    #include "hw/virtio/virtio-gpu.h"
>>>    #include "hw/virtio/virtio-gpu-pixman.h"
>>> +#include "hw/vfio/vfio-device.h"
>>>    #include "trace.h"
>>>    #include "system/ramblock.h"
>>>    #include "system/hostmem.h"
>>> @@ -27,16 +28,18 @@
>>>    #include "standard-headers/linux/udmabuf.h"
>>>    #include "standard-headers/drm/drm_fourcc.h"
>>>
>>> -static void virtio_gpu_create_udmabuf(struct
>> virtio_gpu_simple_resource *res)
>>> +static int virtio_gpu_create_udmabuf(struct
>> virtio_gpu_simple_resource *res,
>>> +                                     Error **errp)
>>>    {
>>>        g_autofree struct udmabuf_create_list *list = NULL;
>>>        RAMBlock *rb;
>>>        ram_addr_t offset;
>>> -    int udmabuf, i;
>>> +    int udmabuf, i, fd;
>>>
>>>        udmabuf = udmabuf_fd();
>>>        if (udmabuf < 0) {
>>> -        return;
>>> +        error_setg(errp, "udmabuf device not available or enabled");
>>> +        return VFIO_DMABUF_CREATE_HOST_ERROR;
>>
>> I failed to point out one thing for naming of the enum. The name should
>> express if the error is caused by invalid iov or something else. Then
>> virtio-gpu decides how to recover the error; it's virtio-gpu who knows
>> the iov is from the guest and decides to report a guest error, not vfio.
>> Otherwise, it reports a host error.
> So, it sounds like you are suggesting that there needs to be three enum
> values:
> enum {
>      /* Guest is responsible for this error */
>      VFIO_DMABUF_CREATE_GUEST_ERROR = -1,
>      /* Guest is at fault as the IOVEC passed in is invalid */
>      VFIO_DMABUF_CREATE_INVALID_IOV_ERROR = -2,
>      /* Host is at fault for this error */
>      VFIO_DMABUF_CREATE_HOST_ERROR = -3,
> }

vfio doesn't know if the guest or the host is responsible, so it cannot 
make use of VFIO_DMABUF_CREATE_GUEST_ERROR and 
VFIO_DMABUF_CREATE_HOST_ERROR.

> 
> Or, please let me know in detail what changes you would like to see.

A possible combination of values is as follows:

- VFIO_DMABUF_CREATE_ERR_UNSPEC
- VFIO_DMABUF_CREATE_ERR_INVALID_IOV

I took the example of VIRTIO_GPU_RESP_ERR_UNSPEC and 
VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER for naming.

> 
>>
>>>        }
>>>
>>>        list = g_malloc0(sizeof(struct udmabuf_create_list) +
>>> @@ -45,7 +48,8 @@ static void virtio_gpu_create_udmabuf(struct
>> virtio_gpu_simple_resource *res)
>>>        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) {
>>> -            return;
>>> +            error_setg(errp, "IOV memory address incompatible with
>> udmabuf ");
>>> +            return VFIO_DMABUF_CREATE_GUEST_ERROR;
>>>            }
>>>
>>>            list->list[i].memfd  = rb->fd;
>>> @@ -56,22 +60,30 @@ static void virtio_gpu_create_udmabuf(struct
>> virtio_gpu_simple_resource *res)
>>>        list->count = res->iov_cnt;
>>>        list->flags = UDMABUF_FLAGS_CLOEXEC;
>>>
>>> -    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
>>> -    if (res->dmabuf_fd < 0) {
>>> -        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
>>> -                    strerror(errno));
>>> +    fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
>>> +    if (fd < 0) {
>>> +        error_setg_errno(errp, errno, "UDMABUF_CREATE_LIST: ioctl
>> failed");
>>> +        if (errno == EINVAL || errno == EBADFD) {
>>> +            return VFIO_DMABUF_CREATE_GUEST_ERROR;
>>> +        }
>>> +        return VFIO_DMABUF_CREATE_HOST_ERROR;
>>>        }
>>> +    return fd;
>>>    }
>>>
>>> -static void virtio_gpu_remap_dmabuf(struct
>> virtio_gpu_simple_resource *res)
>>> +static bool virtio_gpu_remap_dmabuf(struct
>> virtio_gpu_simple_resource *res,
>>> +                                    Error **errp)
>>>    {
>>> -    res->remapped = mmap(NULL, res->blob_size, PROT_READ,
>>> -                         MAP_SHARED, res->dmabuf_fd, 0);
>>> -    if (res->remapped == MAP_FAILED) {
>>> -        warn_report("%s: dmabuf mmap failed: %s", __func__,
>>> -                    strerror(errno));
>>> +    void *map;
>>> +
>>> +    map = mmap(NULL, res->blob_size, PROT_READ, MAP_SHARED, res-
>>> dmabuf_fd, 0);
>>> +    if (map == MAP_FAILED) {
>>> +        error_setg_errno(errp, errno, "dmabuf mmap failed");
>>>            res->remapped = NULL;
>>> +        return false;
>>>        }
>>> +    res->remapped = map;
>>> +    return true;
>>>    }
>>>
>>>    static void virtio_gpu_destroy_dmabuf(struct
>> virtio_gpu_simple_resource *res)
>>> @@ -125,19 +137,32 @@ bool virtio_gpu_have_udmabuf(void)
>>>
>>>    void virtio_gpu_init_dmabuf(struct virtio_gpu_simple_resource *res)
>>>    {
>>> +    Error *local_err = NULL;
>>>        void *pdata = NULL;
>>> +    int ret;
>>>
>>>        res->dmabuf_fd = -1;
>>>        if (res->iov_cnt == 1 &&
>>>            res->iov[0].iov_len < 4096) {
>>>            pdata = res->iov[0].iov_base;
>>>        } else {
>>> -        virtio_gpu_create_udmabuf(res);
>>> -        if (res->dmabuf_fd < 0) {
>>> +        ret = virtio_gpu_create_udmabuf(res, &local_err);
>>> +        if (ret < 0) {
>>> +            error_prepend(&local_err, "Cannot create dmabuf: ");
>>
>> This error_prepend() is redundant. The error messages
>> virtio_gpu_create_udmabuf() emits already suggests they are problems
>> with DMA-BUF.
> Ok, I'll drop the error_prepend().
> 
> Thanks,
> Vivek
> 
>>
>> Regards,
>> Akihiko Odaki
>>
>>> +
>>> +            if (ret == VFIO_DMABUF_CREATE_GUEST_ERROR) {
>>> +                qemu_log_mask(LOG_GUEST_ERROR,
>>> +                              "Cannot create dmabuf: incompatible memory\n");
>>> +                error_free(local_err);
>>> +                return;
>>> +            }
>>> +            error_report_err(local_err);
>>>                return;
>>>            }
>>> -        virtio_gpu_remap_dmabuf(res);
>>> -        if (!res->remapped) {
>>> +
>>> +        res->dmabuf_fd = ret;
>>> +        if (!virtio_gpu_remap_dmabuf(res, &local_err)) {
>>> +            error_report_err(local_err);
>>>                return;
>>>            }
>>>            pdata = res->remapped;
>