[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v5] xen/arm: trap guest WFI



On Fri, 2013-04-19 at 11:01 +0100, Stefano Stabellini wrote:
> On Fri, 19 Apr 2013, Ian Campbell wrote:
> > > > >  }
> > > > >  
> > > > >  static inline void local_event_delivery_enable(void)
> > > > 
> > > > This one is called from do_block in common code but if the guest never
> > > > sets this then it seems a bit pointless. Perhaps this should be
> > > > manipulating CPSR_I instead?
> > > 
> > > I think it's fine to keep it as it is because in the future a guest on
> > > ARM might decide to use it and I think it's an interface that we should
> > > support.
> > 
> > No it isn't, it is a piece of PV legacy which introduces difficult to
> > solve races (all that critical section stuff) on the event re-enable
> > path. There is no reason at all to use it when you have proper
> > virtualised interrupt mask functionality in the hardware.
> 
> evtchn_upcall_mask is part of struct vcpu_info that is defined in
> xen/include/public/xen.h, that is an arch-independent public header.
> 
> So yes, it is part of the interface that we support.

There is all sorts of stuff under xen/include/public which is not part
of the interface on ARM, PVMMU, start_info, etc. You wouldn't say we
support those, would you?

> I fail to see an easy way to remove it from it, we can't just add a
> comment say "please don't use it".

We should take the same approach as with the startinfo, see
<1363178714-5085-1-git-send-email-ian.campbell@xxxxxxxxxx> which is
acked but not yet applied. which is to add a sementic (not arch
specific) define which the archs can opt in or out of.

> I wouldn't be happy with ifdef'ing it out because I would rather keep
> this interface arch-independent.

Right, this shouldn't be an ifdef __arm__ or anything like that. It
should be ifdef XEN_HAVE_PV_INTERRUPT_MASK or some name along those
lines.

> However I do see an easy way to support it on ARM.

I don't think it is actually usable at all even with this patch. But
more importantly the interface is redundant, worse than the other
alternative and actually implementing support for it in the guest is
complex and error prone.

Ian.


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.