[v3,0/6] hw/usb/hcd-xhci: Fixes, improvements, and macOS workaround

Message

Phil Dennis-Jordan Dec. 27, 2024, 12:13 p.m. UTC

For a while now, I've been chasing the problem of macOS's XHCI guest driver not
working properly with QEMU's PCI XHCI controller when MSI-X is unavailable.
I've finally figured out the cause, and I think an acceptable solution. I've
explained the problem and quoted the relevant sections of the XHCI spec in more
detail in the linked GitLab issue:

https://gitlab.com/qemu-project/qemu/-/issues/2705

Essentially, the macOS driver attempts to use XHCI event rings 1 and 2 even
when there is only a single pin-based interrupt available. The interrupts for
rings 1 and 2 are dropped, and so events are only handled after a timeout.
The driver appears to expect the device to act as if interrupter mapping was
not supported - the spec only mentions that interrupter mapping should be
disabled if only one **interrupter** is enabled, not one **interrupt**,
although there is some ambiguity in the spec's wording around enabling and
disabling interrupters.

After feedback to my initial RFC submission and discovering some more unhandled
edge cases, I've now split this up into 6 different patches.

Ultimately, for macOS to be able to drive the XHCI controller with MSI(-X)
disabled, we need to disable interrupter mapping (in the sense of XHCI spec
section 4.17.1) when using a pin-based interrupt.

 1. Fixes an assertion failure crash when XHCI attempts to raise an interrupt
    on a MSI vector higher than the maximum allocated. It turns out the
    spec says the MSI vector needs to be computed from the interrupter
    number by modulo arithmetic, so this patch implements that behaviour.
 2. Moves the msi/msix toggles from the NEC XHCI controller to the generic
    hci-xhci-pci superclass for consistency. This makes testing with MSI-X
    and/or MSI disabled easier when using the qemu-xhci device variant.
 3. Implements interrupter mapping disabling as per XHCI spec, when numintrs==1.
 4. Switches from tracing to LOG_UNIMP and LOG_GUEST_ERROR for certain
    cases of unhandled MMIO. (This is not comprehensive, but came up in
    review of one of Nicholas Piggin's XHCI patch sets.)
 5. Add a new boolean property to hcd-xhci-pci, "conditional-intr-mapping",
    which defaults to false, retaining existing behaviour. When set to true,
    additional constraints for enabling interrupter mapping are enabled,
    so it is disabled in pin-based mode. This works around the macOS guest
    driver quirks.
 6. Enables the "conditional-intr-mapping" property in the VMApple machine
    type, which does not support MSI-X or MSI and has previously suffered
    from the macOS guest driver quirk.

Of course, patch 6 can only be applied once the VMApple patch set is also
applied: https://patchew.org/QEMU/[e-mail]/

Patches 2 and 4 are optional for the purposes of fixing the issue I
set out to fix, but seem sensible enough to include in this series.

You can reproduce the problems being fixed as follows:

 * Assertion failure crash fixed in patch 1: Use a x86-64 VM with macOS guest
   and machine type Q35. For USB, use: -device nec-usb-xhci,msix=off
   QEMU will crash with a failed assertion, "vector < nr_vectors" on guest boot.
 * macOS guest not driving the XHCI controller correctly with pin-based
   interrupts: Either as above but with -device nec-usb-xhci,msix=off,msi=off
   or by following the instructions to boot aarch64 macOS 12 on the VMApple
   machine type.

History:

RFC -> v1:

 * Gated conditional interrupter mapping support behind a property, enabled
   that property in the VMApple machine type.
 * Added patch to fix the MSI vector assertion failure.
 * Moved msi and msix properties from NEC XHCI controller to generic xhci-pci
   superclass as that also seems useful.
 * Broke the workaround up into 2 patches, one for mapping disabling required
   by the standard, and one for the conditional disabling workaround.

v1 -> v2:

 * 1/6: Switch to modulo arithmetic for MSI vector number, as per spec.
 * 6/6: Set the "conditional-intr-mapping" property via compat_props.
 * Commit message tweaks

v2 -> v3:

 * 2/6: In line with recent upstream changes, the property table is now
   const and no longer carries an end-of-list marker.
 * The indentation fix (previously 5/6) has already been merged, so is no
   longer included.
 * Added patch fixing up logging of certain unhandled MMIO cases. (4/6)
 * 6/6: Moved the compat global property table into vmapple patch set -v16;
   we now just add the conditional-intr-mapping property to it in this
   patch. We also set the property on any device implementing the abstract
   TYPE_XHCI_PCI rather than only the TYPE_QEMU_XHCI device specifically.

Phil Dennis-Jordan (6):
  hw/usb/hcd-xhci-pci: Use modulo to select MSI vector as per spec
  hw/usb/hcd-xhci-pci: Move msi/msix properties from NEC to superclass
  hw/usb/hcd-xhci-pci: Use event ring 0 if mapping unsupported
  hw/usb/hcd-xhci: Unimplemented/guest error logging for port MMIO
  hw/usb/hcd-xhci-pci: Adds property for disabling mapping in IRQ mode
  hw/vmapple: XHCI controller's interrupt mapping workaround for macOS

 hw/usb/hcd-xhci-nec.c |  2 --
 hw/usb/hcd-xhci-pci.c | 31 +++++++++++++++++++++++++++++++
 hw/usb/hcd-xhci-pci.h |  1 +
 hw/usb/hcd-xhci.c     | 30 +++++++++++++++++++++++++++---
 hw/usb/hcd-xhci.h     |  5 +++++
 hw/vmapple/vmapple.c  |  8 +++++++-
 6 files changed, 71 insertions(+), 6 deletions(-)