From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=40470 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PIgiP-0004ZR-4I for qemu-devel@nongnu.org; Wed, 17 Nov 2010 07:02:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PIgiO-0007QL-0Q for qemu-devel@nongnu.org; Wed, 17 Nov 2010 07:02:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:10852) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PIgiN-0007PW-P5 for qemu-devel@nongnu.org; Wed, 17 Nov 2010 07:02:15 -0500 Date: Wed, 17 Nov 2010 14:02:00 +0200 From: "Michael S. Tsirkin" Message-ID: <20101117120009.GA10903@redhat.com> References: <20101116105019.GB19668@redhat.com> <20101117020314.GB8131@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20101117020314.GB8131@valinux.co.jp> Subject: [Qemu-devel] Re: [PATCH v9 1/8] pci: revise pci command register initialization List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: skandasa@cisco.com, adnan@khaleel.us, etmartin@cisco.com, qemu-devel@nongnu.org, wexu2@cisco.com On Wed, Nov 17, 2010 at 11:03:14AM +0900, Isaku Yamahata wrote: > On Tue, Nov 16, 2010 at 12:50:19PM +0200, Michael S. Tsirkin wrote: > > On Tue, Nov 16, 2010 at 05:26:05PM +0900, Isaku Yamahata wrote: > > > This patch cleans up command register initialization with > > > comments. It also fixes the initialization of io/memory bit of command register. > > > Those bits for type 1 device is RW. > > > Those bits for type 0 device is > > > RO = 0 if it has no io/memory BAR > > > RW if it has io/memory BAR > > > > > > Signed-off-by: Isaku Yamahata > > > > There's a bug here: you can not assume that device that has > > no io BAR claims no io transactions. > > Which device are you mentioning? Look at appendix D in PCI spec. There are many programming classes like display controllers that claim specific addresses without using a BAR. We could code it all up with a huge table. But what we have is much simpler and works well AFAIK. > Such device should set the bit by itself, not by pci generic layer. Since this is PCI spec specified behaviour, I think it's better to have it in a common place. Devices are sure to get it wrong. > > Another bug is that migrating from qemu where a bit is writeable to one > > where it's RO creates a situation where a RW bit becomes RO, or the > > reverse, which might confuse guests. So we will need a compatibility > > flag and set it for old machine types. > > We needs to keep compatibility. Which way do you prefer? > > - don't care: no way > > - introduce global property to indicate compat qemu version or flags > something like if (compat version <= 0.13) old behaviour... > or if (flags & ...) > > - introduce global-pci property > > - introduce pci bus property > Users needs to specify this property for all pci devices. > > - Don't change common code(pci.c), and provide a helper function. > Each device which needs new behavior like pcie calls it. > Probably each device may provide property to specify compat behavior > > - any other? - Don't change behaviour at all. What is the motivation for the change? Why do we bother? What we have is spec compliant, I think, so it's hard for me to believe pcie *needs* the new behaviour.