Re: [GIT PULL] AMD IOMMU updates for 2.6.28

[Joerg Roedel <joerg.roedel@amd.com>]

On Fri, Sep 19, 2008 at 09:24:18PM +0900, FUJITA Tomonori wrote:
> On Fri, 19 Sep 2008 13:52:59 +0200
> Joerg Roedel <joerg.roedel@amd.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@lab.ntt.co.jp> wrote:
> > > 
> > > 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.

Ok, so now we have fullflush, which makes sense for all x86 hardware
IOMMUs except maybe Calgary (from what I know about Calgary fullflush
can be implemented there only with a abyssal performance penalty nobody
is willing to pay). So it makes sense to have the option in the generic
place. But we can surely add a comment that it does not affect Calgary
to the kernel documentation (and change VT-d to use that parameter too).
But the AMD IOMMU update chain is not the right place for making big
changes to other IOMMUs.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy