Rpmsg: rpmsg services should be properly destroyed after calling rpmsg_device_destroy()

drivers/rpmsg/rpmsg.c

@@ -232,6 +232,7 @@ int rpmsg_register_callback(FAR void *priv,
   cb->ns_bind        = ns_bind;
 
   nxrmutex_lock(&g_rpmsg_lock);
+  metal_list_add_tail(&g_rpmsg_cb, &cb->node);
   metal_list_for_each(&g_rpmsg, node)
     {
       FAR struct rpmsg_s *rpmsg =
@@ -275,7 +276,6 @@ int rpmsg_register_callback(FAR void *priv,
        nxrmutex_unlock(&rpmsg->lock);
     }
 
-  metal_list_add_tail(&g_rpmsg_cb, &cb->node);
   nxrmutex_unlock(&g_rpmsg_lock);
 
   return 0;
@@ -418,6 +418,7 @@ void rpmsg_device_destory(FAR struct rpmsg_s *rpmsg)
 {
   FAR struct metal_list *node;
   FAR struct metal_list *tmp;
+  FAR struct rpmsg_endpoint *ept;
 
 #ifdef CONFIG_RPMSG_PING
   rpmsg_ping_deinit(&rpmsg->ping);
@@ -450,6 +451,24 @@ void rpmsg_device_destory(FAR struct rpmsg_s *rpmsg)
     }
 
   nxrmutex_unlock(&g_rpmsg_lock);
+
+  /* Release all ept attached to current rpmsg device */
+
+  metal_mutex_acquire(&rpmsg->rdev->lock);
+  metal_list_for_each_safe(&rpmsg->rdev->endpoints, tmp, node)
+    {
+      ept = metal_container_of(node, struct rpmsg_endpoint, node);
+      if (ept->ns_unbind_cb)
+        {
+          ept->ns_unbind_cb(ept);
+        }
+      else
+        {
+          rpmsg_destroy_ept(ept);
+        }
+    }
+
+  metal_mutex_release(&rpmsg->rdev->lock);
 }
 
 int rpmsg_register(FAR const char *path, FAR struct rpmsg_s *rpmsg,

drivers/rpmsg/rpmsg_port.c

@@ -612,9 +612,6 @@ int rpmsg_port_initialize(FAR struct rpmsg_port_s *port,
   rdev->support_ack = true;
   rdev->support_ns = true;
 
-  rpmsg_register_endpoint(rdev, &rdev->ns_ept, "NS", RPMSG_NS_EPT_ADDR,
-                          RPMSG_NS_EPT_ADDR, rpmsg_port_ns_callback, NULL,
-                          port);
   port->ops->register_callback(port, rpmsg_port_rx_callback);
 
   return 0;
@@ -626,22 +623,9 @@ int rpmsg_port_initialize(FAR struct rpmsg_port_s *port,
 
 void rpmsg_port_uninitialize(FAR struct rpmsg_port_s *port)
 {
-  FAR struct rpmsg_device *rdev = &port->rdev;
-  FAR struct metal_list *node;
-  FAR struct rpmsg_endpoint *ept;
-
-  while (!metal_list_is_empty(&rdev->endpoints))
-    {
-      node = rdev->endpoints.next;
-      ept = metal_container_of(node, struct rpmsg_endpoint, node);
-      rpmsg_destroy_ept(ept);
-      if (ept->ns_unbind_cb)
-        {
-          ept->ns_unbind_cb(ept);
-        }
-    }
+  FAR struct rpmsg_s *rpmsg = &port->rpmsg;
 
-  metal_mutex_deinit(&rdev->lock);
+  metal_mutex_deinit(&rpmsg->rdev->lock);
   rpmsg_port_destroy_queues(port);
 }
 
@@ -747,6 +731,9 @@ int rpmsg_port_register(FAR struct rpmsg_port_s *port,
       return ret;
     }
 
+  rpmsg_register_endpoint(&port->rdev, &port->rdev.ns_ept, "NS",
+                          RPMSG_NS_EPT_ADDR, RPMSG_NS_EPT_ADDR,
+                          rpmsg_port_ns_callback, NULL, port);
   rpmsg_device_created(&port->rpmsg);
   return ret;
 }
@@ -760,9 +747,9 @@ void rpmsg_port_unregister(FAR struct rpmsg_port_s *port)
   char name[64];
 
   snprintf(name, sizeof(name), "/dev/rpmsg/%s", port->cpuname);
-  rpmsg_unregister(name, &port->rpmsg);
 
   rpmsg_device_destory(&port->rpmsg);
+  rpmsg_unregister(name, &port->rpmsg);
 }
 
 /****************************************************************************
@@ -812,8 +799,7 @@ static void rpmsg_port_dump(FAR struct rpmsg_s *rpmsg)
   FAR struct metal_list *node;
   bool needunlock = false;
 
-  if (!up_interrupt_context() && !sched_idletask() &&
-      !nxmutex_is_hold(&rdev->lock))
+  if (!up_interrupt_context() && !sched_idletask())
     {
       metal_mutex_acquire(&rdev->lock);
       needunlock = true;

drivers/rpmsg/rpmsg_virtio_lite.c

@@ -496,8 +496,7 @@ static void rpmsg_virtio_lite_dump(FAR struct rpmsg_s *rpmsg)
       return;
     }
 
-  if (up_interrupt_context() || sched_idletask() ||
-      nxmutex_is_hold(&rdev->lock))
+  if (up_interrupt_context() || sched_idletask())
     {
       needlock = false;
     }

drivers/rptun/rptun.c

@@ -711,8 +711,7 @@ static void rptun_dump(FAR struct rpmsg_s *rpmsg)
       return;
     }
 
-  if (up_interrupt_context() || sched_idletask() ||
-      nxmutex_is_hold(&rdev->lock))
+  if (up_interrupt_context() || sched_idletask())
     {
       needlock = false;
     }

openamp/0001-libmetal-cmake-set-HAVE_STDATOMIC_H-default-true-in-.patch

@@ -1,8 +1,8 @@
-From fad5ddc80345f97a0c7b20c545502ac1f8e321a5 Mon Sep 17 00:00:00 2001
+From f959a2b5acccd2045c7277c3f3ddefe9bea08887 Mon Sep 17 00:00:00 2001
 From: Bowen Wang <wangbowen6@xiaomi.com>
 Date: Thu, 18 Dec 2025 17:26:13 +0800
-Subject: [PATCH] libmetal(cmake):set HAVE_STDATOMIC_H default true in NuttX
- platform
+Subject: [PATCH 1/3] libmetal(cmake):set HAVE_STDATOMIC_H default true in
+ NuttX platform
 
 Signed-off-by: Bowen Wang <wangbowen6@xiaomi.com>
 ---

openamp/0002-libmetal-atomic-enable-64-bit-atomic-by-toolchain-bu.patch

@@ -1,7 +1,7 @@
-From 61756465a3dc241297b7a12d5500ac36a5ffa070 Mon Sep 17 00:00:00 2001
+From 4aaf8a78da2ac2e9627fcbbefa72db7330a57909 Mon Sep 17 00:00:00 2001
 From: chao an <anchao@lixiang.com>
 Date: Sat, 29 Jun 2024 09:40:26 +0800
-Subject: [PATCH 2/5] libmetal/atomic: enable 64-bit atomic by toolchain
+Subject: [PATCH 2/3] libmetal/atomic: enable 64-bit atomic by toolchain
  builtin flags
 
 Fix compile error:

openamp/0003-mutex-change-the-libmetal-nuttx-mutex-to-recursive-m.patch

@@ -0,0 +1,80 @@
+From ef06cf5ac9be793200783f24996d529db715ac43 Mon Sep 17 00:00:00 2001
+From: Bowen Wang <wangbowen6@xiaomi.com>
+Date: Mon, 25 Nov 2024 23:07:10 +0800
+Subject: [PATCH 3/3] mutex: change the libmetal nuttx mutex to recursive mutex
+
+To avoid the crash when lock are acquired twice, one case is in the
+destroy process:
+we hold the rdev->lock to iterate the rdev->endpoints to destroy all
+the endpoints, but rpmsg_destroy_ept() may send the name service message,
+and need acquire the rdev->lock again to lead crash.
+
+Signed-off-by: Bowen Wang <wangbowen6@xiaomi.com>
+---
+ lib/system/nuttx/mutex.h | 18 +++++++++---------
+ 1 file changed, 9 insertions(+), 9 deletions(-)
+
+diff --git a/lib/system/nuttx/mutex.h libmetal/lib/system/nuttx/mutex.h
+index 26ec4f9..d13c9c5 100644
+--- a/lib/system/nuttx/mutex.h
++++ libmetal/lib/system/nuttx/mutex.h
+@@ -22,47 +22,47 @@
+ extern "C" {
+ #endif
+
+-typedef mutex_t metal_mutex_t;
++typedef rmutex_t metal_mutex_t;
+
+ /*
+  * METAL_MUTEX_INIT - used for initializing an mutex element in a static struct
+  * or global
+  */
+-#define METAL_MUTEX_INIT(m) NXMUTEX_INITIALIZER
++#define METAL_MUTEX_INIT(m) NXRMUTEX_INITIALIZER
+ /*
+  * METAL_MUTEX_DEFINE - used for defining and initializing a global or
+  * static singleton mutex
+  */
+-#define METAL_MUTEX_DEFINE(m) metal_mutex_t m = NXMUTEX_INITIALIZER
++#define METAL_MUTEX_DEFINE(m) metal_mutex_t m = NXRMUTEX_INITIALIZER
+
+ static inline void __metal_mutex_init(metal_mutex_t *mutex)
+ {
+-	nxmutex_init(mutex);
++	nxrmutex_init(mutex);
+ }
+
+ static inline void __metal_mutex_deinit(metal_mutex_t *mutex)
+ {
+-	nxmutex_destroy(mutex);
++	nxrmutex_destroy(mutex);
+ }
+
+ static inline int __metal_mutex_try_acquire(metal_mutex_t *mutex)
+ {
+-	return nxmutex_trylock(mutex);
++	return nxrmutex_trylock(mutex);
+ }
+
+ static inline void __metal_mutex_acquire(metal_mutex_t *mutex)
+ {
+-	nxmutex_lock(mutex);
++	nxrmutex_lock(mutex);
+ }
+
+ static inline void __metal_mutex_release(metal_mutex_t *mutex)
+ {
+-	nxmutex_unlock(mutex);
++	nxrmutex_unlock(mutex);
+ }
+
+ static inline int __metal_mutex_is_acquired(metal_mutex_t *mutex)
+ {
+-	return nxmutex_is_locked(mutex);
++	return nxrmutex_is_locked(mutex);
+ }
+
+ #ifdef __cplusplus
+-- 
+2.34.1
+

openamp/libmetal.cmake

@@ -40,6 +40,8 @@ if(NOT EXISTS ${CMAKE_CURRENT_LIST_DIR}/libmetal)
       ${CMAKE_CURRENT_LIST_DIR}/0001-libmetal-cmake-set-HAVE_STDATOMIC_H-default-true-in-.patch
       && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
       ${CMAKE_CURRENT_LIST_DIR}/0002-libmetal-atomic-enable-64-bit-atomic-by-toolchain-bu.patch
+      && patch -p0 -d ${CMAKE_CURRENT_LIST_DIR} <
+      ${CMAKE_CURRENT_LIST_DIR}/0003-mutex-change-the-libmetal-nuttx-mutex-to-recursive-m.patch
     DOWNLOAD_NO_PROGRESS true
     TIMEOUT 30)
 

openamp/libmetal.defs

@@ -84,6 +84,7 @@ libmetal.zip:
 	mv libmetal-$(LIBMETAL_VERSION) libmetal
 	$(Q) patch -p0 < 0001-libmetal-cmake-set-HAVE_STDATOMIC_H-default-true-in-.patch
 	$(Q) patch -p0 < 0002-libmetal-atomic-enable-64-bit-atomic-by-toolchain-bu.patch
+	$(Q) patch -p0 < 0003-mutex-change-the-libmetal-nuttx-mutex-to-recursive-m.patch
 
 .libmetal_headers: libmetal.zip
 else