From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754642AbbGPPUd (ORCPT ); Thu, 16 Jul 2015 11:20:33 -0400 Received: from 8bytes.org ([81.169.241.247]:36933 "EHLO theia.8bytes.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752708AbbGPPUb (ORCPT ); Thu, 16 Jul 2015 11:20:31 -0400 Date: Thu, 16 Jul 2015 17:20:30 +0200 From: Joerg Roedel To: Alex Williamson Cc: iommu@lists.linux-foundation.org, dwmw2@infradead.org, jiang.liu@linux.intel.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH] iommu/vt-d: Fix VM domain ID leak Message-ID: <20150716152030.GB10969@8bytes.org> References: <20150714204731.10189.28556.stgit@gimli.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150714204731.10189.28556.stgit@gimli.home> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jul 14, 2015 at 02:48:53PM -0600, Alex Williamson wrote: > This continues the attempt to fix commit fb170fb4c548 ("iommu/vt-d: > Introduce helper functions to make code symmetric for readability"). > The previous attempt in commit 71684406905f ("iommu/vt-d: Detach > domain *only* from attached iommus") overlooked the fact that > dmar_domain.iommu_bmp gets cleared for VM domains when devices are > detached: > > intel_iommu_detach_device > domain_remove_one_dev_info > domain_detach_iommu > > The domain is detached from the iommu, but the iommu is still attached > to the domain, for whatever reason. Gaah, this whole vm_or_si special handling is a complete and fragile mess. The only reason for keeping the domain-id allocated in the iommu seems to be that it can be re-used later, when another device behind this iommu is attached to the domain. But this is hardly a justification for the complexity and special case handling here, so how about this diff instead: diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index a98a7b2..c8fc8c8 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -4618,8 +4618,7 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain, if (found == 0) { domain_detach_iommu(domain, iommu); - if (!domain_type_is_vm_or_si(domain)) - iommu_detach_domain(domain, iommu); + iommu_detach_domain(domain, iommu); } } This removes the caching of domain-ids, and iommu_detach_domain correctly handles all types of domains (dma-api, vm and si). It should be safe, but can you please double-check? Joerg