[GIT PULL] AMD IOMMU updates for 2.6.28

[GIT PULL] AMD IOMMU updates for 2.6.28

[Joerg Roedel]

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
      AMD IOMMU: implement lazy IO/TLB flushing
      AMD IOMMU: add branch hints to completion wait checks
      AMD IOMMU: align alloc_coherent addresses properly
      AMD IOMMU: add event buffer allocation
      AMD IOMMU: save pci segment from ACPI tables
      AMD IOMMU: save pci_dev instead of devid
      AMD IOMMU: add MSI interrupt support
      AMD IOMMU: add event handling code
      AMD IOMMU: enable event logging
      AMD IOMMU: allow IO page faults from devices
      AMD IOMMU: add dma_supported callback
      AMD IOMMU: don't assign preallocated protection domains to devices
      AMD IOMMU: some set_device_domain cleanups
      AMD IOMMU: replace memset with __GFP_ZERO in alloc_coherent
      AMD IOMMU: simplify dma_mask_to_pages
      AMD IOMMU: free domain bitmap with its allocation order
      AMD IOMMU: remove unnecessary cast to u64 in the init code
      AMD IOMMU: calculate IVHD size with a function
      AMD IOMMU: use cmd_buf_size when freeing the command buffer
      add AMD IOMMU tree to MAINTAINERS file
      AMD IOMMU: use coherent_dma_mask in alloc_coherent

 Documentation/kernel-parameters.txt       |    4 +
 Documentation/x86/x86_64/boot-options.txt |    2 -
 MAINTAINERS                               |    1 +
 arch/x86/Kconfig                          |    1 +
 arch/x86/kernel/amd_iommu.c               |  318 ++++++++++++++++++++++++-----
 arch/x86/kernel/amd_iommu_init.c          |  191 ++++++++++++++++--
 arch/x86/kernel/pci-dma.c                 |   13 ++
 arch/x86/kernel/pci-gart_64.c             |   13 --
 include/asm-x86/amd_iommu.h               |    3 +
 include/asm-x86/amd_iommu_types.h         |   58 +++++-
 include/asm-x86/iommu.h                   |    1 +
 11 files changed, 517 insertions(+), 88 deletions(-)

-- 
           |           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

Re: [GIT PULL] AMD IOMMU updates for 2.6.28

[FUJITA Tomonori]

On Fri, 19 Sep 2008 12:07:11 +0200
Joerg Roedel <joerg.roedel@amd.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.

Re: [GIT PULL] AMD IOMMU updates for 2.6.28

[Joerg Roedel]

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@amd.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. If we move something then both. But feel
free to submit a patch that removes the nofullflush option. I would not
disagree with that.

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

Re: [GIT PULL] AMD IOMMU updates for 2.6.28

[FUJITA Tomonori]

On Fri, 19 Sep 2008 12:39:23 +0200
Joerg Roedel <joerg.roedel@amd.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@amd.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.


> 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.

If you don't like it, please add 'nofullflush' option as AMD IOMMU
option for now. As long as AMD IOMMU uses the same option name, we can
clean up them later if necessary.

Re: [GIT PULL] AMD IOMMU updates for 2.6.28

[Joerg Roedel]

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@amd.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@amd.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
GART. The nofullflush option has no meaning for GART and for AMD
IOMMU. The only reason to keep it is if we want to change the default in
the future. If you don't think we would change the default any time in
the future feel free to remove it.

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

Re: [GIT PULL] AMD IOMMU updates for 2.6.28

[FUJITA Tomonori]

On Fri, 19 Sep 2008 13:02:40 +0200
Joerg Roedel <joerg.roedel@amd.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@amd.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@amd.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)


Please don't do that.


> GART. The nofullflush option has no meaning for GART and for AMD
> IOMMU. The only reason to keep it is if we want to change the default in
> the future.

No, we can't change the default behavior. It's the user interface.


> If you don't think we would change the default any time in
> the future feel free to remove it.

Re: [GIT PULL] AMD IOMMU updates for 2.6.28

[FUJITA Tomonori]

On Fri, 19 Sep 2008 20:23:50 +0900
FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> On Fri, 19 Sep 2008 13:02:40 +0200
> Joerg Roedel <joerg.roedel@amd.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@amd.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@amd.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.

Re: [GIT PULL] AMD IOMMU updates for 2.6.28

[Joerg Roedel]

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:
> 
> > On Fri, 19 Sep 2008 13:02:40 +0200
> > Joerg Roedel <joerg.roedel@amd.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@amd.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@amd.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.
After this patch we can change other IOMMU implementations with lazy
flushing to use that parameter too.

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

Re: [GIT PULL] AMD IOMMU updates for 2.6.28

[FUJITA Tomonori]

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:
> > 
> > > On Fri, 19 Sep 2008 13:02:40 +0200
> > > Joerg Roedel <joerg.roedel@amd.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@amd.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@amd.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.

Re: [GIT PULL] AMD IOMMU updates for 2.6.28

[Joerg Roedel]

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

Re: [GIT PULL] AMD IOMMU updates for 2.6.28

[FUJITA Tomonori]

On Fri, 19 Sep 2008 14:34:26 +0200
Joerg Roedel <joerg.roedel@amd.com> wrote:

> 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.

Not sure. It would make sense but maybe not.

But it's the different topic. Making a generic option that affects all
the IOMMUs is an important issue for everyone. All the parties need to
discuss it and agree.


> 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.

Yes, that's the point. We should not have such important change in AMD
IOMMU updates.

So please just make 'fullflush' as AMD IOMMU's option.