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

From: mark gross
Date: Tue Jan 08 2008 - 15:40:52 EST


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@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.

> > + }
> > 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/