Re: [PATCH] remove fullflush and nofullflush in IOMMU genericoption

[FUJITA Tomonori]

On Fri, 19 Sep 2008 19:20:36 +0200
Joerg Roedel <joerg.roedel@xxxxxxx> wrote:

> On Sat, Sep 20, 2008 at 02:09:21AM +0900, FUJITA Tomonori wrote:
> > On Fri, 19 Sep 2008 18:45:41 +0200
> > Joerg Roedel <joerg.roedel@xxxxxxx> wrote:
> > 
> > > 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.
> > 
> > The point is we can't remove or rename VT-d option for IOTLB batching
> > because we already exported it for users.
> > 
> > I think that once we export a boot option to users, we can't remove or
> > rename it. It's the user interface.
> > 
> > You have a different policy for the kernel boot option? You think we
> > can change or rename it after exporting it?
> 
> No. But we can have the generic option in parallel. There is no reason
> to remove the VT-d specific option.
> 
> > 
> > Yeah, Intel IOMMU need to keep the current option for IOTLB batching
> > but it also support fullflush. But we don't discuss anything with
> > Intel developers. That's what I'm against.
> 
> Thats why my patch does not change VT-d code. So why do you send revert
> patches and not starting this discussion? If Muli and the Intel people
> disagree we can still revert it.

It's not a proper Linux development process for me.

A patch to make such change should be merged with Acked-by from other
developers. Making such change first then saying, "if you don't like
it, revert it" is not nice.


> > > > '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?
> > 
> > As I wrote, I think that we can't remove the exported user interfacea
> 
> You keep this option for AMD IOMMU too. If you move it to AMD IOMMU code
> then you can remove nofullflush there.

I'm not sure what you mean. You think that we can't change the
exported option, how can we remove nofullflush option?


> > 
> > Please keep it for AMD option for now. Please send a patch to make it
> > generic to other IOMMU people and give them a chance to discuss on
> > it.
> >
> 
> Ok, I will forward the patch to Intel VT-d maintainers and Muli to
> discuss it. If they disagree we can revert the patch.

Again, it's not a proper process for me.
--
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/