Hello Patrick Georgi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32029
to review the following change.
Change subject: libpayload: strtoull: Fix edge case bug with *endptr
......................................................................
libpayload: strtoull: Fix edge case bug with *endptr
strtoull() can optionally take a second pointer as an out-parameter that
will be adjusted to point to the end of the parsed string. This works
almost right, but misses one important edge case: when the parsed string
is "0", the function will interpret the leading '0' as an octal prefix,
so that the first actually parsed digit is already the terminating '\0'
byte. This will cause the function to early abort, which still
(correctly) returns 0 but doesn't adjust *endptr.
The early abort is pointless anyway -- the only other thing the function
does is run a for-loop whose condition is the exact inverse (so it's
guaranteed to run zero iterations in this case) and then adjust *endptr
(which we want). So just take it out. This also technically corrects the
behavior of *endptr for a completely invalid string, since the strtoull
man page says
> If there were no digits at all, strtoul() stores the original value of
> nptr in *endptr (and returns 0).
Change-Id: Idddd74e18e410a9d0b6dce9512ca0412b9e2333c
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
---
M payloads/libpayload/libc/string.c
1 file changed, 0 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/32029/1
diff --git a/payloads/libpayload/libc/string.c b/payloads/libpayload/libc/string.c
index a58efde..2266d4b 100644
--- a/payloads/libpayload/libc/string.c
+++ b/payloads/libpayload/libc/string.c
@@ -521,12 +521,6 @@
ptr += 2;
}
- /* If the first character isn't valid, then don't
- * bother */
-
- if (!*ptr || !_valid(*ptr, base))
- return 0;
-
for( ; *ptr && _valid(*ptr, base); ptr++)
ret = (ret * base) + _offset(*ptr, base);
--
To view, visit https://review.coreboot.org/c/coreboot/+/32029
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idddd74e18e410a9d0b6dce9512ca0412b9e2333c
Gerrit-Change-Number: 32029
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: newchange
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31841 )
Change subject: Documentation/soc/intel: Add MP Initialization document
......................................................................
Documentation/soc/intel: Add MP Initialization document
This patch provides documentation for MP initialization
option available in coreboot.
Change-Id: I055808e2ddf03663e1ec5d3d423054d1caa911cb
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31841
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi(a)google.com>
---
M Documentation/soc/intel/index.md
A Documentation/soc/intel/mp_init/mp_init.md
2 files changed, 57 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Patrick Georgi: Looks good to me, approved
diff --git a/Documentation/soc/intel/index.md b/Documentation/soc/intel/index.md
index 8a4c297..4f6b4f2 100644
--- a/Documentation/soc/intel/index.md
+++ b/Documentation/soc/intel/index.md
@@ -7,3 +7,4 @@
- [Common code development strategy](code_development_model/code_development_model.md)
- [FSP](fsp/index.md)
- [Ice Lake/9th Gen Core-i series](icelake/index.md)
+- [MP Initialization](mp_init/mp_init.md)
diff --git a/Documentation/soc/intel/mp_init/mp_init.md b/Documentation/soc/intel/mp_init/mp_init.md
new file mode 100644
index 0000000..f81ffc1
--- /dev/null
+++ b/Documentation/soc/intel/mp_init/mp_init.md
@@ -0,0 +1,56 @@
+# Multiple Processor (MP) Initialization
+
+This section is intended to document the purpose of performing multiprocessor
+initialization and its possible ways in coreboot space.
+
+Entire CPU multiprocessor initialization can be divided into two parts
+1. BSP (Boot Strap Processor) Initialization
+2. AP (Application Processor) Initialization
+
+* [Multiple Processor Init](https://www.intel.com/content/dam/www/public/us/en/documents/manuals/… - section 8.4
+
+## Problem Statement
+
+1. coreboot is capable enough to handle multiprocessor initialization on
+IA platforms.
+
+2. With restricted CPU programming logic, there might be some cases where
+certain feature programming can't be open sourced at early development of SOC.
+
+Platform code might need to compromise on those closed source nature of CPU
+programming if we don't plan to provide an alternate interface which can be
+used by coreboot to get rid of such close sourced CPU programming.
+
+## Possible Solution Space
+
+Considering these facts, there are 3 possible solutions to perform MP
+initialization from coreboot + FSP space.
+
+1. coreboot to perform complete MP initialization by its own. This includes
+BSP and AP programming of CPU features mostly non-restricted one. Preferred
+Kconfig is USE_COREBOOT_NATIVE_MP_INIT. SoCs like SKL, KBL, APL are okay to
+make use of same Kconfig option for MP initialization.
+
+2. Alternatively, SoC users also can skip coreboot doing MP initialization
+and make use of FSP binary to perform same task. This can be achieved by using
+Kconfig name USE_INTEL_FSP_MP_INIT. As of 2019 all Google Chrome products are
+using coreboot native MP initialization mechanism and some IOTG platforms
+are using FSP MP Init solution as well.
+
+3. Final option is to let coreboot publish PPI (PEIM to PEIM Interface) to
+perform some restricted (closed source) CPU programming. In that case,
+coreboot will use its native MP init and additionally publish MP service PPI
+for FSP to consume. FSP will execute some CPU programming using same PPI
+service from its own context. One can use
+USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI Kconfig to perform this
+operation.
+
+For latest SoCs like CNL, WHL, ICL, etc, its recommended to make use of this
+option in order to perform SGX and C6DRAM enabling.
+
+Typically all platforms supported by FSP 2.1 specification will have
+external PPI service feature implemented.
+
+[References]
+- [PPI](../fsp/ppi/ppi.md)
+- [MP Service PPI](../fsp/ppi/mp_service_ppi.md)
--
To view, visit https://review.coreboot.org/c/coreboot/+/31841
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I055808e2ddf03663e1ec5d3d423054d1caa911cb
Gerrit-Change-Number: 31841
Gerrit-PatchSet: 6
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: merged
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31841 )
Change subject: Documentation/soc/intel: Add MP Initialization document
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/31841
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I055808e2ddf03663e1ec5d3d423054d1caa911cb
Gerrit-Change-Number: 31841
Gerrit-PatchSet: 5
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 26 Mar 2019 11:21:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32046 )
Change subject: mb/google/poppy/variants/rammus: Support new onboard Micron memory
......................................................................
Patch Set 2: Code-Review+1
Yan Ru, you should remove your +1 from the change-set.
--
To view, visit https://review.coreboot.org/c/coreboot/+/32046
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaec4147a64313dcd461affb492805c0453e8703d
Gerrit-Change-Number: 32046
Gerrit-PatchSet: 2
Gerrit-Owner: YanRu Chen <kane_chen(a)pegatron.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: YanRu Chen <kane_chen(a)pegatron.corp-partner.google.com>
Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ken Lu <ken_lu(a)pegatron.corp-partner.google.com>
Gerrit-Comment-Date: Tue, 26 Mar 2019 10:24:53 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31841 )
Change subject: Documentation/soc/intel: Add MP Initialization document
......................................................................
Patch Set 5:
@Patrick, can you please check once.
--
To view, visit https://review.coreboot.org/c/coreboot/+/31841
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I055808e2ddf03663e1ec5d3d423054d1caa911cb
Gerrit-Change-Number: 31841
Gerrit-PatchSet: 5
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 26 Mar 2019 10:18:47 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/27055 )
Change subject: gma registers: Draw usage of Config.Fence_Count into the code
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/27055
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: libgfxinit
Gerrit-Branch: master
Gerrit-Change-Id: I2ab37f4cd9e6b4d37353ae4fd11d7e5a686d166f
Gerrit-Change-Number: 27055
Gerrit-PatchSet: 7
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 26 Mar 2019 09:38:58 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/27054 )
Change subject: gma broxton power/clocks: Turn pre-condition into code
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/27054
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: libgfxinit
Gerrit-Branch: master
Gerrit-Change-Id: I3ccafdc91a6e11fdc565e154f1394598a4572449
Gerrit-Change-Number: 27054
Gerrit-PatchSet: 7
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 26 Mar 2019 09:37:18 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment