| Message ID | 20-v6-e8114faedade+425-iommu_all_defdom_jgg@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iommu: Make default_domain's mandatory | expand |
On 2023/8/3 8:08, Jason Gunthorpe wrote: > Prior to commit 1b932ceddd19 ("iommu: Remove detach_dev callbacks") the > sun50i_iommu_detach_device() function was being called by > ops->detach_dev(). > > This is an IDENTITY domain so convert sun50i_iommu_detach_device() into > sun50i_iommu_identity_attach() and a full IDENTITY domain and thus hook it > back up the same was as the old ops->detach_dev(). > > Signed-off-by: Jason Gunthorpe<jgg@nvidia.com> > --- > drivers/iommu/sun50i-iommu.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c > index 74c5cb93e90027..0bf08b120cf105 100644 > --- a/drivers/iommu/sun50i-iommu.c > +++ b/drivers/iommu/sun50i-iommu.c > @@ -757,21 +757,32 @@ static void sun50i_iommu_detach_domain(struct sun50i_iommu *iommu, > iommu->domain = NULL; > } > > -static void sun50i_iommu_detach_device(struct iommu_domain *domain, > - struct device *dev) > +static int sun50i_iommu_identity_attach(struct iommu_domain *identity_domain, > + struct device *dev) > { > - struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain); > struct sun50i_iommu *iommu = dev_iommu_priv_get(dev); > + struct sun50i_iommu_domain *sun50i_domain; > > dev_dbg(dev, "Detaching from IOMMU domain\n"); > > - if (iommu->domain != domain) > - return; > + if (iommu->domain == identity_domain) > + return 0; > > + sun50i_domain = to_sun50i_domain(iommu->domain); > if (refcount_dec_and_test(&sun50i_domain->refcnt)) > sun50i_iommu_detach_domain(iommu, sun50i_domain); > + return 0; > } Does iommu->domain need to be set to identity_domain before returning? Best regards, baolu
On Mon, Aug 14, 2023 at 02:44:50PM +0800, Baolu Lu wrote: > On 2023/8/3 8:08, Jason Gunthorpe wrote: > > Prior to commit 1b932ceddd19 ("iommu: Remove detach_dev callbacks") the > > sun50i_iommu_detach_device() function was being called by > > ops->detach_dev(). > > > > This is an IDENTITY domain so convert sun50i_iommu_detach_device() into > > sun50i_iommu_identity_attach() and a full IDENTITY domain and thus hook it > > back up the same was as the old ops->detach_dev(). > > > > Signed-off-by: Jason Gunthorpe<jgg@nvidia.com> > > --- > > drivers/iommu/sun50i-iommu.c | 26 +++++++++++++++++++------- > > 1 file changed, 19 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c > > index 74c5cb93e90027..0bf08b120cf105 100644 > > --- a/drivers/iommu/sun50i-iommu.c > > +++ b/drivers/iommu/sun50i-iommu.c > > @@ -757,21 +757,32 @@ static void sun50i_iommu_detach_domain(struct sun50i_iommu *iommu, > > iommu->domain = NULL; > > } > > -static void sun50i_iommu_detach_device(struct iommu_domain *domain, > > - struct device *dev) > > +static int sun50i_iommu_identity_attach(struct iommu_domain *identity_domain, > > + struct device *dev) > > { > > - struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain); > > struct sun50i_iommu *iommu = dev_iommu_priv_get(dev); > > + struct sun50i_iommu_domain *sun50i_domain; > > dev_dbg(dev, "Detaching from IOMMU domain\n"); > > - if (iommu->domain != domain) > > - return; > > + if (iommu->domain == identity_domain) > > + return 0; > > + sun50i_domain = to_sun50i_domain(iommu->domain); > > if (refcount_dec_and_test(&sun50i_domain->refcnt)) > > sun50i_iommu_detach_domain(iommu, sun50i_domain); > > + return 0; > > } > > Does iommu->domain need to be set to identity_domain before returning? sun50i_iommu_detach_domain() does it. This driver is confused because it uses generic_single_device_group but also has this weird refcounting stuff. It should just make the first attach alter the HW and have the remaining ones (eg new domain == current domani) be NOPs. It should not refcount. Jason
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c index 74c5cb93e90027..0bf08b120cf105 100644 --- a/drivers/iommu/sun50i-iommu.c +++ b/drivers/iommu/sun50i-iommu.c @@ -757,21 +757,32 @@ static void sun50i_iommu_detach_domain(struct sun50i_iommu *iommu, iommu->domain = NULL; } -static void sun50i_iommu_detach_device(struct iommu_domain *domain, - struct device *dev) +static int sun50i_iommu_identity_attach(struct iommu_domain *identity_domain, + struct device *dev) { - struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain); struct sun50i_iommu *iommu = dev_iommu_priv_get(dev); + struct sun50i_iommu_domain *sun50i_domain; dev_dbg(dev, "Detaching from IOMMU domain\n"); - if (iommu->domain != domain) - return; + if (iommu->domain == identity_domain) + return 0; + sun50i_domain = to_sun50i_domain(iommu->domain); if (refcount_dec_and_test(&sun50i_domain->refcnt)) sun50i_iommu_detach_domain(iommu, sun50i_domain); + return 0; } +static struct iommu_domain_ops sun50i_iommu_identity_ops = { + .attach_dev = sun50i_iommu_identity_attach, +}; + +static struct iommu_domain sun50i_iommu_identity_domain = { + .type = IOMMU_DOMAIN_IDENTITY, + .ops = &sun50i_iommu_identity_ops, +}; + static int sun50i_iommu_attach_device(struct iommu_domain *domain, struct device *dev) { @@ -789,8 +800,7 @@ static int sun50i_iommu_attach_device(struct iommu_domain *domain, if (iommu->domain == domain) return 0; - if (iommu->domain) - sun50i_iommu_detach_device(iommu->domain, dev); + sun50i_iommu_identity_attach(&sun50i_iommu_identity_domain, dev); sun50i_iommu_attach_domain(iommu, sun50i_domain); @@ -827,6 +837,7 @@ static int sun50i_iommu_of_xlate(struct device *dev, } static const struct iommu_ops sun50i_iommu_ops = { + .identity_domain = &sun50i_iommu_identity_domain, .pgsize_bitmap = SZ_4K, .device_group = sun50i_iommu_device_group, .domain_alloc = sun50i_iommu_domain_alloc, @@ -985,6 +996,7 @@ static int sun50i_iommu_probe(struct platform_device *pdev) if (!iommu) return -ENOMEM; spin_lock_init(&iommu->iommu_lock); + iommu->domain = &sun50i_iommu_identity_domain; platform_set_drvdata(pdev, iommu); iommu->dev = &pdev->dev;
Prior to commit 1b932ceddd19 ("iommu: Remove detach_dev callbacks") the sun50i_iommu_detach_device() function was being called by ops->detach_dev(). This is an IDENTITY domain so convert sun50i_iommu_detach_device() into sun50i_iommu_identity_attach() and a full IDENTITY domain and thus hook it back up the same was as the old ops->detach_dev(). Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/iommu/sun50i-iommu.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)