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

["Michael S. Tsirkin" <mst@xxxxxxxxxx>]

On Tue, Jun 09, 2015 at 02:14:29PM +0000, Paul Durrant wrote:
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@xxxxxxxxxx]
> > Sent: 09 June 2015 13:30
> > To: Paul Durrant
> > Cc: Don Slutz; qemu-devel@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxx; Stefano
> > Stabellini
> > Subject: Re: [PATCH v2 0/4] Fix device listener interface for PCI to PCI 
> > bridges
> > 
> > On Tue, Jun 09, 2015 at 10:58:26AM +0000, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Michael S. Tsirkin [mailto:mst@xxxxxxxxxx]
> > > > Sent: 09 June 2015 11:52
> > > > To: Paul Durrant
> > > > Cc: Don Slutz; qemu-devel@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxx;
> > 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@xxxxxxxxxx]
> > > > > > Sent: 09 June 2015 10:13
> > > > > > To: Don Slutz
> > > > > > Cc: qemu-devel@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxx; 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.
> 
> Well, IMO those need to be enumerated and suitable hooks need to be put in 
> place so that Xen can be informed of the changes.
> 
>   Paul

pci_data_write and pci_data_read already do bus lookups.
It seems that returning a nack there would just require
hooks in these two places.

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel