coreboot-gerrit October 2020

coreboot-gerrit@coreboot.org

1 participants
3474 discussions

Change in coreboot[master]: cpu/intel/common: correct MSR for the Nominal Performance in CPPC
by Michael Niewöhner (Code Review) 15 Oct '20

15 Oct '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46464 ) Change subject: cpu/intel/common: correct MSR for the Nominal Performance in CPPC ...................................................................... Patch Set 4: > Patch Set 4: Code-Review+1 > > Seems alright to me, though I can't speak to the msr.h part. > > It's been awhile so I'm not sure if the use of 0x771 was accidental (i.e., overlooked that this field was an exception to how the ACPI definition seemed to go along with the MSR layout), or intentional (e.g., looked at an older reference or tried to avoid using an MSR that might not be present). In any case, this change seems consistent with other firmwares and Skylake is reading from MSR_PLATFORM_INFO in other places so I shouldn't be too concerned about whether it's present or not. Wow, thanks for that quick feedback! :-) Well, I wasn't able to find a single ACPI out there where 0x771 got read. Platform sample code does the same, so this *should* be fine -- To view, visit https://review.coreboot.org/c/coreboot/+/46464 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic2c27fd3e14af18aa4101c0acd7a5ede15d1f3a9 Gerrit-Change-Number: 46464 Gerrit-PatchSet: 4 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 Delco <delco(a)chromium.org> 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: Thu, 15 Oct 2020 19:33:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/acpi: rename uuid for s0ix
by Michael Niewöhner (Code Review) 15 Oct '20

15 Oct '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46468 ) Change subject: soc/intel/common/acpi: rename uuid for s0ix ...................................................................... Patch Set 1: (4 comments) https://review.coreboot.org/c/coreboot/+/46468/1/src/soc/intel/common/acpi/… File src/soc/intel/common/acpi/lpit.asl: https://review.coreboot.org/c/coreboot/+/46468/1/src/soc/intel/common/acpi/… PS1, Line 23: c4eb40a0-6cd2-11e2-bcfd-0800200c9a66 > This is the "new" UUID for µPEP. it is the uid for the _DSM function for *any* PEP, see https://www.uefi.org/sites/default/files/resources/Intel_ACPI_Low_Power_S0_… https://review.coreboot.org/c/coreboot/+/46468/1/src/soc/intel/common/acpi/… PS1, Line 32: 0x60 > This indicates only functions 5 and 6 are supported which is correct https://review.coreboot.org/c/coreboot/+/46468/1/src/soc/intel/common/acpi/… PS1, Line 38: Return(Package(5) {0, Ones, Ones, Ones, Ones}) > This is where you report the platform requirements for s0ix entry, and I think this may be why s0ix […] this is not in the scope of this patch series; however, we may revisit that later https://review.coreboot.org/c/coreboot/+/46468/1/src/soc/intel/common/acpi/… PS1, Line 44: Return(Buffer(One) {0x0}) > This is required to support ACPI S3. this is also required in Slp_S0, see document mentioned above > If s0ix is disabled, we should just not include this file at all. it is explained, why this is included in the commit message > This is where you return a list of "bug-check critical devices" such as all SATA/AHCI ports and NVMe drives that support D3cold. This is not done here but in the mainboard. However we'd need a callback then -- To view, visit https://review.coreboot.org/c/coreboot/+/46468 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic62c37090ad1b747f9d7d204363cc58f96ef67ef Gerrit-Change-Number: 46468 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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: Thu, 15 Oct 2020 19:12:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib and libpayload: Add popcnt functions
by Angel Pons (Code Review) 15 Oct '20

15 Oct '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46421 ) Change subject: lib and libpayload: Add popcnt functions ...................................................................... lib and libpayload: Add popcnt functions Add 32-bit `popcnt` and 64-bit `popcnt64` helpers. Change-Id: I2e6a1007e475b662a85c067d96f81326e7f02905 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M payloads/libpayload/include/libpayload.h M src/include/lib.h 2 files changed, 6 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/46421/1 diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index 475fe0f..b761f04 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -445,6 +445,8 @@ void hexdump(const void *memory, size_t length); void fatal(const char *msg) __attribute__((noreturn)); +/* Population Count: number of bits that are one */ +static inline int popcnt(u32 x) { return __builtin_popcount(x); } /* Count Leading Zeroes: clz(0) == 32, clz(0xf) == 28, clz(1 << 31) == 0 */ static inline int clz(u32 x) { @@ -455,6 +457,7 @@ /* Find First Set: __ffs(0xf) == 0, __ffs(0) == -1, __ffs(1 << 31) == 31 */ static inline int __ffs(u32 x) { return log2(x & (u32)(-(s32)x)); } +static inline int popcnt64(u64 x) { return __builtin_popcountll(x); } static inline int clz64(u64 x) { return x ? __builtin_clzll(x) : sizeof(x) * 8; diff --git a/src/include/lib.h b/src/include/lib.h index a0003d3..5c6eef2 100644 --- a/src/include/lib.h +++ b/src/include/lib.h @@ -46,6 +46,8 @@ */ size_t hexstrtobin(const char *str, uint8_t *buf, size_t len); +/* Population Count: number of bits that are one */ +static inline int popcnt(u32 x) { return __builtin_popcount(x); } /* Count Leading Zeroes: clz(0) == 32, clz(0xf) == 28, clz(1 << 31) == 0 */ static inline int clz(u32 x) { return x ? __builtin_clz(x) : sizeof(x) * 8; } /* Integer binary logarithm (rounding down): log2(0) == -1, log2(5) == 2 */ @@ -56,6 +58,7 @@ /* Integer binary logarithm (rounding up): log2_ceil(0) == -1, log2(5) == 3 */ static inline int log2_ceil(u32 x) { return (x == 0) ? -1 : log2(x * 2 - 1); } +static inline int popcnt64(u64 x) { return __builtin_popcountll(x); } static inline int clz64(u64 x) { return x ? __builtin_clzll(x) : sizeof(x) * 8; } static inline int log2_64(u64 x) { return sizeof(x) * 8 - clz64(x) - 1; } static inline int __ffs64(u64 x) { return log2_64(x & (u64)(-(s64)x)); } -- To view, visit https://review.coreboot.org/c/coreboot/+/46421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2e6a1007e475b662a85c067d96f81326e7f02905 Gerrit-Change-Number: 46421 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: soc/intel/common/acpi: rename uuid for s0ix
by Michael Niewöhner (Code Review) 15 Oct '20

15 Oct '20

1 0

Change in coreboot[master]: cpu/intel/common: implement the two missing CPPC v2 autonomous registers
by Matt Delco (Code Review) 15 Oct '20

15 Oct '20

Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46463 ) Change subject: cpu/intel/common: implement the two missing CPPC v2 autonomous registers ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/46463 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7e2f4e4ae6a0fdb57204538bd62ead97cb540e91 Gerrit-Change-Number: 46463 Gerrit-PatchSet: 4 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 Delco <delco(a)chromium.org> 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: Thu, 15 Oct 2020 18:50:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: drop unneeded ISST configuration code
by Matt DeVillier (Code Review) 15 Oct '20

15 Oct '20

Matt DeVillier 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: 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 -- 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: Thu, 15 Oct 2020 18:50:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/skl: replace conditional on dt option reading CPUID for CPPC
by Matt DeVillier (Code Review) 15 Oct '20

15 Oct '20

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46461 ) Change subject: soc/intel/skl: replace conditional on dt option reading CPUID for CPPC ...................................................................... Patch Set 3: I'd add comments above the changed code to indicate it's checking ISST, since without it the conditions of the check become far more opaque -- To view, visit https://review.coreboot.org/c/coreboot/+/46461 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f9bf09385627fb6a1d8e566a80370f7ddd8605e Gerrit-Change-Number: 46461 Gerrit-PatchSet: 3 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: Thu, 15 Oct 2020 18:47:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/acpi: rename uuid for s0ix
by Angel Pons (Code Review) 15 Oct '20

15 Oct '20

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46468 ) Change subject: soc/intel/common/acpi: rename uuid for s0ix ...................................................................... Patch Set 1: (4 comments) https://review.coreboot.org/c/coreboot/+/46468/1/src/soc/intel/common/acpi/… File src/soc/intel/common/acpi/lpit.asl: https://review.coreboot.org/c/coreboot/+/46468/1/src/soc/intel/common/acpi/… PS1, Line 23: c4eb40a0-6cd2-11e2-bcfd-0800200c9a66 This is the "new" UUID for µPEP. https://review.coreboot.org/c/coreboot/+/46468/1/src/soc/intel/common/acpi/… PS1, Line 32: 0x60 This indicates only functions 5 and 6 are supported https://review.coreboot.org/c/coreboot/+/46468/1/src/soc/intel/common/acpi/… PS1, Line 38: Return(Package(5) {0, Ones, Ones, Ones, Ones}) This is where you report the platform requirements for s0ix entry, and I think this may be why s0ix entry might be hard (something something XTAL). The return value is a package of packages. You can either return the package directly, or store it in a variable: Name (DEVY, Package() { Package() { "\\_PR.CPU0", // Fully Qualified namestring 1, // Enabled/Disabled state Package() { 0, // Revision (always 0 for now) Package() { 0xff, // Associated LPI state UID (0xff: all) 0, // Minimum Dx state as precondition }, }, }, Package() { "\\_SB.PCI0.GFX0", // Fully Qualified namestring 1, // Enabled/Disabled state Package() { 0, // Revision (always 0 for now) Package() { 0xff, // Associated LPI state UID (0xff: all) 3, // Minimum Dx state as precondition }, }, }, Package() { "\\_SB.PCI0.SAT0", // Fully Qualified namestring 1, // Enabled/Disabled state Package() { 0, // Revision (always 0 for now) Package() { 0xff, // Associated LPI state UID (0xff: all) 0, // Minimum Dx state as precondition 0x81, // Optional: Device-specific F-state constraint }, }, }, } Note that the device list could change across boots. You can use the enabled/disabled state to list the devices statically, but determine whether devices are enabled/disabled at runtime. I think this can be done from within ACPI itself: just use Methods that return the enabled/disabled state for a device, by e.g. querying DEVEN for PCI devices. https://review.coreboot.org/c/coreboot/+/46468/1/src/soc/intel/common/acpi/… PS1, Line 44: Return(Buffer(One) {0x0}) This is required to support ACPI S3. If s0ix is disabled, we should just not include this file at all. This is where you return a list of "bug-check critical devices" such as all SATA/AHCI ports and NVMe drives that support D3cold. -- To view, visit https://review.coreboot.org/c/coreboot/+/46468 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic62c37090ad1b747f9d7d204363cc58f96ef67ef Gerrit-Change-Number: 46468 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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: Thu, 15 Oct 2020 18:43:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: vc/amd/fsp/picasso: Remove typedefs in bl_syscall_public.h
by Jason Glenesk (Code Review) 15 Oct '20

15 Oct '20

Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46334 ) Change subject: vc/amd/fsp/picasso: Remove typedefs in bl_syscall_public.h ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/46334/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/46334/1//COMMIT_MSG@7 PS1, Line 7: soc/amd/picasso > maybe vc/amd/fsp/picasso? changed. -- To view, visit https://review.coreboot.org/c/coreboot/+/46334 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I403075e18886b566f576d9ca0d198c2f5e9c3d96 Gerrit-Change-Number: 46334 Gerrit-PatchSet: 2 Gerrit-Owner: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 15 Oct 2020 18:43:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/intel/common: rework AES-NI locking
by Michael Niewöhner (Code Review) 15 Oct '20

15 Oct '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46275 ) Change subject: cpu/intel/common: rework AES-NI locking ...................................................................... Patch Set 5: (2 comments) https://review.coreboot.org/c/coreboot/+/46275/4/src/cpu/intel/common/commo… File src/cpu/intel/common/common_init.c: https://review.coreboot.org/c/coreboot/+/46275/4/src/cpu/intel/common/commo… PS4, Line 279: msr_unset_and_set > This is where it may still be helpful to have another function like msr_set_bits, that can just call […] Done https://review.coreboot.org/c/coreboot/+/46275/5/src/cpu/intel/common/commo… File src/cpu/intel/common/common_init.c: https://review.coreboot.org/c/coreboot/+/46275/5/src/cpu/intel/common/commo… PS5, Line 280: if (rdmsr(MSR_FEATURE_CONFIG) & AESNI_LOCK) ouch. that won't work ofc. Well, we could add another helper msr_get: if (msr_get(MSR_FEATURE_CONFIG) & AESNI_LOCK) // msr_get returns full msr or maybe if (msr_get(MSR_FEATURE_CONFIG, AESNI_LOCK)) // msr_get returns bitmask or just go back to the initial version? :S -- To view, visit https://review.coreboot.org/c/coreboot/+/46275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib2cda433bbec0192277839c02a1862b8f41340cb Gerrit-Change-Number: 46275 Gerrit-PatchSet: 5 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: Thu, 15 Oct 2020 18:42:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2020