Attention is currently required from: YH Lin, Tarun Tuli, Paul Menzel, Nick Vaccaro, Eric Lai.
Joey Peng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74727 )
Change subject: soc/intel/alderlake: Disable C1E on RPL CPUs
......................................................................
Patch Set 9:
(1 comment)
File src/soc/intel/alderlake/chip.h:
https://review.coreboot.org/c/coreboot/+/74727/comment/f92c8eb5_f84d934e
PS7, Line 690: c1e
> We will add the CPU ID judgement in fsp_params. […]
Hi google team, due to tight schedule, we would need this CL to merge ASAP.Kindly please help to review and approve if there are no concerns, thanks!
--
To view, visit https://review.coreboot.org/c/coreboot/+/74727
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic2d2d5d6075de25141c1d08ec18838731c63a342
Gerrit-Change-Number: 74727
Gerrit-PatchSet: 9
Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-CC: Jerry2 Huang <jerry2.huang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Kevin3 Yang <kevin3.yang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com>
Gerrit-Attention: YH Lin <yueherngl(a)google.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Comment-Date: Tue, 02 May 2023 02:53:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Comment-In-Reply-To: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-MessageType: comment
Attention is currently required from: YH Lin, Tarun Tuli, Paul Menzel, Nick Vaccaro, Eric Lai.
Joey Peng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74727 )
Change subject: soc/intel/alderlake: Disable C1E on RPL CPUs
......................................................................
Patch Set 9:
(2 comments)
Patchset:
PS9:
Adjusted CPU_ID judgement as Subrata suggested.
File src/soc/intel/alderlake/chip.h:
https://review.coreboot.org/c/coreboot/+/74727/comment/5a2e2b25_fc4a166f
PS7, Line 690: c1e
> Change the name to c1e_enable? And use bool for it?
We will add the CPU ID judgement in fsp_params.c as Subrata suggested, so we won't need to modify chip.h.
--
To view, visit https://review.coreboot.org/c/coreboot/+/74727
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic2d2d5d6075de25141c1d08ec18838731c63a342
Gerrit-Change-Number: 74727
Gerrit-PatchSet: 9
Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-CC: Jerry2 Huang <jerry2.huang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Kevin3 Yang <kevin3.yang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com>
Gerrit-Attention: YH Lin <yueherngl(a)google.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Comment-Date: Tue, 02 May 2023 02:51:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-MessageType: comment
Joey Peng has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/74786 )
Change subject: mb/google/brya/var/taeko: Disable C1E for RPL CPU
......................................................................
Abandoned
Will change it in SOC code, not mainboard
--
To view, visit https://review.coreboot.org/c/coreboot/+/74786
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5e4c74aa288f1c824c5e7ce83090cf61d7653183
Gerrit-Change-Number: 74786
Gerrit-PatchSet: 2
Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jerry2 Huang <jerry2.huang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Kevin3 Yang <kevin3.yang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com>
Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com>
Gerrit-MessageType: abandon
Attention is currently required from: YH Lin, Tarun Tuli, Joey Peng, Paul Menzel, Nick Vaccaro.
Hello build bot (Jenkins), YH Lin, Tarun Tuli, Subrata Banik, Nick Vaccaro,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/74727
to look at the new patch set (#9).
Change subject: soc/intel/alderlake: Disable C1E on RPL CPUs
......................................................................
soc/intel/alderlake: Disable C1E on RPL CPUs
Disable C1E on RPL CPUs
BUG=b:278654939
TEST:emerge-brya coreboot
Signed-off-by: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Change-Id: Ic2d2d5d6075de25141c1d08ec18838731c63a342
---
M src/soc/intel/alderlake/fsp_params.c
1 file changed, 19 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/74727/9
--
To view, visit https://review.coreboot.org/c/coreboot/+/74727
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic2d2d5d6075de25141c1d08ec18838731c63a342
Gerrit-Change-Number: 74727
Gerrit-PatchSet: 9
Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-CC: Jerry2 Huang <jerry2.huang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Kevin3 Yang <kevin3.yang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com>
Gerrit-Attention: YH Lin <yueherngl(a)google.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: YH Lin, Tarun Tuli, Joey Peng, Paul Menzel, Nick Vaccaro.
Hello build bot (Jenkins), YH Lin, Tarun Tuli, Subrata Banik, Nick Vaccaro,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/74727
to look at the new patch set (#8).
Change subject: soc/intel/alderlake: Disable C1E on RPL CPUs
......................................................................
soc/intel/alderlake: Disable C1E on RPL CPUs
Disable C1E on RPL CPUs
BUG=b:278654939
TEST:emerge-brya coreboot
Signed-off-by: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Change-Id: Ic2d2d5d6075de25141c1d08ec18838731c63a342
---
M src/soc/intel/alderlake/chip.h
M src/soc/intel/alderlake/fsp_params.c
2 files changed, 26 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/74727/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/74727
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic2d2d5d6075de25141c1d08ec18838731c63a342
Gerrit-Change-Number: 74727
Gerrit-PatchSet: 8
Gerrit-Owner: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-CC: Jerry2 Huang <jerry2.huang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Kevin3 Yang <kevin3.yang(a)lcfc.corp-partner.google.com>
Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com>
Gerrit-Attention: YH Lin <yueherngl(a)google.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Tarun Tuli, Subrata Banik, Reka Norman, Kapil Porwal, Arthur Heymans, Lean Sheng Tan.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74873 )
Change subject: cpu/intel/microcode: Implement microcode info caching inside cbmem
......................................................................
Patch Set 3:
(4 comments)
Patchset:
PS3:
I assume this patch is in response to b/242473942? And your time savings aren't really from "searching", they're from bypassing CBFS verification?
If we determined that we actually *want* to bypass CBFS verification, and that it is safe to do so, there are more straight-forward ways to do that (e.g. cbfs_unverified_area_map()). But my understanding from the previous discussion on the bug was that we don't actually want to do that because there could actually be some attack vector here (even if it's e.g. just a rollback attack). In that case, we obviously also can't do something like this.
File src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h:
https://review.coreboot.org/c/coreboot/+/74873/comment/d3ae97bd_7ef46194
PS3, Line 89: 0x55434f44
Note that this spells DOCU in little endian, if you were aiming for UCOD you need to flip it.
File src/cpu/intel/microcode/microcode.c:
https://review.coreboot.org/c/coreboot/+/74873/comment/21179ead_1325a274
PS3, Line 295: if (ENV_CREATES_CBMEM)
Why this? If you want this to only execute in RAMSTAGE, I think just wrapping the whole thing in `#if ENV_RAMSTAGE` would be clearer.
https://review.coreboot.org/c/coreboot/+/74873/comment/23219a91_b10bccde
PS3, Line 305: info->microcode_ptr = (uintptr_t)find_cbfs_microcode();
Is this just stashing a pointer to MMIO flash across reboots? That sounds like a bad idea. What if you get a firmware update in between where CBFS files can randomly move around?
Besides, what is the security story for this feature? CBMEM cannot be trusted across reboots, it could have been manipulated by any attacker trying to gain persistence (e.g. just point this at a random unused part of flash outside of CBFS and put a malicious microcode blob there). Is there some separate verification check for this microcode later on that we trust?
--
To view, visit https://review.coreboot.org/c/coreboot/+/74873
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I208a2e0e1dce96b2d4d63f882e8aaf4c25f77e3f
Gerrit-Change-Number: 74873
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Comment-Date: Tue, 02 May 2023 01:36:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Matt DeVillier, Christian Walter.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74856 )
Change subject: security/tpm: Add Kconfig to allow payload control of TPM1
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> ok, I tested with CONFIG_NO_TPM selected, and the TPM was no longer manageable from the payload (no […]
Well, did you rewrite your payload to actually send the TPM_Startup command, then? I'm not saying that NO_TPM is an exact drop-in replacement for what your patch does here, but I'm saying that the payload should be written to expect and work with the NO_TPM state instead.
If there are other consequences (e.g. ACPI table setup) of this that make that a problem for your payload, can you track down explicitly where that happens and how it is controlled? In general, the coreboot Kconfig tries to maintain a difference between "there's a TPM chip physically on the board" (CONFIG_MAINBOARD_HAS_TPMx) and "coreboot is communicating with the TPM" (CONFIG_TPMx). Maybe whatever ACPI table info you need should be keyed off a separate Kconfig that only depends on the former but not the latter.
--
To view, visit https://review.coreboot.org/c/coreboot/+/74856
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ieb7db109cbcc1a0166d95b6130b624b635bb7ac9
Gerrit-Change-Number: 74856
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Comment-Date: Tue, 02 May 2023 01:08:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Sean Rhodes, Matt DeVillier.
Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74891 )
Change subject: mb/google/reef: Disable unused devices in devicetrees
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/74891
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iacdf93b4952cbc63fc465f07d440463106527b8d
Gerrit-Change-Number: 74891
Gerrit-PatchSet: 3
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Tue, 02 May 2023 00:37:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment