[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/5] pci: Convert msix_init() to Error and fi
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/5] pci: Convert msix_init() to Error and fix callers to check it |
Date: |
Tue, 13 Sep 2016 10:27:41 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Cao jin <address@hidden> writes:
> On 09/12/2016 09:47 PM, Markus Armbruster wrote:
>> Cao jin <address@hidden> writes:
[...]
>>> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
>>> index 188f954..4280c5d 100644
>>> --- a/hw/usb/hcd-xhci.c
>>> +++ b/hw/usb/hcd-xhci.c
>>> @@ -3594,25 +3594,6 @@ static void usb_xhci_realize(struct PCIDevice *dev,
>>> Error **errp)
>>> dev->config[PCI_CACHE_LINE_SIZE] = 0x10;
>>> dev->config[0x60] = 0x30; /* release number */
>>>
>>> - usb_xhci_init(xhci);
>>> -
>>> - if (xhci->msi != ON_OFF_AUTO_OFF) {
>>> - ret = msi_init(dev, 0x70, xhci->numintrs, true, false, &err);
>>> - /* Any error other than -ENOTSUP(board's MSI support is broken)
>>> - * is a programming error */
>>> - assert(!ret || ret == -ENOTSUP);
>>> - if (ret && xhci->msi == ON_OFF_AUTO_ON) {
>>> - /* Can't satisfy user's explicit msi=on request, fail */
>>> - error_append_hint(&err, "You have to use msi=auto (default) or
>>> "
>>> - "msi=off with this machine type.\n");
>>> - error_propagate(errp, err);
>>> - return;
>>> - }
>>> - assert(!err || xhci->msi == ON_OFF_AUTO_AUTO);
>>> - /* With msi=auto, we fall back to MSI off silently */
>>> - error_free(err);
>>> - }
>>> -
>>> if (xhci->numintrs > MAXINTRS) {
>>> xhci->numintrs = MAXINTRS;
>>> }
>>> @@ -3622,21 +3603,60 @@ 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 {
>>> xhci->max_pstreams_mask = 0;
>>> }
>>>
>>> - xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
>>> xhci_mfwrap_timer, xhci);
>>> + if (xhci->msi != ON_OFF_AUTO_OFF) {
>>> + ret = msi_init(dev, 0x70, xhci->numintrs, true, false, &err);
>>> + /* Any error other than -ENOTSUP(board's MSI support is broken)
>>> + * is a programming error */
>>> + assert(!ret || ret == -ENOTSUP);
>>> + if (ret && xhci->msi == ON_OFF_AUTO_ON) {
>>> + /* Can't satisfy user's explicit msi=on request, fail */
>>> + error_append_hint(&err, "You have to use msi=auto (default) or
>>> "
>>> + "msi=off with this machine type.\n");
>>> + error_propagate(errp, err);
>>> + return;
>>> + }
>>> + assert(!err || xhci->msi == ON_OFF_AUTO_AUTO);
>>> + /* With msi=auto, we fall back to MSI off silently */
>>> + error_free(err);
>>> + }
>>
>> Can you explain why you're moving this code?
>>
>
> Sorry I forget to mention this: msi_init() uses xhci->numintrs, but
> there is value checking/correcting on xhci->numintrs, it should be
> done before using.
If you do the move in a separate patch before this one, you can explain
it in its commit message. Easier to review that way.