From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH 01/12] xen/events: avoid race with raising an event in unmask_evtchn() Date: Wed, 20 Mar 2013 12:21:35 +0000 Message-ID: References: <1363727099-25519-1-git-send-email-david.vrabel@citrix.com> <1363727099-25519-2-git-send-email-david.vrabel@citrix.com> <5149A970.3090804@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5149A970.3090804@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: David Vrabel Cc: Wei Liu , "xen-devel@lists.xen.org" , "Keir (Xen.org)" , Konrad Rzeszutek Wilk , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org 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 > >> > >> 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