[PATCH 0/9] Factor VT-d KVM functions into a generic API

[PATCH 0/9] Factor VT-d KVM functions into a generic API

[Joerg Roedel]

Hi,

this patch series makes the current KVM device passthrough code generic
enough so that other IOMMU implementation can also plug into this code.
It works by factoring the functions Vt-d code exports to KVM into a
generic interface which allows different backends.

This a basic implementation of a generic interface. It can and should be
improved later to support more types of hardware IOMMUs then VT-d and
AMD IOMMU.

Since I have no VT-d hardware available these patches are only compile
tested for now.

Please review, comment and test these patches.

Thanks,

Joerg

diffstat:

 arch/ia64/Kconfig               |    3 +
 arch/ia64/kvm/Makefile          |    2 +-
 arch/x86/Kconfig                |    3 +
 arch/x86/include/asm/kvm_host.h |    3 +-
 arch/x86/kvm/Makefile           |    2 +-
 drivers/base/Makefile           |    1 +
 drivers/base/iommu.c            |   94 +++++++++++++++++++
 drivers/pci/intel-iommu.c       |   91 ++++++++++++++++++
 include/linux/iommu.h           |  100 ++++++++++++++++++++
 virt/kvm/iommu.c                |  193 +++++++++++++++++++++++++++++++++++++++
 virt/kvm/vtd.c                  |  191 --------------------------------------

[PATCH 1/9] KVM: rename vtd.c to iommu.c

[Joerg Roedel]

Impace: file renamed

The code in the vtd.c file can be reused for other IOMMUs as well. So
rename it to make it clear that it handle more than VT-d.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/ia64/kvm/Makefile |    2 +-
 arch/x86/kvm/Makefile  |    2 +-
 virt/kvm/iommu.c       |  191 ++++++++++++++++++++++++++++++++++++++++++++++++
 virt/kvm/vtd.c         |  191 ------------------------------------------------
 4 files changed, 193 insertions(+), 193 deletions(-)
 create mode 100644 virt/kvm/iommu.c
 delete mode 100644 virt/kvm/vtd.c

diff --git a/arch/ia64/kvm/Makefile b/arch/ia64/kvm/Makefile
index 76464dc..cb69dfc 100644
--- a/arch/ia64/kvm/Makefile
+++ b/arch/ia64/kvm/Makefile
@@ -52,7 +52,7 @@ common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o ioapic.o \
 		coalesced_mmio.o irq_comm.o)
 
 ifeq ($(CONFIG_DMAR),y)
-common-objs += $(addprefix ../../../virt/kvm/, vtd.o)
+common-objs += $(addprefix ../../../virt/kvm/, iommu.o)
 endif
 
 kvm-objs := $(common-objs) kvm-ia64.o kvm_fw.o
diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index c023435..00f46c2 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -8,7 +8,7 @@ ifeq ($(CONFIG_KVM_TRACE),y)
 common-objs += $(addprefix ../../../virt/kvm/, kvm_trace.o)
 endif
 ifeq ($(CONFIG_DMAR),y)
-common-objs += $(addprefix ../../../virt/kvm/, vtd.o)
+common-objs += $(addprefix ../../../virt/kvm/, iommu.o)
 endif
 
 EXTRA_CFLAGS += -Ivirt/kvm -Iarch/x86/kvm
diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
new file mode 100644
index 0000000..a770874
--- /dev/null
+++ b/virt/kvm/iommu.c
@@ -0,0 +1,191 @@
+/*
+ * 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);
+static void kvm_iommu_put_pages(struct kvm *kvm,
+				gfn_t base_gfn, unsigned long npages);
+
+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, r = 0;
+	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)
+			continue;
+
+		pfn = gfn_to_pfn(kvm, gfn);
+		r = intel_iommu_page_mapping(domain,
+					     gfn_to_gpa(gfn),
+					     pfn_to_hpa(pfn),
+					     PAGE_SIZE,
+					     DMA_PTE_READ |
+					     DMA_PTE_WRITE);
+		if (r) {
+			printk(KERN_ERR "kvm_iommu_map_pages:"
+			       "iommu failed to map pfn=%lx\n", pfn);
+			goto unmap_pages;
+		}
+		gfn++;
+	}
+	return 0;
+
+unmap_pages:
+	kvm_iommu_put_pages(kvm, base_gfn, i);
+	return r;
+}
+
+static int kvm_iommu_map_memslots(struct kvm *kvm)
+{
+	int i, r;
+
+	down_read(&kvm->slots_lock);
+	for (i = 0; i < kvm->nmemslots; i++) {
+		r = kvm_iommu_map_pages(kvm, kvm->memslots[i].base_gfn,
+					kvm->memslots[i].npages);
+		if (r)
+			break;
+	}
+	up_read(&kvm->slots_lock);
+	return r;
+}
+
+int kvm_iommu_map_guest(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *assigned_dev)
+{
+	struct pci_dev *pdev = NULL;
+	int r;
+
+	if (!intel_iommu_found()) {
+		printk(KERN_ERR "%s: intel iommu not found\n", __func__);
+		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);
+	if (!kvm->arch.intel_iommu_domain)
+		return -ENODEV;
+
+	r = kvm_iommu_map_memslots(kvm);
+	if (r)
+		goto out_unmap;
+
+	intel_iommu_detach_dev(kvm->arch.intel_iommu_domain,
+			       pdev->bus->number, pdev->devfn);
+
+	r = intel_iommu_context_mapping(kvm->arch.intel_iommu_domain,
+					pdev);
+	if (r) {
+		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 r;
+}
+
+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 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));
+
+		/* detach kvm dmar domain */
+		intel_iommu_detach_dev(domain, entry->host_busnr,
+				       entry->host_devfn);
+	}
+	kvm_iommu_unmap_memslots(kvm);
+	intel_iommu_domain_exit(domain);
+	return 0;
+}
diff --git a/virt/kvm/vtd.c b/virt/kvm/vtd.c
deleted file mode 100644
index a770874..0000000
--- a/virt/kvm/vtd.c
+++ /dev/null
@@ -1,191 +0,0 @@
-/*
- * 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);
-static void kvm_iommu_put_pages(struct kvm *kvm,
-				gfn_t base_gfn, unsigned long npages);
-
-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, r = 0;
-	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)
-			continue;
-
-		pfn = gfn_to_pfn(kvm, gfn);
-		r = intel_iommu_page_mapping(domain,
-					     gfn_to_gpa(gfn),
-					     pfn_to_hpa(pfn),
-					     PAGE_SIZE,
-					     DMA_PTE_READ |
-					     DMA_PTE_WRITE);
-		if (r) {
-			printk(KERN_ERR "kvm_iommu_map_pages:"
-			       "iommu failed to map pfn=%lx\n", pfn);
-			goto unmap_pages;
-		}
-		gfn++;
-	}
-	return 0;
-
-unmap_pages:
-	kvm_iommu_put_pages(kvm, base_gfn, i);
-	return r;
-}
-
-static int kvm_iommu_map_memslots(struct kvm *kvm)
-{
-	int i, r;
-
-	down_read(&kvm->slots_lock);
-	for (i = 0; i < kvm->nmemslots; i++) {
-		r = kvm_iommu_map_pages(kvm, kvm->memslots[i].base_gfn,
-					kvm->memslots[i].npages);
-		if (r)
-			break;
-	}
-	up_read(&kvm->slots_lock);
-	return r;
-}
-
-int kvm_iommu_map_guest(struct kvm *kvm,
-			struct kvm_assigned_dev_kernel *assigned_dev)
-{
-	struct pci_dev *pdev = NULL;
-	int r;
-
-	if (!intel_iommu_found()) {
-		printk(KERN_ERR "%s: intel iommu not found\n", __func__);
-		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);
-	if (!kvm->arch.intel_iommu_domain)
-		return -ENODEV;
-
-	r = kvm_iommu_map_memslots(kvm);
-	if (r)
-		goto out_unmap;
-
-	intel_iommu_detach_dev(kvm->arch.intel_iommu_domain,
-			       pdev->bus->number, pdev->devfn);
-
-	r = intel_iommu_context_mapping(kvm->arch.intel_iommu_domain,
-					pdev);
-	if (r) {
-		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 r;
-}
-
-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 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));
-
-		/* detach kvm dmar domain */
-		intel_iommu_detach_dev(domain, entry->host_busnr,
-				       entry->host_devfn);
-	}
-	kvm_iommu_unmap_memslots(kvm);
-	intel_iommu_domain_exit(domain);
-	return 0;
-}
-- 
1.5.6.4

Re: [PATCH 1/9] KVM: rename vtd.c to iommu.c

[Avi Kivity]

Joerg Roedel wrote:
> Impace: file renamed
>
>   

"impact"?

> The code in the vtd.c file can be reused for other IOMMUs as well. So
> rename it to make it clear that it handle more than VT-d.
>
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  arch/ia64/kvm/Makefile |    2 +-
>  arch/x86/kvm/Makefile  |    2 +-
>  virt/kvm/iommu.c       |  191 ++++++++++++++++++++++++++++++++++++++++++++++++
>  virt/kvm/vtd.c         |  191 ------------------------------------------------
>   

Use the git -M switch, which show renames as renames instead of 
removes+adds.

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

Re: [PATCH 1/9] KVM: rename vtd.c to iommu.c

[Joerg Roedel]

On Thu, Nov 27, 2008 at 06:32:56PM +0200, Avi Kivity wrote:
> Joerg Roedel wrote:
> >Impace: file renamed
> >
> >  
> 
> "impact"?

Yeah, patches for x86 should have such a line in the description. I just
added it here too because of the many x86-patches I did in the last
time. Depending on the way this will be merged I can remove the line
from the description.

> 
> >The code in the vtd.c file can be reused for other IOMMUs as well. So
> >rename it to make it clear that it handle more than VT-d.
> >
> >Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> >---
> > arch/ia64/kvm/Makefile |    2 +-
> > arch/x86/kvm/Makefile  |    2 +-
> > virt/kvm/iommu.c       |  191 ++++++++++++++++++++++++++++++++++++++++++++++++
> > virt/kvm/vtd.c         |  191 ------------------------------------------------
> >  
> 
> Use the git -M switch, which show renames as renames instead of removes+adds.

Ok, will do this in the next post

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH 1/9] KVM: rename vtd.c to iommu.c

[Avi Kivity]

Joerg Roedel wrote:
> On Thu, Nov 27, 2008 at 06:32:56PM +0200, Avi Kivity wrote:
>   
>> Joerg Roedel wrote:
>>     
>>> Impace: file renamed
>>>
>>>  
>>>       
>> "impact"?
>>     
>
> Yeah, patches for x86 should have such a line in the description. I just
> added it here too because of the many x86-patches I did in the last
> time. Depending on the way this will be merged I can remove the line
> from the description.
>
>   

I meant, you have a typo.  Sorry for not being explicit.

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

[PATCH 2/9] introcude linux/iommu.h for an iommu api

[Joerg Roedel]

This patch introduces the API to abstract the exported VT-d functions
for KVM into a generic API. This way the AMD IOMMU implementation can
plug into this API later.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 include/linux/iommu.h |  100 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 100 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/iommu.h

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
new file mode 100644
index 0000000..6dbc279
--- /dev/null
+++ b/include/linux/iommu.h
@@ -0,0 +1,100 @@
+/*
+ * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
+ * Author: Joerg Roedel <joerg.roedel@amd.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that 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
+ */
+
+#ifndef __LINUX_IOMMU_H
+#define __LINUX_IOMMU_H
+
+struct device;
+
+struct iommu_domain {
+	void *priv;
+};
+
+struct iommu_ops {
+	int (*domain_init)(struct iommu_domain *domain, struct device *dev);
+	void (*domain_destroy)(struct iommu_domain *domain);
+	int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
+	void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
+	int (*map)(struct iommu_domain *domain, dma_addr_t iova,
+		   phys_addr_t paddr, size_t size, int prot);
+	phys_addr_t (*iova_to_phys)(struct iommu_domain *domain,
+				    dma_addr_t iova);
+};
+
+#ifdef CONFIG_IOMMU_API
+
+extern void register_iommu(struct iommu_ops *ops);
+extern bool iommu_found(void);
+extern struct iommu_domain *iommu_domain_alloc(struct device *dev);
+extern void iommu_domain_free(struct iommu_domain *domain);
+extern int iommu_attach_device(struct iommu_domain *domain,
+			       struct device *dev);
+extern void iommu_detach_device(struct iommu_domain *domain,
+				struct device *dev);
+extern int iommu_map_address(struct iommu_domain *domain, dma_addr_t iova,
+			     phys_addr_t paddr, size_t size, int prot);
+extern phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain,
+				      dma_addr_t iova);
+
+#else /* CONFIG_IOMMU_API */
+
+static inline void register_iommu(struct iommu_ops *ops)
+{
+}
+
+static inline bool iommu_found(void)
+{
+	return false;
+}
+
+static inline struct iommu_domain *iommu_domain_alloc(struct device *dev)
+{
+	return NULL;
+}
+
+static inline void iommu_domain_free(struct iommu_domain *domain)
+{
+}
+
+static inline int iommu_attach_device(struct iommu_domain *domain,
+				      struct device *dev)
+{
+	return -ENODEV;
+}
+
+static inline void iommu_detach_device(struct iommu_domain *domain,
+				       struct device *dev)
+{
+}
+
+static inline int iommu_map_address(struct iommu_domain *domain,
+				    dma_addr_t iova, phys_addr_t paddr,
+				    size_t size, int prot)
+{
+	return -ENODEV;
+}
+
+static inline phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain,
+					     dma_addr_t iova)
+{
+	return 0;
+}
+
+#endif /* CONFIG_IOMMU_API */
+
+#endif /* __LINUX_IOMMU_H */
-- 
1.5.6.4

RE: [PATCH 2/9] introcude linux/iommu.h for an iommu api

[Han, Weidong]

Joerg Roedel wrote:
> This patch introduces the API to abstract the exported VT-d functions
> for KVM into a generic API. This way the AMD IOMMU implementation can
> plug into this API later.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  include/linux/iommu.h |  100
>  +++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed,
>  100 insertions(+), 0 deletions(-) create mode 100644
> include/linux/iommu.h 
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> new file mode 100644
> index 0000000..6dbc279
> --- /dev/null
> +++ b/include/linux/iommu.h
> @@ -0,0 +1,100 @@
> +/*
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
> + * Author: Joerg Roedel <joerg.roedel@amd.com>
> + *
> + * This program is free software; you can redistribute it and/or
> modify it + * under the terms of the GNU General Public License
> version 2 as published + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that 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 + */
> +
> +#ifndef __LINUX_IOMMU_H
> +#define __LINUX_IOMMU_H
> +
> +struct device;
> +
> +struct iommu_domain {
> +	void *priv;
> +};
> +
> +struct iommu_ops {
> +	int (*domain_init)(struct iommu_domain *domain, struct device *dev);

needn't parameter dev, use attach_dev to assign dev to domain.

> +	void (*domain_destroy)(struct iommu_domain *domain);
> +	int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
> +	void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
> +	int (*map)(struct iommu_domain *domain, dma_addr_t iova,
> +		   phys_addr_t paddr, size_t size, int prot);

It maps/unmaps pages for iommu in kvm, so I think it's better change to 
	int (*map_pages)(struct iommu_domain *domain, unsigned long gfn, unsigned long pfn,
			  unsigned long npages, int prot);
and also add unmap op:
	int (*unmap_pages)(struct iommu_domain *domain, unsigned long gfn, unsigned long npages);

> +	phys_addr_t (*iova_to_phys)(struct iommu_domain *domain,
> +				    dma_addr_t iova);
> +};
> +
> +#ifdef CONFIG_IOMMU_API
> +
> +extern void register_iommu(struct iommu_ops *ops);
> +extern bool iommu_found(void);
> +extern struct iommu_domain *iommu_domain_alloc(struct device *dev);
> +extern void iommu_domain_free(struct iommu_domain *domain);
> +extern int iommu_attach_device(struct iommu_domain *domain,
> +			       struct device *dev);
> +extern void iommu_detach_device(struct iommu_domain *domain,
> +				struct device *dev);
> +extern int iommu_map_address(struct iommu_domain *domain, dma_addr_t
> iova, +			     phys_addr_t paddr, size_t size, int prot);

similarly:
extern int iommu_map_pages(struct iommu_domain *domain, unsigned long gfn, unsigned long pfn,
			        unsigned long npages, int prot);
extern int iommu_unmap_pages(struct iommu_domain *domain, unsigned long gfn, unsigned long npages);

Regards,
Weidong

[PATCH 3/9] add frontend implementation for the IOMMU API

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/base/iommu.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 94 insertions(+), 0 deletions(-)
 create mode 100644 drivers/base/iommu.c

diff --git a/drivers/base/iommu.c b/drivers/base/iommu.c
new file mode 100644
index 0000000..7250b9c
--- /dev/null
+++ b/drivers/base/iommu.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
+ * Author: Joerg Roedel <joerg.roedel@amd.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that 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
+ */
+
+#include <linux/bug.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/iommu.h>
+
+static struct iommu_ops *iommu_ops;
+
+void register_iommu(struct iommu_ops *ops)
+{
+	if (iommu_ops)
+		BUG();
+
+	iommu_ops = ops;
+}
+
+bool iommu_found()
+{
+	return iommu_ops != NULL;
+}
+EXPORT_SYMBOL_GPL(iommu_found);
+
+struct iommu_domain *iommu_domain_alloc(struct device *dev)
+{
+	struct iommu_domain *domain;
+	int ret;
+
+	domain = kmalloc(sizeof(*domain), GFP_KERNEL);
+	if (!domain)
+		return NULL;
+
+	ret = iommu_ops->domain_init(domain, dev);
+	if (ret)
+		goto out_free;
+
+	return domain;
+
+out_free:
+	kfree(domain);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(iommu_domain_alloc);
+
+void iommu_domain_free(struct iommu_domain *domain)
+{
+	iommu_ops->domain_destroy(domain);
+	kfree(domain);
+}
+EXPORT_SYMBOL_GPL(iommu_domain_free);
+
+int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
+{
+	return iommu_ops->attach_dev(domain, dev);
+}
+EXPORT_SYMBOL_GPL(iommu_attach_device);
+
+void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
+{
+	iommu_ops->detach_dev(domain, dev);
+}
+EXPORT_SYMBOL_GPL(iommu_detach_device);
+
+int iommu_map_address(struct iommu_domain *domain,
+		      dma_addr_t iova, phys_addr_t paddr,
+		      size_t size, int prot)
+{
+	return iommu_ops->map(domain, iova, paddr, size, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_address);
+
+phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain,
+			       dma_addr_t iova)
+{
+	return iommu_ops->iova_to_phys(domain, iova);
+}
+EXPORT_SYMBOL_GPL(iommu_iova_to_phys);
-- 
1.5.6.4

RE: [PATCH 3/9] add frontend implementation for the IOMMU API

[Han, Weidong]

Joerg Roedel wrote:
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  drivers/base/iommu.c |   94
>  ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed,
>  94 insertions(+), 0 deletions(-) create mode 100644
> drivers/base/iommu.c 
> 
> diff --git a/drivers/base/iommu.c b/drivers/base/iommu.c
> new file mode 100644
> index 0000000..7250b9c
> --- /dev/null
> +++ b/drivers/base/iommu.c
> @@ -0,0 +1,94 @@
> +/*
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
> + * Author: Joerg Roedel <joerg.roedel@amd.com>
> + *
> + * This program is free software; you can redistribute it and/or
> modify it + * under the terms of the GNU General Public License
> version 2 as published + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that 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 + */
> +
> +#include <linux/bug.h>
> +#include <linux/types.h>
> +#include <linux/errno.h>
> +#include <linux/iommu.h>
> +
> +static struct iommu_ops *iommu_ops;
> +
> +void register_iommu(struct iommu_ops *ops)
> +{
> +	if (iommu_ops)
> +		BUG();
> +
> +	iommu_ops = ops;
> +}
> +
> +bool iommu_found()
> +{
> +	return iommu_ops != NULL;
> +}
> +EXPORT_SYMBOL_GPL(iommu_found);
> +
> +struct iommu_domain *iommu_domain_alloc(struct device *dev)
> +{
> +	struct iommu_domain *domain;
> +	int ret;
> +
> +	domain = kmalloc(sizeof(*domain), GFP_KERNEL);
> +	if (!domain)
> +		return NULL;
> +
> +	ret = iommu_ops->domain_init(domain, dev);
> +	if (ret)
> +		goto out_free;
> +
> +	return domain;
> +
> +out_free:
> +	kfree(domain);
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(iommu_domain_alloc);

remove the parameter dev.

> +
> +void iommu_domain_free(struct iommu_domain *domain)
> +{
> +	iommu_ops->domain_destroy(domain);
> +	kfree(domain);
> +}
> +EXPORT_SYMBOL_GPL(iommu_domain_free);
> +
> +int iommu_attach_device(struct iommu_domain *domain, struct device
> *dev) +{
> +	return iommu_ops->attach_dev(domain, dev);
> +}
> +EXPORT_SYMBOL_GPL(iommu_attach_device);
> +
> +void iommu_detach_device(struct iommu_domain *domain, struct device
> *dev) +{
> +	iommu_ops->detach_dev(domain, dev);
> +}
> +EXPORT_SYMBOL_GPL(iommu_detach_device);
> +
> +int iommu_map_address(struct iommu_domain *domain,
> +		      dma_addr_t iova, phys_addr_t paddr,
> +		      size_t size, int prot)
> +{
> +	return iommu_ops->map(domain, iova, paddr, size, prot);
> +}
> +EXPORT_SYMBOL_GPL(iommu_map_address);

change to:
int iommu_map_pages(struct iommu_domain *domain, unsigned long gfn,
		unsigned long pfn, unsigned long npages, int prot)
{
	return iommu_ops->map(domain, gfn, pfn, npages, prot);
}
EXPORT_SYMBOL_GPL(iommu_map_pages);

int iommu_unmap_pages(struct iommu_domain *domain, unsigned long gfn, unsigned long npages)
{
	return iommu_ops->map(domain, gfn, npages);
}
EXPORT_SYMBOL_GPL(iommu_unmap_pages);

Regards,
Weidong

> +
> +phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain,
> +			       dma_addr_t iova)
> +{
> +	return iommu_ops->iova_to_phys(domain, iova);
> +}
> +EXPORT_SYMBOL_GPL(iommu_iova_to_phys);

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

['Joerg Roedel']

On Fri, Nov 28, 2008 at 10:50:36AM +0800, Han, Weidong wrote:
> Joerg Roedel wrote:
> > +struct iommu_domain *iommu_domain_alloc(struct device *dev)
> > +{
> > +	struct iommu_domain *domain;
> > +	int ret;
> > +
> > +	domain = kmalloc(sizeof(*domain), GFP_KERNEL);
> > +	if (!domain)
> > +		return NULL;
> > +
> > +	ret = iommu_ops->domain_init(domain, dev);
> > +	if (ret)
> > +		goto out_free;
> > +
> > +	return domain;
> > +
> > +out_free:
> > +	kfree(domain);
> > +
> > +	return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_domain_alloc);
> 
> remove the parameter dev.

[x] Done.

> > +
> > +void iommu_domain_free(struct iommu_domain *domain)
> > +{
> > +	iommu_ops->domain_destroy(domain);
> > +	kfree(domain);
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_domain_free);
> > +
> > +int iommu_attach_device(struct iommu_domain *domain, struct device
> > *dev) +{
> > +	return iommu_ops->attach_dev(domain, dev);
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_attach_device);
> > +
> > +void iommu_detach_device(struct iommu_domain *domain, struct device
> > *dev) +{
> > +	iommu_ops->detach_dev(domain, dev);
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_detach_device);
> > +
> > +int iommu_map_address(struct iommu_domain *domain,
> > +		      dma_addr_t iova, phys_addr_t paddr,
> > +		      size_t size, int prot)
> > +{
> > +	return iommu_ops->map(domain, iova, paddr, size, prot);
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_map_address);
> 
> change to:
> int iommu_map_pages(struct iommu_domain *domain, unsigned long gfn,
> 		unsigned long pfn, unsigned long npages, int prot)
> {
> 	return iommu_ops->map(domain, gfn, pfn, npages, prot);
> }
> EXPORT_SYMBOL_GPL(iommu_map_pages);
> 
> int iommu_unmap_pages(struct iommu_domain *domain, unsigned long gfn, unsigned long npages)
> {
> 	return iommu_ops->map(domain, gfn, npages);
> }
> EXPORT_SYMBOL_GPL(iommu_unmap_pages);

Ok, I added the unmap function. But I think this API should work with
addresses instead of page numbers. This way the IO page size is
transparent for the user.

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[FUJITA Tomonori]

On Thu, 27 Nov 2008 16:40:48 +0100
Joerg Roedel <joerg.roedel@amd.com> wrote:

> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  drivers/base/iommu.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 94 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/base/iommu.c
> 
> diff --git a/drivers/base/iommu.c b/drivers/base/iommu.c
> new file mode 100644
> index 0000000..7250b9c
> --- /dev/null
> +++ b/drivers/base/iommu.c

Hmm, why is this at drivers/base/? Anyone except for kvm could use
this? If so, under virt/ is more appropriate?

The majority of the names (include/linux/iommu.h, iommu.c, iommu_ops,
etc) looks too generic? We already have lots of similar things
(e.g. arch/{x86,ia64}/asm/iommu.h, several archs' iommu.c, etc). Such
names are expected to be used by all the IOMMUs.

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[Joerg Roedel]

On Fri, Nov 28, 2008 at 06:40:41PM +0900, FUJITA Tomonori wrote:
> On Thu, 27 Nov 2008 16:40:48 +0100
> Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > ---
> >  drivers/base/iommu.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 94 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/base/iommu.c
> > 
> > diff --git a/drivers/base/iommu.c b/drivers/base/iommu.c
> > new file mode 100644
> > index 0000000..7250b9c
> > --- /dev/null
> > +++ b/drivers/base/iommu.c
> 
> Hmm, why is this at drivers/base/? Anyone except for kvm could use
> this? If so, under virt/ is more appropriate?

I don't see a reason why this should be KVM specific. KVM is the only
user for now. But it can be used for i.e. UIO too. Or in drivers to
speed up devices which have bad performance when they do scather gather
IO.

> The majority of the names (include/linux/iommu.h, iommu.c, iommu_ops,
> etc) looks too generic? We already have lots of similar things
> (e.g. arch/{x86,ia64}/asm/iommu.h, several archs' iommu.c, etc). Such
> names are expected to be used by all the IOMMUs.

The API is already useful for more than KVM. I also plan to extend it to
support more types of IOMMUs than VT-d and AMD IOMMU in the future. But
these changes are more intrusive than this patchset and need more
discussion. I prefer to do small steps into this direction.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[FUJITA Tomonori]

On Fri, 28 Nov 2008 12:31:29 +0100
Joerg Roedel <joerg.roedel@amd.com> wrote:

> On Fri, Nov 28, 2008 at 06:40:41PM +0900, FUJITA Tomonori wrote:
> > On Thu, 27 Nov 2008 16:40:48 +0100
> > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > 
> > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > ---
> > >  drivers/base/iommu.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 files changed, 94 insertions(+), 0 deletions(-)
> > >  create mode 100644 drivers/base/iommu.c
> > > 
> > > diff --git a/drivers/base/iommu.c b/drivers/base/iommu.c
> > > new file mode 100644
> > > index 0000000..7250b9c
> > > --- /dev/null
> > > +++ b/drivers/base/iommu.c
> > 
> > Hmm, why is this at drivers/base/? Anyone except for kvm could use
> > this? If so, under virt/ is more appropriate?
> 
> I don't see a reason why this should be KVM specific. KVM is the only
> user for now. But it can be used for i.e. UIO too. Or in drivers to
> speed up devices which have bad performance when they do scather gather
> IO.

If there are some except for kvm that could use this, it should be
fine, I guess.

Can you add such information (e.g. who could use this) to the patch
description? It should be in the git log if the patch is merged.


> > The majority of the names (include/linux/iommu.h, iommu.c, iommu_ops,
> > etc) looks too generic? We already have lots of similar things
> > (e.g. arch/{x86,ia64}/asm/iommu.h, several archs' iommu.c, etc). Such
> > names are expected to be used by all the IOMMUs.
> 
> The API is already useful for more than KVM. I also plan to extend it to
> support more types of IOMMUs than VT-d and AMD IOMMU in the future. But
> these changes are more intrusive than this patchset and need more
> discussion. I prefer to do small steps into this direction.

Can you be more specific? What IOMMU could use this? For example, how
GART can use this? I think that people expect the name 'struct
iommu_ops' to be an abstract for all the IOMMUs (or the majority at
least). If this works like that, the name is a good choice, I think.

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[Joerg Roedel]

On Mon, Dec 01, 2008 at 05:38:11PM +0900, FUJITA Tomonori wrote:
> On Fri, 28 Nov 2008 12:31:29 +0100
> Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > On Fri, Nov 28, 2008 at 06:40:41PM +0900, FUJITA Tomonori wrote:
> > > On Thu, 27 Nov 2008 16:40:48 +0100
> > > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > 
> > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > > ---
> > > >  drivers/base/iommu.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 files changed, 94 insertions(+), 0 deletions(-)
> > > >  create mode 100644 drivers/base/iommu.c
> > > > 
> > > > diff --git a/drivers/base/iommu.c b/drivers/base/iommu.c
> > > > new file mode 100644
> > > > index 0000000..7250b9c
> > > > --- /dev/null
> > > > +++ b/drivers/base/iommu.c
> > > 
> > > Hmm, why is this at drivers/base/? Anyone except for kvm could use
> > > this? If so, under virt/ is more appropriate?
> > 
> > I don't see a reason why this should be KVM specific. KVM is the only
> > user for now. But it can be used for i.e. UIO too. Or in drivers to
> > speed up devices which have bad performance when they do scather gather
> > IO.
> 
> If there are some except for kvm that could use this, it should be
> fine, I guess.
> 
> Can you add such information (e.g. who could use this) to the patch
> description? It should be in the git log if the patch is merged.

Ok, I will add it.

> > > The majority of the names (include/linux/iommu.h, iommu.c, iommu_ops,
> > > etc) looks too generic? We already have lots of similar things
> > > (e.g. arch/{x86,ia64}/asm/iommu.h, several archs' iommu.c, etc). Such
> > > names are expected to be used by all the IOMMUs.
> > 
> > The API is already useful for more than KVM. I also plan to extend it to
> > support more types of IOMMUs than VT-d and AMD IOMMU in the future. But
> > these changes are more intrusive than this patchset and need more
> > discussion. I prefer to do small steps into this direction.
> 
> Can you be more specific? What IOMMU could use this? For example, how
> GART can use this? I think that people expect the name 'struct
> iommu_ops' to be an abstract for all the IOMMUs (or the majority at
> least). If this works like that, the name is a good choice, I think.

GART can't use exactly this. But with some extensions we can make it
useful for GART and GART-like IOMMUs too. For example we can emulate
domains in GART by partitioning the GART aperture space.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[Muli Ben-Yehuda]

On Mon, Dec 01, 2008 at 01:00:26PM +0100, Joerg Roedel wrote:

> > > > The majority of the names (include/linux/iommu.h, iommu.c,
> > > > iommu_ops, etc) looks too generic? We already have lots of
> > > > similar things (e.g. arch/{x86,ia64}/asm/iommu.h, several
> > > > archs' iommu.c, etc). Such names are expected to be used by
> > > > all the IOMMUs.
> > > 
> > > The API is already useful for more than KVM. I also plan to
> > > extend it to support more types of IOMMUs than VT-d and AMD
> > > IOMMU in the future. But these changes are more intrusive than
> > > this patchset and need more discussion. I prefer to do small
> > > steps into this direction.
> > 
> > Can you be more specific? What IOMMU could use this? For example,
> > how GART can use this? I think that people expect the name 'struct
> > iommu_ops' to be an abstract for all the IOMMUs (or the majority
> > at least). If this works like that, the name is a good choice, I
> > think.
> 
> GART can't use exactly this. But with some extensions we can make it
> useful for GART and GART-like IOMMUs too. For example we can emulate
> domains in GART by partitioning the GART aperture space.

That would only work with a pvdma API, since GART doesn't support
multiple address spaces, and you don't get the isolation properties of
a real IOMMU, so... why would you want to do that?

Cheers,
Muli
-- 
The First Workshop on I/O Virtualization (WIOV '08)
Dec 2008, San Diego, CA, http://www.usenix.org/wiov08/
                       <->
SYSTOR 2009---The Israeli Experimental Systems Conference
http://www.haifa.il.ibm.com/conferences/systor2009/

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[Joerg Roedel]

On Mon, Dec 01, 2008 at 03:02:09PM +0200, Muli Ben-Yehuda wrote:
> On Mon, Dec 01, 2008 at 01:00:26PM +0100, Joerg Roedel wrote:
> 
> > > > > The majority of the names (include/linux/iommu.h, iommu.c,
> > > > > iommu_ops, etc) looks too generic? We already have lots of
> > > > > similar things (e.g. arch/{x86,ia64}/asm/iommu.h, several
> > > > > archs' iommu.c, etc). Such names are expected to be used by
> > > > > all the IOMMUs.
> > > > 
> > > > The API is already useful for more than KVM. I also plan to
> > > > extend it to support more types of IOMMUs than VT-d and AMD
> > > > IOMMU in the future. But these changes are more intrusive than
> > > > this patchset and need more discussion. I prefer to do small
> > > > steps into this direction.
> > > 
> > > Can you be more specific? What IOMMU could use this? For example,
> > > how GART can use this? I think that people expect the name 'struct
> > > iommu_ops' to be an abstract for all the IOMMUs (or the majority
> > > at least). If this works like that, the name is a good choice, I
> > > think.
> > 
> > GART can't use exactly this. But with some extensions we can make it
> > useful for GART and GART-like IOMMUs too. For example we can emulate
> > domains in GART by partitioning the GART aperture space.
> 
> That would only work with a pvdma API, since GART doesn't support
> multiple address spaces, and you don't get the isolation properties of
> a real IOMMU, so... why would you want to do that?

Yes, this can not be used for not-pv device passthrough. But I think it
can speed up the pvdma case. Beside that I can be used for UIO and
devices which perform bad with sg.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[FUJITA Tomonori]

On Mon, 1 Dec 2008 15:02:09 +0200
Muli Ben-Yehuda <muli@il.ibm.com> wrote:

> On Mon, Dec 01, 2008 at 01:00:26PM +0100, Joerg Roedel wrote:
> 
> > > > > The majority of the names (include/linux/iommu.h, iommu.c,
> > > > > iommu_ops, etc) looks too generic? We already have lots of
> > > > > similar things (e.g. arch/{x86,ia64}/asm/iommu.h, several
> > > > > archs' iommu.c, etc). Such names are expected to be used by
> > > > > all the IOMMUs.
> > > > 
> > > > The API is already useful for more than KVM. I also plan to
> > > > extend it to support more types of IOMMUs than VT-d and AMD
> > > > IOMMU in the future. But these changes are more intrusive than
> > > > this patchset and need more discussion. I prefer to do small
> > > > steps into this direction.
> > > 
> > > Can you be more specific? What IOMMU could use this? For example,
> > > how GART can use this? I think that people expect the name 'struct
> > > iommu_ops' to be an abstract for all the IOMMUs (or the majority
> > > at least). If this works like that, the name is a good choice, I
> > > think.
> > 
> > GART can't use exactly this. But with some extensions we can make it
> > useful for GART and GART-like IOMMUs too. For example we can emulate
> > domains in GART by partitioning the GART aperture space.
> 
> That would only work with a pvdma API, since GART doesn't support
> multiple address spaces, and you don't get the isolation properties of
> a real IOMMU, so... why would you want to do that?

If this works for only IOMMUs that support kinda domain concept, then
I think that a name like iommu_domain_ops is more appropriate.

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[Joerg Roedel]

On Mon, Dec 01, 2008 at 11:18:39PM +0900, FUJITA Tomonori wrote:
> On Mon, 1 Dec 2008 15:02:09 +0200
> Muli Ben-Yehuda <muli@il.ibm.com> wrote:
> 
> > On Mon, Dec 01, 2008 at 01:00:26PM +0100, Joerg Roedel wrote:
> > 
> > > > > > The majority of the names (include/linux/iommu.h, iommu.c,
> > > > > > iommu_ops, etc) looks too generic? We already have lots of
> > > > > > similar things (e.g. arch/{x86,ia64}/asm/iommu.h, several
> > > > > > archs' iommu.c, etc). Such names are expected to be used by
> > > > > > all the IOMMUs.
> > > > > 
> > > > > The API is already useful for more than KVM. I also plan to
> > > > > extend it to support more types of IOMMUs than VT-d and AMD
> > > > > IOMMU in the future. But these changes are more intrusive than
> > > > > this patchset and need more discussion. I prefer to do small
> > > > > steps into this direction.
> > > > 
> > > > Can you be more specific? What IOMMU could use this? For example,
> > > > how GART can use this? I think that people expect the name 'struct
> > > > iommu_ops' to be an abstract for all the IOMMUs (or the majority
> > > > at least). If this works like that, the name is a good choice, I
> > > > think.
> > > 
> > > GART can't use exactly this. But with some extensions we can make it
> > > useful for GART and GART-like IOMMUs too. For example we can emulate
> > > domains in GART by partitioning the GART aperture space.
> > 
> > That would only work with a pvdma API, since GART doesn't support
> > multiple address spaces, and you don't get the isolation properties of
> > a real IOMMU, so... why would you want to do that?
> 
> If this works for only IOMMUs that support kinda domain concept, then
> I think that a name like iommu_domain_ops is more appropriate.

Hmm, is there any hardware IOMMU with which we can't emulate domains by
partitioning the IO address space? This concept works for GART and
Calgary.

Joerg

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[Avi Kivity]

Joerg Roedel wrote:
> Hmm, is there any hardware IOMMU with which we can't emulate domains by
> partitioning the IO address space? This concept works for GART and
> Calgary.
>
>   

Is partitioning secure?  Domain X's user could program its hardware to 
dma to domain Y's addresses, zapping away Domain Y's user's memory.

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

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[Joerg Roedel]

On Mon, Dec 01, 2008 at 04:33:11PM +0200, Avi Kivity wrote:
> Joerg Roedel wrote:
> > Hmm, is there any hardware IOMMU with which we can't emulate domains by
> > partitioning the IO address space? This concept works for GART and
> > Calgary.
> >
> >   
> 
> Is partitioning secure?  Domain X's user could program its hardware to 
> dma to domain Y's addresses, zapping away Domain Y's user's memory.

No its not secure. But this problem exists with pv-dma without iommu
too.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[FUJITA Tomonori]

On Mon, 01 Dec 2008 16:33:11 +0200
Avi Kivity <avi@redhat.com> wrote:

> Joerg Roedel wrote:
> > Hmm, is there any hardware IOMMU with which we can't emulate domains by
> > partitioning the IO address space? This concept works for GART and
> > Calgary.
> >
> >   
> 
> Is partitioning secure?  Domain X's user could program its hardware to 
> dma to domain Y's addresses, zapping away Domain Y's user's memory.

It can't be secure. So what's the point to emulate the domain
partitioning in many traditional hardware IOMMUs that doesn't support
it.

The emulated domain support with the DMA mapping debugging feature
might be useful to debug drivers but it doesn't mean that we need to
add the emulated domain support to every hardware IOMMU. If you add it
to swiotlb, everyone can enjoy the debugging.

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[Joerg Roedel]

On Tue, Dec 02, 2008 at 12:58:29AM +0900, FUJITA Tomonori wrote:
> On Mon, 01 Dec 2008 16:33:11 +0200
> Avi Kivity <avi@redhat.com> wrote:
> 
> > Joerg Roedel wrote:
> > > Hmm, is there any hardware IOMMU with which we can't emulate domains by
> > > partitioning the IO address space? This concept works for GART and
> > > Calgary.
> > >
> > >   
> > 
> > Is partitioning secure?  Domain X's user could program its hardware to 
> > dma to domain Y's addresses, zapping away Domain Y's user's memory.
> 
> It can't be secure. So what's the point to emulate the domain
> partitioning in many traditional hardware IOMMUs that doesn't support
> it.

Its a generic way to make non-contiguous host memory io-contiguous. I
already pointed out some potential users for this.

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH 3/9] add frontend implementation for the IOMMU API

[Joerg Roedel]

On Tue, Dec 02, 2008 at 12:58:29AM +0900, FUJITA Tomonori wrote:
> On Mon, 01 Dec 2008 16:33:11 +0200
> Avi Kivity <avi@redhat.com> wrote:
> 
> > Joerg Roedel wrote:
> > > Hmm, is there any hardware IOMMU with which we can't emulate domains by
> > > partitioning the IO address space? This concept works for GART and
> > > Calgary.
> > >
> > >   
> > 
> > Is partitioning secure?  Domain X's user could program its hardware to 
> > dma to domain Y's addresses, zapping away Domain Y's user's memory.
> 
> It can't be secure. So what's the point to emulate the domain
> partitioning in many traditional hardware IOMMUs that doesn't support
> it.

Btw, if you use the k8-agp driver the GART space is already partitioned
today. So this concept is not entirely new.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

[PATCH 4/9] select IOMMU_API when DMAR and/or AMD_IOMMU is selected

[Joerg Roedel]

These two IOMMUs can implement the current version of this API. So
select the API if one or both of these IOMMU drivers is selected.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/ia64/Kconfig     |    3 +++
 arch/x86/Kconfig      |    3 +++
 drivers/base/Makefile |    1 +
 3 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 6bd91ed..6a7b0c9 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -687,3 +687,6 @@ config IRQ_PER_CPU
 
 config IOMMU_HELPER
 	def_bool (IA64_HP_ZX1 || IA64_HP_ZX1_SWIOTLB || IA64_GENERIC || SWIOTLB)
+
+config IOMMU_API
+	def_bool (DMAR)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ac22bb7..b9f7187 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -580,6 +580,9 @@ config SWIOTLB
 config IOMMU_HELPER
 	def_bool (CALGARY_IOMMU || GART_IOMMU || SWIOTLB || AMD_IOMMU)
 
+config IOMMU_API
+	def_bool (AMD_IOMMU || DMAR)
+
 config MAXSMP
 	bool "Configure Maximum number of SMP Processors and NUMA Nodes"
 	depends on X86_64 && SMP && BROKEN
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index c666373..b5b8ba5 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
 obj-$(CONFIG_NUMA)	+= node.o
 obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o
 obj-$(CONFIG_SMP)	+= topology.o
+obj-$(CONFIG_IOMMU_API) += iommu.o
 ifeq ($(CONFIG_SYSFS),y)
 obj-$(CONFIG_MODULES)	+= module.o
 endif
-- 
1.5.6.4

[PATCH 5/9] VT-d: add domain init and destroy functions for IOMMU API

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/pci/intel-iommu.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 5c8baa4..b958b6f 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -35,6 +35,7 @@
 #include <linux/mempool.h>
 #include <linux/timer.h>
 #include <linux/iova.h>
+#include <linux/iommu.h>
 #include <linux/intel-iommu.h>
 #include <asm/cacheflush.h>
 #include <asm/iommu.h>
@@ -2436,3 +2437,26 @@ u64 intel_iommu_iova_to_pfn(struct dmar_domain *domain, u64 iova)
 	return pfn >> VTD_PAGE_SHIFT;
 }
 EXPORT_SYMBOL_GPL(intel_iommu_iova_to_pfn);
+
+static int intel_iommu_domain_init(struct iommu_domain *domain,
+				   struct device *dev)
+{
+	struct dmar_domain *dmar_domain;
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	if (!pdev)
+		return -EINVAL;
+
+	dmar_domain = intel_iommu_domain_alloc(pdev);
+	domain->priv = dmar_domain;
+
+	return 0;
+}
+
+static void intel_iommu_domain_destroy(struct iommu_domain *domain)
+{
+	struct dmar_domain *dmar_domain = domain->priv;
+
+	intel_iommu_domain_exit(dmar_domain);
+	domain->priv = NULL;
+}
-- 
1.5.6.4

[PATCH 6/9] VT-d: add device attach and detach functions for IOMMU API

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/pci/intel-iommu.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index b958b6f..a90c832 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -2460,3 +2460,35 @@ static void intel_iommu_domain_destroy(struct iommu_domain *domain)
 	intel_iommu_domain_exit(dmar_domain);
 	domain->priv = NULL;
 }
+
+static int intel_iommu_attach_device(struct iommu_domain *domain,
+				     struct device *dev)
+{
+	struct dmar_domain *dmar_domain;
+	struct pci_dev *pdev;
+
+	dmar_domain = domain->priv;
+	pdev = to_pci_dev(dev);
+
+	if (!pdev)
+		return -EINVAL;
+
+	intel_iommu_context_mapping(dmar_domain, pdev);
+
+	return 0;
+}
+
+static void intel_iommu_detach_device(struct iommu_domain *domain,
+				      struct device *dev)
+{
+	struct dmar_domain *dmar_domain;
+	struct pci_dev *pdev;
+
+	dmar_domain = domain->priv;
+	pdev = to_pci_dev(dev);
+
+	if (!pdev)
+		return;
+
+	intel_iommu_detach_dev(dmar_domain, pdev->bus->number, pdev->devfn);
+}
-- 
1.5.6.4

[PATCH 7/9] VT-d: add domain map and iova_to_phys functions for IOMMU API

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/pci/intel-iommu.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index a90c832..8fa0269 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -2492,3 +2492,25 @@ static void intel_iommu_detach_device(struct iommu_domain *domain,
 
 	intel_iommu_detach_dev(dmar_domain, pdev->bus->number, pdev->devfn);
 }
+
+static int intel_iommu_map(struct iommu_domain *domain, dma_addr_t iova,
+			   phys_addr_t paddr, size_t size, int prot)
+{
+	struct dmar_domain *dmar_domain = domain->priv;
+
+	return intel_iommu_page_mapping(dmar_domain, iova, paddr, size, prot);
+}
+
+static phys_addr_t intel_iova_to_phys(struct iommu_domain *domain,
+				      dma_addr_t iova)
+{
+	struct dmar_domain *dmar_domain = domain->priv;
+	u64 pfn;
+	u64 offset = iova & ~PAGE_MASK;
+
+	pfn = intel_iommu_iova_to_pfn(dmar_domain, iova);
+	if (!pfn)
+		return pfn;
+
+	return (pfn << PAGE_SHIFT) & offset;
+}
-- 
1.5.6.4

[PATCH 8/9] VT-d: register functions for the IOMMU API

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 drivers/pci/intel-iommu.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 8fa0269..71b4d2f 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -57,6 +57,7 @@
 
 
 static void flush_unmaps_timeout(unsigned long data);
+static struct iommu_ops intel_iommu_ops;
 
 DEFINE_TIMER(unmap_timer,  flush_unmaps_timeout, 0, 0);
 
@@ -2326,6 +2327,9 @@ int __init intel_iommu_init(void)
 	init_timer(&unmap_timer);
 	force_iommu = 1;
 	dma_ops = &intel_dma_ops;
+
+	register_iommu(&intel_iommu_ops);
+
 	return 0;
 }
 
@@ -2514,3 +2518,12 @@ static phys_addr_t intel_iova_to_phys(struct iommu_domain *domain,
 
 	return (pfn << PAGE_SHIFT) & offset;
 }
+
+static struct iommu_ops intel_iommu_ops = {
+	.domain_init	= intel_iommu_domain_init,
+	.domain_destroy	= intel_iommu_domain_destroy,
+	.attach_dev	= intel_iommu_attach_device,
+	.detach_dev	= intel_iommu_detach_device,
+	.map		= intel_iommu_map,
+	.iova_to_phys	= intel_iova_to_phys,
+};
-- 
1.5.6.4

RE: [PATCH 8/9] VT-d: register functions for the IOMMU API

[Han, Weidong]

Joerg Roedel wrote:
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  drivers/pci/intel-iommu.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index 8fa0269..71b4d2f 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -57,6 +57,7 @@
> 
> 
>  static void flush_unmaps_timeout(unsigned long data);
> +static struct iommu_ops intel_iommu_ops;
> 
>  DEFINE_TIMER(unmap_timer,  flush_unmaps_timeout, 0, 0);
> 
> @@ -2326,6 +2327,9 @@ int __init intel_iommu_init(void)
>  	init_timer(&unmap_timer);
>  	force_iommu = 1;
>  	dma_ops = &intel_dma_ops;
> +
> +	register_iommu(&intel_iommu_ops);
> +
>  	return 0;
>  }
> 
> @@ -2514,3 +2518,12 @@ static phys_addr_t intel_iova_to_phys(struct
> iommu_domain *domain, 
> 
>  	return (pfn << PAGE_SHIFT) & offset;
>  }
> +
> +static struct iommu_ops intel_iommu_ops = {
> +	.domain_init	= intel_iommu_domain_init,
> +	.domain_destroy	= intel_iommu_domain_destroy,
> +	.attach_dev	= intel_iommu_attach_device,
> +	.detach_dev	= intel_iommu_detach_device,
> +	.map		= intel_iommu_map,

change to:
	.map_pages	= intel_iommu_map_pages,
	.unmap_pages	= intel_iommu_unmap_pages,

> +	.iova_to_phys	= intel_iova_to_phys,
> +};

should remove old kvm VT-d API declarations in include/linux/intel-iommu.h, and don't export them. In fact, should remove old kvm VT-d APIs, and move their code to corresponding new APIs. I can help you to do this if need.

Regards,
Weidong

[PATCH 9/9] KVM: change KVM iommu.c to use IOMMU API

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_host.h |    3 +-
 virt/kvm/iommu.c                |   66 ++++++++++++++++++++-------------------
 2 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f58f7eb..77f4afa 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -14,6 +14,7 @@
 #include <linux/types.h>
 #include <linux/mm.h>
 #include <linux/mmu_notifier.h>
+#include <linux/iommu.h>
 
 #include <linux/kvm.h>
 #include <linux/kvm_para.h>
@@ -356,7 +357,7 @@ struct kvm_arch{
 	 */
 	struct list_head active_mmu_pages;
 	struct list_head assigned_dev_head;
-	struct dmar_domain *intel_iommu_domain;
+	struct iommu_domain *iommu_domain;
 	struct kvm_pic *vpic;
 	struct kvm_ioapic *vioapic;
 	struct kvm_pit *vpit;
diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index a770874..48d1e5e 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -16,15 +16,18 @@
  *
  * Copyright (C) 2006-2008 Intel Corporation
  * Copyright IBM Corporation, 2008
+ * Copyright (C) 2008 Advanced Micro Devices, Inc.
  * Author: Allen M. Kay <allen.m.kay@intel.com>
  * Author: Weidong Han <weidong.han@intel.com>
  * Author: Ben-Ami Yassour <benami@il.ibm.com>
+ * Author: Joerg Roedel <joerg.roedel@amd.com>
  */
 
 #include <linux/list.h>
 #include <linux/kvm_host.h>
 #include <linux/pci.h>
 #include <linux/dmar.h>
+#include <linux/iommu.h>
 #include <linux/intel-iommu.h>
 
 static int kvm_iommu_unmap_memslots(struct kvm *kvm);
@@ -36,8 +39,9 @@ int kvm_iommu_map_pages(struct kvm *kvm,
 {
 	gfn_t gfn = base_gfn;
 	pfn_t pfn;
+	phys_addr_t phys;
 	int i, r = 0;
-	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+	struct iommu_domain *domain = kvm->arch.iommu_domain;
 
 	/* check if iommu exists and in use */
 	if (!domain)
@@ -45,18 +49,16 @@ int kvm_iommu_map_pages(struct kvm *kvm,
 
 	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)
+		phys = iommu_iova_to_phys(domain, gfn_to_gpa(gfn));
+		if (phys)
 			continue;
 
 		pfn = gfn_to_pfn(kvm, gfn);
-		r = intel_iommu_page_mapping(domain,
-					     gfn_to_gpa(gfn),
-					     pfn_to_hpa(pfn),
-					     PAGE_SIZE,
-					     DMA_PTE_READ |
-					     DMA_PTE_WRITE);
+		r = iommu_map_address(domain,
+				      gfn_to_gpa(gfn),
+				      pfn_to_hpa(pfn),
+				      PAGE_SIZE,
+				      DMA_PTE_READ | DMA_PTE_WRITE);
 		if (r) {
 			printk(KERN_ERR "kvm_iommu_map_pages:"
 			       "iommu failed to map pfn=%lx\n", pfn);
@@ -92,12 +94,12 @@ int kvm_iommu_map_guest(struct kvm *kvm,
 	struct pci_dev *pdev = NULL;
 	int r;
 
-	if (!intel_iommu_found()) {
-		printk(KERN_ERR "%s: intel iommu not found\n", __func__);
+	if (!iommu_found()) {
+		printk(KERN_ERR "%s: No IOMMU found\n", __func__);
 		return -ENODEV;
 	}
 
-	printk(KERN_DEBUG "VT-d direct map: host bdf = %x:%x:%x\n",
+	printk(KERN_DEBUG "KVM IOMMU direct map: host bdf = %x:%x:%x\n",
 	       assigned_dev->host_busnr,
 	       PCI_SLOT(assigned_dev->host_devfn),
 	       PCI_FUNC(assigned_dev->host_devfn));
@@ -105,26 +107,24 @@ int kvm_iommu_map_guest(struct kvm *kvm,
 	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;
+		if (kvm->arch.iommu_domain) {
+			iommu_domain_free(kvm->arch.iommu_domain);
+			kvm->arch.iommu_domain = NULL;
 		}
 		return -ENODEV;
 	}
 
-	kvm->arch.intel_iommu_domain = intel_iommu_domain_alloc(pdev);
-	if (!kvm->arch.intel_iommu_domain)
+	kvm->arch.iommu_domain = iommu_domain_alloc(&pdev->dev);
+	if (!kvm->arch.iommu_domain)
 		return -ENODEV;
 
 	r = kvm_iommu_map_memslots(kvm);
 	if (r)
 		goto out_unmap;
 
-	intel_iommu_detach_dev(kvm->arch.intel_iommu_domain,
-			       pdev->bus->number, pdev->devfn);
+	iommu_detach_device(kvm->arch.iommu_domain, &pdev->dev);
 
-	r = intel_iommu_context_mapping(kvm->arch.intel_iommu_domain,
-					pdev);
+	r = iommu_attach_device(kvm->arch.iommu_domain, &pdev->dev);
 	if (r) {
 		printk(KERN_ERR "Domain context map for %s failed",
 		       pci_name(pdev));
@@ -138,16 +138,16 @@ out_unmap:
 }
 
 static void kvm_iommu_put_pages(struct kvm *kvm,
-			       gfn_t base_gfn, unsigned long npages)
+				gfn_t base_gfn, unsigned long npages)
 {
 	gfn_t gfn = base_gfn;
 	pfn_t pfn;
-	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+	struct iommu_domain *domain = kvm->arch.iommu_domain;
 	int i;
 
 	for (i = 0; i < npages; i++) {
-		pfn = (pfn_t)intel_iommu_iova_to_pfn(domain,
-						     gfn_to_gpa(gfn));
+		pfn = (pfn_t)iommu_iova_to_phys(domain, gfn_to_gpa(gfn));
+		pfn >>= PAGE_SHIFT;
 		kvm_release_pfn_clean(pfn);
 		gfn++;
 	}
@@ -169,23 +169,25 @@ static int kvm_iommu_unmap_memslots(struct kvm *kvm)
 int kvm_iommu_unmap_guest(struct kvm *kvm)
 {
 	struct kvm_assigned_dev_kernel *entry;
-	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+	struct iommu_domain *domain = kvm->arch.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",
+		struct pci_dev *pdev = entry->dev;
+		printk(KERN_DEBUG "IOMMU unmap: host bdf = %x:%x:%x\n",
 		       entry->host_busnr,
 		       PCI_SLOT(entry->host_devfn),
 		       PCI_FUNC(entry->host_devfn));
 
-		/* detach kvm dmar domain */
-		intel_iommu_detach_dev(domain, entry->host_busnr,
-				       entry->host_devfn);
+		/* detach kvm iommu domain */
+		iommu_detach_device(domain, &pdev->dev);
 	}
 	kvm_iommu_unmap_memslots(kvm);
-	intel_iommu_domain_exit(domain);
+	iommu_domain_free(kvm->arch.iommu_domain);
+	kvm->arch.iommu_domain = NULL;
+
 	return 0;
 }
-- 
1.5.6.4

Re: [osrc-patches] [PATCH 0/9] Factor VT-d KVM functions into a generic API

[Joerg Roedel]

On Thu, Nov 27, 2008 at 04:40:45PM +0100, Joerg Roedel wrote:
> Hi,
> 
> this patch series makes the current KVM device passthrough code generic
> enough so that other IOMMU implementation can also plug into this code.
> It works by factoring the functions Vt-d code exports to KVM into a
> generic interface which allows different backends.
> 
> This a basic implementation of a generic interface. It can and should be
> improved later to support more types of hardware IOMMUs then VT-d and
> AMD IOMMU.
> 
> Since I have no VT-d hardware available these patches are only compile
> tested for now.
> 
> Please review, comment and test these patches.

For testing, the patches can be pulled against avi/master from

git://git.kernel.org/pub/scm/linux/kernel/git/joro/linux-2.6-iommu.git iommu-api

> 
> Thanks,
> 
> Joerg
> 
> diffstat:
> 
>  arch/ia64/Kconfig               |    3 +
>  arch/ia64/kvm/Makefile          |    2 +-
>  arch/x86/Kconfig                |    3 +
>  arch/x86/include/asm/kvm_host.h |    3 +-
>  arch/x86/kvm/Makefile           |    2 +-
>  drivers/base/Makefile           |    1 +
>  drivers/base/iommu.c            |   94 +++++++++++++++++++
>  drivers/pci/intel-iommu.c       |   91 ++++++++++++++++++
>  include/linux/iommu.h           |  100 ++++++++++++++++++++
>  virt/kvm/iommu.c                |  193 +++++++++++++++++++++++++++++++++++++++
>  virt/kvm/vtd.c                  |  191 --------------------------------------
> 
> 
> _______________________________________________
> osrc-patches mailing list
> osrc-patches@elbe.amd.com
> https://ddcwww.amd.com/mailman/listinfo/osrc-patches

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy