[PATCH 3/4][VTD] vt-d hooks in generic KVM sources

[PATCH 3/4][VTD] vt-d hooks in generic KVM sources

[Kay, Allen M]

[-- Attachment #1: Type: text/plain, Size: 138 bytes --]

vt-d hooks in generic KVM sources for mapping guest memory with vt-d
page table.

Signed-off-by: Allen M. Kay <allen.m.kay@intel.com>

[-- Attachment #2: kvm_generic.patch --]
[-- Type: application/octet-stream, Size: 3117 bytes --]

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index c97d35c..f635fb0 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -10,7 +10,7 @@ endif
 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
+	i8254.o vtd.o
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d8bc492..61052e1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -28,6 +28,7 @@
 #include <linux/module.h>
 #include <linux/mman.h>
 #include <linux/highmem.h>
+#include <linux/pci.h>
 
 #include <asm/uaccess.h>
 #include <asm/msr.h>
@@ -351,6 +352,8 @@ static void kvm_free_pci_passthrough(struct kvm *kvm)
 
 		list_del(&pci_pt_dev->list);
 	}
+	if (kvm_intel_iommu_found())
+		kvm->arch.domain = NULL;
 	write_unlock_irqrestore(&kvm_pci_pt_lock, flags);
 }
 
@@ -1958,6 +1961,11 @@ long kvm_arch_vm_ioctl(struct file *filp,
 		r = kvm_vm_ioctl_pci_pt_dev(kvm, &pci_pt_dev);
 		if (r)
 			goto out;
+		if (kvm_intel_iommu_found()) {
+			r = kvm_iommu_map_guest(kvm, &pci_pt_dev);
+			if (r)
+				goto out;
+		}
 		break;
 	}
 	case KVM_GET_PIT: {
@@ -4213,6 +4221,8 @@ static void kvm_free_vcpus(struct kvm *kvm)
 
 void kvm_arch_destroy_vm(struct kvm *kvm)
 {
+	if (kvm_intel_iommu_found())
+		kvm_iommu_unmap_guest(kvm);
 	kvm_free_pci_passthrough(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 496adbf..c1ca2f2 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -19,6 +19,8 @@
 #include <linux/kvm_types.h>
 
 #include <asm/desc.h>
+#include <linux/dmar.h>
+#include <linux/intel-iommu.h>
 
 #define KVM_MAX_VCPUS 16
 #define KVM_MEMORY_SLOTS 32
@@ -345,6 +347,7 @@ struct kvm_arch{
 	 */
 	struct list_head active_mmu_pages;
 	struct list_head pci_pt_dev_head;
+	struct dmar_domain *domain;
 	struct kvm_pic *vpic;
 	struct kvm_ioapic *vioapic;
 	struct kvm_pit *vpit;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 4741063..080d0c1 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -272,6 +272,13 @@ 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);
 
+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_pci_passthrough_dev *pci_pt_dev);
+int kvm_iommu_unmap_guest(struct kvm *kvm);
+int kvm_intel_iommu_found(void);
+
 static inline void kvm_guest_enter(void)
 {
 	account_system_vtime(current);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e8f9fda..7211823 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -388,6 +388,11 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	}
 
 	kvm_free_physmem_slot(&old, &new);
+
+	/* map the pages in iommu page table */
+	if (kvm_intel_iommu_found())
+		kvm_iommu_map_pages(kvm, base_gfn, npages);
+
 	return 0;
 
 out_free:

Re: [PATCH 3/4][VTD] vt-d hooks in generic KVM sources

[Avi Kivity]

Kay, Allen M wrote:
> vt-d hooks in generic KVM sources for mapping guest memory with vt-d
> page table.
>
> Signed-off-by: Allen M. Kay <allen.m.kay@intel.com>
>   
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index c97d35c..f635fb0 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -10,7 +10,7 @@ endif
>  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
> +    i8254.o vtd.o

This breaks the build.

> /kvm/x86.c b/arch/x86/kvm/x86.c
> index d8bc492..61052e1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -28,6 +28,7 @@
>  #include <linux/module.h>
>  #include <linux/mman.h>
>  #include <linux/highmem.h>
> +#include <linux/pci.h>
>  
>  #include <asm/uaccess.h>
>  #include <asm/msr.h>
> @@ -351,6 +352,8 @@ static void kvm_free_pci_passthrough(struct kvm *kvm)
>  
>          list_del(&pci_pt_dev->list);
>      }
> +    if (kvm_intel_iommu_found())
> +        kvm->arch.domain = NULL;

"domain" is much too generic.  Need something like intel_iommu_domain 
(later we can transform it to iommu_domain as we make it non-intel 
dependent; also move it out of arch so ia64 can benefit too).

>      write_unlock_irqrestore(&kvm_pci_pt_lock, flags);
>  }
>  
> @@ -1958,6 +1961,11 @@ long kvm_arch_vm_ioctl(struct file *filp,
>          r = kvm_vm_ioctl_pci_pt_dev(kvm, &pci_pt_dev);
>          if (r)
>              goto out;
> +        if (kvm_intel_iommu_found()) {
> +            r = kvm_iommu_map_guest(kvm, &pci_pt_dev);
> +            if (r)
> +                goto out;
> +        }

Need to undo the effects of kvm_vm_ioctl_pci_pt_dev() on failure.

>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index e8f9fda..7211823 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -388,6 +388,11 @@ int __kvm_set_memory_region(struct kvm *kvm,
>      }
>  
>      kvm_free_physmem_slot(&old, &new);
> +
> +    /* map the pages in iommu page table */
> +    if (kvm_intel_iommu_found())
> +        kvm_iommu_map_pages(kvm, base_gfn, npages);
> +
>      return 0;

This is generic code.  As this is arch specific for now, please move it 
to arch code.

Also, make sure that each patch builds cleanly.

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

RE: [PATCH 3/4][VTD] vt-d hooks in generic KVM sources

[Han, Weidong]

Avi Kivity wrote:
> Kay, Allen M wrote:
>> vt-d hooks in generic KVM sources for mapping guest memory with vt-d
>> page table. 
>> 
>> Signed-off-by: Allen M. Kay <allen.m.kay@intel.com>
>> 
>> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
>> index c97d35c..f635fb0 100644
>> --- a/arch/x86/kvm/Makefile
>> +++ b/arch/x86/kvm/Makefile
>> @@ -10,7 +10,7 @@ endif
>>  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
>> +    i8254.o vtd.o
> 
> This breaks the build.

Why does it break the build? It works for me. 

> 
>> /kvm/x86.c b/arch/x86/kvm/x86.c
>> index d8bc492..61052e1 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -28,6 +28,7 @@
>>  #include <linux/module.h>
>>  #include <linux/mman.h>
>>  #include <linux/highmem.h>
>> +#include <linux/pci.h>
>> 
>>  #include <asm/uaccess.h>
>>  #include <asm/msr.h>
>> @@ -351,6 +352,8 @@ static void kvm_free_pci_passthrough(struct kvm
>> *kvm) 
>> 
>>          list_del(&pci_pt_dev->list);
>>      }
>> +    if (kvm_intel_iommu_found())
>> +        kvm->arch.domain = NULL;
> 
> "domain" is much too generic.  Need something like intel_iommu_domain
> (later we can transform it to iommu_domain as we make it non-intel
> dependent; also move it out of arch so ia64 can benefit too).
> 
>>      write_unlock_irqrestore(&kvm_pci_pt_lock, flags);  }
>> 
>> @@ -1958,6 +1961,11 @@ long kvm_arch_vm_ioctl(struct file *filp,
>>          r = kvm_vm_ioctl_pci_pt_dev(kvm, &pci_pt_dev);          if
>>              (r) goto out;
>> +        if (kvm_intel_iommu_found()) {
>> +            r = kvm_iommu_map_guest(kvm, &pci_pt_dev); +           
>> if (r) +                goto out;
>> +        }
> 
> Need to undo the effects of kvm_vm_ioctl_pci_pt_dev() on failure.
> 

kvm_vm_ioctl_pci_pt_dev() does cleanup work by itself when it's failed.

Randy (Weidong)

>> 
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index e8f9fda..7211823 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -388,6 +388,11 @@ int __kvm_set_memory_region(struct kvm *kvm,   
>> } 
>> 
>>      kvm_free_physmem_slot(&old, &new);
>> +
>> +    /* map the pages in iommu page table */
>> +    if (kvm_intel_iommu_found())
>> +        kvm_iommu_map_pages(kvm, base_gfn, npages); +
>>      return 0;
> 
> This is generic code.  As this is arch specific for now, please move
> it to arch code.
> 
> Also, make sure that each patch builds cleanly.

Re: [PATCH 3/4][VTD] vt-d hooks in generic KVM sources

[Amit Shah]

On Saturday 21 June 2008 10:27:50 Han, Weidong wrote:
> Avi Kivity wrote:
> > Kay, Allen M wrote:
> >> vt-d hooks in generic KVM sources for mapping guest memory with vt-d
> >> page table.
> >>
> >> Signed-off-by: Allen M. Kay <allen.m.kay@intel.com>
> >>
> >> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> >> index c97d35c..f635fb0 100644
> >> --- a/arch/x86/kvm/Makefile
> >> +++ b/arch/x86/kvm/Makefile
> >> @@ -10,7 +10,7 @@ endif
> >>  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
> >> +    i8254.o vtd.o
> >
> > This breaks the build.
>
> Why does it break the build? It works for me.

The vtd.c file is introduced in the next patch (4/4). For a tree which only 
has the first three patches applied, the build would break.

> >> @@ -1958,6 +1961,11 @@ long kvm_arch_vm_ioctl(struct file *filp,
> >>          r = kvm_vm_ioctl_pci_pt_dev(kvm, &pci_pt_dev);          if
> >>              (r) goto out;
> >> +        if (kvm_intel_iommu_found()) {
> >> +            r = kvm_iommu_map_guest(kvm, &pci_pt_dev); +
> >> if (r) +                goto out;
> >> +        }
> >
> > Need to undo the effects of kvm_vm_ioctl_pci_pt_dev() on failure.
>
> kvm_vm_ioctl_pci_pt_dev() does cleanup work by itself when it's failed.

When the iommu is not found, there's no use in keeping the device assigned and 
all the memory held. Anyhow, moving this hunk into the 
kvm_vm_ioctl_pci_pt_dev() function is a better idea so that all the setup can 
be done there. In the current tree, this hunk is moved into 
kvm_vm_ioctl_pci_pt_dev().