Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34574 )
Change subject: util/amdfwtool: Correct fletcher32 algorithm ......................................................................
util/amdfwtool: Correct fletcher32 algorithm
Change the fletcher32 checksum calculation to match PSP and AGESA implementations.
The symptom of the failure has only been noted in Picasso's BIOS Directory Table, when a BIOS binary image of different sizes were passed to amdfwtool. The PSP halts the boot process with the bad BDT checksum, and if allowed to continue, AGESA asserts later due to a failed BDT verification.
This version has been verified to produce the same result as found at https://en.wikipedia.org/wiki/Fletcher%27s_checksum.
TEST=Build apu2, bettong, grunt and verify before/after amdfw.rom is unchanged.
Change-Id: I2ba2c49a70aa81c15acaab0be6b4c95e7891234f Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M util/amdfwtool/amdfwtool.c 1 file changed, 7 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/34574/1
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 1ecb7aa..4c5b216 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -148,17 +148,15 @@ c0 = 0xFFFF; c1 = 0xFFFF;
- for (index = 0; index < length; index++) { - /* - * Ignore the contents of the checksum field. - */ + while (length) { + index = length >= 359 ? 359 : length; + length -= index; + do { c0 += *(pptr++); c1 += c0; - if ((index % 360) == 0) { - /* Sums[0,1] mod 64K + overflow */ - c0 = (c0 & 0xFFFF) + (c0 >> 16); - c1 = (c1 & 0xFFFF) + (c1 >> 16); - } + } while (--index); + c0 = (c0 & 0xFFFF) + (c0 >> 16); + c1 = (c1 & 0xFFFF) + (c1 >> 16); }
/* Sums[0,1] mod 64K + overflow */
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34574 )
Change subject: util/amdfwtool: Correct fletcher32 algorithm ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34574 )
Change subject: util/amdfwtool: Correct fletcher32 algorithm ......................................................................
util/amdfwtool: Correct fletcher32 algorithm
Change the fletcher32 checksum calculation to match PSP and AGESA implementations.
The symptom of the failure has only been noted in Picasso's BIOS Directory Table, when a BIOS binary image of different sizes were passed to amdfwtool. The PSP halts the boot process with the bad BDT checksum, and if allowed to continue, AGESA asserts later due to a failed BDT verification.
This version has been verified to produce the same result as found at https://en.wikipedia.org/wiki/Fletcher%27s_checksum.
TEST=Build apu2, bettong, grunt and verify before/after amdfw.rom is unchanged.
Change-Id: I2ba2c49a70aa81c15acaab0be6b4c95e7891234f Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34574 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Martin Roth martinroth@google.com --- M util/amdfwtool/amdfwtool.c 1 file changed, 7 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 1ecb7aa..4c5b216 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -148,17 +148,15 @@ c0 = 0xFFFF; c1 = 0xFFFF;
- for (index = 0; index < length; index++) { - /* - * Ignore the contents of the checksum field. - */ + while (length) { + index = length >= 359 ? 359 : length; + length -= index; + do { c0 += *(pptr++); c1 += c0; - if ((index % 360) == 0) { - /* Sums[0,1] mod 64K + overflow */ - c0 = (c0 & 0xFFFF) + (c0 >> 16); - c1 = (c1 & 0xFFFF) + (c1 >> 16); - } + } while (--index); + c0 = (c0 & 0xFFFF) + (c0 >> 16); + c1 = (c1 & 0xFFFF) + (c1 >> 16); }
/* Sums[0,1] mod 64K + overflow */
-
Marshall Dawson (Code Review) -
Martin Roth (Code Review)
-
Patrick Georgi (Code Review)