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

[Avi Kivity <avi@qumranet.com>]

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.