Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35667 )
Change subject: util/amdfwtool: Grow the Embedded Firmware Structure ......................................................................
util/amdfwtool: Grow the Embedded Firmware Structure
Ensure adequate space exists for all Embedded Firmware Structure fields.
Field definitions are NDA only. See PID #55758 "AMD Platform Security Processor BIOS Architecture Design Guide for AMD Family 17h Processors".
BUG=b:141790457 TEST=run on Mandolin
Change-Id: I098ffc7c05d27387a877e6b7c8628d98939bd9af Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M util/amdfwtool/amdfwtool.c 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/35667/1
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 4c5b216..2980622 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -387,6 +387,8 @@ uint32_t comboable; uint32_t bios0_entry; /* todo: add way to select correct entry */ uint32_t bios1_entry; + uint32_t bios2_entry; + uint32_t reserved[0x2c]; /* 0x24 - 0x4f */ } __attribute__((packed, aligned(16))) embedded_firmware;
typedef struct _psp_directory_header {
Justin Frodsham has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35667 )
Change subject: util/amdfwtool: Grow the Embedded Firmware Structure ......................................................................
Patch Set 1: Code-Review+1
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35667 )
Change subject: util/amdfwtool: Grow the Embedded Firmware Structure ......................................................................
Patch Set 2:
(1 comment)
Doing some review while I have nothing to do.
https://review.coreboot.org/c/coreboot/+/35667/2/util/amdfwtool/amdfwtool.c File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/35667/2/util/amdfwtool/amdfwtool.c@... PS2, Line 391: 0x4f Why so much reserved? Is there expectation to fill it up? If not, would it not be preferable 0x3f, making total size 0x40 (power of 2)?
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35667 )
Change subject: util/amdfwtool: Grow the Embedded Firmware Structure ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35667/2/util/amdfwtool/amdfwtool.c File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/35667/2/util/amdfwtool/amdfwtool.c@... PS2, Line 391: 0x4f
Why so much reserved? Is there expectation to fill it up? If not, would it not be preferable 0x3f, m […]
This is simply defining the extent to which the PSP spec defines the structure. This change isn't fixing any existing problem because there's already some padding introduced. However, it ensures we won't attempt to accidentally put something immediately following bios1_entry in violation of what the PSP expects.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35667 )
Change subject: util/amdfwtool: Grow the Embedded Firmware Structure ......................................................................
Patch Set 2: Code-Review+2
Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35667 )
Change subject: util/amdfwtool: Grow the Embedded Firmware Structure ......................................................................
util/amdfwtool: Grow the Embedded Firmware Structure
Ensure adequate space exists for all Embedded Firmware Structure fields.
Field definitions are NDA only. See PID #55758 "AMD Platform Security Processor BIOS Architecture Design Guide for AMD Family 17h Processors".
BUG=b:141790457 TEST=run on Mandolin
Change-Id: I098ffc7c05d27387a877e6b7c8628d98939bd9af Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35667 Reviewed-by: Martin Roth martinroth@google.com Reviewed-by: Justin Frodsham justin.frodsham@amd.corp-partner.google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M util/amdfwtool/amdfwtool.c 1 file changed, 2 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved Justin Frodsham: Looks good to me, but someone else must approve
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 4c5b216..2980622 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -387,6 +387,8 @@ uint32_t comboable; uint32_t bios0_entry; /* todo: add way to select correct entry */ uint32_t bios1_entry; + uint32_t bios2_entry; + uint32_t reserved[0x2c]; /* 0x24 - 0x4f */ } __attribute__((packed, aligned(16))) embedded_firmware;
typedef struct _psp_directory_header {
-
Justin Frodsham (Code Review) -
Marshall Dawson (Code Review)
-
Martin Roth (Code Review)
-
Richard Spiegel (Code Review)