| Message ID | cd613152e5175b5ffac643ee017b1d800e766d99.1663227492.git.nicolinc@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Define EINVAL as device/domain incompatibility | expand |
> From: Nicolin Chen <nicolinc@nvidia.com> > Sent: Thursday, September 15, 2022 3:59 PM > > Following the new rules in include/linux/iommu.h kdocs, EINVAL now can be > used to indicate that domain and device are incompatible by a caller that > treats it as a soft failure and tries attaching to another domain. > > Either mtk_iommu or virtio driver has a place that returns a hard failure > instead of the return value from the function call, where an incompatible > errno EINVAL could potentially occur. in both cases there is no EINVAL returned from the calling stack IMHO error propagation is the right way even w/o talking about EINVAL otherwise we may miss ENOMEM etc. > > Propagate the real return value to not miss a potential soft failure. > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> Apart from that comment, Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On Tue, Sep 20, 2022 at 06:50:22AM +0000, Tian, Kevin wrote: > External email: Use caution opening links or attachments > > > > From: Nicolin Chen <nicolinc@nvidia.com> > > Sent: Thursday, September 15, 2022 3:59 PM > > > > Following the new rules in include/linux/iommu.h kdocs, EINVAL now can be > > used to indicate that domain and device are incompatible by a caller that > > treats it as a soft failure and tries attaching to another domain. > > > > Either mtk_iommu or virtio driver has a place that returns a hard failure > > instead of the return value from the function call, where an incompatible > > errno EINVAL could potentially occur. > > in both cases there is no EINVAL returned from the calling stack > > IMHO error propagation is the right way even w/o talking about EINVAL > otherwise we may miss ENOMEM etc. OK. I changed to: The mtk_iommu and virtio drivers have places in the ->attach_dev callback functions that return hardcode errnos instead of the returned values, but callers of these ->attach_dv callback functions may care. Propagate them directly without the extra conversions. > > Propagate the real return value to not miss a potential soft failure. > > > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > > Apart from that comment, > > Reviewed-by: Kevin Tian <kevin.tian@intel.com> Added this too. Thanks!
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index be1a7d1cc630..c30dc8f81778 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -666,7 +666,7 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain, ret = mtk_iommu_domain_finalise(dom, frstdata, region_id); if (ret) { mutex_unlock(&dom->mutex); - return -ENODEV; + return ret; } dom->bank = &data->bank[bankid]; } diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c index caca0a638c4d..f1ecc5589626 100644 --- a/drivers/iommu/virtio-iommu.c +++ b/drivers/iommu/virtio-iommu.c @@ -696,7 +696,7 @@ static int viommu_domain_finalise(struct viommu_endpoint *vdev, if (ret) { ida_free(&viommu->domain_ids, vdomain->id); vdomain->viommu = NULL; - return -EOPNOTSUPP; + return ret; } }
Following the new rules in include/linux/iommu.h kdocs, EINVAL now can be used to indicate that domain and device are incompatible by a caller that treats it as a soft failure and tries attaching to another domain. Either mtk_iommu or virtio driver has a place that returns a hard failure instead of the return value from the function call, where an incompatible errno EINVAL could potentially occur. Propagate the real return value to not miss a potential soft failure. Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> --- drivers/iommu/mtk_iommu.c | 2 +- drivers/iommu/virtio-iommu.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)