Skip to content

Commit

Permalink
hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X
Browse files Browse the repository at this point in the history
Per xHCI spec v1.2 chapter 4.17.5 page 296:

  If MSI or MSI-X interrupts are enabled, Interrupt Pending (IP)
  shall be cleared automatically when the PCI dword write generated
  by the interrupt assertion is complete.

Currently QEMU does not clear the IP flag in the MSI / MSI-X mode.
This causes subsequent spurious interrupt to be delivered to guests.
To solve this, we change the xhci intr_raise() hook routine to have
a bool return value that is passed to its caller (the xhci core),
with true indicating that IP should be self-cleared.

Fixes: 62c6ae0 ("xhci: Initial xHCI implementation")
Fixes: 4c47f80 ("xhci: add msix support")
Signed-off-by: Ruimei Yan <[email protected]>
[bmeng: move IP clear codes from xhci pci to xhci core]
Signed-off-by: Bin Meng <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Gerd Hoffmann <[email protected]>
  • Loading branch information
ruimei authored and kraxel committed May 28, 2021
1 parent 3c6151c commit fc967aa
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 7 deletions.
8 changes: 5 additions & 3 deletions hw/usb/hcd-xhci-pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ static void xhci_pci_intr_update(XHCIState *xhci, int n, bool enable)
}
}

static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
static bool xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
{
XHCIPciState *s = container_of(xhci, XHCIPciState, xhci);
PCIDevice *pci_dev = PCI_DEVICE(s);
Expand All @@ -70,13 +70,15 @@ static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)

if (msix_enabled(pci_dev) && level) {
msix_notify(pci_dev, n);
return;
return true;
}

if (msi_enabled(pci_dev) && level) {
msi_notify(pci_dev, n);
return;
return true;
}

return false;
}

static void xhci_pci_reset(DeviceState *dev)
Expand Down
4 changes: 3 additions & 1 deletion hw/usb/hcd-xhci-sysbus.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@
#include "hw/acpi/aml-build.h"
#include "hw/irq.h"

static void xhci_sysbus_intr_raise(XHCIState *xhci, int n, bool level)
static bool xhci_sysbus_intr_raise(XHCIState *xhci, int n, bool level)
{
XHCISysbusState *s = container_of(xhci, XHCISysbusState, xhci);

qemu_set_irq(s->irq[n], level);

return false;
}

void xhci_sysbus_reset(DeviceState *dev)
Expand Down
8 changes: 6 additions & 2 deletions hw/usb/hcd-xhci.c
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,9 @@ static void xhci_intr_update(XHCIState *xhci, int v)
level = 1;
}
if (xhci->intr_raise) {
xhci->intr_raise(xhci, 0, level);
if (xhci->intr_raise(xhci, 0, level)) {
xhci->intr[0].iman &= ~IMAN_IP;
}
}
}
if (xhci->intr_update) {
Expand Down Expand Up @@ -579,7 +581,9 @@ static void xhci_intr_raise(XHCIState *xhci, int v)
return;
}
if (xhci->intr_raise) {
xhci->intr_raise(xhci, v, true);
if (xhci->intr_raise(xhci, v, true)) {
xhci->intr[v].iman &= ~IMAN_IP;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion hw/usb/hcd-xhci.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ typedef struct XHCIState {
uint32_t flags;
uint32_t max_pstreams_mask;
void (*intr_update)(XHCIState *s, int n, bool enable);
void (*intr_raise)(XHCIState *s, int n, bool level);
bool (*intr_raise)(XHCIState *s, int n, bool level);
DeviceState *hostOpaque;

/* Operational Registers */
Expand Down

0 comments on commit fc967aa

Please sign in to comment.