lkml.org 
[lkml]   [2008]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] AMD IOMMU: revert "x86, AMD IOMMU: honor iommu=off instead of amd_iommu=off"
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



\
 
 \ /
  Last update: 2008-09-22 16:11    [W:0.054 / U:0.992 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site