[coreboot] Rev 4099+ Broken Tree?

[ron minnich]

On Mon, Apr 27, 2009 at 8:25 AM, Rudolf Marek <r.marek at assembler.cz> wrote:
>
>> Are we going to revert this bit for now, or do we have a way to fix it
>> in the works?
>>
>
> It depends. I can put around #ifdef resume. But I dont think its a proper
> fix. I dont know much about ECC so I dont know what is the correct way to
> fix that.
> If all ECC tags must be cleared by memory write, the code is also wrong
> because it zero out memory only to TOPK and not to MEMTOP or MEMTOP2

It's not that the ECC tags need to be cleared. The issue is that they
must, by design, have a value that is correct for the data
stored in memory.

to recap: we don't want to zero memory on resume, but we need ECC tags
"synced up" on power-on or reset.
Setting a valid value in the tags does not require zero'ing memory. It
requires a write to memory. That is all.
When the CPU writes to memory then the tags get a known value, instead
of the semi-random value they have
on power-on.

As I said, I believe it is sufficient to make the ECC tags clean to do this:

volatile unsigned long *lp; /* or whatever declaration makes gcc do
the right thing */
for(i = 0, cp = start_of_memory; i < (memory_size_in_bytes/
sizeof(*lp); i++, lp++)
  *lp = *lp;

This will preserve memory contents. It will do the right thing on both
resume and power on reset.
This will run more slowly than the memset but it will in fact give you
clean memory tags. It is useful for recovering system state after a
reset.
You should not get ECC interrupts on the read because interrupts are disabled.

I also think the code I just showed might not be necessary. It will
slow down resume if the DRAM has valid data. Maybe what really needs
to be done is more sophisticated code which determines if the dram is
valid and need not be initialized.

I think the code needs to be backed out and a new version tested out
of the tree. Right now the code in the tree is wrong, as we can see
from the fact that it is breaking.

Thanks,

ron