UPSTREAM: misc: fastrpc: Drop unhandled DSP PD exit notification - #1698
UPSTREAM: misc: fastrpc: Drop unhandled DSP PD exit notification#1698Jianping (Jianping-Li) wants to merge 13 commits into
Conversation
…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/[email protected]/ Fixes: c68cfb7 ("misc: fastrpc: Add support for context Invoke method") Cc: [email protected] Signed-off-by: Jianping Li <[email protected]>
…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/[email protected]/ Signed-off-by: Jianping Li <[email protected]>
…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/[email protected]/ Fixes: 0871561 ("misc: fastrpc: Add support for audiopd") Cc: [email protected] Reviewed-by: Dmitry Baryshkov <[email protected]> Signed-off-by: Ekansh Gupta <[email protected]> Signed-off-by: Jianping Li <[email protected]>
…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/[email protected]/ Fixes: 2419e55 ("misc: fastrpc: add mmap/unmap support") Cc: [email protected] Reviewed-by: Dmitry Baryshkov <[email protected]> Signed-off-by: Ekansh Gupta <[email protected]> Signed-off-by: Jianping Li <[email protected]>
… 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/[email protected]/ Fixes: 0871561 ("misc: fastrpc: Add support for audiopd") Cc: [email protected] Signed-off-by: Jianping Li <[email protected]>
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/[email protected]/ Reviewed-by: Dmitry Baryshkov <[email protected]> Signed-off-by: Ekansh Gupta <[email protected]> Signed-off-by: Jianping Li <[email protected]>
On platforms where remote heap memory is not present, dev_err() can flood the kernel log. Use dev_dbg() instead to reduce log verbosity in this expected condition. Signed-off-by: Anandu Krishnan E <[email protected]>
The fdlist is currently part of the meta buffer which is set during fastrpc_get_args(), this fdlist is getting recalculated during fastrpc_put_args(). 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/[email protected]/ Reviewed-by: Dmitry Baryshkov <[email protected]> Signed-off-by: Ekansh Gupta <[email protected]>
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/[email protected]/ Reviewed-by: Konrad Dybcio <[email protected]> Reviewed-by: Dmitry Baryshkov <[email protected]> Signed-off-by: Ekansh Gupta <[email protected]>
…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/[email protected]/ Reviewed-by: Dmitry Baryshkov <[email protected]> Signed-off-by: Ekansh Gupta <[email protected]>
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. In case poll timeout happens, the call will fallback to normal RPC mode. 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/[email protected]/ Signed-off-by: Ekansh Gupta <[email protected]>
…omain When the remoteproc has an IOMMU (kernel running at EL2 without a separate hypervisor), memory carveouts must be explicitly mapped into the remoteproc's IOMMU domain so the DSP can access them. Without this mapping the DSP triggers an SMMU translation fault when accessing the remote heap carveout used for audio PD static process creation. Add has_iommu to fastrpc_channel_ctx, set from the "iommus" property of the remoteproc DT node. When set, map the ADSP remote heap carveout into the remoteproc's IOMMU domain using an identity mapping (IOVA == PA) via iommu_map(), and skip qcom_scm_assign_mem() which is only needed when a separate hypervisor manages inter-VM memory access control. Introduce fastrpc_remote_heap_map() and fastrpc_remote_heap_unmap() helpers to encapsulate the IOMMU domain lookup and map/unmap. Link: https://lore.kernel.org/all/[email protected]/ Signed-off-by: Anandu Krishnan E <[email protected]>
🔨 Build Failure Analysis — PR #1698PR: #1698
VerdictThis is not a compilation failure. The build failed during the pre-merge automerge phase due to a merge conflict in 📎 Detailed analysis: Full report |
🔨 Build Failure Analysis — PR #1698PR: #1698
VerdictThis is not a compilation failure. The build failed during the pre-build automerge/integration phase due to a merge conflict in 📎 Detailed analysis: Full report |
PR #1698 — validate-patchPR: #1698
Final Summary
|
PR #1698 — checker-log-analyzerPR: #1698
Detailed report: Full report
|
Newer DSP firmware implements a PD (Protection Domain) notification framework that sends PD state notifications upon request. The PD exit notification is unconditionally sent by the DSP with a fixed sentinel 0xABCDABCD in the context field. fastrpc_rpmsg_callback() treats every inbound message as an invoke response, so the sentinel is masked and shifted like any real response ((0xABCDABCD & 0xFF0) >> 4 == 188) and looked up in the channel's context idr. This is not merely cosmetic. In the common case idr slot 188 is empty, the lookup fails, and the driver only logs a spurious "No context ID matches response" error on every teardown. But the context idr is shared by every protection domain and the listener thread on the channel and is filled cyclically over [1, FASTRPC_CTX_MAX]. If slot 188 holds a live context when the sentinel arrives, the sentinel's return value is written into that unrelated in-flight invocation and it is completed early. Since neither the fastrpc library nor the driver supports the DSP PD notification framework, it is safe to drop the PD exit notification before it is ever turned into a context lookup. This removes both the log spam and the mis-completion race. A genuine response can never be masked: a real context is (idr_index << 4) | pd (at most 0xFF3) and can never equal the sentinel. Assisted-by: Claude:claude-opus-4-8 Reviewed-by: Ekansh Gupta <[email protected]> Signed-off-by: Shawn Guo <[email protected]> Signed-off-by: Srinivas Kandagatla <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Jianping Li <[email protected]>
6e7ef61 to
0590640
Compare
PR #1698 — validate-patchPR: #1698
Final Summary
|
PR #1698 — checker-log-analyzerPR: #1698
Detailed report: Full report
|
The merge-base changed after approval.
55a9e74 to
1bc7df1
Compare
3 participants
Newer DSP firmware implements a PD (Protection Domain) notification framework that sends PD state notifications upon request. The PD exit notification is unconditionally sent by the DSP with a fixed sentinel 0xABCDABCD in the context field.
fastrpc_rpmsg_callback() treats every inbound message as an invoke response, so the sentinel is masked and shifted like any real response ((0xABCDABCD & 0xFF0) >> 4 == 188) and looked up in the channel's context idr.
This is not merely cosmetic. In the common case idr slot 188 is empty, the lookup fails, and the driver only logs a spurious "No context ID matches response" error on every teardown. But the context idr is shared by every protection domain and the listener thread on the channel and is filled cyclically over [1, FASTRPC_CTX_MAX]. If slot 188 holds a live context when the sentinel arrives, the sentinel's return value is written into that unrelated in-flight invocation and it is completed early.
Since neither the fastrpc library nor the driver supports the DSP PD notification framework, it is safe to drop the PD exit notification before it is ever turned into a context lookup. This removes both the log spam and the mis-completion race. A genuine response can never be masked: a real context is (idr_index << 4) | pd (at most 0xFF3) and can never equal the sentinel.
Link: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/misc/fastrpc.c?id=5533bb4bc53c6cd401b9b329d7e26802fd5aa1f5
CRs-Fixed: 4633198