Re: [PATCH 2/2] KVM: Device assignemnt with VT-d

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

On Wednesday 13 August 2008 17:46:03 Avi Kivity wrote:
> Yang, Sheng wrote:
> > On Thursday 07 August 2008 22:14:47 Ben-Ami Yassour wrote:
> >> Based on a patch by: Kay, Allen M <allen.m.kay@intel.com>
> >>
> >> This patch enables pci device assignment based on VT-d support.
> >> When a device is assigned to the guest, the guest memory is
> >> pinned and the mapping is updated in the VT-d IOMMU.
> >
> > I am afraid there still some compatible problem...
> >
> >> 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      |    3 +
> >>  arch/x86/kvm/vtd.c         |  203
> >
> > [snip]
> >
> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> index a97157c..5cfc21a 100644
> >> --- a/arch/x86/kvm/x86.c
> >> +++ b/arch/x86/kvm/x86.c
> >> @@ -35,6 +35,7 @@
> >>  #include <linux/module.h>
> >>  #include <linux/mman.h>
> >>  #include <linux/highmem.h>
> >> +#include <linux/intel-iommu.h>
> >
> > This broken external kernel modules before 2.6.27... If we
> > wrapped it with CONFIG_DMAR, it would also broken the commit
> > before the patch checked in and after DMAR enabled in kernel...
> > Need a version number judgement?
>
> kernel patches should not consider external module issues.  That
> keeps the code clean (at the expense of making the external
> module's maintainer's life mode difficult, but that's their
> problem).

Yeah, thanks for point it out. It's indeed complicate to consider this 
kind of issues... :)

And I think now our aptitude towards external modules is not 
encouraging? For after this patch, we can discard 
external-modules-compat.h as well. :)

-- 
regards
Yang, Sheng

> >> diff --git a/include/asm-x86/kvm_host.h
> >> b/include/asm-x86/kvm_host.h index ef019b5..b141949 100644
> >> --- a/include/asm-x86/kvm_host.h
> >> +++ b/include/asm-x86/kvm_host.h
> >> @@ -354,6 +354,7 @@ struct kvm_arch{
> >>  	 */
> >>  	struct list_head active_mmu_pages;
> >>  	struct list_head assigned_dev_head;
> >> +	struct dmar_domain *intel_iommu_domain;
> >
> > Need wrapped by CONFIG_DMAR?
>
> I guess we can keep this, one pointer is not that expensive.  But
> we should make sure all the iommu functions are available when
> iommu is unconfigured.