> -----Original Message----- > From: Roger Pau Monne > Sent: 24 April 2017 10:09 > To: Paul Durrant <paul.durr...@citrix.com> > Cc: xen-de...@lists.xenproject.org; konrad.w...@oracle.com; > boris.ostrov...@oracle.com; Ian Jackson <ian.jack...@citrix.com>; Wei Liu > <wei.l...@citrix.com>; Jan Beulich <jbeul...@suse.com>; Andrew Cooper > <andrew.coop...@citrix.com> > Subject: Re: [PATCH v2 1/9] xen/vpci: introduce basic handlers to trap > accesses to the PCI config space > > On Fri, Apr 21, 2017 at 05:07:43PM +0100, Paul Durrant wrote: > > > -----Original Message----- > > > From: Roger Pau Monne [mailto:roger....@citrix.com] > > > Sent: 20 April 2017 16:18 > > > To: xen-de...@lists.xenproject.org > > > Cc: konrad.w...@oracle.com; boris.ostrov...@oracle.com; Roger Pau > Monne > > > <roger....@citrix.com>; Ian Jackson <ian.jack...@citrix.com>; Wei Liu > > > <wei.l...@citrix.com>; Jan Beulich <jbeul...@suse.com>; Andrew > Cooper > > > <andrew.coop...@citrix.com>; Paul Durrant <paul.durr...@citrix.com> > > > Subject: [PATCH v2 1/9] xen/vpci: introduce basic handlers to trap > accesses > > > to the PCI config space > > > > > > This functionality is going to reside in vpci.c (and the corresponding > > > vpci.h > > > header), and should be arch-agnostic. The handlers introduced in this > patch > > > setup the basic functionality required in order to trap accesses to the > > > PCI > > > config space, and allow decoding the address and finding the > corresponding > > > handler that should handle the access (although no handlers are > > > implemented). > > > > > > Note that the traps to the PCI IO ports registers (0xcf8/0xcfc) are setup > > > inside of a x86 HVM file, since that's not shared with other arches. > > > > > > A new XEN_X86_EMU_VPCI x86 domain flag is added in order to signal > Xen > > > whether > > > a domain should use the newly introduced vPCI handlers, this is only > enabled > > > for PVH Dom0 at the moment. > > > > > > A very simple user-space test is also provided, so that the basic > functionality > > > of the vPCI traps can be asserted. This has been proven quite helpful > during > > > development, since the logic to handle partial accesses or accesses that > > > expand > > > across multiple registers is not trivial. > > > > > > The handlers for the registers are added to a red-black tree, that indexes > > > them > > > based on their offset. Since Xen needs to handle partial accesses to the > > > registers and access that expand across multiple registers the logic in > > > xen_vpci_{read/write} is kind of convoluted, I've tried to properly > comment > > > it > > > in order to make it easier to understand. > > > > > > > Since config space is not exactly huge, I'm wondering why you used an r-b > tree rather than a direct map from register to handler? > > Hello, > > For local PCI the configuration space it's 256byte only, which means using 1/2 > a page (256 * 8) so that Xen can store a pointer for each possible register. > The extended configuration space (ECAM) extends the space to 4K, which > means we > would use 8 pages per device (4096*8), I think that's too much.
Ok, but I still think that adding an r-b tree implementation is just more complexity in the way that io handlers are registered in Xen. TBH, the whole thing needs a clean-up. We don't have proper range-based handler registration for port IO or MMIO at all (instead we potentially call the 'accept' function for every handler for every I/O). We then have (IIRC) an ordered list for MSI-X BAR registrations and now you're proposing an r-b system for PCI config space. On top of that, there is then the rangeset based ioreq server selection that occurs if the I/O falls through all of this and needs sending outside Xen. There really has to be at least some scope for unificiation here; it's getting way too convoluted. Paul > > Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
