Re: [PATCH v3 05/32] iommu/generic_pt: implement iova_to_phys_length

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] iova_to_phys_length overstates mapped_length for unaligned IOVAs, causing boundary overshoot.
- [Medium] Unnecessary O(N) page table traversal when mapped_length is not requested.
- [Medium] Contiguous physical length accumulation artificially truncates at page table boundaries.
--

commit a86cc79db974a9c9b664e54606ef192f94d2d77f
Author: Guanghui Feng <guanghuifeng@linux.alibaba.com>

iommu/generic_pt: implement iova_to_phys_length
    
Extend the Generic Page Table framework to implement iova_to_phys_length.
Use pt_entry_oa_lg2sz() to determine PTE block size. Update
IOMMU_PT_DOMAIN_OPS macro to set .iova_to_phys_length.

> diff --git a/drivers/iommu/generic_pt/iommu_pt.h b/drivers/iommu/generic_pt/iommu_pt.h
> index c2752151c80af..914215b467213 100644
> --- a/drivers/iommu/generic_pt/iommu_pt.h
> +++ b/drivers/iommu/generic_pt/iommu_pt.h
[ ... ]
> @@ -180,13 +180,21 @@ static inline unsigned int compute_best_pgsize(struct pt_state *pts,
> +static __always_inline int __do_iova_to_phys_length(struct pt_range *range,
> +					       void *arg, unsigned int level,
> +					       struct pt_table_p *table,
> +					       pt_level_fn_t descend_fn)
>  {
[ ... ]
> +	data->phys = pt_entry_oa_exact(&pts);
> +	entry_lg2sz = pt_entry_oa_lg2sz(&pts);
> +	entry_sz = log2_to_int(entry_lg2sz);
> +
> +	/* Start with the full mapping size of the first entry */
> +	data->length = entry_sz;

[Severity: High]
Does this calculation overstate the mapped length for unaligned IOVAs?

If the IOVA is not aligned to the PTE block size, pt_entry_oa_exact()
includes the intra-page offset in data->phys. However, data->length
is unconditionally initialized to the full entry_sz rather than
entry_sz - offset. Callers relying on mapped_length might operate
on out-of-bounds memory because data->phys + data->length extends
beyond the valid mapped physical memory by the unaligned offset amount.

> +
> +	/* Accumulate subsequent physically contiguous entries */
> +	expected_oa = pt_entry_oa(&pts) + entry_sz;
> +	pts.end_index = log2_to_int(pt_num_items_lg2(&pts));
> +	pt_next_entry(&pts);
> +
> +	while (pts.index < pts.end_index) {

[Severity: Medium]
Could this introduce overhead by performing an unnecessary traversal?

When iova_to_phys_length() is called via API paths that do not request
a length (passing NULL for mapped_length), this loop still
unconditionally iterates over all remaining page table entries in the
current block. This adds extra work for lookups that just need the
physical address.

[Severity: Medium]
Is the contiguous physical length accumulation artificially truncated
at page table boundaries?

The function documentation states that callers can determine the full
contiguous mapping extent with a single call. However, this loop
terminates at pts.end_index, which is the end of the current leaf
page table block. It does not ascend or cross into the next page
table block to verify if the mapping remains physically contiguous.

> +		pt_load_entry(&pts);
> +		if (pts.type != PT_ENTRY_OA)
> +			break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603151804.1963871-1-guanghuifeng@linux.alibaba.com?part=5