Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752121AbcDUIku (ORCPT ); Thu, 21 Apr 2016 04:40:50 -0400 Received: from mail-wm0-f51.google.com ([74.125.82.51]:37572 "EHLO mail-wm0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751845AbcDUIkr (ORCPT ); Thu, 21 Apr 2016 04:40:47 -0400 Subject: Re: [PATCH v7 10/10] iommu/arm-smmu: call iommu_free_reserved_iova_domain on domain destruction To: Robin Murphy , eric.auger@st.com, alex.williamson@redhat.com, will.deacon@arm.com, joro@8bytes.org, tglx@linutronix.de, jason@lakedaemon.net, marc.zyngier@arm.com, christoffer.dall@linaro.org, linux-arm-kernel@lists.infradead.org References: <1461084994-2355-1-git-send-email-eric.auger@linaro.org> <1461084994-2355-11-git-send-email-eric.auger@linaro.org> <5717BDF1.7030400@arm.com> Cc: patches@linaro.org, linux-kernel@vger.kernel.org, Bharat.Bhushan@freescale.com, pranav.sawargaonkar@gmail.com, p.fedin@samsung.com, iommu@lists.linux-foundation.org, Jean-Philippe.Brucker@arm.com, julien.grall@arm.com From: Eric Auger Message-ID: <571891C2.50900@linaro.org> Date: Thu, 21 Apr 2016 10:39:30 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <5717BDF1.7030400@arm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2365 Lines: 79 Hi Robin, On 04/20/2016 07:35 PM, Robin Murphy wrote: > On 19/04/16 17:56, Eric Auger wrote: >> When the domain gets destroyed, let's make sure all reserved iova >> resources get released. >> >> Choice is made to put that call in arm-smmu(-v3).c to do something >> similar >> to what was done for iommu_put_dma_cookie. >> >> Signed-off-by: Eric Auger >> >> --- >> >> v7: new >> --- >> drivers/iommu/arm-smmu-v3.c | 2 ++ >> drivers/iommu/arm-smmu.c | 2 ++ >> 2 files changed, 4 insertions(+) >> >> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c >> index a077a35..afd0dac 100644 >> --- a/drivers/iommu/arm-smmu-v3.c >> +++ b/drivers/iommu/arm-smmu-v3.c >> @@ -22,6 +22,7 @@ >> >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -1444,6 +1445,7 @@ static void arm_smmu_domain_free(struct >> iommu_domain *domain) >> struct arm_smmu_device *smmu = smmu_domain->smmu; >> >> iommu_put_dma_cookie(domain); >> + iommu_free_reserved_iova_domain(domain); > > Yikes! No, drivers shouldn't be randomly freeing things they didn't > allocate - the owner of the domain, who presumably allocated the thing, > can call that right _before_ they call iommu_domain_free(). OK I move that back to vfio_iommu_type1.c. Thanks Eric > >> free_io_pgtable_ops(smmu_domain->pgtbl_ops); >> >> /* Free the CD and ASID, if we allocated them */ >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >> index 8cd7b8a..492339f 100644 >> --- a/drivers/iommu/arm-smmu.c >> +++ b/drivers/iommu/arm-smmu.c >> @@ -30,6 +30,7 @@ >> >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -1009,6 +1010,7 @@ static void arm_smmu_domain_free(struct >> iommu_domain *domain) >> * already been detached. >> */ >> iommu_put_dma_cookie(domain); >> + iommu_free_reserved_iova_domain(domain); > > ...which has the added bonus of preventing needless duplication everywhere. > > Robin. > >> arm_smmu_destroy_domain_context(domain); >> kfree(smmu_domain); >> } >> >