From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 1/2] VT-d: Support multiple device assignment for KVM Date: Wed, 26 Nov 2008 12:53:14 +0200 Message-ID: <492D2A9A.1010900@redhat.com> References: <715D42877B251141A38726ABF5CABF2C018BEB5D99@pdsmsx503.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "Woodhouse, David" , Jesse Barnes , "Kay, Allen M" , "Yu, Fenghua" , "kvm@vger.kernel.org" , "iommu@lists.linux-foundation.org" To: "Han, Weidong" Return-path: Received: from mx2.redhat.com ([66.187.237.31]:44646 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751113AbYKZKxZ (ORCPT ); Wed, 26 Nov 2008 05:53:25 -0500 In-Reply-To: <715D42877B251141A38726ABF5CABF2C018BEB5D99@pdsmsx503.ccr.corp.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: Han, Weidong wrote: > In order to support multiple device assignment for KVM, this patch does following main changes: > - extend dmar_domain to own multiple devices from different iommus, use a bitmap of iommus to replace iommu pointer in dmar_domain. > - add a flag DOMAIN_FLAG_VIRTUAL_MACHINE to represent KVM VT-d usage. Many functions (e.g. intel_map_single() and intel_unmap_single()) won't be used by KVM VT-d. Let them return directly when this flag is set. > This seems brittle. An API that has some functions shorted out depending on some flag is hard to understand and use. We should either implement the functions, or split the API into a basic version that talks only to one device, and an expanded versions that talks to multiple devices, and is implemented by the using the lower level API. This may require more changes due to the need to share io pagetables. > - "SAGAW" capability may be different across iommus, that's to say the VT-d page table levels may be different among iommus. This patch uses a defaut agaw, and skip top levels of page tables for iommus which have smaller agaw than default. > Neat trick. > void free_dmar_iommu(struct intel_iommu *iommu) > { > struct dmar_domain *domain; > @@ -960,7 +1054,14 @@ void free_dmar_iommu(struct intel_iommu *iommu) > for (; i < cap_ndoms(iommu->cap); ) { > domain = iommu->domains[i]; > clear_bit(i, iommu->domain_ids); > - domain_exit(domain); > + > + if (domain->flags & DOMAIN_FLAG_VIRTUAL_MACHINE) { > + /* domain may be referenced by other iommus */ > + if (domain_in_other_iommus(domain, iommu) == 0) > + domain_exit(domain); > + } > + else > + domain_exit(domain); > Things like this are best expressed using reference counts, which removes the need for the test as well. > + > + /* Skip top levels of page tables for > + * iommu which has less agaw than default. > + */ > + for (agaw = domain->agaw; agaw != iommu->agaw; agaw--) { > + pgd = phys_to_virt(dma_pte_addr(*pgd)); > + if (!dma_pte_present(*pgd)) { > + spin_unlock_irqrestore(&iommu->lock, flags); > + return -ENOMEM; > + } > + } > + } > Need to check that the agaw is sufficient for mapped memory (and when adding a device or mapping more memory, need a similar check). -- error compiling committee.c: too many arguments to function