From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: iommu: direct page-table drivers implement iova_to_phys_length
Date: Thu, 04 Jun 2026 15:00:35 +1000
Message-ID: <review-patch6-20260531093637.3893199-7-guanghuifeng@linux.alibaba.com>
In-Reply-To: <20260531093637.3893199-7-guanghuifeng@linux.alibaba.com>
References: <20260531093637.3893199-1-guanghuifeng@linux.alibaba.com> <20260531093637.3893199-7-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

This is the largest patch in v1, touching 11 drivers. Most are mechanical, but a few deserve attention:

**msm_iommu** -- The supersection size is hardcoded as SZ_16M, and the non-supersection case returns SZ_4K. However, MSM IOMMU also supports 64K and 1M pages. The hardware translation (`GET_PAR`) doesn't easily reveal the page size, so SZ_4K is a conservative fallback. This should be documented.

**mtk_iommu_v1** -- The commit message claims a bug fix ("add page offset to PA"), and indeed the patch adds:
```c
if (pa) {
    pa |= (iova & (MT2701_IOMMU_PAGE_SIZE - 1));
    ...
}
```
The original `mtk_iommu_v1_iova_to_phys` did NOT add the page offset, so `iova_to_phys(domain, 0x1234)` would return the page-aligned PA without the `0x234` offset. This is a genuine pre-existing bug. However, mixing bug fixes with interface changes makes bisection harder -- this should be a separate preparatory patch.

**fsl_pamu** -- The `1ULL << 36` (64GB) hardcoded size is correct for the PAMU's single-window identity mapping, and the comment explains it well.

**virtio-iommu** -- Returns the full mapping interval size:
```c
*mapped_length = mapping->iova.last - mapping->iova.start + 1;
```
This is the size of the software-tracked mapping, not a hardware page size. It's semantically different from what other drivers return (PTE entry size), but arguably correct for virtio since the host-side mapping is opaque. Worth noting this semantic inconsistency.

---
Generated by Claude Code Patch Reviewer