Re: [Xen-devel] [PATCH v2 1/9] xen/vpci: introduce basic handlers to trap accesses to the PCI config space

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Mon, Apr 24, 2017 at 12:50:58PM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Roger Pau Monne
> > Sent: 24 April 2017 12:03
> > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; konrad.wilk@xxxxxxxxxx;
> > boris.ostrovsky@xxxxxxxxxx; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu
> > <wei.liu2@xxxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>; Andrew Cooper
> > <Andrew.Cooper3@xxxxxxxxxx>
> > Subject: Re: [PATCH v2 1/9] xen/vpci: introduce basic handlers to trap
> > accesses to the PCI config space
> > IMHO I'm not sure Xen needs PCI register based trapping granularity. I would
> > argue that whatever (IOREQ or Xen internal function) that wants to trap
> > access
> > to a specific PCI config device register needs to take care of all the
> > registers for that device.
> > 
> 
> Having distinct handers for distinct groups of makes sense though... e.g. 
> being able to register a BAR handler for each BAR and then maybe an MSI-X 
> capability handler for wherever that appears in the capability chain, etc. If 
> you don't allow such registration at the top level then it ends up getting 
> done at the next level.

Yes, that's what's done here. Handlers for specific registers are added at the
next level (vPCI). See patches 5, 6, 8 or 9 for examples.

> That said, it may make more sense to have a top level of emulation that just 
> handles all register reads and writes to config space and then a second level 
> that has callbacks for BAR enumeration, bus master enable, MSI-X mask/unmask, 
> etc.
> 
> > I will look into hooking this code (vPCI) into the existing hvm_*_ioreq
> > functionality, so that vPCI claims the full PCI config space for each 
> > device it
> > manages.
> 
> Cool.

I've been looking into this, and I have to say this whole emulation handling is
a mess. The fact that Xen differentiates between internal and external (IOREQ)
handlers so early in the code (hvmemul_do_io) makes it far from trivial to
unify internal and external handlers, the more that external handlers have
grown a complex set of infrastructure that internal handlers don't have at
all.

Ideally I think the IOREQ filtering code should be generalized to apply to both
internal and external handlers, and the difference between external and
internal handlers should just be the set of functions that they use. External
ones would always use generic IOREQ functions for pushing requests to the
external emulators, while internal ones would just implement their own
functions.

That said, I think this is a non-trivial amount of work, that will further
delay this series. I don't see an easy way to integrate this code with the
current IOREQ code at all. I'm willing to do this, but I would rather have this
series merged first, so that other people can start working on PVH Dom0.

ATM, the only think I can see that could be easily shared between the IOREQ
code and vPCI is the PCI address decoding code.

Thanks, Roger.


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