Pr 82 - #2
Pr 82#2jackYoung0915 wants to merge 25 commits into
Conversation
📝 WalkthroughWalkthrough本次改动为 UMMU/UB 子系统引入 SVA 分离页表能力(新增 tdev 复用、matt map/unmap、hw_cap 查询接口),新增 UBMEM-VMMU 平台 IOMMU 驱动与 ubmempfd 字符设备驱动,重构 CDMA 驱动的 SVA/IOPF 集成与内存 pin/unpin 流程,并对 hisilicon UMMU 核心驱动(权限队列/权限表/配置表/logic_ummu)、UBRT 平台设备接口、UMMU PMU 寄存器访问方式进行了内部重构与清理。 ChangesUMMU Core SVA 分离页表功能
UBMEM-VMMU 与 ubmempfd 新驱动
UMMU hisilicon 核心驱动内部重构
UBRT 平台设备接口移除
CDMA 驱动 SVA/IOPF 集成
UMMU PMU 寄存器访问方式调整
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant App as 用户态应用
participant Ubmempfd as ubmempfd 驱动
participant UmmuCore as ummu-core
participant IOMMU as IOMMU 子系统
App->>Ubmempfd: write(ubm_request MAP)
Ubmempfd->>UmmuCore: ummu_core_alloc_tdev(tid)
UmmuCore-->>Ubmempfd: device
Ubmempfd->>IOMMU: iommu_get_domain_for_dev()
Ubmempfd->>IOMMU: get_user_pages_fast + iommu_map
IOMMU-->>Ubmempfd: 映射结果
Ubmempfd-->>App: 返回结果
sequenceDiagram
participant CDMA as cdma_context
participant Core as ummu-core
participant IOMMU as IOMMU 子系统
CDMA->>Core: ummu_get_sva_mode(dev)
Core-->>CDMA: SHARE/SEPARATE
alt SHARE 模式
CDMA->>IOMMU: ummu_sva_bind_device()
else SEPARATE 模式
CDMA->>Core: ummu_alloc_tdev_separated()
Core-->>CDMA: tdev
end
CDMA->>IOMMU: iommu_sva_grant()
IOMMU-->>CDMA: 授权结果
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
drivers/iommu/hisilicon/perm_queue.c (1)
130-146: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift权限队列释放路径无条件释放 DMA 内存,即使硬件释放确认(poll)超时。
ummu_device_release_ucmdq现在不再返回状态,仅在readl_relaxed_poll_timeout失败时打印错误,调用方ummu_release_permq_resource完全不检查该结果,直接继续xa_erase并free_pages/kfree释放pcmdq/pcplq的 DMA 内存。根据变更说明,旧实现中存在基于轮询/释放返回码的失败分支(不会继续释放)。如果硬件释放确认超时意味着设备可能仍在访问这段队列内存,那么无条件释放会造成设备后续 DMA 写入已释放页面,导致内存损坏或安全问题。建议:至少在超时时避免立即释放内存(例如延迟释放、上报给上层做恢复处理,或保留原有的失败分支不再释放),而不是仅打印日志后继续正常释放流程。
🔒 参考修复方向
-static void ummu_device_release_ucmdq(struct ummu_device *ummu, u32 qid) +static int ummu_device_release_ucmdq(struct ummu_device *ummu, u32 qid) { u32 reg_rep; int ret; guard(mutex)(&ummu->permq_ctx_cfg.permq_rel_mutex); writel_relaxed((qid & PERMQ_RELEASE_ID), ummu->base + UMMU_RELEASE_PERMQ_ID); writel_relaxed(PERMQ_RELEASE_CPL_BIT, ummu->base + UMMU_RELEASE_PERMQ); ret = readl_relaxed_poll_timeout(ummu->base + UMMU_RELEASE_PERMQ, reg_rep, !(reg_rep & PERMQ_RELEASE_CPL_BIT), 1, PERMQ_RELEASE_TIMEOUT_US); - if (ret) + if (ret) dev_err(ummu->dev, "ummu release ucmdq failed, qid = %u\n", qid); + return ret; } void ummu_release_permq_resource(struct ummu_domain *domain) { ... domain->qid = UMMU_INVALID_QID; - ummu_device_release_ucmdq(ummu, qid); + if (ummu_device_release_ucmdq(ummu, qid)) { + dev_err(ummu->dev, "skip freeing permq mem due to release failure, qid=%u\n", qid); + return; + } ... }Also applies to: 148-178
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drivers/iommu/hisilicon/perm_queue.c` around lines 130 - 146, The release flow in ummu_device_release_ucmdq and its caller ummu_release_permq_resource currently logs poll timeout errors but still frees the pcmdq/pcplq DMA-backed memory unconditionally, which can race with hardware still using the queue. Update the release path so the timeout result is propagated back to ummu_release_permq_resource and used to stop or defer xa_erase plus free_pages/kfree when the hardware completion bit is not observed; keep the existing release logic only on successful confirmation and route failures to an error-handling path instead of continuing normal cleanup.drivers/iommu/hisilicon/perm_table.c (1)
247-259: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftdrivers/iommu/hisilicon/perm_table.c:247-259 仍需
init_mutex保护。
ummu_get_resource()、ummu_init_sva_mapt_context()、ummu_init_ksva_mapt()都会直接进入这条分配路径,当前调用链里看不到统一串行化;s1_cfg.tct的并发读写仍可能导致 TCT 描述符和内存状态不一致。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drivers/iommu/hisilicon/perm_table.c` around lines 247 - 259, `ummu_alloc_mapt_blk_mem()` still needs `init_mutex` protection because it is reached directly from `ummu_get_resource()`, `ummu_init_sva_mapt_context()`, and `ummu_init_ksva_mapt()` without a visible shared serialization point. Add the mutex locking/unlocking around the mode check and the calls to `ummu_alloc_mapt_mem_for_table()` / `ummu_alloc_mapt_mem_for_entry()` so concurrent access cannot race on `s1_cfg.tct` and the TCT/resource state remains consistent.drivers/iommu/hisilicon/ubmem-mmu/Makefile (1)
3-6: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win将
obj-m改为跟随CONFIG_UB_UBMEM_VMMU,否则内置构建会失效。父目录已经用
obj-$(CONFIG_UB_UBMEM_VMMU) += ubmem-mmu/进入这里;但本文件把目标写死成obj-m,会导致CONFIG_UB_UBMEM_VMMU=y时不会链接进vmlinux,使这个 tristate 选项的内置路径失效。ubmem_vmmu-$(CONFIG_UB_UBMEM_VMMU) := ubmem_vmmu_main.o -obj-m += ubmem_vmmu.o +obj-$(CONFIG_UB_UBMEM_VMMU) += ubmem_vmmu.o🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drivers/iommu/hisilicon/ubmem-mmu/Makefile` around lines 3 - 6, The Makefile for ubmem_vmmu currently hardcodes obj-m, which breaks the built-in path when CONFIG_UB_UBMEM_VMMU is enabled. Update the ubmem_vmmu build rule to follow CONFIG_UB_UBMEM_VMMU, matching the existing ubmem_vmmu-$(CONFIG_UB_UBMEM_VMMU) object selection and ensuring the target is linked into vmlinux when the option is built-in.
🧹 Nitpick comments (6)
drivers/iommu/hisilicon/ummu-core/core_tdev.c (1)
127-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
kstrdup失败应返回-ENOMEM。此处内存分配失败返回
-EINVAL语义不准确,建议改为-ENOMEM。♻️ 建议修改
tdev->pdev.name = kstrdup(attr->name ? : "ummu_vdev", GFP_KERNEL); if (!tdev->pdev.name) - return -EINVAL; + return -ENOMEM;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drivers/iommu/hisilicon/ummu-core/core_tdev.c` around lines 127 - 129, The failure path in the `kstrdup` allocation for `tdev->pdev.name` is using the wrong error code; change the return from `-EINVAL` to `-ENOMEM` in the `core_tdev` initialization flow so allocation failures are reported with the correct semantics.drivers/iommu/hisilicon/ummu-core/core_matt.c (1)
123-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
ssize_t与size_t *参数不匹配,可能触发-Wpointer-sign告警。
l_mapped/r_mapped声明为ssize_t,而sva_matt_inner_map()的mapped形参为size_t *,取地址传入属有符号/无符号指针不兼容。建议将局部变量改为size_t以与形参一致(iommu_map_sgtable的负值已在内部判定后才写入*mapped)。♻️ 建议修改
- ssize_t l_mapped, r_mapped; + size_t l_mapped, r_mapped; int ret;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drivers/iommu/hisilicon/ummu-core/core_matt.c` around lines 123 - 124, `l_mapped` and `r_mapped` in `core_matt.c` are declared as `ssize_t`, but they are passed by address to `sva_matt_inner_map()` whose `mapped` parameter expects a `size_t *`, creating a signed/unsigned pointer mismatch. Update the local declarations in the mapping flow to use `size_t` so they match `sva_matt_inner_map()` and the related `iommu_map_sgtable` writeback, while keeping the existing `ret` handling unchanged.drivers/iommu/hisilicon/logic_ummu/logic_ummu.c (1)
1766-1774: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win在持有 spinlock 时调用外部注册回调存在风险。
logic_ummu_device_support_attr()在guard(spinlock)(&support_cb_list_lock)保护下遍历并调用外部通过logic_ummu_register_support_attr()注册的回调scb_entry->func(...)。若任一回调实现中存在睡眠操作(如分配、加互斥锁)或反过来调用注册/注销接口,将导致死锁或 “sleeping in atomic” 告警。此外
select_logic_ummu未初始化,一旦某回调返回true却未写入该出参,Line 1769 将返回未初始化值。建议:将回调收集到局部快照后在锁外调用,并为select_logic_ummu提供默认值。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drivers/iommu/hisilicon/logic_ummu/logic_ummu.c` around lines 1766 - 1774, logic_ummu_device_support_attr() currently calls support callbacks while holding support_cb_list_lock via guard(spinlock), which can deadlock or trigger sleeping-in-atomic issues if a registered callback blocks or re-enters registration APIs. Refactor the function to snapshot the registered callbacks under the lock, release the lock before invoking scb_entry->func(...) from logic_ummu_register_support_attr() users, and keep the final fallback check on hisi_ummu_tdev_info outside the locked section. Also initialize select_logic_ummu to a safe default before passing it to callbacks so a true return cannot expose an uninitialized value.drivers/ub/cdma/cdma_context.c (1)
101-105: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win无效
sva_mode分支返回了正数,不符合内核 errno 约定。
return sva_mode;会把设备配置的模式值(可能为正数)当作返回码向上传递。虽然cdma_ctx_alloc_tid用if (ret)判断能识别为失败,但正数错误码在更上层可能被误判为成功或直接透传到用户态。建议返回明确的负 errno(如-EINVAL)。♻️ 建议修改
} else { dev_err(cdev->dev, "bind invalid sva mode, mode = %d.\n", sva_mode); - return sva_mode; + return -EINVAL; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drivers/ub/cdma/cdma_context.c` around lines 101 - 105, The invalid sva_mode branch in cdma_ctx_alloc_tid currently returns sva_mode directly, which can be a positive value and violates kernel errno conventions. Update this error path to return a स्पष्ट negative errno such as -EINVAL instead of propagating the mode value, and keep the dev_err message in cdma_context.c aligned with the invalid bind case.drivers/iommu/hisilicon/ubmem-mmu/Kconfig (1)
3-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value建议使用
config而非menuconfig。该选项下未包含任何子配置项,使用
menuconfig通常是为了承载子菜单,此处更符合惯例的写法是config UB_UBMEM_VMMU。不影响功能,纯风格建议。♻️ 建议的修改
-menuconfig UB_UBMEM_VMMU +config UB_UBMEM_VMMU default n tristate "ubmem_vmmu driver" depends on UB_UMMU_CORE && UB_UMMU🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drivers/iommu/hisilicon/ubmem-mmu/Kconfig` around lines 3 - 11, The UB_UBMEM_VMMU Kconfig entry is declared as menuconfig even though it does not define any child options; change it to a plain config entry while keeping the existing symbol name, default, tristate type, dependencies, and help text unchanged.drivers/ub/ubmempfd/ubmempfd_main.c (1)
55-71: 🎯 Functional Correctness | 🔵 Trivial | ⚖️ Poor tradeoff长期 DMA/IOMMU 映射的用户页建议使用
pin_user_pages_fast()而非get_user_pages_fast()+put_page()。这些页会被
iommu_map()映射进设备可访问的 IOMMU domain,且映射生命周期较长(直到显式 unmap),符合内核文档中建议使用pin_user_pages*()/unpin_user_page()的 DMA 长期引用场景,而非普通FOLL_GET/put_page()语义。FOLL_PIN pages must be released, ultimately, by a call to put_user_page(), while FOLL_GET: get_user_pages*() to acquire, and put_page() to release,两套引用计数机制并不等价,混用可能影响页迁移/回收等场景下的正确性判断。另外该函数的
flags形参未被使用(内部硬编码FOLL_GET),如需支持可变标志应改用flags变量。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drivers/ub/ubmempfd/ubmempfd_main.c` around lines 55 - 71, The page-acquisition path in ubmempfd_get_pages() is using get_user_pages_fast() with FOLL_GET and put_page() for pages that are mapped for long-lived DMA/IOMMU use, which should instead use pin_user_pages_fast() with the matching unpin release path. Update ubmempfd_get_pages() to use the pin/unpin API consistently for the iommu_map() lifetime, and avoid mixing FOLL_GET semantics with pinned DMA pages. Also remove the unused flags parameter or wire it through to the page-pinning call so the function does not hardcode FOLL_GET behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@drivers/iommu/hisilicon/flush.c`:
- Around line 425-436: ummu_device_flush_ioplb_all currently ignores the return
value from ummu_mcmdq_issue_cmd_with_sync, unlike ummu_device_flush_plb and
ummu_device_flush_plb_all. Update this function to capture the result of the
command submission, log a dev_err with the ummu device context when it fails,
and keep the error handling consistent with the other flush helpers in the same
file.
In `@drivers/iommu/hisilicon/iommu.c`:
- Around line 776-779: The master pointer in the IOMMU path can be NULL, so
guard the result of dev_iommu_priv_get(dev) before calling
ummu_master_iopf_enabled(). Update the logic in the dev/master handling block to
check master explicitly and keep iopf_enabled at its default true value when
master is NULL, instead of dereferencing it.
In `@drivers/iommu/hisilicon/perm_table.c`:
- Around line 157-164: In the MAPT block allocation path in perm_table.c, the
page allocator currently uses GFP_HIGHUSER_MOVABLE, which makes the pages
migratable even though their physical addresses are stored in
tct_desc->mapt_blk_phys and tct_desc->mapt_blk_tbl_phys for hardware use. Update
the allocation in the MAPT block setup code to use a non-migratable GFP flag
combination instead, keeping the rest of the page allocation and error handling
unchanged.
- Around line 1318-1320: 在 perm_table.c 的 ungrant 失败路径里,ummu_ungrant_imp() 返回非 0
后仍会留下可触发 iommu_plb_sync() 的 plb_gather 状态,导致失败时也执行 PLB 刷新。请在 iommu_sva_ungrant()
相关逻辑中处理 ret != 0 分支,参考 grant 路径的做法,将 plb_gather->size 置 0(必要时同步清理相关 va/size
状态),确保失败时不会继续触发实际 flush。
In `@drivers/iommu/hisilicon/queue.c`:
- Around line 978-983: The queue-wait loops in ummu_mcmdq_insert and the similar
path in drivers/iommu/hisilicon/queue.c currently retry forever after
ummu_mcmdq_poll_until_not_full() times out, which can block the caller
indefinitely when hardware never drains the queue. Add a bounded retry/timeout
limit or propagate an error back to the caller after the poll timeout, and make
sure both existing wait loops use the same capped behavior so the upper layer
can handle backoff or failure instead of spinning forever.
In `@drivers/iommu/hisilicon/ubmem-mmu/ubmem_vmmu_main.c`:
- Around line 406-452: The return value in ubmem_vmmu_iotlb_sync_map is left
uninitialized when no matching context is found, so initialize ret to a safe
default before the ctx_match lookup and only update it on actual request
handling. Make sure the final return from ubmem_vmmu_iotlb_sync_map always
reflects a defined success/failure code even when list_for_each_entry_safe finds
no matching map_ctx, and keep the existing error handling around
ubmem_vmmu_handle_req_vm unchanged.
- Around line 646-707: The global device pointer is assigned too late in
ubmem_vmmu_device_probe, which can let ubmem_vmmu_probe_device run before
global_ubmem_vmmu_dev is set and dereference NULL. Move the
global_ubmem_vmmu_dev assignment to before ummu_core_device_register() (after
ubmem_vmmu is initialized), and make sure every failure path in
ubmem_vmmu_device_probe clears that global back to NULL before returning.
- Around line 636-641: The slot_bitmap allocation in ubmem_vmmu_main.c is using
a word count as if it were a byte size, which can lead to bitmap overrun during
find_first_zero_bit/set_bit/clear_bit operations. Update the allocation in the
ubmem_vmmu setup path to use a bitmap-aware allocator such as bitmap_zalloc for
ubmem_vmmu->slot_num, or otherwise convert the size to bytes correctly before
kzalloc, and keep the existing error handling in the same allocation block.
In `@drivers/iommu/hisilicon/ummu-core/core_ioctl.c`:
- Line 914: Restrict the misc device permissions in the UMMU ioctl registration
so it is not world-writable: the current .mode = 0666 in the device setup
exposes open/ioctl/mmap access to all local users. Update the mode in the misc
device definition around the core ioctl setup to a tighter default such as 0600,
or leave it stricter and rely on udev/group-based access control if broader
access is intended.
- Around line 775-784: The domain retrieved in core_ioctl handling can be NULL,
so add a guard before calling iommu_plb_sync() or iommu_plb_sync_all() in the
UMMU_IOCPLBI_VA and UMMU_IOCPLBI_ALL paths. Use the existing domain assignment
logic around entry->sva and iommu_get_domain_for_dev(), and if domain is missing
return -ENODEV immediately to prevent passing a null domain into the PLB sync
helpers.
In `@drivers/iommu/hisilicon/ummu-core/core_tdev.c`:
- Around line 286-295: The reference handling in the mm_tid_xa lookup path is
unsafe because mm_tid_xa_lock only orders lookup and erase, so tdev->ref can
already be zero before kref_get() runs. Update the lookup logic in core_tdev.c
around the xa_load/tdev path to use kref_get_unless_zero() on tdev->ref, and
treat a failure as a cache miss by unlocking mm_tid_xa_lock and falling through
to the create/new-device path instead of returning the existing tdev.
In `@drivers/iommu/hisilicon/ummu-core/core_tid.c`:
- Around line 320-328: The return path in the token lookup logic reads
tid_data->mm after xa_unlock(), which can race with ummu_global_pasid_free() and
use freed memory. In the tid lookup function that uses xa_lock()/xa_load() on
token_ids, copy tid_data->mm into a local variable while still holding the lock,
then unlock and return the cached mm value instead of dereferencing tid_data
afterward.
In `@drivers/ub/cdma/cdma_common.c`:
- Around line 148-194: The SVA separate-mode unmap path in
cdma_sva_matt_unmap/cdma_put_umem is using current->mm instead of the mm that
was present when the mapping was created, so the teardown can run in the wrong
process context and fail silently. Persist the mm in struct cdma_umem during
cdma_sva_matt_map (taking a reference with mmgrab) and reuse that stored mm in
cdma_sva_matt_unmap/cleanup, then release it with mmdrop when freeing the umem.
Also make sure cdma_put_umem handles the unmap result instead of ignoring
failures so leaked mappings are not hidden.
In `@drivers/ub/cdma/cdma_segment.c`:
- Around line 98-99: The cdma segment grant/ungrant path is passing cfg->sva
directly into iommu_sva_grant() and iommu_sva_ungrant() even when
UMMU_SVA_SEPARATE_MODE clears ctx->sva, which can trigger a null dereference.
Update cdma_seg_grant() and cdma_seg_ungrant() in cdma_segment.c to detect the
separate-mode case and skip the SVA-based path, or switch to the appropriate
tid/vdev handle instead of using a null sva. Ensure the existing
seg->ksva/cfg->sva flow is guarded before calling the IOMMU helpers.
In `@drivers/ub/ubmempfd/Makefile`:
- Around line 3-5: The Makefile for ubmempfd is registering the module
unconditionally with obj-m, which bypasses CONFIG_UB_UBMEMPFD and breaks
optional build behavior. Update the kbuild entries around
ubmempfd-$(CONFIG_UB_UBMEMPFD) and obj-m so the module is only added when
CONFIG_UB_UBMEMPFD is enabled, using the standard conditional module declaration
pattern for ubmempfd.o and ubmempfd_main.o.
In `@include/linux/hisi_ummu.h`:
- Around line 1-9: The header currently uses u64 and u32 without including their
defining types header, creating a fragile implicit dependency. Add the missing
linux types include near the top of this header so UMMU’s type definitions are
self-contained, and keep the include guard and existing declarations in
hisi_ummu.h unchanged.
- Around line 10-23: `hisi_ummu_tdev_info` is being written as `v2` in
`ubmempfd_alloc_tdev()` but later read as `v1` in
`ubmem_mmu_tdev_support_attr()`, so the layout must be made consistent.
Initialize the whole `hisi_ummu_tdev_info` to zero before use, then either
populate and consume the same union member everywhere or branch on `version` in
`ubmem_mmu_tdev_support_attr()` and `ubmempfd_alloc_tdev()` so the code never
interprets `priv` with mismatched `v1`/`v2` fields. Also ensure `ummu_idx_mask`
is explicitly set before any `__ffs()` or device matching logic uses it.
In `@include/linux/ummu_core.h`:
- Line 697: The deprecation note for ummu_is_sva() is inaccurate because it does
not map directly to iommu_is_ksva_domain(); update the comment to describe the
actual SVA check used in the implementation, namely the domain->mm plus
!iommu_is_ksva_domain(domain) condition, or add a dedicated helper if you want a
true SVA predicate. Make sure the replacement wording in ummu_core.h matches the
real behavior and does not reference a nonexistent iommu_is_sva_domain().
- Line 621: The doc comment on the IOMMU API is using a kernel-doc tag-style
"`@Deprecated`:" entry, which gets parsed as an excess description warning. Update
the comment in the relevant declaration block to use a plain note like
"Deprecated:" or "Note:" instead of the `@-prefixed` form, keeping the reference
to iommu_sva_grant clear for readers.
---
Outside diff comments:
In `@drivers/iommu/hisilicon/perm_queue.c`:
- Around line 130-146: The release flow in ummu_device_release_ucmdq and its
caller ummu_release_permq_resource currently logs poll timeout errors but still
frees the pcmdq/pcplq DMA-backed memory unconditionally, which can race with
hardware still using the queue. Update the release path so the timeout result is
propagated back to ummu_release_permq_resource and used to stop or defer
xa_erase plus free_pages/kfree when the hardware completion bit is not observed;
keep the existing release logic only on successful confirmation and route
failures to an error-handling path instead of continuing normal cleanup.
In `@drivers/iommu/hisilicon/perm_table.c`:
- Around line 247-259: `ummu_alloc_mapt_blk_mem()` still needs `init_mutex`
protection because it is reached directly from `ummu_get_resource()`,
`ummu_init_sva_mapt_context()`, and `ummu_init_ksva_mapt()` without a visible
shared serialization point. Add the mutex locking/unlocking around the mode
check and the calls to `ummu_alloc_mapt_mem_for_table()` /
`ummu_alloc_mapt_mem_for_entry()` so concurrent access cannot race on
`s1_cfg.tct` and the TCT/resource state remains consistent.
In `@drivers/iommu/hisilicon/ubmem-mmu/Makefile`:
- Around line 3-6: The Makefile for ubmem_vmmu currently hardcodes obj-m, which
breaks the built-in path when CONFIG_UB_UBMEM_VMMU is enabled. Update the
ubmem_vmmu build rule to follow CONFIG_UB_UBMEM_VMMU, matching the existing
ubmem_vmmu-$(CONFIG_UB_UBMEM_VMMU) object selection and ensuring the target is
linked into vmlinux when the option is built-in.
---
Nitpick comments:
In `@drivers/iommu/hisilicon/logic_ummu/logic_ummu.c`:
- Around line 1766-1774: logic_ummu_device_support_attr() currently calls
support callbacks while holding support_cb_list_lock via guard(spinlock), which
can deadlock or trigger sleeping-in-atomic issues if a registered callback
blocks or re-enters registration APIs. Refactor the function to snapshot the
registered callbacks under the lock, release the lock before invoking
scb_entry->func(...) from logic_ummu_register_support_attr() users, and keep the
final fallback check on hisi_ummu_tdev_info outside the locked section. Also
initialize select_logic_ummu to a safe default before passing it to callbacks so
a true return cannot expose an uninitialized value.
In `@drivers/iommu/hisilicon/ubmem-mmu/Kconfig`:
- Around line 3-11: The UB_UBMEM_VMMU Kconfig entry is declared as menuconfig
even though it does not define any child options; change it to a plain config
entry while keeping the existing symbol name, default, tristate type,
dependencies, and help text unchanged.
In `@drivers/iommu/hisilicon/ummu-core/core_matt.c`:
- Around line 123-124: `l_mapped` and `r_mapped` in `core_matt.c` are declared
as `ssize_t`, but they are passed by address to `sva_matt_inner_map()` whose
`mapped` parameter expects a `size_t *`, creating a signed/unsigned pointer
mismatch. Update the local declarations in the mapping flow to use `size_t` so
they match `sva_matt_inner_map()` and the related `iommu_map_sgtable` writeback,
while keeping the existing `ret` handling unchanged.
In `@drivers/iommu/hisilicon/ummu-core/core_tdev.c`:
- Around line 127-129: The failure path in the `kstrdup` allocation for
`tdev->pdev.name` is using the wrong error code; change the return from
`-EINVAL` to `-ENOMEM` in the `core_tdev` initialization flow so allocation
failures are reported with the correct semantics.
In `@drivers/ub/cdma/cdma_context.c`:
- Around line 101-105: The invalid sva_mode branch in cdma_ctx_alloc_tid
currently returns sva_mode directly, which can be a positive value and violates
kernel errno conventions. Update this error path to return a स्पष्ट negative
errno such as -EINVAL instead of propagating the mode value, and keep the
dev_err message in cdma_context.c aligned with the invalid bind case.
In `@drivers/ub/ubmempfd/ubmempfd_main.c`:
- Around line 55-71: The page-acquisition path in ubmempfd_get_pages() is using
get_user_pages_fast() with FOLL_GET and put_page() for pages that are mapped for
long-lived DMA/IOMMU use, which should instead use pin_user_pages_fast() with
the matching unpin release path. Update ubmempfd_get_pages() to use the
pin/unpin API consistently for the iommu_map() lifetime, and avoid mixing
FOLL_GET semantics with pinned DMA pages. Also remove the unused flags parameter
or wire it through to the page-pinning call so the function does not hardcode
FOLL_GET behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 52f50931-9fd1-4cba-bdc3-da11263f8c7b
📒 Files selected for processing (86)
Documentation/ABI/testing/sysfs-class-iommu-ummu-iommuDocumentation/devicetree/bindings/iommu/hisi,ubmem_vmmu.yamlanolis/configs/L1-RECOMMEND/arm64/CONFIG_UB_UBMEMPFDanolis/configs/L1-RECOMMEND/arm64/CONFIG_UB_UBMEM_VMMUanolis/configs/L1-RECOMMEND/arm64/CONFIG_UB_UBRT_PLAT_DEVanolis/configs/L1-RECOMMEND/arm64/CONFIG_UB_UMMU_SVA_SEPARATED_PAGESdrivers/iommu/hisilicon/Kconfigdrivers/iommu/hisilicon/Makefiledrivers/iommu/hisilicon/attribute.cdrivers/iommu/hisilicon/cfg_table.cdrivers/iommu/hisilicon/cfg_table.hdrivers/iommu/hisilicon/flush.cdrivers/iommu/hisilicon/flush.hdrivers/iommu/hisilicon/interrupt.cdrivers/iommu/hisilicon/iommu.cdrivers/iommu/hisilicon/logic_ummu/logic_ummu.cdrivers/iommu/hisilicon/logic_ummu/logic_ummu.hdrivers/iommu/hisilicon/nested.cdrivers/iommu/hisilicon/page_table.cdrivers/iommu/hisilicon/perm_queue.cdrivers/iommu/hisilicon/perm_queue.hdrivers/iommu/hisilicon/perm_table.cdrivers/iommu/hisilicon/perm_table.hdrivers/iommu/hisilicon/queue.cdrivers/iommu/hisilicon/regs.hdrivers/iommu/hisilicon/seg_mng.cdrivers/iommu/hisilicon/seg_tree.cdrivers/iommu/hisilicon/sva.cdrivers/iommu/hisilicon/sva.hdrivers/iommu/hisilicon/ubmem-mmu/Kconfigdrivers/iommu/hisilicon/ubmem-mmu/Makefiledrivers/iommu/hisilicon/ubmem-mmu/ubmem_mmu.cdrivers/iommu/hisilicon/ubmem-mmu/ubmem_vmmu_main.cdrivers/iommu/hisilicon/ummu-core/Kconfigdrivers/iommu/hisilicon/ummu-core/Makefiledrivers/iommu/hisilicon/ummu-core/core.cdrivers/iommu/hisilicon/ummu-core/core_eid.cdrivers/iommu/hisilicon/ummu-core/core_ioctl.cdrivers/iommu/hisilicon/ummu-core/core_iova.cdrivers/iommu/hisilicon/ummu-core/core_matt.cdrivers/iommu/hisilicon/ummu-core/core_tdev.cdrivers/iommu/hisilicon/ummu-core/core_tid.cdrivers/iommu/hisilicon/ummu-core/ummu_core_priv.hdrivers/iommu/hisilicon/ummu.hdrivers/iommu/hisilicon/ummu_cfg_v1.hdrivers/iommu/hisilicon/ummu_main.cdrivers/iommu/iommu-sva.cdrivers/perf/hisilicon/ummu_pmu.cdrivers/ub/Kconfigdrivers/ub/Makefiledrivers/ub/cdma/cdma.hdrivers/ub/cdma/cdma_api.cdrivers/ub/cdma/cdma_chardev.cdrivers/ub/cdma/cdma_cmd.cdrivers/ub/cdma/cdma_common.cdrivers/ub/cdma/cdma_common.hdrivers/ub/cdma/cdma_context.cdrivers/ub/cdma/cdma_context.hdrivers/ub/cdma/cdma_db.cdrivers/ub/cdma/cdma_debugfs.cdrivers/ub/cdma/cdma_dev.cdrivers/ub/cdma/cdma_eq.cdrivers/ub/cdma/cdma_ioctl.cdrivers/ub/cdma/cdma_jfc.cdrivers/ub/cdma/cdma_jfc.hdrivers/ub/cdma/cdma_jfs.cdrivers/ub/cdma/cdma_mbox.cdrivers/ub/cdma/cdma_mbox.hdrivers/ub/cdma/cdma_queue.cdrivers/ub/cdma/cdma_segment.cdrivers/ub/cdma/cdma_segment.hdrivers/ub/cdma/cdma_tid.cdrivers/ub/ubfi/Kconfigdrivers/ub/ubfi/irq.cdrivers/ub/ubfi/ubrt-fwnode.cdrivers/ub/ubfi/ummu.cdrivers/ub/ubmempfd/Kconfigdrivers/ub/ubmempfd/Makefiledrivers/ub/ubmempfd/ubmempfd_main.cinclude/linux/hisi_ummu.hinclude/linux/iommu.hinclude/linux/ummu_core.hinclude/uapi/linux/ummu_core.hinclude/uapi/ub/ubmempfd/ubmempfd.hinclude/ub/cdma/cdma_api.hinclude/ub/ubfi/ubfi.h
💤 Files with no reviewable changes (11)
- anolis/configs/L1-RECOMMEND/arm64/CONFIG_UB_UBRT_PLAT_DEV
- drivers/iommu/hisilicon/ummu_cfg_v1.h
- drivers/iommu/hisilicon/perm_queue.h
- drivers/ub/ubfi/Kconfig
- drivers/ub/cdma/cdma_mbox.h
- drivers/ub/ubfi/ubrt-fwnode.c
- drivers/iommu/hisilicon/seg_tree.c
- drivers/iommu/hisilicon/attribute.c
- drivers/ub/cdma/cdma_jfc.h
- drivers/ub/ubfi/irq.c
- drivers/iommu/hisilicon/nested.c
| void ummu_device_flush_ioplb_all(struct ummu_device *ummu) | ||
| { | ||
| struct ummu_mcmdq_ent cmd = { | ||
| .opcode = CMD_PLBI_OS_EID, | ||
| .plbi = { | ||
| .tecte_tag = LOCAL_TECT_TAG, | ||
| }, | ||
| }; | ||
|
|
||
| ummu_mcmdq_issue_cmd_with_sync(ummu, &cmd); | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
ummu_device_flush_ioplb_all 未检查/记录命令下发失败,与同文件其他 flush 函数处理方式不一致。
同文件中的 ummu_device_flush_plb(Line 391-396)与 ummu_device_flush_plb_all(Line 417-422)均对 ummu_mcmdq_issue_cmd_with_sync 的返回值做 dev_err 记录,而新增的 ummu_device_flush_ioplb_all 完全忽略返回值。若命令下发失败,IOPLB 缓存未被正确失效,可能残留陈旧的地址转换项,且此故障不会被记录,增加排障难度。
建议保持与同文件其余 flush 接口一致的错误处理方式。
🛠️ 建议修复
void ummu_device_flush_ioplb_all(struct ummu_device *ummu)
{
struct ummu_mcmdq_ent cmd = {
.opcode = CMD_PLBI_OS_EID,
.plbi = {
.tecte_tag = LOCAL_TECT_TAG,
},
};
+ int ret;
- ummu_mcmdq_issue_cmd_with_sync(ummu, &cmd);
+ ret = ummu_mcmdq_issue_cmd_with_sync(ummu, &cmd);
+ if (ret)
+ dev_err(ummu->dev, "issue plbi os eid cmd failed, ret = %d\n", ret);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void ummu_device_flush_ioplb_all(struct ummu_device *ummu) | |
| { | |
| struct ummu_mcmdq_ent cmd = { | |
| .opcode = CMD_PLBI_OS_EID, | |
| .plbi = { | |
| .tecte_tag = LOCAL_TECT_TAG, | |
| }, | |
| }; | |
| ummu_mcmdq_issue_cmd_with_sync(ummu, &cmd); | |
| } | |
| void ummu_device_flush_ioplb_all(struct ummu_device *ummu) | |
| { | |
| struct ummu_mcmdq_ent cmd = { | |
| .opcode = CMD_PLBI_OS_EID, | |
| .plbi = { | |
| .tecte_tag = LOCAL_TECT_TAG, | |
| }, | |
| }; | |
| int ret; | |
| ret = ummu_mcmdq_issue_cmd_with_sync(ummu, &cmd); | |
| if (ret) | |
| dev_err(ummu->dev, "issue plbi os eid cmd failed, ret = %d\n", ret); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drivers/iommu/hisilicon/flush.c` around lines 425 - 436,
ummu_device_flush_ioplb_all currently ignores the return value from
ummu_mcmdq_issue_cmd_with_sync, unlike ummu_device_flush_plb and
ummu_device_flush_plb_all. Update this function to capture the result of the
command submission, log a dev_err with the ummu device context when it fails,
and keep the error handling consistent with the other flush helpers in the same
file.
| if (dev) { | ||
| master = (struct ummu_master *)dev_iommu_priv_get(dev); | ||
| iopf_enabled = ummu_master_iopf_enabled(master); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查 ummu_master_iopf_enabled 定义是否处理 NULL 入参
ast-grep run --pattern 'bool ummu_master_iopf_enabled($$$) { $$$ }' --lang c drivers/iommu/hisilicon/sva.c
rg -nP --type=c -A8 '\bummu_master_iopf_enabled\s*\(' drivers/iommu/hisiliconRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 2912
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the surrounding code and how dev_iommu_priv_get() is used elsewhere.
sed -n '760,790p' drivers/iommu/hisilicon/iommu.c
printf '\n---\n'
rg -n --type=c 'dev_iommu_priv_get\s*\(' drivers/iommu/hisilicon drivers/iommu | sed -n '1,120p'
printf '\n---\n'
sed -n '96,120p' drivers/iommu/hisilicon/sva.cRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 11981
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether the surrounding function already guarantees master is non-NULL.
ast-grep outline drivers/iommu/hisilicon/iommu.c --view expandedRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 3391
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Probe for any NULL checks around master/iopf_enabled in this file.
rg -n --type=c 'master\s*==\s*NULL|!\s*master|iopf_enabled' drivers/iommu/hisilicon/iommu.c drivers/iommu/hisilicon/sva.cRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 1152
给 master 增加空指针保护
dev_iommu_priv_get(dev) 可能为 NULL,而 ummu_master_iopf_enabled() 会直接解引用 master->iopf_enabled,这里会触发空指针崩溃。master == NULL 时应直接保持 iopf_enabled = true 的默认值。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drivers/iommu/hisilicon/iommu.c` around lines 776 - 779, The master pointer
in the IOMMU path can be NULL, so guard the result of dev_iommu_priv_get(dev)
before calling ummu_master_iopf_enabled(). Update the logic in the dev/master
handling block to check master explicitly and keep iopf_enabled at its default
true value when master is NULL, instead of dereferencing it.
| page = alloc_pages(GFP_HIGHUSER_MOVABLE | __GFP_COMP | __GFP_ZERO, | ||
| blk_para->block_size_order); | ||
| if (!page) { | ||
| pr_err("allocate mapt block(%lu bytes) failed\n", | ||
| (1U << blk_para->block_size_order) * PAGE_SIZE); | ||
| return -ENOMEM; | ||
| } | ||
| alloc_ptr = page_address(page); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查 mapt block 分配的历史 GFP 标志与是否存在 pin/隔离迁移的处理
rg -nP 'alloc_pages|__get_free_pages|GFP_HIGHUSER_MOVABLE|__GFP_MOVABLE|pin_user_pages|isolate' drivers/iommu/hisilicon/perm_table.c
# 确认 mapt_blk_phys 如何交给硬件
rg -nP 'mapt_blk_phys|mapt_blk_tbl_phys' drivers/iommu/hisilicon -C2Repository: jackYoung0915/anolis-cloud-kernel
Length of output: 7104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 查看 perm_table.c 相关区间,确认分配/回填/释放路径
sed -n '146,245p' drivers/iommu/hisilicon/perm_table.c
printf '\n----\n'
sed -n '304,395p' drivers/iommu/hisilicon/perm_table.c
# 查找是否有对这些页的 pin / 迁移隔离 / mlock 之类的保护
rg -n 'pin_user_pages|get_user_pages|mlock|migrate|isolate|page_maybe_dma_pinned|try_to_migrate|alloc_pages\(GFP_HIGHUSER_MOVABLE|__GFP_MOVABLE' drivers/iommu/hisilicon -C2Repository: jackYoung0915/anolis-cloud-kernel
Length of output: 9569
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 仅针对当前文件检查是否存在明确的迁移保护或固定页引用
rg -n 'page_address\(|phys_to_virt\(|__free_pages\(|free_pages\(|SetPage|PageMovable|__GFP_MOVABLE|GFP_HIGHUSER_MOVABLE' drivers/iommu/hisilicon/perm_table.c -C3Repository: jackYoung0915/anolis-cloud-kernel
Length of output: 4204
这里不要用 GFP_HIGHUSER_MOVABLE 分配 MAPT 块。 这些页的物理地址会写入 tct_desc->mapt_blk_phys / tct_desc->mapt_blk_tbl_phys 直接交给硬件,entry/table 两处都一样;一旦页被迁移,硬件还会继续使用旧 PA。改成不可迁移的分配标志。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drivers/iommu/hisilicon/perm_table.c` around lines 157 - 164, In the MAPT
block allocation path in perm_table.c, the page allocator currently uses
GFP_HIGHUSER_MOVABLE, which makes the pages migratable even though their
physical addresses are stored in tct_desc->mapt_blk_phys and
tct_desc->mapt_blk_tbl_phys for hardware use. Update the allocation in the MAPT
block setup code to use a non-migratable GFP flag combination instead, keeping
the rest of the page allocation and error handling unchanged.
| ret = ummu_ungrant_imp(mapt_info, &data_info); | ||
| if (ret) | ||
| goto clear_info; | ||
|
|
||
| ret = ummu_update_info(data_info.op, mapt_info, &data_info); | ||
| if (ret == 0) | ||
| ret = ummu_update_info(data_info.op, mapt_info, &data_info); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect the relevant region with line numbers.
git ls-files 'drivers/iommu/hisilicon/perm_table.c'
sed -n '1288,1345p' drivers/iommu/hisilicon/perm_table.cRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 1898
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the full function signature and nearby callers/usages of plb_gather.
sed -n '1230,1355p' drivers/iommu/hisilicon/perm_table.c
printf '\n--- usages ---\n'
rg -n "plb_gather|ummu_ungrant_imp\(|ummu_update_info\(" drivers/iommu/hisilicon/perm_table.cRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 4550
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1160,1238p' drivers/iommu/hisilicon/perm_table.cRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 2370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find call sites and any checks on the return value before PLB flush use.
rg -n "ummu_perm_ungrant\(|plb_gather->size|iommu_plb_gather" -g '!drivers/iommu/hisilicon/perm_table.c' .Repository: jackYoung0915/anolis-cloud-kernel
Length of output: 2257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '390,450p' drivers/iommu/hisilicon/sva.c
printf '\n---\n'
sed -n '470,530p' drivers/iommu/iommu-sva.c
printf '\n---\n'
sed -n '748,810p' drivers/iommu/hisilicon/ummu-core/core_ioctl.c
printf '\n---\n'
sed -n '400,440p' include/linux/iommu.hRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 5946
失败路径不应继续触发 PLB 刷新
ummu_ungrant_imp() 返回非 0 后,这里仍会写入 plb_gather->va/size,而 iommu_sva_ungrant() 会无条件调用 iommu_plb_sync();结果是 ungrant 失败时也会做一次实际的 PLB flush。若失败分支不需要刷新,建议在 ret != 0 时把 plb_gather->size 置 0,和 grant 路径保持一致。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drivers/iommu/hisilicon/perm_table.c` around lines 1318 - 1320, 在
perm_table.c 的 ungrant 失败路径里,ummu_ungrant_imp() 返回非 0 后仍会留下可触发 iommu_plb_sync()
的 plb_gather 状态,导致失败时也执行 PLB 刷新。请在 iommu_sva_ungrant() 相关逻辑中处理 ret != 0 分支,参考
grant 路径的做法,将 plb_gather->size 置 0(必要时同步清理相关 va/size 状态),确保失败时不会继续触发实际 flush。
| while (!ummu_queue_has_space(&llq, n + (sync ? 1 : 0))) { | ||
| local_irq_restore(flags); | ||
| if (ummu_mcmdq_poll_until_not_full(ummu, mcmdq, &llq)) { | ||
| if (ummu_mcmdq_poll_until_not_full(ummu, mcmdq, &llq)) | ||
| dev_err_ratelimited(ummu->dev, "wait MCMDQ not full timeout.\n"); | ||
| return -ETIMEDOUT; | ||
| } | ||
| local_irq_save(flags); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant file and surrounding code
git ls-files drivers/iommu/hisilicon/queue.c drivers/iommu/hisilicon/*.c
echo '--- queue.c around issue path ---'
sed -n '930,1020p' drivers/iommu/hisilicon/queue.c
echo '--- search for ummu_mcmdq_issue_cmd callers ---'
rg -n "ummu_mcmdq_issue_cmd|ummu_mcmdq_poll_until_not_full|ETIMEDOUT|wait MCMDQ not full timeout" drivers/iommu/hisilicon -S
echo '--- candidate caller files excerpts ---'
for f in drivers/iommu/hisilicon/flush.c drivers/iommu/hisilicon/interrupt.c drivers/iommu/hisilicon/queue.c; do
if [ -f "$f" ]; then
echo "### $f"
sed -n '1,260p' "$f" | sed -n '/ummu_mcmdq_issue_cmd/,+40p;/ETIMEDOUT/,+20p;/ummu_mcmdq_poll_until_not_full/,+40p'
fi
doneRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 8644
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file to identify the exact function boundaries first
ast-grep outline drivers/iommu/hisilicon/queue.c --view expanded
echo '--- symbols around issue/wait logic ---'
ast-grep run --pattern 'while (!ummu_queue_has_space(&$Q, $N)) { $$$ }' --lang c drivers/iommu/hisilicon/queue.c
echo '--- direct references to timeout handling in this directory ---'
rg -n "return -ETIMEDOUT|dev_err_ratelimited\\(.*wait MCMDQ not full timeout|poll_until_not_full" drivers/iommu/hisilicon -SRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 5063
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1048,1088p' drivers/iommu/hisilicon/queue.c
sed -n '1176,1210p' drivers/iommu/hisilicon/queue.c
echo '--- poll_until_not_full implementation ---'
sed -n '438,512p' drivers/iommu/hisilicon/queue.cRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 4400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1080,1165p' drivers/iommu/hisilicon/queue.cRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 2880
队列满时不要无界重试
ummu_mcmdq_poll_until_not_full() 超时后只会打日志,外层 while 立刻继续等待;drivers/iommu/hisilicon/queue.c 里两处相同逻辑都会在硬件长期不回收队列时把调用线程卡住。建议保留有界超时/重试上限,或至少向上返回错误,让上层能做退避处理。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drivers/iommu/hisilicon/queue.c` around lines 978 - 983, The queue-wait loops
in ummu_mcmdq_insert and the similar path in drivers/iommu/hisilicon/queue.c
currently retry forever after ummu_mcmdq_poll_until_not_full() times out, which
can block the caller indefinitely when hardware never drains the queue. Add a
bounded retry/timeout limit or propagate an error back to the caller after the
poll timeout, and make sure both existing wait loops use the same capped
behavior so the upper layer can handle backoff or failure instead of spinning
forever.
| ubmempfd-$(CONFIG_UB_UBMEMPFD) := ubmempfd_main.o | ||
|
|
||
| obj-m += ubmempfd.o |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
obj-m 忽略 Kconfig 开关,破坏可选编译语义。
obj-m += ubmempfd.o 是无条件写法,不受 CONFIG_UB_UBMEMPFD 控制;而组成该模块的目标列表 ubmempfd-$(CONFIG_UB_UBMEMPFD) 在选项关闭时为空,会导致构建失败(空模块无源文件)或该模块在未选中时仍被强制编入构建。应改为标准 kbuild 写法。
🛠️ 建议修复
ubmempfd-$(CONFIG_UB_UBMEMPFD) := ubmempfd_main.o
-obj-m += ubmempfd.o
+obj-$(CONFIG_UB_UBMEMPFD) += ubmempfd.o📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ubmempfd-$(CONFIG_UB_UBMEMPFD) := ubmempfd_main.o | |
| obj-m += ubmempfd.o | |
| ubmempfd-$(CONFIG_UB_UBMEMPFD) := ubmempfd_main.o | |
| obj-$(CONFIG_UB_UBMEMPFD) += ubmempfd.o |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drivers/ub/ubmempfd/Makefile` around lines 3 - 5, The Makefile for ubmempfd
is registering the module unconditionally with obj-m, which bypasses
CONFIG_UB_UBMEMPFD and breaks optional build behavior. Update the kbuild entries
around ubmempfd-$(CONFIG_UB_UBMEMPFD) and obj-m so the module is only added when
CONFIG_UB_UBMEMPFD is enabled, using the standard conditional module declaration
pattern for ubmempfd.o and ubmempfd_main.o.
| /* SPDX-License-Identifier: GPL-2.0+ */ | ||
| /* | ||
| * Copyright(c) 2026 HiSilicon Technologies CO., All rights reserved. | ||
| * Description: HiSilicon implementation of the ummu data structure definition. | ||
| */ | ||
|
|
||
| #ifndef _HISI_UMMU_H_ | ||
| #define _HISI_UMMU_H_ | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
缺少 #include <linux/types.h>。
Clang 静态分析报告 u64/u32 类型未定义 (line 14/18/20)。这些类型由 <linux/types.h> 提供,本头文件直接使用却未包含,依赖包含者恰好已引入该类型定义,属于脆弱的隐式依赖。
🐛 建议修复
`#ifndef` _HISI_UMMU_H_
`#define` _HISI_UMMU_H_
+#include <linux/types.h>
+
struct hisi_ummu_tdev_info {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* SPDX-License-Identifier: GPL-2.0+ */ | |
| /* | |
| * Copyright(c) 2026 HiSilicon Technologies CO., All rights reserved. | |
| * Description: HiSilicon implementation of the ummu data structure definition. | |
| */ | |
| #ifndef _HISI_UMMU_H_ | |
| #define _HISI_UMMU_H_ | |
| /* SPDX-License-Identifier: GPL-2.0+ */ | |
| /* | |
| * Copyright(c) 2026 HiSilicon Technologies CO., All rights reserved. | |
| * Description: HiSilicon implementation of the ummu data structure definition. | |
| */ | |
| `#ifndef` _HISI_UMMU_H_ | |
| `#define` _HISI_UMMU_H_ | |
| `#include` <linux/types.h> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@include/linux/hisi_ummu.h` around lines 1 - 9, The header currently uses u64
and u32 without including their defining types header, creating a fragile
implicit dependency. Add the missing linux types include near the top of this
header so UMMU’s type definitions are self-contained, and keep the include guard
and existing declarations in hisi_ummu.h unchanged.
Source: Linters/SAST tools
| struct hisi_ummu_tdev_info { | ||
| int version; | ||
| union { | ||
| struct { | ||
| u64 ummu_idx_mask; | ||
| bool on_chip; | ||
| } v1; | ||
| struct { | ||
| u64 reserved; | ||
| bool on_chip; | ||
| u32 tid; | ||
| } v2; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查 hisi_ummu_tdev_info.version 字段是否被赋值或校验
rg -n -C3 '\.version\b|->version\b' -g '*.c' -g '*.h' | rg -i 'hisi_ummu_tdev_info|ummu_idx_mask|on_chip|\.v1\.|\.v2\.|->v1\.|->v2\.'Repository: jackYoung0915/anolis-cloud-kernel
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== files =='
git ls-files | rg '(^|/)(include/linux/hisi_ummu.h|drivers/ub/ubmempfd/ubmempfd_main.c|drivers/iommu/hisilicon/ubmem-mmu/ubmem_mmu.c)$' || true
echo '== struct definition and references =='
rg -n -C 4 'hisi_ummu_tdev_info|ummu_idx_mask|ubmem_mmu_tdev_support_attr|ubmempfd_alloc_tdev|version' include/linux/hisi_ummu.h drivers/ub/ubmempfd/ubmempfd_main.c drivers/iommu/hisilicon/ubmem-mmu/ubmem_mmu.c || true
echo '== broader references to hisi_ummu_tdev_info symbol =='
rg -n -C 3 'hisi_ummu_tdev_info' . || trueRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 10216
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant files around the suspected call sites.
sed -n '1,220p' include/linux/hisi_ummu.h
printf '\n----\n'
sed -n '1,240p' drivers/ub/ubmempfd/ubmempfd_main.c
printf '\n----\n'
sed -n '1,240p' drivers/iommu/hisilicon/ubmem-mmu/ubmem_mmu.cRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 12902
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every place that writes the union members directly or zero-initializes the struct.
rg -n -C 3 'struct hisi_ummu_tdev_info|\.v1\.|\.v2\.|->v1\.|->v2\.|memset\(.*hisi_ummu_tdev_info|memcpy\(.*hisi_ummu_tdev_info' drivers include || trueRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 50390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== ubmempfd_alloc_tdev full body =='
sed -n '230,290p' drivers/ub/ubmempfd/ubmempfd_main.c
echo '== ubmem_mmu_tdev_support_attr body =='
sed -n '672,705p' drivers/iommu/hisilicon/ubmem-mmu/ubmem_mmu.c
echo '== ubmem_vmmu_tdev_support_attr body =='
sed -n '127,150p' drivers/iommu/hisilicon/ubmem-mmu/ubmem_vmmu_main.c
echo '== logic_ummu_device_support_attr body =='
sed -n '1751,1778p' drivers/iommu/hisilicon/logic_ummu/logic_ummu.cRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 4196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'version\s*=|memset\(&info|struct hisi_ummu_tdev_info info' drivers/ub/ubmempfd/ubmempfd_main.c drivers/iommu/hisilicon/ubmem-mmu/ubmem_vmmu_main.c drivers/iommu/hisilicon/ubmem-mmu/ubmem_mmu.c drivers/iommu/hisilicon/logic_ummu/logic_ummu.c || trueRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 629
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether there are any explicit version-based branches for this struct.
rg -n -C 2 'info->version|\.version\s*=|version\s*==' include drivers | rg 'hisi_ummu_tdev_info|ubmem_mmu|ubmempfd|logic_ummu|ubmem_vmmu' || trueRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'version\s*=|memset\(&info|struct hisi_ummu_tdev_info info' \
drivers/ub/ubmempfd/ubmempfd_main.c \
drivers/iommu/hisilicon/ubmem-mmu/ubmem_vmmu_main.c \
drivers/iommu/hisilicon/ubmem-mmu/ubmem_mmu.c \
drivers/iommu/hisilicon/logic_ummu/logic_ummu.c || trueRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 629
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 'info->version|\.version\s*=|version\s*==' include drivers | \
rg 'hisi_ummu_tdev_info|ubmem_mmu|ubmempfd|logic_ummu|ubmem_vmmu' || trueRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 171
统一初始化 hisi_ummu_tdev_info,不要混用 v1/v2 读取
ubmempfd_alloc_tdev() 只写了 v2.on_chip/v2.tid,而 ubmem_mmu_tdev_support_attr() 却按 v1.ummu_idx_mask 解析同一块 priv。version 目前也没参与写入或分支,至少要先清零并统一按同一版布局读取,否则 ummu_idx_mask 会落到未初始化栈值上,__ffs() 和设备匹配会出错。
🧰 Tools
🪛 Clang (14.0.6)
[error] 14-14: unknown type name 'u64'
(clang-diagnostic-error)
[error] 18-18: unknown type name 'u64'
(clang-diagnostic-error)
[error] 20-20: unknown type name 'u32'
(clang-diagnostic-error)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@include/linux/hisi_ummu.h` around lines 10 - 23, `hisi_ummu_tdev_info` is
being written as `v2` in `ubmempfd_alloc_tdev()` but later read as `v1` in
`ubmem_mmu_tdev_support_attr()`, so the layout must be made consistent.
Initialize the whole `hisi_ummu_tdev_info` to zero before use, then either
populate and consume the same union member everywhere or branch on `version` in
`ubmem_mmu_tdev_support_attr()` and `ubmempfd_alloc_tdev()` so the code never
interprets `priv` with mismatched `v1`/`v2` fields. Also ensure `ummu_idx_mask`
is explicitly set before any `__ffs()` or device matching logic uses it.
| /* UMMU SVA API */ | ||
| /** | ||
| * ummu_sva_grant_range() - Grant va range permission to sva. | ||
| * @Deprecated: use iommu_sva_grant instead. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
kernel-doc excess function parameter description warning @ field
💡 Result:
The "Excess function parameter" warning in kernel-doc occurs when the kernel-doc script detects a discrepancy between the documented parameters in a function's comment block and the actual parameters defined in the function's source code [1][2][3]. Essentially, the documentation identifies a parameter that does not exist in the function signature [1][2]. Common causes include: 1. Mismatched or Stale Documentation: A parameter name may have been renamed, removed, or changed in the function signature, but the corresponding @parameter: entry in the comment block was not updated [1][2]. 2. Incorrect Handling of Variadic Arguments: A frequent source of this warning is using a named parameter (e.g., @args:) to document variadic arguments (...) [3][4]. According to kernel-doc standards, variadic arguments must be documented using the specific notation @...: [5][3]. Using any other name for varargs will cause the script to treat it as an extra, undocumented parameter [3][4]. To resolve this warning: * Verify that every @parameter: entry in your kernel-doc block matches the name of a parameter in the function's signature. * If a parameter has been removed from the function, remove the corresponding documentation line [1]. * If documenting a function with variadic arguments, ensure you use the exact syntax @...: instead of naming the variable (e.g., do not use @args: or @va_args:) [3][6][4]. In some cases, if the code is correct but the documentation is stale, the warning acts as a diagnostic tool to help maintain accurate documentation [1][2]. If you encounter this warning despite correct documentation, it may indicate a limitation or a bug in the kernel-doc script, such as false positives previously associated with specific macro definitions [1][2].
Citations:
- 1: https://lists.openwall.net/linux-kernel/2026/05/01/754
- 2: https://www.spinics.net/lists/kernel/msg6190536.html
- 3: https://lists.openwall.net/linux-kernel/2023/12/20/290
- 4: https://lkml.iu.edu/2312.2/04328.html
- 5: https://docs.kernel.org/6.17/doc-guide/kernel-doc.html
- 6: https://kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant region with line numbers.
wc -l include/linux/ummu_core.h
sed -n '600,900p' include/linux/ummu_core.h | cat -n
# Search for all `@Deprecated` usages in the file.
rg -n '`@Deprecated`:' include/linux/ummu_core.h
# Show nearby context for each hit.
python3 - <<'PY'
from pathlib import Path
path = Path("include/linux/ummu_core.h")
lines = path.read_text().splitlines()
targets = [i+1 for i,l in enumerate(lines) if "`@Deprecated`:" in l]
for n in targets:
start = max(1, n-3)
end = min(len(lines), n+3)
print(f"\n--- lines {start}-{end} around {n} ---")
for i in range(start, end+1):
print(f"{i:4d}: {lines[i-1]}")
PYRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 14514
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant APIs and their replacement names.
rg -n 'iommu_is_(ksva|sva)_domain|iommu_(ksva|sva)_(grant|ungrant|bind_device|unbind_device)|ummu_is_(ksva|sva)\(' include/linux/ummu_core.h
# Search the repository for the underlying iommu APIs to compare naming.
rg -n 'iommu_is_(ksva|sva)_domain|iommu_(ksva|sva)_(grant|ungrant|bind_device|unbind_device)' .
# Show the surrounding definitions where the replacements are declared in this header.
sed -n '680,705p' include/linux/ummu_core.h | cat -nRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 11238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the inline wrappers near the end of the header.
sed -n '915,945p' include/linux/ummu_core.h | cat -n
# Inspect any direct implementations in the C sources.
rg -n 'ummu_is_(ksva|sva)\s*\(' drivers include | sed -n '1,80p'
rg -n 'return .*iommu_is_ksva_domain|!iommu_is_ksva_domain|iommu_is_ksva_domain\(domain\)' drivers include | sed -n '1,120p'Repository: jackYoung0915/anolis-cloud-kernel
Length of output: 2126
将 @Deprecated: 改成普通说明 这些注释会被 kernel-doc 解析成参数条目,W=1/scripts/kernel-doc 会出现 excess description 警告;改成 Deprecated: 或 Note: 即可。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@include/linux/ummu_core.h` at line 621, The doc comment on the IOMMU API is
using a kernel-doc tag-style "`@Deprecated`:" entry, which gets parsed as an
excess description warning. Update the comment in the relevant declaration block
to use a plain note like "Deprecated:" or "Note:" instead of the `@-prefixed`
form, keeping the reference to iommu_sva_grant clear for readers.
|
|
||
| /** | ||
| * ummu_is_sva() - Check whether the UMMU works in sva mode. | ||
| * @Deprecated: use iommu_is_ksva_domain instead. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect the relevant region with line numbers.
grep -nE 'ummu_is_sva|ummu_is_ksva|iommu_is_ksva_domain|Deprecated' include/linux/ummu_core.h | sed -n '1,40p'
echo
nl -ba include/linux/ummu_core.h | sed -n '660,730p'Repository: jackYoung0915/anolis-cloud-kernel
Length of output: 1149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the implementation and nearby declarations without relying on nl.
sed -n '680,950p' include/linux/ummu_core.h
echo
echo "--- search for possible sva/ksva domain helpers ---"
grep -RInE 'iommu_is_(sva|ksva)_domain|ummu_is_(sva|ksva)|sva_domain' include linux 2>/dev/null | sed -n '1,120p'Repository: jackYoung0915/anolis-cloud-kernel
Length of output: 8971
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1980,2060p' include/linux/iommu.hRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 2414
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '920,945p' include/linux/ummu_core.h
echo
grep -RIn --include='*.c' --include='*.h' 'ummu_is_sva(struct iommu_domain' . | sed -n '1,40p'Repository: jackYoung0915/anolis-cloud-kernel
Length of output: 894
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '320,360p' drivers/iommu/hisilicon/ummu-core/core.cRepository: jackYoung0915/anolis-cloud-kernel
Length of output: 1117
弃用说明需要改正:ummu_is_sva() 不是直接对应 iommu_is_ksva_domain();实现里实际是 domain->mm && !iommu_is_ksva_domain(domain),仓库中也没有 iommu_is_sva_domain()。建议把这里的替代说明改成准确的判定,或补一个明确的 SVA helper。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@include/linux/ummu_core.h` at line 697, The deprecation note for
ummu_is_sva() is inaccurate because it does not map directly to
iommu_is_ksva_domain(); update the comment to describe the actual SVA check used
in the implementation, namely the domain->mm plus !iommu_is_ksva_domain(domain)
condition, or add a dedicated helper if you want a true SVA predicate. Make sure
the replacement wording in ummu_core.h matches the real behavior and does not
reference a nonexistent iommu_is_sva_domain().
c94669a to
80f9fd8
Compare
ANBZ: #IJSBBS
commit 8f76aa379f5008e4c26b703dceb9ea56208d8841 openEuler.
The map_pages callback currently hardcodes GFP_KERNEL when allocating
memory, ignoring the gfp_mask argument provided by the caller. This
violates the expected interface contract and may lead to incorrect
allocation behavior in contexts that require specific GFP flags
(e.g., atomic or interrupt context).
Update the allocation site to use the gfp_mask parameter passed into
the callback, ensuring proper memory allocation semantics according
to the caller's requirements.
Fixes: ba669149e43b ("iommu/ummu: Add UB Memory support")
Signed-off-by: Wentao Li <liwentao44@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit b79f2e824f7c1b47a430b0befc0f7a16addbd4ba openEuler.
Fixes: 473c5dcfba43 ("iommu/ummu: Implement iommu_ops and iommu_domain_ops for UMMU driver")
Signed-off-by: Sihui Jiang <jiangsihui@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit e43dd5a1d9253695632a8afd4f502980c11a117f openEuler.
The resources related to power consumption management of the
tdev device are deleted.
Fixes: b46cf0d0b390 ("iommu/ummu-core: add pseudo Token Dev for Token ID abstraction")
Signed-off-by: Jiashun Wang <wangjiashun@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit 6503af822cefd39c468b0d6e66558585c05d263a openEuler.
The ummu/tid permission is changed from the default value 0600 to 0666.
Fixes: b46cf0d0b390 ("iommu/ummu-core: add pseudo Token Dev for Token ID abstraction")
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit c46d6ea08f5e28bf0ce2cd1b9d2038bf853de696 openEuler.
The parameter used for enabling the separated page table is changed from
`en_sva_indep_page_table` to `sva_separated_mode`.
Fixes: e6e805b69b12 ("iommu/ummu: Add ummu sva independent page table parameter")
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit d436c180a6f9e887c80aca728a828986b6099ff3 openEuler.
Disable the UMMU when the initialization fails and exits.
Invalidating the PLB operation is added during initialization.
Fixes: 0db2fc397b9d ("iommu/ummu: Support UMMU device")
Signed-off-by: Jiashun Wang <wangjiashun@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit 1a12b6591b27425644dac40c5316f5b17099c78c openEuler.
Fixes: ba669149e43b ("iommu/ummu: Add UB Memory support")
Signed-off-by: Jiashun Wang <wangjiashun@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit 87f1d87b40af36bf7c02292fd20f66d421176805 openEuler.
ummu driver can directly write to ubmem_mmu register without reading it.
Fixes: ba669149e43b ("iommu/ummu: Add UB Memory support")
Signed-off-by: Jingbin Wu <wujingbin2@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit 1e1c98143474a9f739ca6905096e90fcb65a5291 openEuler.
In the separate page table mode, duplicate address registration may
occur in local memory registration. The l_tid validity check will
restrict this scenario. Therefore, the restriction that l_tid must
be valid is deleted.
Fixes: 47fba8c604d7 ("iommu/ummu: Add the function of creating devices with SVA-separated")
Signed-off-by: Jiashun Wang <wangjiashun@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit 0407d9463704cbc0331b8d8bb2b439bb6d53919f openEuler.
In the code for parsing ummu nodes, the method of obtaining interruption
information is incorrect. The related interfaces and calls are deleted.
Fixes: 010c6364261c ("ub: ubfi: Parsing ummu node in the ubrt table")
Signed-off-by: Jiashun Wang <wangjiashun@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit f24a29be087a040ad1fb0dbdd046f13166c5d578 openEuler.
The hisi_ummu_tdev_info data structure is used externally,
and its definition is moved from the driver to the 'hisi_ummu.h'
header file.
Fixes: 7876e979bbdb ("iommu/ummu: Implement domain and core ops in logic UMMU framework")
Signed-off-by: Jiashun Wang <wangjiashun@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit 44169ac7ea645f80d326d68f2b401ea26c26392e openEuler.
1.The code of the `get_iommu_dev` and `select_ummu_device` functions
is optimized to avoid code duplication.
2.Remove redundant code from `tid_misc_init`
Fixes: b46cf0d0b390 ("iommu/ummu-core: add pseudo Token Dev for Token ID abstraction")
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit d12471e38991efd1fa97cba8eec7ea6b200eda3e openEuler.
Add module author.
Fixes: 086a741d7b71 ("ub: ubmempfd: supports for D2H mapping and demapping")
Signed-off-by: Lizhi He <helizhi1@huawei.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit ff8a83eb72ebbc97bc4b6807bad212f95b6461f9 openEuler.
Adjust log prints.
Fixes: be6b2323aed1 ("ub: ubmem_vmmu: supports IOMMU driver for the UB memory MMU in VMs")
Signed-off-by: Lizhi He <helizhi1@huawei.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit 782f81548d235ed2cfcdb168d8674fc64970ef5b openEuler.
Involving upper limit of parameter values in verification.
Fixes: d12471e38991 ("ub/ubmempfd: Add module author")
Signed-off-by: Lizhi He <helizhi1@huawei.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit 8763959c3c0e0ef3b55e7963dd4f31ae2cd31f68 openEuler.
Mark several UMMU SVA APIs as deprecated and indicate their
replacement functions:
- ummu_sva_grant_range -> iommu_sva_grant
- ummu_sva_ungrant_range -> iommu_sva_ungrant
- ummu_is_ksva -> iommu_is_ksva_domain
- ummu_is_sva -> iommu_is_ksva_domain
- ummu_sva_bind_device -> iommu_sva_bind_device_isolated
- ummu_sva_unbind_device -> iommu_sva_unbind_device_isolated
- ummu_ksva_bind_device -> iommu_ksva_bind_device
- ummu_ksva_unbind_device -> iommu_ksva_unbind_device
Fixes: 9db5eb9c30ff ("iommu/ummu-core: core interfaces for ummu drivers")
Signed-off-by: Yanlong Zhu <zhuyanlong3@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit 0a08911fd09b0edabdc5d002270c8cd8336c2432 openEuler.
Fix resource leak when FREE_TID is called before munmap:
Problem:
- When FREE_TID is called first, xa_erase removes the ktid_info entry
- Subsequent munmap finds no entry and cannot release block/queue resources
- The sva and dev pointers may have already been freed by FREE_TID
Solution:
- Add mmap_count (atomic_t) to ktid_info to track active mmaps
- Increment on successful mmap, decrement on munmap when resources released
- FREE_TID returns -EBUSY if mmap_count > 0
- This ensures sva and dev remain valid for munmap to release resources
Fixes: 8d20e0ec2423 ("iommu/ummu-core: impl ioctl interface for /dev/ummu device")
Signed-off-by: Gao Chao <gaochao24@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit c9d4d65fe047ad565e5e699f9bd8a6d1a0231958 openEuler.
Fix kref imbalance when VMA is split:
Problem:
- Each VMA split calls vm_ops->open which increments kref via kref_get
- But kref_put was only called in the release path (when page_cnt <= 0)
- This causes kref leak when partial unmap occurs (page_cnt > 0)
- Eventually leads to memory leak of mmap_info structures
Fix:
- Always call kref_put at the end of tid_munmap to maintain balance
- Each VMA close corresponds to one VMA open (during split)
- kref_put must be called for every close to maintain balance
Also add NULL check for map_info at the beginning of tid_munmap
to handle edge cases gracefully.
Fixes: 8d20e0ec2423 ("iommu/ummu-core: impl ioctl interface for /dev/ummu device")
Signed-off-by: Gao Chao <gaochao24@huawei.com>
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
ANBZ: #IJSBBS
commit d3cb8570930793e49ac05f1e2caf76a017acca51 openEuler.
Fixes: 010c6364261c ("ub: ubfi: Parsing ummu node in the ubrt table")
Signed-off-by: Liming An <anliming1@h-partners.com>
Signed-off-by: qiushengming <qiushengming1@huawei.com>
commit c92dbb26fb432491ae9f50cb9534947c48c4356e openEuler ubmmu inclusion category: bugfix bugzilla: https://atomgit.com/openeuler/kernel/issues/8473 ---------------------------------------------- Refresh PLB in 'iommu_sva_grant' and 'iommu_sva_ungrant', regardless of success of failure. When the 'grant' and 'ungrant' fail,the PLB must be refresh to ensure that the hardware cache is empty and the permission verification result is consistent with the software configuration. Fixes: d46a4bd5bdfd ("iommu/ummu-core: introduce iommu sva permission operation") Signed-off-by: Yanlong Zhu <zhuyanlong3@huawei.com> Signed-off-by: Liming An <anliming1@h-partners.com> Signed-off-by: Shengming Qiu <846759657@qq.com>
commit 10b6909d17b83854a266b7747f42094098f5912e openEuler ubmempfd inclusion category: bugfix bugzilla: https://atomgit.com/openeuler/kernel/issues/9223 ---------------------------------------------- Add more annotation explanations and optimize formats. Fixes: 086a741d7b71 ("ub: ubmempfd: supports for D2H mapping and demapping") Signed-off-by: Lizhi He <helizhi1@huawei.com> Signed-off-by: Shengming Qiu <846759657@qq.com>
commit b517d489a092defec3c518d88e55616a71dc27a3 openEuler ubmem_vmmu inclusion category: bugfix bugzilla: https://atomgit.com/openeuler/kernel/issues/9223 ---------------------------------------------- Adjust log prints. Fixes: be6b2323aed1 ("ub: ubmem_vmmu: supports IOMMU driver for the UB memory MMU in VMs") Signed-off-by: Lizhi He <helizhi1@huawei.com> Signed-off-by: Shengming Qiu <846759657@qq.com>
commit 4a4e13a98de88861a25473f29a38ad679526146e openEuler ubmmu inclusion category: bugfix bugzilla: https://atomgit.com/openeuler/kernel/issues/9131 -------------------------------- Add plbi_list to struct iommu_plb_gather, which collects PLBIs during grant/ungrant operations. Then PLBIs in plbi_list are issued in iommu_plb_sync. This implementation enables flexible issuance of PLBIs in terms of both quantity and type. Fixes: d46a4bd5bdfd ("iommu/ummu-core: introduce iommu sva permission operation") Signed-off-by: Lizhi He <helizhi1@huawei.com> Signed-off-by: Liming An <anliming1@h-partners.com> Signed-off-by: Shengming Qiu <846759657@qq.com>
ANBZ: #IJSBBS commit 72d49e62f0ab93350e9b98b27fec6d2cb0957e8a openEuler. Add support for sharing TID among multiple tdev allocations within the same mm (mm_struct) when share_by_mm is enabled. Key changes: Introduce tdev_opt structure with mm and share_by_mm fields Introduce ummu_core_alloc_separate_tdev() to support TID sharing within same mm Add xarray-based mm to tdev mapping for TID sharing Implement reference counting with kref for shared tdev lifecycle Behavior: When share_by_mm=false: Always allocate new TID (original behavior) When share_by_mm=true: Return existing TID if mm already has one, otherwise allocate and cache new TID for that mm Shared TIDs are reference counted and freed when last reference is released Signed-off-by: Yanlong Zhu <zhuyanlong3@huawei.com> Signed-off-by: Liming An <anliming1@h-partners.com> Signed-off-by: qiushengming <qiushengming1@huawei.com>
commit 53658de93113f36a2729ca9f0c3a64639e37d100 openEuler ubmmu inclusion category: bugfix bugzilla: https://atomgit.com/openeuler/kernel/issues/9073 -------------------------------- In the user-space udma module, during the invalidation of a tid prior to its deletion, an address access failure is encountered at address 0xdeacfffffffffff8 when the base_domain member is accessed within the ummu_cfg_sync function. This issue arises during process termination when the so (shared object) is being destructed, leading to the removal of all tid and domain resources. If a thread calls urma_unregister_seg during this period, it triggers the udma invalidation process, which unfortunately attempts to access memory that has already been freed, causing the failure. To solve this problem, the key of the proc_info_xa process is changed to the mm of the process, so that the process lock can be obtained during invalidation to be mutually exclusive with the deletion of the tid. Fixes: 9db5eb9c30ff ("iommu/ummu-core: core interfaces for ummu drivers") Signed-off-by: Liming An <anliming1@h-partners.com> Signed-off-by: Shengming Qiu <846759657@qq.com>
Summary by CodeRabbit
新功能
Bug 修复