[PATCH 0/2] xlnx-dp: fix a couple of fuzzer bugs

[PATCH 0/2] xlnx-dp: fix a couple of fuzzer bugs

[Peter Maydell]

This patchset fixes a couple of bugs found ages ago by fuzzer,
where a guest that tells the device to do silly things can
trigger an assert() or abort():

 https://gitlab.com/qemu-project/qemu/-/issues/1415
 https://gitlab.com/qemu-project/qemu/-/issues/1418
 https://gitlab.com/qemu-project/qemu/-/issues/1419
 https://gitlab.com/qemu-project/qemu/-/issues/1424

None of them are really very interesting, but the fixes are
straightforward.

thanks
-- PMM

Peter Maydell (2):
  hw/display/xlnx_dp.c: Don't abort on AUX FIFO overrun/underrun
  hw/display/xlnx_dp: Don't abort for unsupported graphics formats

 hw/display/xlnx_dp.c | 81 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 74 insertions(+), 7 deletions(-)

-- 
2.43.0

[PATCH 1/2] hw/display/xlnx_dp.c: Don't abort on AUX FIFO overrun/underrun

[Peter Maydell]

The documentation of the Xilinx DisplayPort subsystem at
https://www.xilinx.com/support/documents/ip_documentation/v_dp_txss1/v3_1/pg299-v-dp-txss1.pdf
doesn't say what happens if a guest tries to issue an AUX write
command with a length greater than the amount of data in the AUX
write FIFO, or tries to write more data to the write FIFO than it can
hold, or issues multiple commands that put data into the AUX read
FIFO without reading it such that it overflows.

Currently QEMU will abort() in these guest-error situations, either
in xlnx_dp.c itself or in the fifo8 code.  Make these cases all be
logged as guest errors instead.  We choose to ignore the new data on
overflow, and return 0 on underflow. This is in line with how we handled
the "read from empty RX FIFO" case in commit a09ef5040477.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1418
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1419
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1424
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/xlnx_dp.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index 96cbb1b3a7d..c2bf692e7b1 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -435,7 +435,18 @@ static void xlnx_dp_aux_clear_rx_fifo(XlnxDPState *s)
 
 static void xlnx_dp_aux_push_rx_fifo(XlnxDPState *s, uint8_t *buf, size_t len)
 {
+    size_t avail = fifo8_num_free(&s->rx_fifo);
     DPRINTF("Push %u data in rx_fifo\n", (unsigned)len);
+    if (len > avail) {
+        /*
+         * Data sheet doesn't specify behaviour here: we choose to ignore
+         * the excess data.
+         */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: ignoring %zu bytes pushed to full RX_FIFO\n",
+                      __func__, len - avail);
+        len = avail;
+    }
     fifo8_push_all(&s->rx_fifo, buf, len);
 }
 
@@ -466,7 +477,18 @@ static void xlnx_dp_aux_clear_tx_fifo(XlnxDPState *s)
 
 static void xlnx_dp_aux_push_tx_fifo(XlnxDPState *s, uint8_t *buf, size_t len)
 {
+    size_t avail = fifo8_num_free(&s->tx_fifo);
     DPRINTF("Push %u data in tx_fifo\n", (unsigned)len);
+    if (len > avail) {
+        /*
+         * Data sheet doesn't specify behaviour here: we choose to ignore
+         * the excess data.
+         */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: ignoring %zu bytes pushed to full TX_FIFO\n",
+                      __func__, len - avail);
+        len = avail;
+    }
     fifo8_push_all(&s->tx_fifo, buf, len);
 }
 
@@ -475,8 +497,10 @@ static uint8_t xlnx_dp_aux_pop_tx_fifo(XlnxDPState *s)
     uint8_t ret;
 
     if (fifo8_is_empty(&s->tx_fifo)) {
-        error_report("%s: TX_FIFO underflow", __func__);
-        abort();
+        /* Data sheet doesn't specify behaviour here: we choose to return 0 */
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: attempt to read empty TX_FIFO\n",
+                      __func__);
+        return 0;
     }
     ret = fifo8_pop(&s->tx_fifo);
     DPRINTF("pop 0x%2.2X from tx_fifo.\n", ret);
-- 
2.43.0

[PATCH 2/2] hw/display/xlnx_dp: Don't abort for unsupported graphics formats

[Peter Maydell]

If the guest writes an invalid or unsupported value to the
AV_BUF_FORMAT register, currently we abort().  Instead, log this as
either a guest error or an unimplemented error and continue.

The existing code treats DP_NL_VID_CB_Y0_CR_Y1 as x8b8g8r8
via a "case 0" that does not use the enum constant name for some
reason; we leave that alone beyond adding a comment about the
weird code.

Documentation of this register seems to be at:
https://docs.amd.com/r/en-US/ug1087-zynq-ultrascale-registers/AV_BUF_FORMAT-DISPLAY_PORT-Register

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1415
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/xlnx_dp.c | 53 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 48 insertions(+), 5 deletions(-)

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index c2bf692e7b1..d8119a56292 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -665,14 +665,28 @@ static void xlnx_dp_change_graphic_fmt(XlnxDPState *s)
     case DP_GRAPHIC_BGR888:
         s->g_plane.format = PIXMAN_b8g8r8;
         break;
+    case DP_GRAPHIC_RGBA5551:
+    case DP_GRAPHIC_RGBA4444:
+    case DP_GRAPHIC_8BPP:
+    case DP_GRAPHIC_4BPP:
+    case DP_GRAPHIC_2BPP:
+    case DP_GRAPHIC_1BPP:
+        qemu_log_mask(LOG_UNIMP, "%s: unimplemented graphic format %u",
+                      __func__,
+                      s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
+        s->g_plane.format = PIXMAN_r8g8b8a8;
+        break;
     default:
-        error_report("%s: unsupported graphic format %u", __func__,
-                     s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid graphic format %u",
+                      __func__,
+                      s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
+        s->g_plane.format = PIXMAN_r8g8b8a8;
         abort();
     }
 
     switch (s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK) {
     case 0:
+        /* This is DP_NL_VID_CB_Y0_CR_Y1 ??? */
         s->v_plane.format = PIXMAN_x8b8g8r8;
         break;
     case DP_NL_VID_Y0_CB_Y1_CR:
@@ -681,10 +695,39 @@ static void xlnx_dp_change_graphic_fmt(XlnxDPState *s)
     case DP_NL_VID_RGBA8880:
         s->v_plane.format = PIXMAN_x8b8g8r8;
         break;
+    case DP_NL_VID_CR_Y0_CB_Y1:
+    case DP_NL_VID_Y0_CR_Y1_CB:
+    case DP_NL_VID_YV16:
+    case DP_NL_VID_YV24:
+    case DP_NL_VID_YV16CL:
+    case DP_NL_VID_MONO:
+    case DP_NL_VID_YV16CL2:
+    case DP_NL_VID_YUV444:
+    case DP_NL_VID_RGB888:
+    case DP_NL_VID_RGB888_10BPC:
+    case DP_NL_VID_YUV444_10BPC:
+    case DP_NL_VID_YV16CL2_10BPC:
+    case DP_NL_VID_YV16CL_10BPC:
+    case DP_NL_VID_YV16_10BPC:
+    case DP_NL_VID_YV24_10BPC:
+    case DP_NL_VID_Y_ONLY_10BPC:
+    case DP_NL_VID_YV16_420:
+    case DP_NL_VID_YV16CL_420:
+    case DP_NL_VID_YV16CL2_420:
+    case DP_NL_VID_YV16_420_10BPC:
+    case DP_NL_VID_YV16CL_420_10BPC:
+    case DP_NL_VID_YV16CL2_420_10BPC:
+        qemu_log_mask(LOG_UNIMP, "%s: unimplemented video format %u",
+                      __func__,
+                      s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK);
+        s->v_plane.format = PIXMAN_x8b8g8r8;
+        break;
     default:
-        error_report("%s: unsupported video format %u", __func__,
-                     s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK);
-        abort();
+        qemu_log_mask(LOG_UNIMP, "%s: invalid video format %u",
+                      __func__,
+                      s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK);
+        s->v_plane.format = PIXMAN_x8b8g8r8;
+        break;
     }
 
     xlnx_dp_recreate_surface(s);
-- 
2.43.0

Re: [PATCH 1/2] hw/display/xlnx_dp.c: Don't abort on AUX FIFO overrun/underrun

[Philippe Mathieu-Daudé]

On 6/11/25 15:52, Peter Maydell wrote:
> The documentation of the Xilinx DisplayPort subsystem at
> https://www.xilinx.com/support/documents/ip_documentation/v_dp_txss1/v3_1/pg299-v-dp-txss1.pdf
> doesn't say what happens if a guest tries to issue an AUX write
> command with a length greater than the amount of data in the AUX
> write FIFO, or tries to write more data to the write FIFO than it can
> hold, or issues multiple commands that put data into the AUX read
> FIFO without reading it such that it overflows.
> 
> Currently QEMU will abort() in these guest-error situations, either
> in xlnx_dp.c itself or in the fifo8 code.  Make these cases all be
> logged as guest errors instead.  We choose to ignore the new data on
> overflow, and return 0 on underflow. This is in line with how we handled
> the "read from empty RX FIFO" case in commit a09ef5040477.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1418
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1419
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1424
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/display/xlnx_dp.c | 28 ++++++++++++++++++++++++++--
>   1 file changed, 26 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 1/2] hw/display/xlnx_dp.c: Don't abort on AUX FIFO overrun/underrun

[Edgar E. Iglesias]

On Thu, Nov 06, 2025 at 02:52:08PM +0000, Peter Maydell wrote:
> The documentation of the Xilinx DisplayPort subsystem at
> https://www.xilinx.com/support/documents/ip_documentation/v_dp_txss1/v3_1/pg299-v-dp-txss1.pdf
> doesn't say what happens if a guest tries to issue an AUX write
> command with a length greater than the amount of data in the AUX
> write FIFO, or tries to write more data to the write FIFO than it can
> hold, or issues multiple commands that put data into the AUX read
> FIFO without reading it such that it overflows.
> 
> Currently QEMU will abort() in these guest-error situations, either
> in xlnx_dp.c itself or in the fifo8 code.  Make these cases all be
> logged as guest errors instead.  We choose to ignore the new data on
> overflow, and return 0 on underflow. This is in line with how we handled
> the "read from empty RX FIFO" case in commit a09ef5040477.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1418
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1419
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1424
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>



> ---
>  hw/display/xlnx_dp.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index 96cbb1b3a7d..c2bf692e7b1 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -435,7 +435,18 @@ static void xlnx_dp_aux_clear_rx_fifo(XlnxDPState *s)
>  
>  static void xlnx_dp_aux_push_rx_fifo(XlnxDPState *s, uint8_t *buf, size_t len)
>  {
> +    size_t avail = fifo8_num_free(&s->rx_fifo);
>      DPRINTF("Push %u data in rx_fifo\n", (unsigned)len);
> +    if (len > avail) {
> +        /*
> +         * Data sheet doesn't specify behaviour here: we choose to ignore
> +         * the excess data.
> +         */
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: ignoring %zu bytes pushed to full RX_FIFO\n",
> +                      __func__, len - avail);
> +        len = avail;
> +    }
>      fifo8_push_all(&s->rx_fifo, buf, len);
>  }
>  
> @@ -466,7 +477,18 @@ static void xlnx_dp_aux_clear_tx_fifo(XlnxDPState *s)
>  
>  static void xlnx_dp_aux_push_tx_fifo(XlnxDPState *s, uint8_t *buf, size_t len)
>  {
> +    size_t avail = fifo8_num_free(&s->tx_fifo);
>      DPRINTF("Push %u data in tx_fifo\n", (unsigned)len);
> +    if (len > avail) {
> +        /*
> +         * Data sheet doesn't specify behaviour here: we choose to ignore
> +         * the excess data.
> +         */
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: ignoring %zu bytes pushed to full TX_FIFO\n",
> +                      __func__, len - avail);
> +        len = avail;
> +    }
>      fifo8_push_all(&s->tx_fifo, buf, len);
>  }
>  
> @@ -475,8 +497,10 @@ static uint8_t xlnx_dp_aux_pop_tx_fifo(XlnxDPState *s)
>      uint8_t ret;
>  
>      if (fifo8_is_empty(&s->tx_fifo)) {
> -        error_report("%s: TX_FIFO underflow", __func__);
> -        abort();
> +        /* Data sheet doesn't specify behaviour here: we choose to return 0 */
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: attempt to read empty TX_FIFO\n",
> +                      __func__);
> +        return 0;
>      }
>      ret = fifo8_pop(&s->tx_fifo);
>      DPRINTF("pop 0x%2.2X from tx_fifo.\n", ret);
> -- 
> 2.43.0
> 

Re: [PATCH 2/2] hw/display/xlnx_dp: Don't abort for unsupported graphics formats

[Edgar E. Iglesias]

On Thu, Nov 06, 2025 at 02:52:09PM +0000, Peter Maydell wrote:
> If the guest writes an invalid or unsupported value to the
> AV_BUF_FORMAT register, currently we abort().  Instead, log this as
> either a guest error or an unimplemented error and continue.
> 
> The existing code treats DP_NL_VID_CB_Y0_CR_Y1 as x8b8g8r8
> via a "case 0" that does not use the enum constant name for some
> reason; we leave that alone beyond adding a comment about the
> weird code.
> 
> Documentation of this register seems to be at:
> https://docs.amd.com/r/en-US/ug1087-zynq-ultrascale-registers/AV_BUF_FORMAT-DISPLAY_PORT-Register
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1415
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>



> ---
>  hw/display/xlnx_dp.c | 53 +++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index c2bf692e7b1..d8119a56292 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -665,14 +665,28 @@ static void xlnx_dp_change_graphic_fmt(XlnxDPState *s)
>      case DP_GRAPHIC_BGR888:
>          s->g_plane.format = PIXMAN_b8g8r8;
>          break;
> +    case DP_GRAPHIC_RGBA5551:
> +    case DP_GRAPHIC_RGBA4444:
> +    case DP_GRAPHIC_8BPP:
> +    case DP_GRAPHIC_4BPP:
> +    case DP_GRAPHIC_2BPP:
> +    case DP_GRAPHIC_1BPP:
> +        qemu_log_mask(LOG_UNIMP, "%s: unimplemented graphic format %u",
> +                      __func__,
> +                      s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
> +        s->g_plane.format = PIXMAN_r8g8b8a8;
> +        break;
>      default:
> -        error_report("%s: unsupported graphic format %u", __func__,
> -                     s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid graphic format %u",
> +                      __func__,
> +                      s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
> +        s->g_plane.format = PIXMAN_r8g8b8a8;
>          abort();
>      }
>  
>      switch (s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK) {
>      case 0:
> +        /* This is DP_NL_VID_CB_Y0_CR_Y1 ??? */
>          s->v_plane.format = PIXMAN_x8b8g8r8;
>          break;
>      case DP_NL_VID_Y0_CB_Y1_CR:
> @@ -681,10 +695,39 @@ static void xlnx_dp_change_graphic_fmt(XlnxDPState *s)
>      case DP_NL_VID_RGBA8880:
>          s->v_plane.format = PIXMAN_x8b8g8r8;
>          break;
> +    case DP_NL_VID_CR_Y0_CB_Y1:
> +    case DP_NL_VID_Y0_CR_Y1_CB:
> +    case DP_NL_VID_YV16:
> +    case DP_NL_VID_YV24:
> +    case DP_NL_VID_YV16CL:
> +    case DP_NL_VID_MONO:
> +    case DP_NL_VID_YV16CL2:
> +    case DP_NL_VID_YUV444:
> +    case DP_NL_VID_RGB888:
> +    case DP_NL_VID_RGB888_10BPC:
> +    case DP_NL_VID_YUV444_10BPC:
> +    case DP_NL_VID_YV16CL2_10BPC:
> +    case DP_NL_VID_YV16CL_10BPC:
> +    case DP_NL_VID_YV16_10BPC:
> +    case DP_NL_VID_YV24_10BPC:
> +    case DP_NL_VID_Y_ONLY_10BPC:
> +    case DP_NL_VID_YV16_420:
> +    case DP_NL_VID_YV16CL_420:
> +    case DP_NL_VID_YV16CL2_420:
> +    case DP_NL_VID_YV16_420_10BPC:
> +    case DP_NL_VID_YV16CL_420_10BPC:
> +    case DP_NL_VID_YV16CL2_420_10BPC:
> +        qemu_log_mask(LOG_UNIMP, "%s: unimplemented video format %u",
> +                      __func__,
> +                      s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK);
> +        s->v_plane.format = PIXMAN_x8b8g8r8;
> +        break;
>      default:
> -        error_report("%s: unsupported video format %u", __func__,
> -                     s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK);
> -        abort();
> +        qemu_log_mask(LOG_UNIMP, "%s: invalid video format %u",
> +                      __func__,
> +                      s->avbufm_registers[AV_BUF_FORMAT] & DP_NL_VID_FMT_MASK);
> +        s->v_plane.format = PIXMAN_x8b8g8r8;
> +        break;
>      }
>  
>      xlnx_dp_recreate_surface(s);
> -- 
> 2.43.0
> 

Re: [PATCH 2/2] hw/display/xlnx_dp: Don't abort for unsupported graphics formats

[Philippe Mathieu-Daudé]

Hi,

On 6/11/25 15:52, Peter Maydell wrote:
> If the guest writes an invalid or unsupported value to the
> AV_BUF_FORMAT register, currently we abort().  Instead, log this as
> either a guest error or an unimplemented error and continue.
> 
> The existing code treats DP_NL_VID_CB_Y0_CR_Y1 as x8b8g8r8
> via a "case 0" that does not use the enum constant name for some
> reason; we leave that alone beyond adding a comment about the
> weird code.
> 
> Documentation of this register seems to be at:
> https://docs.amd.com/r/en-US/ug1087-zynq-ultrascale-registers/AV_BUF_FORMAT-DISPLAY_PORT-Register
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1415
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/display/xlnx_dp.c | 53 +++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index c2bf692e7b1..d8119a56292 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -665,14 +665,28 @@ static void xlnx_dp_change_graphic_fmt(XlnxDPState *s)
>       case DP_GRAPHIC_BGR888:
>           s->g_plane.format = PIXMAN_b8g8r8;
>           break;
> +    case DP_GRAPHIC_RGBA5551:
> +    case DP_GRAPHIC_RGBA4444:
> +    case DP_GRAPHIC_8BPP:
> +    case DP_GRAPHIC_4BPP:
> +    case DP_GRAPHIC_2BPP:
> +    case DP_GRAPHIC_1BPP:
> +        qemu_log_mask(LOG_UNIMP, "%s: unimplemented graphic format %u",
> +                      __func__,
> +                      s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
> +        s->g_plane.format = PIXMAN_r8g8b8a8;
> +        break;
>       default:
> -        error_report("%s: unsupported graphic format %u", __func__,
> -                     s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid graphic format %u",
> +                      __func__,
> +                      s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
> +        s->g_plane.format = PIXMAN_r8g8b8a8;
>           abort();

Don't we want to remove this abort() call?

Otherwise,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>       }

Re: [PATCH 2/2] hw/display/xlnx_dp: Don't abort for unsupported graphics formats

[Peter Maydell]

On Sun, 9 Nov 2025 at 13:40, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi,
>
> On 6/11/25 15:52, Peter Maydell wrote:
> > If the guest writes an invalid or unsupported value to the
> > AV_BUF_FORMAT register, currently we abort().  Instead, log this as
> > either a guest error or an unimplemented error and continue.
> >
> > The existing code treats DP_NL_VID_CB_Y0_CR_Y1 as x8b8g8r8
> > via a "case 0" that does not use the enum constant name for some
> > reason; we leave that alone beyond adding a comment about the
> > weird code.
> >
> > Documentation of this register seems to be at:
> > https://docs.amd.com/r/en-US/ug1087-zynq-ultrascale-registers/AV_BUF_FORMAT-DISPLAY_PORT-Register
> >
> > Cc: qemu-stable@nongnu.org
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1415
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   hw/display/xlnx_dp.c | 53 +++++++++++++++++++++++++++++++++++++++-----
> >   1 file changed, 48 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> > index c2bf692e7b1..d8119a56292 100644
> > --- a/hw/display/xlnx_dp.c
> > +++ b/hw/display/xlnx_dp.c
> > @@ -665,14 +665,28 @@ static void xlnx_dp_change_graphic_fmt(XlnxDPState *s)
> >       case DP_GRAPHIC_BGR888:
> >           s->g_plane.format = PIXMAN_b8g8r8;
> >           break;
> > +    case DP_GRAPHIC_RGBA5551:
> > +    case DP_GRAPHIC_RGBA4444:
> > +    case DP_GRAPHIC_8BPP:
> > +    case DP_GRAPHIC_4BPP:
> > +    case DP_GRAPHIC_2BPP:
> > +    case DP_GRAPHIC_1BPP:
> > +        qemu_log_mask(LOG_UNIMP, "%s: unimplemented graphic format %u",
> > +                      __func__,
> > +                      s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
> > +        s->g_plane.format = PIXMAN_r8g8b8a8;
> > +        break;
> >       default:
> > -        error_report("%s: unsupported graphic format %u", __func__,
> > -                     s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
> > +        qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid graphic format %u",
> > +                      __func__,
> > +                      s->avbufm_registers[AV_BUF_FORMAT] & DP_GRAPHIC_MASK);
> > +        s->g_plane.format = PIXMAN_r8g8b8a8;
> >           abort();
>
> Don't we want to remove this abort() call?

Whoops, yes. (The test case in the bug goes through the LOG_UNIMP
arm rather than the LOG_GUEST_ERROR one, so I didn't spot this.)

-- PMM

Re: [PATCH 0/2] xlnx-dp: fix a couple of fuzzer bugs

[Peter Maydell]

On Thu, 6 Nov 2025 at 14:52, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset fixes a couple of bugs found ages ago by fuzzer,
> where a guest that tells the device to do silly things can
> trigger an assert() or abort():
>
>  https://gitlab.com/qemu-project/qemu/-/issues/1415
>  https://gitlab.com/qemu-project/qemu/-/issues/1418
>  https://gitlab.com/qemu-project/qemu/-/issues/1419
>  https://gitlab.com/qemu-project/qemu/-/issues/1424
>
> None of them are really very interesting, but the fixes are
> straightforward.

Applied to target-arm.next (with the abort() in patch 2
fixed to 'break'), thanks.

-- PMM