vfio_display_edid_init() can fail for many reasons and return silently.
It would be good to report the error.

Old mdev driver may not support vfio edid region and we allow to go
through in this case.

vfio_display_edid_update() isn't changed because it can be called at
runtime when UI changes (i.e. window resize).

Fixes: 08479114b0de ("vfio/display: add edid support.")
Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/display.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index 5926bd6628..462845ce69 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -124,7 +124,7 @@ static void vfio_display_edid_ui_info(void *opaque, uint32_t idx,
     }
 }
 
-static void vfio_display_edid_init(VFIOPCIDevice *vdev)
+static bool vfio_display_edid_init(VFIOPCIDevice *vdev, Error **errp)
 {
     VFIODisplay *dpy = vdev->dpy;
     int fd = vdev->vbasedev.fd;
@@ -135,7 +135,8 @@ static void vfio_display_edid_init(VFIOPCIDevice *vdev)
                                    VFIO_REGION_SUBTYPE_GFX_EDID,
                                    &dpy->edid_info);
     if (ret) {
-        return;
+        /* Failed to get GFX edid info, allow to go through without edid. */
+        return true;
     }
 
     trace_vfio_display_edid_available();
@@ -167,14 +168,15 @@ static void vfio_display_edid_init(VFIOPCIDevice *vdev)
                                         vfio_display_edid_link_up, vdev);
 
     vfio_display_edid_update(vdev, true, 0, 0);
-    return;
+    return true;
 
 err:
+    error_setg(errp, "vfio: failed to read GFX edid field");
     trace_vfio_display_edid_write_error();
     g_free(dpy->edid_info);
     g_free(dpy->edid_regs);
     dpy->edid_regs = NULL;
-    return;
+    return false;
 }
 
 static void vfio_display_edid_exit(VFIODisplay *dpy)
@@ -367,8 +369,7 @@ static bool vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
             return false;
         }
     }
-    vfio_display_edid_init(vdev);
-    return true;
+    return vfio_display_edid_init(vdev, errp);
 }
 
 static void vfio_display_dmabuf_exit(VFIODisplay *dpy)
-- 
2.34.1

On Fri, Jun 28, 2024 at 1:31 PM Zhenzhong Duan <zhenzhong.duan@intel.com>
wrote:

> vfio_display_edid_init() can fail for many reasons and return silently.
> It would be good to report the error.
>
> Old mdev driver may not support vfio edid region and we allow to go
> through in this case.
>
> vfio_display_edid_update() isn't changed because it can be called at
> runtime when UI changes (i.e. window resize).
>
> Fixes: 08479114b0de ("vfio/display: add edid support.")
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---
>  hw/vfio/display.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index 5926bd6628..462845ce69 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -124,7 +124,7 @@ static void vfio_display_edid_ui_info(void *opaque,
> uint32_t idx,
>      }
>  }
>
> -static void vfio_display_edid_init(VFIOPCIDevice *vdev)
> +static bool vfio_display_edid_init(VFIOPCIDevice *vdev, Error **errp)
>  {
>      VFIODisplay *dpy = vdev->dpy;
>      int fd = vdev->vbasedev.fd;
> @@ -135,7 +135,8 @@ static void vfio_display_edid_init(VFIOPCIDevice *vdev)
>                                     VFIO_REGION_SUBTYPE_GFX_EDID,
>                                     &dpy->edid_info);
>      if (ret) {
> -        return;
> +        /* Failed to get GFX edid info, allow to go through without edid.
> */
> +        return true;
>      }
>
>      trace_vfio_display_edid_available();
> @@ -167,14 +168,15 @@ static void vfio_display_edid_init(VFIOPCIDevice
> *vdev)
>                                          vfio_display_edid_link_up, vdev);
>
>      vfio_display_edid_update(vdev, true, 0, 0);
> -    return;
> +    return true;
>
>  err:
> +    error_setg(errp, "vfio: failed to read GFX edid field");
>      trace_vfio_display_edid_write_error();
>      g_free(dpy->edid_info);
>      g_free(dpy->edid_regs);
>      dpy->edid_regs = NULL;
> -    return;
> +    return false;
>  }
>
>  static void vfio_display_edid_exit(VFIODisplay *dpy)
> @@ -367,8 +369,7 @@ static bool vfio_display_dmabuf_init(VFIOPCIDevice
> *vdev, Error **errp)
>              return false;
>          }
>      }
> -    vfio_display_edid_init(vdev);
> -    return true;
> +    return vfio_display_edid_init(vdev, errp);
>  }
>
>  static void vfio_display_dmabuf_exit(VFIODisplay *dpy)
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau