[PATCH 0/2] amd-iommu: kdump bugfix and couple defensive cleanups

[PATCH 0/2] amd-iommu: kdump bugfix and couple defensive cleanups

[Chris Wright]

The following patches fix a kdump issue and then fix some similar
initialization issues that haven't proven to be problematic but make
good sense anyway.

thanks,
-chris

[PATCH 1/2] amd-iommu: fix issues with kdump

[Chris Wright]

When kexec'ing to a new kernel (for example, when crashing and launching a
kdump session), the AMD IOMMU may have cached translations.  The kexec'd
kernel, during initialization, will invalidate the IOMMU device table
entries, but not the domain translations.  These stale entries can cause
a device's DMA to fail, makes it rough to write a dump to disk when the
disk controller can't DMA ;-)  Just invalidating the iommu pages should
be enough, however the old kernel left the IOMMU enabled and running.
In particular, the command buffer doesn't seem to work properly after
it's been updated while active.  Unlike the event log buffer which is
documented to have undefined behaviour when updated while enabled,
the documentation doesn't specify for the command buffer.  However,
disabling the command buffer before updating the IOMMU w/ new command
buffer and re-enabling is sufficient to ensure future commands are picked
up an processed properly, especially those critical invalidations.

Issue found here https://bugzilla.redhat.com/show_bug.cgi?id=504751, but
this also fixes current (2.6.30) Linux kernel.

Signed-off-by: Chris Wright <chrisw@sous-sol.org>
---
 arch/x86/kernel/amd_iommu.c      |   16 ++++++++++++++++
 arch/x86/kernel/amd_iommu_init.c |    3 +++
 2 files changed, 19 insertions(+)

--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -425,6 +425,16 @@ static void iommu_flush_tlb(struct amd_i
 	iommu_queue_inv_iommu_pages(iommu, address, domid, 0, 1);
 }
 
+/* Flush the whole IO/TLB for a given protection domain - including PDE */
+static void iommu_flush_tlb_pde(struct amd_iommu *iommu, u16 domid)
+{
+	u64 address = CMD_INV_IOMMU_ALL_PAGES_ADDRESS;
+
+	INC_STATS_COUNTER(domain_flush_single);
+
+	iommu_queue_inv_iommu_pages(iommu, address, domid, 1, 1);
+}
+
 /*
  * This function is used to flush the IO/TLB for a given protection domain
  * on every IOMMU in the system
@@ -934,7 +944,13 @@ static void attach_device(struct amd_iom
 	amd_iommu_pd_table[devid] = domain;
 	write_unlock_irqrestore(&amd_iommu_devtable_lock, flags);
 
+	/*
+	 * We might boot into a crash-kernel here. The crashed kernel
+	 * left the caches in the IOMMU dirty. So we have to flush
+	 * here to evict all dirty stuff.
+	 */
 	iommu_queue_inv_dev_entry(iommu, devid);
+	iommu_flush_tlb_pde(iommu, domain->id);
 }
 
 /*
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -420,6 +420,9 @@ static u8 * __init alloc_command_buffer(
 
 	iommu->cmd_buf_size = CMD_BUFFER_SIZE;
 
+	/* disable command buffer processing while setting it up */
+	iommu_feature_disable(iommu, CONTROL_CMDBUF_EN);
+
 	entry = (u64)virt_to_phys(cmd_buf);
 	entry |= MMIO_CMD_SIZE_512;
 	memcpy_toio(iommu->mmio_base + MMIO_CMD_BUF_OFFSET,

[PATCH 2/2] amd-iommu: initialization fixes

[Chris Wright]

If the kernel in initialized w/out coming out of reset (e.g. kexec/kdump), the
prior kernel may leave the IOMMU running.  Here's some fixes just to
ensure all initialization is done in a sane manner.  

1) disable IOMMU when discovered, will get renabled after it's set up
2) disable event log handling while setting up the event log buffer.
   the spec says:
      Software Note: If EventLen or EventBase is changed while the
      EventLogRun=1, the IOMMU behavior is undefined.
3) ensure the head/tail event log buffer pointers are initialized to zero

Signed-off-by: Chris Wright <chrisw@sous-sol.org>
---
 arch/x86/kernel/amd_iommu_init.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -252,6 +252,11 @@ static void __init iommu_enable(struct a
 	iommu_feature_enable(iommu, CONTROL_IOMMU_EN);
 }
 
+static void __init iommu_disable(struct amd_iommu *iommu)
+{
+	iommu_feature_disable(iommu, CONTROL_IOMMU_EN);
+}
+
 /* Function to enable IOMMU event logging and event interrupts */
 static void __init iommu_enable_event_logging(struct amd_iommu *iommu)
 {
@@ -259,6 +264,12 @@ static void __init iommu_enable_event_lo
 	iommu_feature_enable(iommu, CONTROL_EVT_INT_EN);
 }
 
+static void __init iommu_disable_event_logging(struct amd_iommu *iommu)
+{
+	iommu_feature_disable(iommu, CONTROL_EVT_INT_EN);
+	iommu_feature_disable(iommu, CONTROL_EVT_LOG_EN);
+}
+
 /*
  * mapping and unmapping functions for the IOMMU MMIO space. Each AMD IOMMU in
  * the system has one.
@@ -453,12 +464,19 @@ static u8 * __init alloc_event_buffer(st
 	if (iommu->evt_buf == NULL)
 		return NULL;
 
+	/* disable event logging while setting it up */
+	iommu_disable_event_logging(iommu);
+
 	entry = (u64)virt_to_phys(iommu->evt_buf) | EVT_LEN_MASK;
 	memcpy_toio(iommu->mmio_base + MMIO_EVT_BUF_OFFSET,
 		    &entry, sizeof(entry));
 
 	iommu->evt_buf_size = EVT_BUFFER_SIZE;
 
+	/* set head and tail to zero manually */
+	writel(0x00, iommu->mmio_base + MMIO_EVT_HEAD_OFFSET);
+	writel(0x00, iommu->mmio_base + MMIO_EVT_TAIL_OFFSET);
+
 	return iommu->evt_buf;
 }
 
@@ -713,6 +731,9 @@ static int __init init_iommu_one(struct 
 	if (!iommu->mmio_base)
 		return -ENOMEM;
 
+	/* disable IOMMU in case it was left on */
+	iommu_disable(iommu);
+
 	iommu_set_device_table(iommu);
 	iommu->cmd_buf = alloc_command_buffer(iommu);
 	if (!iommu->cmd_buf)

Re: [PATCH 0/2] amd-iommu: kdump bugfix and couple defensive cleanups

[Joerg Roedel]

On Thu, Jun 11, 2009 at 08:03:41PM -0700, Chris Wright wrote:
> The following patches fix a kdump issue and then fix some similar
> initialization issues that haven't proven to be problematic but make
> good sense anyway.

The patches look good but are based on 2.6.30. For 2.6.31-rc1 they must
be changed a bit because some functions implemented in these patches
were already merged for the suspen/resume support in amd-iommu driver.
I will adapt them for 2.6.31. Do you want to send these patches for
-stable inclusion?

	Joerg

-- 
           | Advanced Micro Devices GmbH
 Operating | Karl-Hammerschmidt-Str. 34, 85609 Dornach bei München
 System    | 
 Research  | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
 Center    | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
           | Registergericht München, HRB Nr. 43632

Re: [PATCH 0/2] amd-iommu: kdump bugfix and couple defensive cleanups

[Chris Wright]

* Joerg Roedel (joerg.roedel@amd.com) wrote:
> On Thu, Jun 11, 2009 at 08:03:41PM -0700, Chris Wright wrote:
> > The following patches fix a kdump issue and then fix some similar
> > initialization issues that haven't proven to be problematic but make
> > good sense anyway.
> 
> The patches look good but are based on 2.6.30. For 2.6.31-rc1 they must
> be changed a bit because some functions implemented in these patches
> were already merged for the suspen/resume support in amd-iommu driver.
> I will adapt them for 2.6.31. Do you want to send these patches for
> -stable inclusion?

Yes, I meant to mark at least 1/2 "Cc: stable@kernel.org".

thanks,
-chris