Patchwork fix unexpacted MTRR setup for UMA memory

Hello,

I find an unexpected MTRR setup for my RS780/SB700 UMA board. With
a single 1GB DIMM and 256MB UMA size, the variable MTRRs look like this:

-----coreboot+seabios-----
default memory type: UC
variable range 0: 000000000-03FFFFFFF WB
variable range 1: 040000000-04FFFFFFF WB
variable range 2: 030000000-03FFFFFFF UC
variable range 3: disabled
variable range 4: disabled
variable range 5: disabled
variable range 6: disabled
variable range 7: disabled

Variable range 1 is unexpected. It creates a window of WB-IO, which is
highly unusual. 


-----Production BIOS (AMI):-----
default memory type: UC
variable range 0: 000000000-01FFFFFFF WB
variable range 1: 020000000-02FFFFFFF WB
variable range 2: disabled
variable range 3: disabled
variable range 4: disabled
variable range 5: disabled
variable range 6: disabled
variable range 7: disabled

The above is how uma is normally handled. Build up WB ranges until
the needed size is reached.

-----[patched] coreboot+seabios-----
default memory type: UC
variable range 0: 000000000-01FFFFFFF WB
variable range 1: 020000000-02FFFFFFF WB
variable range 2: disabled
variable range 3: disabled
variable range 4: disabled
variable range 5: disabled
variable range 6: disabled
variable range 7: disabled

With the patch, coreboot matches the production BIOS.


Signed-off-by: Scott Duplichan <scott@notabs.org>
Index: src/cpu/x86/mtrr/mtrr.c
===================================================================
--- src/cpu/x86/mtrr/mtrr.c	(revision 5978)
+++ src/cpu/x86/mtrr/mtrr.c	(working copy)
@@ -423,11 +423,8 @@
 	if (var_state.hole_startk || var_state.hole_sizek) {
 		printk(BIOS_DEBUG, "Warning: Can't set up MTRR hole for UMA due to pre-existing MTRR hole.\n");
 	} else {
-		// Increase the base range and set up UMA as an UC hole instead
-		var_state.range_sizek += (uma_memory_size >> 10);
-
-		var_state.hole_startk = (uma_memory_base >> 10);
-		var_state.hole_sizek = (uma_memory_size >> 10);
+		// Reduce the dram wb mtrr range so that it does not cover the uma at the end
+      var_state.range_sizek -= (uma_memory_size >> 10);
 	}
 #endif
 	/* Write the last range */

Hi,

On 22.10.2010 03:34, Scott Duplichan wrote:
> I find an unexpected MTRR setup for my RS780/SB700 UMA board. With
> a single 1GB DIMM and 256MB UMA size, the variable MTRRs look like this:
>
> -----coreboot+seabios-----
> default memory type: UC
> variable range 0: 000000000-03FFFFFFF WB
> variable range 1: 040000000-04FFFFFFF WB
> variable range 2: 030000000-03FFFFFFF UC
> variable range 3: disabled
> variable range 4: disabled
> variable range 5: disabled
> variable range 6: disabled
> variable range 7: disabled
>
> Variable range 1 is unexpected. It creates a window of WB-IO, which is
> highly unusual. 
>   

AFAIK if two ranges overlap and one of the ranges is UC, the overlapping
region will be UC. At least that's what the specs say.


> -----Production BIOS (AMI):-----
> default memory type: UC
> variable range 0: 000000000-01FFFFFFF WB
> variable range 1: 020000000-02FFFFFFF WB
> variable range 2: disabled
> variable range 3: disabled
> variable range 4: disabled
> variable range 5: disabled
> variable range 6: disabled
> variable range 7: disabled
>
> The above is how uma is normally handled. Build up WB ranges until
> the needed size is reached.
>   

For 1 GB RAM and 256 MB UMA, you have to mark 768 MB as WB. That's OK
either way (adding up WB ranges or using a UC range to carve a hole).

However, once you have 2 GB RAM and 64 MB UMA, you can do it with two
ranges:
00000000-7FFFFFFF WB
7C000000-7FFFFFFF UC
or with five ranges:
00000000-3FFFFFFF WB
40000000-5FFFFFFF WB
60000000-6FFFFFFF WB
70000000-77FFFFFF WB
78000000-7BFFFFFF WB

In the past we had to deal with range exhaustion, and that's the reason
why we carve a hole for UMA. It gets especially funny if you have more
than 4 GB RAM because then you want a hole for PCI devices below 4 GB
and you need additional ranges above 4 GB.

If switching the MTRR allocation is strictly needed for Windows (and
there is no way to avoid it), thats pretty much the only reason we
should go away from carving holes with UC.

Regards,
Carl-Daniel

]Hi,
]
]On 22.10.2010 03:34, Scott Duplichan wrote:
]> I find an unexpected MTRR setup for my RS780/SB700 UMA board. With
]> a single 1GB DIMM and 256MB UMA size, the variable MTRRs look like this:
]>
]> -----coreboot+seabios-----
]> default memory type: UC
]> variable range 0: 000000000-03FFFFFFF WB
]> variable range 1: 040000000-04FFFFFFF WB
]> variable range 2: 030000000-03FFFFFFF UC
]> variable range 3: disabled
]> variable range 4: disabled
]> variable range 5: disabled
]> variable range 6: disabled
]> variable range 7: disabled
]>
]> Variable range 1 is unexpected. It creates a window of WB-IO, which is
]> highly unusual. 
]>   
]
]AFAIK if two ranges overlap and one of the ranges is UC, the overlapping
]region will be UC. At least that's what the specs say.

Hello Carl-Daniel, yes you are correct about this rule of overlap. It
is well documented and reliable in my experience.


]> -----Production BIOS (AMI):-----
]> default memory type: UC
]> variable range 0: 000000000-01FFFFFFF WB
]> variable range 1: 020000000-02FFFFFFF WB
]> variable range 2: disabled
]> variable range 3: disabled
]> variable range 4: disabled
]> variable range 5: disabled
]> variable range 6: disabled
]> variable range 7: disabled
]>
]> The above is how uma is normally handled. Build up WB ranges until
]> the needed size is reached.
]>   
]
]For 1 GB RAM and 256 MB UMA, you have to mark 768 MB as WB. That's OK
]either way (adding up WB ranges or using a UC range to carve a hole).
]
]However, once you have 2 GB RAM and 64 MB UMA, you can do it with two
]ranges:
]00000000-7FFFFFFF WB
]7C000000-7FFFFFFF UC
]or with five ranges:
]00000000-3FFFFFFF WB
]40000000-5FFFFFFF WB
]60000000-6FFFFFFF WB
]70000000-77FFFFFF WB
]78000000-7BFFFFFF WB

One solution to this MTRR exhaustion problem you describe is to just
drop the last few ranges. Reduce the E820 entry to match. In this
example, you could drop the last two ranges and lose 'only' 192MB
or 9% of your memory. Or drop a single range and lose only 64MB (3%).
The real question is how many free MTRRs do we need? Only very old
operating systems need MTRRs for their own use (or new operating
systems running on very old processors).


]In the past we had to deal with range exhaustion, and that's the reason
]why we carve a hole for UMA.

What are the pros and cons of stacking vs carving? In the general case,
I am told stacking WB ranges is preferred but I can't remember the 
justification. For me, carving is fine until proven otherwise. But to
do carving without the unwanted WB-IO range, I need to make a new patch. 

]It gets especially funny if you have more
]than 4 GB RAM because then you want a hole for PCI devices below 4 GB
]and you need additional ranges above 4 GB.

AMD processors have a nice feature: setting bit 22 (Tom2ForceMemTypeWB)
of msr c0010010 (sys_cfg) causes all memory above 4GB to be WB.
No variable MTRR ranges are needed for DRAM memory above 4GB on AMD
systems. Surely Intel has something like this by now?


]If switching the MTRR allocation is strictly needed for Windows (and
]there is no way to avoid it), thats pretty much the only reason we
]should go away from carving holes with UC.

The only real concern at this time is that an OS could map MMIO to
the unexpected WB-IO range. Maybe we should just keep the carving
method in place but remove the unexpected WB-IO range.


]Regards,
]Carl-Daniel
]
]-- 
]http://www.hailfinger.org/

]A patch that gets rid of the unexpected variable MTRR range while
]still using the carve out method of making UMA DRAM UC is easy enough
](see below).
]
]What are the chances that this patch fixes the AMD family 10h systems
]but breaks others? Is there someone here with a non-AMD UMA system?

OK, so in theory there might be a non-AMD UMA project that doesn't
suffer from this incorrect variable MTRR range problem. But no one
has a non-AMD UMA system that can be used to confirm this. The danger
is that fixing the AMD problem the easy way (below) will eventually be
found to break one or more non-AMD systems. Does anyone have a suggested
solution? All I can think of is to make a private AMD copy of the variable
MTRR code, just like we have with the fixed MTRR code. While sharing code
between AMD and non-AMD projects seems efficient on the surface, lack of
non-AMD systems for testing makes it impractical in some cases.

Thanks,
Scott

]If so, it would be useful if you could load it with _less_ than 4GB
]of memory and then dump the variable MTRRs. If it also has the
]unexpected MTRR range, then patching mtrr.c is probably the right 
]thing to do.
]
]Thanks,
]Scott
]
]Signed-off-by: Scott Duplichan <scott@notabs.org>
]
]Index: src/cpu/x86/mtrr/mtrr.c
]===================================================================
]--- src/cpu/x86/mtrr/mtrr.c     (revision 5978)
]+++ src/cpu/x86/mtrr/mtrr.c     (working copy)
]@@ -423,9 +423,7 @@
]        if (var_state.hole_startk || var_state.hole_sizek) {
]                printk(BIOS_DEBUG, "Warning: Can't set up MTRR hole for UMA due to pre-existing MTRR ]hole.\n");
]        } else {
]-               // Increase the base range and set up UMA as an UC hole instead
]-               var_state.range_sizek += (uma_memory_size >> 10);
]-
]+               // Set up UMA as an UC hole
]                var_state.hole_startk = (uma_memory_base >> 10);
]                var_state.hole_sizek = (uma_memory_size >> 10);
]        }
]