diff mbox series

[6/6] hw/vmapple: XHCI controller's interrupt mapping workaround for macOS

Message ID 20241208191646.64857-7-phil@philjordan.eu (mailing list archive)
State New
Headers show
Series hw/usb/hcd-xhci: Fixes, improvements, and macOS workaround | expand

Commit Message

Phil Dennis-Jordan Dec. 8, 2024, 7:16 p.m. UTC
This change enables the new conditional interrupt mapping support
property on the vmapple machine type's integrated XHCI controller.
The macOS guest driver attempts to use event rings 1 and 2 on the XHCI
controller, despite there being only one (PCI pin) interrupt channel
available. With conditional interrupt mapping enabled, the XHCI
controller will only schedule events on interrupter 0 in PCI pin mode
or when only a single MSI vector is active.

Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
---
 hw/vmapple/vmapple.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Akihiko Odaki Dec. 9, 2024, 6:26 a.m. UTC | #1
On 2024/12/09 4:16, Phil Dennis-Jordan wrote:
> This change enables the new conditional interrupt mapping support
> property on the vmapple machine type's integrated XHCI controller.
> The macOS guest driver attempts to use event rings 1 and 2 on the XHCI
> controller, despite there being only one (PCI pin) interrupt channel
> available. With conditional interrupt mapping enabled, the XHCI
> controller will only schedule events on interrupter 0 in PCI pin mode
> or when only a single MSI vector is active.

I think docs/system/arm/vmapple.rst also needs to be updated.

> 
> Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
> ---
>   hw/vmapple/vmapple.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/hw/vmapple/vmapple.c b/hw/vmapple/vmapple.c
> index f607981bc40..156ea33ae79 100644
> --- a/hw/vmapple/vmapple.c
> +++ b/hw/vmapple/vmapple.c
> @@ -453,6 +453,13 @@ static void create_pcie(VMAppleMachineState *vms)
>       }
>   
>       usb_controller = qdev_new(TYPE_QEMU_XHCI);
> +    /*
> +     * macOS XHCI driver attempts to schedule events onto even rings 1 & 2
> +     * even when (as here) there is no MSI-X support on this PCIe bus. Disabling
> +     * interrupter mapping in the XHCI controller works around the problem.
> +     */
> +    object_property_set_bool(OBJECT(usb_controller),
> +                             "conditional-intr-mapping", true, &error_fatal);

Use compat_props to change the global default for this machine.

By the way, this unconditionally adds xHCI and USB devices, but that 
should be avoided so that users can customize the configuration. Use 
defaults_enabled() as a condition.

Regards,
Akihiko Odaki

>       qdev_realize_and_unref(usb_controller, BUS(pci->bus), &error_fatal);
>   
>       usb_bus = USB_BUS(object_resolve_type_unambiguous(TYPE_USB_BUS,
Phil Dennis-Jordan Dec. 9, 2024, 11:14 a.m. UTC | #2
On Mon, 9 Dec 2024 at 07:26, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:

> On 2024/12/09 4:16, Phil Dennis-Jordan wrote:
> > This change enables the new conditional interrupt mapping support
> > property on the vmapple machine type's integrated XHCI controller.
> > The macOS guest driver attempts to use event rings 1 and 2 on the XHCI
> > controller, despite there being only one (PCI pin) interrupt channel
> > available. With conditional interrupt mapping enabled, the XHCI
> > controller will only schedule events on interrupter 0 in PCI pin mode
> > or when only a single MSI vector is active.
>
> I think docs/system/arm/vmapple.rst also needs to be updated.
>

Can you be more specific about what you think I should include? That file
currently does not mention USB in any way, and if we set the new property
in the machine type, there shouldn't be any need for manual configuration
on the command line, should there?


> >
> > Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
> > ---
> >   hw/vmapple/vmapple.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/hw/vmapple/vmapple.c b/hw/vmapple/vmapple.c
> > index f607981bc40..156ea33ae79 100644
> > --- a/hw/vmapple/vmapple.c
> > +++ b/hw/vmapple/vmapple.c
> > @@ -453,6 +453,13 @@ static void create_pcie(VMAppleMachineState *vms)
> >       }
> >
> >       usb_controller = qdev_new(TYPE_QEMU_XHCI);
> > +    /*
> > +     * macOS XHCI driver attempts to schedule events onto even rings 1
> & 2
> > +     * even when (as here) there is no MSI-X support on this PCIe bus.
> Disabling
> > +     * interrupter mapping in the XHCI controller works around the
> problem.
> > +     */
> > +    object_property_set_bool(OBJECT(usb_controller),
> > +                             "conditional-intr-mapping", true,
> &error_fatal);
>
> Use compat_props to change the global default for this machine.
>

Thanks, that works.



> By the way, this unconditionally adds xHCI and USB devices, but that
> should be avoided so that users can customize the configuration. Use
> defaults_enabled() as a condition.
>

Makes sense, I guess I'd better add that to the VMApple patch set though.



> Regards,
> Akihiko Odaki
>
> >       qdev_realize_and_unref(usb_controller, BUS(pci->bus),
> &error_fatal);
> >
> >       usb_bus = USB_BUS(object_resolve_type_unambiguous(TYPE_USB_BUS,
>
>
Akihiko Odaki Dec. 9, 2024, 11:47 a.m. UTC | #3
On 2024/12/09 20:14, Phil Dennis-Jordan wrote:
> 
> 
> On Mon, 9 Dec 2024 at 07:26, Akihiko Odaki <akihiko.odaki@daynix.com 
> <mailto:akihiko.odaki@daynix.com>> wrote:
> 
>     On 2024/12/09 4:16, Phil Dennis-Jordan wrote:
>      > This change enables the new conditional interrupt mapping support
>      > property on the vmapple machine type's integrated XHCI controller.
>      > The macOS guest driver attempts to use event rings 1 and 2 on the
>     XHCI
>      > controller, despite there being only one (PCI pin) interrupt channel
>      > available. With conditional interrupt mapping enabled, the XHCI
>      > controller will only schedule events on interrupter 0 in PCI pin mode
>      > or when only a single MSI vector is active.
> 
>     I think docs/system/arm/vmapple.rst also needs to be updated.
> 
> 
> Can you be more specific about what you think I should include? That 
> file currently does not mention USB in any way, and if we set the new 
> property in the machine type, there shouldn't be any need for manual 
> configuration on the command line, should there?

I mistook the patch message for the documentation. The documentation 
file is fine so there is no need for change here.
diff mbox series

Patch

diff --git a/hw/vmapple/vmapple.c b/hw/vmapple/vmapple.c
index f607981bc40..156ea33ae79 100644
--- a/hw/vmapple/vmapple.c
+++ b/hw/vmapple/vmapple.c
@@ -453,6 +453,13 @@  static void create_pcie(VMAppleMachineState *vms)
     }
 
     usb_controller = qdev_new(TYPE_QEMU_XHCI);
+    /*
+     * macOS XHCI driver attempts to schedule events onto even rings 1 & 2
+     * even when (as here) there is no MSI-X support on this PCIe bus. Disabling
+     * interrupter mapping in the XHCI controller works around the problem.
+     */
+    object_property_set_bool(OBJECT(usb_controller),
+                             "conditional-intr-mapping", true, &error_fatal);
     qdev_realize_and_unref(usb_controller, BUS(pci->bus), &error_fatal);
 
     usb_bus = USB_BUS(object_resolve_type_unambiguous(TYPE_USB_BUS,