Re: [PATCH] iommu: Flow ERR_PTR out from __iommu_domain_alloc()

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Nov 01, 2023 at 11:52:08AM -0700, Jerry Snitselaar wrote:
> On Wed, Nov 01, 2023 at 02:51:02PM -0300, Jason Gunthorpe wrote:
> > Most of the calling code now has error handling that can carry an error
> > code further up the call chain. Keep the exported interface
> > iommu_domain_alloc() returning NULL and reflow the internal code to use
> > ERR_PTR not NULL for domain allocation failure.
> > 
> > Optionally allow drivers to return ERR_PTR from any of the alloc ops. Many
> > of the new ops (user, sva, etc) already return ERR_PTR, so having two
> > rules is confusing and hard on drivers. This fixes a bug in DART that was
> > returning ERR_PTR.
> > 
> > This also fixes a bug in iommu_group_alloc_default_domain() where it
> > returned both NULL and ERR_PTR, now it always returns ERR_PTR.
> > 
> > Fixes: 482feb5c6492 ("iommu/dart: Call apple_dart_finalize_domain() as part of alloc_paging()")
> > Fixes: 1c68cbc64fe6 ("iommu: Add IOMMU_DOMAIN_PLATFORM")
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Link: https://lore.kernel.org/linux-iommu/b85e0715-3224-4f45-ad6b-ebb9f08c015d@moroto.mountain/
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > ---
> >  drivers/iommu/iommu.c | 53 +++++++++++++++++++++++++++++--------------
> >  1 file changed, 36 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index f17a1113f3d6a3..6bc044d2310487 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -1802,10 +1802,10 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type)
> >  
> >  	/* Otherwise IDENTITY and DMA_FQ defaults will try DMA */
> >  	if (iommu_def_domain_type == IOMMU_DOMAIN_DMA)
> > -		return NULL;
> > +		return ERR_PTR(-ENODEV);
> >  	dom = __iommu_group_alloc_default_domain(group, IOMMU_DOMAIN_DMA);
> >  	if (!dom)
> 
> Should this be if(IS_ERR(dom)) ?

Blah, I think I somehow missed this function:

@@ -1788,7 +1788,7 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type)
         */
        if (ops->default_domain) {
                if (req_type)
-                       return NULL;
+                       return ERR_PTR(-EINVAL);
                return ops->default_domain;
        }
 
@@ -1797,14 +1797,14 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type)
 
        /* The driver gave no guidance on what type to use, try the default */
        dom = __iommu_group_alloc_default_domain(group, iommu_def_domain_type);
-       if (dom)
+       if (!IS_ERR(dom))
                return dom;
 
        /* Otherwise IDENTITY and DMA_FQ defaults will try DMA */
        if (iommu_def_domain_type == IOMMU_DOMAIN_DMA)
-               return ERR_PTR(-ENODEV);
+               return ERR_PTR(-EINVAL);
        dom = __iommu_group_alloc_default_domain(group, IOMMU_DOMAIN_DMA);
-       if (!dom)
+       if (IS_ERR(dom))
                return dom;
 
        pr_warn("Failed to allocate default IOMMU domain of type %u for group %s - Falling back to IOMMU_DOMAIN_DMA",


Thanks! 
Jason