Re: [PATCH v3 4/6] iommu: Regulate EINVAL in ->attach_dev callback functions

[Nicolin Chen]

On Tue, Sep 20, 2022 at 06:29:28AM +0000, Tian, Kevin wrote:
> External email: Use caution opening links or attachments
> 
> 
> > From: Nicolin Chen <nicolinc@xxxxxxxxxx>
> > Sent: Thursday, September 15, 2022 3:59 PM
> >
> > diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> > index c5e7e8b020a5..4cadbd744b7f 100644
> > --- a/drivers/iommu/intel/pasid.c
> > +++ b/drivers/iommu/intel/pasid.c
> > @@ -101,8 +101,10 @@ int intel_pasid_alloc_table(struct device *dev)
> >
> >       might_sleep();
> >       info = dev_iommu_priv_get(dev);
> > -     if (WARN_ON(!info || !dev_is_pci(dev) || info->pasid_table))
> > -             return -EINVAL;
> > +     if (WARN_ON(!info || !dev_is_pci(dev)))
> > +             return -ENODEV;
> > +     if (WARN_ON(info->pasid_table))
> > +             return -EBUSY;
> 
> -EEXIST. It's not a busy condition which might change sometime after.
> 
> btw below -EVINAL in viommu should be converted to -ENODEV too
> since it's a check purely on device side:
> 
> viommu_domain_finalise()
>         viommu_page_size = 1UL << __ffs(viommu->pgsize_bitmap);
>         if (viommu_page_size > PAGE_SIZE) {
>                 dev_err(vdev->dev,
>                         "granule 0x%lx larger than system page size 0x%lx\n",
>                         viommu_page_size, PAGE_SIZE);
>                 return -EINVAL;
>         }

Fixed both places. Thanks!