From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:55671)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <marcel.apfelbaum@gmail.com>) id 1Z6aUN-0004Zr-2q
	for qemu-devel@nongnu.org; Sun, 21 Jun 2015 04:20:27 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <marcel.apfelbaum@gmail.com>) id 1Z6aUJ-0004m3-32
	for qemu-devel@nongnu.org; Sun, 21 Jun 2015 04:20:27 -0400
Received: from mail-wi0-x231.google.com ([2a00:1450:400c:c05::231]:34445)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <marcel.apfelbaum@gmail.com>) id 1Z6aUI-0004lq-Rq
	for qemu-devel@nongnu.org; Sun, 21 Jun 2015 04:20:22 -0400
Received: by wicnd19 with SMTP id nd19so51245320wic.1
	for <qemu-devel@nongnu.org>; Sun, 21 Jun 2015 01:20:22 -0700 (PDT)
Message-ID: <558673C2.1000909@gmail.com>
Date: Sun, 21 Jun 2015 11:20:18 +0300
From: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
MIME-Version: 1.0
References: <1434443079-25755-1-git-send-email-shmulik.ladkani@ravellosystems.com>	<55813F8E.1000105@gmail.com>	<55813FCE.2090405@gmail.com>
	<20150617221758.529fae93@pixies>
In-Reply-To: <20150617221758.529fae93@pixies>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v1] pci: Don't register a specialized
 'config_write' if default behavior is intended
Reply-To: marcel@redhat.com
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: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Cc: Leonid Shatz <leonid.shatz@ravellosystems.com>, "Michael S. Tsirkin" <mst@redhat.com>, Jan Kiszka <jan.kiszka@siemens.com>, Idan Brown <idan.brown@ravellosystems.com>, Knut Omang <knut.omang@oracle.com>, qemu-devel@nongnu.org, Hannes Reinecke <hare@suse.de>

On 06/17/2015 10:17 PM, Shmulik Ladkani wrote:
> Hi,
>
> On Wed, 17 Jun 2015 12:37:18 +0300, marcel.apfelbaum@gmail.com wrote:
>> BTW, did you notice a bug here?  If yes, can you elaborate?
>
> No, not a direct bug.
> We noticed this while working on related code areas.
>
> There's some history behind this.
>
> In 95d6580 'msi: Invoke msi/msix_write_config from PCI core', the calls
> to msi[x]_write_config have been added into pci_default_write_config,
> and many specialized 'config_write' methods have been eliminated.
>
> However there was a bug in 95d6580 - the values written to msi/msix
> were always 0.
> This was recently fixed in d7efb7e
>       'pci: avoid losing config updates to MSI/MSIX cap regs'
Got it.

>
> I assume that device authors were either (1) unware of the
> generalization, thus kept invoking msi[x]_write_config explicitly, or
> (2) trying to overcome the "lost writes".
>
> Anyway, I'm no PCI expert here, but I assume the side-effect invoking
> msi[x]_write_config twice (explicitly from the specialized config_write,
> then implicitly from pci_default_write_config) isn't desired.
Of course.

>
> Meaning, the suggested patch follows the spirit of 95d6580.
> Let me know if my analysis is flawed.
Thank you for the patch, you are completely right.
My 'Reviewed-by' tag is there, I think Michael, the PCI
maintainer, will take it shortly.

Thanks,
Marcel

>
> Regards,
> Shmulik
>