Attention is currently required from: Arthur Heymans, Sridhar Siricilla.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63734 )
Change subject: soc/intel/common: Fix buggy code tries to access DESC region
......................................................................
Patch Set 6: Code-Review+2
(1 comment)
File src/soc/intel/common/basecode/debug/debug_feature.c:
https://review.coreboot.org/c/coreboot/+/63734/comment/dea4bc42_da417392
PS6, Line 26: boot_device_ro
actually does boot_device_rw() not work? Then you only need to change one line.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63734
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib144cc0845b7527e5a3032529b0802f961944b87
Gerrit-Change-Number: 63734
Gerrit-PatchSet: 6
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Comment-Date: Thu, 21 Apr 2022 20:51:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Subrata Banik, Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Lean Sheng Tan, Eric Lai, Werner Zeh.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63640 )
Change subject: soc/intel/common/smbus: Add `finalize` operation for smbus
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/63640
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie945680049514e6c5d797790a381a6946e836926
Gerrit-Change-Number: 63640
Gerrit-PatchSet: 6
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Thu, 21 Apr 2022 20:40:21 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Subrata Banik, Tim Wawrzynczak, Angel Pons, Arthur Heymans, Nick Vaccaro, Lean Sheng Tan, Eric Lai, Werner Zeh.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63640 )
Change subject: soc/intel/common/smbus: Add `finalize` operation for smbus
......................................................................
Patch Set 6:
(1 comment)
File src/soc/intel/common/block/smbus/smbus.c:
https://review.coreboot.org/c/coreboot/+/63640/comment/7e24f2e6_8c8394a4
PS5, Line 77: tco_lockdown();
> > > > > t's important to ensure that TCO base address is locked before running 3rd party code
> > > >
> > > > Then it's already too late? running option ROMs happens during PCI device initialization (.init), so by .final, it's already happened.
> > >
> > > Valid point Tim, IMO, we should bind this with a bootstate which is equivalent to (.enable) entry. WDYT ?
> >
> > That would be an ubiquitous problem in coreboot... Option roms from external PCI devices are just an insecure setup and should be disabled by default. CB:63763
>
> Thats very nice @Arthur, but can we say, `no` to launching option rom, atleast few DGPU I have worked during my time at Intel, I have to use Option ROM for pre-os display.
>
> Reg this CL, are you suggesting, we don't need to protect TCOBASE from 3rd party code, then we don't need to worry?
My argument is that your setup is inherently insecure if you use external option roms (without yabel). There a lot of other instances with .final that lock down things that would need to be moved to deal with that. So I'm thinking your change is indeed fine as is.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63640
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie945680049514e6c5d797790a381a6946e836926
Gerrit-Change-Number: 63640
Gerrit-PatchSet: 6
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Thu, 21 Apr 2022 20:39:56 +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: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Wonkyu Kim, Paul Menzel, Angel Pons, Michael Niewöhner.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63615 )
Change subject: intel/common/../systemagent: Add sa_get_mchbar_base api
......................................................................
Patch Set 12:
(4 comments)
Patchset:
PS12:
> I commented yesterday: […]
I agree, I think this belongs in some chipset specific code, not in common code, if MTL has different requirements than others
File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/63615/comment/5dfd2530_fa8b8d66
PS12, Line 87: 4
nit:
`sizeof(uint32_t)`
https://review.coreboot.org/c/coreboot/+/63615/comment/9ddec93e_2af6380f
PS12, Line 89: 0x1
used here and line 97, how about a
`#define MCHBAREN 0x1`
?
https://review.coreboot.org/c/coreboot/+/63615/comment/d80a4beb_060d5f5c
PS12, Line 95: 4
nit:
`sizeof(uint32_t)`
--
To view, visit https://review.coreboot.org/c/coreboot/+/63615
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie4c7af3ea8c2b2b6afcc76e1165fadbe15e0bceb
Gerrit-Change-Number: 63615
Gerrit-PatchSet: 12
Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Thu, 21 Apr 2022 20:07:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Stefan Ott, Felix Singer, Patrick Rudolph, Jonathan Zhang, Nick Vaccaro, Arthur Heymans, Andrey Petrov, Piotr Król, Jes Klinke, Nico Huber, Sean Rhodes, Michał Żygowski, Johnny Lin, Christian Walter, Werner Zeh, Alexander Couzens, Tim Chu, Frans Hendriks, Tristan Corrick, Jeremy Soller, Julius Werner, Angel Pons, Michael Niewöhner, Tim Crawford, Maxim Polyakov.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63424 )
Change subject: tpm: Refactor TPM Kconfig dimensions
......................................................................
Patch Set 20: Code-Review+2
(1 comment)
Patchset:
PS20:
LGTM, this is nice cleanup, thanks Jes! 😊
--
To view, visit https://review.coreboot.org/c/coreboot/+/63424
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4656b2b90363b8dfd008dc281ad591862fe2cc9e
Gerrit-Change-Number: 63424
Gerrit-PatchSet: 20
Gerrit-Owner: Jes Klinke <jbk(a)chromium.org>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Stefan Ott <coreboot(a)desire.ch>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Tristan Corrick <tristan(a)corrick.kiwi>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: siemens-bot
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Stefan Ott <coreboot(a)desire.ch>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Attention: Jes Klinke <jbk(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Attention: Tristan Corrick <tristan(a)corrick.kiwi>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Comment-Date: Thu, 21 Apr 2022 20:02:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Matt DeVillier, Paul Menzel, Angel Pons.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63749 )
Change subject: mb/purism/librem_mini: Rework front status LED to show all disk activity
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/63749
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I34c2a5f3fd1038266f4514544abfc1020da6f85b
Gerrit-Change-Number: 63749
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 21 Apr 2022 19:52:39 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
Hello Arthur Heymans, build bot (Jenkins), Rizwan Qureshi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/63734
to look at the new patch set (#6).
Change subject: soc/intel/common: Fix buggy code tries to access DESC region
......................................................................
soc/intel/common: Fix buggy code tries to access DESC region
The patch fixes the buggy code which tries to access the Descriptor
Region. The existing code doesn't use correct APIs to access the
Descriptor Region. Hence, error message is getting displayed during
the boot.
BUG=b:229003612
TEST=Build and verify no errors seen while accessing the Descriptor
Region.
Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Change-Id: Ib144cc0845b7527e5a3032529b0802f961944b87
---
M src/soc/intel/common/basecode/debug/debug_feature.c
1 file changed, 3 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/63734/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/63734
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib144cc0845b7527e5a3032529b0802f961944b87
Gerrit-Change-Number: 63734
Gerrit-PatchSet: 6
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans.
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63734 )
Change subject: soc/intel/common: Fix buggy code tries to access DESC region
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS4:
> > SI_DESC is going to be remain in future platforms as well. […]
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/63734
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib144cc0845b7527e5a3032529b0802f961944b87
Gerrit-Change-Number: 63734
Gerrit-PatchSet: 5
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 21 Apr 2022 18:03:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur.heymans(a)9elements.com>
Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment