Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31728
Change subject: util/amdfwutil: Clarify call to fletcher32 ......................................................................
util/amdfwutil: Clarify call to fletcher32
The fletcher32 algorighm generates a sum over a range of 16-bit WORDs. Change the function's interface to be more generic, accepting a more intuitive size in BYTEs. Don't require the caller to understand the nature of the algorithm and convert to WORDs prior to calling.
TEST=Verify no difference in amdfw.rom for google/grunt before and after
Change-Id: Iad70558347cbdb3c51bd598479ee4484219c0869 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M util/amdfwtool/amdfwtool.c 1 file changed, 14 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/31728/1
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 3ed6885..e1c59ae 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -121,12 +121,15 @@ * inserted 8 bytes after the beginning of the file. * stderr: Used to print out error messages. */ -static uint32_t fletcher32(const uint16_t *pptr, int length) +static uint32_t fletcher32(const void *data, int length) { uint32_t c0; uint32_t c1; uint32_t checksum; int index; + const uint16_t *pptr = data; + + length /= 2;
c0 = 0xFFFF; c1 = 0xFFFF; @@ -138,13 +141,15 @@ c0 += *(pptr++); c1 += c0; if ((index % 360) == 0) { - c0 = (c0 & 0xFFFF) + (c0 >> 16); // Sum0 modulo 65535 + the overflow - c1 = (c1 & 0xFFFF) + (c1 >> 16); // Sum1 modulo 65535 + the overflow + /* Sums[0,1] mod 64K + overflow */ + c0 = (c0 & 0xFFFF) + (c0 >> 16); + c1 = (c1 & 0xFFFF) + (c1 >> 16); } }
- c0 = (c0 & 0xFFFF) + (c0 >> 16); // Sum0 modulo 65535 + the overflow - c1 = (c1 & 0xFFFF) + (c1 >> 16); // Sum1 modulo 65535 + the overflow + /* Sums[0,1] mod 64K + overflow */ + c0 = (c0 & 0xFFFF) + (c0 >> 16); + c1 = (c1 & 0xFFFF) + (c1 >> 16); checksum = (c1 << 16) | c0;
return checksum; @@ -331,9 +336,9 @@ /* checksum everything that comes after the Checksum field */ pspdir->header.checksum = fletcher32( (void *)&pspdir->header.num_entries, - (count * sizeof(psp_directory_entry) + count * sizeof(psp_directory_entry) + sizeof(pspdir->header.num_entries) - + sizeof(pspdir->header.reserved)) / 2); + + sizeof(pspdir->header.reserved)); }
static uint32_t integrate_firmwares(char *base, uint32_t pos, @@ -848,11 +853,11 @@ combo_dir->header.reserved[1] = 0; combo_dir->header.checksum = fletcher32( (void *)&combo_dir->header.num_entries, - (1 * sizeof(psp_directory_entry) + 1 * sizeof(psp_directory_entry) + sizeof(combo_dir->header.num_entries) + sizeof(combo_dir->header.lookup) + sizeof(combo_dir->header.reserved[0]) - + sizeof(combo_dir->header.reserved[1])) / 2); + + sizeof(combo_dir->header.reserved[1])); #else current = integrate_psp_firmwares(rom, current, psp2dir, amd_psp2_fw_table, rom_size);
Marshall Dawson has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/31728 )
Change subject: util/amdfwtool: Clarify call to fletcher32 ......................................................................
util/amdfwtool: Clarify call to fletcher32
The fletcher32 algorithm generates a sum over a range of 16-bit WORDs. Change the function's interface to be more generic, accepting a more intuitive size in BYTEs. Don't require the caller to understand the nature of the algorithm and convert to WORDs prior to calling.
TEST=Verify no difference in amdfw.rom for google/grunt before and after
Change-Id: Iad70558347cbdb3c51bd598479ee4484219c0869 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M util/amdfwtool/amdfwtool.c 1 file changed, 14 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/31728/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31728 )
Change subject: util/amdfwtool: Clarify call to fletcher32 ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/31728/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31728/2//COMMIT_MSG@15 PS2, Line 15: TEST=Verify no difference in amdfw.rom for google/grunt before and after Wrap long line?
Hello Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31728
to look at the new patch set (#3).
Change subject: util/amdfwtool: Clarify call to fletcher32 ......................................................................
util/amdfwtool: Clarify call to fletcher32
The fletcher32 algorithm generates a sum over a range of 16-bit WORDs. Change the function's interface to be more generic, accepting a more intuitive size in BYTEs. Don't require the caller to understand the nature of the algorithm and convert to WORDs prior to calling.
TEST=Verify no difference in amdfw.rom for google/grunt before and after the patch is applied
Change-Id: Iad70558347cbdb3c51bd598479ee4484219c0869 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M util/amdfwtool/amdfwtool.c 1 file changed, 14 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/31728/3
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31728 )
Change subject: util/amdfwtool: Clarify call to fletcher32 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31728/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31728/2//COMMIT_MSG@15 PS2, Line 15: TEST=Verify no difference in amdfw.rom for google/grunt before and after
Wrap long line?
Done
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31728 )
Change subject: util/amdfwtool: Clarify call to fletcher32 ......................................................................
Patch Set 3: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31728 )
Change subject: util/amdfwtool: Clarify call to fletcher32 ......................................................................
util/amdfwtool: Clarify call to fletcher32
The fletcher32 algorithm generates a sum over a range of 16-bit WORDs. Change the function's interface to be more generic, accepting a more intuitive size in BYTEs. Don't require the caller to understand the nature of the algorithm and convert to WORDs prior to calling.
TEST=Verify no difference in amdfw.rom for google/grunt before and after the patch is applied
Change-Id: Iad70558347cbdb3c51bd598479ee4484219c0869 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/31728 Reviewed-by: Martin Roth martinroth@google.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M util/amdfwtool/amdfwtool.c 1 file changed, 14 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 3ed6885..e1c59ae 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -121,12 +121,15 @@ * inserted 8 bytes after the beginning of the file. * stderr: Used to print out error messages. */ -static uint32_t fletcher32(const uint16_t *pptr, int length) +static uint32_t fletcher32(const void *data, int length) { uint32_t c0; uint32_t c1; uint32_t checksum; int index; + const uint16_t *pptr = data; + + length /= 2;
c0 = 0xFFFF; c1 = 0xFFFF; @@ -138,13 +141,15 @@ c0 += *(pptr++); c1 += c0; if ((index % 360) == 0) { - c0 = (c0 & 0xFFFF) + (c0 >> 16); // Sum0 modulo 65535 + the overflow - c1 = (c1 & 0xFFFF) + (c1 >> 16); // Sum1 modulo 65535 + the overflow + /* Sums[0,1] mod 64K + overflow */ + c0 = (c0 & 0xFFFF) + (c0 >> 16); + c1 = (c1 & 0xFFFF) + (c1 >> 16); } }
- c0 = (c0 & 0xFFFF) + (c0 >> 16); // Sum0 modulo 65535 + the overflow - c1 = (c1 & 0xFFFF) + (c1 >> 16); // Sum1 modulo 65535 + the overflow + /* Sums[0,1] mod 64K + overflow */ + c0 = (c0 & 0xFFFF) + (c0 >> 16); + c1 = (c1 & 0xFFFF) + (c1 >> 16); checksum = (c1 << 16) | c0;
return checksum; @@ -331,9 +336,9 @@ /* checksum everything that comes after the Checksum field */ pspdir->header.checksum = fletcher32( (void *)&pspdir->header.num_entries, - (count * sizeof(psp_directory_entry) + count * sizeof(psp_directory_entry) + sizeof(pspdir->header.num_entries) - + sizeof(pspdir->header.reserved)) / 2); + + sizeof(pspdir->header.reserved)); }
static uint32_t integrate_firmwares(char *base, uint32_t pos, @@ -848,11 +853,11 @@ combo_dir->header.reserved[1] = 0; combo_dir->header.checksum = fletcher32( (void *)&combo_dir->header.num_entries, - (1 * sizeof(psp_directory_entry) + 1 * sizeof(psp_directory_entry) + sizeof(combo_dir->header.num_entries) + sizeof(combo_dir->header.lookup) + sizeof(combo_dir->header.reserved[0]) - + sizeof(combo_dir->header.reserved[1])) / 2); + + sizeof(combo_dir->header.reserved[1])); #else current = integrate_psp_firmwares(rom, current, psp2dir, amd_psp2_fw_table, rom_size);