From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46861) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YB3Ug-0005O6-0v for qemu-devel@nongnu.org; Tue, 13 Jan 2015 10:34:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YB3Ub-0006X1-KI for qemu-devel@nongnu.org; Tue, 13 Jan 2015 10:34:57 -0500 Received: from e8.ny.us.ibm.com ([32.97.182.138]:51134) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YB3Ub-0006WQ-GC for qemu-devel@nongnu.org; Tue, 13 Jan 2015 10:34:53 -0500 Received: from /spool/local by e8.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 13 Jan 2015 10:34:50 -0500 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <20150113101219.GB7152@redhat.com> References: <1419363216-26601-1-git-send-email-mdroth@linux.vnet.ibm.com> <20150112132406.22996.59621@loki> <20150113101219.GB7152@redhat.com> Message-ID: <20150113153442.22996.7921@loki> Date: Tue, 13 Jan 2015 09:34:42 -0600 Subject: Re: [Qemu-devel] [PATCH 0/1] pci: allow 0 address for PCI IO/MEM regions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: peter.maydell@linaro.org, aik@ozlabs.ru, qemu-devel@nongnu.org, agraf@suse.de, qemu-ppc@nongnu.org, hw.claudio@gmail.com, david@gibson.dropbear.id.au Quoting Michael S. Tsirkin (2015-01-13 04:12:19) > On Mon, Jan 12, 2015 at 07:24:06AM -0600, Michael Roth wrote: > > Quoting Michael Roth (2014-12-23 13:33:35) > > > This patch enables the programming of address 0 for IO/MMIO BARs for > > > PCI devices. > > > = > > > It was originally included as part of a series implementing PCI > > > hotplug for pseries guests, where it is needed due to the fact > > > that pseries guests access IO space via MMIO, and that IO > > > space is dedicated to PCI devices, with RTAS calls being used in > > > place of common/legacy IO ports such as config-data/config-address. > > > = > > > Thus, the entire range is unhindered by legacy IO ports, and > > > pseries guest kernels may attempt to program an IO BAR to address 0 > > > as a result. > > > = > > > This has led to a conflict with the existing PCI config space > > > emulation code, where it has been assumed that 0 address are always > > > invalid. > > > = > > > Some background from discussions can be viewed here: > > > = > > > https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg03063.h= tml > > > = > > > The general summary from that discussion seems to be that 0-addresses= are > > > not (at least, are no longer) prohibited by current versions of the P= CI > > > spec, and that the same should apply for MMIO addresses (where allowi= ng > > > 0-addresses are also needed for some ARM-based PCI controllers). > > > = > > > This patch includes support for 0-address MMIO BARs based on that > > > discussion. > > > = > > > One still-lingering concern is whether this change will impact > > > compatibility with guests where 0-addresses are invalid. There was > > > some discussion on whether this issue could be addressed using > > > memory region priorities, but I think that's still an open question > > > that we can hopefully address here. > > = > > Ping > = > Michael, I can't apply this patch to all platforms: guests program 0 addr= ess > and expect that to not override all system devices. > = > If you want a quick hack, you need to find a way to make this apply > only to pseries. > = > One quick work-around for pseries is to limit your patch to devices > behind a pci to pci bridge: I think pseries places most devices behind > such bridges, am I right? Not to my knowledge. It's true that pseries guests on PowerVM generally (but not always) get a dedicated host bridge for hotplugged devices/expansion slots. Maybe that's what you're referring to? > = > The right solution is to locate, for each target, all system devices > that overlap with pci space, and figure out what the correct priorities > are. It's far from trivial, which is likely why no one did this yet. So there are cases where system devices should have higher priorities, and others where PCI devices can re-use the addresses? Would it be possible to have a check to see if a BAR overlaps a region, and then examine the mr->owner of the region to determine if it's a PCI device? If it's not, then assume it's a system device and avoid mapping the BAR as we do now? This would maintain the current behavior while allowing devices to use address 0 when it hasn't been claimed by a system device. I'm sure it's not that easy, but if it a catch-all solution of this sort it acceptable/work-able I can look into it. Otherwise, as far as quick fixes, I was thinking of adding a allow_zero_addr flag to TYPE_PCI_HOST_BRIDGE that can be set by instance_init for the various implementations of it. That would allow us to enable it for pseries, as well as (I think) some of the ARM use-cases Peter mentioned in the older thread. > = > Another issue is that - assuming what we are targeting is purely > theorectical PCI spec compliance - the patch does not go far enough. > We also should drop the check for the all-ones pattern in the > same function, that, too, should be a platform thing. > = > In particular, things break badly if guests size BARs e.g. using e.g. > 8 single-byte accesses, each one being a write of 0xff followed by read a= nd > write of 0x00, as opposed to two 32 bit writes of 0xffffffff followed > by reads and then writes of 0x0000000 - which no one seems to go > but is definitely legal. Yikes. Personally I'm more focused on a specific bug, but perhaps controlling this via a host-bridge flag also makes sense? > = > -- = > MST