* [PULL 1/7] hw/usb/hcd-xhci: Set reentrancy guard in timer functions (CVE-2026-17588)
2026-09-24 9:05 [PULL 0/7] XHCI fixes and drop Haiku VM Thomas Huth
@ 2026-09-24 9:05 ` Thomas Huth
2026-09-24 9:05 ` [PULL 2/7] hw/usb/hcd-xhci: Limit DMA transfers to plain memory Thomas Huth
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2026-09-24 9:05 UTC (permalink / raw)
To: qemu-devel, Richard Henderson; +Cc: XlabAI Team of Tencent Xuanwu Lab
The xHCI controller processes USB events from timer callbacks
(xhci_mfwrap_timer and xhci_ep_kick_timer) outside of any MMIO handler
context. The device's mem_reentrancy_guard is therefore not engaged
during this processing.
A malicious guest can exploit this by pointing the event ring base
address (er_start) at the xHCI doorbell MMIO region. When
xhci_write_event() performs a DMA write to deliver a transfer
completion event, the write lands on doorbell register 0. If the
written value is 0, this triggers xhci_process_commands() reentrantly.
A CR_DISABLE_SLOT command prepared on the command ring then frees
endpoint and transfer objects via xhci_disable_ep() / g_free() while
the outer call stack still holds references to them, causing a
heap use-after-free.
Fix this by setting engaged_in_io on the device's mem_reentrancy_guard
around the processing in these non-MMIO entry points. This mirrors
the protection that EHCI, DWC2, and UHCI already have via
qemu_bh_new_guarded(). With the guard active, the memory subsystem's
automatic reentrancy check (system/memory.c) blocks DMA writes that
would dispatch into the device's own MMIO handlers.
CVE: CVE-2026-17588
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3926
Reported-by: XlabAI Team of Tencent Xuanwu Lab <xlabai@tencent.com>,
Guannan Wang <wgnbuaa@gmail.com>, Zhanpeng Liu <pkugenuine@gmail.com>,
Jiashuo Liang <761232680@qq.com>, Guancheng Li <lgcpku@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4195
Reported By: Ken Hsu and Royce Lu of Palo Alto Networks
Message-ID: <20260915112213.525283-1-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/usb/hcd-xhci.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index d342aa2739e..12f8ffece0f 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -456,9 +456,15 @@ static void xhci_mfwrap_timer(void *opaque)
{
XHCIState *xhci = opaque;
XHCIEvent wrap = { ER_MFINDEX_WRAP, CC_SUCCESS };
+ MemReentrancyGuard *guard = &xhci->parent.mem_reentrancy_guard;
+
+ assert(!guard->engaged_in_io);
+ guard->engaged_in_io = true;
xhci_event(xhci, &wrap, 0);
xhci_mfwrap_update(xhci);
+
+ guard->engaged_in_io = false;
}
static void xhci_die(XHCIState *xhci)
@@ -1086,7 +1092,14 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx,
static void xhci_ep_kick_timer(void *opaque)
{
XHCIEPContext *epctx = opaque;
+ MemReentrancyGuard *guard = &epctx->xhci->parent.mem_reentrancy_guard;
+
+ assert(!guard->engaged_in_io);
+ guard->engaged_in_io = true;
+
xhci_kick_epctx(epctx, 0);
+
+ guard->engaged_in_io = false;
}
static XHCIEPContext *xhci_alloc_epctx(XHCIState *xhci,
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 2/7] hw/usb/hcd-xhci: Limit DMA transfers to plain memory
2026-09-24 9:05 [PULL 0/7] XHCI fixes and drop Haiku VM Thomas Huth
2026-09-24 9:05 ` [PULL 1/7] hw/usb/hcd-xhci: Set reentrancy guard in timer functions (CVE-2026-17588) Thomas Huth
@ 2026-09-24 9:05 ` Thomas Huth
2026-09-24 9:12 ` Philippe Mathieu-Daudé
2026-09-24 9:05 ` [PULL 3/7] tests/vm: Drop the broken Haiku VM Thomas Huth
` (5 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Thomas Huth @ 2026-09-24 9:05 UTC (permalink / raw)
To: qemu-devel, Richard Henderson
XHCI is a complex controller, involving lots of descriptors that
are written and read via DMA transfers. As some recent bugs like
https://gitlab.com/qemu-project/qemu/-/work_items/3926 revealed,
this can sometimes be exploited from the guest side to crash or
stall QEMU.
The code currently does DMA writes with the MEMTXATTRS_UNSPECIFIED
attribute, i.e. the controller is allowed to write to other MMIO
regions, too. However, in normal operation, this should not be
necessary, all descriptors should reside in normal memory. So let's
decrease the attack surface a little bit and limit the DMA writes
to normal memory here.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20260915123242.549150-1-thuth@redhat.com>
---
hw/usb/hcd-xhci.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 12f8ffece0f..0ebf7a638a1 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -518,6 +518,7 @@ static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr,
int i;
uint32_t tmp[5];
uint32_t n = len / sizeof(uint32_t);
+ const MemTxAttrs memtx_attrs = { .memory = true };
assert((len % sizeof(uint32_t)) == 0);
assert(n <= ARRAY_SIZE(tmp));
@@ -525,8 +526,7 @@ static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr,
for (i = 0; i < n; i++) {
tmp[i] = cpu_to_le32(buf[i]);
}
- if (dma_memory_write(xhci->as, addr, tmp, len,
- MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) {
+ if (dma_memory_write(xhci->as, addr, tmp, len, memtx_attrs) != MEMTX_OK) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: DMA memory access failed!\n",
__func__);
xhci_die(xhci);
@@ -613,6 +613,7 @@ static void xhci_write_event(XHCIState *xhci, XHCIEvent *event, int v)
XHCIInterrupter *intr = &xhci->intr[v];
XHCITRB ev_trb;
dma_addr_t addr;
+ const MemTxAttrs memtx_attrs = { .memory = true };
ev_trb.parameter = cpu_to_le64(event->ptr);
ev_trb.status = cpu_to_le32(event->length | (event->ccode << 24));
@@ -629,7 +630,7 @@ static void xhci_write_event(XHCIState *xhci, XHCIEvent *event, int v)
addr = intr->er_start + TRB_SIZE*intr->er_ep_idx;
if (dma_memory_write(xhci->as, addr, &ev_trb, TRB_SIZE,
- MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) {
+ memtx_attrs) != MEMTX_OK) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: DMA memory access failed!\n",
__func__);
xhci_die(xhci);
@@ -2453,6 +2454,7 @@ static void xhci_detach_slot(XHCIState *xhci, USBPort *uport)
static TRBCCode xhci_get_port_bandwidth(XHCIState *xhci, uint64_t pctx)
{
dma_addr_t ctx;
+ const MemTxAttrs memtx_attrs = { .memory = true };
DPRINTF("xhci_get_port_bandwidth()\n");
@@ -2461,9 +2463,9 @@ static TRBCCode xhci_get_port_bandwidth(XHCIState *xhci, uint64_t pctx)
DPRINTF("xhci: bandwidth context at "DMA_ADDR_FMT"\n", ctx);
/* TODO: actually implement real values here. This is 80% for all ports. */
- if (stb_dma(xhci->as, ctx, 0, MEMTXATTRS_UNSPECIFIED) != MEMTX_OK ||
+ if (stb_dma(xhci->as, ctx, 0, memtx_attrs) != MEMTX_OK ||
dma_memory_set(xhci->as, ctx + 1, 80, xhci->numports,
- MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) {
+ memtx_attrs) != MEMTX_OK) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: DMA memory write failed!\n",
__func__);
return CC_TRB_ERROR;
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PULL 2/7] hw/usb/hcd-xhci: Limit DMA transfers to plain memory
2026-09-24 9:05 ` [PULL 2/7] hw/usb/hcd-xhci: Limit DMA transfers to plain memory Thomas Huth
@ 2026-09-24 9:12 ` Philippe Mathieu-Daudé
2026-09-24 9:21 ` Thomas Huth
0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-09-24 9:12 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Richard Henderson
On 24/9/26 11:05, Thomas Huth wrote:
> XHCI is a complex controller, involving lots of descriptors that
> are written and read via DMA transfers. As some recent bugs like
> https://gitlab.com/qemu-project/qemu/-/work_items/3926 revealed,
> this can sometimes be exploited from the guest side to crash or
> stall QEMU.
>
> The code currently does DMA writes with the MEMTXATTRS_UNSPECIFIED
> attribute, i.e. the controller is allowed to write to other MMIO
> regions, too. However, in normal operation, this should not be
> necessary, all descriptors should reside in normal memory. So let's
> decrease the attack surface a little bit and limit the DMA writes
> to normal memory here.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> Message-ID: <20260915123242.549150-1-thuth@redhat.com>
I had this tagged to review but low priority since RFC, anyhow LGTM.
> ---
> hw/usb/hcd-xhci.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PULL 2/7] hw/usb/hcd-xhci: Limit DMA transfers to plain memory
2026-09-24 9:12 ` Philippe Mathieu-Daudé
@ 2026-09-24 9:21 ` Thomas Huth
0 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2026-09-24 9:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, Richard Henderson
On 24/09/2026 11.12, Philippe Mathieu-Daudé wrote:
> On 24/9/26 11:05, Thomas Huth wrote:
>> XHCI is a complex controller, involving lots of descriptors that
>> are written and read via DMA transfers. As some recent bugs like
>> https://gitlab.com/qemu-project/qemu/-/work_items/3926 revealed,
>> this can sometimes be exploited from the guest side to crash or
>> stall QEMU.
>>
>> The code currently does DMA writes with the MEMTXATTRS_UNSPECIFIED
>> attribute, i.e. the controller is allowed to write to other MMIO
>> regions, too. However, in normal operation, this should not be
>> necessary, all descriptors should reside in normal memory. So let's
>> decrease the attack surface a little bit and limit the DMA writes
>> to normal memory here.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> Message-ID: <20260915123242.549150-1-thuth@redhat.com>
>
> I had this tagged to review but low priority since RFC, anyhow LGTM.
Thanks for your plans! ... I already assumed there would be nobody looking
at this. Anyhow, I think we likely won't find out whether there is any valid
scenario where the USB controller could write to MMIO space if we don't try
to include this, thus I added it to this PR to include it rather early than
late in this development cycle, so it gets enough testing coverage. If it
breaks anything, we can simply revert it again.
Thomas
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PULL 3/7] tests/vm: Drop the broken Haiku VM
2026-09-24 9:05 [PULL 0/7] XHCI fixes and drop Haiku VM Thomas Huth
2026-09-24 9:05 ` [PULL 1/7] hw/usb/hcd-xhci: Set reentrancy guard in timer functions (CVE-2026-17588) Thomas Huth
2026-09-24 9:05 ` [PULL 2/7] hw/usb/hcd-xhci: Limit DMA transfers to plain memory Thomas Huth
@ 2026-09-24 9:05 ` Thomas Huth
2026-09-24 9:05 ` [PULL 4/7] hw/usb/hcd-xhci: fix interval alignment after MFINDEX passes 2^32 Thomas Huth
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2026-09-24 9:05 UTC (permalink / raw)
To: qemu-devel, Richard Henderson; +Cc: Daniel P. Berrangé
The Haiku VM has bitrotted again: At least since Haiku r1beta6 has
been released, the image does not work anymore. I tried to update
the script to use the r1beta6 repositories instead, but after the
update, the Haiku kernel only crashes during reboot.
Given the fact that hardly anybody seems to use this image and
that it is more often broken than working, let's stop wasting our
time here and simply remove it.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20260917062615.783485-1-thuth@redhat.com>
---
tests/vm/haiku.x86_64 | 124 ------------------------------------------
1 file changed, 124 deletions(-)
delete mode 100755 tests/vm/haiku.x86_64
diff --git a/tests/vm/haiku.x86_64 b/tests/vm/haiku.x86_64
deleted file mode 100755
index 3ea48dc38a1..00000000000
--- a/tests/vm/haiku.x86_64
+++ /dev/null
@@ -1,124 +0,0 @@
-#!/usr/bin/env python3
-#
-# Haiku VM image
-#
-# Copyright 2020-2022 Haiku, Inc.
-#
-# Authors:
-# Alexander von Gluck IV <kallisti5@unixzen.com>
-#
-# This code is licensed under the GPL version 2 or later. See
-# the COPYING file in the top-level directory.
-#
-
-import os
-import re
-import sys
-import time
-import socket
-import subprocess
-import basevm
-
-VAGRANT_KEY_FILE = os.path.join(os.path.dirname(__file__),
- "..", "keys", "vagrant")
-
-VAGRANT_PUB_KEY_FILE = os.path.join(os.path.dirname(__file__),
- "..", "keys", "vagrant.pub")
-
-HAIKU_CONFIG = {
- 'cpu' : "max",
- 'machine' : 'pc',
- 'guest_user' : "vagrant",
- 'guest_pass' : "",
- 'root_user' : "vagrant",
- 'root_pass' : "",
- 'ssh_key_file' : VAGRANT_KEY_FILE,
- 'ssh_pub_key_file': VAGRANT_PUB_KEY_FILE,
- 'memory' : "4G",
- 'extra_args' : [],
- 'qemu_args' : "-device VGA",
- 'dns' : "",
- 'ssh_port' : 0,
- 'install_cmds' : "",
- 'boot_dev_type' : "block",
- 'ssh_timeout' : 1,
-}
-
-class HaikuVM(basevm.BaseVM):
- name = "haiku"
- arch = "x86_64"
-
- link = "https://app.vagrantup.com/haiku-os/boxes/r1beta4-x86_64/versions/20230114/providers/libvirt.box"
- csum = "6e72a2a470e03dbc3c5e808664e057bb4022b390dca88e4c7da6188f26f6a3c9"
-
- poweroff = "shutdown"
-
- requirements = [
- "devel:libbz2",
- "devel:libcapstone",
- "devel:libcurl",
- "devel:libfdt",
- "devel:libgcrypt",
- "devel:libgl",
- "devel:libglib_2.0",
- "devel:libgnutls",
- "devel:libgpg_error",
- "devel:libintl",
- "devel:libjpeg",
- "devel:liblzo2",
- "devel:libncursesw",
- "devel:libnettle",
- "devel:libpixman_1",
- "devel:libpng16",
- "devel:libsdl2_2.0",
- "devel:libslirp",
- "devel:libsnappy",
- "devel:libssh2",
- "devel:libtasn1",
- "devel:libusb_1.0",
- "devel:libz",
- "ninja",
- "pip",
- "tomli_python310",
- "wheel_python310",
- "setuptools_python310",
- ]
-
- BUILD_SCRIPT = """
- set -e;
- rm -rf /tmp/qemu-test.*
- cd $(mktemp -d /tmp/qemu-test.XXXXXX);
- mkdir src build; cd src;
- tar -xf /dev/disk/virtual/virtio_block/1/raw;
- mkdir -p /usr/bin
- ln -s /boot/system/bin/env /usr/bin/env
- cd ../build
- ../src/configure {configure_opts};
- make --output-sync -j{jobs} {target} {verbose};
- """
-
- def build_image(self, img):
- self.print_step("Downloading disk image")
- tarball = self._download_with_cache(self.link, sha256sum=self.csum)
-
- self.print_step("Extracting disk image")
-
- subprocess.check_call(["tar", "xzf", tarball, "box.img", "-O"],
- stdout=open(img, 'wb'))
-
- self.print_step("Preparing disk image")
- self.boot(img)
-
- # Wait for ssh to be available.
- self.wait_ssh(wait_root=True, cmd="exit 0")
-
- # Install packages
- self.ssh_root("echo yes | pkgman add-repo https://eu.hpkg.haiku-os.org/haiku/r1beta5/$(getarch)/current")
- self.ssh_root("echo yes | pkgman add-repo https://eu.hpkg.haiku-os.org/haikuports/r1beta5/$(getarch)/current")
- self.ssh_root("pkgman install -y %s" % " ".join(self.requirements))
- self.graceful_shutdown()
-
- self.print_step("All done")
-
-if __name__ == "__main__":
- sys.exit(basevm.main(HaikuVM, config=HAIKU_CONFIG))
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 4/7] hw/usb/hcd-xhci: fix interval alignment after MFINDEX passes 2^32
2026-09-24 9:05 [PULL 0/7] XHCI fixes and drop Haiku VM Thomas Huth
` (2 preceding siblings ...)
2026-09-24 9:05 ` [PULL 3/7] tests/vm: Drop the broken Haiku VM Thomas Huth
@ 2026-09-24 9:05 ` Thomas Huth
2026-09-24 9:05 ` [PULL 5/7] hw/usb/hcd-xhci: don't assert on NAK when retrying an isoch transfer Thomas Huth
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2026-09-24 9:05 UTC (permalink / raw)
To: qemu-devel, Richard Henderson
Cc: Junjie Cao, qemu-stable, Philippe Mathieu-Daudé
From: Junjie Cao <junjie.cao@intel.com>
epctx->interval is an unsigned int, so ~(epctx->interval - 1) is a
32-bit mask that is zero-extended when and-ed with the 64-bit microframe
index. Once mfindex no longer fits in 32 bits (2^32 * 125us, about 6.2
days after the controller was started), asap loses its upper half and
always compares below mfindex. Isoch TDs with SIA are then run at once
instead of at the next interval boundary. xhci_calc_intr_kick() has the
same expression.
Use ROUND_UP(), which builds the mask in the type of mfindex. The
interval is always a power of two.
The reporter of #3973 also saw the symptom with UHCI. This change does
not explain that.
Fixes: 3d1396842d ("xhci: iso xfer support")
Fixes: 4d7a81c06f ("xhci: emulate intr endpoint intervals correctly")
Link: https://gitlab.com/qemu-project/qemu/-/issues/3973
Cc: qemu-stable@nongnu.org
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Message-ID: <52c9f935428f2cfc65e4dee7e37638fcb965c6b2.1789968699.git.junjie.cao@intel.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/usb/hcd-xhci.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 0ebf7a638a1..2f82b76272d 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1756,8 +1756,7 @@ static int xhci_fire_ctl_transfer(XHCIState *xhci, XHCITransfer *xfer)
static void xhci_calc_intr_kick(XHCIState *xhci, XHCITransfer *xfer,
XHCIEPContext *epctx, uint64_t mfindex)
{
- uint64_t asap = ((mfindex + epctx->interval - 1) &
- ~(epctx->interval-1));
+ uint64_t asap = ROUND_UP(mfindex, epctx->interval);
uint64_t kick = epctx->mfindex_last + epctx->interval;
assert(epctx->interval != 0);
@@ -1768,8 +1767,7 @@ static void xhci_calc_iso_kick(XHCIState *xhci, XHCITransfer *xfer,
XHCIEPContext *epctx, uint64_t mfindex)
{
if (xfer->trbs[0].control & TRB_TR_SIA) {
- uint64_t asap = ((mfindex + epctx->interval - 1) &
- ~(epctx->interval-1));
+ uint64_t asap = ROUND_UP(mfindex, epctx->interval);
if (asap >= epctx->mfindex_last &&
asap <= epctx->mfindex_last + epctx->interval * 4) {
xfer->mfindex_kick = epctx->mfindex_last + epctx->interval;
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 5/7] hw/usb/hcd-xhci: don't assert on NAK when retrying an isoch transfer
2026-09-24 9:05 [PULL 0/7] XHCI fixes and drop Haiku VM Thomas Huth
` (3 preceding siblings ...)
2026-09-24 9:05 ` [PULL 4/7] hw/usb/hcd-xhci: fix interval alignment after MFINDEX passes 2^32 Thomas Huth
@ 2026-09-24 9:05 ` Thomas Huth
2026-09-24 9:05 ` [PULL 6/7] tests/qtest/usb-hcd-xhci: test isoch pacing with MFINDEX above 2^32 Thomas Huth
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2026-09-24 9:05 UTC (permalink / raw)
To: qemu-devel, Richard Henderson; +Cc: Junjie Cao, Feifan Qian, qemu-stable
From: Junjie Cao <junjie.cao@intel.com>
The endpoint type in the xHCI endpoint context comes from the guest and
is not checked against the device. A guest can configure the interrupt
IN endpoint of usb-kbd as Isoch IN. The idle HID endpoint NAKs, and as
soon as the transfer goes through the retry path in xhci_kick_epctx()
it hits
assert(xfer->packet.status != USB_RET_NAK);
No device model NAKs on an isoch endpoint, so this only triggers with a
mismatched endpoint type.
The two retry branches differ only in what they do on NAK: the isoch one
asserts, the other keeps the transfer pending. Merge them.
Fixes: 3d1396842d ("xhci: iso xfer support")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3886
Reported-by: Feifan Qian <bea1e@proton.me>
Cc: qemu-stable@nongnu.org
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-ID: <44227b05b22064a22f4c135e025ff0b1d838b9e3.1789968699.git.junjie.cao@intel.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/usb/hcd-xhci.c | 27 ++++++++-------------------
1 file changed, 8 insertions(+), 19 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 2f82b76272d..376bc00264d 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -1926,26 +1926,15 @@ static void xhci_kick_epctx(XHCIEPContext *epctx, unsigned int streamid)
xfer->timed_xfer = 0;
xfer->running_retry = 1;
}
- if (xfer->iso_xfer) {
- /* retry iso transfer */
- if (xhci_setup_packet(xfer) < 0) {
- return;
- }
- usb_handle_packet(xfer->packet.ep->dev, &xfer->packet);
- assert(xfer->packet.status != USB_RET_NAK);
- xhci_try_complete_packet(xfer);
- } else {
- /* retry nak'ed transfer */
- if (xhci_setup_packet(xfer) < 0) {
- return;
- }
- usb_handle_packet(xfer->packet.ep->dev, &xfer->packet);
- if (xfer->packet.status == USB_RET_NAK) {
- xhci_xfer_unmap(xfer);
- return;
- }
- xhci_try_complete_packet(xfer);
+ if (xhci_setup_packet(xfer) < 0) {
+ return;
+ }
+ usb_handle_packet(xfer->packet.ep->dev, &xfer->packet);
+ if (xfer->packet.status == USB_RET_NAK) {
+ xhci_xfer_unmap(xfer);
+ return;
}
+ xhci_try_complete_packet(xfer);
assert(!xfer->running_retry);
if (xfer->complete) {
/* update ring dequeue ptr */
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 6/7] tests/qtest/usb-hcd-xhci: test isoch pacing with MFINDEX above 2^32
2026-09-24 9:05 [PULL 0/7] XHCI fixes and drop Haiku VM Thomas Huth
` (4 preceding siblings ...)
2026-09-24 9:05 ` [PULL 5/7] hw/usb/hcd-xhci: don't assert on NAK when retrying an isoch transfer Thomas Huth
@ 2026-09-24 9:05 ` Thomas Huth
2026-09-24 9:05 ` [PULL 7/7] tests/qtest/usb-hcd-xhci: test isoch endpoint type mismatch Thomas Huth
2026-09-24 15:24 ` [PULL 0/7] XHCI fixes and drop Haiku VM Richard Henderson
7 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2026-09-24 9:05 UTC (permalink / raw)
To: qemu-devel, Richard Henderson; +Cc: Junjie Cao, Philippe Mathieu-Daudé
From: Junjie Cao <junjie.cao@intel.com>
Add the minimum needed to drive a transfer ring from the test. Use it to
queue an isoch TD with SIA on usb-audio after stepping the clock past
2^32 microframes, and check that the TD waits for the next interval
boundary. Before the ROUND_UP() change it completed as soon as the
doorbell was rung.
The machine is started with pit=off. With the i8254 present the clock
step takes 26s in an ASan build instead of well under a second.
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
Acked-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Message-ID: <eb57b4b9bd78f08d248fa1ae7ed5d2d5958c9ef4.1789968699.git.junjie.cao@intel.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/qtest/usb-hcd-xhci-test.c | 283 ++++++++++++++++++++++++++++++++
1 file changed, 283 insertions(+)
diff --git a/tests/qtest/usb-hcd-xhci-test.c b/tests/qtest/usb-hcd-xhci-test.c
index b58fa1e2dae..61eca37b157 100644
--- a/tests/qtest/usb-hcd-xhci-test.c
+++ b/tests/qtest/usb-hcd-xhci-test.c
@@ -10,8 +10,68 @@
#include "qemu/osdep.h"
#include "libqtest-single.h"
#include "libqos/usb.h"
+#include "libqos/malloc-pc.h"
#include "qobject/qdict.h"
+/* capability registers */
+#define XHCI_CAPLENGTH 0x00
+#define XHCI_HCSPARAMS1 0x04
+#define XHCI_DBOFF 0x14
+#define XHCI_RTSOFF 0x18
+/* operational registers */
+#define XHCI_USBCMD 0x00
+#define XHCI_USBSTS 0x04
+#define XHCI_CRCR 0x18
+#define XHCI_DCBAAP 0x30
+#define XHCI_CONFIG 0x38
+#define XHCI_PORTSC(n) (0x400 + 0x10 * (n))
+/* interrupter 0, relative to the runtime registers */
+#define XHCI_ERSTSZ 0x28
+#define XHCI_ERSTBA 0x30
+#define XHCI_ERDP 0x38
+
+#define USBCMD_RS (1 << 0)
+#define USBCMD_HCRST (1 << 1)
+#define USBSTS_HCH (1 << 0)
+#define PORTSC_CCS (1 << 0)
+#define PORTSC_PR (1 << 4)
+#define PORTSC_PP (1 << 9)
+#define CRCR_RCS (1 << 0)
+#define ERDP_EHB (1 << 3)
+
+#define TRB_C (1 << 0)
+#define TRB_TR_IOC (1 << 5)
+#define TRB_TR_SIA (1U << 31)
+#define TRB_TYPE(t) ((t) << 10)
+#define TRB_GET_TYPE(control) (((control) >> 10) & 0x3f)
+#define TRB_GET_CCODE(status) ((status) >> 24)
+#define TRB_GET_SLOT(control) ((control) >> 24)
+
+#define TR_ISOCH 5
+#define CR_ENABLE_SLOT 9
+#define CR_ADDRESS_DEVICE 11
+#define CR_CONFIGURE_ENDPOINT 12
+#define ER_TRANSFER 32
+#define ER_COMMAND_COMPLETE 33
+#define CC_SUCCESS 1
+
+#define EP_TYPE_ISOCH_OUT 1
+#define EP_TYPE_CONTROL 4
+
+#define XHCI_RING_TRBS 64
+#define XHCI_MICROFRAME_NS 125000
+
+typedef struct XHCITest {
+ QTestState *qts;
+ QGuestAllocator alloc;
+ QPCIBus *bus;
+ struct qhc hc;
+ uint32_t oper, runtime, doorbell;
+ uint64_t cmd_ring, event_ring, input_ctx;
+ unsigned int cmd_idx, event_idx;
+ unsigned int port, slot;
+} XHCITest;
+
static void wait_device_deleted_event(QTestState *qtest, const char *id)
{
QDict *resp, *data;
@@ -109,6 +169,227 @@ static void test_usb_ccid_hotplug(void)
qtest_qmp_device_del(qts, "ccid");
}
+static uint32_t xhci_readl(XHCITest *x, uint32_t off)
+{
+ return qpci_io_readl(x->hc.dev, x->hc.bar, off);
+}
+
+static void xhci_writel(XHCITest *x, uint32_t off, uint32_t val)
+{
+ qpci_io_writel(x->hc.dev, x->hc.bar, off, val);
+}
+
+static void xhci_writeq(XHCITest *x, uint32_t off, uint64_t val)
+{
+ xhci_writel(x, off, val);
+ xhci_writel(x, off + 4, val >> 32);
+}
+
+static uint64_t xhci_alloc_page(XHCITest *x)
+{
+ uint64_t addr = guest_alloc(&x->alloc, 0x1000);
+
+ qtest_memset(x->qts, addr, 0, 0x1000);
+ return addr;
+}
+
+static void xhci_write_trb(XHCITest *x, uint64_t addr, uint64_t parameter,
+ uint32_t status, uint32_t control)
+{
+ qtest_writeq(x->qts, addr, parameter);
+ qtest_writel(x->qts, addr + 8, status);
+ qtest_writel(x->qts, addr + 12, control);
+}
+
+/* Fetch the next event if there is one. Does not advance the clock. */
+static bool xhci_next_event(XHCITest *x, uint32_t *status, uint32_t *control)
+{
+ uint64_t addr = x->event_ring + 16 * x->event_idx;
+ uint32_t c = qtest_readl(x->qts, addr + 12);
+
+ if (!(c & TRB_C)) {
+ return false;
+ }
+ if (status) {
+ *status = qtest_readl(x->qts, addr + 8);
+ }
+ if (control) {
+ *control = c;
+ }
+ x->event_idx++;
+ g_assert_cmpuint(x->event_idx, <, XHCI_RING_TRBS);
+ xhci_writeq(x, x->runtime + XHCI_ERDP, (addr + 16) | ERDP_EHB);
+ return true;
+}
+
+static unsigned int xhci_command(XHCITest *x, uint64_t parameter,
+ uint32_t control)
+{
+ uint32_t status;
+
+ g_assert_cmpuint(x->cmd_idx, <, XHCI_RING_TRBS);
+ xhci_write_trb(x, x->cmd_ring + 16 * x->cmd_idx++, parameter, 0,
+ control | TRB_C);
+ xhci_writel(x, x->doorbell, 0);
+
+ g_assert_true(xhci_next_event(x, &status, &control));
+ g_assert_cmpuint(TRB_GET_TYPE(control), ==, ER_COMMAND_COMPLETE);
+ g_assert_cmpuint(TRB_GET_CCODE(status), ==, CC_SUCCESS);
+ return TRB_GET_SLOT(control);
+}
+
+/*
+ * Start qemu-xhci with one USB device, run the controller and bring the
+ * device to the Addressed state.
+ */
+static void xhci_test_start(XHCITest *x, const char *usb_device)
+{
+ uint64_t dcbaa, erst, ep0_ring;
+ unsigned int maxports;
+
+ memset(x, 0, sizeof(*x));
+ /* pit=off: a long clock step would run the i8254 timer all the way */
+ x->qts = qtest_initf("-machine pc,pit=off -nodefaults "
+ "-device qemu-xhci,id=xhci,addr=04.0 %s", usb_device);
+ pc_alloc_init(&x->alloc, x->qts, ALLOC_NO_FLAGS);
+ x->bus = qpci_new_pc(x->qts, NULL);
+ qusb_pci_init_one(x->bus, &x->hc, QPCI_DEVFN(4, 0), 0);
+
+ x->oper = qpci_io_readb(x->hc.dev, x->hc.bar, XHCI_CAPLENGTH);
+ x->runtime = xhci_readl(x, XHCI_RTSOFF) & ~0x1f;
+ x->doorbell = xhci_readl(x, XHCI_DBOFF) & ~0x3;
+ maxports = xhci_readl(x, XHCI_HCSPARAMS1) >> 24;
+
+ xhci_writel(x, x->oper + XHCI_USBCMD, USBCMD_HCRST);
+ g_assert_false(xhci_readl(x, x->oper + XHCI_USBCMD) & USBCMD_HCRST);
+
+ dcbaa = xhci_alloc_page(x);
+ erst = xhci_alloc_page(x);
+ x->cmd_ring = xhci_alloc_page(x);
+ x->event_ring = xhci_alloc_page(x);
+ x->input_ctx = xhci_alloc_page(x);
+
+ xhci_writel(x, x->oper + XHCI_CONFIG, 1);
+ xhci_writeq(x, x->oper + XHCI_DCBAAP, dcbaa);
+ qtest_writeq(x->qts, erst, x->event_ring);
+ qtest_writel(x->qts, erst + 8, XHCI_RING_TRBS);
+ xhci_writel(x, x->runtime + XHCI_ERSTSZ, 1);
+ xhci_writeq(x, x->runtime + XHCI_ERSTBA, erst);
+ xhci_writeq(x, x->runtime + XHCI_ERDP, x->event_ring | ERDP_EHB);
+ xhci_writeq(x, x->oper + XHCI_CRCR, x->cmd_ring | CRCR_RCS);
+ xhci_writel(x, x->oper + XHCI_USBCMD, USBCMD_RS);
+ g_assert_false(xhci_readl(x, x->oper + XHCI_USBSTS) & USBSTS_HCH);
+
+ for (x->port = 0; x->port < maxports; x->port++) {
+ if (xhci_readl(x, x->oper + XHCI_PORTSC(x->port)) & PORTSC_CCS) {
+ break;
+ }
+ }
+ g_assert_cmpuint(x->port, <, maxports);
+ xhci_writel(x, x->oper + XHCI_PORTSC(x->port), PORTSC_PP | PORTSC_PR);
+ while (xhci_next_event(x, NULL, NULL)) {
+ /* drop the port status change events */
+ }
+
+ x->slot = xhci_command(x, 0, TRB_TYPE(CR_ENABLE_SLOT));
+ qtest_writeq(x->qts, dcbaa + 8 * x->slot, xhci_alloc_page(x));
+
+ /* input control context: add slot and ep0 */
+ qtest_writel(x->qts, x->input_ctx + 0x04, 0x3);
+ /* slot context: one context entry, root hub port */
+ qtest_writel(x->qts, x->input_ctx + 0x20, 1 << 27);
+ qtest_writel(x->qts, x->input_ctx + 0x24, (x->port + 1) << 16);
+ /* ep0 context */
+ ep0_ring = xhci_alloc_page(x);
+ qtest_writel(x->qts, x->input_ctx + 0x44,
+ (64 << 16) | (EP_TYPE_CONTROL << 3));
+ qtest_writeq(x->qts, x->input_ctx + 0x48, ep0_ring | 1);
+ xhci_command(x, x->input_ctx,
+ TRB_TYPE(CR_ADDRESS_DEVICE) | (x->slot << 24));
+}
+
+/* Returns the address of the transfer ring. */
+static uint64_t xhci_configure_ep(XHCITest *x, unsigned int epid,
+ unsigned int type, unsigned int interval,
+ unsigned int max_packet)
+{
+ uint64_t ring = xhci_alloc_page(x);
+ uint64_t epctx = x->input_ctx + 0x20 * (epid + 1);
+
+ qtest_memset(x->qts, x->input_ctx, 0, 0x1000);
+ qtest_writel(x->qts, x->input_ctx + 0x04, (1 << epid) | 1);
+ qtest_writel(x->qts, x->input_ctx + 0x20, epid << 27);
+ qtest_writel(x->qts, x->input_ctx + 0x24, (x->port + 1) << 16);
+ qtest_writel(x->qts, epctx + 0x00, interval << 16);
+ qtest_writel(x->qts, epctx + 0x04, (max_packet << 16) | (type << 3));
+ qtest_writeq(x->qts, epctx + 0x08, ring | 1);
+ xhci_command(x, x->input_ctx,
+ TRB_TYPE(CR_CONFIGURE_ENDPOINT) | (x->slot << 24));
+ return ring;
+}
+
+static void xhci_test_end(XHCITest *x)
+{
+ g_free(x->hc.dev);
+ qpci_free_pc(x->bus);
+ alloc_destroy(&x->alloc);
+ qtest_quit(x->qts);
+}
+
+static bool xhci_test_supported(const char *usb_device)
+{
+ const char *arch = qtest_get_arch();
+
+ if (strcmp(arch, "i386") != 0 && strcmp(arch, "x86_64") != 0) {
+ g_test_skip("Test only runs on x86 (pc machine)");
+ return false;
+ }
+ if (!qtest_has_device("qemu-xhci") || !qtest_has_device(usb_device)) {
+ g_test_skip("Devices not available");
+ return false;
+ }
+ return true;
+}
+
+/*
+ * An isoch TD with SIA set is run at the next interval boundary. That has to
+ * hold once the microframe index no longer fits in 32 bits as well.
+ */
+static void test_xhci_isoch_mfindex_32bit(void)
+{
+ const unsigned int interval = 6;
+ uint32_t control;
+ uint64_t ring;
+ XHCITest x;
+
+ if (!xhci_test_supported("usb-audio")) {
+ return;
+ }
+
+ xhci_test_start(&x, "-audiodev none,id=snd0 "
+ "-device usb-audio,audiodev=snd0");
+ ring = xhci_configure_ep(&x, 2, EP_TYPE_ISOCH_OUT, interval, 64);
+
+ /* Go past 2^32 microframes and stop off an interval boundary. */
+ qtest_clock_step(x.qts, (1ULL << 32) * XHCI_MICROFRAME_NS);
+ qtest_clock_step(x.qts, 5 * XHCI_MICROFRAME_NS);
+
+ xhci_write_trb(&x, ring, xhci_alloc_page(&x), 64,
+ TRB_TYPE(TR_ISOCH) | TRB_TR_SIA | TRB_TR_IOC | TRB_C);
+ xhci_writel(&x, x.doorbell + 4 * x.slot, 2);
+ g_assert_false(xhci_next_event(&x, NULL, NULL));
+
+ /*
+ * The streaming interface has not been enabled, so usb-audio stalls the
+ * TD. What matters is when that happens.
+ */
+ qtest_clock_step(x.qts, XHCI_MICROFRAME_NS << interval);
+ g_assert_true(xhci_next_event(&x, NULL, &control));
+ g_assert_cmpuint(TRB_GET_TYPE(control), ==, ER_TRANSFER);
+
+ xhci_test_end(&x);
+}
+
int main(int argc, char **argv)
{
int ret;
@@ -123,6 +404,8 @@ int main(int argc, char **argv)
if (qtest_has_device("usb-ccid")) {
qtest_add_func("/xhci/pci/hotplug/usb-ccid", test_usb_ccid_hotplug);
}
+ qtest_add_func("/xhci/pci/isoch/mfindex-32bit",
+ test_xhci_isoch_mfindex_32bit);
qtest_start("-device nec-usb-xhci,id=xhci"
" -drive id=drive0,if=none,file=null-co://,"
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 7/7] tests/qtest/usb-hcd-xhci: test isoch endpoint type mismatch
2026-09-24 9:05 [PULL 0/7] XHCI fixes and drop Haiku VM Thomas Huth
` (5 preceding siblings ...)
2026-09-24 9:05 ` [PULL 6/7] tests/qtest/usb-hcd-xhci: test isoch pacing with MFINDEX above 2^32 Thomas Huth
@ 2026-09-24 9:05 ` Thomas Huth
2026-09-24 15:24 ` [PULL 0/7] XHCI fixes and drop Haiku VM Richard Henderson
7 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2026-09-24 9:05 UTC (permalink / raw)
To: qemu-devel, Richard Henderson; +Cc: Junjie Cao
From: Junjie Cao <junjie.cao@intel.com>
Configure the interrupt IN endpoint of usb-kbd as Isoch IN and queue one
TD. The TD is deferred by a microframe, the kick timer retries it,
usb-kbd NAKs, and a second doorbell retries it again. The TD has to stay
pending both times and the controller has to keep running.
Without the fix QEMU aborts in the timer retry. The endpoint setup is
taken from the reproducer attached to #3886.
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
Message-ID: <c1166d1b83db5a507035d6fc27b1b552d4a95b0e.1789968699.git.junjie.cao@intel.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/qtest/usb-hcd-xhci-test.c | 35 +++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/tests/qtest/usb-hcd-xhci-test.c b/tests/qtest/usb-hcd-xhci-test.c
index 61eca37b157..2e173af7506 100644
--- a/tests/qtest/usb-hcd-xhci-test.c
+++ b/tests/qtest/usb-hcd-xhci-test.c
@@ -33,6 +33,7 @@
#define USBCMD_RS (1 << 0)
#define USBCMD_HCRST (1 << 1)
#define USBSTS_HCH (1 << 0)
+#define USBSTS_HCE (1 << 12)
#define PORTSC_CCS (1 << 0)
#define PORTSC_PR (1 << 4)
#define PORTSC_PP (1 << 9)
@@ -57,6 +58,7 @@
#define EP_TYPE_ISOCH_OUT 1
#define EP_TYPE_CONTROL 4
+#define EP_TYPE_ISOCH_IN 5
#define XHCI_RING_TRBS 64
#define XHCI_MICROFRAME_NS 125000
@@ -390,6 +392,37 @@ static void test_xhci_isoch_mfindex_32bit(void)
xhci_test_end(&x);
}
+/*
+ * The endpoint type in the endpoint context is whatever the guest says. Tell
+ * the controller that the interrupt endpoint of usb-kbd is isoch. The idle
+ * keyboard NAKs, and the TD has to stay pending when first the kick timer and
+ * then a doorbell retry it.
+ */
+static void test_xhci_isoch_ep_type_mismatch(void)
+{
+ uint64_t ring;
+ XHCITest x;
+
+ if (!xhci_test_supported("usb-kbd")) {
+ return;
+ }
+
+ xhci_test_start(&x, "-device usb-kbd");
+ ring = xhci_configure_ep(&x, 3, EP_TYPE_ISOCH_IN, 0, 8);
+
+ xhci_write_trb(&x, ring, xhci_alloc_page(&x), 8,
+ TRB_TYPE(TR_ISOCH) | TRB_TR_SIA | TRB_TR_IOC | TRB_C);
+ xhci_writel(&x, x.doorbell + 4 * x.slot, 3);
+ qtest_clock_step(x.qts, 2 * XHCI_MICROFRAME_NS);
+ xhci_writel(&x, x.doorbell + 4 * x.slot, 3);
+
+ g_assert_false(xhci_next_event(&x, NULL, NULL));
+ g_assert_cmphex(xhci_readl(&x, x.oper + XHCI_USBSTS) &
+ (USBSTS_HCH | USBSTS_HCE), ==, 0);
+
+ xhci_test_end(&x);
+}
+
int main(int argc, char **argv)
{
int ret;
@@ -406,6 +439,8 @@ int main(int argc, char **argv)
}
qtest_add_func("/xhci/pci/isoch/mfindex-32bit",
test_xhci_isoch_mfindex_32bit);
+ qtest_add_func("/xhci/pci/isoch/ep-type-mismatch",
+ test_xhci_isoch_ep_type_mismatch);
qtest_start("-device nec-usb-xhci,id=xhci"
" -drive id=drive0,if=none,file=null-co://,"
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PULL 0/7] XHCI fixes and drop Haiku VM
2026-09-24 9:05 [PULL 0/7] XHCI fixes and drop Haiku VM Thomas Huth
` (6 preceding siblings ...)
2026-09-24 9:05 ` [PULL 7/7] tests/qtest/usb-hcd-xhci: test isoch endpoint type mismatch Thomas Huth
@ 2026-09-24 15:24 ` Richard Henderson
7 siblings, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2026-09-24 15:24 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
On 9/24/26 02:05, Thomas Huth wrote:
> Hi Richard!
>
> The following changes since commit 7074591d7954876951f84c15b994a43251d5a3c1:
>
> Merge tag 'qom-qdev-20260922' ofhttps://gitlab.com/mcayland-ntx/qemu into staging (2026-09-22 07:56:00 -0700)
>
> are available in the Git repository at:
>
> https://gitlab.com/thuth/qemu.git tags/pull-request-2026-09-24
>
> for you to fetch changes up to 1288b0869c082690c04c81ac940c126fb399592b:
>
> tests/qtest/usb-hcd-xhci: test isoch endpoint type mismatch (2026-09-23 15:49:32 +0200)
>
> ----------------------------------------------------------------
> * Some fixes for the USB XHCI controller emulation
> * Drop the broken Haiku VM image
Applied, thanks.
r~
^ permalink raw reply [flat|nested] 11+ messages in thread