On Mon, 25 Mar 2013, David Vrabel wrote: > From: David Vrabel <[email protected]> > > 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. If a hypercall is needed, remask the event as > EVTCHNOP_unmask will only retrigger pending events if they were > masked. > > 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. > > Signed-off-by: David Vrabel <[email protected]> > Cc: [email protected] > Cc: Stefano Stabellini <[email protected]>
It looks correct to me. Assuming that you tested it with a PV on HVM guest: Acked-by: Stefano Stabellini <[email protected]> > Changes in v2: > - set mask before hypercall. > --- > drivers/xen/events.c | 20 +++++++++++++++----- > 1 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/xen/events.c b/drivers/xen/events.c > index d17aa41..aa85881 100644 > --- a/drivers/xen/events.c > +++ b/drivers/xen/events.c > @@ -403,11 +403,23 @@ static void unmask_evtchn(int port) > > if (unlikely((cpu != cpu_from_evtchn(port)))) > do_hypercall = 1; > - else > + else { > + /* > + * Need to clear the mask before checking pending to > + * avoid a race with an event becoming pending. > + * > + * EVTCHNOP_unmask will only trigger an upcall if the > + * mask bit was set, so if a hypercall is needed > + * remask the event. > + */ > + sync_clear_bit(port, BM(&s->evtchn_mask[0])); > evtchn_pending = sync_test_bit(port, BM(&s->evtchn_pending[0])); > > - if (unlikely(evtchn_pending && xen_hvm_domain())) > - do_hypercall = 1; > + if (unlikely(evtchn_pending && xen_hvm_domain())) { > + sync_set_bit(port, BM(&s->evtchn_mask[0])); > + do_hypercall = 1; > + } > + } > > /* Slow path (hypercall) if this is a non-local port or if this is > * an hvm domain and an event is pending (hvm domains don't have > @@ -418,8 +430,6 @@ static void unmask_evtchn(int port) > } else { > struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu); > > - sync_clear_bit(port, BM(&s->evtchn_mask[0])); > - > /* > * The following is basically the equivalent of > * 'hw_resend_irq'. Just like a real IO-APIC we 'lose > -- > 1.7.2.5 > -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
