FROMLIST: misc: fastrpc: create duplicate sessions after all CB probing#1364
FROMLIST: misc: fastrpc: create duplicate sessions after all CB probing#1364quic-vkatoch wants to merge 12 commits into
Conversation
The fdlist is currently part of the meta buffer, computed during put_args. This leads to code duplication when preparing and reading critical meta buffer contents used by the FastRPC driver. Move fdlist to the invoke context structure to improve maintainability and reduce redundancy. This centralizes its handling and simplifies meta buffer preparation and reading logic. Link: https://lore.kernel.org/all/20260215182136.3995111-2-ekansh.gupta@oss.qualcomm.com/ Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
Replace the hardcoded context ID mask (0xFF0) with GENMASK(11, 4) to improve readability and follow kernel bitfield conventions. Use FIELD_PREP and FIELD_GET instead of manual shifts for setting and extracting ctxid values. Link: https://lore.kernel.org/all/20260215182136.3995111-3-ekansh.gupta@oss.qualcomm.com/ Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
…support Current FastRPC context uses a 12-bit mask: [ID(8 bits)][PD type(4 bits)] = GENMASK(11, 4) This works for normal calls but fails for DSP polling mode. Polling mode expects a 16-bit layout: [15:8] = context ID (8 bits) [7:5] = reserved [4] = async mode bit [3:0] = PD type (4 bits) If async bit (bit 4) is set, DSP disables polling. With current mask, odd IDs can set this bit, causing DSP to skip poll updates. Update FASTRPC_CTXID_MASK to GENMASK(15, 8) so IDs occupy upper byte and lower byte is left for DSP flags and PD type. Reserved bits remain unused. This change is compatible with polling mode and does not break non-polling behavior. Bit layout: [15:8] = CCCCCCCC (context ID) [7:5] = xxx (reserved) [4] = A (async mode) [3:0] = PPPP (PD type) Link: https://lore.kernel.org/all/20260215182136.3995111-4-ekansh.gupta@oss.qualcomm.com/ Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
For any remote call to DSP, after sending an invocation message, fastRPC driver waits for glink response and during this time the CPU can go into low power modes. This adds latency to overall fastrpc call as CPU wakeup and scheduling latencies are included. Add polling mode support with which fastRPC driver will poll continuously on a memory after sending a message to remote subsystem which will eliminate CPU wakeup and scheduling latencies and reduce fastRPC overhead. Poll mode can be enabled by user by using FASTRPC_IOCTL_SET_OPTION ioctl request with FASTRPC_POLL_MODE request id. Link: https://lore.kernel.org/all/20260215182136.3995111-5-ekansh.gupta@oss.qualcomm.com/ Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
…rocess abort When a userspace FastRPC client is abruptly terminated, FastRPC cleanup paths can race with device and session teardown. This results in kernel panics in different release paths: - fastrpc_release() when using remote heap, originating from fastrpc_buf_free() - fastrpc_device_release() when using system heap, originating from fastrpc_free_map() In addition, fastrpc_map_put() may trigger refcount use-after-free due to concurrent cleanup without proper synchronization. The root cause is that buffer and map cleanup paths may access map and buf resources after the associated device or session has already been released. Fix this by: - Introducing mutex protection for map and buf lifetime - Serializing buffer and map cleanup against device teardown - Skipping buffer and map operations when the device is already gone These changes ensure cleanup paths are safe against unexpected process aborts and prevent use-after-free and kernel panic scenarios. Link: https://lore.kernel.org/all/20260427105310.4056-1-jianping.li@oss.qualcomm.com/ Fixes: c68cfb7 ("misc: fastrpc: Add support for context Invoke method") Cc: stable@kernel.org Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
…messages On some platforms (e.g. QCS615 Talos), fastrpc may temporarily fail to retrieve DSP attributes during boot, resulting in repeated "Error: dsp information is incorrect" messages printed on the console. These messages are observed continuously during boot when metadata flashing is enabled as part of RC releases, causing unnecessary log noise. Similarly, the absence of reserved DMA memory is a valid configuration and does not represent an error condition. Since these scenarios are expected and do not indicate a failure, downgrade the log level from dev_err/dev_info to dev_dbg to avoid flooding the console. No functional change intended. Link: https://lore.kernel.org/all/20260514062825.50172-1-jianping.li@oss.qualcomm.com/ Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
…eue context
There is a race between fastrpc_device_release() and the workqueue
that processes DSP responses. When the user closes the file descriptor,
fastrpc_device_release() frees the fastrpc_user structure. Concurrently,
an in-flight DSP invocation can complete and fastrpc_rpmsg_callback()
schedules context cleanup via schedule_work(&ctx->put_work). If the
workqueue runs fastrpc_context_free() in parallel with or after
fastrpc_device_release() has freed the user structure, it dereferences
the freed fastrpc_user. Depending on the state of the context at the
time of the race, any one of the following accesses can be hit:
1. fastrpc_buf_free() calls fastrpc_ipa_to_dma_addr(buf->fl->cctx, ...)
to strip the SID bits from the stored IOVA before passing the
physical address to dma_free_coherent().
2. fastrpc_free_map() reads map->fl->cctx->vmperms[0].vmid to
reconstruct the source permission bitmask needed for the
qcom_scm_assign_mem() call that returns memory from the DSP VM
back to HLOS.
3. fastrpc_free_map() acquires map->fl->lock to safely remove the
map node from the fl->maps list.
The resulting use-after-free manifests as:
pc : fastrpc_buf_free+0x38/0x80 [fastrpc]
lr : fastrpc_context_free+0xa8/0x1b0 [fastrpc]
fastrpc_context_free+0xa8/0x1b0 [fastrpc]
fastrpc_context_put_wq+0x78/0xa0 [fastrpc]
process_one_work+0x180/0x450
worker_thread+0x26c/0x388
Add kref-based reference counting to fastrpc_user. Have each invoke
context take a reference on the user at allocation time and release it
when the context is freed. Release the initial reference in
fastrpc_device_release() at file close. Move the teardown of the user
structure — freeing pending contexts, maps, mmaps, and the channel
context reference — into the kref release callback fastrpc_user_free(),
so that it runs only when the last reference is dropped, regardless of
whether that happens at device close or after the final in-flight
context completes.
Link:https://lore.kernel.org/all/20260518203507.3754994-1-anandu.e@oss.qualcomm.com/
Fixes: 6cffd79 ("misc: fastrpc: Add support for dmabuf exporter")
Cc: stable@kernel.org
Signed-off-by: Anandu Krishnan E <anandu.e@oss.qualcomm.com>
…emory pool The initial buffer allocated for the Audio PD memory pool is never added to the pool because pageslen is set to 0. As a result, the buffer is not registered with Audio PD and is never used, causing a memory leak. Audio PD immediately falls back to allocating memory from the remote heap since the pool starts out empty. Fix this by setting pageslen to 1 so that the initially allocated buffer is correctly registered and becomes part of the Audio PD memory pool. Link: https://lore.kernel.org/all/20260609025938.457-2-jianping.li@oss.qualcomm.com/ Fixes: 0871561 ("misc: fastrpc: Add support for audiopd") Cc: stable@kernel.org Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
…tion fastrpc_req_munmap_impl() is called to unmap any buffer. The buffer is getting removed from the list after it is unmapped from DSP. This can create potential race conditions if multiple threads invoke unmap concurrently, where one thread may remove the entry from the list while another thread's unmap operation is still ongoing. Fix this by removing the buffer entry from the list before calling the unmap operation. If the unmap fails, the entry is re-added to the list so that userspace can retry the unmap, or alternatively, the buffer will be cleaned up during device release when the DSP process is torn down and all DSP-side mappings are freed along with remaining buffers in the list. Link: https://lore.kernel.org/all/20260609025938.457-3-jianping.li@oss.qualcomm.com/ Fixes: 2419e55 ("misc: fastrpc: add mmap/unmap support") Cc: stable@kernel.org Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
… in probe Allocating and freeing Audio PD memory from userspace is unsafe because the kernel cannot reliably determine when the DSP has finished using the memory. Userspace may free buffers while they are still in use by the DSP, and remote free requests cannot be safely trusted. Additionally, the current implementation allows userspace to repeatedly grow the Audio PD heap, but does not support shrinking it. This can lead to unbounded memory usage over time, effectively causing a memory leak. Fix this by allocating the entire Audio PD reserved-memory region during rpmsg probe and tying its lifetime to the rpmsg channel. This removes userspace-controlled alloc/free and ensures that memory is reclaimed only when the DSP process is torn down. Add explicit validation for remote_heap presence and size before sending the memory to DSP, and fail early if the reserved-memory region is missing or incomplete. Link: https://lore.kernel.org/all/20260609025938.457-4-jianping.li@oss.qualcomm.com/ Fixes: 0871561 ("misc: fastrpc: Add support for audiopd") Cc: stable@kernel.org Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
Make fastrpc_buf_free() a no-op when passed a NULL pointer, allowing callers to avoid open-coded NULL checks. Link: https://lore.kernel.org/all/20260609025938.457-5-jianping.li@oss.qualcomm.com/ Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
For ADSP, only a limited number of FastRPC context banks (CBs) are available. Each CB supports a single session, which means only a few processes can run on ADSP simultaneously. If all sessions are consumed by fastrpc daemons, no session remains available when a user application starts, causing the application to fail. To address this limitation, a Device Tree change was used till now: qcom,nsessions = <5>; However, feedback from the upstream community indicated that this change should not be made in the Device Tree. Instead, it was recommended to handle this as a driver-level change. Instead of duplicating sessions inline during fastrpc_cb_probe() using the qcom,nsessions DT property, defer duplication until after of_platform_populate() returns in fastrpc_rpmsg_probe(), at which point all compute-CB child nodes have been probed and the session array is fully populated. For the ADSP domain, append FASTRPC_DUP_SESSIONS (4) copies of the last probed session once of_platform_populate() succeeds. This keeps the per-CB probe path simple and ensures duplicates are always derived from a stable, fully-initialised session state. The qcom,nsessions DT property is no longer consumed by the driver; the binding and DT sources are left unchanged. Link: https://lore.kernel.org/all/20260609-dup-sessions-v1-1-26934abb9fa3@oss.qualcomm.com/ Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
🔨 Build Failure Analysis — PR #1364PR: #1364
VerdictThis is not a compilation failure. All 4 errors are merge conflicts that occurred during the CI integration phase when merging 📎 Detailed analysis: Full report |
🔨 Build Failure Analysis — PR #1364PR: #1364
VerdictThe build failed during the merge/integration phase, not compilation. 1 of 4 merge conflicts is directly related to this PR's changes in fastrpc.c; the other 3 conflicts are pre-existing integration issues unrelated to this PR. 📎 Detailed analysis: Full report |
PR #1364 — validate-patchPR: #1364
Final Summary
Recommendation: ❌ DO NOT MERGE until:
|
PR #1364 — checker-log-analyzerPR: #1364
Detailed report: Full report
|
2c551d5 to
ca4fac2
Compare
Patch: Replacing the qcom,nsessions DT property with a driver-level approach that appends FASTRPC_DUP_SESSIONS copies of the last probed session for the ADSP domain.
Link: https://lore.kernel.org/all/20260609-dup-sessions-v1-1-26934abb9fa3@oss.qualcomm.com/