Re: [PATCH] remove fullflush and nofullflush in IOMMU generic option

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

On Sat, Sep 20, 2008 at 01:23:30AM +0900, FUJITA Tomonori wrote:
> This patch against tip/x86/iommu virtually reverts
> 2842e5bf3115193f05dc9dac20f940e7abf44c1a. But just reverting the
> commit breaks AMD IOMMU so this patch also includes some fixes.
 
NACK.

> The above commit adds new two options to x86 IOMMU generic kernel boot
> options, fullflush and nofullflush. But such change that affects all
> the IOMMUs needs more discussion (all IOMMU parties need the chance to
> discuss it):

It affects only IOMMUs which use the iommu_fullflush variable. This are
GART, which used it since ages, and AMD IOMMU which was introduced by
the above commit. It absolutly makes sense to have command line parameters
which make sense for more than one or most of the IOMMUs in x86 into
'iommu='. Ingo agreed with this, see http://lkml.org/lkml/2008/6/30/155
I agree with that too. The commit you are trying to revert here was a
step into this direction.

> http://lkml.org/lkml/2008/9/19/106
> 
> For me, adding these boot parameters doesn't make sense.

May be true for nofullflush. For fullflush it makes sense to have it in
the generic code since all x86 hardware IOMMUs except Calgary can make
use of it. 

> All the hardware IOMMUs could use 'fullflush' for lazy IOTLB flushing
> but Calgary doesn't support it. Intel VT-d has the different option
> for it (and we can't rename it). So it might be useful for only GART
> and AMD IOMMU.

We can keep the current Intel option and have iommu=fullflush in
parallel. It could also affect Intel IOMMU.

> 'nofullflush' definitely is pointless. 'nofullflush' option doesn't
> change any kernel behavior and it was added just for GART
> compatibility. We should not have such generic meaningless option just
> for GART.

So why do you keep it in this patch then?

> This patch removes the fullflush and nofullflush from the generic
> kernel boot options and adds 'fullflush' support for AMD IOMMU.

I disagree. Keep it in the generic code.

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