From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52564)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <shmulik.ladkani@ravellosystems.com>)
	id 1Z5Iqe-0003qy-5m
	for qemu-devel@nongnu.org; Wed, 17 Jun 2015 15:18:09 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <shmulik.ladkani@ravellosystems.com>)
	id 1Z5IqZ-0005Nn-7Y
	for qemu-devel@nongnu.org; Wed, 17 Jun 2015 15:18:08 -0400
Received: from mail-wi0-f170.google.com ([209.85.212.170]:38842)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <shmulik.ladkani@ravellosystems.com>)
	id 1Z5IqZ-0005Nd-1j
	for qemu-devel@nongnu.org; Wed, 17 Jun 2015 15:18:03 -0400
Received: by wibdq8 with SMTP id dq8so64367766wib.1
	for <qemu-devel@nongnu.org>; Wed, 17 Jun 2015 12:18:02 -0700 (PDT)
Date: Wed, 17 Jun 2015 22:17:58 +0300
From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Message-ID: <20150617221758.529fae93@pixies>
In-Reply-To: <55813FCE.2090405@gmail.com>
References: <1434443079-25755-1-git-send-email-shmulik.ladkani@ravellosystems.com>
	<55813F8E.1000105@gmail.com> <55813FCE.2090405@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v1] pci: Don't register a specialized
 'config_write' if default behavior is intended
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: Marcel Apfelbaum <marcel.apfelbaum@gmail.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>

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'

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.

Meaning, the suggested patch follows the spirit of 95d6580.
Let me know if my analysis is flawed.

Regards,
Shmulik