From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754303AbbHFTTz (ORCPT ); Thu, 6 Aug 2015 15:19:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33071 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752185AbbHFTTx (ORCPT ); Thu, 6 Aug 2015 15:19:53 -0400 Message-ID: <1438888792.4023.118.camel@redhat.com> Subject: Re: [PATCH 01/26] iommu/vt-d: Keep track of per-iommu domain ids From: Alex Williamson To: Joerg Roedel Cc: iommu@lists.linux-foundation.org, David Woodhouse , linux-kernel@vger.kernel.org, jroedel@suse.de Date: Thu, 06 Aug 2015 13:19:52 -0600 In-Reply-To: <1438787937-7340-2-git-send-email-joro@8bytes.org> References: <1438787937-7340-1-git-send-email-joro@8bytes.org> <1438787937-7340-2-git-send-email-joro@8bytes.org> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2015-08-05 at 17:18 +0200, Joerg Roedel wrote: > From: Joerg Roedel > > Instead of searching in the domain array for already > allocated domain ids, keep track of them explicitly. > > Signed-off-by: Joerg Roedel > --- > drivers/iommu/intel-iommu.c | 51 +++++++++++++++++++++++---------------------- > 1 file changed, 26 insertions(+), 25 deletions(-) > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > index 0649b94..c3c5167 100644 > --- a/drivers/iommu/intel-iommu.c > +++ b/drivers/iommu/intel-iommu.c > @@ -378,6 +378,9 @@ struct dmar_domain { > DECLARE_BITMAP(iommu_bmp, DMAR_UNITS_SUPPORTED); > /* bitmap of iommus this domain uses*/ > > + u16 iommu_did[DMAR_UNITS_SUPPORTED]; > + /* Domain ids per IOMMU */ Maybe a spec reference here to justify u16 that the 2.3 vt-d spec only supports up to 16bit domain IDs. > + > struct list_head devices; /* all devices' list */ > struct iova_domain iovad; /* iova's that belong to this domain */ > > @@ -1543,11 +1546,13 @@ static int iommu_init_domains(struct intel_iommu *iommu) > } > > /* > - * if Caching mode is set, then invalid translations are tagged > - * with domainid 0. Hence we need to pre-allocate it. > + * If Caching mode is set, then invalid translations are tagged > + * with domain-id 0, hence we need to pre-allocate it. We also > + * use domain-id 0 as a marker for non-allocated domain-id, so > + * make sure it is not used for a real domain. > */ > - if (cap_caching_mode(iommu->cap)) > - set_bit(0, iommu->domain_ids); > + set_bit(0, iommu->domain_ids); > + > return 0; > } > > @@ -1560,9 +1565,10 @@ static void disable_dmar_iommu(struct intel_iommu *iommu) > for_each_set_bit(i, iommu->domain_ids, cap_ndoms(iommu->cap)) { > /* > * Domain id 0 is reserved for invalid translation > - * if hardware supports caching mode. > + * if hardware supports caching mode and used as > + * a non-allocated marker. > */ > - if (cap_caching_mode(iommu->cap) && i == 0) > + if (i == 0) > continue; > > domain = iommu->domains[i]; > @@ -1624,6 +1630,7 @@ static int __iommu_attach_domain(struct dmar_domain *domain, > if (num < ndomains) { > set_bit(num, iommu->domain_ids); > iommu->domains[num] = domain; > + domain->iommu_did[iommu->seq_id] = num; > } else { > num = -ENOSPC; > } > @@ -1650,12 +1657,10 @@ static int iommu_attach_vm_domain(struct dmar_domain *domain, > struct intel_iommu *iommu) > { > int num; > - unsigned long ndomains; > > - ndomains = cap_ndoms(iommu->cap); > - for_each_set_bit(num, iommu->domain_ids, ndomains) > - if (iommu->domains[num] == domain) > - return num; > + num = domain->iommu_did[iommu->seq_id]; > + if (num) > + return num; > > return __iommu_attach_domain(domain, iommu); > } > @@ -1664,22 +1669,17 @@ static void iommu_detach_domain(struct dmar_domain *domain, > struct intel_iommu *iommu) > { > unsigned long flags; > - int num, ndomains; > + int num; > > spin_lock_irqsave(&iommu->lock, flags); > - if (domain_type_is_vm_or_si(domain)) { > - ndomains = cap_ndoms(iommu->cap); > - for_each_set_bit(num, iommu->domain_ids, ndomains) { > - if (iommu->domains[num] == domain) { > - clear_bit(num, iommu->domain_ids); > - iommu->domains[num] = NULL; > - break; > - } > - } > - } else { > - clear_bit(domain->id, iommu->domain_ids); > - iommu->domains[domain->id] = NULL; > - } > + > + num = domain->iommu_did[iommu->seq_id]; > + > + WARN_ON(num == 0); Return -ENODEV and let the caller decide if that's an error? Then we could safely call this over all all iommus. > + > + clear_bit(num, iommu->domain_ids); > + iommu->domains[num] = NULL; > + > spin_unlock_irqrestore(&iommu->lock, flags); > } > > @@ -1708,6 +1708,7 @@ static int domain_detach_iommu(struct dmar_domain *domain, > if (test_and_clear_bit(iommu->seq_id, domain->iommu_bmp)) { > count = --domain->iommu_count; > domain_update_iommu_cap(domain); > + domain->iommu_did[iommu->seq_id] = 0; > } > spin_unlock_irqrestore(&domain->iommu_lock, flags); >