Re: [PATCH] VT-d: Fix iommu map page for mmio pages

[Avi Kivity <avi@redhat.com>]

Han, Weidong wrote:
> Don't need to map mmio pages for iommu. When find mmio pages in
> kvm_iommu_map_pages(), don't map them, and shouldn't return error due to
> it's not an error. If return error (such as -EINVAL), device assigment
> will fail.
>
>   


I don't understand.  Why don't we need to map mmio pages?  We certainly 
don't want them emulated.

> @@ -36,14 +36,13 @@ int kvm_iommu_map_pages(struct kvm *kvm,
>  {
>  	gfn_t gfn = base_gfn;
>  	pfn_t pfn;
> -	int i, r;
> +	int i, r = 0;
>  	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
>  
>  	/* check if iommu exists and in use */
>  	if (!domain)
>  		return 0;
>  
> -	r = -EINVAL;
>  	for (i = 0; i < npages; i++) {
>  		/* check if already mapped */
>  		pfn = (pfn_t)intel_iommu_iova_to_pfn(domain,
> @@ -60,13 +59,14 @@ int kvm_iommu_map_pages(struct kvm *kvm,
>  						     DMA_PTE_READ |
>  						     DMA_PTE_WRITE);
>  			if (r) {
> -				printk(KERN_DEBUG "kvm_iommu_map_pages:"
> +				printk(KERN_ERR "kvm_iommu_map_pages:"
>  				       "iommu failed to map pfn=%lx\n",
> pfn);
>  				goto unmap_pages;
>  			}
>  		} else {
> -			printk(KERN_DEBUG "kvm_iommu_map_page:"
> -			       "invalid pfn=%lx\n", pfn);
> +			printk(KERN_DEBUG "kvm_iommu_map_pages:"
> +			       "invalid pfn=%lx, iommu needn't map "
> +			       "MMIO pages!\n", pfn);
>  			goto unmap_pages;
>  		}

If a slot has a mix of mmio and non-mmio pages, you will unmap the 
non-mmio pages, yet return no error.

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