[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080125180820.GA16868@linux.intel.com>
Date: Fri, 25 Jan 2008 10:08:20 -0800
From: mark gross <mgross@...ux.intel.com>
To: Arjan van de Ven <arjan@...ux.intel.com>,
LKML <linux-kernel@...r.kernel.org>,
Dave Airlie <airlied@...ux.ie>
Subject: Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current
graphics dmar status
On Fri, Jan 25, 2008 at 11:02:55AM +0800, Zhenyu Wang wrote:
>
> 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.
ok. How soon do you need this export?
>
> 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@...el.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.
>
> No, dmar_disabled is not set in origin, which can't tell if it's turned
> off with (intel_iommu=off) later.
>
oops, I missed that how about something like
if (swiotlb || no_iommu || iommu_detected || dmar_disabled) {
dmar_disabled = 1;
return;
}
I guess your way is ok too.
> >
> > > > + }
> > > > 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.
send me your current patch and eta for the graphics module patches so I
can better coordinate with you.
--mgross
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists