[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20080919212327Y.fujita.tomonori@lab.ntt.co.jp>
Date: Fri, 19 Sep 2008 21:24:18 +0900
From: FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
To: joerg.roedel@....com
Cc: fujita.tomonori@....ntt.co.jp, mingo@...e.hu,
linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] AMD IOMMU updates for 2.6.28
On Fri, 19 Sep 2008 13:52:59 +0200
Joerg Roedel <joerg.roedel@....com> wrote:
> On Fri, Sep 19, 2008 at 08:47:54PM +0900, FUJITA Tomonori wrote:
> > On Fri, 19 Sep 2008 20:23:50 +0900
> > FUJITA Tomonori <fujita.tomonori@....ntt.co.jp> wrote:
> >
> > > On Fri, 19 Sep 2008 13:02:40 +0200
> > > Joerg Roedel <joerg.roedel@....com> wrote:
> > >
> > > > On Fri, Sep 19, 2008 at 07:51:11PM +0900, FUJITA Tomonori wrote:
> > > > > On Fri, 19 Sep 2008 12:39:23 +0200
> > > > > Joerg Roedel <joerg.roedel@....com> wrote:
> > > > >
> > > > > > On Fri, Sep 19, 2008 at 07:28:13PM +0900, FUJITA Tomonori wrote:
> > > > > > > On Fri, 19 Sep 2008 12:07:11 +0200
> > > > > > > Joerg Roedel <joerg.roedel@....com> wrote:
> > > > > > >
> > > > > > > > Hi Ingo,
> > > > > > > >
> > > > > > > > please pull the updates for the AMD IOMMU driver for the 2.6.28 merge
> > > > > > > > window. Most of the patches had been sent to LKML for review and got
> > > > > > > > comments. One patch touches the GART driver and was added after an
> > > > > > > > objection with a patch previously in this series. Please pull.
> > > > > > > >
> > > > > > > > The following changes since commit 74546a8cd9a4e2c26a15a534f0be2f9cc08ae675:
> > > > > > > > Ingo Molnar (1):
> > > > > > > > Revert "fix warning in: "x86: HPET_MSI Initialise per-cpu HPET timers""
> > > > > > > >
> > > > > > > > are available in the git repository at:
> > > > > > > >
> > > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/joro/linux-2.6-iommu.git iommu-updates-2.6.28
> > > > > > > >
> > > > > > > > FUJITA Tomonori (1):
> > > > > > > > AMD IOMMU: avoid unnecessary low zone allocation in alloc_coherent
> > > > > > > >
> > > > > > > > Joerg Roedel (24):
> > > > > > > > AMD IOMMU: check for invalid device pointers
> > > > > > > > AMD IOMMU: move TLB flushing to the map/unmap helper functions
> > > > > > > > x86: move GART TLB flushing options to generic code
> > > > > > >
> > > > > > > As I wrote, I don't think that this GART patch is the right approach.
> > > > > >
> > > > > > I don't thinks its clean to leave nofullflush in GART code and move
> > > > > > fullflush to generic code only.
> > > > >
> > > > > It's not clean for GART but clean for other IOMMUs.
> > > >
> > > > So its unclean anyway. It doesn't matter if for GART or for AMD IOMMU.
> > > >
> > > > > > If we move something then both. But feel
> > > > > > free to submit a patch that removes the nofullflush option. I would not
> > > > > > disagree with that.
> > > > >
> > > > > I'm not sure we can remove an option that we added. If we can, please
> > > > > remove it now. There is no point to make the meaningless option
> > > > > generic.
> > > >
> > > > I don't added this option. I just moved it to generic code from the
> > >
> > > You added it to the generic place. Moving it to the generic place
> > > means adding it to the generic place.
> > >
> > > There is no difference in the end. Now 'nofullflush' is listed as the
> > > generic place.
> > >
> > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > index c2e00ee..569527e 100644
> > > --- a/Documentation/kernel-parameters.txt
> > > +++ b/Documentation/kernel-parameters.txt
> > > @@ -888,6 +888,10 @@ and is between 256 and 4096 characters. It is defined in the file
> > > nomerge
> > > forcesac
> > > soft
> > > + fullflush
> > > + Flush IO/TLB at every deallocation
> > > + nofullflush
> > > + Flush IO/TLB only when addresses are reused (default)
> >
> > And you don't need to add 'fullflush' to the generic place too.
> >
> > 'fullflush' will be supported with only GART and AMD IOMMU. So adding
> > the description of it to both GART and AMD IOMMU should be fine.
> >
> > 'fullflush' has the same meaning for both IOMMUs. That's nice
> > consistency, I think.
>
> Huh? The whole point of this patch was to have a common option between
> IOMMUs to disable lazy IOTLB flushing. This was suggested by _you_ and
> the only reason I wrote this patch.
You misunderstand what I meant. I'm sorry if my explanation is not
clear.
> After this patch we can change other IOMMU implementations with lazy
> flushing to use that parameter too.
I'm not sure that Calgary wants to support such option. It always uses
lazy flushing.
What I don't like is that there is no consistency about the option
name for lazy flushing. It doesn't mean that we move the option to the
generic place.
Here's my first reply:
=
Would it be nice to have consistency of IOMMU parameters?
VT-d also has the kernel-boot option for this lazy flushing trick
though VT-d 'strict' option is more vague than 'unmap_flush'
=
What I meant that using the option name 'strict' that VT-d uses for
lazy flushing for AMD IOMMU would be better than introducing a new
option name, "unmap_flush" for AMD IOMMU though I don't think that
'strict' is the good name.
Seems 'fullflush' is better than 'strict'. So I think that it's better
to use 'fullflush' for AMD IMMU rather introducing a new name,
'unmap_flush'. But again, it doesn't mean that 'fullflush' moves to
the generic place.
--
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