qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 5/5] Add param Error ** for msi_init()


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.c

msi_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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]