Re: [PATCH 1/3] x86/xen: use memory barriers when enabling local irqs

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 08/13/2013 10:31 AM, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> Because vcpu->evtchn_upcall_mask and vcpu->evtchn_upcall_pending are
> be written by Xen as well as the guest, using barrier() (a
> compiler-only barrier) in xen_enable_irq() and xen_restore_fl() is not
> sufficient.

Unneeded 'be' and xen_enable_irq -> xen_irq_enable
>
> Use mb() (a full memory barrier) instead.

Are evtchn_upcall_mask and evtchn_upcall_pending written from the same
(physical) processor during the potential race? If yes then I am not sure
this will make any difference since I think sysret/iret, syscall and 
interrupts
have implicit mfence.

It won't hurt to have mb(), all I am trying to say that this may not be 
the cause
of lost events.

-boris

>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>   arch/x86/xen/irq.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> index 01a4dc0..1a8d0d4 100644
> --- a/arch/x86/xen/irq.c
> +++ b/arch/x86/xen/irq.c
> @@ -60,7 +60,7 @@ static void xen_restore_fl(unsigned long flags)
>   
>   	if (flags == 0) {
>   		preempt_check_resched();
> -		barrier(); /* unmask then check (avoid races) */
> +		mb(); /* unmask then check (avoid races) */
>   		if (unlikely(vcpu->evtchn_upcall_pending))
>   			xen_force_evtchn_callback();
>   	}
> @@ -93,7 +93,7 @@ static void xen_irq_enable(void)
>   	/* Doesn't matter if we get preempted here, because any
>   	   pending event will get dealt with anyway. */
>   
> -	barrier(); /* unmask then check (avoid races) */
> +	mb(); /* unmask then check (avoid races) */
>   	if (unlikely(vcpu->evtchn_upcall_pending))
>   		xen_force_evtchn_callback();
>   }