Re: [Xen-devel] [PATCH v6 1/7] xen/x86: remove XENFEAT_hvm_pirqs for PVHv2 guests

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

On Mon, Feb 13, 2017 at 07:33:17AM -0700, Jan Beulich wrote:
> >>> On 13.02.17 at 15:22, <roger.pau@xxxxxxxxxx> wrote:
> > On Mon, Feb 13, 2017 at 06:09:27AM -0700, Jan Beulich wrote:
> >> >>> On 10.02.17 at 13:33, <roger.pau@xxxxxxxxxx> wrote:
> >> > --- a/docs/misc/hvmlite.markdown
> >> > +++ b/docs/misc/hvmlite.markdown
> >> > @@ -75,3 +75,23 @@ info structure that's passed at boot time (field 
> >> > rsdp_paddr).
> >> >  
> >> >  Description of paravirtualized devices will come from XenStore, just as 
> >> > it's
> >> >  done for HVM guests.
> >> > +
> >> > +## Interrupts ##
> >> > +
> >> > +### Interrupts from physical devices ###
> >> > +
> >> > +Interrupts from physical devices are delivered using native methods, 
> >> > this is
> >> > +done in order to take advantage of new hardware assisted virtualization
> >> > +functions, like posted interrupts. This implies that PVHv2 guests with 
> >> > physical
> >> > +devices will also have the necessary interrupt controllers in order to 
> >> > manage
> >> > +the delivery of interrupts from those devices, using the same 
> >> > interfaces that
> >> > +are available on native hardware.
> >> > +
> >> > +### Interrupts from paravirtualized devices ###
> >> > +
> >> > +Interrupts from paravirtualized devices are delivered using event 
> >> > channels, see
> >> > +[Event Channel Internals][event_channels] for more detailed information 
> >> > about
> >> > +event channels. Delivery of those interrupts can be configured in the 
> >> > same way
> >> > +as HVM guests, check xen/include/public/hvm/params.h and
> >> > +xen/include/public/hvm/hvm_op.h for more information about available 
> >> > delivery
> >> > +methods.
> >> 
> >> Just FTR - my comments on v4 still stand; I especially don't buy
> >> your argument [1] of posted interrupts becoming more widely
> >> available, since - as pointed out before - we continue to not fully
> >> enable their use by default, due to unresolved problems with
> >> the implementation. Furthermore emulation_flags_ok() allows
> >> EMU_LAPIC to be clear (together with all other bits being clear),
> >> in which case posted interrupts can't be used at all afaict.
> > 
> > It only allows it for DomUs, which don't support passthrough at all. If a
> > device is passed thought, a LAPIC will be required.
> 
> Is that spelled out anywhere? And I don't recall any fixme
> comments regarding pass-through not working.

I'm not sure there's anywhere were a FIXME would be appropriate, this is not
something that needs fixes, it's something that simply doesn't yet exist.

Maybe libxl should be slightly adjusted to print a nice error message if
someone attempts adding a PCI device to a PVHv2 guest.

It is currently being discussed with the ARM guys also [0], in order to try to
come up with a similar model for both ARM and PVHv2 (except of course for the
arch-specific bits).

> > Also note that the current set of physdev ops available to PVHv2 is 
> > completely
> > wrong. Since PVHv2 is considered a HVM guest from Xen PoV, it will take the
> > is_hvm_domain branch at the top of physdev_map_pirq, which will certainly 
> > lead
> > to all kinds of fun, because that's the weird path used in (PV)HVM guests 
> > that
> > allows them to remap interrupts from emulated or physical devices into event
> > channels. So at the moment, I think this should even be considered a 
> > bug-fix.
> 
> I think I had indicated that I'm okay with the physdev ops change
> for now; i.e. ...
> 
> > As I think I've said before, I don't think we should initially follow this
> > route for PVHv2, but in any case this can always be added afterwards by
> > re-enabling the XENFEAT_hvm_pirqs flag and fixing the physdev ops so they 
> > can
> > work properly in this context.
> 
> ... please note that my comments are specifically against the
> document, not the code (anymore). And the document should
> spell out options, even if their implementation may be deferred.

The document simply describes what I plan to implement myself, so if someone
wants to work on those bits (physdev ops), he/she is free to do it and expand
the document appropriately. I simply don't plan to implement them myself,
because I don't see much value in it and it's an interface that I would rather
get rid of.

Roger.

[0] https://lists.xenproject.org/archives/html/xen-devel/2016-12/msg03032.html

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