Re: [PATCH 4/4][VTD] vt-d specific files in KVM

[Avi Kivity <avi@qumranet.com>]

Kay, Allen M wrote:
> vt-d specific files in KVM for contructing vt-d page tables and
> programming vt-d context entries.
>
> Signed-off-by: Allen M. Kay <allen.m.kay@intel.com>
>   
> diff --git a/arch/x86/kvm/vtd.c b/arch/x86/kvm/vtd.c
> new file mode 100644
> index 0000000..634802c
> --- /dev/null
> +++ b/arch/x86/kvm/vtd.c
> @@ -0,0 +1,197 @@
> +/*
> + * 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
> + * Author: Allen M. Kay <allen.m.kay@intel.com>
> + * Author: Weidong Han <weidong.han@intel.com>
> + */
> +
> +#include <linux/list.h>
> +#include <linux/kvm_host.h>
> +#include <linux/pci.h>
> +#include <linux/dmar.h>
> +#include <linux/intel-iommu.h>
> +#include "vtd.h"
> +
> +int kvm_iommu_map_pages(struct kvm *kvm,
> +    gfn_t base_gfn, unsigned long npages)
> +{
> +    gfn_t gfn = base_gfn;
> +    pfn_t pfn;
> +    struct page *page;
> +    int i, rc;
> +
> +    if (!kvm->arch.domain)
> +        return -EFAULT;
> +
> +    printk(KERN_DEBUG "kvm_iommu_map_page: gpa = %lx\n",
> +        gfn << PAGE_SHIFT);
> +    printk(KERN_DEBUG "kvm_iommu_map_page: hpa = %lx\n",
> +        gfn_to_pfn(kvm, base_gfn) << PAGE_SHIFT);
> +    printk(KERN_DEBUG "kvm_iommu_map_page: size = %lx\n",
> +        npages*PAGE_SIZE);
> +
> +    for (i = 0; i < npages; i++) {
> +        pfn = gfn_to_pfn(kvm, gfn);
> +        if (pfn_valid(pfn)) {
> +            rc = kvm_intel_iommu_page_mapping(kvm->arch.domain,
> +                gfn << PAGE_SHIFT, pfn << PAGE_SHIFT,
> +                PAGE_SIZE, DMA_PTE_READ | DMA_PTE_WRITE);
> +            if (rc) {
> +                page = gfn_to_page(kvm, gfn);
> +                put_page(page);

This is racy.  gfn_to_page() can return a different page each time it is 
called.  Instead iommu_map_page() should drop the refcount if it fails.

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