public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
@ 2008-09-22 13:35 FUJITA Tomonori
  2008-09-22 14:07 ` Joerg Roedel
  0 siblings, 1 reply; 11+ messages in thread
From: FUJITA Tomonori @ 2008-09-22 13:35 UTC (permalink / raw)
  To: joerg.roedel; +Cc: mingo, linux-kernel

This is against tip/x86/iommu.

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"

This reverts:

commit 8b14518fadd9d5915827d86d5c10e602fedf042e
Author: Joerg Roedel <joerg.roedel@amd.com>
Date:   Thu Jul 3 19:35:09 2008 +0200

    x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off

    This patch removes the amd_iommu=off kernel parameter and honors the generic

    iommu=off parameter for the same purpose.


The above commit is wrong.

amd_iommu=off kernel parameter and the generic iommu=off parameter
don't have the same purpose.

Intel IOMMU also has 'off' option as Intel IOMMU specific parameter
(intel_iommu=off) like AMD IOMMU had. intel_iommu=off parameter
for disabling Intel IOMMU.

The generic iommu=off parameter means that the kernel doesn't use any
kind of IOMMU, including SWIOTLB (as documented). So you can't use the
generic iommu=off parameter to disable hardware IOMMU if you use a box
with max_pfn > MAX_DMA32_PFN. It's is a critical difference.

Seems that amd_iommu=off parameter worked in the exact same way as
intel_iommu=off works (though I might be wrong since I can't confirm
this with AMD IOMMU). So the above commit removed the useful feature
to disalbe AMD IOMMU safely.

I'm not sure what the maintainer wanted to do with the commit so I
can't say that the commit (to remove the above feature) is a bug or
regression. I'll leave this issue to the maintainer but at least the
description of how the IOMMU options works is wrong.

Note that this patch is not a simple revert patch due to some
modifications after the above commit.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 Documentation/kernel-parameters.txt |    1 +
 arch/x86/kernel/amd_iommu_init.c    |    8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 569527e..feddbb7 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -281,6 +281,7 @@ and is between 256 and 4096 characters. It is defined in the file
 	amd_iommu=	[HW,X86-84]
 			Pass parameters to the AMD IOMMU driver in the system.
 			Possible values are:
+			off     - disable AMD IOMMU driver
 			isolate - enable device isolation (each device, as far
 			          as possible, will get its own protection
 			          domain)
diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index db0c83a..a71beb4 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -115,6 +115,7 @@ struct ivmd_header {
 } __attribute__((packed));
 
 static int __initdata amd_iommu_detected;
+static int __initdata amd_iommu_disable;
 
 u16 amd_iommu_last_bdf;			/* largest PCI device id we have
 					   to handle */
@@ -1037,7 +1038,7 @@ int __init amd_iommu_init(void)
 	int i, ret = 0;
 
 
-	if (no_iommu) {
+	if (no_iommu || amd_iommu_disable) {
 		printk(KERN_INFO "AMD IOMMU disabled by kernel command line\n");
 		return 0;
 	}
@@ -1189,7 +1190,8 @@ static int __init early_amd_iommu_detect(struct acpi_table_header *table)
 
 void __init amd_iommu_detect(void)
 {
-	if (swiotlb || no_iommu || (iommu_detected && !gart_iommu_aperture))
+	if (swiotlb || no_iommu || (iommu_detected && !gart_iommu_aperture) ||
+		amd_iommu_disable)
 		return;
 
 	if (acpi_table_parse("IVRS", early_amd_iommu_detect) == 0) {
@@ -1212,6 +1214,8 @@ void __init amd_iommu_detect(void)
 static int __init parse_amd_iommu_options(char *str)
 {
 	for (; *str; ++str) {
+		if (!strncmp(str, "off", 3))
+			amd_iommu_disable = 1;
 		if (strncmp(str, "isolate", 7) == 0)
 			amd_iommu_isolate = 1;
 	}
-- 
1.5.4.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
  2008-09-22 13:35 [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off" FUJITA Tomonori
@ 2008-09-22 14:07 ` Joerg Roedel
  2008-09-22 14:16   ` FUJITA Tomonori
  2008-09-22 14:46   ` Ingo Molnar
  0 siblings, 2 replies; 11+ messages in thread
From: Joerg Roedel @ 2008-09-22 14:07 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: mingo, linux-kernel

On Mon, Sep 22, 2008 at 10:35:08PM +0900, FUJITA Tomonori wrote:
> This is against tip/x86/iommu.
> 
> =
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Subject: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
> 
> This reverts:
> 
> commit 8b14518fadd9d5915827d86d5c10e602fedf042e
> Author: Joerg Roedel <joerg.roedel@amd.com>
> Date:   Thu Jul 3 19:35:09 2008 +0200
> 
>     x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off
> 
>     This patch removes the amd_iommu=off kernel parameter and honors the generic
> 
>     iommu=off parameter for the same purpose.
> 
> 
> The above commit is wrong.

It isn't. The user normally don't care about the type of IOMMU in the
system. So disabling it with iommu=off is the right way. To achieve what
you want its better to add iommu=gart and iommu=amd to the option
parser. This will be consistent with Calgary and SWIOTLB too.

> 
> amd_iommu=off kernel parameter and the generic iommu=off parameter
> don't have the same purpose.
> 
> Intel IOMMU also has 'off' option as Intel IOMMU specific parameter
> (intel_iommu=off) like AMD IOMMU had. intel_iommu=off parameter
> for disabling Intel IOMMU.
> 
> The generic iommu=off parameter means that the kernel doesn't use any
> kind of IOMMU, including SWIOTLB (as documented). So you can't use the
> generic iommu=off parameter to disable hardware IOMMU if you use a box
> with max_pfn > MAX_DMA32_PFN. It's is a critical difference.
> 
> Seems that amd_iommu=off parameter worked in the exact same way as
> intel_iommu=off works (though I might be wrong since I can't confirm
> this with AMD IOMMU). So the above commit removed the useful feature
> to disalbe AMD IOMMU safely.
> 
> I'm not sure what the maintainer wanted to do with the commit so I
> can't say that the commit (to remove the above feature) is a bug or
> regression. I'll leave this issue to the maintainer but at least the
> description of how the IOMMU options works is wrong.
> 
> Note that this patch is not a simple revert patch due to some
> modifications after the above commit.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
>  Documentation/kernel-parameters.txt |    1 +
>  arch/x86/kernel/amd_iommu_init.c    |    8 ++++++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 569527e..feddbb7 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -281,6 +281,7 @@ and is between 256 and 4096 characters. It is defined in the file
>  	amd_iommu=	[HW,X86-84]
>  			Pass parameters to the AMD IOMMU driver in the system.
>  			Possible values are:
> +			off     - disable AMD IOMMU driver
>  			isolate - enable device isolation (each device, as far
>  			          as possible, will get its own protection
>  			          domain)
> diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
> index db0c83a..a71beb4 100644
> --- a/arch/x86/kernel/amd_iommu_init.c
> +++ b/arch/x86/kernel/amd_iommu_init.c
> @@ -115,6 +115,7 @@ struct ivmd_header {
>  } __attribute__((packed));
>  
>  static int __initdata amd_iommu_detected;
> +static int __initdata amd_iommu_disable;
>  
>  u16 amd_iommu_last_bdf;			/* largest PCI device id we have
>  					   to handle */
> @@ -1037,7 +1038,7 @@ int __init amd_iommu_init(void)
>  	int i, ret = 0;
>  
>  
> -	if (no_iommu) {
> +	if (no_iommu || amd_iommu_disable) {
>  		printk(KERN_INFO "AMD IOMMU disabled by kernel command line\n");
>  		return 0;
>  	}
> @@ -1189,7 +1190,8 @@ static int __init early_amd_iommu_detect(struct acpi_table_header *table)
>  
>  void __init amd_iommu_detect(void)
>  {
> -	if (swiotlb || no_iommu || (iommu_detected && !gart_iommu_aperture))
> +	if (swiotlb || no_iommu || (iommu_detected && !gart_iommu_aperture) ||
> +		amd_iommu_disable)
>  		return;
>  
>  	if (acpi_table_parse("IVRS", early_amd_iommu_detect) == 0) {
> @@ -1212,6 +1214,8 @@ void __init amd_iommu_detect(void)
>  static int __init parse_amd_iommu_options(char *str)
>  {
>  	for (; *str; ++str) {
> +		if (!strncmp(str, "off", 3))
> +			amd_iommu_disable = 1;
>  		if (strncmp(str, "isolate", 7) == 0)
>  			amd_iommu_isolate = 1;
>  	}
> -- 
> 1.5.4.2
> 
> 

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
  2008-09-22 14:07 ` Joerg Roedel
@ 2008-09-22 14:16   ` FUJITA Tomonori
  2008-09-22 14:27     ` Joerg Roedel
  2008-09-22 14:46   ` Ingo Molnar
  1 sibling, 1 reply; 11+ messages in thread
From: FUJITA Tomonori @ 2008-09-22 14:16 UTC (permalink / raw)
  To: joerg.roedel; +Cc: fujita.tomonori, mingo, linux-kernel

On Mon, 22 Sep 2008 16:07:49 +0200
Joerg Roedel <joerg.roedel@amd.com> wrote:

> On Mon, Sep 22, 2008 at 10:35:08PM +0900, FUJITA Tomonori wrote:
> > This is against tip/x86/iommu.
> > 
> > =
> > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Subject: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
> > 
> > This reverts:
> > 
> > commit 8b14518fadd9d5915827d86d5c10e602fedf042e
> > Author: Joerg Roedel <joerg.roedel@amd.com>
> > Date:   Thu Jul 3 19:35:09 2008 +0200
> > 
> >     x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off
> > 
> >     This patch removes the amd_iommu=off kernel parameter and honors the generic
> > 
> >     iommu=off parameter for the same purpose.
> > 
> > 
> > The above commit is wrong.
> 
> It isn't. The user normally don't care about the type of IOMMU in the
> system. So disabling it with iommu=off is the right way. To achieve what
> you want its better to add iommu=gart and iommu=amd to the option
> parser. This will be consistent with Calgary and SWIOTLB too.

I'm not talking about what the best way to disable IOMMUs for
users. I'm talking about how the current code works. It's a different
topic. If you want to fix this, that's fine.

I just pointed out:

- You think that iommu=off and amd_iommu=off worked in the same way,
but they didn't.

- This commit removed the useful feature.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
  2008-09-22 14:16   ` FUJITA Tomonori
@ 2008-09-22 14:27     ` Joerg Roedel
  2008-09-22 15:25       ` FUJITA Tomonori
  0 siblings, 1 reply; 11+ messages in thread
From: Joerg Roedel @ 2008-09-22 14:27 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: mingo, linux-kernel

On Mon, Sep 22, 2008 at 11:16:25PM +0900, FUJITA Tomonori wrote:
> On Mon, 22 Sep 2008 16:07:49 +0200
> Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > On Mon, Sep 22, 2008 at 10:35:08PM +0900, FUJITA Tomonori wrote:
> > > This is against tip/x86/iommu.
> > > 
> > > =
> > > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > > Subject: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
> > > 
> > > This reverts:
> > > 
> > > commit 8b14518fadd9d5915827d86d5c10e602fedf042e
> > > Author: Joerg Roedel <joerg.roedel@amd.com>
> > > Date:   Thu Jul 3 19:35:09 2008 +0200
> > > 
> > >     x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off
> > > 
> > >     This patch removes the amd_iommu=off kernel parameter and honors the generic
> > > 
> > >     iommu=off parameter for the same purpose.
> > > 
> > > 
> > > The above commit is wrong.
> > 
> > It isn't. The user normally don't care about the type of IOMMU in the
> > system. So disabling it with iommu=off is the right way. To achieve what
> > you want its better to add iommu=gart and iommu=amd to the option
> > parser. This will be consistent with Calgary and SWIOTLB too.
> 
> I'm not talking about what the best way to disable IOMMUs for
> users. I'm talking about how the current code works. It's a different
> topic. If you want to fix this, that's fine.
> 
> I just pointed out:
> 
> - You think that iommu=off and amd_iommu=off worked in the same way,
> but they didn't.
> 
> - This commit removed the useful feature.

Ok, let me explain it this way. Before there was an AMD IOMMU driver
the user had to pass iommu=off to disable iommu usage at all and
iommu=soft to just disable usage of hardware IOMMUs. Further you have
iommu=calgary to select the Calgary driver. With AMD IOMMU the
situation is not very different. You can have two different hardware
IOMMUs in the system: GART and AMD IOMMU. Passing iommu=off still
disables all hardware IOMMUs. Your patch doesn't change that too. But to
be consistent with whats already there, the best solution to achieve
what you want it implementing iommu=gart and iommu=amd to select which
hardware IOMMU in the system you want to use.

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
  2008-09-22 14:07 ` Joerg Roedel
  2008-09-22 14:16   ` FUJITA Tomonori
@ 2008-09-22 14:46   ` Ingo Molnar
  2008-09-22 14:59     ` Joerg Roedel
  1 sibling, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2008-09-22 14:46 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: FUJITA Tomonori, linux-kernel


* Joerg Roedel <joerg.roedel@amd.com> wrote:

> >     x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off
> > 
> >     This patch removes the amd_iommu=off kernel parameter and honors 
> >     the generic
> > 
> >     iommu=off parameter for the same purpose.
> > 
> > 
> > The above commit is wrong.
> 
> It isn't. The user normally don't care about the type of IOMMU in the 
> system. So disabling it with iommu=off is the right way. To achieve 
> what you want its better to add iommu=gart and iommu=amd to the option 
> parser. This will be consistent with Calgary and SWIOTLB too.

well, what matters in the end is to have a consistent set of exclusion 
options:

  gart_iommu=off     # disable the GART (and only that one)
  intel_iommu=off    # disable the Intel IOMMU (and only that one)
  amd_iommu=off      # disable AMD-IOMMU (and only that one)

then there's the all-off option:

  iommu=off          # wildcard: disable all IOMMUs

[ whether iommu=off also disables the swiotlb is a detail. ]

and we could also do the inclusive options in addition:

  iommu=gart         # use the GART as the primary IOMMU [if available]
  iommu=amd          # use the AMD-IOMMU as the primary IOMMU [if available]
  iommu=intel        # use the Intel IOMMU [if available]

so could we please first agree on such a specific list of generic 
options, and then implement them consistently, while keeping legacies 
(the ones that matter) intact as well?

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
  2008-09-22 14:46   ` Ingo Molnar
@ 2008-09-22 14:59     ` Joerg Roedel
  0 siblings, 0 replies; 11+ messages in thread
From: Joerg Roedel @ 2008-09-22 14:59 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: FUJITA Tomonori, linux-kernel

On Mon, Sep 22, 2008 at 04:46:01PM +0200, Ingo Molnar wrote:
> 
> * Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > >     x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off
> > > 
> > >     This patch removes the amd_iommu=off kernel parameter and honors 
> > >     the generic
> > > 
> > >     iommu=off parameter for the same purpose.
> > > 
> > > 
> > > The above commit is wrong.
> > 
> > It isn't. The user normally don't care about the type of IOMMU in the 
> > system. So disabling it with iommu=off is the right way. To achieve 
> > what you want its better to add iommu=gart and iommu=amd to the option 
> > parser. This will be consistent with Calgary and SWIOTLB too.
> 
> well, what matters in the end is to have a consistent set of exclusion 
> options:
> 
>   gart_iommu=off     # disable the GART (and only that one)
>   intel_iommu=off    # disable the Intel IOMMU (and only that one)
>   amd_iommu=off      # disable AMD-IOMMU (and only that one)
> 
> then there's the all-off option:
> 
>   iommu=off          # wildcard: disable all IOMMUs
> 
> [ whether iommu=off also disables the swiotlb is a detail. ]
> 
> and we could also do the inclusive options in addition:
> 
>   iommu=gart         # use the GART as the primary IOMMU [if available]
>   iommu=amd          # use the AMD-IOMMU as the primary IOMMU [if available]
>   iommu=intel        # use the Intel IOMMU [if available]
> 
> so could we please first agree on such a specific list of generic 
> options, and then implement them consistently, while keeping legacies 
> (the ones that matter) intact as well?

Ok, having $(type)_iommu=off in addition to iommu=$type is surely a good
thing.

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
  2008-09-22 14:27     ` Joerg Roedel
@ 2008-09-22 15:25       ` FUJITA Tomonori
  2008-09-22 16:31         ` Joerg Roedel
  0 siblings, 1 reply; 11+ messages in thread
From: FUJITA Tomonori @ 2008-09-22 15:25 UTC (permalink / raw)
  To: joerg.roedel; +Cc: fujita.tomonori, mingo, linux-kernel

On Mon, 22 Sep 2008 16:27:13 +0200
Joerg Roedel <joerg.roedel@amd.com> wrote:

> On Mon, Sep 22, 2008 at 11:16:25PM +0900, FUJITA Tomonori wrote:
> > On Mon, 22 Sep 2008 16:07:49 +0200
> > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > 
> > > On Mon, Sep 22, 2008 at 10:35:08PM +0900, FUJITA Tomonori wrote:
> > > > This is against tip/x86/iommu.
> > > > 
> > > > =
> > > > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > > > Subject: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
> > > > 
> > > > This reverts:
> > > > 
> > > > commit 8b14518fadd9d5915827d86d5c10e602fedf042e
> > > > Author: Joerg Roedel <joerg.roedel@amd.com>
> > > > Date:   Thu Jul 3 19:35:09 2008 +0200
> > > > 
> > > >     x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off
> > > > 
> > > >     This patch removes the amd_iommu=off kernel parameter and honors the generic
> > > > 
> > > >     iommu=off parameter for the same purpose.
> > > > 
> > > > 
> > > > The above commit is wrong.
> > > 
> > > It isn't. The user normally don't care about the type of IOMMU in the
> > > system. So disabling it with iommu=off is the right way. To achieve what
> > > you want its better to add iommu=gart and iommu=amd to the option
> > > parser. This will be consistent with Calgary and SWIOTLB too.
> > 
> > I'm not talking about what the best way to disable IOMMUs for
> > users. I'm talking about how the current code works. It's a different
> > topic. If you want to fix this, that's fine.
> > 
> > I just pointed out:
> > 
> > - You think that iommu=off and amd_iommu=off worked in the same way,
> > but they didn't.
> > 
> > - This commit removed the useful feature.
> 
> Ok, let me explain it this way. Before there was an AMD IOMMU driver
> the user had to pass iommu=off to disable iommu usage at all and
> iommu=soft to just disable usage of hardware IOMMUs.

iommu=soft is not the correct way to disable usage of hardware
IOMMUs. You don't always need SWIOTLB. pci-nommu.c is better if
possible.

> Further you have iommu=calgary to select the Calgary driver.

I don't think that users need to have iommu=calgary parameter to use
calgary IOMMU by default. If you enable CONFIG_CALGARY_IOMMU,
CALGARY_IOMMU_ENABLED_BY_DEFAULT is also enabled by default. If a
kernel finds calgary IOMMU, the kernel uses it by default.

It's also consistent with how SWIOTLB. Users don't need iommu=soft
parameter to enable SWIOTLB. A kernel enables SWIOTLB automatically
when necessary.


Well, this is the problem about the IOMMUs parameters. The IOMMUs
parameters are too complicated for everyone.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
  2008-09-22 15:25       ` FUJITA Tomonori
@ 2008-09-22 16:31         ` Joerg Roedel
  2008-09-22 17:07           ` FUJITA Tomonori
  0 siblings, 1 reply; 11+ messages in thread
From: Joerg Roedel @ 2008-09-22 16:31 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: mingo, linux-kernel

On Tue, Sep 23, 2008 at 12:25:23AM +0900, FUJITA Tomonori wrote:
> I don't think that users need to have iommu=calgary parameter to use
> calgary IOMMU by default. If you enable CONFIG_CALGARY_IOMMU,
> CALGARY_IOMMU_ENABLED_BY_DEFAULT is also enabled by default. If a
> kernel finds calgary IOMMU, the kernel uses it by default.
> 
> It's also consistent with how SWIOTLB. Users don't need iommu=soft
> parameter to enable SWIOTLB. A kernel enables SWIOTLB automatically
> when necessary.

Yes. The parameters are usefull if a user wants to enable a specific
IOMMU implementation. The user could be an IOMMU developer testing
changes in a special implementation the kernel would not choose by
default on his machine.
I like Ingo's idea here. Lets do boths, implementing iommu=$type to
force a specific iommu implementation and $(type)_iommu=off to disable
one.

> Well, this is the problem about the IOMMUs parameters. The IOMMUs
> parameters are too complicated for everyone.

Not so complicated that they can't be understood. But if you have a
proposal how the command line parameters for iommus may look like, send
it. I like join that discussion about the interface.

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
  2008-09-22 16:31         ` Joerg Roedel
@ 2008-09-22 17:07           ` FUJITA Tomonori
  2008-09-22 17:22             ` Joerg Roedel
  2008-09-22 17:52             ` Ingo Molnar
  0 siblings, 2 replies; 11+ messages in thread
From: FUJITA Tomonori @ 2008-09-22 17:07 UTC (permalink / raw)
  To: joerg.roedel; +Cc: fujita.tomonori, mingo, linux-kernel

On Mon, 22 Sep 2008 18:31:09 +0200
Joerg Roedel <joerg.roedel@amd.com> wrote:

> On Tue, Sep 23, 2008 at 12:25:23AM +0900, FUJITA Tomonori wrote:
> > I don't think that users need to have iommu=calgary parameter to use
> > calgary IOMMU by default. If you enable CONFIG_CALGARY_IOMMU,
> > CALGARY_IOMMU_ENABLED_BY_DEFAULT is also enabled by default. If a
> > kernel finds calgary IOMMU, the kernel uses it by default.
> > 
> > It's also consistent with how SWIOTLB. Users don't need iommu=soft
> > parameter to enable SWIOTLB. A kernel enables SWIOTLB automatically
> > when necessary.
> 
> Yes. The parameters are usefull if a user wants to enable a specific
> IOMMU implementation. The user could be an IOMMU developer testing
> changes in a special implementation the kernel would not choose by
> default on his machine.
> I like Ingo's idea here. Lets do boths, implementing iommu=$type to
> force a specific iommu implementation and $(type)_iommu=off to disable
> one.

This is exactly what I've been against, something like "I like it and
let do it".

Sure, Ingo's suggestion looks consistent, but these parameters
interact other parameters. If we are serious about improving the IOMMU
parameters, we need to spend more time to think about the final
picture of all the IOMMU parameters that we need.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
  2008-09-22 17:07           ` FUJITA Tomonori
@ 2008-09-22 17:22             ` Joerg Roedel
  2008-09-22 17:52             ` Ingo Molnar
  1 sibling, 0 replies; 11+ messages in thread
From: Joerg Roedel @ 2008-09-22 17:22 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: mingo, linux-kernel

On Tue, Sep 23, 2008 at 02:07:07AM +0900, FUJITA Tomonori wrote:
> 
> Sure, Ingo's suggestion looks consistent, but these parameters
> interact other parameters. If we are serious about improving the IOMMU
> parameters, we need to spend more time to think about the final
> picture of all the IOMMU parameters that we need.
> 

I am curiously looking forward to your proposal.

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
  2008-09-22 17:07           ` FUJITA Tomonori
  2008-09-22 17:22             ` Joerg Roedel
@ 2008-09-22 17:52             ` Ingo Molnar
  1 sibling, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2008-09-22 17:52 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: joerg.roedel, linux-kernel


* FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> Sure, Ingo's suggestion looks consistent, but these parameters 
> interact other parameters. If we are serious about improving the IOMMU 
> parameters, we need to spend more time to think about the final 
> picture of all the IOMMU parameters that we need.

yeah, definitely.

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-09-22 17:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-22 13:35 [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off" FUJITA Tomonori
2008-09-22 14:07 ` Joerg Roedel
2008-09-22 14:16   ` FUJITA Tomonori
2008-09-22 14:27     ` Joerg Roedel
2008-09-22 15:25       ` FUJITA Tomonori
2008-09-22 16:31         ` Joerg Roedel
2008-09-22 17:07           ` FUJITA Tomonori
2008-09-22 17:22             ` Joerg Roedel
2008-09-22 17:52             ` Ingo Molnar
2008-09-22 14:46   ` Ingo Molnar
2008-09-22 14:59     ` Joerg Roedel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox