Re: [Qemu-devel] [PATCH 0/1] pci: allow 0 address for PCI IO/MEM regions

[Michael S. Tsirkin]

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.html
> > 
> > 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 PCI
> > spec, and that the same should apply for MMIO addresses (where allowing
> > 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 address
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?

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.

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 and
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.

-- 
MST