Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[mark gross <mgross@linux.intel.com>]

Sorry for the late reply.
comments below...


On Fri, Jan 04, 2008 at 09:53:38AM +0800, Zhenyu Wang wrote:
> On 2007.12.19 13:26:08 +0000, Zhenyu Wang wrote:
> > 
> > [agp-mm] [intel_iommu] explicit export current graphics dmar status
> > 
> > To make it possbile to tell other modules about curent
> > graphics dmar engine status, that could decide if graphics
> > driver should remap physical address to dma address.
> > 
> > Also this one trys to make dmar_disabled really present
> > current status of DMAR, which would be used for completely
> > express graphics dmar engine is active or not.

do you think this exporting will be needed?
If so why and when would someone use it?

> > 
> > Signed-off-by: Zhenyu Wang <zhenyu.z.wang@intel.com>
> > ---
> >  drivers/pci/intel-iommu.c |   18 ++++++++++++++++--
> >  include/linux/dmar.h      |    6 ++++++
> >  2 files changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> > index e079a52..81a0abc 100644
> > --- a/drivers/pci/intel-iommu.c
> > +++ b/drivers/pci/intel-iommu.c
> > @@ -53,7 +53,7 @@
> >  static void domain_remove_dev_info(struct dmar_domain *domain);
> >  
> >  static int dmar_disabled;
> > -static int __initdata dmar_map_gfx = 1;
> > +static int dmar_map_gfx = 1;
> >  static int dmar_forcedac;
> >  
> >  #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
> > @@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
> >  }
> >  __setup("intel_iommu=", intel_iommu_setup);
> >  
> > +int intel_iommu_gfx_remapping(void)
> > +{
> > +#ifndef CONFIG_DMAR_GFX_WA
> > +	if (!dmar_disabled && iommu_detected && dmar_map_gfx)
> > +		return 1;
> > +#endif
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
> > +
> >  static struct kmem_cache *iommu_domain_cache;
> >  static struct kmem_cache *iommu_devinfo_cache;
> >  static struct kmem_cache *iommu_iova_cache;
> > @@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
> >  
> >  void __init detect_intel_iommu(void)
> >  {
> > -	if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
> > +	if (dmar_disabled)
> > +		return;
> > +	if (swiotlb || no_iommu || iommu_detected) {
> > +		dmar_disabled = 1;
> >  		return;

Why the bloat?  This block of 7 lines looks like it does the same as the
2 you replaced.

> > +	}
> >  	if (early_dmar_detect()) {
> >  		iommu_detected = 1;
> >  	}
> > diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> > index ffb6439..8ae435e 100644
> > --- a/include/linux/dmar.h
> > +++ b/include/linux/dmar.h
> > @@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
> >  extern int dmar_table_init(void);
> >  extern int early_dmar_detect(void);
> >  
> > +extern int intel_iommu_gfx_remapping(void);
> > +
> >  extern struct list_head dmar_drhd_units;
> >  extern struct list_head dmar_rmrr_units;
> >  
> > @@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
> >  {
> >  	return -ENODEV;
> >  }
> > +static inline int intel_iommu_gfx_remapping(void)
> > +{
> > +	return 0;
> > +}

Um this function is silly lets not post it.

> >  
> >  #endif /* !CONFIG_DMAR */
> >  #endif /* __DMAR_H__ */
> > -- 
> > 1.5.3.4
> > 
> 
> Any comments to this one? Is it ok to push this iommu patch with
> agp dma remapping patches to next -mm?
> 

Its not ready for posting.

--mgross