[PATCH v4 0/3] Rework FIXMEs and fprintfs in hcd-xhci.c

[PATCH v4 0/3] Rework FIXMEs and fprintfs in hcd-xhci.c

[Thomas Huth]

Replace the local FIXME macro and fprintf statement with
qemu_log_mask() and g_assert_not_reached().

v4:
- Remove the DPRINTF in the second patch
- Add the missing \n in the third patch

Thomas Huth (3):
  hw/usb/hcd-xhci: Turn guest-triggerable abort() into qemu_log_mask()
  hw/usb/hcd-xhci: Remove the FIXME macro
  hw/usb/hcd-xhci: Use qemu_log_mask() instead of fprintf() statement

 hw/usb/hcd-xhci.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

-- 
2.55.0

[PATCH v4 1/3] hw/usb/hcd-xhci: Turn guest-triggerable abort() into qemu_log_mask()

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

The FIXME macros in xhci_alloc_device_streams() can be triggered
by a (malicious) guest. Since the macro also contains an abort()
statement, this terminates QEMU. Turn the FIXME statements into
a qemu_log_mask() instead to avoid that a guest can shoot itself
this way.

Reported-by: Feifan Qian <bea1e@proton.me>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3784
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/usb/hcd-xhci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 2cdab3ba0e4..f5bb7d25600 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -965,11 +965,13 @@ static TRBCCode xhci_alloc_device_streams(XHCIState *xhci, unsigned int slotid,
          * together and make an usb_device_alloc_streams call per group.
          */
         if (epctxs[i]->nr_pstreams != req_nr_streams) {
-            FIXME("guest streams config not identical for all eps");
+            qemu_log_mask(LOG_UNIMP,
+                          "guest streams config not identical for all eps\n");
             return CC_RESOURCE_ERROR;
         }
         if (eps[i]->max_streams != dev_max_streams) {
-            FIXME("device streams config not identical for all eps");
+            qemu_log_mask(LOG_UNIMP,
+                          "device streams config not identical for all eps\n");
             return CC_RESOURCE_ERROR;
         }
     }
-- 
2.55.0

[PATCH v4 2/3] hw/usb/hcd-xhci: Remove the FIXME macro

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

The FIXME macro is only used in one case, which should hopefully
never trigger: The containing function handles all the USB_RET_*
values except for USB_RET_ADD_TO_QUEUE and USB_RET_REMOVE_FROM_QUEUE,
which are both internal return values for when an async packet needs
to be queued or dequeued, and which shouldn't still be the status by
the time we get to this function. Thus let's simplify this spot
and use a g_assert_not_reached() instead (and remove the DPRINT()
in front of it to avoid that code analyzers trip over unreachable
code here).

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/usb/hcd-xhci.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index f5bb7d25600..cc61e28d33b 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -39,8 +39,6 @@
 #else
 #define DPRINTF(...) do {} while (0)
 #endif
-#define FIXME(_msg) do { fprintf(stderr, "FIXME %s:%d %s\n", \
-                                 __func__, __LINE__, _msg); abort(); } while (0)
 
 #define TRB_LINK_LIMIT  32
 #define COMMAND_LIMIT   256
@@ -1673,9 +1671,7 @@ static int xhci_try_complete_packet(XHCITransfer *xfer)
         xhci_stall_ep(xfer);
         break;
     default:
-        DPRINTF("%s: FIXME: status = %d\n", __func__,
-                xfer->packet.status);
-        FIXME("unhandled USB_RET_*");
+        g_assert_not_reached();
     }
     return 0;
 }
-- 
2.55.0

[PATCH v4 3/3] hw/usb/hcd-xhci: Use qemu_log_mask() instead of fprintf() statement

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

We've got a proper way for logging unimplemented hardware features,
so use qemu_log_mask() instead of the fprintf() here now.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/usb/hcd-xhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index cc61e28d33b..c7e050e38fd 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1017,7 +1017,8 @@ static XHCIStreamContext *xhci_find_stream(XHCIEPContext *epctx,
         }
         sctx = epctx->pstreams + streamid;
     } else {
-        fprintf(stderr, "xhci: FIXME: secondary streams not implemented yet");
+        qemu_log_mask(LOG_UNIMP,
+                      "xhci: secondary streams not implemented yet\n");
         *cc_error = CC_INVALID_STREAM_TYPE_ERROR;
         return NULL;
     }
-- 
2.55.0

Re: [PATCH v4 3/3] hw/usb/hcd-xhci: Use qemu_log_mask() instead of fprintf() statement

[Philippe Mathieu-Daudé]

On 2026-07-13 18:14, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> We've got a proper way for logging unimplemented hardware features,
> so use qemu_log_mask() instead of the fprintf() here now.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   hw/usb/hcd-xhci.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH v4 0/3] Rework FIXMEs and fprintfs in hcd-xhci.c

[Philippe Mathieu-Daudé]

On 13/7/26 18:14, Thomas Huth wrote:

> Thomas Huth (3):
>    hw/usb/hcd-xhci: Turn guest-triggerable abort() into qemu_log_mask()
>    hw/usb/hcd-xhci: Remove the FIXME macro
>    hw/usb/hcd-xhci: Use qemu_log_mask() instead of fprintf() statement

Series queued, thanks.