On Sat, May 5, 2018 at 6:27 PM, Marshall Dawson marshalldawson3rd@gmail.com wrote:
So did you hit problems with CAR_GLOBAL in agesa_get_dispatcher(), when experimeting CZ vs ST? I don't see why this code works even for dual-core ST but I did not evaluate CAR layout. Documentation part for fixed MTRRs in gcccar.inc appears to be same.
I don't remember specifically where the condition caused a problem. It kind of seems like it was when the third core tried to run printk, wanted a global pointer to a global variable, and got back 0xffffffff. It's been too long to say for certain. IIRC, cores 0 & 1 received fixed MTRR settings allowing 0x30000-0x3ffff, but 2 & 3 started at 0x40000 and couldn't access the globals just above 0x30000. It's worth noting that cores in Family 15h share their MTRRs within a compute unit, so it was never possible for 0 & 1 to be anything but identical.
Every discovered core adds WB regions to MSR. So if you say the tested CZ had separate compute units, while ST only had one, that would indeed explain it.
Now, does this MTRR change have potential to actually become _the_ fix? I don't see why it could not. It's pretty much all that we1 can do with binaryPI, anything else touches the blobs.
Will you sent patch?
Yes.
I don't think _the_ fix has been pursued yet. It's certainly lower priority than other things I have going on right now. But in my mind, it's still a little too hacky. It assumes the hardcoded value of BSP_STACK_BASE_ADDR stays consistent (which needs to also equal DCACHE_RAM_BASE). Maybe a better approach is to allow all cores to have access to 100% of the CAR region. Maybe just their own plus the globals area? I haven't given much thought to why cores received differing regions in AMD's CAR setup code. What I would really enjoy seeing is moving CAR above 1MB and skipping all the fixed MTRR setup, but that's a pretty big rewrite.
Keep in mind, this code ends up in your RO bootblock. Give it some high priority on StoneyRidge buglist.
Kyosti, do you want to submit a patch since you're able to test it on other platforms? I'd be OK with having the 64K region at BSP_STACK_BASE_ADDR being WB for all cores until a more robust solution is available. I don't think this is a characteristic of PI blob implementations, by the way. I would expect all the native AGESA platforms to have the same limitation too.
That's how I have the patch [1] prepared.
Kyösti