From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60520) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aoroZ-0005uq-UF for qemu-devel@nongnu.org; Sat, 09 Apr 2016 08:16:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aoroU-0007X1-VC for qemu-devel@nongnu.org; Sat, 09 Apr 2016 08:16:35 -0400 Received: from [59.151.112.132] (port=47242 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aoroT-0007Uz-SK for qemu-devel@nongnu.org; Sat, 09 Apr 2016 08:16:30 -0400 References: <1459855602-16727-1-git-send-email-caoj.fnst@cn.fujitsu.com> <1459855602-16727-6-git-send-email-caoj.fnst@cn.fujitsu.com> <87pou0sd4u.fsf@dusky.pond.sub.org> From: Cao jin Message-ID: <5708F33B.2040000@cn.fujitsu.com> Date: Sat, 9 Apr 2016 20:19:07 +0800 MIME-Version: 1.0 In-Reply-To: <87pou0sd4u.fsf@dusky.pond.sub.org> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 5/5] Add param Error ** for msi_init() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, mst@redhat.com, jasowang@redhat.com, marcel@redhat.com, alex.williamson@redhat.com, hare@suse.de, dmitry@daynix.com, pbonzini@redhat.com, jsnow@redhat.com, kraxel@redhat.com 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