Attention is currently required from: Arthur Heymans, Felix Singer, Martin L Roth.
Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80731?usp=email )
Change subject: util/crossgcc: Use ninja to build llvm/clang
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> I also did some test builds using 32 threads and 128 GiB RAM. […]
ninja is not the successor of make. ninja is just meant to be builded. So you usually don't manually write a ninja file, but generate it from some other build system (like CMake or meson). `make` is meant to write manually. Although there is also `automake`, so I guess you can do both with `make`.
This one is a bit old but informative:
https://david.rothlis.net/ninja-benchmark/
--
To view, visit https://review.coreboot.org/c/coreboot/+/80731?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: I66d2a4e6e5c265f5580a74e59309e2ad8b1f1f61
Gerrit-Change-Number: 80731
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Tue, 27 Feb 2024 20:43:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-MessageType: comment
Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80719?usp=email )
(
7 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: soc/intel/tigerlake: Remove IOM Mctp command from TCSS ASL
......................................................................
soc/intel/tigerlake: Remove IOM Mctp command from TCSS ASL
Port fix from Alder Lake to not set/reset IOM MCTP during
D3 cold entry or exit.
Ports 5008d340033d ("soc/intel/adl: Remove IOM Mctp command from TCSS
ASL"):
> Recently as part of s0ix hang issue, it was found that sending IOM
> MCTP command as part of TCSS D3 Cold enter-exit sequence created an
> issue.
> We discovered that due to change in hardware sequence, ADL should not
> set/reset IOM MCTP during D3 cold entry or exit. This patch removes
> the bit setting from ASL file to prevent hang in the system.
> This patch also removes obsolete Pcode mailbox communication which
> is no longer required for ADL.
> BUG=b:220796339
> BRANCH=firmware-brya-14505.B
> TEST=Check if hang issue is resolved with the CL and no other
> regression
> observed
> https://review.coreboot.org/c/coreboot/+/62861
Test: build/boot drobit to Win11. Verify TCSS XHCI power management
working and USB Root Hub doesn't Code 43 in device manager
Change-Id: I40a537fd2b0c821caf282f52aaff1874f54325f1
Signed-off-by: CoolStar <coolstarorganization(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80719
Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com>
Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/soc/intel/tigerlake/acpi/tcss.asl
1 file changed, 1 insertion(+), 142 deletions(-)
Approvals:
build bot (Jenkins): Verified
Paul Menzel: Looks good to me, but someone else must approve
Matt DeVillier: Looks good to me, approved
diff --git a/src/soc/intel/tigerlake/acpi/tcss.asl b/src/soc/intel/tigerlake/acpi/tcss.asl
index 608b464..588ed5e 100644
--- a/src/soc/intel/tigerlake/acpi/tcss.asl
+++ b/src/soc/intel/tigerlake/acpi/tcss.asl
@@ -363,12 +363,6 @@
Offset(0x10),
RBAR, 64 /* RegBar, offset 0x7110 in MCHBAR */
}
- Field (MBAR, DWordAcc, NoLock, Preserve)
- {
- Offset(0x304), /* PRIMDN_MASK1_0_0_0_MCHBAR_IMPH, offset 0x7404 */
- , 31,
- TCD3, 1 /* [31:31] TCSS IN D3 bit */
- }
/*
* Operation region defined to access the pCode mailbox interface. Get the MCHBAR
@@ -405,101 +399,6 @@
}
/*
- * Method to send pCode MailBox command TCSS_DEVEN_MAILBOX_SUBCMD_GET_STATUS
- *
- * Result will be updated in DATA[1:0]
- * DATA[0:0] TCSS_DEVEN_CURRENT_STATE:
- * 0 - TCSS Deven in normal state.
- * 1 - TCSS Deven is cleared by BIOS Mailbox request.
- * DATA[1:1] TCSS_DEVEN_REQUEST_STATUS:
- * 0 - IDLE. TCSS DEVEN has reached its final requested state.
- * 1 - In Progress. TCSS DEVEN is currently in progress of switching state
- * according to given request (bit 0 reflects source state).
- *
- * Return 0x00 - TCSS Deven in normal state
- * 0x01 - TCSS Deven is cleared by BIOS Mailbox request
- * 0x1x - TCSS Deven is in progress of switching state according to given request
- * 0xFE - Command timeout
- * 0xFF - Command corrupt
- */
- Method (DSGS, 0)
- {
- If ((PMBY () == 0)) {
- PMBC = MAILBOX_BIOS_CMD_TCSS_DEVEN_INTERFACE
- PSCM = TCSS_DEVEN_MAILBOX_SUBCMD_GET_STATUS
- PMBR = 1
- If (PMBY () == 0) {
- Local0 = PMBD
- Local1 = PMBC
- Stall (10)
- If ((Local0 != PMBD) || (Local1 != PMBC)) {
- Printf("pCode MailBox is corrupt.")
- Return (0xFF)
- }
- Return (Local0)
- } Else {
- Printf("pCode MailBox is not ready.")
- Return (0xFE)
- }
- } Else {
- Printf("pCode MailBox is not ready.")
- Return (0xFE)
- }
- }
-
- /*
- * Method to send pCode MailBox command TCSS_DEVEN_MAILBOX_SUBCMD_TCSS_CHANGE_REQ
- *
- * Arg0 : 0 - Restore to previously saved value of TCSS DEVEN
- * 1 - Save current TCSS DEVEN value and clear it
- *
- * Return 0x00 - MAILBOX_BIOS_CMD_CLEAR_TCSS_DEVEN command completed
- * 0xFD - Input argument is invalid
- * 0xFE - Command timeout
- * 0xFF - Command corrupt
- */
- Method (DSCR, 1)
- {
- If (Arg0 > 1) {
- Printf("pCode MailBox is corrupt.")
- Return (0xFD)
- }
- If ((PMBY () == 0)) {
- PMBC = MAILBOX_BIOS_CMD_TCSS_DEVEN_INTERFACE
- PSCM = TCSS_DEVEN_MAILBOX_SUBCMD_TCSS_CHANGE_REQ
- PMBD = Arg0
- PMBR = 1
- If ((PMBY () == 0)) {
- Local0 = PMBD
- Local1 = PMBC
- Stall (10)
- If ((Local0 != PMBD) || (Local1 != PMBC)) {
- Printf("pCode MailBox is corrupt.")
- Return (0xFF)
- }
- /* Poll TCSS_DEVEN_REQUEST_STATUS, timeout value is 10ms. */
- Local0 = 0
- While ((DSGS () & 0x10) && (Local0 < 100)) {
- Stall (100)
- Local0++
- }
- If (Local0 == 100) {
- Printf("pCode MailBox is not ready.")
- Return (0xFE)
- } Else {
- Return (0x00)
- }
- } Else {
- Printf("pCode MailBox is not ready.")
- Return (0xFE)
- }
- } Else {
- Printf("pCode MailBox is not ready.")
- Return (0xFE)
- }
- }
-
- /*
* IOM REG BAR Base address is in offset 0x7110 in MCHBAR.
*/
Method (IOMA, 0)
@@ -707,26 +606,7 @@
}
Else
{
- /*
- * Program IOP MCTP Drop (TCSS_IN_D3) after D3 cold exit and
- * acknowledgement by IOM.
- */
- TCD3 = 0
- /*
- * If the TCSS Deven is cleared by BIOS Mailbox request, then
- * restore to previously saved value of TCSS DEVNE.
- */
- Local0 = 0
- While (\_SB.PCI0.TXHC.VDID == 0xFFFFFFFF) {
- If (DSGS () == 1) {
- DSCR (0)
- }
- Local0++
- If (Local0 == 5) {
- Printf("pCode mailbox command failed.")
- Break
- }
- }
+ Printf("TCSS D3 exit.");
}
}
Else {
@@ -743,27 +623,6 @@
Return
}
- /*
- * If the TCSS Deven in normal state, then Save current TCSS DEVEN value and
- * clear it.
- */
- Local0 = 0
- While (\_SB.PCI0.TXHC.VDID != 0xFFFFFFFF) {
- If (DSGS () == 0) {
- DSCR (1)
- }
- Local0++
- If (Local0 == 5) {
- Printf("pCode mailbox command failed.")
- Break
- }
- }
-
- /*
- * Program IOM MCTP Drop (TCSS_IN_D3) in D3Cold entry before entering D3 cold.
- */
- TCD3 = 1
-
/* Request IOM for D3 cold entry sequence. */
TD3C = 1
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/80719?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: I40a537fd2b0c821caf282f52aaff1874f54325f1
Gerrit-Change-Number: 80719
Gerrit-PatchSet: 11
Gerrit-Owner: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-CC: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-MessageType: merged
Attention is currently required from: CoolStar, Nick Vaccaro, Subrata Banik.
Hello Matt DeVillier, Nick Vaccaro, Paul Menzel, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80719?usp=email
to look at the new patch set (#10).
Change subject: soc/intel/tigerlake: Remove IOM Mctp command from TCSS ASL
......................................................................
soc/intel/tigerlake: Remove IOM Mctp command from TCSS ASL
Port fix from Alder Lake to not set/reset IOM MCTP during
D3 cold entry or exit.
Ports 5008d340033d ("soc/intel/adl: Remove IOM Mctp command from TCSS
ASL"):
> Recently as part of s0ix hang issue, it was found that sending IOM
> MCTP command as part of TCSS D3 Cold enter-exit sequence created an
> issue.
> We discovered that due to change in hardware sequence, ADL should not
> set/reset IOM MCTP during D3 cold entry or exit. This patch removes
> the bit setting from ASL file to prevent hang in the system.
> This patch also removes obsolete Pcode mailbox communication which
> is no longer required for ADL.
> BUG=b:220796339
> BRANCH=firmware-brya-14505.B
> TEST=Check if hang issue is resolved with the CL and no other
> regression
> observed
> https://review.coreboot.org/c/coreboot/+/62861
Test: build/boot drobit to Win11. Verify TCSS XHCI power management
working and USB Root Hub doesn't Code 43 in device manager
Change-Id: I40a537fd2b0c821caf282f52aaff1874f54325f1
Signed-off-by: CoolStar <coolstarorganization(a)gmail.com>
---
M src/soc/intel/tigerlake/acpi/tcss.asl
1 file changed, 1 insertion(+), 142 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/80719/10
--
To view, visit https://review.coreboot.org/c/coreboot/+/80719?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: I40a537fd2b0c821caf282f52aaff1874f54325f1
Gerrit-Change-Number: 80719
Gerrit-PatchSet: 10
Gerrit-Owner: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-CC: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-MessageType: newpatchset
Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80718?usp=email )
(
7 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: soc/intel/tigerlake: Fix processor hang while plug unplug of TBT device
......................................................................
soc/intel/tigerlake: Fix processor hang while plug unplug of TBT device
Port 9c348a7b7ea3 ("soc/intel/alderlake: Fix processor hang while plug
unplug of TBT device") from Alder Lake to fix a similar issue present
on Tiger Lake:
> Processor hang is observed while hot plug unplug of TBT device. BIOS
> should execute TBT PCIe RP RTD3 flow based on the value of
> TBT_DMA_CFG_VS_CAP_9[30]. It should skip TBT PCIe RP RTD3 flow, if
> BIT30 in TBT FW version is not set.
> BUG=b:194880254
> https://review.coreboot.org/c/coreboot/+/56503
Change-Id: Ie5409111d4239be86c0b153f01b4fe5fc6af352c
Signed-off-by: CoolStar <coolstarorganization(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80718
Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com>
Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/soc/intel/tigerlake/acpi/tcss.asl
M src/soc/intel/tigerlake/acpi/tcss_dma.asl
2 files changed, 10 insertions(+), 1 deletion(-)
Approvals:
Matt DeVillier: Looks good to me, approved
Paul Menzel: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
diff --git a/src/soc/intel/tigerlake/acpi/tcss.asl b/src/soc/intel/tigerlake/acpi/tcss.asl
index 5af78ed..608b464 100644
--- a/src/soc/intel/tigerlake/acpi/tcss.asl
+++ b/src/soc/intel/tigerlake/acpi/tcss.asl
@@ -567,6 +567,10 @@
/* DMA0 is not in D3Cold now. */
\_SB.PCI0.TDM0.D3CE() /* Enable DMA RTD3 */
+ If (\_SB.PCI0.TDM0.IF30 != 1) {
+ Return
+ }
+
Printf("Push TBT RPs to D3Cold together")
If (\_SB.PCI0.TRP0.VDID != 0xFFFFFFFF) {
/* Put RP0 to D3 cold. */
@@ -622,6 +626,10 @@
/* DMA1 is not in D3Cold now */
\_SB.PCI0.TDM1.D3CE() /* Enable DMA RTD3. */
+ If (\_SB.PCI0.TDM0.IF30 != 1) {
+ Return
+ }
+
Printf("Push TBT RPs to D3Cold together")
If (\_SB.PCI0.TRP2.VDID != 0xFFFFFFFF) {
/* Put RP2 to D3 cold. */
diff --git a/src/soc/intel/tigerlake/acpi/tcss_dma.asl b/src/soc/intel/tigerlake/acpi/tcss_dma.asl
index 9082469..2586ecb 100644
--- a/src/soc/intel/tigerlake/acpi/tcss_dma.asl
+++ b/src/soc/intel/tigerlake/acpi/tcss_dma.asl
@@ -11,7 +11,8 @@
, 6,
PMES, 1, /* 15, PME_STATUS */
Offset(0xC8), /* 0xC8, TBT NVM FW Revision */
- , 31,
+ , 30,
+ IF30, 1, /* ITBT FW Version Bit30 */
INFR, 1, /* TBT NVM FW Ready */
Offset(0xEC), /* 0xEC, TBT TO PCIE Register */
TB2P, 32, /* TBT to PCIe */
--
To view, visit https://review.coreboot.org/c/coreboot/+/80718?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: Ie5409111d4239be86c0b153f01b4fe5fc6af352c
Gerrit-Change-Number: 80718
Gerrit-PatchSet: 9
Gerrit-Owner: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-CC: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-MessageType: merged
Attention is currently required from: Nick Vaccaro, Paul Menzel, Subrata Banik.
CoolStar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80718?usp=email )
Change subject: soc/intel/tigerlake: Fix processor hang while plug unplug of TBT device
......................................................................
Patch Set 8:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80718/comment/8d038fd5_cdf792e7 :
PS7, Line 13: ```Processor hang is observed while hot plug unplug of TBT device. BIOS
: should execute TBT PCIe RP RTD3 flow based on the value of
: TBT_DMA_CFG_VS_CAP_9[30]. It should skip TBT PCIe RP RTD3 flow, if
: BIT30 in TBT FW version is not set.
:
: BUG=b:194880254
:
: https://review.coreboot.org/c/coreboot/+/56503
: ```
> In Markdown citation is done as in email citations: Prepend a `> ` to the lines.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/80718?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: Ie5409111d4239be86c0b153f01b4fe5fc6af352c
Gerrit-Change-Number: 80718
Gerrit-PatchSet: 8
Gerrit-Owner: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-CC: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Tue, 27 Feb 2024 20:38:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: CoolStar, Nick Vaccaro, Subrata Banik.
Hello Matt DeVillier, Nick Vaccaro, Paul Menzel, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80718?usp=email
to look at the new patch set (#8).
Change subject: soc/intel/tigerlake: Fix processor hang while plug unplug of TBT device
......................................................................
soc/intel/tigerlake: Fix processor hang while plug unplug of TBT device
Port 9c348a7b7ea3 ("soc/intel/alderlake: Fix processor hang while plug
unplug of TBT device") from Alder Lake to fix a similar issue present
on Tiger Lake:
> Processor hang is observed while hot plug unplug of TBT device. BIOS
> should execute TBT PCIe RP RTD3 flow based on the value of
> TBT_DMA_CFG_VS_CAP_9[30]. It should skip TBT PCIe RP RTD3 flow, if
> BIT30 in TBT FW version is not set.
> BUG=b:194880254
> https://review.coreboot.org/c/coreboot/+/56503
Change-Id: Ie5409111d4239be86c0b153f01b4fe5fc6af352c
Signed-off-by: CoolStar <coolstarorganization(a)gmail.com>
---
M src/soc/intel/tigerlake/acpi/tcss.asl
M src/soc/intel/tigerlake/acpi/tcss_dma.asl
2 files changed, 10 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/80718/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/80718?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: Ie5409111d4239be86c0b153f01b4fe5fc6af352c
Gerrit-Change-Number: 80718
Gerrit-PatchSet: 8
Gerrit-Owner: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-CC: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Forest Mittelberg, Matt DeVillier.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80712?usp=email )
Change subject: ec/chromeec: Enable auto fan control on startup
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80712?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: I08a8562531f8af0c71230477d0221d536443f096
Gerrit-Change-Number: 80712
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Comment-Date: Tue, 27 Feb 2024 20:35:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80580?usp=email )
Change subject: Kconfig: Make the SEPARATE_ROMSTAGE default configurable in other files
......................................................................
Kconfig: Make the SEPARATE_ROMSTAGE default configurable in other files
This also sets a good default in arch and vboot to have a separate
romstage when it makes sense.
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Change-Id: I09ab5f8c79917bf93c9d5c9dfd157c652478b186
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80580
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
Reviewed-by: Maximilian Brune <maximilian.brune(a)9elements.com>
Reviewed-by: Jérémy Compostella <jeremy.compostella(a)intel.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/Kconfig
M src/arch/x86/Kconfig
M src/security/vboot/Kconfig
3 files changed, 6 insertions(+), 3 deletions(-)
Approvals:
Jérémy Compostella: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
Nico Huber: Looks good to me, but someone else must approve
Maximilian Brune: Looks good to me, approved
diff --git a/src/Kconfig b/src/Kconfig
index 2bcc3ce..2afb9db 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -245,7 +245,6 @@
config SEPARATE_ROMSTAGE
bool "Build a separate romstage"
- default y
help
Build a separate romstage that is loaded by bootblock. With this
option disabled the romstage sources are linked inside the bootblock
@@ -1538,3 +1537,6 @@
bool
default n if RAMPAYLOAD
default y
+
+config SEPARATE_ROMSTAGE
+ default y
diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig
index 0c11653..63f71e5 100644
--- a/src/arch/x86/Kconfig
+++ b/src/arch/x86/Kconfig
@@ -251,8 +251,9 @@
bool "Always load fallback"
config BOOTBLOCK_NORMAL
- select CONFIGURABLE_CBFS_PREFIX
bool "Switch to normal if CMOS says so"
+ select CONFIGURABLE_CBFS_PREFIX
+ select SEPARATE_ROMSTAGE
endchoice
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig
index 4bd36f5..d42dc74 100644
--- a/src/security/vboot/Kconfig
+++ b/src/security/vboot/Kconfig
@@ -90,7 +90,7 @@
config VBOOT_STARTS_IN_BOOTBLOCK
bool
default n
- depends on SEPARATE_ROMSTAGE
+ select SEPARATE_ROMSTAGE
help
Firmware verification happens during the end of or right after the
bootblock. This implies that a static VBOOT2_WORK() buffer must be
--
To view, visit https://review.coreboot.org/c/coreboot/+/80580?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: I09ab5f8c79917bf93c9d5c9dfd157c652478b186
Gerrit-Change-Number: 80580
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Alper Nebi Yasak.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80365?usp=email )
Change subject: mb/qemu/fw_cfg: Support using DMA to select fw_cfg file
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
hi Alper, can you address the unresolved comments so we can get this patch merged?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80365?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: I46f9915e6df04d371c7084815f16034c7e9879d4
Gerrit-Change-Number: 80365
Gerrit-PatchSet: 2
Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Comment-Date: Tue, 27 Feb 2024 20:27:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/77338?usp=email )
Change subject: device/pciexp_device.c: Fix setting Max Payload Size
......................................................................
device/pciexp_device.c: Fix setting Max Payload Size
Current implementation assumes that the endpoint device is connected
directly to the PCIe Root Port, which does not always have to be true.
In a case where there is a PCIe switch between the endpoint and the
root port, the Max Payload Size capability may differ across the
devices in the chain and coreboot will not set a correct Max Payload
Size. This results in a PCIe device malfunction in pre-OS environment,
e.g. if the Ethernet NICs are connected behind a PCIe switch, the iPXE
fails to obtain the DHCP configuration.
Fix this by traversing the topology and programming the highest common
Max Payload Size in the given PCIe device chain during enumeration.
Once finished, the root port has the highest common Max Payload Size
supported by all the devices in the chain. So at the end of root port
bus scan, propagate the root port's Max Payload Size to all downstream
devices to keep Max Payload Size in sync within the whole chain.
TEST=Perform successful dhcp command in iPXE on the NIC connected to
the PCIe root port via ASMedia ASM1806 PCIe switch and again on the
NIC connected directly to the PCIe root port.
Change-Id: I24386dc208363b7d94fea46dec25c231a3968225
Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/77338
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Krystian Hebel <krystian.hebel(a)3mdeb.com>
---
M src/device/pciexp_device.c
1 file changed, 134 insertions(+), 29 deletions(-)
Approvals:
build bot (Jenkins): Verified
Krystian Hebel: Looks good to me, approved
diff --git a/src/device/pciexp_device.c b/src/device/pciexp_device.c
index 969dbb00..db351ef 100644
--- a/src/device/pciexp_device.c
+++ b/src/device/pciexp_device.c
@@ -144,6 +144,35 @@
return from;
}
+static bool pcie_is_root_port(struct device *dev)
+{
+ unsigned int pcie_pos, pcie_type;
+
+ pcie_pos = pci_find_capability(dev, PCI_CAP_ID_PCIE);
+ if (!pcie_pos)
+ return false;
+
+ pcie_type = pci_read_config16(dev, pcie_pos + PCI_EXP_FLAGS) & PCI_EXP_FLAGS_TYPE;
+ pcie_type >>= 4;
+
+ return (pcie_type == PCI_EXP_TYPE_ROOT_PORT);
+}
+
+static bool pcie_is_endpoint(struct device *dev)
+{
+ unsigned int pcie_pos, pcie_type;
+
+ pcie_pos = pci_find_capability(dev, PCI_CAP_ID_PCIE);
+ if (!pcie_pos)
+ return false;
+
+ pcie_type = pci_read_config16(dev, pcie_pos + PCI_EXP_FLAGS) & PCI_EXP_FLAGS_TYPE;
+ pcie_type >>= 4;
+
+ return ((pcie_type == PCI_EXP_TYPE_ENDPOINT) || (pcie_type == PCI_EXP_TYPE_LEG_END));
+}
+
+
/*
* Re-train a PCIe link
*/
@@ -561,26 +590,63 @@
printk(BIOS_INFO, "ASPM: Enabled %s\n", aspm_type_str[apmc]);
}
-/*
- * Set max payload size of endpoint in accordance with max payload size of root port.
- */
-static void pciexp_set_max_payload_size(struct device *root, unsigned int root_cap,
- struct device *endp, unsigned int endp_cap)
+static void pciexp_dev_set_max_payload_size(struct device *dev, unsigned int max_payload)
{
- unsigned int endp_max_payload, root_max_payload, max_payload;
- u16 endp_devctl, root_devctl;
- u32 endp_devcap, root_devcap;
+ u16 devctl;
+ unsigned int pcie_cap = pci_find_capability(dev, PCI_CAP_ID_PCIE);
- /* Get max payload size supported by endpoint */
- endp_devcap = pci_read_config32(endp, endp_cap + PCI_EXP_DEVCAP);
- endp_max_payload = endp_devcap & PCI_EXP_DEVCAP_PAYLOAD;
+ if (!pcie_cap)
+ return;
- /* Get max payload size supported by root port */
- root_devcap = pci_read_config32(root, root_cap + PCI_EXP_DEVCAP);
- root_max_payload = root_devcap & PCI_EXP_DEVCAP_PAYLOAD;
+ devctl = pci_read_config16(dev, pcie_cap + PCI_EXP_DEVCTL);
+ devctl &= ~PCI_EXP_DEVCTL_PAYLOAD;
+ /*
+ * Should never overflow to higher bits, due to how max_payload is
+ * guarded in this file.
+ */
+ devctl |= max_payload << 5;
+ pci_write_config16(dev, pcie_cap + PCI_EXP_DEVCTL, devctl);
+}
- /* Set max payload to smaller of the reported device capability. */
- max_payload = MIN(endp_max_payload, root_max_payload);
+static unsigned int pciexp_dev_get_current_max_payload_size(struct device *dev)
+{
+ u16 devctl;
+ unsigned int pcie_cap = pci_find_capability(dev, PCI_CAP_ID_PCIE);
+
+ if (!pcie_cap)
+ return 0;
+
+ devctl = pci_read_config16(dev, pcie_cap + PCI_EXP_DEVCTL);
+ devctl &= PCI_EXP_DEVCTL_PAYLOAD;
+ return (devctl >> 5);
+}
+
+static unsigned int pciexp_dev_get_max_payload_size_cap(struct device *dev)
+{
+ u16 devcap;
+ unsigned int pcie_cap = pci_find_capability(dev, PCI_CAP_ID_PCIE);
+
+ if (!pcie_cap)
+ return 0;
+
+ devcap = pci_read_config16(dev, pcie_cap + PCI_EXP_DEVCAP);
+ return (devcap & PCI_EXP_DEVCAP_PAYLOAD);
+}
+
+/*
+ * Set max payload size of a parent based on max payload size capability of the child.
+ */
+static void pciexp_configure_max_payload_size(struct device *parent, struct device *child)
+{
+ unsigned int child_max_payload, parent_max_payload, max_payload;
+
+ /* Get max payload size supported by child */
+ child_max_payload = pciexp_dev_get_current_max_payload_size(child);
+ /* Get max payload size configured by parent */
+ parent_max_payload = pciexp_dev_get_current_max_payload_size(parent);
+ /* Set max payload to smaller of the reported device capability or parent config. */
+ max_payload = MIN(child_max_payload, parent_max_payload);
+
if (max_payload > 5) {
/* Values 6 and 7 are reserved in PCIe 3.0 specs. */
printk(BIOS_ERR, "PCIe: Max_Payload_Size field restricted from %d to 5\n",
@@ -588,17 +654,11 @@
max_payload = 5;
}
- endp_devctl = pci_read_config16(endp, endp_cap + PCI_EXP_DEVCTL);
- endp_devctl &= ~PCI_EXP_DEVCTL_PAYLOAD;
- endp_devctl |= max_payload << 5;
- pci_write_config16(endp, endp_cap + PCI_EXP_DEVCTL, endp_devctl);
-
- root_devctl = pci_read_config16(root, root_cap + PCI_EXP_DEVCTL);
- root_devctl &= ~PCI_EXP_DEVCTL_PAYLOAD;
- root_devctl |= max_payload << 5;
- pci_write_config16(root, root_cap + PCI_EXP_DEVCTL, root_devctl);
-
- printk(BIOS_INFO, "PCIe: Max_Payload_Size adjusted to %d\n", (1 << (max_payload + 7)));
+ if (max_payload != parent_max_payload) {
+ pciexp_dev_set_max_payload_size(parent, max_payload);
+ printk(BIOS_INFO, "%s: Max_Payload_Size adjusted to %d\n", dev_path(parent),
+ (1 << (max_payload + 7)));
+ }
}
/*
@@ -658,19 +718,47 @@
if (CONFIG(PCIEXP_LANE_ERR_STAT_CLEAR))
clear_lane_error_status(root);
- /* Adjust Max_Payload_Size of link ends. */
- pciexp_set_max_payload_size(root, root_cap, dev, cap);
+ /* Set the Max Payload Size to the maximum supported capability for this device */
+ if (pcie_is_endpoint(dev))
+ pciexp_dev_set_max_payload_size(dev, pciexp_dev_get_max_payload_size_cap(dev));
+
+ /* Limit the parent's Max Payload Size if needed */
+ pciexp_configure_max_payload_size(root, dev);
pciexp_configure_ltr(root, root_cap, dev, cap);
}
+static void pciexp_sync_max_payload_size(struct bus *bus, unsigned int max_payload)
+{
+ struct device *child;
+
+ /* Set the max payload for children on the bus and their children, etc. */
+ for (child = bus->children; child; child = child->sibling) {
+ if (!is_pci(child))
+ continue;
+
+ pciexp_dev_set_max_payload_size(child, max_payload);
+
+ if (child->downstream)
+ pciexp_sync_max_payload_size(child->downstream, max_payload);
+ }
+}
+
void pciexp_scan_bus(struct bus *bus, unsigned int min_devfn,
unsigned int max_devfn)
{
struct device *child;
+ unsigned int max_payload;
pciexp_enable_ltr(bus->dev);
+ /*
+ * Set the Max Payload Size to the maximum supported capability for this bridge.
+ * This value will be used in pciexp_tune_dev to limit the Max Payload size if needed.
+ */
+ max_payload = pciexp_dev_get_max_payload_size_cap(bus->dev);
+ pciexp_dev_set_max_payload_size(bus->dev, max_payload);
+
pci_scan_bus(bus, min_devfn, max_devfn);
for (child = bus->children; child; child = child->sibling) {
@@ -682,6 +770,23 @@
}
pciexp_tune_dev(child);
}
+
+ /*
+ * Now the root port's Max Payload Size should be set to the highest
+ * possible value supported by all devices under a given root port.
+ * Propagate that value down from root port to all devices, so the Max
+ * Payload Size is equal on all devices, as some devices may have
+ * different capabilities and the programmed value depends on the
+ * order of device population the in the subtree.
+ */
+ if (pcie_is_root_port(bus->dev)) {
+ max_payload = pciexp_dev_get_current_max_payload_size(bus->dev);
+
+ printk(BIOS_INFO, "%s: Setting Max_Payload_Size to %d for devices under this"
+ " root port\n", dev_path(bus->dev), 1 << (max_payload + 7));
+
+ pciexp_sync_max_payload_size(bus, max_payload);
+ }
}
void pciexp_scan_bridge(struct device *dev)
--
To view, visit https://review.coreboot.org/c/coreboot/+/77338?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: I24386dc208363b7d94fea46dec25c231a3968225
Gerrit-Change-Number: 77338
Gerrit-PatchSet: 22
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-MessageType: merged