Re: [PATCH] KVM: PCIPT: VT-d support

["Yang, Sheng" <sheng.yang@intel.com>]

On Thursday 10 July 2008 17:51:53 Ben-Ami Yassour wrote:
> On Thu, 2008-07-10 at 17:30 +0800, Yang, Sheng wrote:
> > On Thursday 10 July 2008 17:14:42 Ben-Ami Yassour wrote:
> > > From: Kay, Allen M <allen.m.kay@intel.com>
> > >
> > > This patch includes the functions to support VT-d for
> > > passthrough devices.
> > >
> > > [Ben: fixed memory pinning]
> > >
> > > Signed-off-by: Kay, Allen M <allen.m.kay@intel.com>
> > > Signed-off-by: Weidong Han <weidong.han@intel.com>
> > > Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
> > > ---
> > >  arch/x86/kvm/Makefile      |    2 +-
> > >  arch/x86/kvm/vtd.c         |  176
> > > ++++++++++++++++++++++++++++++++++++++++++++ arch/x86/kvm/x86.c
> > >
> > >     |   10 +++
> > >
> > >  include/asm-x86/kvm_host.h |    1 +
> > >  include/linux/kvm_host.h   |    6 ++
> > >  virt/kvm/kvm_main.c        |    6 ++
> > >  6 files changed, 200 insertions(+), 1 deletions(-)
> > >  create mode 100644 arch/x86/kvm/vtd.c
> > >
> > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > > index 531d635..dc67d90 100644
> > > --- a/virt/kvm/kvm_main.c
> > > +++ b/virt/kvm/kvm_main.c
> > > @@ -41,6 +41,7 @@
> > >  #include <linux/pagemap.h>
> > >  #include <linux/mman.h>
> > >  #include <linux/swap.h>
> > > +#include <linux/intel-iommu.h>
> > >
> > >  #include <asm/processor.h>
> > >  #include <asm/io.h>
> > > @@ -422,6 +423,11 @@ int __kvm_set_memory_region(struct kvm
> > > *kvm, }
> > >
> > >  	kvm_free_physmem_slot(&old, &new);
> > > +
> > > +	/* map the pages in iommu page table */
> > > +	if (intel_iommu_found())
> > > +		kvm_iommu_map_pages(kvm, base_gfn, npages);
> > > +
> >
> > I don't understand why we need this along with
> > kvm_iommu_map_memslots(). This works during the memory setup, and
> > in kvm_iommu_map_guest() we do it again with the overlapped
> > memory region?
> >
> > I think even if we need pin all pages, we still just need do it
> > once...
>
> We map the entire guest memory on initialization by going over all
> the existing memory slots.
> If a new slot is created later then we need to map it as well, this
> is the call that you see here.

I think it's may be unnecessary to map pages when device assigned. The 
table can be set up along with set_memory_region(), it covered all 
memory slots already, or I miss something here?

-- 
Thanks
Yang, Sheng