coreboot-gerrit April 2022

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

Change in coreboot[master]: soc/intel: Drop copy-pasted `MSR_VR_MISC_CONFIG2` define
by Jeff Daly (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Subrata Banik, Mariusz Szafrański, Suresh Bellampalli, Vanessa Eusebio, Angel Pons, Arthur Heymans. Jeff Daly has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63367 ) Change subject: soc/intel: Drop copy-pasted `MSR_VR_MISC_CONFIG2` define ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: fine for DNV. -- To view, visit https://review.coreboot.org/c/coreboot/+/63367 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1b203aa243ab97e3569f7347b424dd72318bd65f Gerrit-Change-Number: 63367 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Reviewer: Mariusz Szafrański <mariuszx.szafranski(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Mariusz Szafrański <mariuszx.szafranski(a)intel.com> Gerrit-Attention: Suresh Bellampalli <suresh.bellampalli(a)intel.com> Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Mon, 11 Apr 2022 15:59:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: use gpmr api in common drivers
by Subrata Banik (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Wonkyu Kim, Ravishankar Sarawadi, Angel Pons, Nick Vaccaro, Tim Wawrzynczak. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63471 ) Change subject: soc/intel/common: use gpmr api in common drivers ...................................................................... Patch Set 6: (1 comment) File src/soc/intel/common/pch/lockdown/lockdown.c: https://review.coreboot.org/c/coreboot/+/63471/comment/6ec2042c_5fc8fb79 PS6, Line 29: { : /* : * GCS reg of DMI : * : * When set, prevents GCS.BBS from being changed : * GCS.BBS: (Boot BIOS Strap) This field determines the destination : * of accesses to the BIOS memory range. : * Bits Description : * "0b": SPI : * "1b": LPC/eSPI : */ > I removed this as it's DMI specific. Do you want to keep it and update later? yeah remove the DMI words bt in general the purpose, you can keep if you think it can add value. -- To view, visit https://review.coreboot.org/c/coreboot/+/63471 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I00ac667e8d3f2ccefd8d51a8150a989fc8e5c7e2 Gerrit-Change-Number: 63471 Gerrit-PatchSet: 6 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 15:56:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Wonkyu Kim <wonkyu.kim(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/teleplatforms/D4E4S16P8: Remove extra highspeed pad configurations
by Jeff Daly (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Dmitry Ponamorev, Angel Pons, Kyösti Mälkki. Jeff Daly has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60092 ) Change subject: mb/teleplatforms/D4E4S16P8: Remove extra highspeed pad configurations ...................................................................... Patch Set 5: Code-Review+1 (1 comment) Patchset: PS5: if there's only ever a 20-lane SKU on the platform, this is fine. -- To view, visit https://review.coreboot.org/c/coreboot/+/60092 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64a705ed3cb1d787031818e50c3f9dd8d7f1d8ed Gerrit-Change-Number: 60092 Gerrit-PatchSet: 5 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Dmitry Ponamorev <dponamorev(a)gmail.com> Gerrit-Reviewer: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dmitry Ponamorev <dponamorev(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 15:52:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: use gpmr api in common drivers
by Wonkyu Kim (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Subrata Banik, Ravishankar Sarawadi, Angel Pons, Nick Vaccaro, Tim Wawrzynczak. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63471 ) Change subject: soc/intel/common: use gpmr api in common drivers ...................................................................... Patch Set 6: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63471/comment/613c368d_2876eefa PS6, Line 9: Move GPMR(General Purpose Memory Range) APIs to gpmr driver from dmi.c : For this, 3 patches are used. : 1. Add GPMR common driver in IA common code(CB:63170) : 2. Migrate all DMI API usage to GPMR(CB:63471) : 3. Drop DMI driver (CB:63472) > can you please write what this CL does, rather than the plan. […] Ack File src/soc/intel/common/pch/lockdown/lockdown.c: https://review.coreboot.org/c/coreboot/+/63471/comment/85e493aa_ec4cca59 PS6, Line 29: { : /* : * GCS reg of DMI : * : * When set, prevents GCS.BBS from being changed : * GCS.BBS: (Boot BIOS Strap) This field determines the destination : * of accesses to the BIOS memory range. : * Bits Description : * "0b": SPI : * "1b": LPC/eSPI : */ > can we keep these comments ? I removed this as it's DMI specific. Do you want to keep it and update later? https://review.coreboot.org/c/coreboot/+/63471/comment/5b780056_eff3ab65 PS6, Line 28: dmi > u should change this prefix as well? isn't it ? Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/63471 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I00ac667e8d3f2ccefd8d51a8150a989fc8e5c7e2 Gerrit-Change-Number: 63471 Gerrit-PatchSet: 6 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(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: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 15:50:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: intel/common/block: Add gpmr common driver
by Subrata Banik (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Nico Huber, Jamie Ryu, Wonkyu Kim, Ethan Tsao, Ravishankar Sarawadi, Angel Pons, Nick Vaccaro, Michael Niewöhner, Tim Wawrzynczak. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63170 ) Change subject: intel/common/block: Add gpmr common driver ...................................................................... Patch Set 14: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/63170/comment/db2d4927_7207db6e PS14, Line 9: Move GPMR(General Purpose Memory Range) APIs to gpmr driver from dmi.c : For this, 3 patches are used. : 1. Add GPMR common driver in IA common code(CB:63170) : 2. Migrate all DMI API usage to GPMR(CB:63471) : 3. Drop DMI driver (CB:63472) : > Agreed. Let me re-arrange patches to utilize rename. > I think it's better start from move folder first from dmi to gpmr folder without any code change and start migrating into gpmr driver yeah, I think lets just rename at first without changing the API name, that way we maintain the git history and later all other API name change and SoC code clean up. -- To view, visit https://review.coreboot.org/c/coreboot/+/63170 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d57f4b8bd06e0cf6c9afa4baf4a7bed64ecb56b Gerrit-Change-Number: 63170 Gerrit-PatchSet: 14 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 15:46:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Wonkyu Kim <wonkyu.kim(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: intel/common/block: Add gpmr common driver
by Wonkyu Kim (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Nico Huber, Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Angel Pons, Nick Vaccaro, Michael Niewöhner, Tim Wawrzynczak. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63170 ) Change subject: intel/common/block: Add gpmr common driver ...................................................................... Patch Set 14: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/63170/comment/548e9b5a_6bf09779 PS14, Line 9: Move GPMR(General Purpose Memory Range) APIs to gpmr driver from dmi.c : For this, 3 patches are used. : 1. Add GPMR common driver in IA common code(CB:63170) : 2. Migrate all DMI API usage to GPMR(CB:63471) : 3. Drop DMI driver (CB:63472) : > It's indeed helpful to make use of Git's rename detection. Not obvious right […] Agreed. Let me re-arrange patches to utilize rename. I think it's better start from move folder first from dmi to gpmr folder without any code change and start migrating into gpmr driver -- To view, visit https://review.coreboot.org/c/coreboot/+/63170 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d57f4b8bd06e0cf6c9afa4baf4a7bed64ecb56b Gerrit-Change-Number: 63170 Gerrit-PatchSet: 14 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 15:42:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: arch/x86/postcar_loader: Correct off-by-one of MTRR end address in log
by Subrata Banik (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Paul Menzel, Tim Wawrzynczak, Angel Pons, Werner Zeh. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63541 ) Change subject: arch/x86/postcar_loader: Correct off-by-one of MTRR end address in log ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/63541/comment/075a6e94_794b4dbd PS2, Line 12: wrong > Thanks Angel. I wasn't aware of the other places but it makes sense to me having all of them consistently. I will refactor and provide a new version of this CL. here is postcar debug print without Werner's cl: [DEBUG] MTRR Range: Start=76000000 End=77000000 (Size 1000000) [DEBUG] MTRR Range: Start=7b800000 End=7c000000 (Size 800000) [DEBUG] MTRR Range: Start=f9000000 End=fa000000 (Size 1000000) [DEBUG] MTRR Range: Start=ff000000 End=0 (Size 1000000) [DEBUG] Normal boot MTRR snapshot [DEBUG] 0x00000000ff000005: PHYBASE0: Address = 0x00000000ff000000, WP [DEBUG] 0x00003fffff000800: PHYMASK0: Length = 0x0000000001000000, Valid [DEBUG] 0x00000000f9000005: PHYBASE1: Address = 0x00000000f9000000, WP [DEBUG] 0x00003fffff000800: PHYMASK1: Length = 0x0000000001000000, Valid [DEBUG] 0x000000007b800006: PHYBASE2: Address = 0x000000007b800000, WB [DEBUG] 0x00003fffff800800: PHYMASK2: Length = 0x0000000000800000, Valid [DEBUG] 0x0000000076000006: PHYBASE3: Address = 0x0000000076000000, WB [DEBUG] 0x00003fffff000800: PHYMASK3: Length = 0x0000000001000000, Valid [DEBUG] 0x0000000000000000: PHYBASE4 [DEBUG] 0x0000000000000000: PHYMASK4: Disabled [DEBUG] 0x0000000000000000: PHYBASE5 [DEBUG] 0x0000000000000000: PHYMASK5: Disabled [DEBUG] 0x0000000000000000: PHYBASE6 [DEBUG] 0x0000000000000000: PHYMASK6: Disabled [DEBUG] 0x0000000000000000: PHYBASE7 [DEBUG] 0x0000000000000000: PHYMASK7: Disabled [DEBUG] 0x0000000000000000: PHYBASE8 [DEBUG] 0x0000000000000000: PHYMASK8: Disabled [DEBUG] 0x0000000000000000: PHYBASE9 [DEBUG] 0x0000000000000000: PHYMASK9: Disabled here is postcar debug print with Werner's CL [DEBUG] MTRR Range: Start=76000000 End=76ffffff (Size 1000000) [DEBUG] MTRR Range: Start=7b800000 End=7bffffff (Size 800000) [DEBUG] MTRR Range: Start=f9000000 End=f9ffffff (Size 1000000) [DEBUG] MTRR Range: Start=ff000000 End=ffffffff (Size 1000000) This code change make sense to me as the MTRR start and end range is now proper where previously both the start and end were inclusive. ideally end should be always N-1. -- To view, visit https://review.coreboot.org/c/coreboot/+/63541 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ca292f9cf272564cb5ef1c4ea38f5c483605c94 Gerrit-Change-Number: 63541 Gerrit-PatchSet: 2 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Angel Pons <th3fanbus(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: 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-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 15:39:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Enable rom cache on all CPU threads
by Arthur Heymans (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Subrata Banik, Kane Chen, Arthur Heymans. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63486 ) Change subject: soc/intel/common: Enable rom cache on all CPU threads ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > I'm not sure if i get Arthur's question. > > The issue is when bsp asks all threads to run _x86_setup_mtrrs. > BSP won't wait all threads to be finished. BSP (core 0 Thread0) just keeps exe post_cpus_add_romcache. > > > And then Core 0 thread 1, finally finishes _x86_setup_mtrrs, but it's after post_cpus_add_romcache run by BSP Core 0 thread 0. > the mtrr is share btwn Core 0 thread 0 and Core 0 thread 1. > > > So eventually post_cpus_add_romcache doesn't work as expect and spi address are not actually put in var mtrr since _x86_setup_mtrrs run by Core 0 thread 1 overwrite everything again. > > thanks. Right. I suggested adding a dummy call on all APs to make sure they finished setting up MTRRs before setting the temp MTRR on the BSP. Did that work? -- To view, visit https://review.coreboot.org/c/coreboot/+/63486 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1d7ffc6e5f5ec49abf848d3cd9f0435c93f834dc Gerrit-Change-Number: 63486 Gerrit-PatchSet: 3 Gerrit-Owner: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.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-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Mon, 11 Apr 2022 15:22:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur.heymans(a)9elements.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: Reset XHCI controller while preparing for S5
by Subrata Banik (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Tim Wawrzynczak. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63552 ) Change subject: mb/google/brya: Reset XHCI controller while preparing for S5 ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/63552/comment/a58f26a8_0c6adc64 PS1, Line 8: : This patch calls into `xhci_host_reset()` function to perform XHCI : controller reset. > Can you describe the problem here? Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/63552 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibf06a64f055a0cee3659b410652082f31e18e149 Gerrit-Change-Number: 63552 Gerrit-PatchSet: 2 Gerrit-Owner: 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-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Mon, 11 Apr 2022 15:02:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: Reset XHCI controller while preparing for S5
by Subrata Banik (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Subrata Banik. Hello build bot (Jenkins), Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63552 to look at the new patch set (#2). Change subject: mb/google/brya: Reset XHCI controller while preparing for S5 ...................................................................... mb/google/brya: Reset XHCI controller while preparing for S5 This patch calls into `xhci_host_reset()` function to perform XHCI controller reset. Without this patch the PMC IPC timeout issue is seen while sending the USB-C (0xA7) command during resume from S5 (S5->S4->S3->S0). On Brya variants, resume from S5 state results in PMC error while sending PMC IPC (0xA7) to USB-C active ports, log here: localhost ~ # cbmem -c | grep ERROR [ERROR] PMC IPC timeout after 1000 ms [ERROR] PMC IPC command 0x200a7 failed [ERROR] pmc_send_ipc_cmd failed [ERROR] Failed to setup port:0 to initial state [ERROR] PMC IPC timeout after 1000 ms [ERROR] PMC IPC command 0x200a7 failed [ERROR] pmc_send_ipc_cmd failed [ERROR] Failed to setup port:1 to initial state [ERROR] PMC IPC timeout after 1000 ms [ERROR] PMC IPC command 0x20a0 failed This problem is not seen while resuming from G3 (G3->S5->S4->S3->S0). Upon resume the state of USB ports are not the same between S5 and G3 and it appears that the active USB port still is in U3 (suspend) while PMC tries to send the IPC command, finally, results into timeout. This patch utilises the S5 SMI handler to reset the XHCI controller using `xhci_host_reset()` prior entering into the S5, it helps to restore the port state to active hence, no PMC timeout is seen with this code change. BUG=b:227289581 TEST=No PMC timeout is observed while sending USB-C PMC command (0xA7) during resume from S5. Total Time: 1,045,855 localhost ~ # cbmem -c | grep ERROR No PMC timeout error is observed with this CL. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: Ibf06a64f055a0cee3659b410652082f31e18e149 --- M src/mainboard/google/brya/smihandler.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/63552/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63552 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibf06a64f055a0cee3659b410652082f31e18e149 Gerrit-Change-Number: 63552 Gerrit-PatchSet: 2 Gerrit-Owner: 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-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2022