2008-02-08 12:22:54

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [1/2] Use standard global TLB flushes in MTRR code v2


This is more idiomatic and it does not really make sense for this
code to implement a own TLB flushing variant.

The control registers will be read/written a few times more, but
that should not really matter for this code.

v1->v2: Remove unused variable to fix warning

Signed-off-by: Andi Kleen <[email protected]>

---
arch/x86/kernel/cpu/mtrr/generic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux/arch/x86/kernel/cpu/mtrr/generic.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mtrr/generic.c
+++ linux/arch/x86/kernel/cpu/mtrr/generic.c
@@ -324,7 +324,6 @@ static unsigned long set_mtrr_state(void
}


-static unsigned long cr4 = 0;
static DEFINE_SPINLOCK(set_atomicity_lock);

/*
@@ -349,14 +348,8 @@ static void prepare_set(void) __acquires
write_cr0(cr0);
wbinvd();

- /* Save value of CR4 and clear Page Global Enable (bit 7) */
- if ( cpu_has_pge ) {
- cr4 = read_cr4();
- write_cr4(cr4 & ~X86_CR4_PGE);
- }
-
- /* Flush all TLBs via a mov %cr3, %reg; mov %reg, %cr3 */
- __flush_tlb();
+ /* Flush all TLBs */
+ __flush_tlb_all();

/* Save MTRR state */
rdmsr(MTRRdefType_MSR, deftype_lo, deftype_hi);
@@ -368,7 +361,7 @@ static void prepare_set(void) __acquires
static void post_set(void) __releases(set_atomicity_lock)
{
/* Flush TLBs (no need to flush caches - they are disabled) */
- __flush_tlb();
+ __flush_tlb_all();

/* Intel (P6) standard MTRRs */
mtrr_wrmsr(MTRRdefType_MSR, deftype_lo, deftype_hi);
@@ -376,9 +369,9 @@ static void post_set(void) __releases(se
/* Enable caches */
write_cr0(read_cr0() & 0xbfffffff);

- /* Restore value of CR4 */
- if ( cpu_has_pge )
- write_cr4(cr4);
+ /* Flush TLBs again to handle prefetches etc. */
+ __flush_tlb_all();
+
spin_unlock(&set_atomicity_lock);
}


2008-02-08 12:23:17

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [2/2] Fix and simplify k8.c Kconfig dependencies


- Check for K8_NUMA instead of NUMA && PCI
- No need to check for x86_64 explicitely

Signed-off-by: Andi Kleen <[email protected]>

---
arch/x86/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/arch/x86/Kconfig
===================================================================
--- linux.orig/arch/x86/Kconfig
+++ linux/arch/x86/Kconfig
@@ -1540,7 +1540,7 @@ endif # X86_32

config K8_NB
def_bool y
- depends on AGP_AMD64 || (X86_64 && (GART_IOMMU || (PCI && NUMA)))
+ depends on AGP_AMD64 || GART_IOMMU || K8_NUMA

source "drivers/pcmcia/Kconfig"

2008-02-09 11:16:32

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] [1/2] Use standard global TLB flushes in MTRR code v2


* Andi Kleen <[email protected]> wrote:

> This is more idiomatic and it does not really make sense for this code
> to implement a own TLB flushing variant.
>
> The control registers will be read/written a few times more, but that
> should not really matter for this code.

what you do not point out in the changelog, and what i've mentioned to
you in past replies but you still ignore it: that the patch changes what
the code does - we now keep PGE enabled in cr4 during the MTRR changing.

the MTRR code is historically fragile, rarely triggered code, laced with
CPU errata. The change brings us absolutely nothing (it in fact
increases the code size a bit) and it is just not worth the risk at this
stage.

Ingo