[coreboot-gerrit] Change in coreboot[master]: cpu/x86/mtrr: add support for base address bigger than 4GB

[Jonathan Zhang (Code Review)]

Hello Marc Jones,

I'd like you to do a code review. Please visit

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

to review the following change.

Change subject: cpu/x86/mtrr: add support for base address bigger than 4GB
......................................................................

cpu/x86/mtrr: add support for base address bigger than 4GB

On DeltaLake server, there are following entry in MTRR address space:
0x0000201000000000 - 0x0000201000400000 size 0x00400000 type 0

The base address (after 4k page size adjustment) cannot be held in
uint32_t. This results incorrect MTRR register setup. As the consequence
UEFI forum FWTS reports following critical error:
Memory range 0x100000000 to 0x183fffffff (System RAM) has incorrect attribute Uncached.

Change appropriate variables' data type from uint32_t to uint64_t.

Add fls64() to find least significant bit set in a 64-bit word. Right
now a stop-gap method is used. The 64-bit assembly command causes
compilation error.

Signed-off-by: Jonathan Zhang 
Signed-off-by: Marc Jones 
Change-Id: I41bc5befcc1374c838c91b9f7c5279ea76dd67c7
---
M src/cpu/x86/mtrr/mtrr.c
M src/include/cpu/x86/mtrr.h
2 files changed, 25 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/46435/1

diff --git a/src/cpu/x86/mtrr/mtrr.c b/src/cpu/x86/mtrr/mtrr.c
index 77525a7..27384cc 100644
--- a/src/cpu/x86/mtrr/mtrr.c
+++ b/src/cpu/x86/mtrr/mtrr.c
@@ -121,12 +121,12 @@
 #define MTRR_ALGO_SHIFT (8)
 #define MTRR_TAG_MASK ((1 << MTRR_ALGO_SHIFT) - 1)

-static inline uint32_t range_entry_base_mtrr_addr(struct range_entry *r)
+static inline uint64_t range_entry_base_mtrr_addr(struct range_entry *r)
 {
    return PHYS_TO_RANGE_ADDR(range_entry_base(r));
 }

-static inline uint32_t range_entry_end_mtrr_addr(struct range_entry *r)
+static inline uint64_t range_entry_end_mtrr_addr(struct range_entry *r)
 {
    return PHYS_TO_RANGE_ADDR(range_entry_end(r));
 }
@@ -402,7 +402,7 @@
 }

static void prep_var_mtrr(struct var_mtrr_state *var_state,
-			  uint32_t base, uint32_t size, int mtrr_type)
+			  uint64_t base, uint64_t size, int mtrr_type)
 {
    struct var_mtrr_regs *regs;
    resource_t rbase;
@@ -444,15 +444,30 @@
    regs->mask.hi = rsize >> 32;
 }

+/* fls64: find least significant bit set in a 64-bit word*/
+static uint32_t fls64(uint64_t x)
+{
+	if (x == 0) return 0;
+	uint32_t pos = 0;
+	while (!(x & 1)) {
+		x >>= 1;
+		++pos;
+	}
+	return pos;
+}
+
 static void calc_var_mtrr_range(struct var_mtrr_state *var_state,
-				uint32_t base, uint32_t size, int mtrr_type)
+				uint64_t base, uint64_t size, int mtrr_type)
 {
    while (size != 0) {
    	uint32_t addr_lsb;
    	uint32_t size_msb;
-		uint32_t mtrr_size;
+		uint64_t mtrr_size;

-		addr_lsb = fls(base);
+		if (base > 0x100000000)
+			addr_lsb = fls64(base);
+		else
+			addr_lsb = fls(base);
    	size_msb = fms(size);

/* All MTRR entries need to have their base aligned to the mask
@@ -531,7 +546,7 @@
 static void calc_var_mtrrs_with_hole(struct var_mtrr_state *var_state,
    			     struct range_entry *r)
 {
-	uint32_t a1, a2, b1, b2;
+	uint64_t a1, a2, b1, b2;
    int mtrr_type, carve_hole;

/*
@@ -671,6 +686,8 @@
    		wb_deftype_count += var_state.mtrr_index;
    	}
    }
+	printk(BIOS_DEBUG, "%s %d, uc_deftype_count: %d, wb_deftype_count: %d\n", __func__, __LINE__, uc_deftype_count, wb_deftype_count);
+
    *num_def_wb_mtrrs = wb_deftype_count;
    *num_def_uc_mtrrs = uc_deftype_count;
 }
diff --git a/src/include/cpu/x86/mtrr.h b/src/include/cpu/x86/mtrr.h
index 3bf8301c..1497387 100644
--- a/src/include/cpu/x86/mtrr.h
+++ b/src/include/cpu/x86/mtrr.h
@@ -173,6 +173,7 @@
    	"1:" : "=r" (r) : "mr" (x));
    return r;
 }
+
 #endif /* !defined(__ASSEMBLER__) */

/* Align up/down to next power of 2, suitable for assembler