From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: iommu: introduce iova_to_phys_length in iommu_domain_ops Date: Thu, 04 Jun 2026 15:00:34 +1000 Message-ID: In-Reply-To: <20260531093637.3893199-2-guanghuifeng@linux.alibaba.com> References: <20260531093637.3893199-1-guanghuifeng@linux.alibaba.com> <20260531093637.3893199-2-guanghuifeng@linux.alibaba.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review The core infrastructure patch. The v1 version uses 0 as error return and the v2 switches to PHYS_ADDR_MAX -- I'll review the v1 as submitted. **v1 issues:** The IDENTITY domain case doesn't set `*mapped_length`: ```c if (domain->type == IOMMU_DOMAIN_IDENTITY) return iova; ``` If a caller passes a non-NULL `mapped_length` and gets an IDENTITY domain, `mapped_length` stays 0 (from the initialization at the top). This is arguably fine since IDENTITY domains shouldn't be used in the VFIO/iommufd paths, but it's an inconsistency -- the caller cannot distinguish "no translation" from "identity" by looking at `mapped_length`. The BLOCKED domain check was removed in v1 but the fallback handles it implicitly (both `iova_to_phys_length` and `iova_to_phys` would be NULL for BLOCKED domains, falling through to `return 0`). This is fine. **v2 improvement**: The v2 version checks `!domain->ops->iova_to_phys_length` instead of `domain->type`, which is cleaner since it handles BLOCKED domains naturally and doesn't need to know about domain type internals. However, the v2 fallback for legacy `iova_to_phys` returns `PHYS_ADDR_MAX` when `phys == 0`, which maps "no translation" to "error" -- correct but worth documenting clearly. --- Generated by Claude Code Patch Reviewer