On Sat, May 5, 2018 at 6:27 PM, Marshall Dawson <marshalldawson...@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 [1] https://review.coreboot.org/#/c/coreboot/+/26115 -- coreboot mailing list: coreboot@coreboot.org https://mail.coreboot.org/mailman/listinfo/coreboot