Patchwork fix unexpacted MTRR setup for UMA memory

-----Original Message-----
From: coreboot-bounces@coreboot.org [mailto:coreboot-bounces@coreboot.org] On Behalf Of Carl-Daniel Hailfinger
Sent: Thursday, October 21, 2010 09:15 PM
To: Scott Duplichan
Cc: coreboot@coreboot.org
Subject: Re: [coreboot] [patch] fix unexpacted MTRR setup for UMA memory

]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
]
]-- 
]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?
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);
 	}

On 22.10.2010, at 14:01, "Scott Duplichan" <scott@notabs.org> wrote:
> 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?
> 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);
>        }

I think the problem you are seeing might be that on some chipsets the memory resource still contains the UMA part of that memory while on others it just contains the usable memory. That would explain why removing that particular line from the code would solve the problem for you.

Not sure what the right solution would be but we should make sure that it's consistent across chipsets..

Stefan

Hi,

I just want to tell "me too", so question is we go to direction of claiming less 
ram_resource as intel do or others.

Is the AMD uma limited to < 4GB?

Thanks,
Rudolf

-----Original Message-----
From: coreboot-bounces@coreboot.org [mailto:coreboot-bounces@coreboot.org] On Behalf Of Rudolf Marek
Sent: Wednesday, November 03, 2010 06:07 PM
To: coreboot@coreboot.org
Subject: Re: [coreboot] [patch] fix unexpacted MTRR setup for UMA memory

]Hi,
]
]I just want to tell "me too", so question is we go to direction of claiming less 
]ram_resource as intel do or others.

So far there has been no comments about non-AMD UMA systems. This is one of several
items that need to be resolved before the trunk can support Win7 for AMS UMA systems.

]Is the AMD uma limited to < 4GB?

The RS780 frame buffer can be placed above 4GB. I remember booting a
Tilapia board a while back and finding the frame buffer is above 4GB
by default. A setup option can be used to select above or below.


]Thanks,
]Rudolf