[coreboot] [PATCH] Avoid hang when 4GB or more DRAM is installed on AMD RS780 UMA systems

Patrick Georgi patrick at georgi-clan.de
Wed Nov 10 19:01:04 CET 2010


Am 10.11.2010 18:43, schrieb Scott Duplichan:
> Another possibility is to go back to the idea of one x86_setup_var_mtrrs
> function. A new flag could be passed to x86_setup_var_mtrrs that tells
> tells the function that it ius running on an AMD system.
In that case, you could as well keep the separate AMD function around.
My main "issue" was that there's vendor specific code in a (supposedly)
generic file, esp. when there's already an amd_mtrr.c file.
But I guess it doesn't matter that much, and when you're using static
variables of mtrr.c, then it might actually be the right place.

> It is a little frustrating to have to worry about the possibility of
> a code change breaking non-AMD UMA operation when I have no way to 
> test it. For example, I suspect the UMA logic for > 4GB systems is
> not working for all systems. While I can confirm this for AMD systems,
> I cannot confirm it for non-AMD systems. So I assume I must use one
> method or another to preserve the existing logic for non-AMD use, just
> in case it really does work for UMA systems with more than 4GB DRAM.
What's the highlevel algorithm there?
>From what I understand, the following happens - please correct me here:
You look up the resources like the "generic" mtrr code, and then there
are two differences to the generic code path:
1. (4gb-tom2 .. 4gb) is left uncached (because of Tom2ForceMemTypeWB)
2. UMA is put at (4gb-umasize .. 4gb) and left uncached

Maybe we could do that with a weak function:
First, the generic code in x86_setup_var_mtrrs determines the set of
memory regions, then calls a weak function (if implemented), and only
then makes the call to enable_var_mtrr.

The AMD code would then provide that function and handle the special
cases (eg. UMA and that TOM2 space). It wouldn't require those other
functions as it's given the var_state struct to work on.

On i945, UMA is done by providing a fixed resource. I don't think any
other changes were necessary (see src/northbridge/intel/i945/northbridge.c)

As for the TOM2 stuff: Do you _have_ to mark it uncached? Depending on
the situation, it might be less work to just mark it WB in the MTRRs (if
that provides a full power-of-two region).
But that's a potential optimization to look into separately.


Patrick




More information about the coreboot mailing list