[PATCH] usb: add support for sending MSIs from EHCI PCI devices

Isaac Woods posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240309151031.29417-2-isaacwoods.home@gmail.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
There is a newer version of this series
hw/usb/hcd-ehci-pci.c    | 27 +++++++++++++++++++++++++++
hw/usb/hcd-ehci-sysbus.c |  7 +++++++
hw/usb/hcd-ehci.c        |  2 +-
hw/usb/hcd-ehci.h        |  2 ++
4 files changed, 37 insertions(+), 1 deletion(-)
[PATCH] usb: add support for sending MSIs from EHCI PCI devices
Posted by Isaac Woods 1 month, 2 weeks ago
Signed-off-by: Isaac Woods <isaacwoods.home@gmail.com>
---
 hw/usb/hcd-ehci-pci.c    | 27 +++++++++++++++++++++++++++
 hw/usb/hcd-ehci-sysbus.c |  7 +++++++
 hw/usb/hcd-ehci.c        |  2 +-
 hw/usb/hcd-ehci.h        |  2 ++
 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 3ff54edf62..8a714b6733 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -21,6 +21,8 @@
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "qemu/range.h"
+#include "hw/pci/msi.h"
+#include "qapi/error.h"
 
 typedef struct EHCIPCIInfo {
     const char *name;
@@ -30,11 +32,27 @@ typedef struct EHCIPCIInfo {
     bool companion;
 } EHCIPCIInfo;
 
+static void ehci_pci_intr_update(EHCIState *ehci, bool enable)
+{
+    EHCIPCIState *s = container_of(ehci, EHCIPCIState, ehci);
+    PCIDevice *pci_dev = PCI_DEVICE(s);
+
+    if (msi_enabled(pci_dev)) {
+        if (enable) {
+            msi_notify(pci_dev, 0);
+        }
+    } else {
+        pci_set_irq(pci_dev, enable);
+    }
+}
+
 static void usb_ehci_pci_realize(PCIDevice *dev, Error **errp)
 {
     EHCIPCIState *i = PCI_EHCI(dev);
     EHCIState *s = &i->ehci;
     uint8_t *pci_conf = dev->config;
+    Error *err = NULL;
+    int ret;
 
     pci_set_byte(&pci_conf[PCI_CLASS_PROG], 0x20);
 
@@ -68,8 +86,17 @@ static void usb_ehci_pci_realize(PCIDevice *dev, Error **errp)
     s->irq = pci_allocate_irq(dev);
     s->as = pci_get_address_space(dev);
 
+    s->intr_update = ehci_pci_intr_update;
+
     usb_ehci_realize(s, DEVICE(dev), NULL);
     pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mem);
+
+    ret = msi_init(dev, 0x70, 1, true, false, &err);
+    if (ret) {
+        error_propagate(errp, err);
+    } else {
+        error_free(err);
+    }
 }
 
 static void usb_ehci_pci_init(Object *obj)
diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index fe1dabd0bb..e08c856e2b 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -38,6 +38,11 @@ static Property ehci_sysbus_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void usb_ehci_sysbus_intr_update(EHCIState *ehci, bool level)
+{
+    qemu_set_irq(s->irq, level);
+}
+
 static void usb_ehci_sysbus_realize(DeviceState *dev, Error **errp)
 {
     SysBusDevice *d = SYS_BUS_DEVICE(dev);
@@ -70,6 +75,8 @@ static void ehci_sysbus_init(Object *obj)
     s->portnr = sec->portnr;
     s->as = &address_space_memory;
 
+    s->intr_update = usb_ehci_sysbus_intr_update;
+
     usb_ehci_init(s, DEVICE(obj));
     sysbus_init_mmio(d, &s->mem);
 }
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 01864d4649..e1f71dc445 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -209,7 +209,7 @@ static inline void ehci_update_irq(EHCIState *s)
     }
 
     trace_usb_ehci_irq(level, s->frindex, s->usbsts, s->usbintr);
-    qemu_set_irq(s->irq, level);
+    (s->intr_update)(s, level);
 }
 
 /* flag interrupt condition */
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 56a1c09d1f..bc017aec79 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -305,6 +305,8 @@ struct EHCIState {
     EHCIQueueHead aqueues;
     EHCIQueueHead pqueues;
 
+    void (*intr_update)(EHCIState *s, bool enable);
+
     /* which address to look at next */
     uint32_t a_fetch_addr;
     uint32_t p_fetch_addr;
-- 
2.44.0
Re: [PATCH] usb: add support for sending MSIs from EHCI PCI devices
Posted by BALATON Zoltan 1 month, 2 weeks ago
On Sat, 9 Mar 2024, Isaac Woods wrote:
> Signed-off-by: Isaac Woods <isaacwoods.home@gmail.com>
> ---
> hw/usb/hcd-ehci-pci.c    | 27 +++++++++++++++++++++++++++
> hw/usb/hcd-ehci-sysbus.c |  7 +++++++
> hw/usb/hcd-ehci.c        |  2 +-
> hw/usb/hcd-ehci.h        |  2 ++
> 4 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
> index 3ff54edf62..8a714b6733 100644
> --- a/hw/usb/hcd-ehci-pci.c
> +++ b/hw/usb/hcd-ehci-pci.c
> @@ -21,6 +21,8 @@
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> #include "qemu/range.h"
> +#include "hw/pci/msi.h"
> +#include "qapi/error.h"
>
> typedef struct EHCIPCIInfo {
>     const char *name;
> @@ -30,11 +32,27 @@ typedef struct EHCIPCIInfo {
>     bool companion;
> } EHCIPCIInfo;
>
> +static void ehci_pci_intr_update(EHCIState *ehci, bool enable)
> +{
> +    EHCIPCIState *s = container_of(ehci, EHCIPCIState, ehci);
> +    PCIDevice *pci_dev = PCI_DEVICE(s);
> +
> +    if (msi_enabled(pci_dev)) {
> +        if (enable) {
> +            msi_notify(pci_dev, 0);
> +        }
> +    } else {
> +        pci_set_irq(pci_dev, enable);
> +    }
> +}
> +
> static void usb_ehci_pci_realize(PCIDevice *dev, Error **errp)
> {
>     EHCIPCIState *i = PCI_EHCI(dev);
>     EHCIState *s = &i->ehci;
>     uint8_t *pci_conf = dev->config;
> +    Error *err = NULL;
> +    int ret;
>
>     pci_set_byte(&pci_conf[PCI_CLASS_PROG], 0x20);
>
> @@ -68,8 +86,17 @@ static void usb_ehci_pci_realize(PCIDevice *dev, Error **errp)
>     s->irq = pci_allocate_irq(dev);
>     s->as = pci_get_address_space(dev);
>
> +    s->intr_update = ehci_pci_intr_update;
> +
>     usb_ehci_realize(s, DEVICE(dev), NULL);
>     pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mem);
> +
> +    ret = msi_init(dev, 0x70, 1, true, false, &err);
> +    if (ret) {
> +        error_propagate(errp, err);
> +    } else {
> +        error_free(err);
> +    }

I could be wrong but I think you don't have to do this and can just pass 
errp to msi_init here. You might need to check if (msi_init()) { return;} 
but as this is at the end even that does not matter (althogh may worth 
adding in case something is later added after this).

Regards,
BALATON Zoltan

> }
>
> static void usb_ehci_pci_init(Object *obj)
> diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
> index fe1dabd0bb..e08c856e2b 100644
> --- a/hw/usb/hcd-ehci-sysbus.c
> +++ b/hw/usb/hcd-ehci-sysbus.c
> @@ -38,6 +38,11 @@ static Property ehci_sysbus_properties[] = {
>     DEFINE_PROP_END_OF_LIST(),
> };
>
> +static void usb_ehci_sysbus_intr_update(EHCIState *ehci, bool level)
> +{
> +    qemu_set_irq(s->irq, level);
> +}
> +
> static void usb_ehci_sysbus_realize(DeviceState *dev, Error **errp)
> {
>     SysBusDevice *d = SYS_BUS_DEVICE(dev);
> @@ -70,6 +75,8 @@ static void ehci_sysbus_init(Object *obj)
>     s->portnr = sec->portnr;
>     s->as = &address_space_memory;
>
> +    s->intr_update = usb_ehci_sysbus_intr_update;
> +
>     usb_ehci_init(s, DEVICE(obj));
>     sysbus_init_mmio(d, &s->mem);
> }
> diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
> index 01864d4649..e1f71dc445 100644
> --- a/hw/usb/hcd-ehci.c
> +++ b/hw/usb/hcd-ehci.c
> @@ -209,7 +209,7 @@ static inline void ehci_update_irq(EHCIState *s)
>     }
>
>     trace_usb_ehci_irq(level, s->frindex, s->usbsts, s->usbintr);
> -    qemu_set_irq(s->irq, level);
> +    (s->intr_update)(s, level);
> }
>
> /* flag interrupt condition */
> diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
> index 56a1c09d1f..bc017aec79 100644
> --- a/hw/usb/hcd-ehci.h
> +++ b/hw/usb/hcd-ehci.h
> @@ -305,6 +305,8 @@ struct EHCIState {
>     EHCIQueueHead aqueues;
>     EHCIQueueHead pqueues;
>
> +    void (*intr_update)(EHCIState *s, bool enable);
> +
>     /* which address to look at next */
>     uint32_t a_fetch_addr;
>     uint32_t p_fetch_addr;
>