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

[Zhenyu Wang]

Mark, sorry for missing this for long time...

On 2008.01.08 12:44:20 -0800, mark gross 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?

This is used for our graphics driver module to know if we have to
do dma remapping in iommu case, both in kernel config and kernel
boot time param config. 

The simplest case is that DMAR_GFX_WA is on, which no dma mapping
will act in intel_agp.

If no DMAR_GFX_WA, we still have boot param to turn off whole intel
iommu (intel_iommu=off), just turn off graphics remap engine
(intel_iommu=igfx_off). So this exported interface is used to know
that in runtime.

> 
> > > 
> > > Signed-off-by: Zhenyu Wang <zhenyu.z.wang@xxxxxxxxx>
> > > ---
> > >  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.

No, dmar_disabled is not set in origin, which can't tell if it's turned
off with (intel_iommu=off) later.

> 
> > > +	}
> > >  	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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/