Re: [Qemu-devel] [PATCH v2 0/4] Fix device listener interface for PCI to PCI bridges

[Paul Durrant]

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:address@hidden
> Sent: 09 June 2015 10:13
> To: Don Slutz
> Cc: address@hidden; address@hidden; Paul Durrant;
> Stefano Stabellini
> Subject: Re: [PATCH v2 0/4] Fix device listener interface for PCI to PCI 
> bridges
> 
> On Mon, Jun 08, 2015 at 05:18:48PM -0400, Don Slutz wrote:
> > changes v1 to v2:
> >   Split v1 patch into 3.
> >
> >   Added a BUG FIX patch (#1: "exec: Do not use MemoryRegion after
> >   free").
> >
> >     Technically this bug fix should be a separate patch, however this
> >     issue only seems to reproduce when this patch set is applied.
> >
> >
> >   Michael S. Tsirkin:
> >     "You need some other API that makes sense, probably PCI specific."
> >       This is basically patch #2: "Extend device listener interface..."
> >
> >     "This is relying on undocumented assumptions and how specific
> >     firmware works. There's nothing special about bus number 255,
> >     and 0 is not very special either (except it happens to be the
> >     reset value)."
> >       Dropped all checking of 0 and 255.
> >
> >
> > Open question by Michael S. Tsirkin:
> >
> > >>>> On Thu, May 28, 2015 at 07:25:50AM -0400, Don Slutz wrote:
> > ...
> > >>>> It is not clear to me that the complexity of tracking bus
> > >>>> visibility make sense.  Clearly you do.
> > >>> Well what was the point of the change?
> > >
> > > To get config cycles that are valid that you do not get today.
> > >
> > >>> If you don't care that we get irrelevant config cycles why not
> > >>> just send them all to QEMU?
> > >>>
> > >
> > > That would need to be answered by Paul Durrant and/or other people
> (see
> > > below)
> > >
> >
> > We could broadcast config space ioreqs to all emulators, the problem
> > is how do we know (in the case of a read) which emulator is actually
> > the one supplying the data? At some level Xen needs to know who is
> > implementing what.
> >
> >   Paul Durrant
> 
> Can irrelevant emulators all respond with some kind of nack?
> Xen would pick the one that did respond correctly.
> 

I guess that's possible if we add an extra bit to the ioreq_t, but QEMU would 
still need to know when to nack and when to ack. It's still much simpler if 
qemu updates Xen with exact set of (S)BDFs it's handling.

  Paul

> > So this patch set just adjusts Xen to correctly know the current set
> > of PCI devices and their bus, device, and function.
> >
> > It does not attempt to calculate the visibility of the PCI devices.
> 
> So non-visible devices will appear with invalid bus number values,
> conflicting with the visible devices.
> Seems wrong.
> 
> 
> >
> > v1:
> >
> > Note: Right now hvmloader in Xen does not handle PCI to PCI bridges
> > and so SeaBIOS does not have access to PCI device(s) on secondary
> > buses.  How ever domU can setup the secondary bus(es) and this patch
> > will restore access to these PCI devices.
> >
> >
> > Don Slutz (4):
> >   exec: Do not use MemoryRegion after free
> >   Extend device listener interface for PCI to PCI bridges
> >   xen: Add usage of device listener interface for PCI to PCI bridges
> >   xen: Fix map/unmap of pcidev to ioreq server
> >
> >  exec.c                      |  8 ++++--
> >  hw/core/qdev.c              |  7 ++++++
> >  hw/pci/pci_bridge.c         | 18 +++++++++++++
> >  include/hw/qdev-core.h      |  3 +++
> >  include/hw/xen/xen_common.h | 61
> ++++++++++++++++++++++++++++++++++-----------
> >  trace-events                |  6 +++--
> >  xen-hvm.c                   | 20 +++++++++++++--
> >  7 files changed, 102 insertions(+), 21 deletions(-)
> >
> > --
> > 1.8.4