Re: [PATCH 8/8] iommu: Improve map/unmap sanity checks

[Jason Gunthorpe <jgg@ziepe.ca>]

On Tue, Sep 12, 2023 at 05:18:44PM +0100, Robin Murphy wrote:
> The current checks for the __IOMMU_DOMAIN_PAGING capability seem a
> bit stifled, since it is quite likely now that a non-paging domain
> won't have a pgsize_bitmap and/or mapping ops, and thus get caught
> by the earlier condition anyway. Swap them around to test the more
> fundamental condition first, then we can reasonably also upgrade
> the other to a WARN_ON, since if a driver does ever expose a paging
> domain without the means to actually page, it's clearly very broken.

> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  drivers/iommu/iommu.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index c83f2e4c56f5..391bcae4d02d 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2427,12 +2427,12 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,
>  	phys_addr_t orig_paddr = paddr;
>  	int ret = 0;
>  
> -	if (unlikely(!ops->map_pages || domain->pgsize_bitmap == 0UL))
> -		return -ENODEV;
> -
>  	if (unlikely(!(domain->type & __IOMMU_DOMAIN_PAGING)))
>  		return -EINVAL;

Why isn't this a WARN_ON? The caller is clearly lost its mind if it
is calling map on a non paging domain..

> +	if (WARN_ON(!ops->map_pages || domain->pgsize_bitmap == 0UL))
> +		return -ENODEV;
> +

And these could be moved to after attach so they are not in any fast
path, and eventually to after alloc..

Jason