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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

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

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

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