coreboot-gerrit April 2023

coreboot-gerrit@coreboot.org

1 participants
5761 discussions

[L] Change in coreboot[master]: arch/x86/smbios: Fix file formatting
by Angel Pons (Code Review) 02 Apr '23

02 Apr '23

Attention is currently required from: Utkarsh Verma. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74170 ) Change subject: arch/x86/smbios: Fix file formatting ...................................................................... Patch Set 2: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/74170/comment/15f241f9_74f9af6a PS2, Line 9: minor Not sure what "minor" is supposed to mean here. File src/arch/x86/smbios.c: https://review.coreboot.org/c/coreboot/+/74170/comment/5c181d99_839cba31 PS2, Line 216: dimm->serial[1], dimm->serial[2], dimm->serial[3]); It's a good idea to ask oneself whether the suggestions from automated tools make sense from a human perspective. Putting `dimm->serial[0]` on one line and the rest of `dimm->serial[x]` arguments on the next line is odd, especially consideringt that all four arguments could simply be together in one line. TL;DR style "fixes" in coreboot tend to be frowned upon because people have different opinions. https://review.coreboot.org/c/coreboot/+/74170/comment/d71b0e3d_40b54bc9 PS2, Line 378: : t->bios_characteristics = BIOS_CHARACTERISTICS_PCI_SUPPORTED : | BIOS_CHARACTERISTICS_SELECTABLE_BOOT : | BIOS_CHARACTERISTICS_UPGRADEABLE; This is not an improvement w.r.t. readability. https://review.coreboot.org/c/coreboot/+/74170/comment/410baf8d_926f80a0 PS2, Line 1276: || device_type == SMBIOS_DEVICE_TYPE_UNKNOWN) What exactly is fixed here? Both conditions were nicely aligned before. -- To view, visit https://review.coreboot.org/c/coreboot/+/74170 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I40f44eec4d14d8b905f438c5305b640a33408c45 Gerrit-Change-Number: 74170 Gerrit-PatchSet: 2 Gerrit-Owner: Utkarsh Verma <utkarsh(a)bitbanged.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Utkarsh Verma <utkarsh(a)bitbanged.com> Gerrit-Comment-Date: Sun, 02 Apr 2023 16:32:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/common: Add code to order the different types of cores
by Sukumar Ghorai (Code Review) 02 Apr '23

02 Apr '23

Attention is currently required from: Subrata Banik, Sridhar Siricilla, Kyösti Mälkki, Elyes Haouas. Sukumar Ghorai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72132 ) Change subject: soc/intel/common: Add code to order the different types of cores ...................................................................... Patch Set 14: (1 comment) Patchset: PS14: verified -- To view, visit https://review.coreboot.org/c/coreboot/+/72132 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21487a5eb0439ea0cb5976787d1769ee94777469 Gerrit-Change-Number: 72132 Gerrit-PatchSet: 14 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Sun, 02 Apr 2023 16:22:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: arch/x86/smbios: Avoid buffer overflows
by Angel Pons (Code Review) 02 Apr '23

02 Apr '23

Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Utkarsh Verma. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74128 ) Change subject: arch/x86/smbios: Avoid buffer overflows ...................................................................... Patch Set 4: (3 comments) Patchset: PS4: No idea what Coverity is complaining about, but these changes don't really make sense... Will have a second look tomorrow after resting. File src/arch/x86/smbios.c: https://review.coreboot.org/c/coreboot/+/74128/comment/4b6dad88_96e413f6 PS4, Line 148: char string_buffer[15]; How exactly does reducing the buffer size avoid buffer overflows? It would only seem to make them more likely... https://review.coreboot.org/c/coreboot/+/74128/comment/29f27f55_5c9ca798 PS4, Line 552: static unsigned short cnt = 0; Why change the type? -- To view, visit https://review.coreboot.org/c/coreboot/+/74128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibbf93fd113791b6faa1e5128da8e692f7f00ac73 Gerrit-Change-Number: 74128 Gerrit-PatchSet: 4 Gerrit-Owner: Utkarsh Verma <utkarsh(a)bitbanged.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Utkarsh Verma <utkarsh(a)bitbanged.com> Gerrit-Comment-Date: Sun, 02 Apr 2023 16:22:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/common: Add code to order the different types of cores
by Sukumar Ghorai (Code Review) 02 Apr '23

02 Apr '23

Attention is currently required from: Subrata Banik, Sridhar Siricilla, Kyösti Mälkki, Elyes Haouas. Sukumar Ghorai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72132 ) Change subject: soc/intel/common: Add code to order the different types of cores ...................................................................... Patch Set 14: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/72132 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21487a5eb0439ea0cb5976787d1769ee94777469 Gerrit-Change-Number: 72132 Gerrit-PatchSet: 14 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Sun, 02 Apr 2023 16:22:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/common: Order the CPUs based on their APIC IDs
by Sukumar Ghorai (Code Review) 02 Apr '23

02 Apr '23

Sukumar Ghorai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72131 ) Change subject: soc/intel/common: Order the CPUs based on their APIC IDs ...................................................................... Patch Set 12: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/72131 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8ab6053ffd036185d74d5469fbdf36d48e0021ce Gerrit-Change-Number: 72131 Gerrit-PatchSet: 12 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Sun, 02 Apr 2023 16:21:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: commonlib/coreboot_tables.h: Fix typo in enum type name
by Angel Pons (Code Review) 02 Apr '23

02 Apr '23

Attention is currently required from: Benjamin Doron, Maximilian Brune, Lean Sheng Tan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74120 ) Change subject: commonlib/coreboot_tables.h: Fix typo in enum type name ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/74120/comment/7db4c774_e7c5ab8c PS1, Line 9: Fix a typo in an enum's type name, "tmp" ---> "tpm". The enum type is > It's an enum, we can use it unnamed. […] The enum type is not used because the coreboot tables need to use fixed-size types. The type is probably there to help readers associate the enum with what it should be used with. Anything to do? -- To view, visit https://review.coreboot.org/c/coreboot/+/74120 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie7529e7ee80aa9661ec053da8211c2c3295b3942 Gerrit-Change-Number: 74120 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Sun, 02 Apr 2023 16:07:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Benjamin Doron <benjamin.doron00(a)gmail.com> Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: arch/x86/smbios: Avoid buffer overflows
by Utkarsh Verma (Code Review) 02 Apr '23

02 Apr '23

Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons. Utkarsh Verma has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74128 ) Change subject: arch/x86/smbios: Avoid buffer overflows ...................................................................... Patch Set 4: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/74128/comment/441c9605_56779b3a PS3, Line 9: The format specifiers for numbers in snprintf() calls are updated to be > The checkpatch comments are unfortunately added to the wrong line and refer to the line below, which […] Done https://review.coreboot.org/c/coreboot/+/74128/comment/3c503b76_d24780a2 PS3, Line 21: > The blank line could be removed. Done Patchset: PS4: I hope this fixes it. -- To view, visit https://review.coreboot.org/c/coreboot/+/74128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibbf93fd113791b6faa1e5128da8e692f7f00ac73 Gerrit-Change-Number: 74128 Gerrit-PatchSet: 4 Gerrit-Owner: Utkarsh Verma <utkarsh(a)bitbanged.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 02 Apr 2023 14:43:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Utkarsh Verma <utkarsh(a)bitbanged.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/meteorlake: Enable VMX
by Subrata Banik (Code Review) 02 Apr '23

02 Apr '23

Attention is currently required from: Tarun Tuli, Paul Menzel, Kapil Porwal, Ivy Jian, Eric Lai, Ronak Kanabar. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74161 ) Change subject: soc/intel/meteorlake: Enable VMX ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/74161/comment/60efec94_603606bd PS3, Line 7: soc/intel/meteorlake: Enable VMX > Reading the diff, it’s only related to `DROP_CPU_FEATURE_PROGRAM_IN_FSP`? today VMX enablement during MP init has done by FSP which we want to get rid starting with MTL (read the details here https://review.coreboot.org/c/coreboot/+/74167/1/src/soc/intel/meteorlake/K…). and would like to move it into CB (refer to cannonlakehttps://github.com/coreboot/coreboot/blob/master/src/soc/intel/ca…) https://review.coreboot.org/c/coreboot/+/74161/comment/1236374f_c7195605 PS3, Line 11: > I think it was discussed in the past, that coreboot does not need to do it, but it looks like there is `ENABLE_VMX` already in Intel’s common code. I was just confused by the commit message summary/title. i still don't understand why having Kconfig enabled is enough to ensure VMX being enabled by CB?user need to call into set_feature_ctrl_vmx_arg() or set_feature_ctrl_vmx() to program the registers https://github.com/coreboot/coreboot/blob/37ccb2ce82d1c4782356a569737dd72bf… -- To view, visit https://review.coreboot.org/c/coreboot/+/74161 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7e49c15fd4f78a3e633855fea550720f0a685062 Gerrit-Change-Number: 74161 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Comment-Date: Sun, 02 Apr 2023 14:43:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: arch/x86/smbios: Avoid buffer overflows
by Utkarsh Verma (Code Review) 02 Apr '23

02 Apr '23

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons, Utkarsh Verma. Hello Felix Singer, build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/74128 to look at the new patch set (#4). Change subject: arch/x86/smbios: Avoid buffer overflows ...................................................................... arch/x86/smbios: Avoid buffer overflows The format specifiers for numbers in snprintf() calls are updated to be more specific to the data types used, making things more obvious for static analysers. This was found by coverity scan and raised in ticket #431: https://scan6.scan.coverity.com/reports.htm#v55284/p10744 CID: 1487449 CID: 1487312 CID: 1487457 Found by: Coverity Scan Signed-off-by: Utkarsh Verma <utkarsh(a)bitbanged.com> Change-Id: Ibbf93fd113791b6faa1e5128da8e692f7f00ac73 --- M src/arch/x86/smbios.c M src/arch/x86/smbios_defaults.c 2 files changed, 30 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/74128/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/74128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibbf93fd113791b6faa1e5128da8e692f7f00ac73 Gerrit-Change-Number: 74128 Gerrit-PatchSet: 4 Gerrit-Owner: Utkarsh Verma <utkarsh(a)bitbanged.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Utkarsh Verma <utkarsh(a)bitbanged.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: soc/intel/common: Order the CPUs based on their APIC IDs
by Paul Menzel (Code Review) 02 Apr '23

02 Apr '23

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72131 ) Change subject: soc/intel/common: Order the CPUs based on their APIC IDs ...................................................................... Patch Set 12: (1 comment) Patchset: PS12: > The 9elements QA shows, Linux logs the messages below: […] I couldn’t reproduce the error with `emulation/qemu-i440fx/4.18-1557*/*/config.txt` from the board status repository (and `make olddefconfig`). -- To view, visit https://review.coreboot.org/c/coreboot/+/72131 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8ab6053ffd036185d74d5469fbdf36d48e0021ce Gerrit-Change-Number: 72131 Gerrit-PatchSet: 12 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Sun, 02 Apr 2023 14:26:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2023