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
Attention is currently required from: Tim Wawrzynczak, Angel Pons.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52893 )
Change subject: include/console: Align ramstage Boot State Machine postcodes
......................................................................
Patch Set 1:
(1 comment)
File src/include/console/post_codes.h:
https://review.coreboot.org/c/coreboot/+/52893/comment/51370995_fba70382
PS1, Line 182: 0x79
> I think it is okay. […]
Sure Furquan, Please let me know https://review.coreboot.org/c/coreboot/+/52895/1 if you would like to add more details ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/52893
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9e03159fdf07a73f5f8eec1bbf32fcb47dd4af84
Gerrit-Change-Number: 52893
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 04 May 2021 18:32:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-MessageType: comment
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52895 )
Change subject: include/console: Fix duplicate entry of postcode 0x79
......................................................................
include/console: Fix duplicate entry of postcode 0x79
Change POST_PRE_HARDWAREMAIN postcode value from 0x79 to 0x7c to
avoid duplicate entry.
Change-Id: I50cc75cd3097fba3e7faff05188511bba69ef1e7
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
---
M src/arch/x86/c_start.S
M src/include/console/post_codes.h
2 files changed, 8 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/52895/1
diff --git a/src/arch/x86/c_start.S b/src/arch/x86/c_start.S
index 19532d8..e515dbc 100644
--- a/src/arch/x86/c_start.S
+++ b/src/arch/x86/c_start.S
@@ -89,7 +89,7 @@
* bss is cleared. Now we call the main routine and
* let it do the rest.
*/
- post_code(POST_PRE_HARDWAREMAIN) /* post fe */
+ post_code(POST_PRE_HARDWAREMAIN) /* post 7c */
andl $0xFFFFFFF0, %esp
diff --git a/src/include/console/post_codes.h b/src/include/console/post_codes.h
index 69047f8..5af6734 100644
--- a/src/include/console/post_codes.h
+++ b/src/include/console/post_codes.h
@@ -175,13 +175,6 @@
#define POST_BS_WRITE_TABLES 0x79
/**
- * \brief Pre call to RAM stage main()
- *
- * POSTed right before RAM stage main() is called from c_start.S
- */
-#define POST_PRE_HARDWAREMAIN 0x79
-
-/**
* \brief Load Payload
*
* Boot State Machine: bs_payload_load()
@@ -196,6 +189,13 @@
#define POST_BS_PAYLOAD_BOOT 0x7b
/**
+ * \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
--
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-MessageType: newchange
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/+/52893 )
Change subject: include/console: Align ramstage Boot State Machine postcodes
......................................................................
Patch Set 1:
(1 comment)
File src/include/console/post_codes.h:
https://review.coreboot.org/c/coreboot/+/52893/comment/443e6550_6828c9d2
PS1, Line 182: 0x79
> Yes, i was adding 0x7c for POST_PRE_HARDWAREMAIN, submitting the CL, hopes that fine, because we hav […]
I think it is okay. Only thing I would be worried about is if anyone is using any scripts/tests that look for that particular post code. But, then those scripts would really be checking for post code at incorrect point in boot. Let's make sure that the detail is properly captured in the commit message of the new CL so that people understand the impact of the change. Thanks!
--
To view, visit https://review.coreboot.org/c/coreboot/+/52893
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9e03159fdf07a73f5f8eec1bbf32fcb47dd4af84
Gerrit-Change-Number: 52893
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:30:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Angel Pons.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52893 )
Change subject: include/console: Align ramstage Boot State Machine postcodes
......................................................................
Patch Set 1:
(1 comment)
File src/include/console/post_codes.h:
https://review.coreboot.org/c/coreboot/+/52893/comment/99f57b87_4d860067
PS1, Line 182: 0x79
> Just noticed that we have duplicate 0x79 post codes.
Yes, i was adding 0x7c for POST_PRE_HARDWAREMAIN, submitting the CL, hopes that fine, because we have 0x7b-0x7f empty.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52893
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9e03159fdf07a73f5f8eec1bbf32fcb47dd4af84
Gerrit-Change-Number: 52893
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: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 04 May 2021 18:26:24 +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: Tim Wawrzynczak, Subrata Banik, Angel Pons.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52894 )
Change subject: include/console: Fix comments for FSP Notify End of Firmware postcodes
......................................................................
Patch Set 1:
(3 comments)
File src/include/console/post_codes.h:
https://review.coreboot.org/c/coreboot/+/52894/comment/da6175c5_07146876
PS1, Line 209: phase
Probably say "(end of firmware)" after phase? Same for after call?
https://review.coreboot.org/c/coreboot/+/52894/comment/42935a25_6518ea45
PS1, Line 256: before
This seems to have the same problem.
https://review.coreboot.org/c/coreboot/+/52894/comment/b8a837cb_11b4fe54
PS1, Line 260: POST_FSP_NOTIFY_BEFORE_FINALIZE
Just noticed that we are incorrectly using this even after finalize notification is done: https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/sr…
Same for enumeration phase.
--
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: 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-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:26:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Maulik V Vaghela, Selma Bensaid.
Bora Guvendik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52865 )
Change subject: mb/intel/adlrvp_m: Program CPU PCIE RP GPIOs in early GPIO
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/52865/comment/5ea06f3c_9dd1c7f4
PS1, Line 9: We need to configure CPU PCIE root port related gpios in early
: boot block stage for CPU root ports to work.
> Thanks Maulik. […]
Thanks Maulik.
Done.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52865
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I27c898943471d834bd82e3c7e8b36cceb12de099
Gerrit-Change-Number: 52865
Gerrit-PatchSet: 2
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Comment-Date: Tue, 04 May 2021 18:22:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-MessageType: comment