From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42631)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <caoj.fnst@cn.fujitsu.com>) id 1b8O8V-0003eg-1b
	for qemu-devel@nongnu.org; Thu, 02 Jun 2016 04:37:52 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <caoj.fnst@cn.fujitsu.com>) id 1b8O8P-00066m-DV
	for qemu-devel@nongnu.org; Thu, 02 Jun 2016 04:37:49 -0400
Received: from [59.151.112.132] (port=8267 helo=heian.cn.fujitsu.com)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <caoj.fnst@cn.fujitsu.com>) id 1b8O8O-0005yu-Jc
	for qemu-devel@nongnu.org; Thu, 02 Jun 2016 04:37:45 -0400
References: <1464062689-32156-1-git-send-email-caoj.fnst@cn.fujitsu.com>
	<1464062689-32156-8-git-send-email-caoj.fnst@cn.fujitsu.com>
	<878typ2tgn.fsf@dusky.pond.sub.org>
From: Cao jin <caoj.fnst@cn.fujitsu.com>
Message-ID: <574FF17F.8060408@cn.fujitsu.com>
Date: Thu, 2 Jun 2016 16:42:39 +0800
MIME-Version: 1.0
In-Reply-To: <878typ2tgn.fsf@dusky.pond.sub.org>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v6 07/11] intel-hda: change msi property
 type
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, Marcel Apfelbaum <marcel@redhat.com>, Gerd
	Hoffmann <kraxel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>



On 06/01/2016 04:39 PM, Markus Armbruster wrote:
> Cao jin <caoj.fnst@cn.fujitsu.com> writes:
>
>
> Not covered:
>
>     static void intel_hda_update_irq(IntelHDAState *d)
>     {
> -->    int msi = d->msi && msi_enabled(&d->pci);
>         int level;
>
>         intel_hda_update_int_sts(d);
>         if (d->int_sts & (1U << 31) && d->int_ctl & (1U << 31)) {
>             level = 1;
>         } else {
>             level = 0;
>         }
>         dprint(d, 2, "%s: level %d [%s]\n", __FUNCTION__,
>                level, msi ? "msi" : "intx");
>         if (msi) {
>             if (level) {
>                 msi_notify(&d->pci, 0);
>             }
>         } else {
>             pci_set_irq(&d->pci, level);
>         }
>     }
>
> This is wrong, because the meaning of the test changes from
>
>      (user didn't specify msi=off) && (guest enabled MSI)
>
> to
>
>      (user didn't specify msi=on or msi=off) && (guest enabled MSI)
>

Thanks for pointing the error, seems I lost the fact that 
ON_OFF_AUTO_AUTO equals 0.

> What about:
>
>         int msi = msi_enabled(&d->pci);
>
> If I understand it correctly, it can only be true when we added MSI
> capability, and we do that only when msi=auto (default) or msi=on.
>

I think the modification is ok.

-- 
Yours Sincerely,

Cao jin