Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption

Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption

[Rafael J. Wysocki]

On Tuesday, 27 of May 2008, Pavel Machek wrote:
> Hi!
> 
> > > Can you try this one? It should prevent suspend in the broken cases,
> > > but allow it in mem=4G config.
> > 
> > Sure!
> > 
> > root@babar:/usr/src/linux-2.6.25# patch -p1 < /home/pat/patch-2.6.26-rc3.gart-suspend
> > patching file arch/x86/kernel/pci-gart_64.c
> > Hunk #4 succeeded at 629 with fuzz 2 (offset 11 lines).
> > 
> > .....make; cp bzImage /boot; reboot....
> 
> Thanks!
> 
> This goes on top of the second patch... it makes it work.
> 
> 								Pavel
> 
> For iommu suspend/resume code to work, functions it calls may not be
> __init.
> 
> Signed-off-by: Pavel Machek <pavel@suse.cz>

I consolidated some of your patches sent in this thread and made the result
apply to the current -git.  It hasn't been tested yet, but does it look good?

It's on top of the patch that adds the GART sysdev.

Thanks,
Rafael

---
Handle GART IOMMU suspend and resume.

Not-yet-signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/x86/kernel/aperture_64.c |   34 ++++++++++++++++---------
 arch/x86/kernel/k8.c          |    2 -
 arch/x86/kernel/pci-gart_64.c |   55 +++++++++++++++++++++++++++++-------------
 include/asm-x86/gart.h        |    2 +
 4 files changed, 63 insertions(+), 30 deletions(-)

Index: linux-2.6/arch/x86/kernel/pci-gart_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/pci-gart_64.c
+++ linux-2.6/arch/x86/kernel/pci-gart_64.c
@@ -549,14 +549,50 @@ static __init unsigned read_aperture(str
 	return aper_base;
 }
 
+static void enable_gart_translations(void)
+{
+	int i;
+
+	for (i = 0; i < num_k8_northbridges; i++) {
+		struct pci_dev *dev;
+		u32 gatt_reg;
+		u32 ctl;
+
+		dev = k8_northbridges[i];
+		gatt_reg = __pa(agp_gatt_table) >> 12;
+		gatt_reg <<= 4;
+		pci_write_config_dword(dev, 0x98, gatt_reg);
+		pci_read_config_dword(dev, 0x90, &ctl);
+
+		ctl |= 1;
+		ctl &= ~((1<<4) | (1<<5));
+
+		pci_write_config_dword(dev, 0x90, ctl);
+	}
+}
+
+static bool fix_north_bridges;	/* call fix_up_north_bridges() on resume */
+static u32 aperture_order;	/* arguments for fix_up_north_bridges() */
+static u32 aperture_alloc;
+
+void set_gart_resume_data(u32 aper_order, u32 aper_alloc)
+{
+	fix_north_bridges = true;
+	aperture_order = aper_order;
+	aperture_alloc = aper_alloc;
+}
+
 static int gart_resume(struct sys_device *dev)
 {
+	if (fix_north_bridges)
+		fix_up_north_bridges(aperture_order, aperture_alloc);
+	enable_gart_translations();
 	return 0;
 }
 
 static int gart_suspend(struct sys_device *dev, pm_message_t state)
 {
-	return -EINVAL;
+	return 0;
 }
 
 static struct sysdev_class gart_sysdev_class = {
@@ -613,27 +649,14 @@ static __init int init_k8_gatt(struct ag
 	memset(gatt, 0, gatt_size);
 	agp_gatt_table = gatt;
 
-	for (i = 0; i < num_k8_northbridges; i++) {
-		u32 gatt_reg;
-		u32 ctl;
-
-		dev = k8_northbridges[i];
-		gatt_reg = __pa(gatt) >> 12;
-		gatt_reg <<= 4;
-		pci_write_config_dword(dev, 0x98, gatt_reg);
-		pci_read_config_dword(dev, 0x90, &ctl);
-
-		ctl |= 1;
-		ctl &= ~((1<<4) | (1<<5));
-
-		pci_write_config_dword(dev, 0x90, ctl);
-	}
+	enable_gart_translations();
 
 	error = sysdev_class_register(&gart_sysdev_class);
 	if (!error)
 		error = sysdev_register(&device_gart);
 	if (error)
 		panic("Could not register gart_sysdev -- would corrupt data on next suspend");
+
 	flush_gart();
 
 	printk(KERN_INFO "PCI-DMA: aperture base @ %x size %u KB\n",
Index: linux-2.6/arch/x86/kernel/k8.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/k8.c
+++ linux-2.6/arch/x86/kernel/k8.c
@@ -76,7 +76,7 @@ EXPORT_SYMBOL_GPL(cache_k8_northbridges)
 
 /* Ignores subdevice/subvendor but as far as I can figure out
    they're useless anyways */
-int __init early_is_k8_nb(u32 device)
+int early_is_k8_nb(u32 device)
 {
 	struct pci_device_id *id;
 	u32 vendor = device & 0xffff;
Index: linux-2.6/arch/x86/kernel/aperture_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/aperture_64.c
+++ linux-2.6/arch/x86/kernel/aperture_64.c
@@ -310,6 +310,25 @@ void __init early_gart_iommu_check(void)
 
 }
 
+void fix_up_north_bridges(u32 aper_order, u32 aper_alloc)
+{
+	int num;
+
+	/* Fix up the north bridges */
+	for (num = 24; num < 32; num++) {
+		if (!early_is_k8_nb(read_pci_config(0, num, 3, 0x00)))
+			continue;
+
+		/*
+		 * Don't enable translation yet. That is done later.
+		 * Assume this BIOS didn't initialise the GART so
+		 * just overwrite all previous bits
+		 */
+		write_pci_config(0, num, 3, 0x90, aper_order << 1);
+		write_pci_config(0, num, 3, 0x94, aper_alloc >> 25);
+	}
+}
+
 void __init gart_iommu_hole_init(void)
 {
 	u32 aper_size, aper_alloc = 0, aper_order = 0, last_aper_order = 0;
@@ -400,17 +419,6 @@ void __init gart_iommu_hole_init(void)
 		return;
 	}
 
-	/* Fix up the north bridges */
-	for (num = 24; num < 32; num++) {
-		if (!early_is_k8_nb(read_pci_config(0, num, 3, 0x00)))
-			continue;
-
-		/*
-		 * Don't enable translation yet. That is done later.
-		 * Assume this BIOS didn't initialise the GART so
-		 * just overwrite all previous bits
-		 */
-		write_pci_config(0, num, 3, 0x90, aper_order<<1);
-		write_pci_config(0, num, 3, 0x94, aper_alloc>>25);
-	}
+	fix_up_north_bridges(aper_order, aper_alloc);
+	set_gart_resume_data(aper_order, aper_alloc);
 }
Index: linux-2.6/include/asm-x86/gart.h
===================================================================
--- linux-2.6.orig/include/asm-x86/gart.h
+++ linux-2.6/include/asm-x86/gart.h
@@ -11,6 +11,8 @@ extern void gart_iommu_shutdown(void);
 extern void __init gart_parse_options(char *);
 extern void early_gart_iommu_check(void);
 extern void gart_iommu_hole_init(void);
+extern void set_gart_resume_data(u32, u32);
+extern void fix_up_north_bridges(u32, u32);
 extern int fallback_aper_order;
 extern int fallback_aper_force;
 extern int gart_iommu_aperture;

Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption

[Pavel Machek]

Hi!

> > > > Can you try this one? It should prevent suspend in the broken cases,
> > > > but allow it in mem=4G config.
> > > 
> > > Sure!
> > > 
> > > root@babar:/usr/src/linux-2.6.25# patch -p1 < /home/pat/patch-2.6.26-rc3.gart-suspend
> > > patching file arch/x86/kernel/pci-gart_64.c
> > > Hunk #4 succeeded at 629 with fuzz 2 (offset 11 lines).
> > > 
> > > .....make; cp bzImage /boot; reboot....
> > 
> > Thanks!
> > 
> > This goes on top of the second patch... it makes it work.
> > 
> > 								Pavel
> > 
> > For iommu suspend/resume code to work, functions it calls may not be
> > __init.
> > 
> > Signed-off-by: Pavel Machek <pavel@suse.cz>
> 
> I consolidated some of your patches sent in this thread and made the result
> apply to the current -git.  It hasn't been tested yet, but does it look good?
> 
> It's on top of the patch that adds the GART sysdev.

Looks ok to me.

							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption

[Rafael J. Wysocki]

On Friday, 6 of June 2008, Pavel Machek wrote:
> Hi!
> 
> > > > > Can you try this one? It should prevent suspend in the broken cases,
> > > > > but allow it in mem=4G config.
> > > > 
> > > > Sure!
> > > > 
> > > > root@babar:/usr/src/linux-2.6.25# patch -p1 < /home/pat/patch-2.6.26-rc3.gart-suspend
> > > > patching file arch/x86/kernel/pci-gart_64.c
> > > > Hunk #4 succeeded at 629 with fuzz 2 (offset 11 lines).
> > > > 
> > > > .....make; cp bzImage /boot; reboot....
> > > 
> > > Thanks!
> > > 
> > > This goes on top of the second patch... it makes it work.
> > > 
> > > 								Pavel
> > > 
> > > For iommu suspend/resume code to work, functions it calls may not be
> > > __init.
> > > 
> > > Signed-off-by: Pavel Machek <pavel@suse.cz>
> > 
> > I consolidated some of your patches sent in this thread and made the result
> > apply to the current -git.  It hasn't been tested yet, but does it look good?
> > 
> > It's on top of the patch that adds the GART sysdev.
> 
> Looks ok to me.

Still, it may be improved. :-)

First, we shouldn't mix the "early PCI config access" thing with the "normal"
method.  Second, we don't have to check for the K8 north bridges on resume,
because we already know where they are in the configuration space and we can
use this information.

Updated patch follows.  It has been tested a little on my new 4 GB test box on
which 2.6.26-rc4 failed miserably with severe consequences.  More testing
welcome, but please be careful.

Thanks,
Rafael

---
Add resume handling to GART IOMMU.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/x86/kernel/aperture_64.c |    2 +
 arch/x86/kernel/pci-gart_64.c |   75 +++++++++++++++++++++++++++++++++---------
 include/asm-x86/gart.h        |    1 
 3 files changed, 62 insertions(+), 16 deletions(-)

Index: linux-2.6/arch/x86/kernel/pci-gart_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/pci-gart_64.c
+++ linux-2.6/arch/x86/kernel/pci-gart_64.c
@@ -549,14 +549,70 @@ static __init unsigned read_aperture(str
 	return aper_base;
 }
 
+static void enable_gart_translations(void)
+{
+	int i;
+
+	for (i = 0; i < num_k8_northbridges; i++) {
+		struct pci_dev *dev;
+		u32 gatt_reg;
+		u32 ctl;
+
+		dev = k8_northbridges[i];
+		gatt_reg = __pa(agp_gatt_table) >> 12;
+		gatt_reg <<= 4;
+		pci_write_config_dword(dev, 0x98, gatt_reg);
+		pci_read_config_dword(dev, 0x90, &ctl);
+
+		ctl |= 1;
+		ctl &= ~((1<<4) | (1<<5));
+
+		pci_write_config_dword(dev, 0x90, ctl);
+	}
+}
+
+/*
+ * If fix_up_north_bridges is set, the north bridges have to be fixed up on
+ * resume in the same way as they are handled in gart_iommu_hole_init().
+ */
+static bool fix_up_north_bridges;
+static u32 aperture_order;
+static u32 aperture_alloc;
+
+void set_up_gart_resume(u32 aper_order, u32 aper_alloc)
+{
+	fix_up_north_bridges = true;
+	aperture_order = aper_order;
+	aperture_alloc = aper_alloc;
+}
+
 static int gart_resume(struct sys_device *dev)
 {
+	printk(KERN_INFO "PCI-DMA: Resuming GART IOMMU\n");
+
+	if (fix_up_north_bridges) {
+		int i;
+
+		for (i = 0; i < num_k8_northbridges; i++) {
+			struct pci_dev *dev = k8_northbridges[i];
+
+			/*
+			 * Don't enable translations just yet.  That is the next
+			 * step.  Restore the pre-suspend aperture settings.
+			 */
+			pci_write_config_dword(dev, 0x90, aperture_order << 1);
+			pci_write_config_dword(dev, 0x94, aperture_alloc >> 25);
+		}
+	}
+
+	enable_gart_translations();
+
 	return 0;
 }
 
 static int gart_suspend(struct sys_device *dev, pm_message_t state)
 {
-	return -EINVAL;
+	return 0;
 }
 
 static struct sysdev_class gart_sysdev_class = {
@@ -614,27 +670,14 @@ static __init int init_k8_gatt(struct ag
 	memset(gatt, 0, gatt_size);
 	agp_gatt_table = gatt;
 
-	for (i = 0; i < num_k8_northbridges; i++) {
-		u32 gatt_reg;
-		u32 ctl;
-
-		dev = k8_northbridges[i];
-		gatt_reg = __pa(gatt) >> 12;
-		gatt_reg <<= 4;
-		pci_write_config_dword(dev, 0x98, gatt_reg);
-		pci_read_config_dword(dev, 0x90, &ctl);
-
-		ctl |= 1;
-		ctl &= ~((1<<4) | (1<<5));
-
-		pci_write_config_dword(dev, 0x90, ctl);
-	}
+	enable_gart_translations();
 
 	error = sysdev_class_register(&gart_sysdev_class);
 	if (!error)
 		error = sysdev_register(&device_gart);
 	if (error)
 		panic("Could not register gart_sysdev -- would corrupt data on next suspend");
+
 	flush_gart();
 
 	printk(KERN_INFO "PCI-DMA: aperture base @ %x size %u KB\n",
Index: linux-2.6/arch/x86/kernel/aperture_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/aperture_64.c
+++ linux-2.6/arch/x86/kernel/aperture_64.c
@@ -413,4 +413,6 @@ void __init gart_iommu_hole_init(void)
 		write_pci_config(0, num, 3, 0x90, aper_order<<1);
 		write_pci_config(0, num, 3, 0x94, aper_alloc>>25);
 	}
+
+	set_up_gart_resume(aper_order, aper_alloc);
 }
Index: linux-2.6/include/asm-x86/gart.h
===================================================================
--- linux-2.6.orig/include/asm-x86/gart.h
+++ linux-2.6/include/asm-x86/gart.h
@@ -11,6 +11,7 @@ extern void gart_iommu_shutdown(void);
 extern void __init gart_parse_options(char *);
 extern void early_gart_iommu_check(void);
 extern void gart_iommu_hole_init(void);
+extern void set_up_gart_resume(u32, u32);
 extern int fallback_aper_order;
 extern int fallback_aper_force;
 extern int gart_iommu_aperture;

[PATCH] x86 GART: Add resume handling (was: Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption)

[Rafael J. Wysocki]

On Monday, 9 of June 2008, Ingo Molnar wrote:
> 
> * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> 
> > > Looks ok to me.
> > 
> > Still, it may be improved. :-)
> > 
> > First, we shouldn't mix the "early PCI config access" thing with the 
> > "normal" method.  Second, we don't have to check for the K8 north 
> > bridges on resume, because we already know where they are in the 
> > configuration space and we can use this information.
> > 
> > Updated patch follows.  It has been tested a little on my new 4 GB 
> > test box on which 2.6.26-rc4 failed miserably with severe 
> > consequences.  More testing welcome, but please be careful.
> 
> Rafael, could we try this against the tip/x86/gart tree perhaps? It 
> already has a couple of fixes from Pavel and your patch collides with 
> them in a non-obvious way.
> 
>   http://people.redhat.com/mingo/tip.git/README

Okay, appended is the patch rebased on tip/x86/gart with (mainline) commit
cd76374e9de4501acc74f833dc6cb5e7a5dca115 "suspend-vs-iommu: prevent suspend if
we could not resume" (which appears to be missing from tip/x86/gart) applied.

This version of the patch doesn't break compilation, but it hasn't been really
tested yet.

Thanks,
Rafael

---
Add resume handling to GART IOMMU.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/x86/kernel/aperture_64.c |    2 +
 arch/x86/kernel/pci-gart_64.c |   57 ++++++++++++++++++++++++++++++++++++++----
 include/asm-x86/gart.h        |    1 
 3 files changed, 55 insertions(+), 5 deletions(-)

Index: tip.git/arch/x86/kernel/aperture_64.c
===================================================================
--- tip.git.orig/arch/x86/kernel/aperture_64.c
+++ tip.git/arch/x86/kernel/aperture_64.c
@@ -496,4 +496,6 @@ out:
 			write_pci_config(bus, slot, 3, AMD64_GARTAPERTUREBASE, aper_alloc >> 25);
 		}
 	}
+
+	set_up_gart_resume(aper_order, aper_alloc);
 }
Index: tip.git/arch/x86/kernel/pci-gart_64.c
===================================================================
--- tip.git.orig/arch/x86/kernel/pci-gart_64.c
+++ tip.git/arch/x86/kernel/pci-gart_64.c
@@ -549,14 +549,63 @@ static __init unsigned read_aperture(str
 	return aper_base;
 }
 
+static void enable_gart_translations(void)
+{
+	int i;
+
+	for (i = 0; i < num_k8_northbridges; i++) {
+		struct pci_dev *dev = k8_northbridges[i];
+
+		enable_gart_translation(dev, __pa(agp_gatt_table));
+	}
+}
+
+/*
+ * If fix_up_north_bridges is set, the north bridges have to be fixed up on
+ * resume in the same way as they are handled in gart_iommu_hole_init().
+ */
+static bool fix_up_north_bridges;
+static u32 aperture_order;
+static u32 aperture_alloc;
+
+void set_up_gart_resume(u32 aper_order, u32 aper_alloc)
+{
+	fix_up_north_bridges = true;
+	aperture_order = aper_order;
+	aperture_alloc = aper_alloc;
+}
+
 static int gart_resume(struct sys_device *dev)
 {
+	printk(KERN_INFO "PCI-DMA: Resuming GART IOMMU\n");
+
+	if (fix_up_north_bridges) {
+		int i;
+
+		printk(KERN_INFO "PCI-DMA: Restoring GART aperture settings\n");
+
+		for (i = 0; i < num_k8_northbridges; i++) {
+			struct pci_dev *dev = k8_northbridges[i];
+
+			/*
+			 * Don't enable translations just yet.  That is the next
+			 * step.  Restore the pre-suspend aperture settings.
+			 */
+			pci_write_config_dword(dev, AMD64_GARTAPERTURECTL,
+						aperture_order << 1);
+			pci_write_config_dword(dev, AMD64_GARTAPERTUREBASE,
+						aperture_alloc >> 25);
+		}
+	}
+
+	enable_gart_translations();
+
 	return 0;
 }
 
 static int gart_suspend(struct sys_device *dev, pm_message_t state)
 {
-	return -EINVAL;
+	return 0;
 }
 
 static struct sysdev_class gart_sysdev_class = {
@@ -614,16 +663,14 @@ static __init int init_k8_gatt(struct ag
 	memset(gatt, 0, gatt_size);
 	agp_gatt_table = gatt;
 
-	for (i = 0; i < num_k8_northbridges; i++) {
-		dev = k8_northbridges[i];
-		enable_gart_translation(dev, __pa(gatt));
-	}
+	enable_gart_translations();
 
 	error = sysdev_class_register(&gart_sysdev_class);
 	if (!error)
 		error = sysdev_register(&device_gart);
 	if (error)
 		panic("Could not register gart_sysdev -- would corrupt data on next suspend");
+
 	flush_gart();
 
 	printk(KERN_INFO "PCI-DMA: aperture base @ %x size %u KB\n",
Index: tip.git/include/asm-x86/gart.h
===================================================================
--- tip.git.orig/include/asm-x86/gart.h
+++ tip.git/include/asm-x86/gart.h
@@ -14,6 +14,7 @@ extern void gart_iommu_shutdown(void);
 extern void __init gart_parse_options(char *);
 extern void early_gart_iommu_check(void);
 extern void gart_iommu_hole_init(void);
+extern void set_up_gart_resume(u32, u32);
 extern int fallback_aper_order;
 extern int fallback_aper_force;
 extern int gart_iommu_aperture;

Re: [PATCH] x86 GART: Add resume handling (was: Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption)

[Rafael J. Wysocki]

On Tuesday, 10 of June 2008, Rafael J. Wysocki wrote:
> On Monday, 9 of June 2008, Ingo Molnar wrote:
> > 
> > * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > 
> > > > Looks ok to me.
> > > 
> > > Still, it may be improved. :-)
> > > 
> > > First, we shouldn't mix the "early PCI config access" thing with the 
> > > "normal" method.  Second, we don't have to check for the K8 north 
> > > bridges on resume, because we already know where they are in the 
> > > configuration space and we can use this information.
> > > 
> > > Updated patch follows.  It has been tested a little on my new 4 GB 
> > > test box on which 2.6.26-rc4 failed miserably with severe 
> > > consequences.  More testing welcome, but please be careful.
> > 
> > Rafael, could we try this against the tip/x86/gart tree perhaps? It 
> > already has a couple of fixes from Pavel and your patch collides with 
> > them in a non-obvious way.
> > 
> >   http://people.redhat.com/mingo/tip.git/README
> 
> Okay, appended is the patch rebased on tip/x86/gart with (mainline) commit
> cd76374e9de4501acc74f833dc6cb5e7a5dca115 "suspend-vs-iommu: prevent suspend if
> we could not resume" (which appears to be missing from tip/x86/gart) applied.
> 
> This version of the patch doesn't break compilation, but it hasn't been really
> tested yet.

Now it has been (successfully) tested too. :-)

Thanks,
Rafael

 
> ---
> Add resume handling to GART IOMMU.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  arch/x86/kernel/aperture_64.c |    2 +
>  arch/x86/kernel/pci-gart_64.c |   57 ++++++++++++++++++++++++++++++++++++++----
>  include/asm-x86/gart.h        |    1 
>  3 files changed, 55 insertions(+), 5 deletions(-)
> 
> Index: tip.git/arch/x86/kernel/aperture_64.c
> ===================================================================
> --- tip.git.orig/arch/x86/kernel/aperture_64.c
> +++ tip.git/arch/x86/kernel/aperture_64.c
> @@ -496,4 +496,6 @@ out:
>  			write_pci_config(bus, slot, 3, AMD64_GARTAPERTUREBASE, aper_alloc >> 25);
>  		}
>  	}
> +
> +	set_up_gart_resume(aper_order, aper_alloc);
>  }
> Index: tip.git/arch/x86/kernel/pci-gart_64.c
> ===================================================================
> --- tip.git.orig/arch/x86/kernel/pci-gart_64.c
> +++ tip.git/arch/x86/kernel/pci-gart_64.c
> @@ -549,14 +549,63 @@ static __init unsigned read_aperture(str
>  	return aper_base;
>  }
>  
> +static void enable_gart_translations(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < num_k8_northbridges; i++) {
> +		struct pci_dev *dev = k8_northbridges[i];
> +
> +		enable_gart_translation(dev, __pa(agp_gatt_table));
> +	}
> +}
> +
> +/*
> + * If fix_up_north_bridges is set, the north bridges have to be fixed up on
> + * resume in the same way as they are handled in gart_iommu_hole_init().
> + */
> +static bool fix_up_north_bridges;
> +static u32 aperture_order;
> +static u32 aperture_alloc;
> +
> +void set_up_gart_resume(u32 aper_order, u32 aper_alloc)
> +{
> +	fix_up_north_bridges = true;
> +	aperture_order = aper_order;
> +	aperture_alloc = aper_alloc;
> +}
> +
>  static int gart_resume(struct sys_device *dev)
>  {
> +	printk(KERN_INFO "PCI-DMA: Resuming GART IOMMU\n");
> +
> +	if (fix_up_north_bridges) {
> +		int i;
> +
> +		printk(KERN_INFO "PCI-DMA: Restoring GART aperture settings\n");
> +
> +		for (i = 0; i < num_k8_northbridges; i++) {
> +			struct pci_dev *dev = k8_northbridges[i];
> +
> +			/*
> +			 * Don't enable translations just yet.  That is the next
> +			 * step.  Restore the pre-suspend aperture settings.
> +			 */
> +			pci_write_config_dword(dev, AMD64_GARTAPERTURECTL,
> +						aperture_order << 1);
> +			pci_write_config_dword(dev, AMD64_GARTAPERTUREBASE,
> +						aperture_alloc >> 25);
> +		}
> +	}
> +
> +	enable_gart_translations();
> +
>  	return 0;
>  }
>  
>  static int gart_suspend(struct sys_device *dev, pm_message_t state)
>  {
> -	return -EINVAL;
> +	return 0;
>  }
>  
>  static struct sysdev_class gart_sysdev_class = {
> @@ -614,16 +663,14 @@ static __init int init_k8_gatt(struct ag
>  	memset(gatt, 0, gatt_size);
>  	agp_gatt_table = gatt;
>  
> -	for (i = 0; i < num_k8_northbridges; i++) {
> -		dev = k8_northbridges[i];
> -		enable_gart_translation(dev, __pa(gatt));
> -	}
> +	enable_gart_translations();
>  
>  	error = sysdev_class_register(&gart_sysdev_class);
>  	if (!error)
>  		error = sysdev_register(&device_gart);
>  	if (error)
>  		panic("Could not register gart_sysdev -- would corrupt data on next suspend");
> +
>  	flush_gart();
>  
>  	printk(KERN_INFO "PCI-DMA: aperture base @ %x size %u KB\n",
> Index: tip.git/include/asm-x86/gart.h
> ===================================================================
> --- tip.git.orig/include/asm-x86/gart.h
> +++ tip.git/include/asm-x86/gart.h
> @@ -14,6 +14,7 @@ extern void gart_iommu_shutdown(void);
>  extern void __init gart_parse_options(char *);
>  extern void early_gart_iommu_check(void);
>  extern void gart_iommu_hole_init(void);
> +extern void set_up_gart_resume(u32, u32);
>  extern int fallback_aper_order;
>  extern int fallback_aper_force;
>  extern int gart_iommu_aperture;
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 



-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH] x86 GART: Add resume handling (was: Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption)

[Ingo Molnar]

* Rafael J. Wysocki <rjw@sisk.pl> wrote:

> > Okay, appended is the patch rebased on tip/x86/gart with (mainline) 
> > commit cd76374e9de4501acc74f833dc6cb5e7a5dca115 "suspend-vs-iommu: 
> > prevent suspend if we could not resume" (which appears to be missing 
> > from tip/x86/gart) applied.
> > 
> > This version of the patch doesn't break compilation, but it hasn't 
> > been really tested yet.
> 
> Now it has been (successfully) tested too. :-)

applied to tip/x86/gart, thanks Rafael.

	Ingo

Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption

[Patrick]

<linux-pm@lists.linux-foundation.org>: host
    lists.linux-foundation.org[140.211.169.51] said: 554 5.7.1 Email body
    contains Spam URI (in reply to end of DATA command)

-----------------------------------

From: Patrick <ragamuffin@datacomm.ch>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Pavel Machek <pavel@suse.cz>, linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>, pm list <linux-pm@lists.linux-foundation.org>, Andi Kleen <andi@firstfloor.org>
In-Reply-To: <200806090036.32267.rjw@sisk.pl>
References: <1211717435.6038.53.camel@localhost.localdomain>
	 <200806040033.20736.rjw@sisk.pl> <20080606132011.GA9027@ucw.cz>
	 <200806090036.32267.rjw@sisk.pl>
Content-Type: text/plain; charset=UTF-8
Message-Id: <1213184587.6376.37.camel@localhost.localdomain>
Mime-Version: 1.0
Date: Wed, 11 Jun 2008 13:43:08 +0200
Content-Transfer-Encoding: 8bit

Hello

On Mon, 2008-06-09 at 00:36 +0200, Rafael J. Wysocki wrote:

> > > It's on top of the patch that adds the GART sysdev.

> Updated patch follows.  It has been tested a little on my new 4 GB test box on
> which 2.6.26-rc4 failed miserably with severe consequences.  More testing
> welcome, but please be careful.

No risk no fun! :-)

As the suspend-vs-iommu-prevent-suspend-if-we-could-not-resume.patch,
where the GART sysdev is added, is now included in 2.6.26-rc5, I was
able to apply this one seamlessly to it (-rc5 tree) and have just
rebooted *, suspended to disk and ram once wiht all filesystems (>400GB)
mounted rw ** and everything is _working fine_.

As usual, a kernel log, the applied patch and a test report are
available at http://zefir dot 890m dot com/kernel-testing/ .

pat@babar:~/tmp/dmesg$ grep -C3 GART dmesg.2.6.26-rc5-gart-suspend.txt
[    0.294419] PCI: Using ACPI for IRQ routing
[    0.314420] PCI-DMA: Disabling AGP.
[    0.314420] PCI-DMA: aperture base @ 4000000 size 65536 KB
[    0.314420] PCI-DMA: using GART IOMMU.
[    0.314420] PCI-DMA: Reserving 64MB of IOMMU area in the AGP aperture
[    0.314420] hpet0: at MMIO 0xfed00000, IRQs 2, 8, 0, 0
[    0.314420] hpet0: 4 32-bit timers, 14318180 Hz
--
[  221.770810] CPU1 is down
[  221.770966] PM: Creating hibernation image:
[  221.772014] PM: Need to copy 220260 pages
[  221.772014] PCI-DMA: Resuming GART IOMMU
[  221.772014] Enabling non-boot CPUs ...
[  221.772014] CPU0 attaching NULL sched-domain.
[  221.781073] SMP alternatives: switching to SMP code
--
[  251.756678]   groups: 0
[  251.756984] CPU1 is down
[  251.756984] Back to C!
[  251.756988] PCI-DMA: Resuming GART IOMMU
[  251.757355] Enabling non-boot CPUs ...
[  251.757515] CPU0 attaching NULL sched-domain.
[  251.766920] SMP alternatives: switching to SMP code



* Rebooted for the first time since i compiled and rebooted -rc4 with
a similar "home-grown" patch on 31. of May, with which I suspended
several times to disk and ram without any problems.

** and also generally the same setup where suspending has disastrous
effects using kernel < 2.6.26-rc5

> ---
> Add resume handling to GART IOMMU.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  arch/x86/kernel/aperture_64.c |    2 +
>  arch/x86/kernel/pci-gart_64.c |   75 +++++++++++++++++++++++++++++++++---------
>  include/asm-x86/gart.h        |    1 
>  3 files changed, 62 insertions(+), 16 deletions(-)
> 
> Index: linux-2.6/arch/x86/kernel/pci-gart_64.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/pci-gart_64.c
> +++ linux-2.6/arch/x86/kernel/pci-gart_64.c
> @@ -549,14 +549,70 @@ static __init unsigned read_aperture(str
>  	return aper_base;
>  }
>  
> +static void enable_gart_translations(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < num_k8_northbridges; i++) {
> +		struct pci_dev *dev;
> +		u32 gatt_reg;
> +		u32 ctl;
> +
> +		dev = k8_northbridges[i];
> +		gatt_reg = __pa(agp_gatt_table) >> 12;
> +		gatt_reg <<= 4;
> +		pci_write_config_dword(dev, 0x98, gatt_reg);
> +		pci_read_config_dword(dev, 0x90, &ctl);
> +
> +		ctl |= 1;
> +		ctl &= ~((1<<4) | (1<<5));
> +
> +		pci_write_config_dword(dev, 0x90, ctl);
> +	}
> +}
> +
> +/*
> + * If fix_up_north_bridges is set, the north bridges have to be fixed up on
> + * resume in the same way as they are handled in gart_iommu_hole_init().
> + */
> +static bool fix_up_north_bridges;
> +static u32 aperture_order;
> +static u32 aperture_alloc;
> +
> +void set_up_gart_resume(u32 aper_order, u32 aper_alloc)
> +{
> +	fix_up_north_bridges = true;
> +	aperture_order = aper_order;
> +	aperture_alloc = aper_alloc;
> +}
> +
>  static int gart_resume(struct sys_device *dev)
>  {
> +	printk(KERN_INFO "PCI-DMA: Resuming GART IOMMU\n");
> +
> +	if (fix_up_north_bridges) {
> +		int i;
> +
> +		for (i = 0; i < num_k8_northbridges; i++) {
> +			struct pci_dev *dev = k8_northbridges[i];
> +
> +			/*
> +			 * Don't enable translations just yet.  That is the next
> +			 * step.  Restore the pre-suspend aperture settings.
> +			 */
> +			pci_write_config_dword(dev, 0x90, aperture_order << 1);
> +			pci_write_config_dword(dev, 0x94, aperture_alloc >> 25);
> +		}
> +	}
> +
> +	enable_gart_translations();
> +
>  	return 0;
>  }
>  
>  static int gart_suspend(struct sys_device *dev, pm_message_t state)
>  {
> -	return -EINVAL;
> +	return 0;
>  }
>  
>  static struct sysdev_class gart_sysdev_class = {
> @@ -614,27 +670,14 @@ static __init int init_k8_gatt(struct ag
>  	memset(gatt, 0, gatt_size);
>  	agp_gatt_table = gatt;
>  
> -	for (i = 0; i < num_k8_northbridges; i++) {
> -		u32 gatt_reg;
> -		u32 ctl;
> -
> -		dev = k8_northbridges[i];
> -		gatt_reg = __pa(gatt) >> 12;
> -		gatt_reg <<= 4;
> -		pci_write_config_dword(dev, 0x98, gatt_reg);
> -		pci_read_config_dword(dev, 0x90, &ctl);
> -
> -		ctl |= 1;
> -		ctl &= ~((1<<4) | (1<<5));
> -
> -		pci_write_config_dword(dev, 0x90, ctl);
> -	}
> +	enable_gart_translations();
>  
>  	error = sysdev_class_register(&gart_sysdev_class);
>  	if (!error)
>  		error = sysdev_register(&device_gart);
>  	if (error)
>  		panic("Could not register gart_sysdev -- would corrupt data on next suspend");
> +
>  	flush_gart();
>  
>  	printk(KERN_INFO "PCI-DMA: aperture base @ %x size %u KB\n",
> Index: linux-2.6/arch/x86/kernel/aperture_64.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/aperture_64.c
> +++ linux-2.6/arch/x86/kernel/aperture_64.c
> @@ -413,4 +413,6 @@ void __init gart_iommu_hole_init(void)
>  		write_pci_config(0, num, 3, 0x90, aper_order<<1);
>  		write_pci_config(0, num, 3, 0x94, aper_alloc>>25);
>  	}
> +
> +	set_up_gart_resume(aper_order, aper_alloc);
>  }
> Index: linux-2.6/include/asm-x86/gart.h
> ===================================================================
> --- linux-2.6.orig/include/asm-x86/gart.h
> +++ linux-2.6/include/asm-x86/gart.h
> @@ -11,6 +11,7 @@ extern void gart_iommu_shutdown(void);
>  extern void __init gart_parse_options(char *);
>  extern void early_gart_iommu_check(void);
>  extern void gart_iommu_hole_init(void);
> +extern void set_up_gart_resume(u32, u32);
>  extern int fallback_aper_order;
>  extern int fallback_aper_force;
>  extern int gart_iommu_aperture;

_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption

[Andi Kleen]

> Well, I was hoping to be able to get this patch into 2.6.26, as I don't really
> like the temporary hack, preventing the affected systems from resuming
> at all, that we have in there, but it seems to be too late. :-(

I agree with you that this patch would be far better than the hack.

-Andi

Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption

[Patrick]

*bump*

Why "too late" ??? It (2.6.26) is still not out! Please (;-) oh please!)
reconsider / push it a bit! It's tested and working perfectly here ever
since. But I'd like for example vmware to work again as will and it
doesn't yet even with 2.6.25... Please let this patch go into 2.6.26 so
there's a chance of having a stock kernel with working vmware, fglrx,
and such again soon!

Thx

Greets - Patrick

On Mit, 2008-06-11 at 17:04 +0200, Andi Kleen wrote:
> > Well, I was hoping to be able to get this patch into 2.6.26, as I don't really
> > like the temporary hack, preventing the affected systems from resuming
> > at all, that we have in there, but it seems to be too late. :-(
> 
> I agree with you that this patch would be far better than the hack.
> 
> -Andi

Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption

[Pavel Machek]

Hi!

> *bump*
> 
> Why "too late" ??? It (2.6.26) is still not out! Please (;-) oh please!)
> reconsider / push it a bit! It's tested and working perfectly here ever
> since. But I'd like for example vmware to work again as will and it
> doesn't yet even with 2.6.25... Please let this patch go into 2.6.26 so
> there's a chance of having a stock kernel with working vmware, fglrx,
> and such again soon!

Can you test 2.6.27-rc2? It should be all there...
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

YOUR SPAM FILTER IS BROCKEN -- was Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption

[Patrick]

Please remove zefir.890m.com from spam urls list!!!

---------------
<linux-pm@lists.linux-foundation.org>: host
    lists.linux-foundation.org[140.211.169.51] said: 554 5.7.1 Email body
    contains Spam URI (in reply to end of DATA command)
---------------
To: 	Pavel Machek <pavel@suse.cz>
Cc: 	Andi Kleen <andi@firstfloor.org>, Rafael J. Wysocki <rjw@sisk.pl>, linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>, pm list <linux-pm@lists.linux-foundation.org>
-----------------

On Thu, 2008-08-07 at 10:17 +0200, Pavel Machek wrote:

> Can you test 2.6.27-rc2? It should be all there...

Ja, sure! I noticed that it's there already... thank you all!

So as you requested, I got myself a -rc2 tree, compiled and tested
#s2ram and #s2disk. It works!

pat@babar:~/tmp/dmesg$ grep -C3 GART dmesg.2.6.27-rc2-gart-suspend.txt

[    0.208013] PCI: Using ACPI for IRQ routing
[    0.228126] PCI-DMA: Disabling AGP.
[    0.228918] PCI-DMA: aperture base @ 20000000 size 65536 KB
[    0.228956] PCI-DMA: using GART IOMMU.
[    0.228993] PCI-DMA: Reserving 64MB of IOMMU area in the AGP aperture
[    0.229429] hpet0: at MMIO 0xfed00000, IRQs 2, 8, 0, 0
[    0.229615] hpet0: 4 32-bit timers, 14318180 Hz
--
[   45.170754] [drm] Initialized drm 1.1.0 20060810
[   45.199305] pci 0000:01:05.0: PCI INT A -> GSI 18 (level, low) -> IRQ
18
[   45.199447] [drm] Initialized radeon 1.29.0 20080528 on minor 0
[   46.127083] [drm] Setting GART location based on new memory map
[   46.128083] [drm] Loading RS690 Microcode
[   46.128116] [drm] Num pipes: 1
[   46.128123] [drm] writeback test succeeded in 1 usecs
--
[ 5341.902040]   groups: 0
[ 5341.902511] CPU1 is down
[ 5341.902511] Back to C!
[ 5341.902535] PCI-DMA: Resuming GART IOMMU
[ 5341.902537] PCI-DMA: Restoring GART aperture settings
[ 5341.902831] Enabling non-boot CPUs ...
[ 5341.903244] SMP alternatives: switching to SMP code
[ 5341.914231] Booting processor 1/1 ip 6000
--
[ 5778.246557] CPU1 is down
[ 5778.246726] PM: Creating hibernation image:
[ 5778.256008] PM: Need to copy 236714 pages
[ 5778.256008] PCI-DMA: Resuming GART IOMMU
[ 5778.256008] PCI-DMA: Restoring GART aperture settings
[ 5778.256008] Enabling non-boot CPUs ...
[ 5778.256008] SMP alternatives: switching to SMP code
[ 5778.267186] Booting processor 1/1 ip 6000

--------

full log and small test report is here:
http://zefir dot 890m dot com/kernel-testing/
http://zefir dot 890m dot com/kernel-testing/dmesg.2.6.27-rc2.txt

I have no clue what caused the oops (see dmesg), but I think it's
unrelated (look at the time stamps).

Maybe it's got something to do with the fact that I'm using
https://launchpad.net/~xorg-edgers xserver now to have 3d acceleration
working for my radeon X1200 (RS690 builtin) and the drm module versions
don't match. Xserver wants 1.30 but starts anyway. I can't fix this
because the script can't compile the
git://anongit.freedesktop.org/git/mesa/drm modules for 2.6.27-rc2 ....
This creates some unpredictable problems with the X server process when
I log into my account (can't find out what triggers it ... cpu usage
goes to 100%, have to reboot) As a workaround, I could log into a guest
account to do the tests but switched back to my patched 2.6.26-rc5 for
now for everyday PC usage.

BTW, if somebody would like to explain to me, why the simultanious usage
of the GART by drm and as iommu DON'T bite each other, I'd be quiet
interrested to learn.

Patrick

Re: >3G => iommu => suspend problems -- was Re: SB600 AHCI: Hard Disk Corruption

[Pavel Machek]

Hi!

> > Can you test 2.6.27-rc2? It should be all there...
> 
> Ja, sure! I noticed that it's there already... thank you all!
> 
> So as you requested, I got myself a -rc2 tree, compiled and tested
> #s2ram and #s2disk. It works!

Good.

> Maybe it's got something to do with the fact that I'm using
> https://launchpad.net/~xorg-edgers xserver now to have 3d acceleration
> working for my radeon X1200 (RS690 builtin) and the drm module versions
> don't match. Xserver wants 1.30 but starts anyway. I can't fix this
> because the script can't compile the
> git://anongit.freedesktop.org/git/mesa/drm modules for 2.6.27-rc2 ....
> This creates some unpredictable problems with the X server process when
> I log into my account (can't find out what triggers it ... cpu usage
> goes to 100%, have to reboot) As a workaround, I could log into a guest
> account to do the tests but switched back to my patched 2.6.26-rc5 for
> now for everyday PC usage.

Yes, 3d has some problems.

> BTW, if somebody would like to explain to me, why the simultanious usage
> of the GART by drm and as iommu DON'T bite each other, I'd be quiet
> interrested to learn.

No idea, sorry.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html