Change in coreboot[master]: util/amdfwtool: Fix warning taking address of packed struct member
Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44986 )
Change subject: util/amdfwtool: Fix warning taking address of packed struct member ......................................................................
util/amdfwtool: Fix warning taking address of packed struct member
GCC9 introduced a new warning [-Waddress-of-packed-member]. This is giving the following warning when building amdfwtool: warning: taking address of packed member of ‘struct _bios_directory_entry’ may result in an unaligned pointer value. Looking at the definition of the struct, it looks like this is probably true.
Since the function being called doesn't read from the values, zeroing them out in the beginning of the function, the code just passes pointers to the temporary variables without initializing them.
BUG=None TEST=Build & use AMD firmware table. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I2f1e0aede8563e39ab0f2ec6daed91d6431eac43 --- M util/amdfwtool/amdfwtool.c 1 file changed, 6 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/44986/1
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 499a1bd..299ed0c 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -888,6 +888,8 @@ unsigned int i, count; int level; int apob_idx; + uint32_t size; + uint64_t source;
/* This function can create a primary table, a secondary table, or a * flattened table which contains all applicable types. These if-else @@ -996,10 +998,11 @@ break; case AMD_BIOS_BIN: /* Don't make a 2nd copy, point to the same one */ - if (level == BDT_LVL1 && locate_bdt2_bios(biosdir2, - &biosdir->entries[count].source, - &biosdir->entries[count].size)) + if (level == BDT_LVL1 && locate_bdt2_bios(biosdir2, &source, &size)) { + biosdir->entries[count].source = source; + biosdir->entries[count].size = size; break; + }
/* level 2, or level 1 and no copy found in level 2 */ biosdir->entries[count].source = fw_table[i].src;
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44986 )
Change subject: util/amdfwtool: Fix warning taking address of packed struct member ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44986 )
Change subject: util/amdfwtool: Fix warning taking address of packed struct member ......................................................................
Patch Set 1: Code-Review+2
Eric Peers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44986 )
Change subject: util/amdfwtool: Fix warning taking address of packed struct member ......................................................................
Patch Set 1: Code-Review+1
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44986 )
Change subject: util/amdfwtool: Fix warning taking address of packed struct member ......................................................................
util/amdfwtool: Fix warning taking address of packed struct member
GCC9 introduced a new warning [-Waddress-of-packed-member]. This is giving the following warning when building amdfwtool: warning: taking address of packed member of ‘struct _bios_directory_entry’ may result in an unaligned pointer value. Looking at the definition of the struct, it looks like this is probably true.
Since the function being called doesn't read from the values, zeroing them out in the beginning of the function, the code just passes pointers to the temporary variables without initializing them.
BUG=None TEST=Build & use AMD firmware table. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I2f1e0aede8563e39ab0f2ec6daed91d6431eac43 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44986 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Raul Rangel rrangel@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Eric Peers epeers@google.com --- M util/amdfwtool/amdfwtool.c 1 file changed, 6 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Raul Rangel: Looks good to me, approved Eric Peers: Looks good to me, but someone else must approve
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 499a1bd..299ed0c 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -888,6 +888,8 @@ unsigned int i, count; int level; int apob_idx; + uint32_t size; + uint64_t source;
/* This function can create a primary table, a secondary table, or a * flattened table which contains all applicable types. These if-else @@ -996,10 +998,11 @@ break; case AMD_BIOS_BIN: /* Don't make a 2nd copy, point to the same one */ - if (level == BDT_LVL1 && locate_bdt2_bios(biosdir2, - &biosdir->entries[count].source, - &biosdir->entries[count].size)) + if (level == BDT_LVL1 && locate_bdt2_bios(biosdir2, &source, &size)) { + biosdir->entries[count].source = source; + biosdir->entries[count].size = size; break; + }
/* level 2, or level 1 and no copy found in level 2 */ biosdir->entries[count].source = fw_table[i].src;
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44986 )
Change subject: util/amdfwtool: Fix warning taking address of packed struct member ......................................................................
Patch Set 2:
Automatic boot test returned (PASS/FAIL/TOTAL): 7/1/8 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/17705 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17704 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/17703 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17702 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/17701 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/17708 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/17707 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17706
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Aaron Durbin (Code Review)
-
Eric Peers (Code Review)
-
Furquan Shaikh (Code Review)
-
Martin Roth (Code Review)
-
Raul Rangel (Code Review)