2008-07-03 17:35:32

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 0/6] AMD IOMMU updates

Hi,

this series of patches contain some updates to the AMD IOMMU code currently in
tip/master. The updates address some objections discussed on the initial
patches and also contain optimization and small code cleanup.
The code with these updates is tested on real hardware under load and showed no
problems.

diffstat:

arch/x86/Kconfig | 10 +++++++++-
arch/x86/kernel/amd_iommu.c | 15 ++++++++++-----
arch/x86/kernel/amd_iommu_init.c | 16 ++++++----------
include/asm-x86/amd_iommu_types.h | 2 ++
4 files changed, 27 insertions(+), 16 deletions(-)



2008-07-03 17:35:45

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 5/6] AMD IOMMU: don't try to init IOMMU if early detect code did not detect one

This patch adds a check if the early detect code has found AMD IOMMU hardware
descriptions and does not try to initialize hardware if the check failed.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/kernel/amd_iommu_init.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index c54c823..c970fac 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -101,6 +101,8 @@ struct ivmd_header {
u64 range_length;
} __attribute__((packed));

+static int __initdata amd_iommu_detected;
+
u16 amd_iommu_last_bdf;
struct list_head amd_iommu_unity_map;
unsigned amd_iommu_aperture_order = 26;
@@ -689,6 +691,9 @@ int __init amd_iommu_init(void)
return 0;
}

+ if (!amd_iommu_detected)
+ return -ENODEV;
+
/*
* First parse ACPI tables to find the largest Bus/Dev/Func
* we need to handle. Upon this information the shared data
@@ -831,6 +836,7 @@ void __init amd_iommu_detect(void)

if (acpi_table_parse("IVRS", early_amd_iommu_detect) == 0) {
iommu_detected = 1;
+ amd_iommu_detected = 1;
#ifdef CONFIG_GART_IOMMU
gart_iommu_aperture_disabled = 1;
gart_iommu_aperture = 0;
--
1.5.3.7

2008-07-03 17:36:22

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 3/6] AMD IOMMU: flush domain TLB when there is more than one page to flush

This patch changes the domain TLB flushing behavior of the driver. When there
is more than one page to flush it flushes the whole domain TLB instead of every
single page. So we send only a single command to the IOMMU in every case which
is faster to execute.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/kernel/amd_iommu.c | 14 ++++++++++----
include/asm-x86/amd_iommu_types.h | 2 ++
2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 329b2c3..f2766d8 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -140,16 +140,22 @@ static int iommu_queue_inv_iommu_pages(struct amd_iommu *iommu,
static int iommu_flush_pages(struct amd_iommu *iommu, u16 domid,
u64 address, size_t size)
{
- int i;
+ int s = 0;
unsigned pages = to_pages(address, size);

address &= PAGE_MASK;

- for (i = 0; i < pages; ++i) {
- iommu_queue_inv_iommu_pages(iommu, address, domid, 0, 0);
- address += PAGE_SIZE;
+ if (pages > 1) {
+ /*
+ * If we have to flush more than one page, flush all
+ * TLB entries for this domain
+ */
+ address = CMD_INV_IOMMU_ALL_PAGES_ADDRESS;
+ s = 1;
}

+ iommu_queue_inv_iommu_pages(iommu, address, domid, 0, s);
+
return 0;
}

diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index 0f39550..7bfcb47 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -93,6 +93,8 @@
#define CMD_INV_IOMMU_PAGES_SIZE_MASK 0x01
#define CMD_INV_IOMMU_PAGES_PDE_MASK 0x02

+#define CMD_INV_IOMMU_ALL_PAGES_ADDRESS 0x7fffffffffffffffULL
+
/* macros and definitions for device table entries */
#define DEV_ENTRY_VALID 0x00
#define DEV_ENTRY_TRANSLATION 0x01
--
1.5.3.7

2008-07-03 17:35:59

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 1/6] AMD IOMMU: more verbose Kconfig description text

This patch replaces the short description text for AMD IOMMU in Kconfig with a
more verbose one.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/Kconfig | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index dc6425e..c31047a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -568,7 +568,15 @@ config AMD_IOMMU
select SWIOTLB
depends on X86_64 && PCI && ACPI
help
- Select this to get support for AMD IOMMU hardware in your system.
+ With this option you can enable support for AMD IOMMU hardware in
+ your system. An IOMMU is a hardware component which provides
+ remapping of DMA memory accesses from devices. With an AMD IOMMU you
+ can isolate the the DMA memory of different devices and protect the
+ system from misbehaving device drivers or hardware.
+
+ You can find out if your system has an AMD IOMMU if you look into
+ your BIOS for an option to enable it or if you have an IVRS ACPI
+ table.

# need this always selected by IOMMU for the VIA workaround
config SWIOTLB
--
1.5.3.7

2008-07-03 17:38:16

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 2/6] AMD IOMMU: remove unnecessary set_bit_string

The set_bit_string call in the address allocator is not necessary because its
already called in iommu_area_alloc().

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/kernel/amd_iommu.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index a1b3856..329b2c3 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -301,7 +301,6 @@ static unsigned long dma_ops_alloc_addresses(struct device *dev,
0, boundary_size, 0);

if (likely(address != -1)) {
- set_bit_string(dom->bitmap, address, pages);
dom->next_bit = address + pages;
address <<= PAGE_SHIFT;
} else
--
1.5.3.7

2008-07-03 18:00:17

by Duran, Leo

[permalink] [raw]
Subject: RE: [PATCH 3/6] AMD IOMMU: flush domain TLB when there is more than onepage to flush

On Thursday, July 03, 2008 12:35 PM, Joerg Roedel wrote:

> This patch changes the domain TLB flushing behavior of the driver.
When
> there
> is more than one page to flush it flushes the whole domain TLB instead
> of every
> single page. So we send only a single command to the IOMMU in every
> case which
> is faster to execute.
>

Joerg,

Instead of flushing the complete TLB, how about just setting the
page-size encoding bits so as to cover all of the pages (rounded up to a
power of 2... which in some cases may flush a few more pages, but not
the complete TLB). This way other devices don't pay the 'flushing'
penalty every time you unmap a buffer for a given device.

Leo.

2008-07-03 18:52:59

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 6/6] AMD IOMMU: remove unnecessary code from the iommu_enable function

This code removes a leftover from the iommu_enable function. The ctrl variable
is assigned but never used.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/kernel/amd_iommu_init.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index c970fac..2a13e43 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -169,14 +169,11 @@ static void __init iommu_feature_disable(struct amd_iommu *iommu, u8 bit)

void __init iommu_enable(struct amd_iommu *iommu)
{
- u32 ctrl;
-
printk(KERN_INFO "AMD IOMMU: Enabling IOMMU at ");
print_devid(iommu->devid, 0);
printk(" cap 0x%hx\n", iommu->cap_ptr);

iommu_feature_enable(iommu, CONTROL_IOMMU_EN);
- ctrl = readl(iommu->mmio_base + MMIO_CONTROL_OFFSET);
}

static u8 * __init iommu_map_mmio_space(u64 address)
--
1.5.3.7

2008-07-03 19:05:30

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 4/6] 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.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/kernel/amd_iommu_init.c | 9 +--------
1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index 5d9e45c..c54c823 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -101,8 +101,6 @@ struct ivmd_header {
u64 range_length;
} __attribute__((packed));

-static int __initdata amd_iommu_disable;
-
u16 amd_iommu_last_bdf;
struct list_head amd_iommu_unity_map;
unsigned amd_iommu_aperture_order = 26;
@@ -686,7 +684,7 @@ int __init amd_iommu_init(void)
int i, ret = 0;


- if (amd_iommu_disable) {
+ if (no_iommu) {
printk(KERN_INFO "AMD IOMMU disabled by kernel command line\n");
return 0;
}
@@ -831,9 +829,6 @@ void __init amd_iommu_detect(void)
if (swiotlb || no_iommu || iommu_detected)
return;

- if (amd_iommu_disable)
- return;
-
if (acpi_table_parse("IVRS", early_amd_iommu_detect) == 0) {
iommu_detected = 1;
#ifdef CONFIG_GART_IOMMU
@@ -846,8 +841,6 @@ void __init amd_iommu_detect(void)
static int __init parse_amd_iommu_options(char *str)
{
for (; *str; ++str) {
- if (strcmp(str, "off") == 0)
- amd_iommu_disable = 1;
if (strcmp(str, "isolate") == 0)
amd_iommu_isolate = 1;
}
--
1.5.3.7

2008-07-04 09:52:55

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 0/6] AMD IOMMU updates


* Joerg Roedel <[email protected]> wrote:

> Hi,
>
> this series of patches contain some updates to the AMD IOMMU code
> currently in tip/master. The updates address some objections discussed
> on the initial patches and also contain optimization and small code
> cleanup. The code with these updates is tested on real hardware under
> load and showed no problems.

applied to tip/x86/amd-iommu and pushed that topic out, thanks Joerg.

also, i've integrated it into tip/master as well (will push that out
after some testing).

The IOMMU driver code seems to be getting into good shape, so i've also
added the topic to auto-x86-next, which means it will go into linux-next
on the next iteration (in 1-2 days). Does anyone see any remaining
problems?

Ingo

2008-07-07 07:42:30

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH 0/6] AMD IOMMU updates

On Fri, 4 Jul 2008 11:52:13 +0200
Ingo Molnar <[email protected]> wrote:

>
> * Joerg Roedel <[email protected]> wrote:
>
> > Hi,
> >
> > this series of patches contain some updates to the AMD IOMMU code
> > currently in tip/master. The updates address some objections discussed
> > on the initial patches and also contain optimization and small code
> > cleanup. The code with these updates is tested on real hardware under
> > load and showed no problems.
>
> applied to tip/x86/amd-iommu and pushed that topic out, thanks Joerg.
>
> also, i've integrated it into tip/master as well (will push that out
> after some testing).
>
> The IOMMU driver code seems to be getting into good shape, so i've also
> added the topic to auto-x86-next, which means it will go into linux-next
> on the next iteration (in 1-2 days). Does anyone see any remaining
> problems?

Not a showstopper but I like to see the cleanups of the kernel
parameters, as I said before. The AMD IOMMU has two parameters,
amd_iommu=isolate and amd_iommu_size=32M|64M|128M|256M|512M|1G

The former might be useful for the VT-d. The latter is useful for most
of the IOMMU drivers. So I think that it would be better to make them
common parameters.

And if we can change the existing kernel parameters, I think that it
would be better to convert some of the x86 IOMMU parameters to common
parameters.


BTW, "[PATCH 4/6] AMD IOMMU: honor iommu=off instead of amd_iommu=off"
patch in auto-x86-next needs update the kernel parameter.

=
From: FUJITA Tomonori <[email protected]>
Subject: [PATCH] AMD IOMMU: fix kernel parameter

amd_iommu=off was replaced with a common parameter, iommu=off.

Signed-off-by: FUJITA Tomonori <[email protected]>
---
Documentation/kernel-parameters.txt | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index cd98762..1a6bb46 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -274,7 +274,6 @@ 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 the driver for AMD IOMMU
isolate - enable device isolation (each device, as far
as possible, will get its own protection
domain)
--
1.5.5.GIT

2008-07-07 07:44:44

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 0/6] AMD IOMMU updates


* FUJITA Tomonori <[email protected]> wrote:

> BTW, "[PATCH 4/6] AMD IOMMU: honor iommu=off instead of amd_iommu=off"
> patch in auto-x86-next needs update the kernel parameter.
>
> =
> From: FUJITA Tomonori <[email protected]>
> Subject: [PATCH] AMD IOMMU: fix kernel parameter
>
> amd_iommu=off was replaced with a common parameter, iommu=off.
>
> Signed-off-by: FUJITA Tomonori <[email protected]>

applied to tip/x86/amd-iommu, thanks.

Ingo