Attention is currently required from: Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik, Weimin Wu.
Qinghong Zeng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80158?usp=email )
Change subject: mb/google/brya: Correct EC-is-trusted logic
......................................................................
Patch Set 1:
This change is ready for review.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80158?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I308f8b36411030911c4421d80827fc49ff325a1b
Gerrit-Change-Number: 80158
Gerrit-PatchSet: 1
Gerrit-Owner: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 13:27:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Matt DeVillier.
Jonathon Hall has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80182?usp=email )
Change subject: mb/purism/librem_cnl: Set edk2 boot timeout for Librem Mini
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80182?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I9d2d514719a9918ee58cc63969b3adae44ac1632
Gerrit-Change-Number: 80182
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 13:26:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Qinghong Zeng has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80158?usp=email )
Change subject: mb/google/brya: Correct EC-is-trusted logic
......................................................................
mb/google/brya: Correct EC-is-trusted logic
With Cr50, the GPIO EC_IN_RW is used to determine whether EC is
trusted. However, with Ti50 where brya has been switched to, it is
determined by Ti50's boot mode. If the boot mode is TRUSTED_RO, the
VB2_CONTEXT_EC_TRUSTED flag will be set in check_boot_mode(). Therefore
in the Ti50 case get_ec_is_trusted() can just return 0.
The current code of get_ec_is_trusted() only checks the GPIO, which
causes the EC to be always considered "trusted". Therefore, correct the
return value to 0 for TPM_GOOGLE_TI50.
BUG=b:321172119
TEST=emerge-nissa coreboot chromeos-bootimage
TEST=firmware-DevMode passed in FAFT test
Change-Id: I308f8b36411030911c4421d80827fc49ff325a1b
Signed-off-by: zengqinghong <zengqinghong(a)huaqin.corp-partner.google.com>
---
M src/mainboard/google/brya/chromeos.c
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/80158/1
diff --git a/src/mainboard/google/brya/chromeos.c b/src/mainboard/google/brya/chromeos.c
index 5c99371..73b7237 100644
--- a/src/mainboard/google/brya/chromeos.c
+++ b/src/mainboard/google/brya/chromeos.c
@@ -24,6 +24,10 @@
int get_ec_is_trusted(void)
{
+ /* With Ti50, VB2_CONTEXT_EC_TRUSTED should be set according to the boot mode. */
+ if (CONFIG(TPM_GOOGLE_TI50))
+ return 0;
+
/* EC is trusted if not in RW. */
return !gpio_get(GPIO_EC_IN_RW);
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/80158?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I308f8b36411030911c4421d80827fc49ff325a1b
Gerrit-Change-Number: 80158
Gerrit-PatchSet: 1
Gerrit-Owner: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com>
Gerrit-MessageType: newchange
Attention is currently required from: Cliff Huang, David Ruth, Lance Zhao, Paul Menzel, Subrata Banik, Subrata Banik, Tim Wawrzynczak.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80170?usp=email )
Change subject: Add MTCL function to ACPI SSDT tables
......................................................................
Patch Set 6:
(1 comment)
File src/acpi/acpigen.c:
https://review.coreboot.org/c/coreboot/+/80170/comment/c7c4ae6b_36a6d1d2 :
PS6, Line 1968: acpigen_write_byte(bytes[i]);
> Is this valid AML? I would expect a Buffer() object around the bytes. […]
Looking into the kernel code (which looks crazy btw.) it seems to expect
individual integers (bails out if `sar_unit->type != ACPI_TYPE_INTEGER`,
doesn't check the number of bytes read). So we might have to prefix every
single byte with a BYTE_OP?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80170?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I9b5e7312a44e114270e664b983626faa6cfee350
Gerrit-Change-Number: 80170
Gerrit-PatchSet: 6
Gerrit-Owner: David Ruth <druth(a)chromium.org>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: David Ruth <druth(a)chromium.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 13:05:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Cliff Huang, David Ruth, Lance Zhao, Paul Menzel, Subrata Banik, Subrata Banik, Tim Wawrzynczak.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80170?usp=email )
Change subject: Add MTCL function to ACPI SSDT tables
......................................................................
Patch Set 6:
(7 comments)
Patchset:
PS6:
Hi, I have a very general question: why is this information put into the firmware?
Is it chip or board specific?
File src/acpi/acpigen.c:
https://review.coreboot.org/c/coreboot/+/80170/comment/be805c5e_46e529ce :
PS6, Line 1968: acpigen_write_byte(bytes[i]);
Is this valid AML? I would expect a Buffer() object around the bytes.
What does the decompilation look like?
File src/drivers/wifi/generic/acpi.c:
https://review.coreboot.org/c/coreboot/+/80170/comment/60399bdb_b31b9635 :
PS6, Line 45: }
Why the weak function? Wouldn't it be an error to select USE_MTCL without
a proper implementation?
https://review.coreboot.org/c/coreboot/+/80170/comment/745312d5_db4d21cd :
PS6, Line 591: CONFIG(USE_MTCL)
Putting this first would allow the compiler/linker to eliminate the other
checks, in case it's disabled.
https://review.coreboot.org/c/coreboot/+/80170/comment/19eddb55_327c8d6a :
PS6, Line 592: uint8_t mtcl_package[sizeof(struct wifi_mtcl)];
Why not declare a `struct wifi_mtcl`?
File src/vendorcode/google/chromeos/mtcl.c:
PS6:
What is chromeos specific about this?
https://review.coreboot.org/c/coreboot/+/80170/comment/c1beb8e2_f6bf911e :
PS6, Line 59: if (mtcl_bin_len != sizeof(struct wifi_mtcl)) {
If this function knows `struct wifi_mtcl` why isn't it used in the signature?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80170?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I9b5e7312a44e114270e664b983626faa6cfee350
Gerrit-Change-Number: 80170
Gerrit-PatchSet: 6
Gerrit-Owner: David Ruth <druth(a)chromium.org>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: David Ruth <druth(a)chromium.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 12:56:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: David Wu, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Tarun, Tyler Wang.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80000?usp=email )
Change subject: mb/google/rex/var/karis: Set SOC_TCHSCR_RST output low in bootblock
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/80000?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ieebd3cf3c320bc895d036c372f792ec7b5d7ebf9
Gerrit-Change-Number: 80000
Gerrit-PatchSet: 4
Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 12:20:06 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment