Re: [PATCH v6 2/3] xen/evtchn: rework per event channel lock

2020-11-22 Thread Jürgen Groß

On 18.11.20 14:19, Jan Beulich wrote:

On 09.11.2020 17:38, Juergen Gross wrote:

Currently the lock for a single event channel needs to be taken with
interrupts off, which causes deadlocks in some cases.

Rework the per event channel lock to be non-blocking for the case of
sending an event and removing the need for disabling interrupts for
taking the lock.

The lock is needed for avoiding races between event channel state
changes (creation, closing, binding) against normal operations (set
pending, [un]masking, priority changes).

Use a rwlock, but with some restrictions:

- Changing the state of an event channel (creation, closing, binding)
   needs to use write_lock(), with ASSERT()ing that the lock is taken as
   writer only when the state of the event channel is either before or
   after the locked region appropriate (either free or unbound).

- Sending an event needs to use read_trylock() mostly, in case of not
   obtaining the lock the operation is omitted. This is needed as
   sending an event can happen with interrupts off (at least in some
   cases).

- Dumping the event channel state for debug purposes is using
   read_trylock(), too, in order to avoid blocking in case the lock is
   taken as writer for a long time.

- All other cases can use read_lock().


One of the implications is that racing invocations of ->set_pending()
are now possible for the same port. Beyond what I said in reply to
0/3 already, I'm afraid there are (latent) issues:

1) The update of ->pending (or basically any bitfield in struct
evtchn, or yet more generically any field getting updated in a read-
modify-write fashion) is no longer generally safe in any of the
hooks called with just a read lock held. ->pending itself is not an
issue now merely because it shares storage only with xen_consumer,
which won't get updated once a port was bound.


This is fragile.

We should put the pending indicator into a dedicated byte.


2) Of two racing sends, one may now complete without the port
actually having got fully recorded as linked in the FIFO code. This
is because the party losing the race of setting EVTCHN_FIFO_LINKED
will return early, without regard to whether the winner has made
enough progress. (Of course this is possible only with an
intermediate queue change, as only then the lock would become
available to the second of the senders early enough.)


No, I don't think this is limited to a queue change. If a caller of
evtchn_fifo_set_pending() is being interrupted after setting
EVTCHN_FIFO_PENDING, and then a second caller can make it to setting
EVTCHN_FIFO_LINKED, the first caller won't even try to take the queue
lock, resulting in evtchn_check_pollers() being called before the
event might have been put properly into the queue.

I'd suggest to extend the fifo queue lock region in order to mitigate
this problem.



I've gone through other functions called from this path and didn't
find any further race potential there, but I'm not entirely certain
I didn't miss anything.


I can prepare a patch if you agree my ideas.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature


Re: [PATCH v6 2/3] xen/evtchn: rework per event channel lock

2020-11-18 Thread Jan Beulich
On 09.11.2020 17:38, Juergen Gross wrote:
> Currently the lock for a single event channel needs to be taken with
> interrupts off, which causes deadlocks in some cases.
> 
> Rework the per event channel lock to be non-blocking for the case of
> sending an event and removing the need for disabling interrupts for
> taking the lock.
> 
> The lock is needed for avoiding races between event channel state
> changes (creation, closing, binding) against normal operations (set
> pending, [un]masking, priority changes).
> 
> Use a rwlock, but with some restrictions:
> 
> - Changing the state of an event channel (creation, closing, binding)
>   needs to use write_lock(), with ASSERT()ing that the lock is taken as
>   writer only when the state of the event channel is either before or
>   after the locked region appropriate (either free or unbound).
> 
> - Sending an event needs to use read_trylock() mostly, in case of not
>   obtaining the lock the operation is omitted. This is needed as
>   sending an event can happen with interrupts off (at least in some
>   cases).
> 
> - Dumping the event channel state for debug purposes is using
>   read_trylock(), too, in order to avoid blocking in case the lock is
>   taken as writer for a long time.
> 
> - All other cases can use read_lock().

One of the implications is that racing invocations of ->set_pending()
are now possible for the same port. Beyond what I said in reply to
0/3 already, I'm afraid there are (latent) issues:

1) The update of ->pending (or basically any bitfield in struct
evtchn, or yet more generically any field getting updated in a read-
modify-write fashion) is no longer generally safe in any of the
hooks called with just a read lock held. ->pending itself is not an
issue now merely because it shares storage only with xen_consumer,
which won't get updated once a port was bound.

2) Of two racing sends, one may now complete without the port
actually having got fully recorded as linked in the FIFO code. This
is because the party losing the race of setting EVTCHN_FIFO_LINKED
will return early, without regard to whether the winner has made
enough progress. (Of course this is possible only with an
intermediate queue change, as only then the lock would become
available to the second of the senders early enough.)

I've gone through other functions called from this path and didn't
find any further race potential there, but I'm not entirely certain
I didn't miss anything.

Jan



Re: [PATCH v6 2/3] xen/evtchn: rework per event channel lock

2020-11-10 Thread Jan Beulich
On 10.11.2020 10:47, Julien Grall wrote:
> Hi,
> 
> On 10/11/2020 07:39, Jan Beulich wrote:
>> On 09.11.2020 18:46, Julien Grall wrote:
>>> Hi,
>>>
>>> On 09/11/2020 16:48, Jan Beulich wrote:
 On 09.11.2020 17:38, Juergen Gross wrote:
> Currently the lock for a single event channel needs to be taken with
> interrupts off, which causes deadlocks in some cases.
>
> Rework the per event channel lock to be non-blocking for the case of
> sending an event and removing the need for disabling interrupts for
> taking the lock.
>
> The lock is needed for avoiding races between event channel state
> changes (creation, closing, binding) against normal operations (set
> pending, [un]masking, priority changes).
>
> Use a rwlock, but with some restrictions:
>
> - Changing the state of an event channel (creation, closing, binding)
> needs to use write_lock(), with ASSERT()ing that the lock is taken as
> writer only when the state of the event channel is either before or
> after the locked region appropriate (either free or unbound).
>
> - Sending an event needs to use read_trylock() mostly, in case of not
> obtaining the lock the operation is omitted. This is needed as
> sending an event can happen with interrupts off (at least in some
> cases).
>
> - Dumping the event channel state for debug purposes is using
> read_trylock(), too, in order to avoid blocking in case the lock is
> taken as writer for a long time.
>
> - All other cases can use read_lock().
>
> Fixes: e045199c7c9c54 ("evtchn: address races with evtchn_reset()")
> Signed-off-by: Juergen Gross 
> ---
> V4:
> - switch to rwlock
> - add ASSERT() to verify correct write_lock() usage
>
> V3:
> - corrected a copy-and-paste error (Jan Beulich)
> - corrected unlocking in two cases (Jan Beulich)
> - renamed evtchn_read_trylock() (Jan Beulich)
> - added some comments and an ASSERT() for evtchn_write_lock()
> - set EVENT_WRITE_LOCK_INC to INT_MIN
>
> V2:
> - added needed barriers
>
> Signed-off-by: Juergen Gross 

 Reviewed-by: Jan Beulich 

 I'll give it overnight for others to possibly comment, but I'd
 like to get this in tomorrow if at all possible.
>>>
>>> IIRC, Citrix originally reported the issues. I would like to give them
>>> an opportunity to test the patches and confirm this effectively fixed
>>> there issues.
>>
>> I would consider waiting longer, but I'd like to get staging unblocked.
> 
> So the plan is to have the patches sitting in staging for a few days and 
> then backport?

Right, the usual thing - wait at least until a push has happened.
Unless we learn of problems with the change, I definitely want to
have this in for 4.14.1.

>> Just like with 52e1fc47abc3a0123 ("evtchn/Flask: pre-allocate node on
>> send path") we can always decide to revert / fix up afterwards. The
>> patch here surely is an improvement, even if it was to turn out it
>> doesn't fix all issues yes. I'd appreciate if you would confirm you
>> don't object to the patch going in - I'll wait a few more hours,
>> perhaps until around noon.
> I would suggest to wait until noon and if you don't get any answer, then 
> merge it.

Okay.

Jan



Re: [PATCH v6 2/3] xen/evtchn: rework per event channel lock

2020-11-10 Thread Julien Grall

Hi,

On 10/11/2020 07:39, Jan Beulich wrote:

On 09.11.2020 18:46, Julien Grall wrote:

Hi,

On 09/11/2020 16:48, Jan Beulich wrote:

On 09.11.2020 17:38, Juergen Gross wrote:

Currently the lock for a single event channel needs to be taken with
interrupts off, which causes deadlocks in some cases.

Rework the per event channel lock to be non-blocking for the case of
sending an event and removing the need for disabling interrupts for
taking the lock.

The lock is needed for avoiding races between event channel state
changes (creation, closing, binding) against normal operations (set
pending, [un]masking, priority changes).

Use a rwlock, but with some restrictions:

- Changing the state of an event channel (creation, closing, binding)
needs to use write_lock(), with ASSERT()ing that the lock is taken as
writer only when the state of the event channel is either before or
after the locked region appropriate (either free or unbound).

- Sending an event needs to use read_trylock() mostly, in case of not
obtaining the lock the operation is omitted. This is needed as
sending an event can happen with interrupts off (at least in some
cases).

- Dumping the event channel state for debug purposes is using
read_trylock(), too, in order to avoid blocking in case the lock is
taken as writer for a long time.

- All other cases can use read_lock().

Fixes: e045199c7c9c54 ("evtchn: address races with evtchn_reset()")
Signed-off-by: Juergen Gross 
---
V4:
- switch to rwlock
- add ASSERT() to verify correct write_lock() usage

V3:
- corrected a copy-and-paste error (Jan Beulich)
- corrected unlocking in two cases (Jan Beulich)
- renamed evtchn_read_trylock() (Jan Beulich)
- added some comments and an ASSERT() for evtchn_write_lock()
- set EVENT_WRITE_LOCK_INC to INT_MIN

V2:
- added needed barriers

Signed-off-by: Juergen Gross 


Reviewed-by: Jan Beulich 

I'll give it overnight for others to possibly comment, but I'd
like to get this in tomorrow if at all possible.


IIRC, Citrix originally reported the issues. I would like to give them
an opportunity to test the patches and confirm this effectively fixed
there issues.


I would consider waiting longer, but I'd like to get staging unblocked.


So the plan is to have the patches sitting in staging for a few days and 
then backport?



Just like with 52e1fc47abc3a0123 ("evtchn/Flask: pre-allocate node on
send path") we can always decide to revert / fix up afterwards. The
patch here surely is an improvement, even if it was to turn out it
doesn't fix all issues yes. I'd appreciate if you would confirm you
don't object to the patch going in - I'll wait a few more hours,
perhaps until around noon.
I would suggest to wait until noon and if you don't get any answer, then 
merge it.


Cheers,

--
Julien Grall



Re: [PATCH v6 2/3] xen/evtchn: rework per event channel lock

2020-11-09 Thread Jan Beulich
On 09.11.2020 18:46, Julien Grall wrote:
> Hi,
> 
> On 09/11/2020 16:48, Jan Beulich wrote:
>> On 09.11.2020 17:38, Juergen Gross wrote:
>>> Currently the lock for a single event channel needs to be taken with
>>> interrupts off, which causes deadlocks in some cases.
>>>
>>> Rework the per event channel lock to be non-blocking for the case of
>>> sending an event and removing the need for disabling interrupts for
>>> taking the lock.
>>>
>>> The lock is needed for avoiding races between event channel state
>>> changes (creation, closing, binding) against normal operations (set
>>> pending, [un]masking, priority changes).
>>>
>>> Use a rwlock, but with some restrictions:
>>>
>>> - Changing the state of an event channel (creation, closing, binding)
>>>needs to use write_lock(), with ASSERT()ing that the lock is taken as
>>>writer only when the state of the event channel is either before or
>>>after the locked region appropriate (either free or unbound).
>>>
>>> - Sending an event needs to use read_trylock() mostly, in case of not
>>>obtaining the lock the operation is omitted. This is needed as
>>>sending an event can happen with interrupts off (at least in some
>>>cases).
>>>
>>> - Dumping the event channel state for debug purposes is using
>>>read_trylock(), too, in order to avoid blocking in case the lock is
>>>taken as writer for a long time.
>>>
>>> - All other cases can use read_lock().
>>>
>>> Fixes: e045199c7c9c54 ("evtchn: address races with evtchn_reset()")
>>> Signed-off-by: Juergen Gross 
>>> ---
>>> V4:
>>> - switch to rwlock
>>> - add ASSERT() to verify correct write_lock() usage
>>>
>>> V3:
>>> - corrected a copy-and-paste error (Jan Beulich)
>>> - corrected unlocking in two cases (Jan Beulich)
>>> - renamed evtchn_read_trylock() (Jan Beulich)
>>> - added some comments and an ASSERT() for evtchn_write_lock()
>>> - set EVENT_WRITE_LOCK_INC to INT_MIN
>>>
>>> V2:
>>> - added needed barriers
>>>
>>> Signed-off-by: Juergen Gross 
>>
>> Reviewed-by: Jan Beulich 
>>
>> I'll give it overnight for others to possibly comment, but I'd
>> like to get this in tomorrow if at all possible.
> 
> IIRC, Citrix originally reported the issues. I would like to give them 
> an opportunity to test the patches and confirm this effectively fixed 
> there issues.

I would consider waiting longer, but I'd like to get staging unblocked.
Just like with 52e1fc47abc3a0123 ("evtchn/Flask: pre-allocate node on
send path") we can always decide to revert / fix up afterwards. The
patch here surely is an improvement, even if it was to turn out it
doesn't fix all issues yes. I'd appreciate if you would confirm you
don't object to the patch going in - I'll wait a few more hours,
perhaps until around noon.

Jan



Re: [PATCH v6 2/3] xen/evtchn: rework per event channel lock

2020-11-09 Thread Jürgen Groß

On 09.11.20 18:46, Julien Grall wrote:

Hi,

On 09/11/2020 16:48, Jan Beulich wrote:

On 09.11.2020 17:38, Juergen Gross wrote:

Currently the lock for a single event channel needs to be taken with
interrupts off, which causes deadlocks in some cases.

Rework the per event channel lock to be non-blocking for the case of
sending an event and removing the need for disabling interrupts for
taking the lock.

The lock is needed for avoiding races between event channel state
changes (creation, closing, binding) against normal operations (set
pending, [un]masking, priority changes).

Use a rwlock, but with some restrictions:

- Changing the state of an event channel (creation, closing, binding)
   needs to use write_lock(), with ASSERT()ing that the lock is taken as
   writer only when the state of the event channel is either before or
   after the locked region appropriate (either free or unbound).

- Sending an event needs to use read_trylock() mostly, in case of not
   obtaining the lock the operation is omitted. This is needed as
   sending an event can happen with interrupts off (at least in some
   cases).

- Dumping the event channel state for debug purposes is using
   read_trylock(), too, in order to avoid blocking in case the lock is
   taken as writer for a long time.

- All other cases can use read_lock().

Fixes: e045199c7c9c54 ("evtchn: address races with evtchn_reset()")
Signed-off-by: Juergen Gross 
---
V4:
- switch to rwlock
- add ASSERT() to verify correct write_lock() usage

V3:
- corrected a copy-and-paste error (Jan Beulich)
- corrected unlocking in two cases (Jan Beulich)
- renamed evtchn_read_trylock() (Jan Beulich)
- added some comments and an ASSERT() for evtchn_write_lock()
- set EVENT_WRITE_LOCK_INC to INT_MIN

V2:
- added needed barriers

Signed-off-by: Juergen Gross 


Reviewed-by: Jan Beulich 

I'll give it overnight for others to possibly comment, but I'd
like to get this in tomorrow if at all possible.


IIRC, Citrix originally reported the issues. I would like to give them 
an opportunity to test the patches and confirm this effectively fixed 
there issues.


@Roger, @Andrew, could you give a try to confirm this unblock the vm 
event issue?


It should be noted that this series ought to fix current unstable
failures of XSM tests, as those are triggering the advanced lock checks
of rwlocks due to the per-event channel lock disabling interrupts.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature


Re: [PATCH v6 2/3] xen/evtchn: rework per event channel lock

2020-11-09 Thread Julien Grall

Hi,

On 09/11/2020 16:48, Jan Beulich wrote:

On 09.11.2020 17:38, Juergen Gross wrote:

Currently the lock for a single event channel needs to be taken with
interrupts off, which causes deadlocks in some cases.

Rework the per event channel lock to be non-blocking for the case of
sending an event and removing the need for disabling interrupts for
taking the lock.

The lock is needed for avoiding races between event channel state
changes (creation, closing, binding) against normal operations (set
pending, [un]masking, priority changes).

Use a rwlock, but with some restrictions:

- Changing the state of an event channel (creation, closing, binding)
   needs to use write_lock(), with ASSERT()ing that the lock is taken as
   writer only when the state of the event channel is either before or
   after the locked region appropriate (either free or unbound).

- Sending an event needs to use read_trylock() mostly, in case of not
   obtaining the lock the operation is omitted. This is needed as
   sending an event can happen with interrupts off (at least in some
   cases).

- Dumping the event channel state for debug purposes is using
   read_trylock(), too, in order to avoid blocking in case the lock is
   taken as writer for a long time.

- All other cases can use read_lock().

Fixes: e045199c7c9c54 ("evtchn: address races with evtchn_reset()")
Signed-off-by: Juergen Gross 
---
V4:
- switch to rwlock
- add ASSERT() to verify correct write_lock() usage

V3:
- corrected a copy-and-paste error (Jan Beulich)
- corrected unlocking in two cases (Jan Beulich)
- renamed evtchn_read_trylock() (Jan Beulich)
- added some comments and an ASSERT() for evtchn_write_lock()
- set EVENT_WRITE_LOCK_INC to INT_MIN

V2:
- added needed barriers

Signed-off-by: Juergen Gross 


Reviewed-by: Jan Beulich 

I'll give it overnight for others to possibly comment, but I'd
like to get this in tomorrow if at all possible.


IIRC, Citrix originally reported the issues. I would like to give them 
an opportunity to test the patches and confirm this effectively fixed 
there issues.


@Roger, @Andrew, could you give a try to confirm this unblock the vm 
event issue?


Cheers,

--
Julien Grall



Re: [PATCH v6 2/3] xen/evtchn: rework per event channel lock

2020-11-09 Thread Julien Grall

Hi Juergen,

On 09/11/2020 16:38, Juergen Gross wrote:

Currently the lock for a single event channel needs to be taken with
interrupts off, which causes deadlocks in some cases.

Rework the per event channel lock to be non-blocking for the case of
sending an event and removing the need for disabling interrupts for
taking the lock.

The lock is needed for avoiding races between event channel state
changes (creation, closing, binding) against normal operations (set
pending, [un]masking, priority changes).

Use a rwlock, but with some restrictions:

- Changing the state of an event channel (creation, closing, binding)
   needs to use write_lock(), with ASSERT()ing that the lock is taken as
   writer only when the state of the event channel is either before or
   after the locked region appropriate (either free or unbound).

- Sending an event needs to use read_trylock() mostly, in case of not
   obtaining the lock the operation is omitted. This is needed as
   sending an event can happen with interrupts off (at least in some
   cases).

- Dumping the event channel state for debug purposes is using
   read_trylock(), too, in order to avoid blocking in case the lock is
   taken as writer for a long time.

- All other cases can use read_lock().

Fixes: e045199c7c9c54 ("evtchn: address races with evtchn_reset()")
Signed-off-by: Juergen Gross 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall



Re: [PATCH v6 2/3] xen/evtchn: rework per event channel lock

2020-11-09 Thread Jan Beulich
On 09.11.2020 17:38, Juergen Gross wrote:
> Currently the lock for a single event channel needs to be taken with
> interrupts off, which causes deadlocks in some cases.
> 
> Rework the per event channel lock to be non-blocking for the case of
> sending an event and removing the need for disabling interrupts for
> taking the lock.
> 
> The lock is needed for avoiding races between event channel state
> changes (creation, closing, binding) against normal operations (set
> pending, [un]masking, priority changes).
> 
> Use a rwlock, but with some restrictions:
> 
> - Changing the state of an event channel (creation, closing, binding)
>   needs to use write_lock(), with ASSERT()ing that the lock is taken as
>   writer only when the state of the event channel is either before or
>   after the locked region appropriate (either free or unbound).
> 
> - Sending an event needs to use read_trylock() mostly, in case of not
>   obtaining the lock the operation is omitted. This is needed as
>   sending an event can happen with interrupts off (at least in some
>   cases).
> 
> - Dumping the event channel state for debug purposes is using
>   read_trylock(), too, in order to avoid blocking in case the lock is
>   taken as writer for a long time.
> 
> - All other cases can use read_lock().
> 
> Fixes: e045199c7c9c54 ("evtchn: address races with evtchn_reset()")
> Signed-off-by: Juergen Gross 
> ---
> V4:
> - switch to rwlock
> - add ASSERT() to verify correct write_lock() usage
> 
> V3:
> - corrected a copy-and-paste error (Jan Beulich)
> - corrected unlocking in two cases (Jan Beulich)
> - renamed evtchn_read_trylock() (Jan Beulich)
> - added some comments and an ASSERT() for evtchn_write_lock()
> - set EVENT_WRITE_LOCK_INC to INT_MIN
> 
> V2:
> - added needed barriers
> 
> Signed-off-by: Juergen Gross 

Reviewed-by: Jan Beulich 

I'll give it overnight for others to possibly comment, but I'd
like to get this in tomorrow if at all possible.

I also think this will want backporting beyond just the fully
maintained branches.

Jan



[PATCH v6 2/3] xen/evtchn: rework per event channel lock

2020-11-09 Thread Juergen Gross
Currently the lock for a single event channel needs to be taken with
interrupts off, which causes deadlocks in some cases.

Rework the per event channel lock to be non-blocking for the case of
sending an event and removing the need for disabling interrupts for
taking the lock.

The lock is needed for avoiding races between event channel state
changes (creation, closing, binding) against normal operations (set
pending, [un]masking, priority changes).

Use a rwlock, but with some restrictions:

- Changing the state of an event channel (creation, closing, binding)
  needs to use write_lock(), with ASSERT()ing that the lock is taken as
  writer only when the state of the event channel is either before or
  after the locked region appropriate (either free or unbound).

- Sending an event needs to use read_trylock() mostly, in case of not
  obtaining the lock the operation is omitted. This is needed as
  sending an event can happen with interrupts off (at least in some
  cases).

- Dumping the event channel state for debug purposes is using
  read_trylock(), too, in order to avoid blocking in case the lock is
  taken as writer for a long time.

- All other cases can use read_lock().

Fixes: e045199c7c9c54 ("evtchn: address races with evtchn_reset()")
Signed-off-by: Juergen Gross 
---
V4:
- switch to rwlock
- add ASSERT() to verify correct write_lock() usage

V3:
- corrected a copy-and-paste error (Jan Beulich)
- corrected unlocking in two cases (Jan Beulich)
- renamed evtchn_read_trylock() (Jan Beulich)
- added some comments and an ASSERT() for evtchn_write_lock()
- set EVENT_WRITE_LOCK_INC to INT_MIN

V2:
- added needed barriers

Signed-off-by: Juergen Gross 
---
 xen/arch/x86/irq.c |   6 +-
 xen/arch/x86/pv/shim.c |   9 +--
 xen/common/event_channel.c | 138 +
 xen/include/xen/event.h|  29 ++--
 xen/include/xen/sched.h|   5 +-
 5 files changed, 112 insertions(+), 75 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 93c4fb9a79..8d1f9a9fc6 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2495,14 +2495,12 @@ static void dump_irqs(unsigned char key)
 pirq = domain_irq_to_pirq(d, irq);
 info = pirq_info(d, pirq);
 evtchn = evtchn_from_port(d, info->evtchn);
-local_irq_disable();
-if ( spin_trylock(>lock) )
+if ( evtchn_read_trylock(evtchn) )
 {
 pending = evtchn_is_pending(d, evtchn);
 masked = evtchn_is_masked(d, evtchn);
-spin_unlock(>lock);
+evtchn_read_unlock(evtchn);
 }
-local_irq_enable();
 printk("d%d:%3d(%c%c%c)%c",
d->domain_id, pirq, "-P?"[pending],
"-M?"[masked], info->masked ? 'M' : '-',
diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index 9aef7a860a..b4e83e0778 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -660,11 +660,12 @@ void pv_shim_inject_evtchn(unsigned int port)
 if ( port_is_valid(guest, port) )
 {
 struct evtchn *chn = evtchn_from_port(guest, port);
-unsigned long flags;
 
-spin_lock_irqsave(>lock, flags);
-evtchn_port_set_pending(guest, chn->notify_vcpu_id, chn);
-spin_unlock_irqrestore(>lock, flags);
+if ( evtchn_read_trylock(chn) )
+{
+evtchn_port_set_pending(guest, chn->notify_vcpu_id, chn);
+evtchn_read_unlock(chn);
+}
 }
 }
 
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index cd4a2c0501..43e3520df6 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -50,6 +50,29 @@
 
 #define consumer_is_xen(e) (!!(e)->xen_consumer)
 
+/*
+ * Lock an event channel exclusively. This is allowed only when the channel is
+ * free or unbound either when taking or when releasing the lock, as any
+ * concurrent operation on the event channel using evtchn_read_trylock() will
+ * just assume the event channel is free or unbound at the moment when the
+ * evtchn_read_trylock() returns false.
+ */
+static inline void evtchn_write_lock(struct evtchn *evtchn)
+{
+write_lock(>lock);
+
+evtchn->old_state = evtchn->state;
+}
+
+static inline void evtchn_write_unlock(struct evtchn *evtchn)
+{
+/* Enforce lock discipline. */
+ASSERT(evtchn->old_state == ECS_FREE || evtchn->old_state == ECS_UNBOUND ||
+   evtchn->state == ECS_FREE || evtchn->state == ECS_UNBOUND);
+
+write_unlock(>lock);
+}
+
 /*
  * The function alloc_unbound_xen_event_channel() allows an arbitrary
  * notifier function to be specified. However, very few unique functions
@@ -133,7 +156,7 @@ static struct evtchn *alloc_evtchn_bucket(struct domain *d, 
unsigned int port)
 return NULL;
 }
 chn[i].port = port + i;
-