Change in coreboot[master]: nb/intel/nehalem: Use common clflush function

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37195 )

Change subject: nb/intel/nehalem: Use common clflush function
......................................................................

nb/intel/nehalem: Use common clflush function

Change-Id: I0ca311c12f013e54e23ff0427421bfad0b747ea6
Signed-off-by: Arthur Heymans 
---
M src/northbridge/intel/nehalem/raminit.c
1 file changed, 1 insertion(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/37195/1

diff --git a/src/northbridge/intel/nehalem/raminit.c b/src/northbridge/intel/nehalem/raminit.c
index a393cb7..fe5d5c9 100644
--- a/src/northbridge/intel/nehalem/raminit.c
+++ b/src/northbridge/intel/nehalem/raminit.c
@@ -21,6 +21,7 @@
 #include <device/mmio.h>
 #include <device/pci_ops.h>
 #include <cpu/x86/msr.h>
+#include <cpu/x86/cache.h>
 #include <cbmem.h>
 #include <cf9_reset.h>
 #include <arch/cbfs.h>
@@ -97,12 +98,6 @@

#include <lib.h>		/* Prototypes */

-
-static void clflush(u32 addr)
-{
-	asm volatile ("clflush (%0)"::"r" (addr));
-}
-
 typedef struct _u128 {
    u64 lo;
    u64 hi;

[Arthur Heymans (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37195

to look at the new patch set (#2).

Change subject: nb/intel/nehalem: Use common clflush function
......................................................................

nb/intel/nehalem: Use common clflush function

Change-Id: I0ca311c12f013e54e23ff0427421bfad0b747ea6
Signed-off-by: Arthur Heymans 
---
M src/northbridge/intel/nehalem/raminit.c
1 file changed, 2 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/37195/2

Change in coreboot[master]: nb/intel/nehalem: Use cache.h functions

[Arthur Heymans (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37195

to look at the new patch set (#3).

Change subject: nb/intel/nehalem: Use cache.h functions
......................................................................

nb/intel/nehalem: Use cache.h functions

Some local functions need renaming to avoid name collision.

Change-Id: I0ca311c12f013e54e23ff0427421bfad0b747ea6
Signed-off-by: Arthur Heymans 
---
M src/northbridge/intel/nehalem/raminit.c
1 file changed, 8 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/37195/3

Change in coreboot[master]: nb/intel/nehalem: Use cache.h functions

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37195 )

Change subject: nb/intel/nehalem: Use cache.h functions
......................................................................

Patch Set 4: Code-Review+2

Change in coreboot[master]: nb/intel/nehalem: Use cache.h functions

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37195 )

Change subject: nb/intel/nehalem: Use cache.h functions
......................................................................

nb/intel/nehalem: Use cache.h functions

Some local functions need renaming to avoid name collision.

Change-Id: I0ca311c12f013e54e23ff0427421bfad0b747ea6
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37195
Reviewed-by: Patrick Georgi 
Tested-by: build bot (Jenkins) 
---
M src/northbridge/intel/nehalem/raminit.c
1 file changed, 8 insertions(+), 13 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved

diff --git a/src/northbridge/intel/nehalem/raminit.c b/src/northbridge/intel/nehalem/raminit.c
index 7735522..de02882 100644
--- a/src/northbridge/intel/nehalem/raminit.c
+++ b/src/northbridge/intel/nehalem/raminit.c
@@ -21,6 +21,7 @@
 #include <device/mmio.h>
 #include <device/pci_ops.h>
 #include <cpu/x86/msr.h>
+#include <cpu/x86/cache.h>
 #include <cbmem.h>
 #include <cf9_reset.h>
 #include <ip_checksum.h>
@@ -96,12 +97,6 @@

#include <lib.h>		/* Prototypes */

-
-static void clflush(u32 addr)
-{
-	asm volatile ("clflush (%0)"::"r" (addr));
-}
-
 typedef struct _u128 {
    u64 lo;
    u64 hi;
@@ -1956,7 +1951,7 @@
    return ret;
 }

-static void disable_cache(void)
+static void disable_cache_region(void)
 {
    msr_t msr = {.lo = 0, .hi = 0 };

@@ -1964,7 +1959,7 @@
    wrmsr(MTRR_PHYS_MASK(3), msr);
 }

-static void enable_cache(unsigned int base, unsigned int size)
+static void enable_cache_region(unsigned int base, unsigned int size)
 {
    msr_t msr;
    msr.lo = base | MTRR_TYPE_WRPROT;
@@ -1983,7 +1978,7 @@

end = start + (ALIGN_DOWN(size + 4096, 4096));
    for (addr = start; addr < end; addr += 64)
-		clflush(addr);
+		clflush((void *)addr);
 }

static void clear_errors(void)
@@ -2019,7 +2014,7 @@
    int comp1, comp2, comp3;
    u32 failxor[2] = { 0, 0 };

-	enable_cache((total_rank << 28), 1728 * 5 * 4);
+	enable_cache_region((total_rank << 28), 1728 * 5 * 4);

for (comp3 = 0; comp3 < 9 && failmask != 0xff; comp3++) {
    	for (comp1 = 0; comp1 < 4; comp1++)
@@ -2042,7 +2037,7 @@
    		if ((0xff << (8 * (i % 4))) & failxor[i / 4])
    			failmask |= 1 << i;
    }
-	disable_cache();
+	disable_cache_region();
    flush_cache((total_rank << 28), 1728 * 5 * 4);
    return failmask;
 }
@@ -2132,7 +2127,7 @@
    failxor[0] = 0;
    failxor[1] = 0;

-	enable_cache(totalrank << 28, 134217728);
+	enable_cache_region(totalrank << 28, 134217728);
    for (comp3 = 0; comp3 < 2 && failmask != 0xff; comp3++) {
    	for (comp1 = 0; comp1 < 16; comp1++)
    		for (comp2 = 0; comp2 < 64; comp2++) {
@@ -2148,7 +2143,7 @@
    		if ((0xff << (8 * (i % 4))) & failxor[i / 4])
    			failmask |= 1 << i;
    }
-	disable_cache();
+	disable_cache_region();
    flush_cache((totalrank << 28) | (region << 25) | (block << 16), 16384);
    return failmask;
 }

Change in coreboot[master]: nb/intel/nehalem: Use cache.h functions

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37195 )

Change subject: nb/intel/nehalem: Use cache.h functions
......................................................................

Patch Set 5:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3
Emulation targets:
EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/1102
EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1101
EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1100

Please note: This test is under development and might not be accurate at all!