2008-08-14 17:56:16

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 0/4] AMD IOMMU updates for 2.6.27-rc3

Hi Ingo,

here are 4 update patches for AMD IOMMU which should be merged in 2.6.27. They
fix various issues that came up during tests. They also fix some minor
style issues.

Joerg

git diff --stat tip/master.. :

arch/x86/kernel/amd_iommu.c | 19 +++++++++++--------
arch/x86/kernel/amd_iommu_init.c | 24 +++++++++++++++++++++---
include/asm-x86/amd_iommu_types.h | 8 +++++---
3 files changed, 37 insertions(+), 14 deletions(-)



2008-08-14 17:55:51

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 2/4] AMD IOMMU: initialize device table properly

This patch adds device table initializations which forbids memory accesses for
devices per default and disables all page faults.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/kernel/amd_iommu_init.c | 18 ++++++++++++++++++
include/asm-x86/amd_iommu_types.h | 1 +
2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index d9a9da5..ceba338 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -801,6 +801,21 @@ static int __init init_memory_definitions(struct acpi_table_header *table)
}

/*
+ * Init the device table to not allow DMA access for devices and
+ * suppress all page faults
+ */
+static void init_device_table(void)
+{
+ u16 devid;
+
+ for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
+ set_dev_entry_bit(devid, DEV_ENTRY_VALID);
+ set_dev_entry_bit(devid, DEV_ENTRY_TRANSLATION);
+ set_dev_entry_bit(devid, DEV_ENTRY_NO_PAGE_FAULT);
+ }
+}
+
+/*
* This function finally enables all IOMMUs found in the system after
* they have been initialized
*/
@@ -931,6 +946,9 @@ int __init amd_iommu_init(void)
if (amd_iommu_pd_alloc_bitmap == NULL)
goto free;

+ /* init the device table */
+ init_device_table();
+
/*
* let all alias entries point to itself
*/
diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index 2c23834..01412f0 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -103,6 +103,7 @@
#define DEV_ENTRY_TRANSLATION 0x01
#define DEV_ENTRY_IR 0x3d
#define DEV_ENTRY_IW 0x3e
+#define DEV_ENTRY_NO_PAGE_FAULT 0x62
#define DEV_ENTRY_EX 0x67
#define DEV_ENTRY_SYSMGT1 0x68
#define DEV_ENTRY_SYSMGT2 0x69
--
1.5.3.7

2008-08-14 17:56:31

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 4/4] AMD IOMMU: initialize dma_ops after sysfs registration

If sysfs registration fails all memory used by IOMMU is freed. This happens
after dma_ops initialization and the functions will access the freed memory
then. Fix this by initializing dma_ops after the sysfs registration.

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

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index ceba338..a69cc0f 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -972,15 +972,15 @@ int __init amd_iommu_init(void)
if (acpi_table_parse("IVRS", init_memory_definitions) != 0)
goto free;

- ret = amd_iommu_init_dma_ops();
+ ret = sysdev_class_register(&amd_iommu_sysdev_class);
if (ret)
goto free;

- ret = sysdev_class_register(&amd_iommu_sysdev_class);
+ ret = sysdev_register(&device_amd_iommu);
if (ret)
goto free;

- ret = sysdev_register(&device_amd_iommu);
+ ret = amd_iommu_init_dma_ops();
if (ret)
goto free;

--
1.5.3.7

2008-08-14 17:56:50

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 3/4] AMD IOMMU: replace LOW_U32 macro with generic lower_32_bits

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

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 028e945..de39e1f 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -164,7 +164,7 @@ static int iommu_queue_inv_iommu_pages(struct amd_iommu *iommu,
address &= PAGE_MASK;
CMD_SET_TYPE(&cmd, CMD_INV_IOMMU_PAGES);
cmd.data[1] |= domid;
- cmd.data[2] = LOW_U32(address);
+ cmd.data[2] = lower_32_bits(address);
cmd.data[3] = upper_32_bits(address);
if (s) /* size bit - we flush more than one 4kb page */
cmd.data[2] |= CMD_INV_IOMMU_PAGES_SIZE_MASK;
diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index 01412f0..1ffa4e5 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -31,9 +31,6 @@
#define ALIAS_TABLE_ENTRY_SIZE 2
#define RLOOKUP_TABLE_ENTRY_SIZE (sizeof(void *))

-/* helper macros */
-#define LOW_U32(x) ((x) & ((1ULL << 32)-1))
-
/* Length of the MMIO region for the AMD IOMMU */
#define MMIO_REGION_LENGTH 0x4000

--
1.5.3.7

2008-08-14 17:57:35

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 1/4] AMD IOMMU: use status bit instead of memory write-back for completion wait

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

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 22d7d05..028e945 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -101,16 +101,13 @@ static int iommu_queue_command(struct amd_iommu *iommu, struct iommu_cmd *cmd)
*/
static int iommu_completion_wait(struct amd_iommu *iommu)
{
- int ret;
+ int ret, ready = 0;
+ unsigned status = 0;
struct iommu_cmd cmd;
- volatile u64 ready = 0;
- unsigned long ready_phys = virt_to_phys(&ready);
unsigned long i = 0;

memset(&cmd, 0, sizeof(cmd));
- cmd.data[0] = LOW_U32(ready_phys) | CMD_COMPL_WAIT_STORE_MASK;
- cmd.data[1] = upper_32_bits(ready_phys);
- cmd.data[2] = 1; /* value written to 'ready' */
+ cmd.data[0] = CMD_COMPL_WAIT_INT_MASK;
CMD_SET_TYPE(&cmd, CMD_COMPL_WAIT);

iommu->need_sync = 0;
@@ -122,9 +119,15 @@ static int iommu_completion_wait(struct amd_iommu *iommu)

while (!ready && (i < EXIT_LOOP_COUNT)) {
++i;
- cpu_relax();
+ /* wait for the bit to become one */
+ status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
+ ready = status & MMIO_STATUS_COM_WAIT_INT_MASK;
}

+ /* set bit back to zero */
+ status &= ~MMIO_STATUS_COM_WAIT_INT_MASK;
+ writel(status, iommu->mmio_base + MMIO_STATUS_OFFSET);
+
if (unlikely((i == EXIT_LOOP_COUNT) && printk_ratelimit()))
printk(KERN_WARNING "AMD IOMMU: Completion wait loop failed\n");

diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index e6b4d5b..2c23834 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -69,6 +69,9 @@
#define MMIO_EVT_TAIL_OFFSET 0x2018
#define MMIO_STATUS_OFFSET 0x2020

+/* MMIO status bits */
+#define MMIO_STATUS_COM_WAIT_INT_MASK 0x04
+
/* feature control bits */
#define CONTROL_IOMMU_EN 0x00ULL
#define CONTROL_HT_TUN_EN 0x01ULL
@@ -89,6 +92,7 @@
#define CMD_INV_IOMMU_PAGES 0x03

#define CMD_COMPL_WAIT_STORE_MASK 0x01
+#define CMD_COMPL_WAIT_INT_MASK 0x02
#define CMD_INV_IOMMU_PAGES_SIZE_MASK 0x01
#define CMD_INV_IOMMU_PAGES_PDE_MASK 0x02

--
1.5.3.7

2008-08-14 18:42:22

by Ingo Oeser

[permalink] [raw]
Subject: Re: [PATCH 1/4] AMD IOMMU: use status bit instead of memory write-back for completion wait

Hi Joerg,

On Thursday 14 August 2008, Joerg Roedel wrote:
> diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
> index 22d7d05..028e945 100644
> --- a/arch/x86/kernel/amd_iommu.c
> +++ b/arch/x86/kernel/amd_iommu.c
> @@ -122,9 +119,15 @@ static int iommu_completion_wait(struct amd_iommu *iommu)
>
> while (!ready && (i < EXIT_LOOP_COUNT)) {
> ++i;
> - cpu_relax();

Could you elaborate, why you had to remove this?

Busy waiting loops should always do cpu_relay()
to reduce heat procution and power consumption while idle.

> + /* wait for the bit to become one */
> + status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
> + ready = status & MMIO_STATUS_COM_WAIT_INT_MASK;
> }
>

Best Regards

Ingo Oeser

2008-08-14 20:02:58

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 1/4] AMD IOMMU: use status bit instead of memory write-back for completion wait

On Thu, Aug 14, 2008 at 08:35:30PM +0200, Ingo Oeser wrote:
> Hi Joerg,
>
> On Thursday 14 August 2008, Joerg Roedel wrote:
> > diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
> > index 22d7d05..028e945 100644
> > --- a/arch/x86/kernel/amd_iommu.c
> > +++ b/arch/x86/kernel/amd_iommu.c
> > @@ -122,9 +119,15 @@ static int iommu_completion_wait(struct amd_iommu *iommu)
> >
> > while (!ready && (i < EXIT_LOOP_COUNT)) {
> > ++i;
> > - cpu_relax();
>
> Could you elaborate, why you had to remove this?
>
> Busy waiting loops should always do cpu_relay()
> to reduce heat procution and power consumption while idle.

I removed it because its not idling anymore in that loop imho. It reads
data from MMIO and does a small calculation with it.

Joerg

2008-08-15 11:58:17

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 0/4] AMD IOMMU updates for 2.6.27-rc3


* Joerg Roedel <[email protected]> wrote:

> Hi Ingo,
>
> here are 4 update patches for AMD IOMMU which should be merged in
> 2.6.27. They fix various issues that came up during tests. They also
> fix some minor style issues.

applied to tip/x86/amd-iommu - and merged into tip/x86/urgent as well
for v2.6.27. Thanks Joerg,

Ingo