Attention is currently required from: Bora Guvendik, Subrata Banik, Selma Bensaid, Tim Wawrzynczak, Paul Menzel, Julius Werner, Patrick Rudolph.
Hello build bot (Jenkins), Subrata Banik, Selma Bensaid, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/59507
to look at the new patch set (#17).
Change subject: soc/intel/alderlake: Inject CSE TS into CBMEM timestamp table
......................................................................
soc/intel/alderlake: Inject CSE TS into CBMEM timestamp table
Get boot performance timestamps from CSE and inject them into CBMEM
timestamp table after normalizing to the zero-point value. Although
consumer CSE sku also supports this feature, it was validated on
CSE Lite sku only.
BUG=b:182575295
TEST=Able to see TS elapse prior to IA reset on Brya/Redrix
990:CSME ROM started execution 0
944:CSE sent 'Boot Stall Done' to PMC 88,000
945:CSE started to handle ICC configuration 88,000 (0)
946:CSE sent 'Host BIOS Prep Done' to PMC 90,000 (2,000)
947:CSE received 'CPU Reset Done Ack sent' from PMC 282,000 (192,000)
0:1st timestamp 330,857 (48,857)
11:start of bootblock 341,811 (10,953)
12:end of bootblock 349,299 (7,487)
Signed-off-by: Bora Guvendik <bora.guvendik(a)intel.com>
Change-Id: Idcdbb69538ca2977cd97ce1ef9b211ff6510a3f8
---
M src/soc/intel/alderlake/romstage/romstage.c
M src/soc/intel/common/block/cse/Kconfig
M src/soc/intel/common/block/cse/Makefile.inc
A src/soc/intel/common/block/cse/telemetry.c
M src/soc/intel/common/block/include/intelblocks/cse.h
5 files changed, 92 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/59507/17
--
To view, visit https://review.coreboot.org/c/coreboot/+/59507
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idcdbb69538ca2977cd97ce1ef9b211ff6510a3f8
Gerrit-Change-Number: 59507
Gerrit-PatchSet: 17
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.corp-partner.google.com>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Subrata Banik, Angel Pons, Patrick Rudolph, EricR Lai.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60406 )
Change subject: soc/intel/alderlake: Skip FSP Notify APIs
......................................................................
Patch Set 21:
(1 comment)
File src/soc/intel/alderlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/60406/comment/239e24fb_9a15413b
PS21, Line 360:
: config USE_COREBOOT_NATIVE_END_OF_POST_DRIVER
: bool
: default y
: select SKIP_FSP_NOTIFY_PHASE_READY_TO_BOOT
: select SKIP_FSP_NOTIFY_PHASE_END_OF_FIRMWARE
:
I would skip this and just select the two SKIP Kconfigs
--
To view, visit https://review.coreboot.org/c/coreboot/+/60406
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0198c9568de0e74053775682a44324405746389a
Gerrit-Change-Number: 60406
Gerrit-PatchSet: 21
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 09 Feb 2022 20:45:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Angel Pons, Patrick Rudolph, EricR Lai.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61649 )
Change subject: soc/intel/*/pmc: Add `finalize` operation for pmc
......................................................................
Patch Set 6:
(1 comment)
File src/soc/intel/alderlake/pmc.c:
https://review.coreboot.org/c/coreboot/+/61649/comment/c2cf11ae_b26fdb97
PS6, Line 182: if (CONFIG(SKIP_FSP_NOTIFY_PHASE_END_OF_FIRMWARE))
do we even really need the Kconfig here? I don't think there would be any harm in clearing the bits again?
--
To view, visit https://review.coreboot.org/c/coreboot/+/61649
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0a0b869849d5d8c76031b8999f3d28817ac69247
Gerrit-Change-Number: 61649
Gerrit-PatchSet: 6
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 09 Feb 2022 20:44:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
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/+/61680 )
Change subject: soc/amd/common/block/pci/amd_pci_mmconf: add assert for MMCONF region
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/common/block/pci/amd_pci_mmconf.c:
https://review.coreboot.org/c/coreboot/+/61680/comment/4b8c899f_332cd00b
PS1, Line 15: CONFIG_ECAM_MMCONF_BASE_ADDRESS
> I wonder if the number is getting truncated since it's missing the ULL suffix. […]
i set CONFIG_ECAM_MMCONF_BASE_ADDRESS to 1f8000000 and had to add a few & 0xffffffff t make things compile and got this output:
[DEBUG] 1f8000000
so it doesn't truncate, but this check is probably not needed, since compiling coreboot fails in both fsp_m_params.c and acpi/acpi.c due to wrong types. might still be a good idea to add some sort of comment that CONFIG_ECAM_MMCONF_BASE_ADDRESS is assumed to be below 4GB
--
To view, visit https://review.coreboot.org/c/coreboot/+/61680
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib0b4443d7a86f20c4c2508a88db5d731560f8312
Gerrit-Change-Number: 61680
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-CC: Paul Menzel <paulepanter(a)mailbox.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: Wed, 09 Feb 2022 20:44:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Anjaneya "Reddy" Chagam, Subrata Banik, Jonathan Zhang, Johnny Lin, Christian Walter, Angel Pons, Nick Vaccaro, Arthur Heymans, Patrick Rudolph, EricR Lai, Tim Chu.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61652 )
Change subject: soc/intel/xeon_sp: Add function to clear PMCON status bits
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/61652
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I22650f539a1646f93f2c6494cbf54b8ca785d6ad
Gerrit-Change-Number: 61652
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Wed, 09 Feb 2022 20:43:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Felix Singer, Subrata Banik, Sridhar Siricilla, Angel Pons, Lean Sheng Tan, Werner Zeh, Patrick Rudolph, EricR Lai.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60405 )
Change subject: soc/intel/common/cse: Add `finalize` operation for CSE
......................................................................
Patch Set 25:
(1 comment)
File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/60405/comment/94634828_f69dd35f
PS24, Line 1195: static void cse_final(struct device *dev)
: {
: if (CONFIG(SKIP_FSP_NOTIFY_PHASE_READY_TO_BOOT)) {
: cse_send_end_of_post();
:
: cse_control_global_reset_lock();
:
: if (CONFIG(DISABLE_HECI1_AT_PRE_BOOT)) {
: cse_set_to_d0i3();
: heci1_disable();
: }
: }
:
: if (CONFIG(SKIP_FSP_NOTIFY_PHASE_END_OF_FIRMWARE))
: heci_set_to_d0i3();
: }
> I saw from coreboot perspective when I made my proposal, let coreboot implement SoC recommendations […]
Sridhar, I think of this process as us all just working together to find the solution that is OK with everyone 😊
I also agree with Arthur, and think that a good followup patch would be to rename the "skip FSP notify..." to "use FSP notify..." , similar to USE_NEM / USE_ENEM, not "skip FSP-T"
--
To view, visit https://review.coreboot.org/c/coreboot/+/60405
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I70bde33f77026e8be165ff082defe3cab6686ec7
Gerrit-Change-Number: 60405
Gerrit-PatchSet: 25
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 09 Feb 2022 20:40:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Christian Walter, Patrick Rudolph.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61721 )
Change subject: drivers/i2c/tpm/cr50: Add support to get and set BOARD_CFG register
......................................................................
Patch Set 6:
(1 comment)
File src/drivers/i2c/tpm/cr50.c:
https://review.coreboot.org/c/coreboot/+/61721/comment/546b7d9c_63ec6599
PS5, Line 533: int
> can you use `bool` which is like do_cr50_fw_supports_brd_cfg()
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/61721
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9fcfed9fa133db2560c4a855376d249e865c1335
Gerrit-Change-Number: 61721
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 09 Feb 2022 20:33:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: comment