Re: [PATCH 4/9] iommu/arm-smmu: implement iova_to_phys_length

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] `arm_smmu_iova_to_phys_length` fails to populate `mapped_length` when using the hardware translation path, silently defeating the page table walk optimization.

Pre-existing issues:
- [High] The `qcom_smmu_context_fault` interrupt handler can dereference a NULL `pgtbl_ops` pointer if a context fault interrupt fires before domain initialization completes.
--

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

iommu/arm-smmu: implement iova_to_phys_length

Migrate ARM SMMU drivers to implement iova_to_phys_length, calling
ops->iova_to_phys_length on the io-pgtable layer to get both phys
and PTE page size.

> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c
> index 65e0ef6539fe7..4fd01341157f3 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c
> @@ -415,7 +415,7 @@ irqreturn_t qcom_smmu_context_fault(int irq, void *dev)
>  		return IRQ_HANDLED;
>  	}
>  
> -	phys_soft = ops->iova_to_phys(ops, cfi.iova);
> +	phys_soft = ops->iova_to_phys_length(ops, cfi.iova, NULL);

[Severity: High]
This is a pre-existing issue, but is it possible for ops to be NULL here?

If a context fault interrupt fires early during domain initialization, before
pgtbl_ops is safely published in arm_smmu_init_domain_context():

    ret = devm_request_irq(smmu->dev, irq, context_fault, ...);
    ...
    mutex_unlock(&smmu_domain->init_mutex);
    smmu_domain->pgtbl_ops = pgtbl_ops;

Could an unhandled fault on a reused context bank cause an immediate
interrupt, leading to a NULL pointer dereference when
ops->iova_to_phys_length() is called?

> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 0bd21d206eb3e..e29e0ea12f24a 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -1366,7 +1366,7 @@ static phys_addr_t arm_smmu_iova_to_phys_hard(struct iommu_domain *domain,
>  			"iova to phys timed out on %pad. Falling back to software table walk.\n",
>  			&iova);
>  		arm_smmu_rpm_put(smmu);
> -		return ops->iova_to_phys(ops, iova);
> +		return ops->iova_to_phys_length(ops, iova, NULL);

[Severity: High]
Does this hardware translation fallback path fail to populate mapped_length?

Since mapped_length is passed as NULL, callers like VFIO attempting to unmap
large regions might receive a length of 0. This could force them to fall back
to PAGE_SIZE iterations, which would trigger hundreds of slow ATOS MMIO
operations and severely impact performance.

[ ... ]

> @@ -1384,12 +1384,15 @@ static phys_addr_t arm_smmu_iova_to_phys_hard(struct iommu_domain *domain,
>  	return addr;
>  }
>  
> -static phys_addr_t arm_smmu_iova_to_phys(struct iommu_domain *domain,
> -					dma_addr_t iova)
> +static phys_addr_t arm_smmu_iova_to_phys_length(struct iommu_domain *domain,
> +					dma_addr_t iova, size_t *mapped_length)
>  {
>  	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>  	struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops;
>  
> +	if (mapped_length)
> +		*mapped_length = 0;
> +
>  	if (!ops)
>  		return 0;
>  
> @@ -1397,7 +1400,7 @@ static phys_addr_t arm_smmu_iova_to_phys(struct iommu_domain *domain,
>  			smmu_domain->stage == ARM_SMMU_DOMAIN_S1)
>  		return arm_smmu_iova_to_phys_hard(domain, iova);

[Severity: High]
Does the hardware translation path here also defeat the page table walk
optimization by failing to populate mapped_length?

When arm_smmu_iova_to_phys_hard() is called, mapped_length is left as 0 since
the function doesn't accept or update it. Will this result in callers
receiving a length of 0 and performing inefficient PAGE_SIZE iterations?

> -	return ops->iova_to_phys(ops, iova);
> +	return ops->iova_to_phys_length(ops, iova, mapped_length);
>  }

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