[coreboot-gerrit] [L] Change in coreboot[master]: util/amdfwtool: Deal with psp position in flash offset directly

[Martin Roth (Code Review)]

Attention is currently required from: Bao Zheng, Jason Glenesk, Raul Rangel, Jason Nien, Matt DeVillier, Arthur Heymans, Fred Reitberger, Karthik Ramasubramanian, Felix Held.

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69852 )

Change subject: util/amdfwtool: Deal with psp position in flash offset directly
......................................................................

Patch Set 4:

(1 comment)

File src/soc/amd/common/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/69852/comment/92e80956_7931813e 
PS4, Line 25: $(call int-subtract, 0xffffffff $(CONFIG_ROM_SIZE)) 1 $(CONFIG_AMD_FWM_POSITION))
> > would it make sense to add flash offset support to cbfstool? It already knows the flash size from  […]

I agree with what Karthik's saying here.  The values that are listed all assume offsets from the bottom of a 16MiB ROM space.  When changing that to starting at the bottom of an 8MiB ROM, the 0xC20000 needs to become 0x420000.  I don't think we want that.

So either you need to subtract the rom size in two different locations,
Once as you are to adjust the start of the rom area

and again subtracting (16Mib-ROM size) to correctly adjust the location selected.

Since we have a set 16MiB ROM area now, I think we can just keep the actual ROM size out of the equation (Though maybe we want to check that the selected space is inside the ROM).

For upcoming chips, my understanding is that this is all going to go away, and the 16MiB ROM (or larger) becomes mandatory, and the only supported offset is 0x020000.