On 17/01/2019 15:13, Dmitry Osipenko wrote:
16.01.2019 23:50, Navneet Kumar пишет:
* Allocate dma iova cookie for a domain while adding dma iommu
devices.
* Perform a stricter check for domain type parameter.
Commit message should tell what exactly is getting "fixed". Apparently you're trying to support T132 ARM64 here.
Signed-off-by: Navneet Kumar <navneetk@xxxxxxxxxx>
---
drivers/iommu/tegra-smmu.c | 43 +++++++++++++++++++++++++++----------------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 543f7c9..ee4d8a8 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -16,6 +16,7 @@
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/dma-mapping.h>
+#include <linux/dma-iommu.h>
#include <soc/tegra/ahb.h>
#include <soc/tegra/mc.h>
@@ -271,8 +272,10 @@ static bool tegra_smmu_capable(enum iommu_cap cap)
static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
{
struct tegra_smmu_as *as;
+ int ret;
- if (type != IOMMU_DOMAIN_UNMANAGED)
+ if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA &&
+ type != IOMMU_DOMAIN_IDENTITY)
return NULL;
Should be better to write these lines like this for the sake of readability:
if (type != IOMMU_DOMAIN_UNMANAGED &&
type != IOMMU_DOMAIN_DMA &&
type != IOMMU_DOMAIN_IDENTITY)
return NULL;
Furthermore, AFAICS there's no special handling being added for identity
domains - don't pretend to support them by giving back a regular
translation domain, that's just misleading and broken.
as = kzalloc(sizeof(*as), GFP_KERNEL);
@@ -281,26 +284,22 @@ static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
as->attr = SMMU_PD_READABLE | SMMU_PD_WRITABLE | SMMU_PD_NONSECURE;
+ ret = (type == IOMMU_DOMAIN_DMA) ? iommu_get_dma_cookie(&as->domain) :
+ -ENODEV;
This makes to fail allocation of domain that has type other than IOMMU_DOMAIN_DMA.
Should be:
if (type == IOMMU_DOMAIN_DMA) {
err = iommu_get_dma_cookie(&as->domain);
if (err)
goto free_as;
}
+ if (ret)
+ goto free_as;
+
as->pd = alloc_page(GFP_KERNEL | __GFP_DMA | __GFP_ZERO);
- if (!as->pd) {
- kfree(as);
- return NULL;
- }
+ if (!as->pd)
+ goto put_dma_cookie;
as->count = kcalloc(SMMU_NUM_PDE, sizeof(u32), GFP_KERNEL);
- if (!as->count) {
- __free_page(as->pd);
- kfree(as);
- return NULL;
- }
+ if (!as->count)
+ goto free_pd_range;
as->pts = kcalloc(SMMU_NUM_PDE, sizeof(*as->pts), GFP_KERNEL);
- if (!as->pts) {
- kfree(as->count);
- __free_page(as->pd);
- kfree(as);
- return NULL;
- }
+ if (!as->pts)
+ goto free_pts;
/* setup aperture */
as->domain.geometry.aperture_start = 0;
@@ -308,6 +307,18 @@ static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
as->domain.geometry.force_aperture = true;
return &as->domain;
+
+free_pts:
+ kfree(as->pts);
+free_pd_range:
+ __free_page(as->pd);
+put_dma_cookie:
+ if (type == IOMMU_DOMAIN_DMA)
FWIW you don't strictly need that check - since domain->iova_cookie
won't be set for other domain types anyway, iommu_put_dma_cookie() will
simply ignore them.
+ iommu_put_dma_cookie(&as->domain);
+free_as:
+ kfree(as);
+
+ return NULL;
}
static void tegra_smmu_domain_free(struct iommu_domain *domain)
How about not leaking the cookie when you free a DMA ops domain?
Robin.
