Change in coreboot[master]: src/vendorcode/amd : Zork, MI: Bug in AGESA FabricResourceManagerUnbL...

[Marshall Dawson (Code Review)]

Attention is currently required from: Bao Zheng, Jason Glenesk, Jason Nein, Matt Papageorge, Felix Held.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58803 )

Change subject: src/vendorcode/amd : Zork, MI: Bug in AGESA FabricResourceManagerUnbLib.c
......................................................................

Patch Set 1:

(5 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/58803/comment/8ac1b980_76f35ee9 
PS1, Line 7: src/vendorcode/amd : Zork, MI: Bug in AGESA FabricResourceManagerUnbLib.c
coreboot has a couple of references for how to write a good commit message although none are really excellent.  But you can make sure you follow https://www.coreboot.org/Git#Commit_messages or https://doc.coreboot.org/tutorial/part2.html#step-4a-use-the-command-line-to....

Specifically, you'll often be asked to shorten the subject line:
* the subject needs to give some hint of what you're doing
* no need to use src in the path
* you can substitute vc for vendorcode
* no space before a colon
* this patch doesn't apply to Zork nor to MI
* I often feel there's no reason to identify the filename as it's already pretty evident

So the subject line might be "vc/amd/agesa: fix out of bounds read".

https://review.coreboot.org/c/coreboot/+/58803/comment/744a0d42_2f83f9bd 
PS1, Line 9: Fixing coverity issues: Out-of-bounds read
Make sure you use complete sentences and that your verbs are imperative, e.g. "Fix out-of-bounds read issue found by coverity".  And it doesn't hurt to put the coverity error into the commit message too.

235               MmioRange[MmioPair].Modified = TRUE;
    >>>     CID 1376956:  Memory - illegal accesses  (OVERRUN)
    >>>     Overrunning array "MmioRange" of 12 20-byte elements at element index 12 (byte offset 240) using index "MmioPair + i" (which evaluates to 12).
    236               for (i = 1; NewMmioRange.Limit >= MmioRange[MmioPair + i].Base; i++) {

https://review.coreboot.org/c/coreboot/+/58803/comment/1b23749e_e465fae4 
PS1, Line 11: BUG=b:147411796
This doesn't really apply for Trinity and Kabini changes.  You could omit it.

https://review.coreboot.org/c/coreboot/+/58803/comment/22b68957_16e38f99 
PS1, Line 12: Build pass and run the normal boot on Guybrush
Guybrush doesn't use this code.  Since you don't have HW to test with, and Jenkins has built it, I'd be fine with simply saying TEST=none.

File src/vendorcode/amd/agesa/f15tn/Proc/CPU/Family/0x15/cpuF15MmioMap.c:

https://review.coreboot.org/c/coreboot/+/58803/comment/9ead3bfc_6fc213d9 
PS1, Line 232: 12
Is there a defined value somewhere that can be used instead of 12?