Messages in this thread |  | | | Date | Sat, 20 Sep 2008 02:34:15 +0900 | | Subject | Re: [PATCH] remove fullflush and nofullflush in IOMMU generic option | | From | FUJITA Tomonori <> |
| |
On Fri, 19 Sep 2008 19:20:36 +0200 Joerg Roedel <joerg.roedel@amd.com> 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@amd.com> 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.
|  |