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

Avi Kivity
2008-06-20T18:24:19Z

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.