Re: [Qemu-devel] [PATCH v3 5/8] hcd-xhci: change behaviour of msix switch

[Markus Armbruster <armbru@redhat.com>]

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)