[git pull] AMD IOMMU fixes for 2.6.36

[git pull] AMD IOMMU fixes for 2.6.36

[Joerg Roedel]

Hi Ingo,

The following changes since commit 49553c2ef88749dd502687f4eb9c258bb10a4f44:
  Linus Torvalds (1):
        Linux 2.6.36-rc4

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/joro/linux-2.6-iommu.git amd-iommu/2.6.36

Joerg Roedel (3):
      x86/amd-iommu: Set iommu configuration flags in enable-loop
      x86/amd-iommu: Work around S3 BIOS bug
      x86/amd-iommu: Fix rounding-bug in __unmap_single

 arch/x86/include/asm/amd_iommu_proto.h |    6 +++
 arch/x86/include/asm/amd_iommu_types.h |   12 ++++++
 arch/x86/kernel/amd_iommu.c            |    4 +-
 arch/x86/kernel/amd_iommu_init.c       |   67 +++++++++++++++++++++----------
 include/linux/pci_ids.h                |    3 +
 5 files changed, 69 insertions(+), 23 deletions(-)

The first two patches make resume from memory working with the AMD IOMMU driver
(one fixes a problem in the driver and the other works around a BIOS bug). The
third patch fixes a long standing rounding bug in the driver which may lead to
stale io/tlb entries. Please pull.

Thanks,
	Joerg

[PATCH 1/3] x86/amd-iommu: Set iommu configuration flags in enable-loop

[Joerg Roedel]

This patch moves the setting of the configuration and
feature flags out out the acpi table parsing path and moves
it into the iommu-enable path. This is needed to reliably
fix resume-from-s3.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/amd_iommu_types.h |    3 ++
 arch/x86/kernel/amd_iommu_init.c       |   49 +++++++++++++++++--------------
 2 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/amd_iommu_types.h b/arch/x86/include/asm/amd_iommu_types.h
index 7014e88..ef2d5cd 100644
--- a/arch/x86/include/asm/amd_iommu_types.h
+++ b/arch/x86/include/asm/amd_iommu_types.h
@@ -368,6 +368,9 @@ struct amd_iommu {
 	/* capabilities of that IOMMU read from ACPI */
 	u32 cap;
 
+	/* flags read from acpi table */
+	u8 acpi_flags;
+
 	/*
 	 * Capability pointer. There could be more than one IOMMU per PCI
 	 * device function if there are more than one AMD IOMMU capability
diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index 3cc63e2..85e9817 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -649,29 +649,9 @@ static void __init init_iommu_from_acpi(struct amd_iommu *iommu,
 	struct ivhd_entry *e;
 
 	/*
-	 * First set the recommended feature enable bits from ACPI
-	 * into the IOMMU control registers
+	 * First save the recommended feature enable bits from ACPI
 	 */
-	h->flags & IVHD_FLAG_HT_TUN_EN_MASK ?
-		iommu_feature_enable(iommu, CONTROL_HT_TUN_EN) :
-		iommu_feature_disable(iommu, CONTROL_HT_TUN_EN);
-
-	h->flags & IVHD_FLAG_PASSPW_EN_MASK ?
-		iommu_feature_enable(iommu, CONTROL_PASSPW_EN) :
-		iommu_feature_disable(iommu, CONTROL_PASSPW_EN);
-
-	h->flags & IVHD_FLAG_RESPASSPW_EN_MASK ?
-		iommu_feature_enable(iommu, CONTROL_RESPASSPW_EN) :
-		iommu_feature_disable(iommu, CONTROL_RESPASSPW_EN);
-
-	h->flags & IVHD_FLAG_ISOC_EN_MASK ?
-		iommu_feature_enable(iommu, CONTROL_ISOC_EN) :
-		iommu_feature_disable(iommu, CONTROL_ISOC_EN);
-
-	/*
-	 * make IOMMU memory accesses cache coherent
-	 */
-	iommu_feature_enable(iommu, CONTROL_COHERENT_EN);
+	iommu->acpi_flags = h->flags;
 
 	/*
 	 * Done. Now parse the device entries
@@ -1116,6 +1096,30 @@ static void init_device_table(void)
 	}
 }
 
+static void iommu_init_flags(struct amd_iommu *iommu)
+{
+	iommu->acpi_flags & IVHD_FLAG_HT_TUN_EN_MASK ?
+		iommu_feature_enable(iommu, CONTROL_HT_TUN_EN) :
+		iommu_feature_disable(iommu, CONTROL_HT_TUN_EN);
+
+	iommu->acpi_flags & IVHD_FLAG_PASSPW_EN_MASK ?
+		iommu_feature_enable(iommu, CONTROL_PASSPW_EN) :
+		iommu_feature_disable(iommu, CONTROL_PASSPW_EN);
+
+	iommu->acpi_flags & IVHD_FLAG_RESPASSPW_EN_MASK ?
+		iommu_feature_enable(iommu, CONTROL_RESPASSPW_EN) :
+		iommu_feature_disable(iommu, CONTROL_RESPASSPW_EN);
+
+	iommu->acpi_flags & IVHD_FLAG_ISOC_EN_MASK ?
+		iommu_feature_enable(iommu, CONTROL_ISOC_EN) :
+		iommu_feature_disable(iommu, CONTROL_ISOC_EN);
+
+	/*
+	 * make IOMMU memory accesses cache coherent
+	 */
+	iommu_feature_enable(iommu, CONTROL_COHERENT_EN);
+}
+
 /*
  * This function finally enables all IOMMUs found in the system after
  * they have been initialized
@@ -1126,6 +1130,7 @@ static void enable_iommus(void)
 
 	for_each_iommu(iommu) {
 		iommu_disable(iommu);
+		iommu_init_flags(iommu);
 		iommu_set_device_table(iommu);
 		iommu_enable_command_buffer(iommu);
 		iommu_enable_event_buffer(iommu);
-- 
1.7.0.4

[PATCH 2/3] x86/amd-iommu: Work around S3 BIOS bug

[Joerg Roedel]

This patch adds a workaround for an IOMMU BIOS problem to
the AMD IOMMU driver. The result of the bug is that the
IOMMU does not execute commands anymore when the system
comes out of the S3 state resulting in system failure. The
bug in the BIOS is that is does not restore certain hardware
specific registers correctly. This workaround reads out the
contents of these registers at boot time and restores them
on resume from S3. The workaround is limited to the specific
IOMMU chipset where this problem occurs.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/amd_iommu_proto.h |    6 ++++++
 arch/x86/include/asm/amd_iommu_types.h |    9 +++++++++
 arch/x86/kernel/amd_iommu_init.c       |   18 ++++++++++++++++++
 include/linux/pci_ids.h                |    3 +++
 4 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/amd_iommu_proto.h b/arch/x86/include/asm/amd_iommu_proto.h
index d2544f1..cb03037 100644
--- a/arch/x86/include/asm/amd_iommu_proto.h
+++ b/arch/x86/include/asm/amd_iommu_proto.h
@@ -38,4 +38,10 @@ static inline void amd_iommu_stats_init(void) { }
 
 #endif /* !CONFIG_AMD_IOMMU_STATS */
 
+static inline bool is_rd890_iommu(struct pci_dev *pdev)
+{
+	return (pdev->vendor == PCI_VENDOR_ID_ATI) &&
+	       (pdev->device == PCI_DEVICE_ID_RD890_IOMMU);
+}
+
 #endif /* _ASM_X86_AMD_IOMMU_PROTO_H  */
diff --git a/arch/x86/include/asm/amd_iommu_types.h b/arch/x86/include/asm/amd_iommu_types.h
index ef2d5cd..0861618 100644
--- a/arch/x86/include/asm/amd_iommu_types.h
+++ b/arch/x86/include/asm/amd_iommu_types.h
@@ -414,6 +414,15 @@ struct amd_iommu {
 
 	/* default dma_ops domain for that IOMMU */
 	struct dma_ops_domain *default_dom;
+
+	/*
+	 * This array is required to work around a potential BIOS bug.
+	 * The BIOS may miss to restore parts of the PCI configuration
+	 * space when the system resumes from S3. The result is that the
+	 * IOMMU does not execute commands anymore which leads to system
+	 * failure.
+	 */
+	u32 cache_cfg[4];
 };
 
 /*
diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index 85e9817..5a170cb 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -632,6 +632,13 @@ static void __init init_iommu_from_pci(struct amd_iommu *iommu)
 	iommu->last_device = calc_devid(MMIO_GET_BUS(range),
 					MMIO_GET_LD(range));
 	iommu->evt_msi_num = MMIO_MSI_NUM(misc);
+
+	if (is_rd890_iommu(iommu->dev)) {
+		pci_read_config_dword(iommu->dev, 0xf0, &iommu->cache_cfg[0]);
+		pci_read_config_dword(iommu->dev, 0xf4, &iommu->cache_cfg[1]);
+		pci_read_config_dword(iommu->dev, 0xf8, &iommu->cache_cfg[2]);
+		pci_read_config_dword(iommu->dev, 0xfc, &iommu->cache_cfg[3]);
+	}
 }
 
 /*
@@ -1120,6 +1127,16 @@ static void iommu_init_flags(struct amd_iommu *iommu)
 	iommu_feature_enable(iommu, CONTROL_COHERENT_EN);
 }
 
+static void iommu_apply_quirks(struct amd_iommu *iommu)
+{
+	if (is_rd890_iommu(iommu->dev)) {
+		pci_write_config_dword(iommu->dev, 0xf0, iommu->cache_cfg[0]);
+		pci_write_config_dword(iommu->dev, 0xf4, iommu->cache_cfg[1]);
+		pci_write_config_dword(iommu->dev, 0xf8, iommu->cache_cfg[2]);
+		pci_write_config_dword(iommu->dev, 0xfc, iommu->cache_cfg[3]);
+	}
+}
+
 /*
  * This function finally enables all IOMMUs found in the system after
  * they have been initialized
@@ -1130,6 +1147,7 @@ static void enable_iommus(void)
 
 	for_each_iommu(iommu) {
 		iommu_disable(iommu);
+		iommu_apply_quirks(iommu);
 		iommu_init_flags(iommu);
 		iommu_set_device_table(iommu);
 		iommu_enable_command_buffer(iommu);
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 10d3330..570fdde 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -393,6 +393,9 @@
 #define PCI_DEVICE_ID_VLSI_82C147	0x0105
 #define PCI_DEVICE_ID_VLSI_VAS96011	0x0702
 
+/* AMD RD890 Chipset */
+#define PCI_DEVICE_ID_RD890_IOMMU	0x5a23
+
 #define PCI_VENDOR_ID_ADL		0x1005
 #define PCI_DEVICE_ID_ADL_2301		0x2301
 
-- 
1.7.0.4

Re: [PATCH 2/3] x86/amd-iommu: Work around S3 BIOS bug

[Matthew Garrett]

On Thu, Sep 23, 2010 at 04:44:48PM +0200, Joerg Roedel wrote:
> +static void iommu_apply_quirks(struct amd_iommu *iommu)
> +{
> +	if (is_rd890_iommu(iommu->dev)) {
> +		pci_write_config_dword(iommu->dev, 0xf0, iommu->cache_cfg[0]);
> +		pci_write_config_dword(iommu->dev, 0xf4, iommu->cache_cfg[1]);
> +		pci_write_config_dword(iommu->dev, 0xf8, iommu->cache_cfg[2]);
> +		pci_write_config_dword(iommu->dev, 0xfc, iommu->cache_cfg[3]);
> +	}
> +}
> +

This doesn't look right. 0xf0-0xff are for indexed register access, so 
what you're doing here is just restoring the last of each of those 
registers that the BIOS programmed - and if the BIOS cleared the 
writable flag afterwards, you're not even doing that.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/3] x86/amd-iommu: Work around S3 BIOS bug

[Roedel, Joerg]

On Thu, Sep 30, 2010 at 04:21:04PM -0400, Matthew Garrett wrote:
> On Thu, Sep 23, 2010 at 04:44:48PM +0200, Joerg Roedel wrote:
> > +static void iommu_apply_quirks(struct amd_iommu *iommu)
> > +{
> > +	if (is_rd890_iommu(iommu->dev)) {
> > +		pci_write_config_dword(iommu->dev, 0xf0, iommu->cache_cfg[0]);
> > +		pci_write_config_dword(iommu->dev, 0xf4, iommu->cache_cfg[1]);
> > +		pci_write_config_dword(iommu->dev, 0xf8, iommu->cache_cfg[2]);
> > +		pci_write_config_dword(iommu->dev, 0xfc, iommu->cache_cfg[3]);
> > +	}
> > +}
> > +
> 
> This doesn't look right. 0xf0-0xff are for indexed register access, so 
> what you're doing here is just restoring the last of each of those 
> registers that the BIOS programmed - and if the BIOS cleared the 
> writable flag afterwards, you're not even doing that.

With a half-fixed BIOS (a BIOS which re-enables the IOMMU on resume)
this was the necessary step to make the IOMMU execute commands again. So
it actually fixed the problem I have seen. I agree that its better to
fully restore the indirect register spaces when we workaround the fully
broken BIOSes. When we have this I remove this code, but until then it
fixes the problem.

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

[PATCH 3/3] x86/amd-iommu: Fix rounding-bug in __unmap_single

[Joerg Roedel]

In the __unmap_single function the dma_addr is rounded down
to a page boundary before the dma pages are unmapped. The
address is later also used to flush the TLB entries for that
mapping. But without the offset into the dma page the amount
of pages to flush might be miscalculated in the TLB flushing
path. This patch fixes this bug by using the original
address to flush the TLB.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index fa044e1..679b645 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -1953,6 +1953,7 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
 			   size_t size,
 			   int dir)
 {
+	dma_addr_t flush_addr;
 	dma_addr_t i, start;
 	unsigned int pages;
 
@@ -1960,6 +1961,7 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
 	    (dma_addr + size > dma_dom->aperture_size))
 		return;
 
+	flush_addr = dma_addr;
 	pages = iommu_num_pages(dma_addr, size, PAGE_SIZE);
 	dma_addr &= PAGE_MASK;
 	start = dma_addr;
@@ -1974,7 +1976,7 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
 	dma_ops_free_addresses(dma_dom, dma_addr, pages);
 
 	if (amd_iommu_unmap_flush || dma_dom->need_flush) {
-		iommu_flush_pages(&dma_dom->domain, dma_addr, size);
+		iommu_flush_pages(&dma_dom->domain, flush_addr, size);
 		dma_dom->need_flush = false;
 	}
 }
-- 
1.7.0.4

Re: [git pull] AMD IOMMU fixes for 2.6.36

[Ingo Molnar]

* Joerg Roedel <joerg.roedel@amd.com> wrote:

> Hi Ingo,
> 
> The following changes since commit 49553c2ef88749dd502687f4eb9c258bb10a4f44:
>   Linus Torvalds (1):
>         Linux 2.6.36-rc4
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/joro/linux-2.6-iommu.git amd-iommu/2.6.36
> 
> Joerg Roedel (3):
>       x86/amd-iommu: Set iommu configuration flags in enable-loop
>       x86/amd-iommu: Work around S3 BIOS bug
>       x86/amd-iommu: Fix rounding-bug in __unmap_single
> 
>  arch/x86/include/asm/amd_iommu_proto.h |    6 +++
>  arch/x86/include/asm/amd_iommu_types.h |   12 ++++++
>  arch/x86/kernel/amd_iommu.c            |    4 +-
>  arch/x86/kernel/amd_iommu_init.c       |   67 +++++++++++++++++++++----------
>  include/linux/pci_ids.h                |    3 +
>  5 files changed, 69 insertions(+), 23 deletions(-)
>
> The first two patches make resume from memory working with the AMD 
> IOMMU driver (one fixes a problem in the driver and the other works 
> around a BIOS bug). The third patch fixes a long standing rounding bug 
> in the driver which may lead to stale io/tlb entries. Please pull.

Pulled, thanks Joerg!

	Ingo