diff mbox series

[2/2] vfio/display: Fix vfio_display_edid_init() error path

Message ID 20240628092815.164423-3-zhenzhong.duan@intel.com (mailing list archive)
State New, archived
Headers show
Series Misc fixes on vfio display | expand

Commit Message

Duan, Zhenzhong June 28, 2024, 9:28 a.m. UTC
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(-)

Comments

Marc-André Lureau June 29, 2024, 12:17 p.m. UTC | #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
>
>
>
diff mbox series

Patch

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)