Re: [PATCH 01/12] xen/events: avoid race with raising an event in unmask_evtchn()

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Wed, 20 Mar 2013, David Vrabel wrote:
> On 20/03/13 11:00, Stefano Stabellini wrote:
> > On Tue, 19 Mar 2013, David Vrabel wrote:
> >> From: David Vrabel <david.vrabel@citrix.com>
> >>
> >> In unmask_evtchn(), when the mask bit is cleared after testing for
> >> pending and the event becomes pending between the test and clear, then
> >> the upcall will not become pending and the event may be lost or
> >> delayed.
> >>
> >> Avoid this by always clearing the mask bit before checking for
> >> pending.
> >>
> >> This fixes a regression introduced in 3.7 by
> >> b5e579232d635b79a3da052964cb357ccda8d9ea (xen/events: fix
> >> unmask_evtchn for PV on HVM guests) which reordered the clear mask and
> >> check pending operations.
> > 
> > The race you are trying to fix is real, but the fix you are proposing
> > breaks PV on HVM and ARM guests again.
> > 
> > From the description of b5e579232d635b79a3da052964cb357ccda8d9ea, it's
> > clear that the reason to call EVTCHNOP_unmask is to trigger an event
> > notification injection again.
> > But if you clear the evtchn_mask bit *before* calling EVTCHNOP_unmask,
> > EVTCHNOP_unmask won't reinject the event.
> > From evtchn_unmask:
> > 
> >     if ( test_and_clear_bit(port, &shared_info(d, evtchn_mask)) &&
> >          test_bit          (port, &shared_info(d, evtchn_pending)) &&
> >          !test_and_set_bit (port / BITS_PER_EVTCHN_WORD(d),
> >                             &vcpu_info(v, evtchn_pending_sel)) )
> >     {
> >         vcpu_mark_events_pending(v);
> >     }
> > 
> > The first condition for reinjection would fail.
> 
> I missed this.  The only way I can think of fixing this is to set the
> mask bit before call the unmask hypercall.

that might work