All of lore.kernel.org
 help / color / mirror / Atom feed
From: Juergen Gross <jgross@suse.com>
To: xen-devel@lists.xenproject.org
Cc: Juergen Gross <jgross@suse.com>, Wei Liu <wl@xen.org>,
	Anthony PERARD <anthony.perard@citrix.com>
Subject: [PATCH] tools/libs/evtchn: fix locking in Mini-OS
Date: Mon, 11 Dec 2023 08:10:17 +0100	[thread overview]
Message-ID: <20231211071017.27752-1-jgross@suse.com> (raw)

When adding locking to tools/libs/evtchn/minios.c a semaphore was
used. This can result in deadlocks, as the lock is taken inside the
event handler, which can interrupt an already locked region.

The fix is rather simple, as Mini-OS is supporting a single vcpu
only. So instead of the semaphore it is enough to disable interrupts
when operating on the port list.

Fixes: bc4fe94a69d4 ("ools/libs/evtchn: replace assert()s in stubdom with proper locking")
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libs/evtchn/minios.c | 45 +++++++++++++-------------------------
 1 file changed, 15 insertions(+), 30 deletions(-)

diff --git a/tools/libs/evtchn/minios.c b/tools/libs/evtchn/minios.c
index c807e17f55..046cd28d37 100644
--- a/tools/libs/evtchn/minios.c
+++ b/tools/libs/evtchn/minios.c
@@ -25,7 +25,6 @@
 #include <mini-os/os.h>
 #include <mini-os/lib.h>
 #include <mini-os/events.h>
-#include <mini-os/semaphore.h>
 #include <mini-os/wait.h>
 
 #include <assert.h>
@@ -43,7 +42,6 @@ XEN_LIST_HEAD(port_list, struct port_info);
 
 struct ports {
     struct port_list list;
-    struct semaphore sem;
 };
 
 struct port_info {
@@ -58,6 +56,7 @@ static struct port_info *port_alloc(xenevtchn_handle *xce)
     struct port_info *port_info;
     struct file *file = get_file_from_fd(xce->fd);
     struct ports *ports = file->dev;
+    unsigned long flags;
 
     port_info = malloc(sizeof(struct port_info));
     if ( port_info == NULL )
@@ -67,19 +66,24 @@ static struct port_info *port_alloc(xenevtchn_handle *xce)
     port_info->port = -1;
     port_info->bound = false;
 
-    down(&ports->sem);
+    local_irq_save(flags);
     XEN_LIST_INSERT_HEAD(&ports->list, port_info, list);
-    up(&ports->sem);
+    local_irq_restore(flags);
 
     return port_info;
 }
 
 static void port_dealloc(struct port_info *port_info)
 {
+    unsigned long flags;
+
     if ( port_info->bound )
         unbind_evtchn(port_info->port);
 
+    local_irq_save(flags);
     XEN_LIST_REMOVE(port_info, list);
+    local_irq_restore(flags);
+
     free(port_info);
 }
 
@@ -135,7 +139,6 @@ int osdep_evtchn_open(xenevtchn_handle *xce, unsigned int flags)
 
     file->dev = ports;
     XEN_LIST_INIT(&ports->list);
-    init_SEMAPHORE(&ports->sem, 1);
     xce->fd = fd;
     printf("evtchn_open() -> %d\n", fd);
 
@@ -183,16 +186,11 @@ static void evtchn_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
     ports = file->dev;
     mask_evtchn(port);
 
-    down(&ports->sem);
     XEN_LIST_FOREACH(port_info, &ports->list, list)
     {
         if ( port_info->port == port )
-        {
-            up(&ports->sem);
             goto found;
-        }
     }
-    up(&ports->sem);
 
     printk("Unknown port %d for handle %d\n", port, xce->fd);
     return;
@@ -203,16 +201,6 @@ static void evtchn_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
     wake_up(&event_queue);
 }
 
-static void port_remove(xenevtchn_handle *xce, struct port_info *port_info)
-{
-    struct file *file = get_file_from_fd(xce->fd);
-    struct ports *ports = file->dev;
-
-    down(&ports->sem);
-    port_dealloc(port_info);
-    up(&ports->sem);
-}
-
 xenevtchn_port_or_error_t xenevtchn_bind_unbound_port(xenevtchn_handle *xce,
                                                       uint32_t domid)
 {
@@ -230,7 +218,7 @@ xenevtchn_port_or_error_t xenevtchn_bind_unbound_port(xenevtchn_handle *xce,
 
     if ( ret < 0 )
     {
-        port_remove(xce, port_info);
+        port_dealloc(port_info);
         errno = -ret;
         return -1;
     }
@@ -261,7 +249,7 @@ xenevtchn_port_or_error_t xenevtchn_bind_interdomain(xenevtchn_handle *xce,
 
     if ( ret < 0 )
     {
-        port_remove(xce, port_info);
+        port_dealloc(port_info);
         errno = -ret;
         return -1;
     }
@@ -279,18 +267,19 @@ int xenevtchn_unbind(xenevtchn_handle *xce, evtchn_port_t port)
     struct file *file = get_file_from_fd(fd);
     struct port_info *port_info;
     struct ports *ports = file->dev;
+    unsigned long flags;
 
-    down(&ports->sem);
+    local_irq_save(flags);
     XEN_LIST_FOREACH(port_info, &ports->list, list)
     {
         if ( port_info->port == port )
         {
             port_dealloc(port_info);
-            up(&ports->sem);
+            local_irq_restore(flags);
             return 0;
         }
     }
-    up(&ports->sem);
+    local_irq_restore(flags);
 
     printf("Warning: couldn't find port %"PRId32" for xc handle %x\n",
            port, fd);
@@ -315,7 +304,7 @@ xenevtchn_port_or_error_t xenevtchn_bind_virq(xenevtchn_handle *xce,
 
     if ( port < 0 )
     {
-        port_remove(xce, port_info);
+        port_dealloc(port_info);
         errno = -port;
         return -1;
     }
@@ -335,8 +324,6 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
     unsigned long flags;
     evtchn_port_t ret = -1;
 
-    down(&ports->sem);
-
     local_irq_save(flags);
 
     file->read = false;
@@ -360,8 +347,6 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
 
     local_irq_restore(flags);
 
-    up(&ports->sem);
-
     return ret;
 }
 
-- 
2.35.3



             reply	other threads:[~2023-12-11  7:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-11  7:10 Juergen Gross [this message]
2023-12-13 15:21 ` [PATCH] tools/libs/evtchn: fix locking in Mini-OS Anthony PERARD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231211071017.27752-1-jgross@suse.com \
    --to=jgross@suse.com \
    --cc=anthony.perard@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.