Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35499 )
Change subject: sc7180: Add QUPv3 FW load & config
......................................................................
Patch Set 41:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35499/40/src/soc/qualcomm/sc7180/q…
File src/soc/qualcomm/sc7180/qupv3_config.c:
https://review.coreboot.org/c/coreboot/+/35499/40/src/soc/qualcomm/sc7180/q…
PS40, Line 53:
One more thing I just noticed, it would be good to have *some* kind of sanity checking here to make sure the file we have is not complete garbage before we start trying to use it. It looks like you have a magic number ('SEFW') at the top of the header, can you check that? (If there's some way to check that the protocol matches, that would be even better.)
--
To view, visit https://review.coreboot.org/c/coreboot/+/35499
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4d91dd10488931247f81a87b0bdcc598f4bceb31
Gerrit-Change-Number: 35499
Gerrit-PatchSet: 41
Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org>
Gerrit-Reviewer: Doug Anderson <dianders(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org>
Gerrit-CC: Akash Asthana <akashast(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Roja Rani Yarubandi <c_rojay(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Satya Priya Kakitapalli <c_skakit(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Stephen Boyd <swboyd(a)chromium.org>
Gerrit-Comment-Date: Thu, 05 Mar 2020 02:01:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39273
to look at the new patch set (#2).
Change subject: mb/google/hatch/puff: Enable VBOOT_EC_EFS
......................................................................
mb/google/hatch/puff: Enable VBOOT_EC_EFS
If the ChromeOS EC uses EC early firmware selection (EFS), the AP vboot
build must also enable EC EFS. Puff EC uses EFS, so enable it in the AP
vboot build.
BUG=b:150742950
TEST=Puff can boot with EC EFS with hardware write protect enabled
BRANCH=none
Signed-off-by: Sam McNally <sammc(a)chromium.org>
Change-Id: I0877000b7d277106436831f2d69775c25299da9e
---
M src/mainboard/google/hatch/Kconfig.name
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/39273/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/39273
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0877000b7d277106436831f2d69775c25299da9e
Gerrit-Change-Number: 39273
Gerrit-PatchSet: 2
Gerrit-Owner: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Edward O'Callaghan, Tim Wawrzynczak, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39272
to look at the new patch set (#2).
Change subject: security/vboot: Support enabling EC EFS with EC software sync
......................................................................
security/vboot: Support enabling EC EFS with EC software sync
If the ChromeOS EC uses EC early firmware selection (EFS), the AP vboot
build must also enable EC EFS. Add an option to control this, passing it
through to vboot.
BUG=b:150742950
TEST=none
BRANCH=none
Signed-off-by: Sam McNally <sammc(a)chromium.org>
Change-Id: I697e90748e19d15af154011413b30c0f2a0bf52e
---
M src/security/vboot/Kconfig
M src/security/vboot/Makefile.inc
2 files changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/39272/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/39272
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I697e90748e19d15af154011413b30c0f2a0bf52e
Gerrit-Change-Number: 39272
Gerrit-PatchSet: 2
Gerrit-Owner: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(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)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39265 )
Change subject: soc/intel/common/block/smm: add case intrusion to SMI handler
......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39265/5/src/soc/intel/common/block…
File src/soc/intel/common/block/smm/smihandler.c:
https://review.coreboot.org/c/coreboot/+/39265/5/src/soc/intel/common/block…
PS5, Line 444: if (tco_sts & (TCO_INTRD_DET << 16)) { /* INTRUDER# assertion */
> Shouldn't all these events have optional callback instead of just printing things?
I had that idea, too, but I wasn't sure if anyone would be happy with introducing more weak functions
--
To view, visit https://review.coreboot.org/c/coreboot/+/39265
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifad675bb09215ada760efebdcd915958febf5778
Gerrit-Change-Number: 39265
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 05 Mar 2020 00:20:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39262 )
Change subject: intel/soc: skl,apl,cnl,icl,tgl,common: enable TCO SMIs
......................................................................
Patch Set 5:
> Patch Set 5:
>
> > Patch Set 5:
> >
> > > Patch Set 4:
> > >
> > > > Patch Set 4:
> > > >
> > > > > Patch Set 4: Code-Review-1
> > > > >
> > > > > This should be made into an option. From a Chrome OS perspective we do not want to take SMIs for these events. It leads to having more complex handlers and the associated policy with them.
> > > >
> > > > What is Chrome OS using instead?
> > >
> > > We don't use TCO SMIs for anything. For intruder specifically we don't currently plumb anything up. For example, I don't want to take an SMI for a TCO timer expiration -- just want a reset on double expiration. Maybe we will in the future, but I don't want to enable TCO SMIs by default.
> >
> > Hm, but why can't you just ignore the fact that maybe SMI are raised? It doesn't interfere with anything, does it? Currently there is no functionality implemented but it's just a stub to make that possible.
> >
> > Don't get me wrong, I'm not against having an option for that. I just want to understand how that could affect Chrome OS or others.
>
> I don't like taking unnecessary SMIs as it steals cycles and it adds more attack surface area.
ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/39262
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If63effe74ac59b5d051a6454bc6375bb89605215
Gerrit-Change-Number: 39262
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Comment-Date: Thu, 05 Mar 2020 00:19:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39264 )
Change subject: soc/intel/common/block: tco: enable intruder SMI
......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39264/5/src/soc/intel/common/block…
File src/soc/intel/common/block/smbus/tco.c:
https://review.coreboot.org/c/coreboot/+/39264/5/src/soc/intel/common/block…
PS5, Line 105: /* Make TCO issue an SMI on INTRD_DET assertion */
> This should be driven by config policy as well -- falls under the broader TCO SMI option.
not sure if I got this right; you would like to have multiple options, one for TCO SMI general and one for each sub-SMI?
--
To view, visit https://review.coreboot.org/c/coreboot/+/39264
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3bc62c79ca3dc9e8896d9e2b9abdc14cfa46a9e7
Gerrit-Change-Number: 39264
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 05 Mar 2020 00:18:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-MessageType: comment