From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:33302)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1bjh1F-000677-Vv
	for qemu-devel@nongnu.org; Tue, 13 Sep 2016 02:16:37 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1bjh17-0002NQ-9W
	for qemu-devel@nongnu.org; Tue, 13 Sep 2016 02:16:28 -0400
Received: from mx1.redhat.com ([209.132.183.28]:47400)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1bjh17-0002N5-3p
	for qemu-devel@nongnu.org; Tue, 13 Sep 2016 02:16:25 -0400
From: Markus Armbruster <armbru@redhat.com>
References: <1471944454-13895-1-git-send-email-caoj.fnst@cn.fujitsu.com>
	<1471944454-13895-2-git-send-email-caoj.fnst@cn.fujitsu.com>
	<87y42xw8jk.fsf@dusky.pond.sub.org> <57D769C7.4000503@cn.fujitsu.com>
Date: Tue, 13 Sep 2016 08:16:20 +0200
In-Reply-To: <57D769C7.4000503@cn.fujitsu.com> (Cao jin's message of "Tue, 13
	Sep 2016 10:51:51 +0800")
Message-ID: <87fup4iatn.fsf@dusky.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error
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: Cao jin <caoj.fnst@cn.fujitsu.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>, qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>, Alex Williamson <alex.williamson@redhat.com>

Cc: Alex for device assignment expertise.

Cao jin <caoj.fnst@cn.fujitsu.com> writes:

> On 09/12/2016 09:29 PM, Markus Armbruster wrote:
>> Cao jin <caoj.fnst@cn.fujitsu.com> writes:
>>
>>> The input parameters is used for creating the msix capable device, so
>>> they must obey the PCI spec, or else, it should be programming error.
>>
>> True when the the parameters come from a device model attempting to
>> define a PCI device violating the spec.  But what if the parameters come
>> from an actual PCI device violating the spec, via device assignment?
>
> Before the patch, on invalid param, the vfio behaviour is:
>   error_report("vfio: msix_init failed");
>   then, device create fail.
>
> After the patch, its behaviour is:
>   asserted.
>
> Do you mean we should still report some useful info to user on invalid
> params?

In the normal case, asking msix_init() to create MSI-X that are out of
spec is a programming error: the code that does it is broken and needs
fixing.

Device assignment might be the exception: there, the parameters for
msix_init() come from the assigned device, not the program.  If they
violate the spec, the device is broken.  This wouldn't be a programming
error.  Alex, can this happen?

If yes, we may want to handle it by failing device assignment.

> Cao jin
>>
>> For what it's worth, the new behavior seems consistent with msi_init(),
>> which is good.

Whatever behavior on out-of-spec parameters we choose, msi_init() and
msix_init() should behave the same.