coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: mb/google,intel: Split chromeos.c files
by Kyösti Mälkki (Code Review) 10 Nov '21

10 Nov '21

Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59011 ) Change subject: mb/google,intel: Split chromeos.c files ...................................................................... Patch Set 6: (3 comments) File src/mainboard/google/dedede/chromeos.c: https://review.coreboot.org/c/coreboot/+/59011/comment/7a16667d_4c7aaf73 PS6, Line 15: {GPIO_EC_IN_RW, ACTIVE_HIGH, gpio_get(GPIO_EC_IN_RW), "EC in RW"}, What is the relevance of passing a GPIO pin number and polarity here? Is the payload able (and platform dependent) to read the current GPIO state? For EC_IN_RW, isn't this flag in vboot2 context too? Or is the entire lb_gpio struct obsolete and was only ever used with depthcharge? File src/mainboard/google/eve/chromeos.c: https://review.coreboot.org/c/coreboot/+/59011/comment/0d256aaf_d57f1fed PS6, Line 26: CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_DEVICE_NAME), I understand this fills CRHW.GPIO sysfs subtree with chromeos_acpi driver. Is there some specification what one expects to see there? Does a virtual GPIO need a name or polarity? How does user-space determine the state of it with no GPIO associated? CROS_GPIO_DEVICE_NAME does not expand to ACPI device object name. Can userspace do anything with a GPIO pin number like GPIO_PCH_WP? File src/mainboard/google/parrot/chromeos.c: https://review.coreboot.org/c/coreboot/+/59011/comment/ad460c08_f622ebec PS6, Line 25: {101, ACTIVE_LOW, (gen_pmcon_1 >> 9) & 1, "power"}, GPIO pin 101? Should these non-GPIO pins be -1 for virtual? -- To view, visit https://review.coreboot.org/c/coreboot/+/59011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I71a02c5fa1b256316b86b673660bf22dfd284f7f Gerrit-Change-Number: 59011 Gerrit-PatchSet: 6 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> 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: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Wed, 10 Nov 2021 12:12:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mainboard/starlabs: Add StarBook Mk V
by Sean Rhodes (Code Review) 10 Nov '21

10 Nov '21

Attention is currently required from: Angel Pons, Felix Held. Hello build bot (Jenkins), Angel Pons, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56088 to look at the new patch set (#110). Change subject: mainboard/starlabs: Add StarBook Mk V ...................................................................... mainboard/starlabs: Add StarBook Mk V Tested using MrChromeBox's `uefipayload_202107` branch: * Windows 10 * Ubuntu 20.04 * MX Linux 19.4 * Manjaro 21 No known issues. https://starlabs.systems/pages/starbook-specification Signed-off-by: Sean Rhodes <sean(a)starlabs.systems> Change-Id: I090971a9e8d2be5b08be886d00d304607304b645 --- M 3rdparty/blobs M Documentation/distributions.md M Documentation/mainboard/index.md A Documentation/mainboard/starlabs/BiosLock.jpg A Documentation/mainboard/starlabs/starbook_tgl.md M MAINTAINERS A src/mainboard/starlabs/Kconfig A src/mainboard/starlabs/Kconfig.name A src/mainboard/starlabs/labtop/Kconfig A src/mainboard/starlabs/labtop/Kconfig.name A src/mainboard/starlabs/labtop/Makefile.inc A src/mainboard/starlabs/labtop/acpi/mainboard.asl A src/mainboard/starlabs/labtop/acpi/sleep.asl A src/mainboard/starlabs/labtop/board_info.txt A src/mainboard/starlabs/labtop/bootblock.c A src/mainboard/starlabs/labtop/cmos.default A src/mainboard/starlabs/labtop/cmos.layout A src/mainboard/starlabs/labtop/dsdt.asl A src/mainboard/starlabs/labtop/hda_verb.c A src/mainboard/starlabs/labtop/include/variants.h A src/mainboard/starlabs/labtop/mainboard.c A src/mainboard/starlabs/labtop/ramstage.c A src/mainboard/starlabs/labtop/variants/tgl/Makefile.inc A src/mainboard/starlabs/labtop/variants/tgl/board.fmd A src/mainboard/starlabs/labtop/variants/tgl/data.vbt A src/mainboard/starlabs/labtop/variants/tgl/devicetree.cb A src/mainboard/starlabs/labtop/variants/tgl/devtree.c A src/mainboard/starlabs/labtop/variants/tgl/gpio.c A src/mainboard/starlabs/labtop/variants/tgl/hda_verb.c A src/mainboard/starlabs/labtop/variants/tgl/romstage.c 30 files changed, 1,418 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/56088/110 -- To view, visit https://review.coreboot.org/c/coreboot/+/56088 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I090971a9e8d2be5b08be886d00d304607304b645 Gerrit-Change-Number: 56088 Gerrit-PatchSet: 110 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: arch/x86: Refactor the SMBIOS type 17 write function
by Jakub Czapiga (Code Review) 10 Nov '21

10 Nov '21

Attention is currently required from: Jason Glenesk, Marshall Dawson, Tim Wawrzynczak, Subrata Banik, Angel Pons, Rob Barnes, Patrick Rudolph, Felix Held. Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56628 ) Change subject: arch/x86: Refactor the SMBIOS type 17 write function ...................................................................... Patch Set 25: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/56628 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia337ac8f50b61ae78d86a07c7a86aa9c248bad50 Gerrit-Change-Number: 56628 Gerrit-PatchSet: 25 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(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: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 10 Nov 2021 12:08:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mainboard/starlabs: Add StarBook Mk V
by Sean Rhodes (Code Review) 10 Nov '21

10 Nov '21

Attention is currently required from: Angel Pons, Felix Held. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56088 ) Change subject: mainboard/starlabs: Add StarBook Mk V ...................................................................... Patch Set 109: (21 comments) File Documentation/distributions.md: https://review.coreboot.org/c/coreboot/+/56088/comment/97fa8858_9be5d3de PS109, Line 29: offer > singular: offer*s* Done File Documentation/mainboard/starlabs/BiosLock.jpg: PS109: > Converting this image to WebP reduces its size by 37. […] Safari doesn't support WebP though, which is granted a minority, but better to have it visible for all. File Documentation/mainboard/starlabs/starbook_tgl.md: https://review.coreboot.org/c/coreboot/+/56088/comment/1fea8f14_967a33fe PS109, Line 50: The below are optional > I'm not sure if this is gramatically correct. How about: […] Done File src/mainboard/starlabs/labtop/Kconfig: https://review.coreboot.org/c/coreboot/+/56088/comment/d08e5912_ae89985b PS109, Line 45: string > Declaring the type shouldn't be needed Done https://review.coreboot.org/c/coreboot/+/56088/comment/d7dfcfac_a1325506 PS109, Line 52: int > Declaring the type shouldn't be needed Done https://review.coreboot.org/c/coreboot/+/56088/comment/c8f776f5_2a58ec41 PS109, Line 56: int > Declaring the type shouldn't be needed Done https://review.coreboot.org/c/coreboot/+/56088/comment/815a099e_edbf00f6 PS109, Line 66: string > Declaring the type shouldn't be needed Done File src/mainboard/starlabs/labtop/acpi/mainboard.asl: https://review.coreboot.org/c/coreboot/+/56088/comment/6bbae148_4a5bef9f PS109, Line 8: Comet > Huh, isn't this a TGL board? Done https://review.coreboot.org/c/coreboot/+/56088/comment/35dd7970_df402179 PS109, Line 25: "PRP00001" > This device ID is invalid, one 0 too many. […] It does! File src/mainboard/starlabs/labtop/acpi/sleep.asl: https://review.coreboot.org/c/coreboot/+/56088/comment/f9ef8cc0_6455a344 PS109, Line 3: // _PTS: Prepare To Sleep > nit: Drop these comments? Done File src/mainboard/starlabs/labtop/dsdt.asl: https://review.coreboot.org/c/coreboot/+/56088/comment/4d535917_3544e410 PS109, Line 12: // OEM revision > nit: remove? Done File src/mainboard/starlabs/labtop/hda_verb.c: https://review.coreboot.org/c/coreboot/+/56088/comment/97d7773a_8d4db3e3 PS109, Line 15: u32 viddid > Unused parameter Done https://review.coreboot.org/c/coreboot/+/56088/comment/7e83b0c4_60da7399 PS109, Line 23: %d > Why not use `%08x` instead? Done File src/mainboard/starlabs/labtop/mainboard.c: https://review.coreboot.org/c/coreboot/+/56088/comment/82068bc3_a4fd1858 PS109, Line 31: return "B5"; > How about `return CONFIG_MAINBOARD_FAMILY;` instead? Done File src/mainboard/starlabs/labtop/variants/tgl/board.fmd: https://review.coreboot.org/c/coreboot/+/56088/comment/09420b11_ac3aa9b9 PS109, Line 6: SI_ALL@0x0 0x500000 { : SI_DESC@0x0 0x1000 : SI_ME@0x1000 : } > Why is this part indented with spaces? Done File src/mainboard/starlabs/labtop/variants/tgl/devicetree.cb: https://review.coreboot.org/c/coreboot/+/56088/comment/40eca644_dd3d63c4 PS109, Line 3: register "power_limits_config[POWER_LIMITS_U_2_CORE]" = "{ : .tdp_pl1_override = 15, : .tdp_pl2_override = 15, : }" : : register "power_limits_config[POWER_LIMITS_U_4_CORE]" = "{ : .tdp_pl1_override = 15, : .tdp_pl2_override = 15, : }" > I see `devtree_update()` overrides these values. […] Done https://review.coreboot.org/c/coreboot/+/56088/comment/ef6ebc79_1e84e262 PS109, Line 74: register "PcieRpEnable[0]" = "1" : register "PcieRpEnable[2]" = "1" > The PCI devices for these RPs are disabled, why? Left over from debugging TCSS/D3C stuff. They can go. https://review.coreboot.org/c/coreboot/+/56088/comment/bfef92ea_1220162a PS109, Line 82: register "PcieClkSrcUsage[0]" = "0x40" > TGL-U doesn't have PEG (PCI Express Graphics, i.e. CPU PCIe ports). Done https://review.coreboot.org/c/coreboot/+/56088/comment/d9fe0380_eb934496 PS109, Line 236: device pci 1f.4 off end # SMBus > Shouldn't SMBus be enabled? It's used to read the DIMM SPDs Done File src/mainboard/starlabs/labtop/variants/tgl/hda_verb.c: https://review.coreboot.org/c/coreboot/+/56088/comment/a155e073_a5884b9f PS109, Line 8: 0x10ec1200 > Why is this value different from the one in the `AZALIA_SUBVENDOR` macro? Done https://review.coreboot.org/c/coreboot/+/56088/comment/40ac83d2_edf617bf PS109, Line 7: 0x10ec0256, // Codec Vendor / Device ID: Realtek ALC256 : 0x10ec1200, // Subsystem ID : 38, // Number of jacks (NID entries) > Please use consistent comment style Done -- To view, visit https://review.coreboot.org/c/coreboot/+/56088 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I090971a9e8d2be5b08be886d00d304607304b645 Gerrit-Change-Number: 56088 Gerrit-PatchSet: 109 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 10 Nov 2021 12:07:18 +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]: arch/x86: Refactor the SMBIOS type 17 write function
by Subrata Banik (Code Review) 10 Nov '21

10 Nov '21

Attention is currently required from: Jason Glenesk, Jakub Czapiga, Marshall Dawson, Tim Wawrzynczak, Angel Pons, Rob Barnes, Patrick Rudolph, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56628 ) Change subject: arch/x86: Refactor the SMBIOS type 17 write function ...................................................................... Patch Set 24: (3 comments) File src/include/spd.h: https://review.coreboot.org/c/coreboot/+/56628/comment/6713d408_ba090bef PS24, Line 201: SPD_UNDEFINED 0x00 > I think, this should stay. Using DDR2_SPD_UNDEFINED, DDR3_SPD_UNDEFINED etc is weird to me. Ack File tests/lib/dimm_info_util-test.c: https://review.coreboot.org/c/coreboot/+/56628/comment/500083b2_77ddb2ce PS24, Line 109: DDR2_SPD_UNDEFINED > Why not the DDR3_SPD_UNDEFINED or LPX_SPD_UNDEFINED? […] Ack https://review.coreboot.org/c/coreboot/+/56628/comment/b4102de3_163d3b7f PS24, Line 151: MEMORY_TYPE_DDR2, { DDR2_SPD_UDIMM, DDR2_SPD_MICRO_DIMM, : DDR2_SPD_MINI_UDIMM }, { DDR2_SPD_RDIMM, DDR2_SPD_MINI_RDIMM }, : DDR2_SPD_SODIMM > Please, use named fields. […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/56628 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia337ac8f50b61ae78d86a07c7a86aa9c248bad50 Gerrit-Change-Number: 56628 Gerrit-PatchSet: 24 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(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: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 10 Nov 2021 12:05:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: arch/x86: Refactor the SMBIOS type 17 write function
by Subrata Banik (Code Review) 10 Nov '21

10 Nov '21

Attention is currently required from: Jason Glenesk, Marshall Dawson, Tim Wawrzynczak, Subrata Banik, Angel Pons, Rob Barnes, Patrick Rudolph, Felix Held. Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Furquan Shaikh, Jakub Czapiga, Marshall Dawson, Tim Wawrzynczak, Angel Pons, Rob Barnes, Patrick Rudolph, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56628 to look at the new patch set (#25). Change subject: arch/x86: Refactor the SMBIOS type 17 write function ...................................................................... arch/x86: Refactor the SMBIOS type 17 write function List of changes: 1. Create Module Type macros as per Memory Type (i.e. DDR2/DDR3/DDR4/DDR5/LPDDR4/LPDDR5) and fix compilation issue due to renaming of existing macros due to scoping the Memory Type. 2. Use dedicated Memory Type and Module type for `Form Factor` and `TypeDetail` conversion using `get_spd_info()` function. 3. Create a new API (convert_form_factor_to_module_type()) for `Form Factor` to 'Module type' conversion as per `Memory Type`. 4. Add new argument as `Memory Type` to smbios_form_factor_to_spd_mod_type() so that it can internally call convert_form_factor_to_module_type() for `Module Type` conversion. 5. Update `test_smbios_form_factor_to_spd_mod_type()` to accommodate different memory types. 6. Skip fixed module type to form factor conversion using DDR2 SPD4 specification (inside dimm_info_fill()). Refer to datasheet SPD4.1.2.M-1 for LPDDRx and SPD4.1.2.L-3 for DDRx. BUG=b:194659789 TEST=Refer to dmidecode -t 17 output as below: Without this code change: Handle 0x0012, DMI type 17, 40 bytes Memory Device Array Handle: 0x000A Error Information Handle: Not Provided Total Width: 16 bits Data Width: 16 bits Size: 2048 MB Form Factor: Unknown .... With this code change: Handle 0x0012, DMI type 17, 40 bytes Memory Device Array Handle: 0x000A Error Information Handle: Not Provided Total Width: 16 bits Data Width: 16 bits Size: 2048 MB Form Factor: Row Of Chips .... Change-Id: Ia337ac8f50b61ae78d86a07c7a86aa9c248bad50 Signed-off-by: Subrata Banik <subrata.banik(a)intel.com> --- M src/arch/x86/smbios.c M src/device/dram/ddr3.c M src/device/dram/ddr4.c M src/device/dram/spd.c M src/include/device/dram/spd.h M src/include/dimm_info_util.h M src/include/spd.h M src/lib/dimm_info_util.c M src/mainboard/scaleway/tagada/ramstage.c M src/northbridge/intel/haswell/haswell_mrc/raminit.c M src/soc/amd/common/fsp/dmi.c M src/soc/amd/common/pi/amd_late_init.c M src/soc/intel/common/smbios.c M tests/lib/Makefile.inc M tests/lib/dimm_info_util-test.c 15 files changed, 404 insertions(+), 107 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/56628/25 -- To view, visit https://review.coreboot.org/c/coreboot/+/56628 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia337ac8f50b61ae78d86a07c7a86aa9c248bad50 Gerrit-Change-Number: 56628 Gerrit-PatchSet: 25 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(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: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: [WIP,RFC] Clean up spinlocks
by Kyösti Mälkki (Code Review) 10 Nov '21

10 Nov '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Julius Werner, Felix Held. Hello Jason Glenesk, Raul Rangel, Marshall Dawson, Julius Werner, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59094 to look at the new patch set (#2). Change subject: [WIP,RFC] Clean up spinlocks ...................................................................... [WIP,RFC] Clean up spinlocks This reverts commit 037ee4b5562099771795d52cf7149ddff8dd3595. TBD: Breaks romstage co-operative threading, since those need implementation of spin_lock() that is not a spinlock. IMO commit a98d302fe9 x86/smp/spinlock: Disable thread coop when taking spinlock is wrong and should be reverted too. Change-Id: Iba52febdeee78294f916775ee9ce8a82d6203570 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- D src/arch/arm/include/armv4/arch/smp/spinlock.h M src/arch/x86/include/arch/smp/spinlock.h M src/include/rules.h M src/include/smp/spinlock.h D src/soc/amd/common/psp_verstage/include/arch/smp/spinlock.h 5 files changed, 11 insertions(+), 45 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/59094/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59094 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iba52febdeee78294f916775ee9ce8a82d6203570 Gerrit-Change-Number: 59094 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/brya/var/brask: Enable LAN driver to write MAC
by Zhuohao Lee (Code Review) 10 Nov '21

10 Nov '21

Attention is currently required from: David Wu, Tim Wawrzynczak, Alan Huang. Zhuohao Lee has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59087 ) Change subject: mb/google/brya/var/brask: Enable LAN driver to write MAC ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59087 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibb95b02fd6d61621ef46db4d63b48456a0a72732 Gerrit-Change-Number: 59087 Gerrit-PatchSet: 1 Gerrit-Owner: Alan Huang <alan-huang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Alan Huang <alan-huang(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Wed, 10 Nov 2021 11:49:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: security/vboot: Add NVRAM counter for TPM 2.0
by Miriam Polzer (Code Review) 10 Nov '21

10 Nov '21

Miriam Polzer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59097 ) Change subject: security/vboot: Add NVRAM counter for TPM 2.0 ...................................................................... security/vboot: Add NVRAM counter for TPM 2.0 Create an NVRAM counter in TPM 2.0 that survives owner clear and can be read and written without authorization. This counter allows to seal data with the TPM that can only be unsealed before the counter was incremented. It will be used during Chrome OS rollback to securely carry data across a TPM clear. Signed-off-by: Miriam Polzer <mpolzer(a)google.com> Change-Id: I511dba3b3461713ce20fb2bda9fced0fee6517e1 --- M src/security/vboot/antirollback.h M src/security/vboot/secdata_tpm.c M src/security/vboot/vboot_logic.c 3 files changed, 43 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/59097/1 diff --git a/src/security/vboot/antirollback.h b/src/security/vboot/antirollback.h index a208c04..483efd9 100644 --- a/src/security/vboot/antirollback.h +++ b/src/security/vboot/antirollback.h @@ -28,6 +28,7 @@ /* 0x100d: Hash of MRC_CACHE training data for non-recovery boot */ #define MRC_RW_HASH_NV_INDEX 0x100d #define HASH_NV_SIZE VB2_SHA256_DIGEST_SIZE +#define ENT_ROLLBACK_NV_INDEX 0x100e /* Zero-Touch Enrollment related spaces */ #define ZTE_BOARD_ID_NV_INDEX 0x3fff00 #define ZTE_RMA_SN_BITS_INDEX 0x3fff01 @@ -93,4 +94,9 @@ */ uint32_t antirollback_lock_space_mrc_hash(uint32_t index); +/* + * Create a new tpm counter that is used for enterprise rollback. + */ +uint32_t enterprise_rollback_create_counter(void); + #endif /* ANTIROLLBACK_H_ */ diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index a95e7d1..bc9c216 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -110,6 +110,13 @@ .TPMA_NV_WRITE_STCLEAR = 1, }; +const static TPMA_NV rw_counter_attributes = { + .TPMA_NV_AUTHWRITE = 1, + .TPMA_NV_AUTHREAD = 1, + .TPMA_NV_PLATFORMCREATE = 1, + .TPMA_NV_COUNTER = 1, +}; + static const TPMA_NV fwmp_attr = { .TPMA_NV_PLATFORMCREATE = 1, .TPMA_NV_OWNERWRITE = 1, @@ -178,6 +185,14 @@ 0x50, 0xEF, 0x96, 0x98, 0x0A, 0x2B, 0x96, 0x6E, 0xA9, 0x09, 0x04, 0x4A, 0x01, 0xB8, 0x5F, 0xA5, 0x4A, 0x96, 0xFC, 0x59, 0x84}; +/** + * Empty policy digest. + */ +const static uint8_t empty_policy[] = { + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + static const uint8_t unsatisfiable_policy[VB2_SHA256_DIGEST_SIZE] = "hmwhat if RBR beat merc in 2021"; @@ -411,6 +426,16 @@ return tlcl_lock_nv_write(index); } +uint32_t enterprise_rollback_create_counter(void) +{ + /* + * No need to increment the counter to initialize, this can be done later. + */ + return tlcl_define_space(ENT_ROLLBACK_NV_INDEX, /*size=*/8, + rw_counter_attributes, + empty_policy, sizeof(empty_policy)); +} + #else /** @@ -510,6 +535,14 @@ return tlcl_set_global_lock(); } +uint32_t enterprise_rollback_create_counter(void) +{ + /* + * Do nothing, NVRAM counters don't exist under TPM 1.2. + */ + return TPM_SUCCESS; +} + #endif /** diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 2973934..f439de8 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -275,6 +275,10 @@ if (vboot_setup_tpm(ctx) == TPM_SUCCESS) { antirollback_read_space_firmware(ctx); antirollback_read_space_kernel(ctx); + /* On TPM 2.0, create a counter that survives TPM clear. This allows to + * securely lock data during enterprise rollback by binding to this + * counter's value. */ + enterprise_rollback_create_counter(); } timestamp_add_now(TS_END_TPMINIT); -- To view, visit https://review.coreboot.org/c/coreboot/+/59097 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I511dba3b3461713ce20fb2bda9fced0fee6517e1 Gerrit-Change-Number: 59097 Gerrit-PatchSet: 1 Gerrit-Owner: Miriam Polzer <mpolzer(a)google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: [WIP] util: Add coreboot Configurator
by Sean Rhodes (Code Review) 10 Nov '21

10 Nov '21

Attention is currently required from: Sean Rhodes, Martin Roth, Matt DeVillier, Paul Menzel, Tim Wawrzynczak, Angel Pons. Hello build bot (Jenkins), Martin Roth, Matt DeVillier, Tim Wawrzynczak, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/58854 to look at the new patch set (#49). Change subject: [WIP] util: Add coreboot Configurator ...................................................................... [WIP] util: Add coreboot Configurator A simple GUI to change settings in coreboot's CBFS, via the nvramtool utility. Signed-off-by: Sean Rhodes <sean(a)starlabs.systems> Change-Id: Ie654f6aca57a792be0c295afc081a95ed4d875d1 --- A util/coreboot-configurator/README.md A util/coreboot-configurator/contrib/PKGBUILD A util/coreboot-configurator/contrib/README.md A util/coreboot-configurator/contrib/debian/changelog A util/coreboot-configurator/contrib/debian/compat A util/coreboot-configurator/contrib/debian/control A util/coreboot-configurator/contrib/debian/files A util/coreboot-configurator/contrib/debian/rules A util/coreboot-configurator/contrib/debian/source/format A util/coreboot-configurator/contrib/flatpak/coreboot-configurator.json A util/coreboot-configurator/contrib/flatpak/org.coreboot.coreboot-configurator.json A util/coreboot-configurator/meson.build A util/coreboot-configurator/meson_options.txt A util/coreboot-configurator/screenshots/coreboot-configurator.gif A util/coreboot-configurator/screenshots/coreboot-configurator.png A util/coreboot-configurator/src/application/aboutwindow.cpp A util/coreboot-configurator/src/application/aboutwindow.h A util/coreboot-configurator/src/application/aboutwindow.ui A util/coreboot-configurator/src/application/categories.yaml A util/coreboot-configurator/src/application/configuration.cpp A util/coreboot-configurator/src/application/configuration.h A util/coreboot-configurator/src/application/corebootconfigurator.cpp A util/coreboot-configurator/src/application/corebootconfigurator.h A util/coreboot-configurator/src/application/corebootconfigurator.ui A util/coreboot-configurator/src/application/images.qrc A util/coreboot-configurator/src/application/main.cpp A util/coreboot-configurator/src/application/meson.build A util/coreboot-configurator/src/application/nvramtoolcli.cpp A util/coreboot-configurator/src/application/nvramtoolcli.h A util/coreboot-configurator/src/application/qtyaml.h A util/coreboot-configurator/src/application/toggle-off.svg A util/coreboot-configurator/src/application/toggle-on.svg A util/coreboot-configurator/src/application/toggleswitch.cpp A util/coreboot-configurator/src/application/toggleswitch.h A util/coreboot-configurator/src/application/util.h A util/coreboot-configurator/src/meson.build A util/coreboot-configurator/src/static/aboutIcon.png A util/coreboot-configurator/src/static/categories.yaml A util/coreboot-configurator/src/static/coreboot-configurator.desktop A util/coreboot-configurator/src/static/coreboot_configurator.svg A util/coreboot-configurator/src/static/meson.build A util/coreboot-configurator/src/static/org.coreboot.nvramtool.policy A util/coreboot-configurator/src/static/org.coreboot.reboot.policy 43 files changed, 3,166 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/58854/49 -- To view, visit https://review.coreboot.org/c/coreboot/+/58854 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie654f6aca57a792be0c295afc081a95ed4d875d1 Gerrit-Change-Number: 58854 Gerrit-PatchSet: 49 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.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)mailbox.org> Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021