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

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Jun 09, 2015 at 10:58:26AM +0000, Paul Durrant wrote:
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: 09 June 2015 11:52
> > To: Paul Durrant
> > Cc: Don Slutz; qemu-devel@nongnu.org; xen-devel@lists.xen.org; Stefano
> > Stabellini
> > Subject: Re: [PATCH v2 0/4] Fix device listener interface for PCI to PCI bridges
> > 
> > On Tue, Jun 09, 2015 at 09:18:49AM +0000, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > > Sent: 09 June 2015 10:13
> > > > To: Don Slutz
> > > > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; 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 simple: ack if we find a device handling the specific BDF.
> > The result would at least be contained.
> > OTOH detecting master aborts in core is useful since it would
> > let us implement error reporting correctly.
> > 
> > 
> > > It's still much simpler if qemu updates Xen with exact set of (S)BDFs it's
> > handling.
> > >
> > >   Paul
> > 
> > 
> > I suspect this calls for a bigger change, you need to re-scan
> > all of the tree to detect visible devices.
> > Maybe just write some xen-specific code to do this on each
> > config access.
> 
> Well, that's the thing really... what triggers the re-scan. Do we really need to scan on each access or is there a way to know when the topology is changed? Doing the former doesn't really sound wonderfully efficient and, if the answer to the second is yes, then that's the time to update Xen with the currently valid set of BDFs.
> 
>   Paul


Several things can trigger a topology change.
One other option is switching to a memory API
for config accesses, then using a memory listener to detect
topology changes. That would be a lot of work I'm afraid.

-- 
MST