[PATCH 1/2] VT-d: changes to support KVM

[PATCH 1/2] VT-d: changes to support KVM

[Ben-Ami Yassour]

From: Kay, Allen M <allen.m.kay@intel.com>

This patch extends the VT-d driver to support KVM

[Ben: fixed memory pinning]

Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
Signed-off-by: Weidong Han <weidong.han@intel.com>
Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
---
 drivers/pci/dmar.c                           |    4 +-
 drivers/pci/intel-iommu.c                    |  117 +++++++++++++++++++++++++-
 drivers/pci/iova.c                           |    2 +-
 {drivers/pci => include/linux}/intel-iommu.h |   11 +++
 {drivers/pci => include/linux}/iova.h        |    0 
 5 files changed, 127 insertions(+), 7 deletions(-)
 rename {drivers/pci => include/linux}/intel-iommu.h (94%)
 rename {drivers/pci => include/linux}/iova.h (100%)

diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
index 8bf86ae..1df28ea 100644
--- a/drivers/pci/dmar.c
+++ b/drivers/pci/dmar.c
@@ -26,8 +26,8 @@
 
 #include <linux/pci.h>
 #include <linux/dmar.h>
-#include "iova.h"
-#include "intel-iommu.h"
+#include <linux/iova.h>
+#include <linux/intel-iommu.h>
 
 #undef PREFIX
 #define PREFIX "DMAR:"
diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 8d0e60a..1eefc60 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -20,6 +20,7 @@
  * Author: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
  */
 
+#undef DEBUG
 #include <linux/init.h>
 #include <linux/bitmap.h>
 #include <linux/debugfs.h>
@@ -33,8 +34,8 @@
 #include <linux/dma-mapping.h>
 #include <linux/mempool.h>
 #include <linux/timer.h>
-#include "iova.h"
-#include "intel-iommu.h"
+#include <linux/iova.h>
+#include <linux/intel-iommu.h>
 #include <asm/proto.h> /* force_iommu in this header in x86-64*/
 #include <asm/cacheflush.h>
 #include <asm/iommu.h>
@@ -160,7 +161,7 @@ static inline void *alloc_domain_mem(void)
 	return iommu_kmem_cache_alloc(iommu_domain_cache);
 }
 
-static inline void free_domain_mem(void *vaddr)
+static void free_domain_mem(void *vaddr)
 {
 	kmem_cache_free(iommu_domain_cache, vaddr);
 }
@@ -1414,7 +1415,7 @@ static void domain_remove_dev_info(struct dmar_domain *domain)
  * find_domain
  * Note: we use struct pci_dev->dev.archdata.iommu stores the info
  */
-struct dmar_domain *
+static struct dmar_domain *
 find_domain(struct pci_dev *pdev)
 {
 	struct device_domain_info *info;
@@ -2430,3 +2431,111 @@ int __init intel_iommu_init(void)
 	return 0;
 }
 
+void intel_iommu_domain_exit(struct dmar_domain *domain)
+{
+	u64 end;
+
+	/* Domain 0 is reserved, so dont process it */
+	if (!domain)
+		return;
+
+	end = DOMAIN_MAX_ADDR(domain->gaw);
+	end = end & (~PAGE_MASK_4K);
+
+	/* clear ptes */
+	dma_pte_clear_range(domain, 0, end);
+
+	/* free page tables */
+	dma_pte_free_pagetable(domain, 0, end);
+
+	iommu_free_domain(domain);
+	free_domain_mem(domain);
+}
+EXPORT_SYMBOL_GPL(intel_iommu_domain_exit);
+
+struct dmar_domain *intel_iommu_domain_alloc(struct pci_dev *pdev)
+{
+	struct dmar_drhd_unit *drhd;
+	struct dmar_domain *domain;
+	struct intel_iommu *iommu;
+
+	drhd = dmar_find_matched_drhd_unit(pdev);
+	if (!drhd) {
+		printk(KERN_ERR "intel_iommu_domain_alloc: drhd == NULL\n");
+		return NULL;
+	}
+
+	iommu = drhd->iommu;
+	if (!iommu) {
+		printk(KERN_ERR
+			"intel_iommu_domain_alloc: iommu == NULL\n");
+		return NULL;
+	}
+	domain = iommu_alloc_domain(iommu);
+	if (!domain) {
+		printk(KERN_ERR
+			"intel_iommu_domain_alloc: domain == NULL\n");
+		return NULL;
+	}
+	if (domain_init(domain, DEFAULT_DOMAIN_ADDRESS_WIDTH)) {
+		printk(KERN_ERR
+			"intel_iommu_domain_alloc: domain_init() failed\n");
+		intel_iommu_domain_exit(domain);
+		return NULL;
+	}
+	return domain;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_domain_alloc);
+
+int intel_iommu_context_mapping(
+	struct dmar_domain *domain, struct pci_dev *pdev)
+{
+	int rc;
+	rc = domain_context_mapping(domain, pdev);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_context_mapping);
+
+int intel_iommu_page_mapping(
+	struct dmar_domain *domain, dma_addr_t iova,
+	u64 hpa, size_t size, int prot)
+{
+	int rc;
+	rc = domain_page_mapping(domain, iova, hpa, size, prot);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_page_mapping);
+
+void intel_iommu_detach_dev(struct dmar_domain *domain, u8 bus, u8 devfn)
+{
+	detach_domain_for_dev(domain, bus, devfn);
+}
+EXPORT_SYMBOL_GPL(intel_iommu_detach_dev);
+
+struct dmar_domain *
+intel_iommu_find_domain(struct pci_dev *pdev)
+{
+	return find_domain(pdev);
+}
+EXPORT_SYMBOL_GPL(intel_iommu_find_domain);
+
+int intel_iommu_found(void)
+{
+	return g_num_of_iommus;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_found);
+
+u64 intel_iommu_iova_to_pfn(struct dmar_domain *domain, u64 iova)
+{
+	struct dma_pte *pte;
+	u64 pfn;
+
+	pfn = 0;
+	pte = addr_to_dma_pte(domain, iova);
+
+	if (pte)
+		pfn = dma_pte_addr(*pte);
+
+	return pfn >> PAGE_SHIFT_4K;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_iova_to_pfn);
diff --git a/drivers/pci/iova.c b/drivers/pci/iova.c
index 3ef4ac0..2287116 100644
--- a/drivers/pci/iova.c
+++ b/drivers/pci/iova.c
@@ -7,7 +7,7 @@
  * Author: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
  */
 
-#include "iova.h"
+#include <linux/iova.h>
 
 void
 init_iova_domain(struct iova_domain *iovad, unsigned long pfn_32bit)
diff --git a/drivers/pci/intel-iommu.h b/include/linux/intel-iommu.h
similarity index 94%
rename from drivers/pci/intel-iommu.h
rename to include/linux/intel-iommu.h
index afc0ad9..1490fc0 100644
--- a/drivers/pci/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -341,4 +341,15 @@ static inline void iommu_prepare_gfx_mapping(void)
 }
 #endif /* !CONFIG_DMAR_GFX_WA */
 
+void intel_iommu_domain_exit(struct dmar_domain *domain);
+struct dmar_domain *intel_iommu_domain_alloc(struct pci_dev *pdev);
+int intel_iommu_context_mapping(struct dmar_domain *domain,
+				struct pci_dev *pdev);
+int intel_iommu_page_mapping(struct dmar_domain *domain, dma_addr_t iova,
+			     u64 hpa, size_t size, int prot);
+void intel_iommu_detach_dev(struct dmar_domain *domain, u8 bus, u8 devfn);
+struct dmar_domain *intel_iommu_find_domain(struct pci_dev *pdev);
+int intel_iommu_found(void);
+u64 intel_iommu_iova_to_pfn(struct dmar_domain *domain, u64 iova);
+
 #endif
diff --git a/drivers/pci/iova.h b/include/linux/iova.h
similarity index 100%
rename from drivers/pci/iova.h
rename to include/linux/iova.h
-- 
1.5.6

[PATCH 2/2] KVM: Device assignemnt with VT-d

[Ben-Ami Yassour]

Based on a patch by: Kay, Allen M <allen.m.kay@intel.com>

This patch enables pci device assignment based on VT-d support.
When a device is assigned to the guest, the guest memory is pinned and
the mapping is updated in the VT-d IOMMU.

Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
Signed-off-by: Weidong Han <weidong.han@intel.com>
Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
---
 arch/x86/kvm/Makefile      |    3 +
 arch/x86/kvm/vtd.c         |  203 ++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c         |   10 ++
 include/asm-x86/kvm_host.h |    3 +
 include/linux/kvm_host.h   |   32 +++++++
 virt/kvm/kvm_main.c        |    9 ++-
 6 files changed, 259 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/kvm/vtd.c

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index d0e940b..3072b17 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -12,6 +12,9 @@ EXTRA_CFLAGS += -Ivirt/kvm -Iarch/x86/kvm
 
 kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o \
 	i8254.o
+ifeq ($(CONFIG_DMAR),y)
+kvm-objs += vtd.o
+endif
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
diff --git a/arch/x86/kvm/vtd.c b/arch/x86/kvm/vtd.c
new file mode 100644
index 0000000..4336769
--- /dev/null
+++ b/arch/x86/kvm/vtd.c
@@ -0,0 +1,203 @@
+/*
+ * Copyright (c) 2006, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * Copyright (C) 2006-2008 Intel Corporation
+ * Copyright IBM Corporation, 2008
+ * Author: Allen M. Kay <allen.m.kay@intel.com>
+ * Author: Weidong Han <weidong.han@intel.com>
+ * Author: Ben-Ami Yassour <benami@il.ibm.com>
+ */
+
+#include <linux/list.h>
+#include <linux/kvm_host.h>
+#include <linux/pci.h>
+#include <linux/dmar.h>
+#include <linux/intel-iommu.h>
+
+static int kvm_iommu_unmap_memslots(struct kvm *kvm);
+
+int kvm_iommu_map_pages(struct kvm *kvm,
+			  gfn_t base_gfn, unsigned long npages)
+{
+	gfn_t gfn = base_gfn;
+	pfn_t pfn;
+	int i, rc;
+	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+
+	/* check if iommu exists and in use */
+	if (!domain)
+		return 0;
+
+	for (i = 0; i < npages; i++) {
+		/* check if already mapped */
+		pfn = (pfn_t)intel_iommu_iova_to_pfn(domain,
+						     gfn_to_gpa(gfn));
+		if (pfn && !is_mmio_pfn(pfn))
+			continue;
+
+		pfn = gfn_to_pfn(kvm, gfn);
+		if (!is_mmio_pfn(pfn)) {
+			rc = intel_iommu_page_mapping(domain,
+						      gfn_to_gpa(gfn),
+						      pfn_to_hpa(pfn),
+						      PAGE_SIZE,
+						      DMA_PTE_READ |
+						      DMA_PTE_WRITE);
+			if (rc) {
+				kvm_release_pfn_clean(pfn);
+				printk(KERN_DEBUG "kvm_iommu_map_pages:"
+				       "iommu failed to map pfn=%lx\n", pfn);
+				return rc;
+			}
+		} else {
+			printk(KERN_DEBUG "kvm_iommu_map_page:"
+			       "invalid pfn=%lx\n", pfn);
+			return 0;
+		}
+
+		gfn++;
+	}
+	return 0;
+}
+
+static int kvm_iommu_map_memslots(struct kvm *kvm)
+{
+	int i, rc;
+
+	down_read(&kvm->slots_lock);
+	for (i = 0; i < kvm->nmemslots; i++) {
+		rc = kvm_iommu_map_pages(kvm, kvm->memslots[i].base_gfn,
+					 kvm->memslots[i].npages);
+		if (rc) {
+			up_read(&kvm->slots_lock);
+			return rc;
+		}
+	}
+	up_read(&kvm->slots_lock);
+	return 0;
+}
+
+int kvm_iommu_map_guest(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *assigned_dev)
+{
+	struct pci_dev *pdev = NULL;
+	int rc;
+
+	if (!intel_iommu_found()) {
+		printk(KERN_ERR "intel iommu not found\n");
+		return -ENODEV;
+	}
+
+	printk(KERN_DEBUG "VT-d direct map: host bdf = %x:%x:%x\n",
+	       assigned_dev->host_busnr,
+	       PCI_SLOT(assigned_dev->host_devfn),
+	       PCI_FUNC(assigned_dev->host_devfn));
+
+	pdev = assigned_dev->dev;
+
+	if (pdev == NULL) {
+		if (kvm->arch.intel_iommu_domain) {
+			intel_iommu_domain_exit(kvm->arch.intel_iommu_domain);
+			kvm->arch.intel_iommu_domain = NULL;
+		}
+		return -ENODEV;
+	}
+
+	kvm->arch.intel_iommu_domain = intel_iommu_domain_alloc(pdev);
+
+	rc = kvm_iommu_map_memslots(kvm);
+	if (rc)
+		goto out_unmap;
+
+	intel_iommu_detach_dev(kvm->arch.intel_iommu_domain,
+			       pdev->bus->number, pdev->devfn);
+
+	rc = intel_iommu_context_mapping(kvm->arch.intel_iommu_domain,
+					 pdev);
+	if (rc) {
+		printk(KERN_ERR "Domain context map for %s failed",
+		       pci_name(pdev));
+		goto out_unmap;
+	}
+	return 0;
+
+out_unmap:
+	kvm_iommu_unmap_memslots(kvm);
+	return rc;
+}
+
+static void kvm_iommu_put_pages(struct kvm *kvm,
+			       gfn_t base_gfn, unsigned long npages)
+{
+	gfn_t gfn = base_gfn;
+	pfn_t pfn;
+	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+	int i;
+
+	for (i = 0; i < npages; i++) {
+		pfn = (pfn_t)intel_iommu_iova_to_pfn(domain,
+						     gfn_to_gpa(gfn));
+		kvm_release_pfn_clean(pfn);
+		gfn++;
+	}
+}
+
+static int kvm_iommu_unmap_memslots(struct kvm *kvm)
+{
+	int i;
+	down_read(&kvm->slots_lock);
+	for (i = 0; i < kvm->nmemslots; i++) {
+		kvm_iommu_put_pages(kvm, kvm->memslots[i].base_gfn,
+				    kvm->memslots[i].npages);
+	}
+	up_read(&kvm->slots_lock);
+
+	return 0;
+}
+
+int kvm_iommu_unmap_guest(struct kvm *kvm)
+{
+	struct kvm_assigned_dev_kernel *entry;
+	struct pci_dev *pdev = NULL;
+	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+
+	/* check if iommu exists and in use */
+	if (!domain)
+		return 0;
+
+	list_for_each_entry(entry, &kvm->arch.assigned_dev_head, list) {
+		printk(KERN_DEBUG "VT-d unmap: host bdf = %x:%x:%x\n",
+		       entry->host_busnr,
+		       PCI_SLOT(entry->host_devfn),
+		       PCI_FUNC(entry->host_devfn));
+
+		for_each_pci_dev(pdev) {
+			if ((pdev->bus->number == entry->host_busnr) &&
+			    (pdev->devfn == entry->host_devfn))
+				break;
+		}
+
+		if (pdev == NULL)
+			return -ENODEV;
+
+		/* detach kvm dmar domain */
+		intel_iommu_detach_dev(domain,
+				       pdev->bus->number, pdev->devfn);
+	}
+	kvm_iommu_unmap_memslots(kvm);
+	intel_iommu_domain_exit(domain);
+	return 0;
+}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a97157c..5cfc21a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -35,6 +35,7 @@
 #include <linux/module.h>
 #include <linux/mman.h>
 #include <linux/highmem.h>
+#include <linux/intel-iommu.h>
 
 #include <asm/uaccess.h>
 #include <asm/msr.h>
@@ -271,9 +272,17 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
 
 	list_add(&match->list, &kvm->arch.assigned_dev_head);
 
+	/* currenlty iommu is required for handling dma */
+	r = kvm_iommu_map_guest(kvm, match);
+	if (r)
+		goto out_list_del;
+
 out:
 	mutex_unlock(&kvm->lock);
 	return r;
+out_list_del:
+	list_del(&match->list);
+	pci_release_regions(dev);
 out_disable:
 	pci_disable_device(dev);
 out_put:
@@ -4219,6 +4228,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
 
 void kvm_arch_destroy_vm(struct kvm *kvm)
 {
+	kvm_iommu_unmap_guest(kvm);
 	kvm_free_assigned_devices(kvm);
 	kvm_free_pit(kvm);
 	kfree(kvm->arch.vpic);
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index ef019b5..b141949 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -354,6 +354,7 @@ struct kvm_arch{
 	 */
 	struct list_head active_mmu_pages;
 	struct list_head assigned_dev_head;
+	struct dmar_domain *intel_iommu_domain;
 	struct kvm_pic *vpic;
 	struct kvm_ioapic *vioapic;
 	struct kvm_pit *vpit;
@@ -503,6 +504,8 @@ int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
 int kvm_pv_mmu_op(struct kvm_vcpu *vcpu, unsigned long bytes,
 		  gpa_t addr, unsigned long *ret);
 
+int is_mmio_pfn(pfn_t pfn);
+
 extern bool tdp_enabled;
 
 enum emulation_result {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index a18aaad..b703890 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -285,6 +285,33 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v);
 int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu);
 void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
 
+#ifdef CONFIG_DMAR
+int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
+			unsigned long npages);
+int kvm_iommu_map_guest(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *assigned_dev);
+int kvm_iommu_unmap_guest(struct kvm *kvm);
+#else /* CONFIG_DMAR */
+static inline int kvm_iommu_map_pages(struct kvm *kvm,
+				      gfn_t base_gfn,
+				      unsigned long npages)
+{
+	return 0;
+}
+
+static inline int kvm_iommu_map_guest(struct kvm *kvm,
+				      struct kvm_assigned_dev_kernel
+				      *assigned_dev)
+{
+	return -ENODEV;
+}
+
+static inline int kvm_iommu_unmap_guest(struct kvm *kvm)
+{
+	return 0;
+}
+#endif /* CONFIG_DMAR */
+
 static inline void kvm_guest_enter(void)
 {
 	account_system_vtime(current);
@@ -307,6 +334,11 @@ static inline gpa_t gfn_to_gpa(gfn_t gfn)
 	return (gpa_t)gfn << PAGE_SHIFT;
 }
 
+static inline hpa_t pfn_to_hpa(pfn_t pfn)
+{
+	return (hpa_t)pfn << PAGE_SHIFT;
+}
+
 static inline void kvm_migrate_timers(struct kvm_vcpu *vcpu)
 {
 	set_bit(KVM_REQ_MIGRATE_TIMER, &vcpu->requests);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 5eb96c7..ffd9069 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -41,6 +41,7 @@
 #include <linux/pagemap.h>
 #include <linux/mman.h>
 #include <linux/swap.h>
+#include <linux/intel-iommu.h>
 
 #include <asm/processor.h>
 #include <asm/io.h>
@@ -76,7 +77,7 @@ static inline int valid_vcpu(int n)
 	return likely(n >= 0 && n < KVM_MAX_VCPUS);
 }
 
-static inline int is_mmio_pfn(pfn_t pfn)
+inline int is_mmio_pfn(pfn_t pfn)
 {
 	if (pfn_valid(pfn))
 		return PageReserved(pfn_to_page(pfn));
@@ -578,6 +579,12 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	}
 
 	kvm_free_physmem_slot(&old, &new);
+
+	/* map the pages in iommu page table */
+	r = kvm_iommu_map_pages(kvm, base_gfn, npages);
+	if (r)
+		goto out_free;
+
 	return 0;
 
 out_free:
-- 
1.5.6

Patch-set description

[Ben-Ami Yassour]

This patch-set contains the two kernel patches for device assignment
with VT-d.

The main changes with respect to the previous version are based on 
comments by Yang Sheng:
1. Fix compilation if CONFIG_DMAR is not defined.
2. Don't map iommu if a device is not assigned, and synchronize the
mapping of a device with the initial mapping of all the slots.

Tested  on top of the current KVM tree (which includes mmu notifiers).

Comments are appreciated.

Thanks,
Ben

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Yang, Sheng]

On Thursday 07 August 2008 22:14:47 Ben-Ami Yassour wrote:
> Based on a patch by: Kay, Allen M <allen.m.kay@intel.com>
>
> This patch enables pci device assignment based on VT-d support.
> When a device is assigned to the guest, the guest memory is pinned
> and the mapping is updated in the VT-d IOMMU.
>

I am afraid there still some compatible problem...

> Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> Signed-off-by: Weidong Han <weidong.han@intel.com>
> Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
> ---
>  arch/x86/kvm/Makefile      |    3 +
>  arch/x86/kvm/vtd.c         |  203
[snip]
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index a97157c..5cfc21a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -35,6 +35,7 @@
>  #include <linux/module.h>
>  #include <linux/mman.h>
>  #include <linux/highmem.h>
> +#include <linux/intel-iommu.h>

This broken external kernel modules before 2.6.27... If we wrapped it 
with CONFIG_DMAR, it would also broken the commit before the patch 
checked in and after DMAR enabled in kernel... Need a version number 
judgement?

> diff --git a/include/asm-x86/kvm_host.h
> b/include/asm-x86/kvm_host.h index ef019b5..b141949 100644
> --- a/include/asm-x86/kvm_host.h
> +++ b/include/asm-x86/kvm_host.h
> @@ -354,6 +354,7 @@ struct kvm_arch{
>  	 */
>  	struct list_head active_mmu_pages;
>  	struct list_head assigned_dev_head;
> +	struct dmar_domain *intel_iommu_domain;

Need wrapped by CONFIG_DMAR?

-- 
regards
Yang, Sheng

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Avi Kivity]

Yang, Sheng wrote:
> On Thursday 07 August 2008 22:14:47 Ben-Ami Yassour wrote:
>   
>> Based on a patch by: Kay, Allen M <allen.m.kay@intel.com>
>>
>> This patch enables pci device assignment based on VT-d support.
>> When a device is assigned to the guest, the guest memory is pinned
>> and the mapping is updated in the VT-d IOMMU.
>>
>>     
>
> I am afraid there still some compatible problem...
>
>   
>> Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
>> Signed-off-by: Weidong Han <weidong.han@intel.com>
>> Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
>> ---
>>  arch/x86/kvm/Makefile      |    3 +
>>  arch/x86/kvm/vtd.c         |  203
>>     
> [snip]
>   
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index a97157c..5cfc21a 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -35,6 +35,7 @@
>>  #include <linux/module.h>
>>  #include <linux/mman.h>
>>  #include <linux/highmem.h>
>> +#include <linux/intel-iommu.h>
>>     
>
> This broken external kernel modules before 2.6.27... If we wrapped it 
> with CONFIG_DMAR, it would also broken the commit before the patch 
> checked in and after DMAR enabled in kernel... Need a version number 
> judgement?
>
>   

kernel patches should not consider external module issues.  That keeps 
the code clean (at the expense of making the external module's 
maintainer's life mode difficult, but that's their problem).

>> diff --git a/include/asm-x86/kvm_host.h
>> b/include/asm-x86/kvm_host.h index ef019b5..b141949 100644
>> --- a/include/asm-x86/kvm_host.h
>> +++ b/include/asm-x86/kvm_host.h
>> @@ -354,6 +354,7 @@ struct kvm_arch{
>>  	 */
>>  	struct list_head active_mmu_pages;
>>  	struct list_head assigned_dev_head;
>> +	struct dmar_domain *intel_iommu_domain;
>>     
>
> Need wrapped by CONFIG_DMAR?
>
>   

I guess we can keep this, one pointer is not that expensive.  But we 
should make sure all the iommu functions are available when iommu is 
unconfigured.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Yang, Sheng]

On Wednesday 13 August 2008 17:46:03 Avi Kivity wrote:
> Yang, Sheng wrote:
> > On Thursday 07 August 2008 22:14:47 Ben-Ami Yassour wrote:
> >> Based on a patch by: Kay, Allen M <allen.m.kay@intel.com>
> >>
> >> This patch enables pci device assignment based on VT-d support.
> >> When a device is assigned to the guest, the guest memory is
> >> pinned and the mapping is updated in the VT-d IOMMU.
> >
> > I am afraid there still some compatible problem...
> >
> >> Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> >> Signed-off-by: Weidong Han <weidong.han@intel.com>
> >> Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
> >> ---
> >>  arch/x86/kvm/Makefile      |    3 +
> >>  arch/x86/kvm/vtd.c         |  203
> >
> > [snip]
> >
> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> index a97157c..5cfc21a 100644
> >> --- a/arch/x86/kvm/x86.c
> >> +++ b/arch/x86/kvm/x86.c
> >> @@ -35,6 +35,7 @@
> >>  #include <linux/module.h>
> >>  #include <linux/mman.h>
> >>  #include <linux/highmem.h>
> >> +#include <linux/intel-iommu.h>
> >
> > This broken external kernel modules before 2.6.27... If we
> > wrapped it with CONFIG_DMAR, it would also broken the commit
> > before the patch checked in and after DMAR enabled in kernel...
> > Need a version number judgement?
>
> kernel patches should not consider external module issues.  That
> keeps the code clean (at the expense of making the external
> module's maintainer's life mode difficult, but that's their
> problem).

Yeah, thanks for point it out. It's indeed complicate to consider this 
kind of issues... :)

And I think now our aptitude towards external modules is not 
encouraging? For after this patch, we can discard 
external-modules-compat.h as well. :)

-- 
regards
Yang, Sheng

> >> diff --git a/include/asm-x86/kvm_host.h
> >> b/include/asm-x86/kvm_host.h index ef019b5..b141949 100644
> >> --- a/include/asm-x86/kvm_host.h
> >> +++ b/include/asm-x86/kvm_host.h
> >> @@ -354,6 +354,7 @@ struct kvm_arch{
> >>  	 */
> >>  	struct list_head active_mmu_pages;
> >>  	struct list_head assigned_dev_head;
> >> +	struct dmar_domain *intel_iommu_domain;
> >
> > Need wrapped by CONFIG_DMAR?
>
> I guess we can keep this, one pointer is not that expensive.  But
> we should make sure all the iommu functions are available when
> iommu is unconfigured.

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Amit Shah]

* On Thursday 07 Aug 2008 19:44:47 Ben-Ami Yassour wrote:
> Based on a patch by: Kay, Allen M <allen.m.kay@intel.com>
>
> This patch enables pci device assignment based on VT-d support.
> When a device is assigned to the guest, the guest memory is pinned and
> the mapping is updated in the VT-d IOMMU.
>
> Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> Signed-off-by: Weidong Han <weidong.han@intel.com>
> Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
> ---
>  arch/x86/kvm/Makefile      |    3 +
>  arch/x86/kvm/vtd.c         |  203
> ++++++++++++++++++++++++++++++++++++++++++++ arch/x86/kvm/x86.c         |  
> 10 ++
>  include/asm-x86/kvm_host.h |    3 +
>  include/linux/kvm_host.h   |   32 +++++++
>  virt/kvm/kvm_main.c        |    9 ++-
>  6 files changed, 259 insertions(+), 1 deletions(-)
>  create mode 100644 arch/x86/kvm/vtd.c

> +int kvm_iommu_map_guest(struct kvm *kvm,
> +			struct kvm_assigned_dev_kernel *assigned_dev)
> +{
> +	struct pci_dev *pdev = NULL;
> +	int rc;
> +
> +	if (!intel_iommu_found()) {
> +		printk(KERN_ERR "intel iommu not found\n");
> +		return -ENODEV;
> +	}
> +
> +	printk(KERN_DEBUG "VT-d direct map: host bdf = %x:%x:%x\n",
> +	       assigned_dev->host_busnr,
> +	       PCI_SLOT(assigned_dev->host_devfn),
> +	       PCI_FUNC(assigned_dev->host_devfn));
> +
> +	pdev = assigned_dev->dev;
> +
> +	if (pdev == NULL) {
> +		if (kvm->arch.intel_iommu_domain) {
> +			intel_iommu_domain_exit(kvm->arch.intel_iommu_domain);
> +			kvm->arch.intel_iommu_domain = NULL;
> +		}
> +		return -ENODEV;
> +	}
> +
> +	kvm->arch.intel_iommu_domain = intel_iommu_domain_alloc(pdev);

> +int kvm_iommu_unmap_guest(struct kvm *kvm)
> +{
> +	struct kvm_assigned_dev_kernel *entry;
> +	struct pci_dev *pdev = NULL;
> +	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
> +
> +	/* check if iommu exists and in use */
> +	if (!domain)
> +		return 0;
> +
> +	list_for_each_entry(entry, &kvm->arch.assigned_dev_head, list) {
> +		printk(KERN_DEBUG "VT-d unmap: host bdf = %x:%x:%x\n",
> +		       entry->host_busnr,
> +		       PCI_SLOT(entry->host_devfn),
> +		       PCI_FUNC(entry->host_devfn));
> +
> +		for_each_pci_dev(pdev) {
> +			if ((pdev->bus->number == entry->host_busnr) &&
> +			    (pdev->devfn == entry->host_devfn))
> +				break;
> +		}
> +
> +		if (pdev == NULL)
> +			return -ENODEV;
> +
> +		/* detach kvm dmar domain */
> +		intel_iommu_detach_dev(domain,
> +				       pdev->bus->number, pdev->devfn);
> +	}
> +	kvm_iommu_unmap_memslots(kvm);
> +	intel_iommu_domain_exit(domain);
> +	return 0;
> +}
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index a97157c..5cfc21a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -35,6 +35,7 @@
>  #include <linux/module.h>
>  #include <linux/mman.h>
>  #include <linux/highmem.h>
> +#include <linux/intel-iommu.h>
>
>  #include <asm/uaccess.h>
>  #include <asm/msr.h>
> @@ -271,9 +272,17 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
>
>  	list_add(&match->list, &kvm->arch.assigned_dev_head);
>
> +	/* currenlty iommu is required for handling dma */
> +	r = kvm_iommu_map_guest(kvm, match);
> +	if (r)
> +		goto out_list_del;
> +

This will break pvdma support. Please check if intel-iommu is found and only 
then bail out of here.

Even though pvdma is out-of-tree, it doesn't make sense to restrict our usage 
here. AMD IOMMU support will need this to be handled in the right way as 
well.

>  out:
>  	mutex_unlock(&kvm->lock);
>  	return r;
> +out_list_del:
> +	list_del(&match->list);
> +	pci_release_regions(dev);
>  out_disable:
>  	pci_disable_device(dev);
>  out_put:
> @@ -4219,6 +4228,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
>
>  void kvm_arch_destroy_vm(struct kvm *kvm)
>  {
> +	kvm_iommu_unmap_guest(kvm);

Likewise, check if intel-iommu is found as a first step in this function.

As part of getting AMD-iommu and pvdma support in, we should have generic 
function names, but that can be done later.

> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index a18aaad..b703890 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -285,6 +285,33 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v);
>  int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu);
>  void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
>
> +#ifdef CONFIG_DMAR
> +int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
> +			unsigned long npages);
> +int kvm_iommu_map_guest(struct kvm *kvm,
> +			struct kvm_assigned_dev_kernel *assigned_dev);
> +int kvm_iommu_unmap_guest(struct kvm *kvm);
> +#else /* CONFIG_DMAR */
> +static inline int kvm_iommu_map_pages(struct kvm *kvm,
> +				      gfn_t base_gfn,
> +				      unsigned long npages)
> +{
> +	return 0;
> +}
> +
> +static inline int kvm_iommu_map_guest(struct kvm *kvm,
> +				      struct kvm_assigned_dev_kernel
> +				      *assigned_dev)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int kvm_iommu_unmap_guest(struct kvm *kvm)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_DMAR */

This means you can't use pvdma with iommu. Make these functions that check if 
iommu support for device assignment is enabled. A command-line parameter to 
qemu stating you want to enable vt-d for device assignment would be 
appropriate.

For example, we could have the device assignment parameter changed to look 
like this:

-pcidevice dev=00:13.0,dma=vtd,dma=pvdma

which will mean we should use vtd in conjunction with pvdma for this assigned 
device.

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Ben-Ami Yassour]

On Thu, 2008-08-21 at 12:13 +0530, Amit Shah wrote:
> * On Thursday 07 Aug 2008 19:44:47 Ben-Ami Yassour wrote:
> > Based on a patch by: Kay, Allen M <allen.m.kay@intel.com>
> >
> > This patch enables pci device assignment based on VT-d support.
> > When a device is assigned to the guest, the guest memory is pinned and
> > the mapping is updated in the VT-d IOMMU.
> >
> > Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> > Signed-off-by: Weidong Han <weidong.han@intel.com>
> > Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
> > ---
> >  arch/x86/kvm/Makefile      |    3 +
> >  arch/x86/kvm/vtd.c         |  203
> > ++++++++++++++++++++++++++++++++++++++++++++ arch/x86/kvm/x86.c         |  
> > 10 ++
> >  include/asm-x86/kvm_host.h |    3 +
> >  include/linux/kvm_host.h   |   32 +++++++
> >  virt/kvm/kvm_main.c        |    9 ++-
> >  6 files changed, 259 insertions(+), 1 deletions(-)
> >  create mode 100644 arch/x86/kvm/vtd.c
> 
> > +int kvm_iommu_map_guest(struct kvm *kvm,
> > +			struct kvm_assigned_dev_kernel *assigned_dev)
> > +{
> > +	struct pci_dev *pdev = NULL;
> > +	int rc;
> > +
> > +	if (!intel_iommu_found()) {
> > +		printk(KERN_ERR "intel iommu not found\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	printk(KERN_DEBUG "VT-d direct map: host bdf = %x:%x:%x\n",
> > +	       assigned_dev->host_busnr,
> > +	       PCI_SLOT(assigned_dev->host_devfn),
> > +	       PCI_FUNC(assigned_dev->host_devfn));
> > +
> > +	pdev = assigned_dev->dev;
> > +
> > +	if (pdev == NULL) {
> > +		if (kvm->arch.intel_iommu_domain) {
> > +			intel_iommu_domain_exit(kvm->arch.intel_iommu_domain);
> > +			kvm->arch.intel_iommu_domain = NULL;
> > +		}
> > +		return -ENODEV;
> > +	}
> > +
> > +	kvm->arch.intel_iommu_domain = intel_iommu_domain_alloc(pdev);
> 
> > +int kvm_iommu_unmap_guest(struct kvm *kvm)
> > +{
> > +	struct kvm_assigned_dev_kernel *entry;
> > +	struct pci_dev *pdev = NULL;
> > +	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
> > +
> > +	/* check if iommu exists and in use */
> > +	if (!domain)
> > +		return 0;
> > +
> > +	list_for_each_entry(entry, &kvm->arch.assigned_dev_head, list) {
> > +		printk(KERN_DEBUG "VT-d unmap: host bdf = %x:%x:%x\n",
> > +		       entry->host_busnr,
> > +		       PCI_SLOT(entry->host_devfn),
> > +		       PCI_FUNC(entry->host_devfn));
> > +
> > +		for_each_pci_dev(pdev) {
> > +			if ((pdev->bus->number == entry->host_busnr) &&
> > +			    (pdev->devfn == entry->host_devfn))
> > +				break;
> > +		}
> > +
> > +		if (pdev == NULL)
> > +			return -ENODEV;
> > +
> > +		/* detach kvm dmar domain */
> > +		intel_iommu_detach_dev(domain,
> > +				       pdev->bus->number, pdev->devfn);
> > +	}
> > +	kvm_iommu_unmap_memslots(kvm);
> > +	intel_iommu_domain_exit(domain);
> > +	return 0;
> > +}
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index a97157c..5cfc21a 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -35,6 +35,7 @@
> >  #include <linux/module.h>
> >  #include <linux/mman.h>
> >  #include <linux/highmem.h>
> > +#include <linux/intel-iommu.h>
> >
> >  #include <asm/uaccess.h>
> >  #include <asm/msr.h>
> > @@ -271,9 +272,17 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
> >
> >  	list_add(&match->list, &kvm->arch.assigned_dev_head);
> >
> > +	/* currenlty iommu is required for handling dma */
> > +	r = kvm_iommu_map_guest(kvm, match);
> > +	if (r)
> > +		goto out_list_del;
> > +
> 
> This will break pvdma support. Please check if intel-iommu is found and only 
> then bail out of here.
> 
> Even though pvdma is out-of-tree, it doesn't make sense to restrict our usage 
> here. AMD IOMMU support will need this to be handled in the right way as 
> well.
> 
> >  out:
> >  	mutex_unlock(&kvm->lock);
> >  	return r;
> > +out_list_del:
> > +	list_del(&match->list);
> > +	pci_release_regions(dev);
> >  out_disable:
> >  	pci_disable_device(dev);
> >  out_put:
> > @@ -4219,6 +4228,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
> >
> >  void kvm_arch_destroy_vm(struct kvm *kvm)
> >  {
> > +	kvm_iommu_unmap_guest(kvm);
> 
> Likewise, check if intel-iommu is found as a first step in this function.
> 
> As part of getting AMD-iommu and pvdma support in, we should have generic 
> function names, but that can be done later.
> 
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index a18aaad..b703890 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -285,6 +285,33 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v);
> >  int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu);
> >  void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
> >
> > +#ifdef CONFIG_DMAR
> > +int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
> > +			unsigned long npages);
> > +int kvm_iommu_map_guest(struct kvm *kvm,
> > +			struct kvm_assigned_dev_kernel *assigned_dev);
> > +int kvm_iommu_unmap_guest(struct kvm *kvm);
> > +#else /* CONFIG_DMAR */
> > +static inline int kvm_iommu_map_pages(struct kvm *kvm,
> > +				      gfn_t base_gfn,
> > +				      unsigned long npages)
> > +{
> > +	return 0;
> > +}
> > +
> > +static inline int kvm_iommu_map_guest(struct kvm *kvm,
> > +				      struct kvm_assigned_dev_kernel
> > +				      *assigned_dev)
> > +{
> > +	return -ENODEV;
> > +}
> > +
> > +static inline int kvm_iommu_unmap_guest(struct kvm *kvm)
> > +{
> > +	return 0;
> > +}
> > +#endif /* CONFIG_DMAR */
> 
> This means you can't use pvdma with iommu. Make these functions that check if 
> iommu support for device assignment is enabled. A command-line parameter to 
> qemu stating you want to enable vt-d for device assignment would be 
> appropriate.
> 
> For example, we could have the device assignment parameter changed to look 
> like this:
> 
> -pcidevice dev=00:13.0,dma=vtd,dma=pvdma
> 
> which will mean we should use vtd in conjunction with pvdma for this assigned 
> device.

Amit,

I agree with your comments that adding pvdma and AMD iommu support will
require more changes.
But I think that we should merge this code now, knowing that it will
need to change when more functionality is added in the future (isn't
that the way it usually works?).
I think we need to avoid the scenario we had two months ago with a lot
of pending code on a separate tree. It was much harder for people to
review the code and to consider merging it. If we merge this code then
the future changes will hopefully be smaller and easier to review and
merge.

Regards,
Ben

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Amit Shah]

* On Thursday 21 Aug 2008 16:35:57 Ben-Ami Yassour wrote:
> On Thu, 2008-08-21 at 12:13 +0530, Amit Shah wrote:
> > * On Thursday 07 Aug 2008 19:44:47 Ben-Ami Yassour wrote:
> > > Based on a patch by: Kay, Allen M <allen.m.kay@intel.com>
> > >
> > > This patch enables pci device assignment based on VT-d support.
> > > When a device is assigned to the guest, the guest memory is pinned and
> > > the mapping is updated in the VT-d IOMMU.
> > >
> > > Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> > > Signed-off-by: Weidong Han <weidong.han@intel.com>
> > > Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
> > > ---
> > >  arch/x86/kvm/Makefile      |    3 +
> > >  arch/x86/kvm/vtd.c         |  203
> > > ++++++++++++++++++++++++++++++++++++++++++++ arch/x86/kvm/x86.c        
> > > | 10 ++
> > >  include/asm-x86/kvm_host.h |    3 +
> > >  include/linux/kvm_host.h   |   32 +++++++
> > >  virt/kvm/kvm_main.c        |    9 ++-
> > >  6 files changed, 259 insertions(+), 1 deletions(-)
> > >  create mode 100644 arch/x86/kvm/vtd.c
> > >
> > > +int kvm_iommu_map_guest(struct kvm *kvm,
> > > +			struct kvm_assigned_dev_kernel *assigned_dev)
> > > +{
> > > +	struct pci_dev *pdev = NULL;
> > > +	int rc;
> > > +
> > > +	if (!intel_iommu_found()) {
> > > +		printk(KERN_ERR "intel iommu not found\n");
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	printk(KERN_DEBUG "VT-d direct map: host bdf = %x:%x:%x\n",
> > > +	       assigned_dev->host_busnr,
> > > +	       PCI_SLOT(assigned_dev->host_devfn),
> > > +	       PCI_FUNC(assigned_dev->host_devfn));
> > > +
> > > +	pdev = assigned_dev->dev;
> > > +
> > > +	if (pdev == NULL) {
> > > +		if (kvm->arch.intel_iommu_domain) {
> > > +			intel_iommu_domain_exit(kvm->arch.intel_iommu_domain);
> > > +			kvm->arch.intel_iommu_domain = NULL;
> > > +		}
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	kvm->arch.intel_iommu_domain = intel_iommu_domain_alloc(pdev);
> > >
> > > +int kvm_iommu_unmap_guest(struct kvm *kvm)
> > > +{
> > > +	struct kvm_assigned_dev_kernel *entry;
> > > +	struct pci_dev *pdev = NULL;
> > > +	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
> > > +
> > > +	/* check if iommu exists and in use */
> > > +	if (!domain)
> > > +		return 0;
> > > +
> > > +	list_for_each_entry(entry, &kvm->arch.assigned_dev_head, list) {
> > > +		printk(KERN_DEBUG "VT-d unmap: host bdf = %x:%x:%x\n",
> > > +		       entry->host_busnr,
> > > +		       PCI_SLOT(entry->host_devfn),
> > > +		       PCI_FUNC(entry->host_devfn));
> > > +
> > > +		for_each_pci_dev(pdev) {
> > > +			if ((pdev->bus->number == entry->host_busnr) &&
> > > +			    (pdev->devfn == entry->host_devfn))
> > > +				break;
> > > +		}
> > > +
> > > +		if (pdev == NULL)
> > > +			return -ENODEV;
> > > +
> > > +		/* detach kvm dmar domain */
> > > +		intel_iommu_detach_dev(domain,
> > > +				       pdev->bus->number, pdev->devfn);
> > > +	}
> > > +	kvm_iommu_unmap_memslots(kvm);
> > > +	intel_iommu_domain_exit(domain);
> > > +	return 0;
> > > +}
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index a97157c..5cfc21a 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -35,6 +35,7 @@
> > >  #include <linux/module.h>
> > >  #include <linux/mman.h>
> > >  #include <linux/highmem.h>
> > > +#include <linux/intel-iommu.h>
> > >
> > >  #include <asm/uaccess.h>
> > >  #include <asm/msr.h>
> > > @@ -271,9 +272,17 @@ static int kvm_vm_ioctl_assign_device(struct kvm
> > > *kvm,
> > >
> > >  	list_add(&match->list, &kvm->arch.assigned_dev_head);
> > >
> > > +	/* currenlty iommu is required for handling dma */
> > > +	r = kvm_iommu_map_guest(kvm, match);
> > > +	if (r)
> > > +		goto out_list_del;
> > > +
> >
> > This will break pvdma support. Please check if intel-iommu is found and
> > only then bail out of here.
> >
> > Even though pvdma is out-of-tree, it doesn't make sense to restrict our
> > usage here. AMD IOMMU support will need this to be handled in the right
> > way as well.
> >
> > >  out:
> > >  	mutex_unlock(&kvm->lock);
> > >  	return r;
> > > +out_list_del:
> > > +	list_del(&match->list);
> > > +	pci_release_regions(dev);
> > >  out_disable:
> > >  	pci_disable_device(dev);
> > >  out_put:
> > > @@ -4219,6 +4228,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
> > >
> > >  void kvm_arch_destroy_vm(struct kvm *kvm)
> > >  {
> > > +	kvm_iommu_unmap_guest(kvm);
> >
> > Likewise, check if intel-iommu is found as a first step in this function.
> >
> > As part of getting AMD-iommu and pvdma support in, we should have generic
> > function names, but that can be done later.
> >
> > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > > index a18aaad..b703890 100644
> > > --- a/include/linux/kvm_host.h
> > > +++ b/include/linux/kvm_host.h
> > > @@ -285,6 +285,33 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v);
> > >  int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu);
> > >  void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
> > >
> > > +#ifdef CONFIG_DMAR
> > > +int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
> > > +			unsigned long npages);
> > > +int kvm_iommu_map_guest(struct kvm *kvm,
> > > +			struct kvm_assigned_dev_kernel *assigned_dev);
> > > +int kvm_iommu_unmap_guest(struct kvm *kvm);
> > > +#else /* CONFIG_DMAR */
> > > +static inline int kvm_iommu_map_pages(struct kvm *kvm,
> > > +				      gfn_t base_gfn,
> > > +				      unsigned long npages)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > > +static inline int kvm_iommu_map_guest(struct kvm *kvm,
> > > +				      struct kvm_assigned_dev_kernel
> > > +				      *assigned_dev)
> > > +{
> > > +	return -ENODEV;
> > > +}
> > > +
> > > +static inline int kvm_iommu_unmap_guest(struct kvm *kvm)
> > > +{
> > > +	return 0;
> > > +}
> > > +#endif /* CONFIG_DMAR */
> >
> > This means you can't use pvdma with iommu. Make these functions that
> > check if iommu support for device assignment is enabled. A command-line
> > parameter to qemu stating you want to enable vt-d for device assignment
> > would be appropriate.
> >
> > For example, we could have the device assignment parameter changed to
> > look like this:
> >
> > -pcidevice dev=00:13.0,dma=vtd,dma=pvdma
> >
> > which will mean we should use vtd in conjunction with pvdma for this
> > assigned device.
>
> Amit,
>
> I agree with your comments that adding pvdma and AMD iommu support will
> require more changes.
> But I think that we should merge this code now, knowing that it will
> need to change when more functionality is added in the future (isn't
> that the way it usually works?).

I don't agree with this; at the minimum, something like this should be done:

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 29f94ba..d655188 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -277,10 +277,12 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,

        list_add(&match->list, &kvm->arch.assigned_dev_head);

-       /* currenlty iommu is required for handling dma */
-       r = kvm_iommu_map_guest(kvm, match);
-       if (r)
-               goto out_list_del;
+       /* Enable the Intel IOMMU if available */
+       if (intel_iommu_found()) {
+               r = kvm_iommu_map_guest(kvm, match);
+               if (r)
+                       goto out_list_del;
+       }

 out:
        mutex_unlock(&kvm->lock);

> I think we need to avoid the scenario we had two months ago with a lot
> of pending code on a separate tree. It was much harder for people to

That happened because the code was under development. It wasn't sitting out 
for lack of reviews or users.

> review the code and to consider merging it. If we merge this code then
> the future changes will hopefully be smaller and easier to review and
> merge.

Though true, the changes suggested are small enough and are a big convenience 
to the other developers involved. Remember that parallel development on other 
modes of DMA translation is ongoing and all are based on kvm-mainline. Once 
we have VT-d in, the others will base their work off this. If a small patch 
can make their lives easier later, it's worth it to include the patch now.

I'll mostly do the command-line option-based checking patch tomorrow unless 
someone else steps up first.

Amit

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Amit Shah]

* On Thursday 21 Aug 2008 16:35:57 Ben-Ami Yassour wrote:
> On Thu, 2008-08-21 at 12:13 +0530, Amit Shah wrote:
> > * On Thursday 07 Aug 2008 19:44:47 Ben-Ami Yassour wrote:
> > > Based on a patch by: Kay, Allen M <allen.m.kay@intel.com>
> > >
> > > This patch enables pci device assignment based on VT-d support.
> > > When a device is assigned to the guest, the guest memory is pinned and
> > > the mapping is updated in the VT-d IOMMU.
> > >
> > > Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> > > Signed-off-by: Weidong Han <weidong.han@intel.com>
> > > Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
> > > ---
> > >  arch/x86/kvm/Makefile      |    3 +
> > >  arch/x86/kvm/vtd.c         |  203
> > > ++++++++++++++++++++++++++++++++++++++++++++ arch/x86/kvm/x86.c        
> > > | 10 ++
> > >  include/asm-x86/kvm_host.h |    3 +
> > >  include/linux/kvm_host.h   |   32 +++++++
> > >  virt/kvm/kvm_main.c        |    9 ++-
> > >  6 files changed, 259 insertions(+), 1 deletions(-)
> > >  create mode 100644 arch/x86/kvm/vtd.c
> > >
> > > +int kvm_iommu_map_guest(struct kvm *kvm,
> > > +			struct kvm_assigned_dev_kernel *assigned_dev)
> > > +{
> > > +	struct pci_dev *pdev = NULL;
> > > +	int rc;
> > > +
> > > +	if (!intel_iommu_found()) {
> > > +		printk(KERN_ERR "intel iommu not found\n");
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	printk(KERN_DEBUG "VT-d direct map: host bdf = %x:%x:%x\n",
> > > +	       assigned_dev->host_busnr,
> > > +	       PCI_SLOT(assigned_dev->host_devfn),
> > > +	       PCI_FUNC(assigned_dev->host_devfn));
> > > +
> > > +	pdev = assigned_dev->dev;
> > > +
> > > +	if (pdev == NULL) {
> > > +		if (kvm->arch.intel_iommu_domain) {
> > > +			intel_iommu_domain_exit(kvm->arch.intel_iommu_domain);
> > > +			kvm->arch.intel_iommu_domain = NULL;
> > > +		}
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	kvm->arch.intel_iommu_domain = intel_iommu_domain_alloc(pdev);
> > >
> > > +int kvm_iommu_unmap_guest(struct kvm *kvm)
> > > +{
> > > +	struct kvm_assigned_dev_kernel *entry;
> > > +	struct pci_dev *pdev = NULL;
> > > +	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
> > > +
> > > +	/* check if iommu exists and in use */
> > > +	if (!domain)
> > > +		return 0;
> > > +
> > > +	list_for_each_entry(entry, &kvm->arch.assigned_dev_head, list) {
> > > +		printk(KERN_DEBUG "VT-d unmap: host bdf = %x:%x:%x\n",
> > > +		       entry->host_busnr,
> > > +		       PCI_SLOT(entry->host_devfn),
> > > +		       PCI_FUNC(entry->host_devfn));
> > > +
> > > +		for_each_pci_dev(pdev) {
> > > +			if ((pdev->bus->number == entry->host_busnr) &&
> > > +			    (pdev->devfn == entry->host_devfn))
> > > +				break;
> > > +		}
> > > +
> > > +		if (pdev == NULL)
> > > +			return -ENODEV;
> > > +
> > > +		/* detach kvm dmar domain */
> > > +		intel_iommu_detach_dev(domain,
> > > +				       pdev->bus->number, pdev->devfn);
> > > +	}
> > > +	kvm_iommu_unmap_memslots(kvm);
> > > +	intel_iommu_domain_exit(domain);
> > > +	return 0;
> > > +}
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index a97157c..5cfc21a 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -35,6 +35,7 @@
> > >  #include <linux/module.h>
> > >  #include <linux/mman.h>
> > >  #include <linux/highmem.h>
> > > +#include <linux/intel-iommu.h>
> > >
> > >  #include <asm/uaccess.h>
> > >  #include <asm/msr.h>
> > > @@ -271,9 +272,17 @@ static int kvm_vm_ioctl_assign_device(struct kvm
> > > *kvm,
> > >
> > >  	list_add(&match->list, &kvm->arch.assigned_dev_head);
> > >
> > > +	/* currenlty iommu is required for handling dma */
> > > +	r = kvm_iommu_map_guest(kvm, match);
> > > +	if (r)
> > > +		goto out_list_del;
> > > +
> >
> > This will break pvdma support. Please check if intel-iommu is found and
> > only then bail out of here.
> >
> > Even though pvdma is out-of-tree, it doesn't make sense to restrict our
> > usage here. AMD IOMMU support will need this to be handled in the right
> > way as well.
> >
> > >  out:
> > >  	mutex_unlock(&kvm->lock);
> > >  	return r;
> > > +out_list_del:
> > > +	list_del(&match->list);
> > > +	pci_release_regions(dev);
> > >  out_disable:
> > >  	pci_disable_device(dev);
> > >  out_put:
> > > @@ -4219,6 +4228,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
> > >
> > >  void kvm_arch_destroy_vm(struct kvm *kvm)
> > >  {
> > > +	kvm_iommu_unmap_guest(kvm);
> >
> > Likewise, check if intel-iommu is found as a first step in this function.
> >
> > As part of getting AMD-iommu and pvdma support in, we should have generic
> > function names, but that can be done later.
> >
> > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > > index a18aaad..b703890 100644
> > > --- a/include/linux/kvm_host.h
> > > +++ b/include/linux/kvm_host.h
> > > @@ -285,6 +285,33 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v);
> > >  int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu);
> > >  void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
> > >
> > > +#ifdef CONFIG_DMAR
> > > +int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
> > > +			unsigned long npages);
> > > +int kvm_iommu_map_guest(struct kvm *kvm,
> > > +			struct kvm_assigned_dev_kernel *assigned_dev);
> > > +int kvm_iommu_unmap_guest(struct kvm *kvm);
> > > +#else /* CONFIG_DMAR */
> > > +static inline int kvm_iommu_map_pages(struct kvm *kvm,
> > > +				      gfn_t base_gfn,
> > > +				      unsigned long npages)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > > +static inline int kvm_iommu_map_guest(struct kvm *kvm,
> > > +				      struct kvm_assigned_dev_kernel
> > > +				      *assigned_dev)
> > > +{
> > > +	return -ENODEV;
> > > +}
> > > +
> > > +static inline int kvm_iommu_unmap_guest(struct kvm *kvm)
> > > +{
> > > +	return 0;
> > > +}
> > > +#endif /* CONFIG_DMAR */
> >
> > This means you can't use pvdma with iommu. Make these functions that
> > check if iommu support for device assignment is enabled. A command-line
> > parameter to qemu stating you want to enable vt-d for device assignment
> > would be appropriate.
> >
> > For example, we could have the device assignment parameter changed to
> > look like this:
> >
> > -pcidevice dev=00:13.0,dma=vtd,dma=pvdma
> >
> > which will mean we should use vtd in conjunction with pvdma for this
> > assigned device.
>
> Amit,
>
> I agree with your comments that adding pvdma and AMD iommu support will
> require more changes.
> But I think that we should merge this code now, knowing that it will
> need to change when more functionality is added in the future (isn't
> that the way it usually works?).

I needed to do this:

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 29f94ba..40782df 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -277,10 +277,11 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,

        list_add(&match->list, &kvm->arch.assigned_dev_head);

-       /* currenlty iommu is required for handling dma */
-       r = kvm_iommu_map_guest(kvm, match);
-       if (r)
-               goto out_list_del;
+       if (assigned_dev->flags & KVM_DEV_ASSIGN_USE_VTD) {
+               r = kvm_iommu_map_guest(kvm, match);
+               if (r)
+                       goto out_list_del;
+       }

 out:
        mutex_unlock(&kvm->lock);
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index d9ef7d3..2956e35 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -495,4 +495,6 @@ struct kvm_assigned_irq {
        __u32 flags;
 };

+#define KVM_DEV_ASSIGN_USE_VTD (1 << 1)
+
 #endif


The userspace has a much bigger patch; but I'm prepping a new userspace patch 
since the last one submitted by you has a few bugs (most notable ones: host 
freeze if request_irq fails and also we pass 0 as the guest interrupt number, 
causing the interrupt ack notifier being called for int 0 before the 
request_irq succeeds)

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Avi Kivity]

Amit Shah wrote:
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index d9ef7d3..2956e35 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -495,4 +495,6 @@ struct kvm_assigned_irq {
>         __u32 flags;
>  };
>
> +#define KVM_DEV_ASSIGN_USE_VTD (1 << 1)
> +
>  #endif
>
>
>   

(1 >> 0)?

This is a userspace inteface, so use a generic name like iommu.  We also
need a KVM_CAP so userspace can check whether an iommu is present or not.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Amit Shah]

* On Friday 22 Aug 2008 23:48:42 Avi Kivity wrote:
> Amit Shah wrote:
> > diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> > index d9ef7d3..2956e35 100644
> > --- a/include/linux/kvm.h
> > +++ b/include/linux/kvm.h
> > @@ -495,4 +495,6 @@ struct kvm_assigned_irq {
> >         __u32 flags;
> >  };
> >
> > +#define KVM_DEV_ASSIGN_USE_VTD (1 << 1)
> > +
> >  #endif
>
> (1 >> 0)?

I kept the 1st field reserved for no particular implementation in mind as of 
now.

> This is a userspace inteface, so use a generic name like iommu.  We also
> need a KVM_CAP so userspace can check whether an iommu is present or not.

We could have multiple hardware IOMMU implementations, like Intel's VT-d and 
AMD's IOMMU.

Also, is KVM_CAP_foo needed for this? This is the only #define that'll be used 
and we can simply do something like

#ifdef KVM_DEV_ASSIGN_USE_VTD
flags |= KVM_DEV_ASSIGN_USE_VTD
#endif

?

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Avi Kivity]

Amit Shah wrote:
> * On Friday 22 Aug 2008 23:48:42 Avi Kivity wrote:
>   
>> Amit Shah wrote:
>>     
>>> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
>>> index d9ef7d3..2956e35 100644
>>> --- a/include/linux/kvm.h
>>> +++ b/include/linux/kvm.h
>>> @@ -495,4 +495,6 @@ struct kvm_assigned_irq {
>>>         __u32 flags;
>>>  };
>>>
>>> +#define KVM_DEV_ASSIGN_USE_VTD (1 << 1)
>>> +
>>>  #endif
>>>       
>> (1 >> 0)?
>>     
>
> I kept the 1st field reserved for no particular implementation in mind as of 
> now.
>
>   

Why?

>> This is a userspace inteface, so use a generic name like iommu.  We also
>> need a KVM_CAP so userspace can check whether an iommu is present or not.
>>     
>
> We could have multiple hardware IOMMU implementations, like Intel's VT-d and 
> AMD's IOMMU.
>
>   

Not in userspace.  Userspace sees either iommu or no iommu; it doesn't 
care about the iommu model.

> Also, is KVM_CAP_foo needed for this? This is the only #define that'll be used 
> and we can simply do something like
>
> #ifdef KVM_DEV_ASSIGN_USE_VTD
> flags |= KVM_DEV_ASSIGN_USE_VTD
> #endif
>
> ?
>   

That only detects if the headers have the flag, not if the kernel 
actually supports it (and whether there is an iommu in the host).  We 
need run-time detection.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Amit Shah]

* On Saturday 23 Aug 2008 14:58:50 Avi Kivity wrote:
> Amit Shah wrote:

> > Also, is KVM_CAP_foo needed for this? This is the only #define that'll be
> > used and we can simply do something like
> >
> > #ifdef KVM_DEV_ASSIGN_USE_VTD
> > flags |= KVM_DEV_ASSIGN_USE_VTD
> > #endif
> >
> > ?
>
> That only detects if the headers have the flag, not if the kernel
> actually supports it (and whether there is an iommu in the host).  We
> need run-time detection.

Which means we expose KVM_CAP_IOMMU only if one was detected? How to do this 
correctly?

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Avi Kivity]

Amit Shah wrote:
>> That only detects if the headers have the flag, not if the kernel
>> actually supports it (and whether there is an iommu in the host).  We
>> need run-time detection.
>>     
>
> Which means we expose KVM_CAP_IOMMU only if one was detected? 

Yes.

> How to do this 
> correctly?
>   

I don't know.  Can't the VT-d api tell you if an iommu exists?

-- 
error compiling committee.c: too many arguments to function

RE: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Han, Weidong]

Avi Kivity wrote:
> Amit Shah wrote:
>>> That only detects if the headers have the flag, not if the kernel
>>> actually supports it (and whether there is an iommu in the host). 
>>> We need run-time detection. 
>>> 
>> 
>> Which means we expose KVM_CAP_IOMMU only if one was detected?
> 
> Yes.
> 
>> How to do this
>> correctly?
>> 
> 
> I don't know.  Can't the VT-d api tell you if an iommu exists?

I think KVM_CAP_IOMMU is good. At the same time, there is only one iommu
at most, and userspace doesn't care about the iommu model. It's easy to
know whether an iommu exists. There is a VT-d api (intel_iommu_found())
to know if VT-d exists. Other iommu implementation can easily have this
api too.

Randy (Weidong)

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Amit Shah]

* On Friday 22 Aug 2008 23:48:42 Avi Kivity wrote:
> Amit Shah wrote:
> > diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> > index d9ef7d3..2956e35 100644
> > --- a/include/linux/kvm.h
> > +++ b/include/linux/kvm.h
> > @@ -495,4 +495,6 @@ struct kvm_assigned_irq {
> >         __u32 flags;
> >  };
> >
> > +#define KVM_DEV_ASSIGN_USE_VTD (1 << 1)
> > +
> >  #endif
>
> (1 >> 0)?
>
> This is a userspace inteface, so use a generic name like iommu.  We also
> need a KVM_CAP so userspace can check whether an iommu is present or not.

How about this?

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 29f94ba..38ab48b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -277,10 +277,11 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
 
 	list_add(&match->list, &kvm->arch.assigned_dev_head);
 
-	/* currenlty iommu is required for handling dma */
-	r = kvm_iommu_map_guest(kvm, match);
-	if (r)
-		goto out_list_del;
+	if (assigned_dev->flags & KVM_DEV_ASSIGN_ENABLE_IOMMU) {
+		r = kvm_iommu_map_guest(kvm, match);
+		if (r)
+			goto out_list_del;
+	}
 
 out:
 	mutex_unlock(&kvm->lock);
@@ -1154,6 +1155,9 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_PV_MMU:
 		r = !tdp_enabled;
 		break;
+	case KVM_CAP_IOMMU:
+		r = intel_iommu_found();
+		break;
 	default:
 		r = 0;
 		break;
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index d9ef7d3..f2eafe1 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -384,6 +384,7 @@ struct kvm_trace_rec {
 #define KVM_CAP_COALESCED_MMIO 15
 #define KVM_CAP_SYNC_MMU 16  /* Changes to host mmap are reflected in guest 
*/
 #define KVM_CAP_DEVICE_ASSIGNMENT 17
+#define KVM_CAP_IOMMU 18
 
 /*
  * ioctls for VM fds
@@ -495,4 +496,6 @@ struct kvm_assigned_irq {
 	__u32 flags;
 };
 
+#define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
+
 #endif

Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

[Avi Kivity]

Amit Shah wrote:
>> (1 >> 0)?
>>
>> This is a userspace inteface, so use a generic name like iommu.  We also
>> need a KVM_CAP so userspace can check whether an iommu is present or not.
>>     
>
> How about this?
>   

Looks fine.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/2] VT-d: changes to support KVM

[Yang, Sheng]

On Thursday 07 August 2008 22:14:46 Ben-Ami Yassour wrote:
> From: Kay, Allen M <allen.m.kay@intel.com>
>
> This patch extends the VT-d driver to support KVM

Seems OK to me.

Avi, is this patch good enough to send to PCI guys to have a review?

-- 
regards
Yang, Sheng
>
> [Ben: fixed memory pinning]
>
> Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> Signed-off-by: Weidong Han <weidong.han@intel.com>
> Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
> ---
>  drivers/pci/dmar.c                           |    4 +-
>  drivers/pci/intel-iommu.c                    |  117
> +++++++++++++++++++++++++- drivers/pci/iova.c                      
>     |    2 +-
>  {drivers/pci => include/linux}/intel-iommu.h |   11 +++
>  {drivers/pci => include/linux}/iova.h        |    0
>  5 files changed, 127 insertions(+), 7 deletions(-)
>  rename {drivers/pci => include/linux}/intel-iommu.h (94%)
>  rename {drivers/pci => include/linux}/iova.h (100%)
>
> diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
> index 8bf86ae..1df28ea 100644
> --- a/drivers/pci/dmar.c
> +++ b/drivers/pci/dmar.c
> @@ -26,8 +26,8 @@
>
>  #include <linux/pci.h>
>  #include <linux/dmar.h>
> -#include "iova.h"
> -#include "intel-iommu.h"
> +#include <linux/iova.h>
> +#include <linux/intel-iommu.h>
>
>  #undef PREFIX
>  #define PREFIX "DMAR:"
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index 8d0e60a..1eefc60 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -20,6 +20,7 @@
>   * Author: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
>   */
>
> +#undef DEBUG
>  #include <linux/init.h>
>  #include <linux/bitmap.h>
>  #include <linux/debugfs.h>
> @@ -33,8 +34,8 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/mempool.h>
>  #include <linux/timer.h>
> -#include "iova.h"
> -#include "intel-iommu.h"
> +#include <linux/iova.h>
> +#include <linux/intel-iommu.h>
>  #include <asm/proto.h> /* force_iommu in this header in x86-64*/
>  #include <asm/cacheflush.h>
>  #include <asm/iommu.h>
> @@ -160,7 +161,7 @@ static inline void *alloc_domain_mem(void)
>  	return iommu_kmem_cache_alloc(iommu_domain_cache);
>  }
>
> -static inline void free_domain_mem(void *vaddr)
> +static void free_domain_mem(void *vaddr)
>  {
>  	kmem_cache_free(iommu_domain_cache, vaddr);
>  }
> @@ -1414,7 +1415,7 @@ static void domain_remove_dev_info(struct
> dmar_domain *domain) * find_domain
>   * Note: we use struct pci_dev->dev.archdata.iommu stores the info
>   */
> -struct dmar_domain *
> +static struct dmar_domain *
>  find_domain(struct pci_dev *pdev)
>  {
>  	struct device_domain_info *info;
> @@ -2430,3 +2431,111 @@ int __init intel_iommu_init(void)
>  	return 0;
>  }
>
> +void intel_iommu_domain_exit(struct dmar_domain *domain)
> +{
> +	u64 end;
> +
> +	/* Domain 0 is reserved, so dont process it */
> +	if (!domain)
> +		return;
> +
> +	end = DOMAIN_MAX_ADDR(domain->gaw);
> +	end = end & (~PAGE_MASK_4K);
> +
> +	/* clear ptes */
> +	dma_pte_clear_range(domain, 0, end);
> +
> +	/* free page tables */
> +	dma_pte_free_pagetable(domain, 0, end);
> +
> +	iommu_free_domain(domain);
> +	free_domain_mem(domain);
> +}
> +EXPORT_SYMBOL_GPL(intel_iommu_domain_exit);
> +
> +struct dmar_domain *intel_iommu_domain_alloc(struct pci_dev *pdev)
> +{
> +	struct dmar_drhd_unit *drhd;
> +	struct dmar_domain *domain;
> +	struct intel_iommu *iommu;
> +
> +	drhd = dmar_find_matched_drhd_unit(pdev);
> +	if (!drhd) {
> +		printk(KERN_ERR "intel_iommu_domain_alloc: drhd == NULL\n");
> +		return NULL;
> +	}
> +
> +	iommu = drhd->iommu;
> +	if (!iommu) {
> +		printk(KERN_ERR
> +			"intel_iommu_domain_alloc: iommu == NULL\n");
> +		return NULL;
> +	}
> +	domain = iommu_alloc_domain(iommu);
> +	if (!domain) {
> +		printk(KERN_ERR
> +			"intel_iommu_domain_alloc: domain == NULL\n");
> +		return NULL;
> +	}
> +	if (domain_init(domain, DEFAULT_DOMAIN_ADDRESS_WIDTH)) {
> +		printk(KERN_ERR
> +			"intel_iommu_domain_alloc: domain_init() failed\n");
> +		intel_iommu_domain_exit(domain);
> +		return NULL;
> +	}
> +	return domain;
> +}
> +EXPORT_SYMBOL_GPL(intel_iommu_domain_alloc);
> +
> +int intel_iommu_context_mapping(
> +	struct dmar_domain *domain, struct pci_dev *pdev)
> +{
> +	int rc;
> +	rc = domain_context_mapping(domain, pdev);
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(intel_iommu_context_mapping);
> +
> +int intel_iommu_page_mapping(
> +	struct dmar_domain *domain, dma_addr_t iova,
> +	u64 hpa, size_t size, int prot)
> +{
> +	int rc;
> +	rc = domain_page_mapping(domain, iova, hpa, size, prot);
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(intel_iommu_page_mapping);
> +
> +void intel_iommu_detach_dev(struct dmar_domain *domain, u8 bus, u8
> devfn) +{
> +	detach_domain_for_dev(domain, bus, devfn);
> +}
> +EXPORT_SYMBOL_GPL(intel_iommu_detach_dev);
> +
> +struct dmar_domain *
> +intel_iommu_find_domain(struct pci_dev *pdev)
> +{
> +	return find_domain(pdev);
> +}
> +EXPORT_SYMBOL_GPL(intel_iommu_find_domain);
> +
> +int intel_iommu_found(void)
> +{
> +	return g_num_of_iommus;
> +}
> +EXPORT_SYMBOL_GPL(intel_iommu_found);
> +
> +u64 intel_iommu_iova_to_pfn(struct dmar_domain *domain, u64 iova)
> +{
> +	struct dma_pte *pte;
> +	u64 pfn;
> +
> +	pfn = 0;
> +	pte = addr_to_dma_pte(domain, iova);
> +
> +	if (pte)
> +		pfn = dma_pte_addr(*pte);
> +
> +	return pfn >> PAGE_SHIFT_4K;
> +}
> +EXPORT_SYMBOL_GPL(intel_iommu_iova_to_pfn);
> diff --git a/drivers/pci/iova.c b/drivers/pci/iova.c
> index 3ef4ac0..2287116 100644
> --- a/drivers/pci/iova.c
> +++ b/drivers/pci/iova.c
> @@ -7,7 +7,7 @@
>   * Author: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
>   */
>
> -#include "iova.h"
> +#include <linux/iova.h>
>
>  void
>  init_iova_domain(struct iova_domain *iovad, unsigned long
> pfn_32bit) diff --git a/drivers/pci/intel-iommu.h
> b/include/linux/intel-iommu.h similarity index 94%
> rename from drivers/pci/intel-iommu.h
> rename to include/linux/intel-iommu.h
> index afc0ad9..1490fc0 100644
> --- a/drivers/pci/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -341,4 +341,15 @@ static inline void
> iommu_prepare_gfx_mapping(void) }
>  #endif /* !CONFIG_DMAR_GFX_WA */
>
> +void intel_iommu_domain_exit(struct dmar_domain *domain);
> +struct dmar_domain *intel_iommu_domain_alloc(struct pci_dev
> *pdev); +int intel_iommu_context_mapping(struct dmar_domain
> *domain, +				struct pci_dev *pdev);
> +int intel_iommu_page_mapping(struct dmar_domain *domain,
> dma_addr_t iova, +			     u64 hpa, size_t size, int prot);
> +void intel_iommu_detach_dev(struct dmar_domain *domain, u8 bus, u8
> devfn); +struct dmar_domain *intel_iommu_find_domain(struct pci_dev
> *pdev); +int intel_iommu_found(void);
> +u64 intel_iommu_iova_to_pfn(struct dmar_domain *domain, u64 iova);
> +
>  #endif
> diff --git a/drivers/pci/iova.h b/include/linux/iova.h
> similarity index 100%
> rename from drivers/pci/iova.h
> rename to include/linux/iova.h

Re: [PATCH 1/2] VT-d: changes to support KVM

[Avi Kivity]

Yang, Sheng wrote:
> On Thursday 07 August 2008 22:14:46 Ben-Ami Yassour wrote:
>   
>> From: Kay, Allen M <allen.m.kay@intel.com>
>>
>> This patch extends the VT-d driver to support KVM
>>     
>
> Seems OK to me.
>
> Avi, is this patch good enough to send to PCI guys to have a review?
>
>   

I think so.  There are some trivial coding style issues, but nothing 
serious.

-- 
error compiling committee.c: too many arguments to function

VT-d support for device assignment

[Ben-Ami Yassour]

Avi,

I am re-sending the VT-d patches, please apply.

The following two patches contains the VT-d support for device
assignment.
The first patch contains the changes that are required to the generic
VT-d code.
The second patch contains the changes to KVM.

Regards,
Ben

[PATCH 1/2] VT-d: changes to support KVM

[Ben-Ami Yassour]

From: Kay, Allen M <allen.m.kay@intel.com>

This patch extends the VT-d driver to support KVM

[Ben: fixed memory pinning]

Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
Signed-off-by: Weidong Han <weidong.han@intel.com>
Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
---
 drivers/pci/dmar.c                           |    4 +-
 drivers/pci/intel-iommu.c                    |  117 +++++++++++++++++++++++++-
 drivers/pci/iova.c                           |    2 +-
 {drivers/pci => include/linux}/intel-iommu.h |   11 +++
 {drivers/pci => include/linux}/iova.h        |    0 
 5 files changed, 127 insertions(+), 7 deletions(-)
 rename {drivers/pci => include/linux}/intel-iommu.h (94%)
 rename {drivers/pci => include/linux}/iova.h (100%)

diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
index 8bf86ae..1df28ea 100644
--- a/drivers/pci/dmar.c
+++ b/drivers/pci/dmar.c
@@ -26,8 +26,8 @@
 
 #include <linux/pci.h>
 #include <linux/dmar.h>
-#include "iova.h"
-#include "intel-iommu.h"
+#include <linux/iova.h>
+#include <linux/intel-iommu.h>
 
 #undef PREFIX
 #define PREFIX "DMAR:"
diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 8d0e60a..1eefc60 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -20,6 +20,7 @@
  * Author: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
  */
 
+#undef DEBUG
 #include <linux/init.h>
 #include <linux/bitmap.h>
 #include <linux/debugfs.h>
@@ -33,8 +34,8 @@
 #include <linux/dma-mapping.h>
 #include <linux/mempool.h>
 #include <linux/timer.h>
-#include "iova.h"
-#include "intel-iommu.h"
+#include <linux/iova.h>
+#include <linux/intel-iommu.h>
 #include <asm/proto.h> /* force_iommu in this header in x86-64*/
 #include <asm/cacheflush.h>
 #include <asm/iommu.h>
@@ -160,7 +161,7 @@ static inline void *alloc_domain_mem(void)
 	return iommu_kmem_cache_alloc(iommu_domain_cache);
 }
 
-static inline void free_domain_mem(void *vaddr)
+static void free_domain_mem(void *vaddr)
 {
 	kmem_cache_free(iommu_domain_cache, vaddr);
 }
@@ -1414,7 +1415,7 @@ static void domain_remove_dev_info(struct dmar_domain *domain)
  * find_domain
  * Note: we use struct pci_dev->dev.archdata.iommu stores the info
  */
-struct dmar_domain *
+static struct dmar_domain *
 find_domain(struct pci_dev *pdev)
 {
 	struct device_domain_info *info;
@@ -2430,3 +2431,111 @@ int __init intel_iommu_init(void)
 	return 0;
 }
 
+void intel_iommu_domain_exit(struct dmar_domain *domain)
+{
+	u64 end;
+
+	/* Domain 0 is reserved, so dont process it */
+	if (!domain)
+		return;
+
+	end = DOMAIN_MAX_ADDR(domain->gaw);
+	end = end & (~PAGE_MASK_4K);
+
+	/* clear ptes */
+	dma_pte_clear_range(domain, 0, end);
+
+	/* free page tables */
+	dma_pte_free_pagetable(domain, 0, end);
+
+	iommu_free_domain(domain);
+	free_domain_mem(domain);
+}
+EXPORT_SYMBOL_GPL(intel_iommu_domain_exit);
+
+struct dmar_domain *intel_iommu_domain_alloc(struct pci_dev *pdev)
+{
+	struct dmar_drhd_unit *drhd;
+	struct dmar_domain *domain;
+	struct intel_iommu *iommu;
+
+	drhd = dmar_find_matched_drhd_unit(pdev);
+	if (!drhd) {
+		printk(KERN_ERR "intel_iommu_domain_alloc: drhd == NULL\n");
+		return NULL;
+	}
+
+	iommu = drhd->iommu;
+	if (!iommu) {
+		printk(KERN_ERR
+			"intel_iommu_domain_alloc: iommu == NULL\n");
+		return NULL;
+	}
+	domain = iommu_alloc_domain(iommu);
+	if (!domain) {
+		printk(KERN_ERR
+			"intel_iommu_domain_alloc: domain == NULL\n");
+		return NULL;
+	}
+	if (domain_init(domain, DEFAULT_DOMAIN_ADDRESS_WIDTH)) {
+		printk(KERN_ERR
+			"intel_iommu_domain_alloc: domain_init() failed\n");
+		intel_iommu_domain_exit(domain);
+		return NULL;
+	}
+	return domain;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_domain_alloc);
+
+int intel_iommu_context_mapping(
+	struct dmar_domain *domain, struct pci_dev *pdev)
+{
+	int rc;
+	rc = domain_context_mapping(domain, pdev);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_context_mapping);
+
+int intel_iommu_page_mapping(
+	struct dmar_domain *domain, dma_addr_t iova,
+	u64 hpa, size_t size, int prot)
+{
+	int rc;
+	rc = domain_page_mapping(domain, iova, hpa, size, prot);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_page_mapping);
+
+void intel_iommu_detach_dev(struct dmar_domain *domain, u8 bus, u8 devfn)
+{
+	detach_domain_for_dev(domain, bus, devfn);
+}
+EXPORT_SYMBOL_GPL(intel_iommu_detach_dev);
+
+struct dmar_domain *
+intel_iommu_find_domain(struct pci_dev *pdev)
+{
+	return find_domain(pdev);
+}
+EXPORT_SYMBOL_GPL(intel_iommu_find_domain);
+
+int intel_iommu_found(void)
+{
+	return g_num_of_iommus;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_found);
+
+u64 intel_iommu_iova_to_pfn(struct dmar_domain *domain, u64 iova)
+{
+	struct dma_pte *pte;
+	u64 pfn;
+
+	pfn = 0;
+	pte = addr_to_dma_pte(domain, iova);
+
+	if (pte)
+		pfn = dma_pte_addr(*pte);
+
+	return pfn >> PAGE_SHIFT_4K;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_iova_to_pfn);
diff --git a/drivers/pci/iova.c b/drivers/pci/iova.c
index 3ef4ac0..2287116 100644
--- a/drivers/pci/iova.c
+++ b/drivers/pci/iova.c
@@ -7,7 +7,7 @@
  * Author: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
  */
 
-#include "iova.h"
+#include <linux/iova.h>
 
 void
 init_iova_domain(struct iova_domain *iovad, unsigned long pfn_32bit)
diff --git a/drivers/pci/intel-iommu.h b/include/linux/intel-iommu.h
similarity index 94%
rename from drivers/pci/intel-iommu.h
rename to include/linux/intel-iommu.h
index afc0ad9..1490fc0 100644
--- a/drivers/pci/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -341,4 +341,15 @@ static inline void iommu_prepare_gfx_mapping(void)
 }
 #endif /* !CONFIG_DMAR_GFX_WA */
 
+void intel_iommu_domain_exit(struct dmar_domain *domain);
+struct dmar_domain *intel_iommu_domain_alloc(struct pci_dev *pdev);
+int intel_iommu_context_mapping(struct dmar_domain *domain,
+				struct pci_dev *pdev);
+int intel_iommu_page_mapping(struct dmar_domain *domain, dma_addr_t iova,
+			     u64 hpa, size_t size, int prot);
+void intel_iommu_detach_dev(struct dmar_domain *domain, u8 bus, u8 devfn);
+struct dmar_domain *intel_iommu_find_domain(struct pci_dev *pdev);
+int intel_iommu_found(void);
+u64 intel_iommu_iova_to_pfn(struct dmar_domain *domain, u64 iova);
+
 #endif
diff --git a/drivers/pci/iova.h b/include/linux/iova.h
similarity index 100%
rename from drivers/pci/iova.h
rename to include/linux/iova.h
-- 
1.5.6

[PATCH 2/2] KVM: Device assignemnt with VT-d

[Ben-Ami Yassour]

Based on a patch by: Kay, Allen M <allen.m.kay@intel.com>

This patch enables pci device assignment based on VT-d support.
When a device is assigned to the guest, the guest memory is pinned and
the mapping is updated in the VT-d IOMMU.

Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
Signed-off-by: Weidong Han <weidong.han@intel.com>
Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
---
 arch/x86/kvm/Makefile      |    3 +
 arch/x86/kvm/vtd.c         |  203 ++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c         |   10 ++
 include/asm-x86/kvm_host.h |    3 +
 include/linux/kvm_host.h   |   32 +++++++
 virt/kvm/kvm_main.c        |    9 ++-
 6 files changed, 259 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/kvm/vtd.c

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index d0e940b..3072b17 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -12,6 +12,9 @@ EXTRA_CFLAGS += -Ivirt/kvm -Iarch/x86/kvm
 
 kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o \
 	i8254.o
+ifeq ($(CONFIG_DMAR),y)
+kvm-objs += vtd.o
+endif
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
diff --git a/arch/x86/kvm/vtd.c b/arch/x86/kvm/vtd.c
new file mode 100644
index 0000000..4336769
--- /dev/null
+++ b/arch/x86/kvm/vtd.c
@@ -0,0 +1,203 @@
+/*
+ * Copyright (c) 2006, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * Copyright (C) 2006-2008 Intel Corporation
+ * Copyright IBM Corporation, 2008
+ * Author: Allen M. Kay <allen.m.kay@intel.com>
+ * Author: Weidong Han <weidong.han@intel.com>
+ * Author: Ben-Ami Yassour <benami@il.ibm.com>
+ */
+
+#include <linux/list.h>
+#include <linux/kvm_host.h>
+#include <linux/pci.h>
+#include <linux/dmar.h>
+#include <linux/intel-iommu.h>
+
+static int kvm_iommu_unmap_memslots(struct kvm *kvm);
+
+int kvm_iommu_map_pages(struct kvm *kvm,
+			  gfn_t base_gfn, unsigned long npages)
+{
+	gfn_t gfn = base_gfn;
+	pfn_t pfn;
+	int i, rc;
+	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+
+	/* check if iommu exists and in use */
+	if (!domain)
+		return 0;
+
+	for (i = 0; i < npages; i++) {
+		/* check if already mapped */
+		pfn = (pfn_t)intel_iommu_iova_to_pfn(domain,
+						     gfn_to_gpa(gfn));
+		if (pfn && !is_mmio_pfn(pfn))
+			continue;
+
+		pfn = gfn_to_pfn(kvm, gfn);
+		if (!is_mmio_pfn(pfn)) {
+			rc = intel_iommu_page_mapping(domain,
+						      gfn_to_gpa(gfn),
+						      pfn_to_hpa(pfn),
+						      PAGE_SIZE,
+						      DMA_PTE_READ |
+						      DMA_PTE_WRITE);
+			if (rc) {
+				kvm_release_pfn_clean(pfn);
+				printk(KERN_DEBUG "kvm_iommu_map_pages:"
+				       "iommu failed to map pfn=%lx\n", pfn);
+				return rc;
+			}
+		} else {
+			printk(KERN_DEBUG "kvm_iommu_map_page:"
+			       "invalid pfn=%lx\n", pfn);
+			return 0;
+		}
+
+		gfn++;
+	}
+	return 0;
+}
+
+static int kvm_iommu_map_memslots(struct kvm *kvm)
+{
+	int i, rc;
+
+	down_read(&kvm->slots_lock);
+	for (i = 0; i < kvm->nmemslots; i++) {
+		rc = kvm_iommu_map_pages(kvm, kvm->memslots[i].base_gfn,
+					 kvm->memslots[i].npages);
+		if (rc) {
+			up_read(&kvm->slots_lock);
+			return rc;
+		}
+	}
+	up_read(&kvm->slots_lock);
+	return 0;
+}
+
+int kvm_iommu_map_guest(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *assigned_dev)
+{
+	struct pci_dev *pdev = NULL;
+	int rc;
+
+	if (!intel_iommu_found()) {
+		printk(KERN_ERR "intel iommu not found\n");
+		return -ENODEV;
+	}
+
+	printk(KERN_DEBUG "VT-d direct map: host bdf = %x:%x:%x\n",
+	       assigned_dev->host_busnr,
+	       PCI_SLOT(assigned_dev->host_devfn),
+	       PCI_FUNC(assigned_dev->host_devfn));
+
+	pdev = assigned_dev->dev;
+
+	if (pdev == NULL) {
+		if (kvm->arch.intel_iommu_domain) {
+			intel_iommu_domain_exit(kvm->arch.intel_iommu_domain);
+			kvm->arch.intel_iommu_domain = NULL;
+		}
+		return -ENODEV;
+	}
+
+	kvm->arch.intel_iommu_domain = intel_iommu_domain_alloc(pdev);
+
+	rc = kvm_iommu_map_memslots(kvm);
+	if (rc)
+		goto out_unmap;
+
+	intel_iommu_detach_dev(kvm->arch.intel_iommu_domain,
+			       pdev->bus->number, pdev->devfn);
+
+	rc = intel_iommu_context_mapping(kvm->arch.intel_iommu_domain,
+					 pdev);
+	if (rc) {
+		printk(KERN_ERR "Domain context map for %s failed",
+		       pci_name(pdev));
+		goto out_unmap;
+	}
+	return 0;
+
+out_unmap:
+	kvm_iommu_unmap_memslots(kvm);
+	return rc;
+}
+
+static void kvm_iommu_put_pages(struct kvm *kvm,
+			       gfn_t base_gfn, unsigned long npages)
+{
+	gfn_t gfn = base_gfn;
+	pfn_t pfn;
+	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+	int i;
+
+	for (i = 0; i < npages; i++) {
+		pfn = (pfn_t)intel_iommu_iova_to_pfn(domain,
+						     gfn_to_gpa(gfn));
+		kvm_release_pfn_clean(pfn);
+		gfn++;
+	}
+}
+
+static int kvm_iommu_unmap_memslots(struct kvm *kvm)
+{
+	int i;
+	down_read(&kvm->slots_lock);
+	for (i = 0; i < kvm->nmemslots; i++) {
+		kvm_iommu_put_pages(kvm, kvm->memslots[i].base_gfn,
+				    kvm->memslots[i].npages);
+	}
+	up_read(&kvm->slots_lock);
+
+	return 0;
+}
+
+int kvm_iommu_unmap_guest(struct kvm *kvm)
+{
+	struct kvm_assigned_dev_kernel *entry;
+	struct pci_dev *pdev = NULL;
+	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+
+	/* check if iommu exists and in use */
+	if (!domain)
+		return 0;
+
+	list_for_each_entry(entry, &kvm->arch.assigned_dev_head, list) {
+		printk(KERN_DEBUG "VT-d unmap: host bdf = %x:%x:%x\n",
+		       entry->host_busnr,
+		       PCI_SLOT(entry->host_devfn),
+		       PCI_FUNC(entry->host_devfn));
+
+		for_each_pci_dev(pdev) {
+			if ((pdev->bus->number == entry->host_busnr) &&
+			    (pdev->devfn == entry->host_devfn))
+				break;
+		}
+
+		if (pdev == NULL)
+			return -ENODEV;
+
+		/* detach kvm dmar domain */
+		intel_iommu_detach_dev(domain,
+				       pdev->bus->number, pdev->devfn);
+	}
+	kvm_iommu_unmap_memslots(kvm);
+	intel_iommu_domain_exit(domain);
+	return 0;
+}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a97157c..5cfc21a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -35,6 +35,7 @@
 #include <linux/module.h>
 #include <linux/mman.h>
 #include <linux/highmem.h>
+#include <linux/intel-iommu.h>
 
 #include <asm/uaccess.h>
 #include <asm/msr.h>
@@ -271,9 +272,17 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
 
 	list_add(&match->list, &kvm->arch.assigned_dev_head);
 
+	/* currenlty iommu is required for handling dma */
+	r = kvm_iommu_map_guest(kvm, match);
+	if (r)
+		goto out_list_del;
+
 out:
 	mutex_unlock(&kvm->lock);
 	return r;
+out_list_del:
+	list_del(&match->list);
+	pci_release_regions(dev);
 out_disable:
 	pci_disable_device(dev);
 out_put:
@@ -4219,6 +4228,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
 
 void kvm_arch_destroy_vm(struct kvm *kvm)
 {
+	kvm_iommu_unmap_guest(kvm);
 	kvm_free_assigned_devices(kvm);
 	kvm_free_pit(kvm);
 	kfree(kvm->arch.vpic);
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index ef019b5..b141949 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -354,6 +354,7 @@ struct kvm_arch{
 	 */
 	struct list_head active_mmu_pages;
 	struct list_head assigned_dev_head;
+	struct dmar_domain *intel_iommu_domain;
 	struct kvm_pic *vpic;
 	struct kvm_ioapic *vioapic;
 	struct kvm_pit *vpit;
@@ -503,6 +504,8 @@ int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
 int kvm_pv_mmu_op(struct kvm_vcpu *vcpu, unsigned long bytes,
 		  gpa_t addr, unsigned long *ret);
 
+int is_mmio_pfn(pfn_t pfn);
+
 extern bool tdp_enabled;
 
 enum emulation_result {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index a18aaad..b703890 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -285,6 +285,33 @@ int kvm_cpu_has_interrupt(struct kvm_vcpu *v);
 int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu);
 void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
 
+#ifdef CONFIG_DMAR
+int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
+			unsigned long npages);
+int kvm_iommu_map_guest(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *assigned_dev);
+int kvm_iommu_unmap_guest(struct kvm *kvm);
+#else /* CONFIG_DMAR */
+static inline int kvm_iommu_map_pages(struct kvm *kvm,
+				      gfn_t base_gfn,
+				      unsigned long npages)
+{
+	return 0;
+}
+
+static inline int kvm_iommu_map_guest(struct kvm *kvm,
+				      struct kvm_assigned_dev_kernel
+				      *assigned_dev)
+{
+	return -ENODEV;
+}
+
+static inline int kvm_iommu_unmap_guest(struct kvm *kvm)
+{
+	return 0;
+}
+#endif /* CONFIG_DMAR */
+
 static inline void kvm_guest_enter(void)
 {
 	account_system_vtime(current);
@@ -307,6 +334,11 @@ static inline gpa_t gfn_to_gpa(gfn_t gfn)
 	return (gpa_t)gfn << PAGE_SHIFT;
 }
 
+static inline hpa_t pfn_to_hpa(pfn_t pfn)
+{
+	return (hpa_t)pfn << PAGE_SHIFT;
+}
+
 static inline void kvm_migrate_timers(struct kvm_vcpu *vcpu)
 {
 	set_bit(KVM_REQ_MIGRATE_TIMER, &vcpu->requests);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 5eb96c7..ffd9069 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -41,6 +41,7 @@
 #include <linux/pagemap.h>
 #include <linux/mman.h>
 #include <linux/swap.h>
+#include <linux/intel-iommu.h>
 
 #include <asm/processor.h>
 #include <asm/io.h>
@@ -76,7 +77,7 @@ static inline int valid_vcpu(int n)
 	return likely(n >= 0 && n < KVM_MAX_VCPUS);
 }
 
-static inline int is_mmio_pfn(pfn_t pfn)
+inline int is_mmio_pfn(pfn_t pfn)
 {
 	if (pfn_valid(pfn))
 		return PageReserved(pfn_to_page(pfn));
@@ -578,6 +579,12 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	}
 
 	kvm_free_physmem_slot(&old, &new);
+
+	/* map the pages in iommu page table */
+	r = kvm_iommu_map_pages(kvm, base_gfn, npages);
+	if (r)
+		goto out_free;
+
 	return 0;
 
 out_free:
-- 
1.5.6