Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46460 )
Change subject: soc/intel: drop unneeded ISST configuration code
......................................................................
Patch Set 2:
> Patch Set 2:
>
> > Patch Set 2:
> >
> > > Patch Set 2:
> > >
> > > IMO a cleaner approach would be to move the code to a function in the MSR common block, switch any boards disabling ISST to use the function directly vs setting DT register, and then dropping from the SoC code
> >
> > Cleaner? How could useless code be cleaner than no code?
>
> perhaps i was unclear, I meant in conjunction with the parent commit. And the code wouldn't be useless, it would be used by google/dedede
Ah, got it. Thanks. Well, I'd really like to know if it's required anyway. I've added Karthikeyan Ramasubramanian, who initially added the code, to CB:46459. Hoping they can help
--
To view, visit https://review.coreboot.org/c/coreboot/+/46460
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I952720cf1de78b00b1bf749f10e9c0acd6ecb6b7
Gerrit-Change-Number: 46460
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 16 Oct 2020 20:36:54 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46276 )
Change subject: cpu/intel/common: only lock AES-NI when supported
......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46276/6/src/cpu/intel/common/commo…
File src/cpu/intel/common/common_init.c:
https://review.coreboot.org/c/coreboot/+/46276/6/src/cpu/intel/common/commo…
PS6, Line 278: cpuid_ecx(1)
> heh, that's twice as long \o/ done
sure, but helps give a little more context, and it's in common code now, so it's not typed out N times 😉
--
To view, visit https://review.coreboot.org/c/coreboot/+/46276
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia7ffd5393a3e972f461ff7991b9c5bd363712361
Gerrit-Change-Number: 46276
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.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)users.sourceforge.net>
Gerrit-Comment-Date: Fri, 16 Oct 2020 20:33:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Hello Felix Singer, build bot (Jenkins), Nico Huber, Furquan Shaikh, David Guckian, Paul Menzel, Tim Wawrzynczak, Vanessa Eusebio, Subrata Banik, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46278
to look at the new patch set (#9).
Change subject: {cpu,soc}/intel: replace AES-NI locking by common implemenation call
......................................................................
{cpu,soc}/intel: replace AES-NI locking by common implemenation call
Deduplicate code by using the new common cpu code implementation of
AES-NI locking.
For model_2065x and model_206ax locking was moved from SMM to core init
because the MSR is core-scoped, not package-scoped.
Change-Id: I7ab2d3839ecb758335ef8cc6a0c0c7103db0fa50
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M src/cpu/intel/model_2065x/finalize.c
M src/cpu/intel/model_2065x/model_2065x_init.c
M src/cpu/intel/model_206ax/finalize.c
M src/cpu/intel/model_206ax/model_206ax_init.c
M src/include/cpu/intel/msr.h
M src/soc/intel/apollolake/cpu.c
M src/soc/intel/common/block/cpu/cpulib.c
M src/soc/intel/common/block/include/intelblocks/msr.h
M src/soc/intel/denverton_ns/Kconfig
M src/soc/intel/denverton_ns/cpu.c
M src/soc/intel/skylake/cpu.c
11 files changed, 19 insertions(+), 44 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/46278/9
--
To view, visit https://review.coreboot.org/c/coreboot/+/46278
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7ab2d3839ecb758335ef8cc6a0c0c7103db0fa50
Gerrit-Change-Number: 46278
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Subrata Banik <subrata.banik(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-MessageType: newpatchset
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46273 )
Change subject: soc/intel/cnl: lock AES-NI feature if selected
......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46273/6/src/soc/intel/cannonlake/K…
File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/46273/6/src/soc/intel/cannonlake/K…
PS6, Line 81: select CPU_INTEL_COMMON_HYPERTHREADING
> It's required for the call to `intel_ht_sibling`, I'm not sure which currently selected one is inclu […]
Tim, now it doesn't exist anymore :P
--
To view, visit https://review.coreboot.org/c/coreboot/+/46273
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I79495bfbd3ebf3b712ce9ecf2040cecfd954178d
Gerrit-Change-Number: 46273
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 16 Oct 2020 20:31:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Hello Felix Singer, build bot (Jenkins), Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Paul Menzel, Subrata Banik, Aamir Bohra, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46273
to look at the new patch set (#10).
Change subject: soc/intel/cnl: lock AES-NI feature if selected
......................................................................
soc/intel/cnl: lock AES-NI feature if selected
Lock AES-NI (MSR_FEATURE_CONFIG) to prevent unintended changes of
AES-NI enablement as precaution, as suggested in Intel document
325384-070US.
Locking is enabled by default (as already done in SKL and Arrandale) and
may be disabled by the newly introduced Kconfig in the parent change.
Tested by checking the MSR.
Change-Id: I79495bfbd3ebf3b712ce9ecf2040cecfd954178d
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M src/soc/intel/cannonlake/cpu.c
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/46273/10
--
To view, visit https://review.coreboot.org/c/coreboot/+/46273
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I79495bfbd3ebf3b712ce9ecf2040cecfd954178d
Gerrit-Change-Number: 46273
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Hello Felix Singer, build bot (Jenkins), Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Subrata Banik, Aamir Bohra, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46276
to look at the new patch set (#7).
Change subject: cpu/intel/common: only lock AES-NI when supported
......................................................................
cpu/intel/common: only lock AES-NI when supported
Add a check to only lock AES-NI when AES is supported.
Change-Id: Ia7ffd5393a3e972f461ff7991b9c5bd363712361
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M src/cpu/intel/common/common_init.c
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/46276/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/46276
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia7ffd5393a3e972f461ff7991b9c5bd363712361
Gerrit-Change-Number: 46276
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.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)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Hello Felix Singer, build bot (Jenkins), Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Paul Menzel, Subrata Banik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46354
to look at the new patch set (#7).
Change subject: include/cpu/x86: introduce new helper for (un)setting MSRs
......................................................................
include/cpu/x86: introduce new helper for (un)setting MSRs
msr_set_bit can only set single bits in MSRs and causes mixing of bit
positions and bitmasks in the MSR header files. Thus, replace the helper
by versions which can unset and set whole MSR bitmasks, just like the
"and-or"-helper, but in the way commit 64a6b6c was done (inversion done
in the helper). This helps keeping the MSR macros unified in bitmask
style.
In sum, the three helpers msr_set, msr_unset and msr_unset_and_set get
added.
The few uses of msr_set_bit have been replaced by the new version, while
the used macros have been converted accordingly.
Change-Id: Idfe9b66e7cfe78ec295a44a2a193f530349f7689
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M src/cpu/intel/haswell/finalize.c
M src/cpu/intel/model_2065x/finalize.c
M src/cpu/intel/model_206ax/finalize.c
M src/include/cpu/x86/msr.h
M src/soc/intel/common/block/cpu/cpulib.c
M src/soc/intel/common/block/include/intelblocks/msr.h
6 files changed, 48 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/46354/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/46354
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idfe9b66e7cfe78ec295a44a2a193f530349f7689
Gerrit-Change-Number: 46354
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset