|
From: | Cao jin |
Subject: | Re: [Qemu-devel] [PATCH v4 5/5] Add param Error ** for msi_init() |
Date: | Sat, 9 Apr 2016 20:19:07 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
Hi On 04/08/2016 04:44 PM, Markus Armbruster wrote:
diff --git a/hw/ide/ich.c b/hw/ide/ich.c index 0a13334..db4fdb5 100644 --- a/hw/ide/ich.c +++ b/hw/ide/ich.c @@ -146,7 +146,7 @@ static void pci_ich9_ahci_realize(PCIDevice *dev, Error **errp) /* Although the AHCI 1.3 specification states that the first capability * should be PMCAP, the Intel ICH9 data sheet specifies that the ICH9 * AHCI device puts the MSI capability first, pointing to 0x80. */ - msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false); + msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false, errp);Sure there's nothing to undo on error? Instead of undoing, you may want to move msi_init() before the stuff that needs undoing.
ich9-ahci is a on-board device of Q35, like cover-letter says: when it fail, qemu will exit. So, is it necessary to undo on error?
maybe you saw, I did move msi_init() for some other devices.
diff --git a/hw/pci/msi.c b/hw/pci/msi.cmsi_init() has three failure modes: * -ENOTSUP Board's MSI emulation is not known to work: !msi_nonbroken. This is not necessarily an error. It is when the device model requires MSI. It isnt' when a non-MSI variant of the device model exists. Then caller should silently switch to the non-MSI variant[*].
Several questions on this topic:1. How to confirm whether a device model has non-MSI variant? AFAICT, it is these who have msi property.
2. For those have non-MSI variant devices(have msi property), as I see in the code, they all have it on by default, So we won`t know it is user order, or user don`t set it at all.
If user don`t know msi and don`t set it on, I think it is acceptable to create the non-msi variant for user silently. But if it is user order, like you said, it is an error.
So, how about: inform user to swich msi off and try again when encounter -ENOTSUP, no matter it is user order, or user doesn`t set it at all?
Actually in this v4, I do checked whether device has a msi property, like cover-letter said:
3. most devices have msi/msix(except vmxnet3 & pvscsi) property as a switch, if it has and is switched on, then msi_init() failure should results in return directly. So in this version, mptsas is updated
-- Yours Sincerely, Cao jin
[Prev in Thread] | Current Thread | [Next in Thread] |