diff mbox

[v3,5/8] hcd-xhci: change behaviour of msix switch

Message ID 1473839464-8670-6-git-send-email-caoj.fnst@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Cao jin Sept. 14, 2016, 7:51 a.m. UTC
Resolve the TODO, msix=auto means msix on; if user specify msix=on,
then device creation fail on msix_init failure.

CC: Gerd Hoffmann <kraxel@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/usb/hcd-xhci.c | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

Comments

Markus Armbruster Sept. 29, 2016, 2:32 p.m. UTC | #1
Cao jin <caoj.fnst@cn.fujitsu.com> writes:

> Resolve the TODO, msix=auto means msix on; if user specify msix=on,
> then device creation fail on msix_init failure.
>
> CC: Gerd Hoffmann <kraxel@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Marcel Apfelbaum <marcel@redhat.com>
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> ---
>  hw/usb/hcd-xhci.c | 35 ++++++++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 59f0239..4280c5d 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -3603,12 +3603,14 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
>      if (xhci->numintrs < 1) {
>          xhci->numintrs = 1;
>      }
> +
>      if (xhci->numslots > MAXSLOTS) {
>          xhci->numslots = MAXSLOTS;
>      }
>      if (xhci->numslots < 1) {
>          xhci->numslots = 1;
>      }
> +
>      if (xhci_get_flag(xhci, XHCI_FLAG_ENABLE_STREAMS)) {
>          xhci->max_pstreams_mask = 7; /* == 256 primary streams */
>      } else {
> @@ -3633,6 +3635,28 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
>      }
>  
>      memory_region_init(&xhci->mem, OBJECT(xhci), "xhci", LEN_REGS);
> +    if (xhci->msix != ON_OFF_AUTO_OFF) {
> +        ret = msix_init(dev, xhci->numintrs,
> +                        &xhci->mem, 0, OFF_MSIX_TABLE,
> +                        &xhci->mem, 0, OFF_MSIX_PBA,
> +                        0x90, &err);
> +        /* Any error other than -ENOTSUP(board's MSI support is broken)
> +         * is a programming error */
> +        assert(!ret || ret == -ENOTSUP);
> +        if (ret && xhci->msix == ON_OFF_AUTO_ON) {
> +            /* Can't satisfy user's explicit msix=on request, fail */
> +            error_append_hint(&err, "You have to use msix=auto (default) or "
> +                    "msic=off with this machine type.\n");

s/msic/msix/

Can hopefully be touched up on commit.

> +            /* No instance_finalize method, need to free the resource here */
> +            object_unref(OBJECT(&xhci->mem));
> +            error_propagate(errp, err);
> +            return;
> +        }
> +        assert(!err || xhci->msix == ON_OFF_AUTO_AUTO);
> +        /* With msix=auto, we fall back to MSI off silently */
> +        error_free(err);
> +    }
> +
>      memory_region_init_io(&xhci->mem_cap, OBJECT(xhci), &xhci_cap_ops, xhci,
>                            "capabilities", LEN_CAP);
>      memory_region_init_io(&xhci->mem_oper, OBJECT(xhci), &xhci_oper_ops, xhci,
> @@ -3668,17 +3692,6 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
>          ret = pcie_endpoint_cap_init(dev, 0xa0);
>          assert(ret >= 0);
>      }
> -
> -    if (xhci->msix != ON_OFF_AUTO_OFF) {
> -        /* TODO check for errors, and should fail when msix=on */
> -        ret = msix_init(dev, xhci->numintrs,
> -                        &xhci->mem, 0, OFF_MSIX_TABLE,
> -                        &xhci->mem, 0, OFF_MSIX_PBA,
> -                        0x90, &err);
> -        if (ret) {
> -            error_report_err(err);
> -        }
> -    }
>  }
>  
>  static void usb_xhci_exit(PCIDevice *dev)
Gerd Hoffmann Sept. 29, 2016, 3:22 p.m. UTC | #2
On Do, 2016-09-29 at 16:32 +0200, Markus Armbruster wrote:
> > Resolve the TODO, msix=auto means msix on; if user specify msix=on,
> > then device creation fail on msix_init failure.

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
diff mbox

Patch

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 59f0239..4280c5d 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3603,12 +3603,14 @@  static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
     if (xhci->numintrs < 1) {
         xhci->numintrs = 1;
     }
+
     if (xhci->numslots > MAXSLOTS) {
         xhci->numslots = MAXSLOTS;
     }
     if (xhci->numslots < 1) {
         xhci->numslots = 1;
     }
+
     if (xhci_get_flag(xhci, XHCI_FLAG_ENABLE_STREAMS)) {
         xhci->max_pstreams_mask = 7; /* == 256 primary streams */
     } else {
@@ -3633,6 +3635,28 @@  static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
     }
 
     memory_region_init(&xhci->mem, OBJECT(xhci), "xhci", LEN_REGS);
+    if (xhci->msix != ON_OFF_AUTO_OFF) {
+        ret = msix_init(dev, xhci->numintrs,
+                        &xhci->mem, 0, OFF_MSIX_TABLE,
+                        &xhci->mem, 0, OFF_MSIX_PBA,
+                        0x90, &err);
+        /* Any error other than -ENOTSUP(board's MSI support is broken)
+         * is a programming error */
+        assert(!ret || ret == -ENOTSUP);
+        if (ret && xhci->msix == ON_OFF_AUTO_ON) {
+            /* Can't satisfy user's explicit msix=on request, fail */
+            error_append_hint(&err, "You have to use msix=auto (default) or "
+                    "msic=off with this machine type.\n");
+            /* No instance_finalize method, need to free the resource here */
+            object_unref(OBJECT(&xhci->mem));
+            error_propagate(errp, err);
+            return;
+        }
+        assert(!err || xhci->msix == ON_OFF_AUTO_AUTO);
+        /* With msix=auto, we fall back to MSI off silently */
+        error_free(err);
+    }
+
     memory_region_init_io(&xhci->mem_cap, OBJECT(xhci), &xhci_cap_ops, xhci,
                           "capabilities", LEN_CAP);
     memory_region_init_io(&xhci->mem_oper, OBJECT(xhci), &xhci_oper_ops, xhci,
@@ -3668,17 +3692,6 @@  static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
         ret = pcie_endpoint_cap_init(dev, 0xa0);
         assert(ret >= 0);
     }
-
-    if (xhci->msix != ON_OFF_AUTO_OFF) {
-        /* TODO check for errors, and should fail when msix=on */
-        ret = msix_init(dev, xhci->numintrs,
-                        &xhci->mem, 0, OFF_MSIX_TABLE,
-                        &xhci->mem, 0, OFF_MSIX_PBA,
-                        0x90, &err);
-        if (ret) {
-            error_report_err(err);
-        }
-    }
 }
 
 static void usb_xhci_exit(PCIDevice *dev)