<div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
So did you hit problems with CAR_GLOBAL in agesa_get_dispatcher(),<br>
when experimeting CZ vs ST? I don't see why this code works even for<br>
dual-core ST but I did not evaluate CAR layout. Documentation part for<br>
fixed MTRRs in gcccar.inc appears to be same.<br></blockquote><br>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.<br><div><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Now, does this MTRR change have potential to actually become _the_<br>
fix? I don't see why it could not. It's pretty much all that we can do<br>
with binaryPI, anything else touches the blobs.

<br></blockquote><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Will you
sent patch?

<br></blockquote><br></div><div>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.<br><br></div><div></div><div>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.<br><br></div><div>Thanks,<br></div><div>Marshall<br><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, May 5, 2018 at 4:22 AM, Piotr Król <span dir="ltr"><<a href="mailto:piotr.krol@3mdeb.com" target="_blank">piotr.krol@3mdeb.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">-----BEGIN PGP SIGNED MESSAGE-----<br>
Hash: SHA256<br>
<div><div class="h5"><br>
<br>
<br>
On 05/05/2018 11:53 AM, Kyösti Mälkki wrote:<br>
> On Sat, May 5, 2018 at 3:06 AM, Marshall Dawson <br>
> <<a href="mailto:marshalldawson3rd@gmail.com">marshalldawson3rd@gmail.com</a>> wrote:<br>
>>> My current guess is AP CPUs do not see initialised memory for <br>
>>> _car_region_start .. _end. That region is set up using fixed<br>
>>> MTRRs in low memory and probably not synced between cores so<br>
>>> early in romstage.<br>
>> <br>
>> <br>
>> Kyosti, I haven't kept a close watch on CAR changes in the other<br>
>> AMD systems, however in experimenting with CZ using ST source I<br>
>> found that 4 cores had a problem using CAR globals, so I think<br>
>> you're on the right track. AMD's CAR setup code assumes each core<br>
>> only needs fixed MTRRs configured for their own particular region<br>
>> of storage -- they all don't get access to 100% of the CAR area.<br>
>> Going from 2 cores (ST) to 4 cores (CZ), the higher two cores<br>
>> could no longer access the CAR globals at the bottom of the<br>
>> region. Try this hack and see if it improves for you:<br>
>> <br>
> <br>
> Hi Marshall<br>
> <br>
> Thanks, your suggested MTRR change seems to solve the regression.<br>
> It was definetely about CAR_GLOBALs failing for AP CPUs in general.<br>
> We just had not hit this case before for typical builds, as use of <br>
> POSTCAR_STAGE barely avoided the error from triggering.<br>
> <br>
> So did you hit problems with CAR_GLOBAL in agesa_get_dispatcher(), <br>
> when experimeting CZ vs ST? I don't see why this code works even<br>
> for dual-core ST but I did not evaluate CAR layout. Documentation<br>
> part for fixed MTRRs in gcccar.inc appears to be same.<br>
> <br>
> Now, does this MTRR change have potential to actually become _the_ <br>
> fix? I don't see why it could not. It's pretty much all that we can<br>
> do with binaryPI, anything else touches the blobs.<br>
<br>
</div></div>Hi Marshall, Kyösti,<br>
also confirm that this patch fix regression on recent master. Will you<br>
sent patch?<br>
<br>
Best Regards,<br>
- -- <br>
<span class="">Piotr Król<br>
Embedded Systems Consultant<br>
<a href="https://3mdeb.com" rel="noreferrer" target="_blank">https://3mdeb.com</a> | @3mdeb_com<br>
</span>-----BEGIN PGP SIGNATURE-----<br>
<br>
iQIzBAEBCAAdFiEE4DCbLYWmfoRjKe<wbr>NLsu5x6WeqnkwFAlrthdYACgkQsu5x<wbr>6Weq<br>
nkwqPQ//QGpX6w23CWwN/<wbr>lJQPMIB2Qpw0rdL5GTa2L1axsdL8GL<wbr>Wr+TgRkeleGi4<br>
q77x+<wbr>jG9xPP5fBkoEiZfBOkZTwhfKIKygtS<wbr>dwHybs0Qn7aammItCgdOyz1tJj9Gu<br>
/i6ncXK3dsWYwOj1r+<wbr>qkMm0phGQDgq+<wbr>uW17CnsmOUfXMIMm6ULIF6f/8t/<wbr>aNvdGC<br>
N6gNPIjI7oDlkI73CRZjLV+<wbr>bWIsFzfvoJWTZrilNz41zxI5ERCKpm<wbr>GCNy93qPsMl<br>
p5GoGZvzE2rwniRqju8oDlw2P+<wbr>o3OZ3ukAJWbQxid83dn7NHAo95ZFZ8<wbr>cx7umLvN<br>
J+<wbr>vFrD9n1CzwQRXtqWoSelYmGJkJWxx/<wbr>p5Co8JK+<wbr>x2yLDShrojJraV02NX32k4Yd<br>
Bf4RZW9nK4QnPqrH647FfBEOBZfX0r<wbr>uizjTQzFLd7moTYDry8zIOwOuLc3If<wbr>tT09<br>
edPrxnwlp6q6F8EopGXsnTbqFfbxGR<wbr>RVzGICvd2/<wbr>eFtSXZBULFQBfutKYrK8KVaS<br>
/<wbr>VLXSpPQ6f07vZnWQMrCndaHIC0cShn<wbr>HE1ekb2uY8kmkqzgXZetaKGsDkCXlJ<wbr>VRF<br>
vlJEoEu9qWmglqMYzxPF8MnHBx9DLA<wbr>gFk/<wbr>AMMmQk5yzv8Ki1m3kgJu9Fxdc0iJ3y<br>
Ka9voYxzP2r+<wbr>K8CaJgvtXYwSeHoIqDdERBAz0lN3KT<wbr>qGuXtewSs=<br>
=0Gj7<br>
-----END PGP SIGNATURE-----<br>
<div class="HOEnZb"><div class="h5"><br>
-- <br>
coreboot mailing list: <a href="mailto:coreboot@coreboot.org">coreboot@coreboot.org</a><br>
<a href="https://mail.coreboot.org/mailman/listinfo/coreboot" rel="noreferrer" target="_blank">https://mail.coreboot.org/<wbr>mailman/listinfo/coreboot</a></div></div></blockquote></div><br></div>