[PATCH 0/4] Fix MTRR suspend support for specific machines (some AMD64, maybe others also)

[PATCH 0/4] Fix MTRR suspend support for specific machines (some AMD64, maybe others also)

[Bernhard Kaindl]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 7989 bytes --]

Hi,

With at least 3 of the following 4 patches, s2ram and s2disk are
fixed on at least the Acer Ferrari 1000 notebooks and at least
s2disk on the Acer Ferrari 5000 notebooks.

The Acer Ferrari 1000 is a 12" Turion 64 X2 notebook with only 1.7 kg weight
while the Ferrari 5000 is a 14" AMD Turion notebook and a bit older but
still not quite old.

Introduction:
-------------

The memory interface of AMD K8 CPUs supports "Extended fixed-range MTRR
Type-Field Encodings" which allow to specify whether accesses to certain
address ranges are executed by accessing RAM thru the AMD Direct Connect
Architecture or by executing memory-mapped I/O. The associated CPU feature
is called IORR's or I/O Range Registers. This allows e.g. to implement
Shadow RAM by copying ROM contents into RAM.

The BIOS of these Acer AMD Turion 64 notebooks makes use of fixed-range
IORRs to implement shadow RAM and other configurations, and it does
this while it transitions the system into ACPI mode, but Linux is not
prepared for that and breaks the IORRs/MTRRs while suspending, resulting
in errors which look like hardware errors. Symptoms vary from from instant
resets and power-offs to SATA link failures.

References:
http://en.wikipedia.org/wiki/MTRR
http://en.wikipedia.org/wiki/Direct_Connect_Architecture
http://en.wikipedia.org/wiki/Memory-mapped_IO
http://en.wikipedia.org/wiki/Random_access_memory#Shadow_RAM

In-depth problem description:
-----------------------------

AMD introduced this MTRR extension with the AMD64 CPUs which have integrated
memory controllers. In part (fixed-range IORRs for addresses below 1MB), AMD
uses the fixed-range MTRR registers which already configure the address range
below 1MB to implement corresponding IORR bits and calls the resulting
memory access methods in combination with the original Intel-style MTRR
bits "Extended fixed-range MTRR Type-Field Encodings". They are documented
in section 7.8.1 of the "AMD64 Architecture Programmer's Manual Volume 2:
System Programming", starting with page 234:
http://amd.com/us-en/assets/content_type/white_papers_and_tech_docs/24593.pdf

The extended fixed-range type-field encodings are enabled using two
bits in the AMD64-specific SYSCFG MSR (described in detail on page 59).

When the extended fixed-range type-field encodings are enabled, all
fixed-range MTRR fields are defined this way:

  7      5          4                3         2                   0
+--------------------------------------------------------------------+
| reserved | IORR RdMem bit | IORR WrMem bit | Intel-style MTRR bits |
+--------------------------------------------------------------------+

Linux MTRR code does not yet support that the BIOS may change fixed-range
MTRRs and when suspending, this leads Linux MTRR code to clear the IORR bits
for some memory regions. The resulting combination of IORR and Intel-style
MTRR bits is not allowed and causes undefined and unpredictable behaviour
(see the last paragraph before table 7-10).

A possible workaround is to detect the Acer Ferraris through DMI and
don't change the fixed-range MTTRs on them. Linux MTRR code has no
external interface to change fixed-range MTRRs, so no functionality
and no syncronisation (it's broken on the Ferrari 1000 after ACPI
is enabled if no real fix is applied) is lost by this patch:

--- linux/arch/i386/kernel/cpu/mtrr/generic.c
+++ linux/arch/i386/kernel/cpu/mtrr/generic.c
@@ -3,6 +3,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/mm.h>
+#include <linux/dmi.h>
 #include <asm/io.h>
 #include <asm/mtrr.h>
 #include <asm/msr.h>
@@ -149,6 +150,13 @@ static int set_fixed_ranges(mtrr_type *
 	int changed = FALSE;
 	int i;
 	unsigned int lo, hi;
+	char *vendor  = dmi_get_system_info(DMI_SYS_VENDOR);
+	char *product = dmi_get_system_info(DMI_PRODUCT_NAME);
+
+	if (vendor && product && !strncmp(vendor, "Acer", 4) &&
+	    (!strncmp(product, "Ferrari 1000", 12) ||
+	     !strncmp(product, "Ferrari 5000", 12)))
+		return FALSE;

 	rdmsr(MTRRfix64K_00000_MSR, lo, hi);
 	if (p[0] != lo || p[1] != hi) {

As DMI is defined "y" on i386 and x86_64 and x86_64 is the only $ARCH
which includes $TOPDIR/arch/i386/kernel/cpu/mtrr, DMI checks can be
done without #ifdef CONFIG_DMI. It's a possible hack for older kernels.

But do we want that workaround also for upstream?

The changes which fix the issues themselfes (and properly work around the
lazyness or "limits" of the BIOS) are a bit more complicated, and need
at least 3 of the following 4 patches. Linux MTRR code is well-trained
in correcting broken BIOS MTRR settings, so from that history, I think
it is good routine to continue that tradition. In this case, most of
what the BIOS does is not broken by design, but just not supported
by Linux yet. The only bug is that the BIOS does not initialize the
2nd core with the same MTRRs that the Boot CPU has after it has enabled
ACPI mode. Linux MTRR code has already mechanisms in place to fix such
BIOS issues, they just need to be extended to also work in circumstances
like those found on the Ferraris.

State of Linux fixed-range MTRR code and changes done by the patches
--------------------------------------------------------------------

Currently, we only get the state of fixed-range MTRRs eary at boot,
before ACPI is enabled. This information is outdated later after
ACPI mode has been activated in the Ferraris.

At least when we suspend the system, we need to have our backup of
the fixed-range MTRRs in sync with the fixed-range MTRRs which are
currently in force.

The first patch provides a function (mtrr_save_fixed_ranges) to
save the current state of the fixed-range MTRRs.

The second patch calls mtrr_save_fixed_ranges() before the non-boot
CPUs are booted. At these times, ACPI is already initialized and the
Ferrari BIOSes changed the MTRRs already, so the new CPUs are then
always initialized with the current fixed-range MTRR of the boot CPU.

Alternatively, or in addition, it might be a good idea to save
the current state of the fixed-range MTRRs of the boot CPU before
suspending to capture any change of the fixed-range MTRRs which
may potentially have occurred after the last secondary CPU has
been booted. That is what the third patch does.

The fourth patch is AMD64-specific and adds support for keeping
the fixed-range MTRRS in sync across CPUs if some fixed-range IORR
bits are set on the boot CPU. If no fixed-range IORR bit is set,
there is no need to enable the support for "Extended fixed-range
type-field encodings" because the access mode only changes if
these bits are set, otherwise, we do not need to enable them.

Best Regards,
Bernhard Kaindl

PS: I attached a program which uses msr.ko (CONFIG_X86_MSR) from
 Processor type and features
     -> [M/*] /dev/cpu/*/msr - Model-specific register support
to dump the contents of the fixed-range MTTRs to stdout.

On a two-CPU system, the output format looks like this:

MSR 0x250: 1e1e1e1e1e1e1e1e | 0606060606060606 |
MSR 0x258: 1e1e1e1e1e1e1e1e | 0606060606060606 |
MSR 0x259: 0000000000000000 | 0000000000000000 |
MSR 0x268: 1515151515151515 | 0505050505050505 |
MSR 0x269: 1515151515151515 | 0505050505050505 |
MSR 0x26a: 0000000000000000 | 0000000000000000 |
MSR 0x26b: 0000000000000000 | 0000000000000000 |
MSR 0x26c: 1515151500000000 | 0505050500000000 |
MSR 0x26d: 1515151515151515 | 0505050505050505 |
MSR 0x26e: 1515151515151515 | 0505050505050505 |
MSR 0x26f: 1515151515151515 | 0505050505050505 |

This dump comes from the Ferrari 1000, where the 2nd core uses the
same Intel-style MTRR values as the 1st core, but the 1st core has
the AMD64 IORR bits set in many places.

On suspend, 2.6.20 sets all of them to 0, making the dump look like
the dump of the second core, and shortly afterwards, the problems
start. To have a sane MTRR setup the MTRRs of the CPUs should always
match, but this has so far not lead to problems, it seems. However,
Intel and AMD manuals tell us to keep MTRRs in sync for good reasons.

[-- Attachment #2: Type: TEXT/X-CSRC, Size: 1349 bytes --]

/*
 * Test program for reading current MTRR values
 *
 *	(C) 2007 Bernhard Kaindl
 *	(C) 2000 Dave Jones, Arjan van de Ven.
 */

#include <stdio.h>
#include <unistd.h>

/*  rdmsr - read MSR, taken from Powertweak Linux
 *	(C) 2000 Dave Jones, Arjan van de Ven.
 * 	Licensed under the terms of the GNU GPL License version 2.
 */
int rdmsr(int cpu, unsigned long msrindex, unsigned long long *val)
{
	char cpuname[36];
	int fh, ret;

	snprintf (cpuname,15, "/dev/cpu/%d/msr", cpu);

	fh = open (cpuname, 0);

	if (fh==-1)
		ret = -1;
	else {
		lseek (fh, msrindex, SEEK_SET);
		ret = (read (fh, val, 8) == 8);
		close (fh);
	}
	return ret;
}

int print_msr_cpu(int cpu, int reg)
{
	unsigned long long val;
	int ret;

	if ((ret = rdmsr(cpu, reg, &val)) > 0)
		printf("%016llx", val);
	return ret;
}

print_msr(int reg)
{
	int i = 1, cpu = 0;
	printf("MSR 0x%03x: ", reg);

	if (print_msr_cpu(cpu++, reg) <= 0)
		printf("read error - Is msr.ko loaded and are the devices created?\n");

	while (cpu < 16) {
		printf(" | ");
		if (print_msr_cpu(cpu++, reg) <= 0)
			break;
	}

	printf("\n");
}

main() {
	int i;

	print_msr(0x250);
	print_msr(0x258);
	print_msr(0x259);
	for (i=0x268; i<0x270; i++)
		print_msr(i);
}

[PATCH 1/4] Implement mtrr_save_fixed_ranges()

[Bernhard Kaindl]

This patch implements mtrr_save_fixed_ranges() which can be called
from anywhere anytime to retrieve the fixed-range MTRRs from the
current CPU and saves these MTRR values to the MTRR state struct.

This function calls get_fixed_ranges(), passing mtrr_state.fixed_ranges
which is the element of the static struct which stores our current
backup of the fixed-range MTRR values which all CPUs shall be
using.

Because mtrr_save_fixed_ranges calls get_fixed_ranges() after
kernel initialisation time, __init needs to be removed from
the declaration of get_fixed_ranges().

Signed-off-by: Bernhard Kaindl <bk@suse.de>

--- linux-2.6.20.orig/arch/i386/kernel/cpu/mtrr/generic.c
+++ linux-2.6.20/arch/i386/kernel/cpu/mtrr/generic.c
@@ -37,7 +37,11 @@ get_mtrr_var_range(unsigned int index, s
 	rdmsr(MTRRphysMask_MSR(index), vr->mask_lo, vr->mask_hi);
 }

-static void __init
+/**
+ * Retrieves the current fixed-range MTRRs from the current CPU
+ * \param frs address where to write the current MTRR contents
+ */
+static void
 get_fixed_ranges(mtrr_type * frs)
 {
 	unsigned int *p = (unsigned int *) frs;
@@ -51,6 +55,16 @@ get_fixed_ranges(mtrr_type * frs)
 		rdmsr(MTRRfix4K_C0000_MSR + i, p[6 + i * 2], p[7 + i * 2]);
 }

+/**
+ * Updates our copy of the state of the fixed-range MTRR values
+ * with the current fixed-range MTRR contents from the current CPU
+ * \param info dummy needed for use by smp_call_function_single()
+ */
+void mtrr_save_fixed_ranges(void *info)
+{
+	get_fixed_ranges(mtrr_state.fixed_ranges);
+}
+
 static void __init print_fixed(unsigned base, unsigned step, const mtrr_type*types)
 {
 	unsigned i;
Index: linux-2.6.20/include/asm-i386/mtrr.h
===================================================================
--- linux-2.6.20.orig/include/asm-i386/mtrr.h
+++ linux-2.6.20/include/asm-i386/mtrr.h
@@ -69,6 +69,7 @@ struct mtrr_gentry

 /*  The following functions are for use by other drivers  */
 # ifdef CONFIG_MTRR
+extern void mtrr_save_fixed_ranges(void *);
 extern int mtrr_add (unsigned long base, unsigned long size,
 		     unsigned int type, char increment);
 extern int mtrr_add_page (unsigned long base, unsigned long size,
@@ -79,6 +80,7 @@ extern void mtrr_centaur_report_mcr(int
 extern void mtrr_ap_init(void);
 extern void mtrr_bp_init(void);
 #  else
+#define mtrr_save_fixed_ranges(arg) do {} while (0)
 static __inline__ int mtrr_add (unsigned long base, unsigned long size,
 				unsigned int type, char increment)
 {
Index: linux-2.6.20/include/asm-x86_64/proto.h
===================================================================
--- linux-2.6.20.orig/include/asm-x86_64/proto.h
+++ linux-2.6.20/include/asm-x86_64/proto.h
@@ -19,9 +19,11 @@ extern void mcheck_init(struct cpuinfo_x
 #ifdef CONFIG_MTRR
 extern void mtrr_ap_init(void);
 extern void mtrr_bp_init(void);
+extern void mtrr_save_fixed_ranges(void *);
 #else
 #define mtrr_ap_init() do {} while (0)
 #define mtrr_bp_init() do {} while (0)
+#define mtrr_save_fixed_ranges(arg) do {} while (0)
 #endif
 extern void init_memory_mapping(unsigned long start, unsigned long end);

In this current implementation, which is used in other patches,
mtrr_save_fixed_ranges() accepts a dummy void pointer because
in the current implementation of one of these patches, this
function may be called from smp_call_function_single() which
requires that this function takes a void pointer argument.

If CONFIG_MTRR is not set, we define mtrr_save_fixed_ranges
as an empty statement because there is nothing to do.

[PATCH 2/4] Save the MTRRs of the BSP before booting an AP

[Bernhard Kaindl]

While I haven't found this as cause of the MTRR suspend issue on the
Ferraris, AMD and Intel x86 CPU manuals state that it is the responsibility
of system software to initialize and maintain MTRR consistency across
all processors in Multi-Processing Environments:

Quote from page 188 of the AMD64 System Programming manual (Volume 2):

-----------------------------------------------------------------------------
7.6.5 MTRRs in Multi-Processing Environments

"In multi-processing environments, the MTRRs located in all processors must
characterize memory in the same way. Generally, this means that identical
values are written to the MTRRs used by the processors." (short omission here)
"Failure to do so may result in coherency violations or loss of atomicity.
Processor implementations do not check the MTRR settings in other processors
to ensure consistency. It is the responsibility of system software to
initialize and maintain MTRR consistency across all processors."
-----------------------------------------------------------------------------

Current Linux MTRR code already implements the above in the case that the
BIOS does not properly initialize MTRRs on the secondary processors,
but the case where the fixed-range MTRRs of the boot processor are changed
after Linux started to boot, before the initialsation of a secondary
processor, is not handled yet.

In this case, secondary processors are currently initialized by Linux
with MTRRs which the boot processor had very early, when mtrr_bp_init()
did run, but not with the MTRRs which the boot processor uses at the
time when that secondary processors is actually booted,
causing differing MTRR contents on the secondary processors.

Such situation happens on Acer Ferrari 1000 and 5000 notebooks where the
BIOS enables and sets AMD-specific IORR bits in the fixed-range MTRRs
of the boot processor when it transitions the system into ACPI mode.
The SMI handler of the BIOS does this in SMM, entered while Linux ACPI
code runs acpi_enable().

Other occasions where the SMI handler of the BIOS may change bits in
the MTRRs could occur as well. To initialize newly booted secodary
processors with the fixed-range MTRRs which the boot processor uses
at that time, this patch saves the fixed-range MTRRs of the boot
processor before new secondary processors are started. When the
secondary processors run their Linux initialisation code, their
fixed-range MTRRs will be updated with the saved fixed-range MTRRs.

Possible TODOs:

*) CPU-hotplugging outside of SMP suspend/resume is not yet tested
   with this patch.

*) If, even in this case, an AP never runs i386/do_boot_cpu or x86_64/cpu_up,
   then the calls to mtrr_save_state() could be replaced by calls to
   mtrr_save_fixed_ranges(NULL) and  mtrr_save_state() would not be
   needed.

   That would need either verification of the CPU-hotplug code.

*) The MTRRs of other running processors are not yet checked at this
   time but it might be interesting to syncronize the MTTRs of all
   processors before booting new CPUs. That would be an incremental
   patch, of rather low priority since there is no machine known so
   far which would require this.

Signed-off-by: Bernhard Kaindl <bk@suse.de>

--- linux-2.6.20.orig/arch/i386/kernel/smpboot.c
+++ linux-2.6.20/arch/i386/kernel/smpboot.c
@@ -59,6 +59,7 @@
 #include <asm/nmi.h>
 #include <asm/pda.h>
 #include <asm/genapic.h>
+#include <asm/mtrr.h>

 #include <mach_apic.h>
 #include <mach_wakecpu.h>
@@ -951,6 +952,12 @@ static int __cpuinit do_boot_cpu(int api
 	unsigned short nmi_high = 0, nmi_low = 0;

 	/*
+	 * Save current MTRR state in case it was changed since early boot
+	 * (e.g. by the ACPI SMI) to initialize new CPUs with MTRRs in sync:
+	 */
+	mtrr_save_state();
+
+	/*
 	 * We can't use kernel_thread since we must avoid to
 	 * reschedule the child.
 	 */
Index: linux-2.6.20/arch/x86_64/kernel/smpboot.c
===================================================================
--- linux-2.6.20.orig/arch/x86_64/kernel/smpboot.c
+++ linux-2.6.20/arch/x86_64/kernel/smpboot.c
@@ -1157,6 +1157,12 @@ int __cpuinit __cpu_up(unsigned int cpu)
  		return -ENOSYS;
 	}

+	/*
+	 * Save current MTRR state in case it was changed since early boot
+	 * (e.g. by the ACPI SMI) to initialize new CPUs with MTRRs in sync:
+	 */
+	mtrr_save_state();
+
 	per_cpu(cpu_state, cpu) = CPU_UP_PREPARE;
 	/* Boot it! */
 	err = do_boot_cpu(cpu, apicid);
Index: linux-2.6.20/include/asm-i386/mtrr.h
===================================================================
--- linux-2.6.20.orig/include/asm-i386/mtrr.h
+++ linux-2.6.20/include/asm-i386/mtrr.h
@@ -25,6 +25,7 @@

 #include <linux/ioctl.h>
 #include <linux/errno.h>
+#include <linux/smp.h>

 #define	MTRR_IOCTL_BASE	'M'

@@ -70,6 +71,16 @@ struct mtrr_gentry
 /*  The following functions are for use by other drivers  */
 # ifdef CONFIG_MTRR
 extern void mtrr_save_fixed_ranges(void *);
+/**
+ * Save current fixed-range MTRR state of the BSP
+ */
+static __inline__ void mtrr_save_state (void)
+{
+	if (smp_processor_id() == 0)
+		mtrr_save_fixed_ranges(NULL);
+	else
+		smp_call_function_single(0, mtrr_save_fixed_ranges, NULL, 1, 1);
+}
 extern int mtrr_add (unsigned long base, unsigned long size,
 		     unsigned int type, char increment);
 extern int mtrr_add_page (unsigned long base, unsigned long size,
@@ -81,6 +92,7 @@ extern void mtrr_ap_init(void);
 extern void mtrr_bp_init(void);
 #  else
 #define mtrr_save_fixed_ranges(arg) do {} while (0)
+#define mtrr_save_state() do {} while (0)
 static __inline__ int mtrr_add (unsigned long base, unsigned long size,
 				unsigned int type, char increment)
 {
Index: linux-2.6.20/include/asm-x86_64/proto.h
===================================================================
--- linux-2.6.20.orig/include/asm-x86_64/proto.h
+++ linux-2.6.20/include/asm-x86_64/proto.h
@@ -2,6 +2,7 @@
 #define _ASM_X8664_PROTO_H 1

 #include <asm/ldt.h>
+#include <linux/smp.h>

 /* misc architecture specific prototypes */

@@ -20,10 +21,21 @@ extern void mcheck_init(struct cpuinfo_x
 extern void mtrr_ap_init(void);
 extern void mtrr_bp_init(void);
 extern void mtrr_save_fixed_ranges(void *);
+static __inline__ void mtrr_save_state (void)
+{
+	/*
+	 * Save current fixed-range MTRR state of the BSP:
+	 */
+	if (smp_processor_id() == 0)
+		mtrr_save_fixed_ranges(NULL);
+	else
+		smp_call_function_single(0, mtrr_save_fixed_ranges, NULL, 1, 1);
+}
 #else
 #define mtrr_ap_init() do {} while (0)
 #define mtrr_bp_init() do {} while (0)
 #define mtrr_save_fixed_ranges(arg) do {} while (0)
+#define mtrr_save_state() do {} while (0)
 #endif
 extern void init_memory_mapping(unsigned long start, unsigned long end);

If CONFIG_MTRR is not set, we define mtrr_save_state
as an empty statement because there is nothing to do.

Re: [PATCH 2/4] Save the MTRRs of the BSP before booting an AP

[Andrew Morton]

On Tue, 3 Apr 2007 16:00:36 +0200 (CEST) Bernhard Kaindl <bk@suse.de> wrote:

> --- linux-2.6.20.orig/arch/i386/kernel/smpboot.c
> +++ linux-2.6.20/arch/i386/kernel/smpboot.c
> @@ -59,6 +59,7 @@
>  #include <asm/nmi.h>
>  #include <asm/pda.h>
>  #include <asm/genapic.h>
> +#include <asm/mtrr.h>
> 

This inclusion breaks `make headers_check'.

Please always at least test allmodconfig builds before releasing a
patchset.  Additional hints are in Documentation/SubmitChecklist.

+static __inline__ void mtrr_save_state (void)
+{
+	if (smp_processor_id() == 0)
+		mtrr_save_fixed_ranges(NULL);
+	else
+		smp_call_function_single(0, mtrr_save_fixed_ranges, NULL, 1, 1);
+}

- Please use inline, not __inline__

- No space before the (

- We should uninline this function

  - It is not performance critical

  - It is probably too large to be inlined anwyay

  - It uses a lot of tricky stuff which requires a lot of header files.


From: Andrew Morton <akpm@linux-foundation.org>

Fix `make headers_check'.

Cc: Andi Kleen <ak@suse.de>
Cc: Bernhard Kaindl <bk@suse.de>
Cc: Dave Jones <davej@codemonkey.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 arch/i386/kernel/cpu/mtrr/main.c |   11 +++++++++++
 include/asm-i386/mtrr.h          |   12 +-----------
 include/asm-x86_64/proto.h       |   12 +-----------
 3 files changed, 13 insertions(+), 22 deletions(-)

diff -puN arch/i386/kernel/smpboot.c~mtrr-save-the-mtrrs-of-the-bsp-before-booting-an-ap-fix arch/i386/kernel/smpboot.c
diff -puN arch/x86_64/kernel/smpboot.c~mtrr-save-the-mtrrs-of-the-bsp-before-booting-an-ap-fix arch/x86_64/kernel/smpboot.c
diff -puN include/asm-i386/mtrr.h~mtrr-save-the-mtrrs-of-the-bsp-before-booting-an-ap-fix include/asm-i386/mtrr.h
--- a/include/asm-i386/mtrr.h~mtrr-save-the-mtrrs-of-the-bsp-before-booting-an-ap-fix
+++ a/include/asm-i386/mtrr.h
@@ -25,7 +25,6 @@
 
 #include <linux/ioctl.h>
 #include <linux/errno.h>
-#include <linux/smp.h>
 
 #define	MTRR_IOCTL_BASE	'M'
 
@@ -71,16 +70,7 @@ struct mtrr_gentry
 /*  The following functions are for use by other drivers  */
 # ifdef CONFIG_MTRR
 extern void mtrr_save_fixed_ranges(void *);
-/**
- * Save current fixed-range MTRR state of the BSP
- */
-static __inline__ void mtrr_save_state (void)
-{
-	if (smp_processor_id() == 0)
-		mtrr_save_fixed_ranges(NULL);
-	else
-		smp_call_function_single(0, mtrr_save_fixed_ranges, NULL, 1, 1);
-}
+extern void mtrr_save_state(void);
 extern int mtrr_add (unsigned long base, unsigned long size,
 		     unsigned int type, char increment);
 extern int mtrr_add_page (unsigned long base, unsigned long size,
diff -puN include/asm-x86_64/proto.h~mtrr-save-the-mtrrs-of-the-bsp-before-booting-an-ap-fix include/asm-x86_64/proto.h
--- a/include/asm-x86_64/proto.h~mtrr-save-the-mtrrs-of-the-bsp-before-booting-an-ap-fix
+++ a/include/asm-x86_64/proto.h
@@ -2,7 +2,6 @@
 #define _ASM_X8664_PROTO_H 1
 
 #include <asm/ldt.h>
-#include <linux/smp.h>
 
 /* misc architecture specific prototypes */
 
@@ -19,16 +18,7 @@ extern void mcheck_init(struct cpuinfo_x
 extern void mtrr_ap_init(void);
 extern void mtrr_bp_init(void);
 extern void mtrr_save_fixed_ranges(void *);
-static __inline__ void mtrr_save_state (void)
-{
-	/*
-	 * Save current fixed-range MTRR state of the BSP:
-	 */
-	if (smp_processor_id() == 0)
-		mtrr_save_fixed_ranges(NULL);
-	else
-		smp_call_function_single(0, mtrr_save_fixed_ranges, NULL, 1, 1);
-}
+extern void mtrr_save_state(void);
 #else
 #define mtrr_ap_init() do {} while (0)
 #define mtrr_bp_init() do {} while (0)
diff -puN arch/i386/kernel/cpu/mtrr/main.c~mtrr-save-the-mtrrs-of-the-bsp-before-booting-an-ap-fix arch/i386/kernel/cpu/mtrr/main.c
--- a/arch/i386/kernel/cpu/mtrr/main.c~mtrr-save-the-mtrrs-of-the-bsp-before-booting-an-ap-fix
+++ a/arch/i386/kernel/cpu/mtrr/main.c
@@ -729,6 +729,17 @@ void mtrr_ap_init(void)
 	local_irq_restore(flags);
 }
 
+/**
+ * Save current fixed-range MTRR state of the BSP
+ */
+void mtrr_save_state(void)
+{
+	if (smp_processor_id() == 0)
+		mtrr_save_fixed_ranges(NULL);
+	else
+		smp_call_function_single(0, mtrr_save_fixed_ranges, NULL, 1, 1);
+}
+
 static int __init mtrr_init_finialize(void)
 {
 	if (!mtrr_if)
_

[PATCH 3/4] BSP: save and restore fixed-range MTRRs during suspend

[Bernhard Kaindl]

Some BIOSes may modify fixed-range MTRRs in SMM, e.g. when they
transition the system into ACPI mode, which is entered thru an SMI,
triggered by Linux in acpi_enable().

SMIs which cause that Linux is interrupted and BIOS code is
executed (which may change e.g. fixed-range MTRRs) in SMM may
be raised by an embedded system controller which is often found
in notebooks also at other occasions.

If we would not update our copy of the fixed-range MTRRs before
suspending to RAM or to disk, restore_processor_state() would
set the fixed-range MTRRs of the BSP using old backup values
which may be outdated and this could cause the system to fail
later during resume.

This patch ensures that our copy of the fixed-range MTRRs
is updated when saving the boot processor state on suspend
to disk and suspend to RAM.

In combination with other patches this allows to fix s2ram
and s2disk on the Acer Ferrari 1000 notebook and at least
s2disk on the Acer Ferrari 5000 notebook.

Signed-off-by: Bernhard Kaindl <bk@suse.de>

--- linux-2.6.20/arch/i386/power/cpu.c
+++ linux-2.6.20/arch/i386/power/cpu.c
@@ -21,6 +21,7 @@ unsigned long saved_context_eflags;

 void __save_processor_state(struct saved_context *ctxt)
 {
+	mtrr_save_fixed_ranges(NULL);
 	kernel_fpu_begin();

 	/*
--- linux-2.6.20/arch/x86_64/kernel/suspend.c
+++ linux-2.6.20/arch/x86_64/kernel/suspend.c
@@ -46,6 +46,7 @@ void __save_processor_state(struct saved
 	rdmsrl(MSR_FS_BASE, ctxt->fs_base);
 	rdmsrl(MSR_GS_BASE, ctxt->gs_base);
 	rdmsrl(MSR_KERNEL_GS_BASE, ctxt->gs_kernel_base);
+	mtrr_save_fixed_ranges(NULL);

 	/*
 	 * control registers

[PATCH 4/4] Enable fixed-range IORRs to sync RdMem and WrMem

[Bernhard Kaindl]

If our copy of the MTRRs of the BSP has RdMem or WrMem set and
we are running on an AMD64/K8 system, the boot CPU must have had
MtrrFixDramEn and MtrrFixDramModEn set (otherwise our RDMSR would
have copied these bits cleared), so we set them on this CPU as well.

This allows us to keep the AMD64/K8 RdMem and WrMem bits in sync
across the CPUs of SMP systems in order to fullfill the duty of
system software to "initialize and maintain MTRR consistency
across all processors." as written in the AMD and Intel manuals.

If an WRMSR instruction fails because MtrrFixDramModEn is not
set, I expect that also the Intel-style MTRR bits are not updated.

Signed-off-by: Bernhard Kaindl <bk@suse.de>

--- linux-2.6.20.orig/arch/i386/kernel/cpu/mtrr/generic.c
+++ linux-2.6.20/arch/i386/kernel/cpu/mtrr/generic.c
@@ -20,12 +20,29 @@ struct mtrr_state {
 	mtrr_type def_type;
 };

+struct fixed_range_block {
+	int base_msr; /* start address of an MTRR block */
+	int ranges;   /* number of MTRRs in this block  */
+};
+
+static struct fixed_range_block fixed_range_blocks[] = {
+	{ MTRRfix64K_00000_MSR, 1 }, /* one  64k MTRR  */
+	{ MTRRfix16K_80000_MSR, 2 }, /* two  16k MTRRs */
+	{ MTRRfix4K_C0000_MSR,  8 }, /* eight 4k MTRRs */
+	{}
+};
+
 static unsigned long smp_changes_mask;
 static struct mtrr_state mtrr_state = {};

 #undef MODULE_PARAM_PREFIX
 #define MODULE_PARAM_PREFIX "mtrr."

+#define K8_MSR_SYSCFG			0xc0010010 /* AMD64/K8 SYSCFG MSR  */
+#define K8_MTRRFIXRANGE_DRAM_ENABLE	0x00040000 /* MtrrFixDramEn bit    */
+#define K8_MTRRFIXRANGE_DRAM_MODIFY	0x00080000 /* MtrrFixDramModEn bit */
+#define K8_MTRR_RDMEM_WRMEM_MASK	0x18181818 /* Mask: RdMem|WrMem    */
+
 static __initdata int mtrr_show;
 module_param_named(show, mtrr_show, bool, 0);

@@ -161,6 +178,44 @@ void mtrr_wrmsr(unsigned msr, unsigned a
 			smp_processor_id(), msr, a, b);
 }

+/**
+ * Enable and allow read/write of extended fixed-range MTRR bits on K8 CPUs
+ * see AMD publication no. 24593, chapter 3.2.1 for more information
+ */
+static inline void k8_enable_fixed_iorrs(void)
+{
+	unsigned lo, hi;
+
+	rdmsr(K8_MSR_SYSCFG, lo, hi);
+	mtrr_wrmsr(K8_MSR_SYSCFG, lo
+				| K8_MTRRFIXRANGE_DRAM_ENABLE
+				| K8_MTRRFIXRANGE_DRAM_MODIFY, hi);
+}
+
+/**
+ * Checks and updates an fixed-range MTRR if it differs from the value it
+ * should have. If K8 extenstions are wanted, update the K8 SYSCFG MSR also.
+ * see AMD publication no. 24593, chapter 7.8.1, page 233 for more information
+ * \param msr MSR address of the MTTR which should be checked and updated
+ * \param changed pointer which indicates whether the MTRR needed to be changed
+ * \param msrwords pointer to the MSR values which the MSR should have
+ */
+static void set_fixed_range(int msr, int * changed, unsigned int * msrwords)
+{
+	unsigned lo, hi;
+
+	rdmsr(msr, lo, hi);
+
+	if (lo != msrwords[0] || hi != msrwords[1]) {
+		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
+		    boot_cpu_data.x86 == 15 &&
+		    ((msrwords[0] | msrwords[1]) & K8_MTRR_RDMEM_WRMEM_MASK))
+			k8_enable_fixed_iorrs();
+		mtrr_wrmsr(msr, msrwords[0], msrwords[1]);
+		*changed = TRUE;
+	}
+}
+
 int generic_get_free_region(unsigned long base, unsigned long size, int replace_reg)
 /*  [SUMMARY] Get a free MTRR.
     <base> The starting (base) address of the region.
@@ -210,36 +265,21 @@ static void generic_get_mtrr(unsigned in
 	*type = base_lo & 0xff;
 }

+/**
+ * Checks and updates the fixed-range MTRRs if they differ from the saved set
+ * \param frs pointer to fixed-range MTRR values, saved by get_fixed_ranges()
+ */
 static int set_fixed_ranges(mtrr_type * frs)
 {
-	unsigned int *p = (unsigned int *) frs;
+	unsigned long long *saved = (unsigned long long *) frs;
 	int changed = FALSE;
-	int i;
-	unsigned int lo, hi;
+	int block=-1, range;

-	rdmsr(MTRRfix64K_00000_MSR, lo, hi);
-	if (p[0] != lo || p[1] != hi) {
-		mtrr_wrmsr(MTRRfix64K_00000_MSR, p[0], p[1]);
-		changed = TRUE;
-	}
+	while (fixed_range_blocks[++block].ranges)
+	    for (range=0; range < fixed_range_blocks[block].ranges; range++)
+		set_fixed_range(fixed_range_blocks[block].base_msr + range,
+		    &changed, (unsigned int *) saved++);

-	for (i = 0; i < 2; i++) {
-		rdmsr(MTRRfix16K_80000_MSR + i, lo, hi);
-		if (p[2 + i * 2] != lo || p[3 + i * 2] != hi) {
-			mtrr_wrmsr(MTRRfix16K_80000_MSR + i, p[2 + i * 2],
-			      p[3 + i * 2]);
-			changed = TRUE;
-		}
-	}
-
-	for (i = 0; i < 8; i++) {
-		rdmsr(MTRRfix4K_C0000_MSR + i, lo, hi);
-		if (p[6 + i * 2] != lo || p[7 + i * 2] != hi) {
-			mtrr_wrmsr(MTRRfix4K_C0000_MSR + i, p[6 + i * 2],
-			      p[7 + i * 2]);
-			changed = TRUE;
-		}
-	}
 	return changed;
 }

Re: [PATCH 0/4] Fix MTRR suspend support for specific machines (some AMD64, maybe others also)

[Dave Jones]

On Tue, Apr 03, 2007 at 03:55:32PM +0200, Bernhard Kaindl wrote:

 > PS: I attached a program which uses msr.ko (CONFIG_X86_MSR) from
 >  Processor type and features
 >      -> [M/*] /dev/cpu/*/msr - Model-specific register support
 > to dump the contents of the fixed-range MTTRs to stdout.
 > 
 > On a two-CPU system, the output format looks like this:
 > 
 > MSR 0x250: 1e1e1e1e1e1e1e1e | 0606060606060606 |
 > MSR 0x258: 1e1e1e1e1e1e1e1e | 0606060606060606 |
 > MSR 0x259: 0000000000000000 | 0000000000000000 |
 > MSR 0x268: 1515151515151515 | 0505050505050505 |
 > MSR 0x269: 1515151515151515 | 0505050505050505 |
 > MSR 0x26a: 0000000000000000 | 0000000000000000 |
 > MSR 0x26b: 0000000000000000 | 0000000000000000 |
 > MSR 0x26c: 1515151500000000 | 0505050500000000 |
 > MSR 0x26d: 1515151515151515 | 0505050505050505 |
 > MSR 0x26e: 1515151515151515 | 0505050505050505 |
 > MSR 0x26f: 1515151515151515 | 0505050505050505 |

FWIW, x86info --mtrr would have done this for you :)

 > To have a sane MTRR setup the MTRRs of the CPUs should always
 > match, but this has so far not lead to problems, it seems. However,
 > Intel and AMD manuals tell us to keep MTRRs in sync for good reasons.

Indeed.  From a quick lookover, I don't see anything obviously
wrong in your patches, though Jan and Andi probably remember more
gory details about that code than I'll admit to.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH 0/4] Fix MTRR suspend support for specific machines (some AMD64, maybe others also)

[Pavel Machek]

Hi!
> 
> With at least 3 of the following 4 patches, s2ram and s2disk are
> fixed on at least the Acer Ferrari 1000 notebooks and at least
> s2disk on the Acer Ferrari 5000 notebooks.
> 
> The Acer Ferrari 1000 is a 12" Turion 64 X2 notebook with only 1.7 kg weight
> while the Ferrari 5000 is a 14" AMD Turion notebook and a bit older but
> still not quite old.
> 
> Introduction:
> -------------
> 
> The memory interface of AMD K8 CPUs supports "Extended fixed-range MTRR
> Type-Field Encodings" which allow to specify whether accesses to certain
> address ranges are executed by accessing RAM thru the AMD Direct Connect
> Architecture or by executing memory-mapped I/O. The associated CPU feature
> is called IORR's or I/O Range Registers. This allows e.g. to implement
> Shadow RAM by copying ROM contents into RAM.
> 
> The BIOS of these Acer AMD Turion 64 notebooks makes use of fixed-range
> IORRs to implement shadow RAM and other configurations, and it does
> this while it transitions the system into ACPI mode, but Linux is not
> prepared for that and breaks the IORRs/MTRRs while suspending, resulting
> in errors which look like hardware errors. Symptoms vary from from instant
> resets and power-offs to SATA link failures.
> 
> References:
> http://en.wikipedia.org/wiki/MTRR
> http://en.wikipedia.org/wiki/Direct_Connect_Architecture
> http://en.wikipedia.org/wiki/Memory-mapped_IO
> http://en.wikipedia.org/wiki/Random_access_memory#Shadow_RAM
> 
> In-depth problem description:
> -----------------------------
> 
> AMD introduced this MTRR extension with the AMD64 CPUs which have integrated
> memory controllers. In part (fixed-range IORRs for addresses below 1MB), AMD
> uses the fixed-range MTRR registers which already configure the address range
> below 1MB to implement corresponding IORR bits and calls the resulting
> memory access methods in combination with the original Intel-style MTRR
> bits "Extended fixed-range MTRR Type-Field Encodings". They are documented
> in section 7.8.1 of the "AMD64 Architecture Programmer's Manual Volume 2:
> System Programming", starting with page 234:
> http://amd.com/us-en/assets/content_type/white_papers_and_tech_docs/24593.pdf
> 
> The extended fixed-range type-field encodings are enabled using two
> bits in the AMD64-specific SYSCFG MSR (described in detail on page 59).
> 
> When the extended fixed-range type-field encodings are enabled, all
> fixed-range MTRR fields are defined this way:
> 
>   7      5          4                3         2                   0
> +--------------------------------------------------------------------+
> | reserved | IORR RdMem bit | IORR WrMem bit | Intel-style MTRR bits |
> +--------------------------------------------------------------------+
> 
> Linux MTRR code does not yet support that the BIOS may change fixed-range
> MTRRs and when suspending, this leads Linux MTRR code to clear the IORR bits
> for some memory regions. The resulting combination of IORR and Intel-style
> MTRR bits is not allowed and causes undefined and unpredictable behaviour
> (see the last paragraph before table 7-10).
> 
> A possible workaround is to detect the Acer Ferraris through DMI and
> don't change the fixed-range MTTRs on them. Linux MTRR code has no
> external interface to change fixed-range MTRRs, so no functionality
> and no syncronisation (it's broken on the Ferrari 1000 after ACPI
> is enabled if no real fix is applied) is lost by this patch:
> 
> --- linux/arch/i386/kernel/cpu/mtrr/generic.c
> +++ linux/arch/i386/kernel/cpu/mtrr/generic.c
> @@ -3,6 +3,7 @@
>  #include <linux/init.h>
>  #include <linux/slab.h>
>  #include <linux/mm.h>
> +#include <linux/dmi.h>
>  #include <asm/io.h>
>  #include <asm/mtrr.h>
>  #include <asm/msr.h>
> @@ -149,6 +150,13 @@ static int set_fixed_ranges(mtrr_type *
>  	int changed = FALSE;
>  	int i;
>  	unsigned int lo, hi;
> +	char *vendor  = dmi_get_system_info(DMI_SYS_VENDOR);
> +	char *product = dmi_get_system_info(DMI_PRODUCT_NAME);
> +
> +	if (vendor && product && !strncmp(vendor, "Acer", 4) &&
> +	    (!strncmp(product, "Ferrari 1000", 12) ||
> +	     !strncmp(product, "Ferrari 5000", 12)))
> +		return FALSE;


What would happen if we just did "return FALSE" here, for all
machines? Lower system performance?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 0/4] Fix MTRR suspend support for specific machines (some AMD64, maybe others also)

[Andrew Morton]

On Tue, 3 Apr 2007 15:55:32 +0200 (CEST) Bernhard Kaindl <bk@suse.de> wrote:

> With at least 3 of the following 4 patches, s2ram and s2disk are
> fixed on at least the Acer Ferrari 1000 notebooks and at least
> s2disk on the Acer Ferrari 5000 notebooks.

These patches cause my Vaio to oops during suspend-to-disk.

oops: http://userweb.kernel.org/~akpm/s5000499.jpg
config: http://userweb.kernel.org/~akpm/config-sony.txt