Attention is currently required from: Nico Huber.
Krystian Hebel has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/84089?usp=email )
Change subject: mb/qemu-i440fx/rom_media: Use MEM_REGION_DEV_INIT() for boot_dev
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84089?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Icd3757ba4b5e8bfbee9e9c9d18bf0ee71520a8ac
Gerrit-Change-Number: 84089
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Mon, 26 Aug 2024 10:23:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans, Dinesh Gehlot, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Paul Menzel, Rishika Raj.
Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84080?usp=email )
Change subject: soc/intel/adl: Prevent unconditional legacy COM ports initialization
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84080/comment/c2ad1721_9ec8ccb5?us… :
PS1, Line 18: As a result, this code is being removed and platforms that select
: DRIVERS_UART_8250IO can activate legacy COM ports.
> Are there boards, that need to be adapted?
Based on my understanding the boards that are expected to use Legacy COMs have already selected the correct Kconfig. For example: there are boards that select `DRIVERS_UART_8250IO` config. Ideally we don't need to keep legacy COMs default enabled.
```
src/mainboard/intel/adlrvp/Kconfig:25: select DRIVERS_UART_8250IO
src/mainboard/intel/adlrvp/Kconfig:41: select DRIVERS_UART_8250IO
src/mainboard/intel/adlrvp/Kconfig:70: select DRIVERS_UART_8250IO
```
https://review.coreboot.org/c/coreboot/+/84080/comment/047025e6_725d664d?us… :
PS1, Line 22: TEST=Able to boot google/redrix to OS.
> How can it be checked, that no traffic goes over the IO bus?
eSPI protocol analysis is used to ensure there is no traffic over legacy COMs.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84080?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7a6e38bd151f823d37c07ee89a800489122cc209
Gerrit-Change-Number: 84080
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Mon, 26 Aug 2024 10:14:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Arthur Heymans, Dinesh Gehlot, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Subrata Banik.
Hello Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84080?usp=email
to look at the new patch set (#2).
Change subject: soc/intel/adl: Prevent unconditional legacy COM ports initialization
......................................................................
soc/intel/adl: Prevent unconditional legacy COM ports initialization
This patch eliminates the LPC_IOE_COMA_EN and LPC_IOE_COMB_EN IO enables
from the io_enables variable in the pch_early_iorange_init() function
because lpc_io_setup_comm_a_b() is intended to activate legacy COM
ports like COM-A (0x3F8 - 0x3FF) and COM_B (0x2F8 - 0x2FF).
These COM ports are being activated unconditionally, which is
undesirable for the Intel Alder Lake platform and causes traffic over
the IO bus.
As a result, this code is being removed and platforms that select
DRIVERS_UART_8250IO can activate legacy COM ports.
BUG=b:354066052
TEST=Able to boot google/redrix to the operating system and confirm
that there was no traffic over legacy COMs while being monitored
using the eSPI analyzer.
Change-Id: I7a6e38bd151f823d37c07ee89a800489122cc209
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/alderlake/bootblock/pch.c
1 file changed, 1 insertion(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/84080/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84080?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7a6e38bd151f823d37c07ee89a800489122cc209
Gerrit-Change-Number: 84080
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Julius Werner, Maximilian Brune, Philipp Hug, ron minnich.
Nico Huber has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/84087?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: fit_payload: Use our region API
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Copying from CB:79907 so it's not forgotten:
> It's um, a bit ugly: CB:84087. We don't have an API to manipulate regions
> and the FIT code uses the struct to first only pass some offsets and sizes
> around, patching them until it's a region. Not sure if we should extend the
> API or live with it. For now, I've done everything with region_create().
>
> Also not sure if the data is considered truted. In case of vboot, does the
> CBFS API take care of verification?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84087?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I0b4cbd5bd38832036a765952b6fec2af5e63f541
Gerrit-Change-Number: 84087
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-Attention: Philipp Hug <philipp(a)hug.cx>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: ron minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Mon, 26 Aug 2024 10:07:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Arthur Heymans, Dinesh Gehlot, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Subrata Banik.
Paul Menzel has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84080?usp=email )
Change subject: soc/intel/adl: Prevent unconditional legacy COM ports initialization
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84080/comment/4455e767_b6feee1c?us… :
PS1, Line 22: TEST=Able to boot google/redrix to OS.
How can it be checked, that no traffic goes over the IO bus?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84080?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7a6e38bd151f823d37c07ee89a800489122cc209
Gerrit-Change-Number: 84080
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Mon, 26 Aug 2024 10:05:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Arthur Heymans, Dinesh Gehlot, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Subrata Banik.
Paul Menzel has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84080?usp=email )
Change subject: soc/intel/adl: Prevent unconditional legacy COM ports initialization
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84080/comment/c814f100_9519fe6b?us… :
PS1, Line 18: As a result, this code is being removed and platforms that select
: DRIVERS_UART_8250IO can activate legacy COM ports.
Are there boards, that need to be adapted?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84080?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7a6e38bd151f823d37c07ee89a800489122cc209
Gerrit-Change-Number: 84080
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Mon, 26 Aug 2024 10:04:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Jakub Czapiga, Nicholas Sudsgaard.
Nico Huber has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/84088?usp=email )
Change subject: b64_decode-test: Properly terminate strings before comparison
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84088/comment/0be0c47d_5a051bf5?us… :
PS1, Line 10: encoded
> Do you mean decoded?
No, quite the encoded or `.enc` in the code. In
```
{"QQ==", "A"},
```
the left one, as that seemed more specific. Because the right one is used as
a string in the code, it has an implicit termination.
It may be too much information, though. I could just drop the `encoded' WDYT?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84088?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Id1bd2c3ff06bc1d4e5aa21ddd0f1d5802540999d
Gerrit-Change-Number: 84088
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Comment-Date: Mon, 26 Aug 2024 10:03:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>