[PATCH 0/4] AMD IOMMU updates for 2.6.27-rc3

[PATCH 0/4] AMD IOMMU updates for 2.6.27-rc3

[Joerg Roedel]

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

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

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 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

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

[Joerg Roedel]

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

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 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

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

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 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

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

[Joerg Roedel]

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 <joerg.roedel@amd.com>
---
 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

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

[Ingo Oeser]

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

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

[Joerg Roedel]

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

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

[Ingo Molnar]

* Joerg Roedel <joerg.roedel@amd.com> 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