[PATCH 1/4] Adds mtrr_save_fixed_ranges() for use in two later patches.

[PATCH 1/4] Adds mtrr_save_fixed_ranges() for use in two later patches.

[Bernhard Kaindl]

-----------------------------------------------------------------
Update: Rediffed against 2.6.21-rc6-mm1, the only change I had
to make was tiny: Andrew's tree has a patch applied which changes
many functions from being prefixed with __init to __cpuinit, which
is actually wrong and will (IMHO) be replaced by another patch:

http://lkml.org/lkml/2007/4/7/113
Jeremy Fitzhardinge: [PATCH 2/2] x86: clean up identify_cpu)

The above patch splits up identify_cpu() into a part for the BSP and
the APs, which also means that the patch to which this patch rejected
to, will then be obsolete, many of the functions can be freed with
__init again, but this is a incremental cleanup which can happen
after the patch from Jeremy Fitzhardinge above is applied. NOTE:
I didn't test Jeremy's patch on x86_64 so far (he has no such
machine) but it's definitely right to do what his patch does, IMHO.
-----------------------------------------------------------------

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.

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

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

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

---
 arch/i386/kernel/cpu/mtrr/generic.c |   16 +++++++++++++++-
 include/asm-i386/mtrr.h             |    2 ++
 include/asm-x86_64/proto.h          |    2 ++
 3 files changed, 19 insertions(+), 1 deletion(-)

Index: 2.6.21-rc6-mm1/arch/i386/kernel/cpu/mtrr/generic.c
===================================================================
--- 2.6.21-rc6-mm1.orig/arch/i386/kernel/cpu/mtrr/generic.c
+++ 2.6.21-rc6-mm1/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 __cpuinit
+/**
+ * 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 __cpuinit print_fixed(unsigned base, unsigned step, const mtrr_type*types)
 {
 	unsigned i;
Index: 2.6.21-rc6-mm1/include/asm-i386/mtrr.h
===================================================================
--- 2.6.21-rc6-mm1.orig/include/asm-i386/mtrr.h
+++ 2.6.21-rc6-mm1/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: 2.6.21-rc6-mm1/include/asm-x86_64/proto.h
===================================================================
--- 2.6.21-rc6-mm1.orig/include/asm-x86_64/proto.h
+++ 2.6.21-rc6-mm1/include/asm-x86_64/proto.h
@@ -17,9 +17,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);


-- 

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

[Bernhard Kaindl]

Applied fix by Andew Morton:
http://lkml.org/lkml/2007/4/8/88 - Fix `make headers_check'.

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.

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

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 or
   at least a test on a >2 CPU machine.

*) 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. That would be an incremental patch,
   but of rather low priority since there is no machine known so
   far which would require this.

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

---
 arch/i386/kernel/cpu/mtrr/main.c |   11 +++++++++++
 arch/i386/kernel/smpboot.c       |    7 +++++++
 arch/x86_64/kernel/smpboot.c     |    6 ++++++
 include/asm-i386/mtrr.h          |    2 ++
 include/asm-x86_64/proto.h       |    2 ++
 5 files changed, 28 insertions(+)

Index: 2.6.21-rc6-mm1/arch/i386/kernel/smpboot.c
===================================================================
--- 2.6.21-rc6-mm1.orig/arch/i386/kernel/smpboot.c
+++ 2.6.21-rc6-mm1/arch/i386/kernel/smpboot.c
@@ -58,6 +58,7 @@
 #include <mach_wakecpu.h>
 #include <smpboot_hooks.h>
 #include <asm/vmi.h>
+#include <asm/mtrr.h>

 /* Set if we find a B stepping CPU */
 static int __devinitdata smp_b_stepping;
@@ -816,6 +817,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: 2.6.21-rc6-mm1/arch/x86_64/kernel/smpboot.c
===================================================================
--- 2.6.21-rc6-mm1.orig/arch/x86_64/kernel/smpboot.c
+++ 2.6.21-rc6-mm1/arch/x86_64/kernel/smpboot.c
@@ -943,6 +943,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: 2.6.21-rc6-mm1/include/asm-i386/mtrr.h
===================================================================
--- 2.6.21-rc6-mm1.orig/include/asm-i386/mtrr.h
+++ 2.6.21-rc6-mm1/include/asm-i386/mtrr.h
@@ -70,6 +70,7 @@ struct mtrr_gentry
 /*  The following functions are for use by other drivers  */
 # ifdef CONFIG_MTRR
 extern void mtrr_save_fixed_ranges(void *);
+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,
@@ -81,6 +82,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: 2.6.21-rc6-mm1/include/asm-x86_64/proto.h
===================================================================
--- 2.6.21-rc6-mm1.orig/include/asm-x86_64/proto.h
+++ 2.6.21-rc6-mm1/include/asm-x86_64/proto.h
@@ -18,10 +18,12 @@ 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 *);
+extern void mtrr_save_state(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)
+#define mtrr_save_state() do {} while (0)
 #endif
 extern void init_memory_mapping(unsigned long start, unsigned long end);

Index: 2.6.21-rc6-mm1/arch/i386/kernel/cpu/mtrr/main.c
===================================================================
--- 2.6.21-rc6-mm1.orig/arch/i386/kernel/cpu/mtrr/main.c
+++ 2.6.21-rc6-mm1/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] Save and restore the fixed-range MTRRs of the BSP when suspending

[Bernhard Kaindl]

Note: This patch didn'nt need an update since it's initial post.

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>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andi Kleen <ak@suse.de>
Cc: Dave Jones <davej@codemonkey.org.uk>

---
 arch/i386/power/cpu.c        |    1 +
 arch/x86_64/kernel/suspend.c |    1 +
 2 files changed, 2 insertions(+)

Index: 2.6.21-rc6-mm1/arch/i386/power/cpu.c
===================================================================
--- 2.6.21-rc6-mm1.orig/arch/i386/power/cpu.c
+++ 2.6.21-rc6-mm1/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();

 	/*
Index: 2.6.21-rc6-mm1/arch/x86_64/kernel/suspend.c
===================================================================
--- 2.6.21-rc6-mm1.orig/arch/x86_64/kernel/suspend.c
+++ 2.6.21-rc6-mm1/arch/x86_64/kernel/suspend.c
@@ -48,6 +48,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

-- 
Bernhard Kaindl

[PATCH 4/4] Enable support for fixed-range IORRs to keep RdMem & WrMem in sync

[Bernhard Kaindl]

--------------------------------------------------------------
Update: Only removed __initdata from mtrr_show in the diff context
since the initial post to make it apply to 2.6.21-rc6-mm1

Note to Andrew: The patch which you applied which removes __initdata
from mtrr_show is actually wrong because mtrr_show is really only
accessed before __initdata cleanup. The correct fix is to apply
this patch instead:

http://lkml.org/lkml/2007/4/7/113
Jeremy Fitzhardinge: Subject [PATCH 2/2] x86: clean up identify_cpu

Jeremy tested his patch on i386, but not on x86_64 because he
has no such machine, but his cleanup to split

    identify_cpu(struct cpuinfo_x86 *);

which has

    /*
     * On SMP, boot_cpu_data holds the common feature set between
     * all CPUs; so make sure that we indicate which features are
-    * common between the CPUs.  The first time this routine gets
-    * executed, c == &boot_cpu_data.
-    */
-   if (c != &boot_cpu_data) {
-       /* AND the already accumulated flags with these */
-       for (i = 0 ; i < NCAPINTS ; i++)
-           boot_cpu_data.x86_capability[i] &= c->x86_capability[i];
-   }

    -   if (c == &boot_cpu_data)
    -       mtrr_bp_init();
    -   else
    -       mtrr_ap_init();

identify_boot_cpu()
identify_secondary_cpu()

is much preferred by me, as it allows to do the __init / __initdata
markers in the mtrr code right again.

Conseqently, identify_boot_cpu() and identify_secondary_cpu() should
IMHO be renamed top init_boot_cpu() and init_secondary_cpu(), because
after Jeremy's cleanup, the CPU initialsation code is properly split
apart from the pure CPU identification code which stays in a common
identify_cpu() function, which is used by both functions, so then
the code also is much less ugly to read.

--------------------------------------------------------------
Original Patch description with patch updated to apply to 2.6.21-rc6-mm1:

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>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andi Kleen <ak@suse.de>
Cc: Dave Jones <davej@codemonkey.org.uk>

---
 arch/i386/kernel/cpu/mtrr/generic.c |   90 ++++++++++++++++++++++++++----------
 1 file changed, 65 insertions(+), 25 deletions(-)

Index: 2.6.21-rc6-mm1/arch/i386/kernel/cpu/mtrr/generic.c
===================================================================
--- 2.6.21-rc6-mm1.orig/arch/i386/kernel/cpu/mtrr/generic.c
+++ 2.6.21-rc6-mm1/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 int mtrr_show;
 module_param_named(show, mtrr_show, bool, 0);

@@ -162,6 +179,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.
@@ -211,36 +266,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 1/4] Adds mtrr_save_fixed_ranges() for use in two later patches.

[Randy Dunlap]

On Fri, 13 Apr 2007 17:50:50 +0200 (CEST) Bernhard Kaindl wrote:

> ---
>  arch/i386/kernel/cpu/mtrr/generic.c |   16 +++++++++++++++-
>  include/asm-i386/mtrr.h             |    2 ++
>  include/asm-x86_64/proto.h          |    2 ++
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> Index: 2.6.21-rc6-mm1/arch/i386/kernel/cpu/mtrr/generic.c
> ===================================================================
> --- 2.6.21-rc6-mm1.orig/arch/i386/kernel/cpu/mtrr/generic.c
> +++ 2.6.21-rc6-mm1/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 __cpuinit
> +/**
> + * Retrieves the current fixed-range MTRRs from the current CPU
> + * \param frs address where to write the current MTRR contents
> + */

Please use correct kernel-doc notation.
See Documentation/kernel-doc-nano-HOWTO.txt for details.

Briefly:  "/**" begins a kernel-doc comment block.
Next line is "function short description" (when it's a function).
Parameters are referred to as @param, not \param.

Please check the entire patch series for this.

> +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 __cpuinit print_fixed(unsigned base, unsigned step, const mtrr_type*types)
>  {
>  	unsigned i;
> Index: 2.6.21-rc6-mm1/include/asm-i386/mtrr.h
> ===================================================================
> --- 2.6.21-rc6-mm1.orig/include/asm-i386/mtrr.h
> +++ 2.6.21-rc6-mm1/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 *);

We prefer to have a variable name in the prototype (cf. below).


>  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,


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***