[dm][rpmsg] support Remote Processor Messaging (RPMSG)#11303
[dm][rpmsg] support Remote Processor Messaging (RPMSG)#11303GuEe-GUI wants to merge 1 commit intoRT-Thread:masterfrom
Conversation
Signed-off-by: GuEe-GUI <2991707448@qq.com>
📌 Code Review Assignment🏷️ Tag: componentsReviewers: @Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2026-03-29 22:30 CST)
📝 Review Instructions
|
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
There was a problem hiding this comment.
Pull request overview
This PR introduces an RPMsg (Remote Processor Messaging) subsystem under RT-Thread’s driver model, aiming to provide endpoint management, name-service handling, and a character-device interface, with an intended VirtIO-based transport for QEMU.
Changes:
- Add RPMsg core bus/device/endpoint APIs (
rpmsg.c) and public header (drivers/rpmsg.h). - Add RPMsg name-service driver (
rpmsg_ns.c) and RPMsg char/control device (rpmsg_char.c). - Add build integration via
components/drivers/rpmsg/{Kconfig,SConscript}and hook intocomponents/drivers/Kconfig+rtdevice.h.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| components/drivers/rpmsg/rpmsg.c | RPMsg bus registration and endpoint lifecycle + send APIs |
| components/drivers/rpmsg/rpmsg_char.c | RPMsg char/control device implementation with endpoint create/destroy |
| components/drivers/rpmsg/rpmsg_ns.c | RPMsg name-service message handling and driver registration |
| components/drivers/rpmsg/rpmsg-rt-thread-virtio.c | Intended VirtIO transport glue for RPMsg over mailbox/shared-memory |
| components/drivers/rpmsg/SConscript | Build group and source selection for RPMsg module |
| components/drivers/rpmsg/Kconfig | Kconfig options for enabling RPMsg + char limits + virtio option |
| components/drivers/include/drivers/rpmsg.h | Public RPMsg API/types exported to the system |
| components/drivers/include/rtdevice.h | Expose RPMsg header via rtdevice.h when enabled |
| components/drivers/Kconfig | Adds rpmsg Kconfig entry to drivers menu |
| rt_device_register(&rch->parent, rt_dm_dev_get_name(&rch->parent), | ||
| RT_DEVICE_FLAG_RDWR | RT_DEVICE_FLAG_REMOVABLE); | ||
|
|
There was a problem hiding this comment.
[bug/缺陷]: rt_device_register() return value is ignored. If registration fails, the code still returns RT_EOK, leaking the endpoint/device-id and leaving an unregistered device object in the list.
English: Check the return value of rt_device_register() and on failure roll back: remove from list, free IDA id, destroy endpoint, and free rch.
中文：这里忽略了 rt_device_register() 的返回值；如果注册失败，代码仍然返回 RT_EOK，会造成 endpoint/device-id 泄漏，并且链表中残留未注册的设备对象。请检查 rt_device_register() 的返回值，失败时回滚：从链表移除、释放 IDA、销毁 endpoint，并释放 rch。
| rt_device_unregister(&rchc->parent); | ||
|
|
||
| rt_free(rchc); | ||
|
|
There was a problem hiding this comment.
[concurrency/并发问题]: del_ept_work can be (re)submitted asynchronously, but rpmsg_char_remove() frees rchc without canceling the pending work item. This can lead to the workqueue executing with freed work_data (use-after-free).
English: Call rt_work_cancel(&rchc->del_ept_work) (or a sync variant if needed) before freeing rchc, and ensure no further resubmits occur during teardown.
中文：del_ept_work 可能被异步提交/重复提交，但 rpmsg_char_remove() 在未取消该 work 的情况下就释放了 rchc，可能导致 workqueue 在 work_data 已释放时执行（UAF）。建议在释放 rchc 前调用 rt_work_cancel(&rchc->del_ept_work)（必要时用同步取消接口），并确保卸载过程中不会再次提交。
| struct rt_rpmsg_endpoint *ept; | ||
| struct rt_rpmsg_endpoint_info info; | ||
|
|
||
| RT_ASSERT(len <= RT_RPMSG_CHAR_MSG_SIZE_MAX); |
There was a problem hiding this comment.
[operational_implications/运行风险]: RX length is validated with RT_ASSERT(). Since RPMsg payload comes from a remote processor, a malformed/oversized message could trigger an assert and crash the system.
English: Prefer a runtime check (e.g., if (len > RT_RPMSG_CHAR_MSG_SIZE_MAX) return -RT_EINVAL;) and drop/log the frame instead of asserting.
中文：这里用 RT_ASSERT() 校验接收长度。RPMsg 数据来自对端处理器，若收到畸形/超长消息可能触发断言导致系统崩溃。建议改为运行时判断（例如 len > RT_RPMSG_CHAR_MSG_SIZE_MAX 则返回错误并丢包/记录日志），避免断言。
| RT_ASSERT(len <= RT_RPMSG_CHAR_MSG_SIZE_MAX); | |
| if (len > RT_RPMSG_CHAR_MSG_SIZE_MAX) | |
| { | |
| LOG_W("rx msg too long: len=%d, max=%d", (int)len, RT_RPMSG_CHAR_MSG_SIZE_MAX); | |
| return -RT_EINVAL; | |
| } |
| #include "../virtio/virtio_config/virtio-rpmsg.h" | ||
| #include "../virtio/virtio_internal.h" | ||
|
|
There was a problem hiding this comment.
[bug/缺陷]: The included headers appear to not exist in this repository (../virtio/virtio_config/virtio-rpmsg.h, ../virtio/virtio_internal.h), and the file also uses struct rt_virtio_device/rt_virtqueue_* APIs that cannot be found in-tree.
English: Either add the missing virtio headers/APIs in this PR, or refactor this driver to use the existing components/drivers/virtio/* interfaces (struct virtio_device, virtq, etc.) so the build succeeds.
中文：当前包含的头文件在仓库内似乎不存在（../virtio/virtio_config/virtio-rpmsg.h、../virtio/virtio_internal.h），且代码使用了仓库中查不到的 struct rt_virtio_device/rt_virtqueue_* 接口，会导致无法编译。请在本 PR 补齐缺失的 virtio 头文件/接口，或改为基于现有 components/drivers/virtio/* 的接口实现（struct virtio_device、virtq 等）。
| rv->is_master = rt_dm_dev_prop_read_bool(dev, "master"); | ||
| rv->regs = rv->shmem; | ||
| rv->pool = rv->shmem + ARCH_PAGE_SIZE; | ||
|
|
There was a problem hiding this comment.
[operational_implications/运行风险]: size = (size - ARCH_PAGE_SIZE) >> 1; will underflow if the shared-memory region is smaller than (or equal to) ARCH_PAGE_SIZE, producing a huge size and causing out-of-bounds pool setup.
English: Add a sanity check that size > ARCH_PAGE_SIZE (and that the remaining size is large enough for two halves) before subtracting/shifting.
中文：size = (size - ARCH_PAGE_SIZE) >> 1; 在共享内存区域大小小于等于 ARCH_PAGE_SIZE 时会发生下溢，导致 size 变成很大的值，从而出现越界的 pool 计算。建议在减法/移位前检查 size > ARCH_PAGE_SIZE，并确认剩余空间足够分成两半。
| if (size <= ARCH_PAGE_SIZE) | |
| { | |
| LOG_E("Invalid shmem size (0x%lx), must be larger than 0x%lx", | |
| (unsigned long)size, (unsigned long)ARCH_PAGE_SIZE); | |
| err = -RT_EINVAL; | |
| goto _fail; | |
| } |
| rt_uint32_t event; | ||
| struct rpmsg_virtio *rv = raw_to_rpmsg_virtio(vq->vdev); | ||
|
|
||
| event = RPMSG_VIRTIO_EVENT_QUEUE; | ||
| rt_mbox_send(rv->chan, &event, RT_WAITING_FOREVER); | ||
|
|
There was a problem hiding this comment.
[bug/缺陷]: Use-after-free risk: rt_mbox_send() stores the data pointer in chan->data until rt_mbox_send_done() runs, so passing the address of a stack variable (&event) is unsafe and can be dereferenced after the function returns.
English: Use a persistent buffer for the mailbox payload (e.g., store event in struct rpmsg_virtio or allocate it) and pass that pointer to rt_mbox_send().
中文：rt_mbox_send() 会把 data 指针保存到 chan->data 直到发送完成回调执行，因此这里把栈变量地址（&event）传进去会在函数返回后变成悬空指针。请使用持久化的缓冲区（例如把 event 放到 struct rpmsg_virtio 里或动态分配）再传给 rt_mbox_send()。
| /* Ask peer to link */ | ||
| event = RPMSG_VIRTIO_EVENT_LINK; | ||
| rt_mbox_send(rv->chan, &event, RT_WAITING_FOREVER); | ||
|
|
There was a problem hiding this comment.
[bug/缺陷]: Same stack-lifetime issue as above: &event is a stack pointer passed to rt_mbox_send() and may be used after return.
English: Make the mailbox payload storage persistent (e.g., rv->event_link) before calling rt_mbox_send().
中文：同样的栈生命周期问题：把 &event（栈地址）传给 rt_mbox_send() 可能在函数返回后被继续使用。请改为使用持久化存储（例如 rv->event_link）后再调用 rt_mbox_send()。
| rt_ofw_get_address(shmem_np, 0, &addr, &size); | ||
| rt_ofw_node_put(shmem_np); |
There was a problem hiding this comment.
[bug/缺陷]: rt_ofw_parse_phandle() can return NULL; rt_ofw_get_address() also returns an error code. Here neither is checked before use, which can lead to a NULL dereference or using uninitialized addr/size.
English: Validate shmem_np != RT_NULL and check the return value of rt_ofw_get_address() (and handle failures with cleanup) before continuing.
中文：rt_ofw_parse_phandle() 可能返回 NULL，rt_ofw_get_address() 也会返回错误码；这里未做检查就直接使用，会导致空指针解引用或使用未初始化的 addr/size。请先判断 shmem_np != RT_NULL 并检查 rt_ofw_get_address() 的返回值，失败时走清理流程。
| rt_ofw_get_address(shmem_np, 0, &addr, &size); | |
| rt_ofw_node_put(shmem_np); | |
| if (shmem_np == RT_NULL) | |
| { | |
| err = -RT_EINVAL; | |
| LOG_E("Get shmem node fail"); | |
| goto _fail; | |
| } | |
| err = rt_ofw_get_address(shmem_np, 0, &addr, &size); | |
| rt_ofw_node_put(shmem_np); | |
| if (err) | |
| { | |
| LOG_E("Get shmem address fail"); | |
| goto _fail; | |
| } |
| if (err && err != -RT_EEMPTY) | ||
| { | ||
| LOG_E("Attach power domain error = %s in device %s", | ||
| rt_dm_dev_get_name(dev), rt_strerror(err)); |
There was a problem hiding this comment.
[bug/缺陷]: Log arguments are swapped: format is "Attach power domain error = %s in device %s" but it passes (device_name, strerror).
English: Swap the arguments so the first %s is rt_strerror(err) and the second is the device name.
中文：日志参数顺序写反了：格式为 "Attach power domain error = %s in device %s"，但实际传参是 (device_name, strerror)。请交换参数，使第一个 %s 对应 rt_strerror(err)，第二个 %s 对应设备名。
| rt_dm_dev_get_name(dev), rt_strerror(err)); | |
| rt_strerror(err), rt_dm_dev_get_name(dev)); |
| } | ||
|
|
||
| if ((device_id = rt_dm_ida_alloc(&rpmsg_char_ida)) < 0) | ||
| { |
There was a problem hiding this comment.
[bug/缺陷]: Memory leak on rt_dm_ida_alloc() failure: rchc is allocated but not freed when rt_dm_ida_alloc(&rpmsg_char_ida) fails.
English: Free rchc (and ensure rdev->parent.user_data isn’t left pointing to freed/partial state) before returning.
中文：rt_dm_ida_alloc() 失败时会内存泄漏：rchc 已经分配，但在 rt_dm_ida_alloc(&rpmsg_char_ida) 失败后直接返回，没有释放。请在返回前释放 rchc（并确保 rdev->parent.user_data 不会指向半初始化状态）。
| { | |
| { | |
| rt_free(rchc); |
拉取/合并请求描述：(PR description)
[
support Remote Processor Messaging, The virtio driver will support in QEMU
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up