Re: [PATCH 04/12] evtchn: evtchn_set_priority() needs to acquire the per-channel lock

[Julien Grall]

Hi Jan,

On 28/09/2020 11:57, Jan Beulich wrote:

evtchn_fifo_set_pending() (invoked with the per-channel lock held) has
two uses of the channel's priority field. The field gets updated by
evtchn_fifo_set_priority() with only the per-domain event_lock held,
i.e. the two reads may observe two different values. While the 2nd use
could - afaict - in principle be replaced by q->priority, I think
evtchn_set_priority() should acquire the per-channel lock in any event.

Signed-off-by: Jan Beulich <jbeul...@suse.com>

--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1132,7 +1132,9 @@ static long evtchn_set_priority(const st
  {
      struct domain *d = current->domain;
      unsigned int port = set_priority->port;
+    struct evtchn *chn;
      long ret;
+    unsigned long flags;

spin_lock(&d->event_lock);

Is it still necessary to hold d->event_lock?

@@ -1142,8 +1144,10 @@ static long evtchn_set_priority(const st

          return -EINVAL;
      }

- ret = evtchn_port_set_priority(d, evtchn_from_port(d, port),

-                                   set_priority->priority);
+    chn = evtchn_from_port(d, port);
+    spin_lock_irqsave(&chn->lock, flags);
+    ret = evtchn_port_set_priority(d, chn, set_priority->priority);
+    spin_unlock_irqrestore(&chn->lock, flags);

spin_unlock(&d->event_lock);

Cheers,

--
Julien Grall