Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Andrey Petrov, Patrick Rudolph.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52894 )
Change subject: include/console: Fix FSP Notify phase postcodes discrepancy
......................................................................
Patch Set 2:
(3 comments)
File src/include/console/post_codes.h:
https://review.coreboot.org/c/coreboot/+/52894/comment/804ffb9b_63f20d3f
PS1, Line 209: phase
> Probably say "(end of firmware)" after phase? Same for after call?
Ack
https://review.coreboot.org/c/coreboot/+/52894/comment/a98fe577_da027211
PS1, Line 256: before
> This seems to have the same problem.
Ack
https://review.coreboot.org/c/coreboot/+/52894/comment/9e7fb7d7_3011f0f5
PS1, Line 260: POST_FSP_NOTIFY_BEFORE_FINALIZE
> Just noticed that we are incorrectly using this even after finalize notification is done: https://re […]
https://review.coreboot.org/c/coreboot/+/52894/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52894
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c825d5f1f31f80ad2a03ff5d6006daa7104d23
Gerrit-Change-Number: 52894
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 04 May 2021 18:58:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52896 )
Change subject: soc/amd/common/fsp/fsp-acpi: factor out SSDT from HOB functionality
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/common/fsp/fsp-acpi.c:
https://review.coreboot.org/c/coreboot/+/52896/comment/e92cf9b4_078a2f9c
PS1, Line 18: add_agesa_fsp_acpi_table
> Should we add alib into the name and drop the name parameter? This no longer searches for specific t […]
this is a generic function to add a ssdt from a hob. i'm not sure what you mean with "searches for specific tables"; it finds the table by the guid of the hob. sure, i don't expect that we need to add another ssdt that gets passed from the fsp to coreboot via a hob, but this isn't alib specific. the alib-specific part is passed as parameters to the call in agesa_write_acpi_tables. for an example how that can grab another hob and process it into a ssdt, see the line of code that got removed in this patch https://review.coreboot.org/c/coreboot/+/47727/6/src/soc/amd/picasso/agesa_…
--
To view, visit https://review.coreboot.org/c/coreboot/+/52896
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b256de712fe60d1c021cb875aaadec1d331584b
Gerrit-Change-Number: 52896
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Comment-Date: Tue, 04 May 2021 18:54:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Subrata Banik, Angel Pons.
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52894
to look at the new patch set (#2).
Change subject: include/console: Fix FSP Notify phase postcodes discrepancy
......................................................................
include/console: Fix FSP Notify phase postcodes discrepancy
List of changes:
1. Make the FSP notify phases name prior in comments section.
2. Fix discrepancies in FSP notify before and after postcode comments.
3. Add FSP notify postcode macros for after pci enumeration(0xb2)
and ready to boot(0x3) call.
Change-Id: Ib4c825d5f1f31f80ad2a03ff5d6006daa7104d23
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
---
M src/drivers/intel/fsp2_0/notify.c
M src/include/console/post_codes.h
2 files changed, 24 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/52894/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52894
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4c825d5f1f31f80ad2a03ff5d6006daa7104d23
Gerrit-Change-Number: 52894
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52896 )
Change subject: soc/amd/common/fsp/fsp-acpi: factor out SSDT from HOB functionality
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/common/fsp/fsp-acpi.c:
https://review.coreboot.org/c/coreboot/+/52896/comment/884e47ec_8f97b896
PS1, Line 18: add_agesa_fsp_acpi_table
Should we add alib into the name and drop the name parameter? This no longer searches for specific tables.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52896
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b256de712fe60d1c021cb875aaadec1d331584b
Gerrit-Change-Number: 52896
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 04 May 2021 18:41:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Subrata Banik, Angel Pons.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52895 )
Change subject: include/console: Fix duplicate entry of postcode 0x79
......................................................................
Patch Set 1:
(1 comment)
File src/include/console/post_codes.h:
https://review.coreboot.org/c/coreboot/+/52895/comment/5aaa1cd9_e6af10a5
PS1, Line 191: /**
: * \brief Pre call to RAM stage main()
: *
: * POSTed right before RAM stage main() is called from c_start.S
: */
: #define POST_PRE_HARDWAREMAIN 0x7c
:
: /**
: * \brief Entry into coreboot in RAM stage main()
: *
: * This is the first call in hardwaremain.c. If this code is POSTed, then
: * ramstage has successfully loaded and started executing.
: */
: #define POST_ENTRY_RAMSTAGE 0x80
Just a thought: Should POST_PRE_HARDWAREMAIN be 0x6e and POST_ENTRY_RAMSTAGE 0x6f so that the post codes appear in an increasing fashion? Does that even matter?
--
To view, visit https://review.coreboot.org/c/coreboot/+/52895
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I50cc75cd3097fba3e7faff05188511bba69ef1e7
Gerrit-Change-Number: 52895
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 04 May 2021 18:40:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52897 )
Change subject: soc/amd/common/fsp/fsp-acpi: add check for maximum table size
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52897
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I965c01bd9ab66b14d6f77b6f23c28479ae6d6a50
Gerrit-Change-Number: 52897
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 04 May 2021 18:38:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held.
Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52897 )
Change subject: soc/amd/common/fsp/fsp-acpi: add check for maximum table size
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
good catch
--
To view, visit https://review.coreboot.org/c/coreboot/+/52897
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I965c01bd9ab66b14d6f77b6f23c28479ae6d6a50
Gerrit-Change-Number: 52897
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 04 May 2021 18:34:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson.
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52897 )
Change subject: soc/amd/common/fsp/fsp-acpi: add check for maximum table size
......................................................................
soc/amd/common/fsp/fsp-acpi: add check for maximum table size
If the ACPI table size in the HOB data header is larger than the maximum
HOB payload, don't add the table at all and print an error instead,
since in this case the memcpy would read past the end of the HOB data
structure.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I965c01bd9ab66b14d6f77b6f23c28479ae6d6a50
---
M src/soc/amd/common/fsp/fsp-acpi.c
1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/52897/1
diff --git a/src/soc/amd/common/fsp/fsp-acpi.c b/src/soc/amd/common/fsp/fsp-acpi.c
index b530688..f92d571 100644
--- a/src/soc/amd/common/fsp/fsp-acpi.c
+++ b/src/soc/amd/common/fsp/fsp-acpi.c
@@ -28,6 +28,12 @@
return current;
}
+ if (data->table_size_in_bytes > sizeof(data->hob_payload)) {
+ printk(BIOS_ERR, "AGESA %s ACPI table size larger than maximum HOB payload "
+ "size.\n", name);
+ return current;
+ }
+
printk(BIOS_INFO, "ACPI: * %s (AGESA).\n", name);
memcpy(table, data->hob_payload, data->table_size_in_bytes);
--
To view, visit https://review.coreboot.org/c/coreboot/+/52897
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I965c01bd9ab66b14d6f77b6f23c28479ae6d6a50
Gerrit-Change-Number: 52897
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newchange