coreboot-gerrit November 2023

coreboot-gerrit@coreboot.org

1 participants
3036 discussions

[S] Change in coreboot[main]: arch/arm64/arch_timer: Fix possible overflow in multiplication
by Julius Werner (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Arthur Heymans, Paul Menzel, Yidi Lin, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78800?usp=email ) Change subject: arch/arm64/arch_timer: Fix possible overflow in multiplication ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: Would be great if you could also add a patch to fix this in libpayload! -- To view, visit https://review.coreboot.org/c/coreboot/+/78800?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I366667de05392913150414f0fa9058725be71c52 Gerrit-Change-Number: 78800 Gerrit-PatchSet: 3 Gerrit-Owner: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Wed, 01 Nov 2023 22:10:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: Use common GCD function
by Julius Werner (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Arthur Heymans, Yidi Lin, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78799?usp=email ) Change subject: Use common GCD function ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78799?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I30e4b02a9ca6a15c9bc4edcf4143ffa13a21a732 Gerrit-Change-Number: 78799 Gerrit-PatchSet: 2 Gerrit-Owner: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Wed, 01 Nov 2023 22:07:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: lib: Add GCD function
by Julius Werner (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Arthur Heymans, Paul Menzel, Yidi Lin, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78798?usp=email ) Change subject: lib: Add GCD function ...................................................................... Patch Set 2: (4 comments) File src/commonlib/bsd/gcd.c: https://review.coreboot.org/c/coreboot/+/78798/comment/caac2e4a_462afb80 : PS2, Line 6: unsigned long gcd(unsigned long a, unsigned long b) > Change to uint64_t to allow larger values? Looks like all the use cases we have are 32-bit and on x86 doing this at 64-bit would be expensive, so maybe we should restrict this function to uint32_t for now? (Maybe call it `gcd32()` then to clarify?) I agree that `unsigned long` seems a bad choice, it's better to be clear about type widths. https://review.coreboot.org/c/coreboot/+/78798/comment/3cea4537_5c9c15c9 : PS2, Line 10: if (a == 0 || b == 0) Shouldn't we just let these cases run into the divide-by-zero? I wouldn't consider asking for the GCD of 0 valid. https://review.coreboot.org/c/coreboot/+/78798/comment/e4df50fa_8be6c5aa : PS2, Line 12: if (a < b) I think writing this explicitly is unnecessary. If `a < b`, then `c = a % b` means `c = a`, so in the first loop iteration you'll get `a = b`, `b = c (the old a)` and then you're back on track. File src/commonlib/bsd/include/commonlib/bsd/gcd.h: https://review.coreboot.org/c/coreboot/+/78798/comment/b139cfc4_e9b2a36d : PS2, Line 3: #ifndef __GCD_H__ nit: Include guards should include the COMMONLIB_BSD somewhere so that there won't be any clashes in case we ever get a src/include/gcd.h as well. -- To view, visit https://review.coreboot.org/c/coreboot/+/78798?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I21819cda4299b3809b8ca7a95cbdc6a87e4b3481 Gerrit-Change-Number: 78798 Gerrit-PatchSet: 2 Gerrit-Owner: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Wed, 01 Nov 2023 22:06:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: vboot: Add catchall recovery reason for unspecified phase 4 errors
by Julius Werner (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Yu-Ping Wu. Hello build bot (Jenkins), Yu-Ping Wu, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/78866?usp=email to review the following change. Change subject: vboot: Add catchall recovery reason for unspecified phase 4 errors ...................................................................... vboot: Add catchall recovery reason for unspecified phase 4 errors The code for "phase 4" of firmware verification currently only sets a recovery reason when there's an actual hash mismatch detected in vb2api_check_hash_get_digest(). This is the most likely way how this section of code can fail but not the only one. If any other unexpected issue occurs, we should still set a recovery reason rather than just reboot and risk an infinite boot loop. This patch adds a catchall recovery reason for any error code that falls out of this block of code. If a more specific recovery reason had already been set beforehand, we'll continue to use that -- if not, we'll set VB2_RECOVERY_FW_GET_FW_BODY. Change-Id: If00f00f00f00aa113e0325aad58d367f244aca49 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78821 Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/security/vboot/vboot_logic.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/78866/1 diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 11983b9..93a188c 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -374,7 +374,7 @@ } if (rv) - vboot_save_and_reboot(ctx, rv); + vboot_fail_and_reboot(ctx, VB2_RECOVERY_FW_GET_FW_BODY, rv); vboot_save_data(ctx); /* Only extend PCRs once on boot. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/78866?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If00f00f00f00aa113e0325aad58d367f244aca49 Gerrit-Change-Number: 78866 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: Update vboot submodule to upstream main
by Julius Werner (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Eric Lai, Felix Singer, Yu-Ping Wu. Hello Felix Singer, Eric Lai, build bot (Jenkins), Yu-Ping Wu, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/78865?usp=email to review the following change. Change subject: Update vboot submodule to upstream main ...................................................................... Update vboot submodule to upstream main Updating from commit id 24cb127a: 2023-08-21 Nicholas Bishop sign_uefi_unittest.py: Fix long-line lint to commit id f2b01bf0: 2023-10-27 Julius Werner firmware: Undeprecate VB2_RECOVERY_FW_GET_FW_BODY This brings in 47 new commits: f2b01bf0 firmware: Undeprecate VB2_RECOVERY_FW_GET_FW_BODY ef6d02df futility/vb2_helper: Add missing newline for error messages 886d13d7 PRESUBMIT: switch to cros format ac2e1a75 host/lib: Decouple openssl headers from HOSTLIB 86ec05f7 futility: updater: Add help info for --quirks 2850244e futility: updater: Abort if the unlock_csme_* is used on a locked device f1b5c88d devkeys: delete old unused firmware_bmpfv.bin 4444c5fe crossystem: Fix tpm_fwver for fwid < 12935 98ef339f 2lib: Prevent overwriting the value of fw_vboot2 c7517eb4 make_dev_ssd: support ChromeOS Kdump 8e3462cc tlcl: Increase the TPM_BUFFER_SIZE 740a2966 vboot_reference: Drop 'host' usage for 'internal' in flashrom.h 57877a44 vboot: Remove comments about physical dev switch 3401d16c 2lib: Fix typos, comments and formats fdf52d45 scripts/: Drop deprecated {g,s}et_gbb_flags.sh scripts bf76e9ee 2lib: Output the correct kernel_version 1ac4663e make_dev_firmware.sh: update pattern for matching wp status c57ab9f7 2lib: Add recovery reason VB2_RECOVERY_WIDEVINE_PREPARE e094ba31 tlcl: Reduce the variants of TPM2B b047600d sign_official_build: support key config for pkcs11 f8712b73 vboot: support signing with pkcs11 private key 17fe786f strip_boot_from_image.sh: sfill fast 6c856cd3 futility/updater: Fix EC software write protection logic 1dc5a421 futility: update: Deprecate --unlock_me by --quirk unlock_csme_nissa f0d88587 futility: update: Refactor the 'unlock ME' quirk(s) 81429ee9 futility: update: Do not update RO when the AP RO is locked a3beb737 futility: update: Revise the ordering or quirks 2c1844fa futility: update: Remove unused quirk 'unlock_wilco_me_for_update' 75530d32 tests/futility: Test with new signer_config.csv based firmware updater cba649fa 2lib: Expose 2hmac ab015448 2lib: Refactor hmac to vb2_hmac_calculate 3545f8b4 Revert "sign_uefi: Remove exception catching" 55f625a9 dump_fmap: Add offset and size to flash_ec format output a27ee336 keygeneration: add shellcheck source statements to help linting 055f9aa2 keygeneration: replace_recovery_key.sh: make minios key optional 6cb8ab60 scripts: delete unused values kernel command line 1f76c38b vboot: Drop phone recovery support ccf6b037 scripts: Legacy fix for set_gbb_flags.sh 8f03069e futility: Add basic README.md 88963df8 utility: Query platform wp status with futility 6c3817d2 utility: Drop cros_alias technical debt in dev_debug_vboot df85f512 scripts: Drop cros_alias technical debt in make_dev_firmware.sh 7395cd68 futility/updater_utils.c: Match on EC path to prepare for split 52518415 crossystem: Recover corrupted RW_NVRAM on flash writes 81f9ddaf futility/cmd_gbb_utility.md: Add basic GBB subcmd doc c4995268 futility/: Fix define confusion 69dab5a6 crossystem: Avoid writing duplicate entries to RW_NVRAM 6c37b520 Revert "crossystem: stop supporting legacy chromeos_acpi driver" Change-Id: Ic7ecdabcdd26df349b8abf1c5a77c806facfe1d8 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78820 Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Reviewed-by: Eric Lai <ericllai(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> --- M 3rdparty/vboot 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/78865/1 diff --git a/3rdparty/vboot b/3rdparty/vboot index 24cb127..f2b01bf 160000 --- a/3rdparty/vboot +++ b/3rdparty/vboot @@ -1 +1 @@ -Subproject commit 24cb127a5e83a713131ab75cc39b11336019443c +Subproject commit f2b01bf08a813fb3035102c7c948b78fa191e544 -- To view, visit https://review.coreboot.org/c/coreboot/+/78865?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic7ecdabcdd26df349b8abf1c5a77c806facfe1d8 Gerrit-Change-Number: 78865 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: soc/intel/mtl: Display Sign-of-Life message using FSP-M
by Jérémy Compostella (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Eran Mitrani, Kapil Porwal, Shelley Chen, Subrata Banik, Tarun. Jérémy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78244?usp=email ) Change subject: soc/intel/mtl: Display Sign-of-Life message using FSP-M ...................................................................... Patch Set 14: (4 comments) File src/soc/intel/meteorlake/Kconfig: https://review.coreboot.org/c/coreboot/+/78244/comment/0f581783_4527d8bf : PS12, Line 458: SOC_INTEL_METEORLAKE_SIGN_OF_LIFE > this should be somewhere in the common code as we are planning to use this feature now onwards for a […] As discussed earlier today, I suggest to keep as-is for now and move it to common once code once we know it really becomes common code. File src/soc/intel/meteorlake/romstage/fsp_params.c: https://review.coreboot.org/c/coreboot/+/78244/comment/71666461_291081e5 : PS12, Line 468: 1 > use the same log as GFX PEIM […] Done. https://review.coreboot.org/c/coreboot/+/78244/comment/5600ec49_7dad78bb : PS12, Line 469: VgaMessage > the text alignment logic is part of FSP-M code I assume ? Correct and this is not configurable at the moment. https://review.coreboot.org/c/coreboot/+/78244/comment/101b936f_d8a5025a : PS12, Line 497: fill_fspm_sign_of_life > ``` […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/78244?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8e7772582b1895fa8e38780932346683be998558 Gerrit-Change-Number: 78244 Gerrit-PatchSet: 14 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Wed, 01 Nov 2023 20:40:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: acpi: Add PPTT support
by David Milosevic (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Arthur Heymans, Cliff Huang, Lance Zhao, Maximilian Brune, Nico Huber, Patrick Rudolph, Tim Wawrzynczak. David Milosevic has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78071?usp=email ) Change subject: acpi: Add PPTT support ...................................................................... Patch Set 9: (7 comments) File src/acpi/acpi_pptt.c: https://review.coreboot.org/c/coreboot/+/78071/comment/166f1ef1_775ae89f : PS6, Line 66: > I agree with Arthur here. […] Guess I am outnumbered here. I will drop the newline. File src/acpi/acpi_pptt.c: https://review.coreboot.org/c/coreboot/+/78071/comment/fdde42ee_86291740 : PS8, Line 13: * Disclaimer: : * : * The generated PPTT table might not be optimized for space, since this : * implementation creates unique resources (caches) for each CPU provided by : * the topology tree. Going by the ACPI 6.4 spec, this is fine. We do this to : * avoid further edge cases and keep the logic as simple as possible. : * : * "though less space efficient, it is also acceptable to declare a node : * for each instance of a resource. In the example above, it would be legal to : * declare an L1 for each processor." : * : * "Compaction of identical resources must be avoided if an implementation requires : * any resource instance to be referenced uniquely. For example, in the above example, : * the L1 resource of each processor must be declared using a dedicated structure to : * permit unique references to it." : * > I found this to be more confusing than helpful. The quotes reference examples […] With this comment I wanted to explain, that there is still room for optimizations regarding the table size, since the current approach creates unique structures instead of compacting them, in order to reduce code complexity. The quotes simply serve as proof, that this is, nevertheless, still valid ACPI. Quote 1 allows us to declare unique resources instead of compacting them into a single resource. Quote 2 states, that there might be cases, where we really need unique resources (in order to have unique references), and therefore, have to avoid compaction. By leveraging Quote 1, we avoid dealing with potential edge cases (see Quote 2), as we completely avoid resource compaction. Regarding converging cache linked lists: I will take a look. Its probably something we would have to implement in order to be spec compliant. https://review.coreboot.org/c/coreboot/+/78071/comment/987bd2f0_b144b690 : PS8, Line 40: * the difference between the start of the PPTT table > Nit, I would call it `offset`. And TBH, I don't think we need to say […] I agree, the diagram is not really needed for such a simple concept, but was requested by Arthur, so I think we should keep it. I will adjust the description though. https://review.coreboot.org/c/coreboot/+/78071/comment/e224b057_1595d51d : PS8, Line 56: u32 > It's uncommon to use a fixed-size integer unless you need one (e.g. for […] Although we don't populate the PPTT table directly with the return value of "count_resources", it is still based on the "Number of private resources" field (acpi_pptt_cpu_node_t.n_resources) which is exactly 4 bytes wide. That's why I chose "u32". https://review.coreboot.org/c/coreboot/+/78071/comment/257e460d_9f3b2303 : PS8, Line 127: setup_topology(node->sibling, parent_ref, current); > This is just walking the list without any reference to the current […] To be honest, I find it harder to understand/read when we mix recursion with loops. Not sure, if this is an improvement. Maybe a third opinion would be nice? File src/include/acpi/acpi.h: https://review.coreboot.org/c/coreboot/+/78071/comment/7598f8e4_e077332a : PS4, Line 1412: acpi_pptt_cpu_node > If you don't mind, I would mark this comment as done by end of tomorrow. Done https://review.coreboot.org/c/coreboot/+/78071/comment/f3fc7af2_db22f6b5 : PS4, Line 1416: u > If you don't mind, I would mark this comment as done by end of tomorrow. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/78071?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia119e1ba15756704668116bdbc655190ec94ff10 Gerrit-Change-Number: 78071 Gerrit-PatchSet: 9 Gerrit-Owner: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 01 Nov 2023 20:36:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: acpi: Add PPTT support
by David Milosevic (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Arthur Heymans, Cliff Huang, David Milosevic, Lance Zhao, Maximilian Brune, Patrick Rudolph, Tim Wawrzynczak. Hello Arthur Heymans, Cliff Huang, Lance Zhao, Maximilian Brune, Tim Wawrzynczak, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78071?usp=email to look at the new patch set (#9). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: acpi: Add PPTT support ...................................................................... acpi: Add PPTT support This patch adds code to generate Processor Properties Topology Tables (PPTT) compliant to the ACPI 6.4 specification. - The 'acpi_get_pptt_topology' hook is mandatory once ACPI_PPTT is selected. Its purpose is to return a pointer to a topology tree, which describes the relationship between CPUs and caches. The hook can be provided by, for example, mainboard code. - The optional hook 'acpi_cleanup_pptt_topology' can be implemented to cleanup remainders of 'acpi_get_pptt_topology'. For example, one might use dynamic memory allocations in order to setup the topology tree. The hook can then be used to free that memory. Background: We are currently working on a mainboard which requires a valid PPTT table. Patch was tested for that mainboard. Change-Id: Ia119e1ba15756704668116bdbc655190ec94ff10 Signed-off-by: David Milosevic <David.Milosevic(a)9elements.com> --- M src/acpi/Kconfig M src/acpi/Makefile.inc M src/acpi/acpi.c A src/acpi/acpi_pptt.c M src/include/acpi/acpi.h 5 files changed, 285 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/78071/9 -- To view, visit https://review.coreboot.org/c/coreboot/+/78071?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia119e1ba15756704668116bdbc655190ec94ff10 Gerrit-Change-Number: 78071 Gerrit-PatchSet: 9 Gerrit-Owner: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: soc/intel/cmn/gfx: Join MBUS while FSP-S performs GFX init
by Subrata Banik (Code Review) 01 Nov '23

01 Nov '23

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78802?usp=email ) Change subject: soc/intel/cmn/gfx: Join MBUS while FSP-S performs GFX init ...................................................................... soc/intel/cmn/gfx: Join MBUS while FSP-S performs GFX init This patch calls into the function to join the MBUS if the GFX PEIM module inside the FSP binary is taking care of graphics initialization based on the RUN_FSP_GOP config option. The FW skips joining the MBUS in case of a non-FSP solution and/or SOC_INTEL_GFX_MBUS_JOIN config is not enabled. BUG=b:284799726 TEST=MBUS joining is only applicable for google/rex while using GFX PEIM. Change-Id: I50d719a286722f5aafbad48ab4ca60500c836dd6 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/soc/intel/common/block/graphics/graphics.c 1 file changed, 32 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/78802/1 diff --git a/src/soc/intel/common/block/graphics/graphics.c b/src/soc/intel/common/block/graphics/graphics.c index e744ca7..9b780da 100644 --- a/src/soc/intel/common/block/graphics/graphics.c +++ b/src/soc/intel/common/block/graphics/graphics.c @@ -280,31 +280,42 @@ } } +static void graphics_join_mbus(void) +{ + enum display_type type = get_external_display_status(); + uint32_t hashing_mode = 0; /* 2x2 */ + if (type == INTERNAL_DISPLAY_ONLY) { + hashing_mode = GFX_MBUS_HASHING_MODE; /* 1x4 */ + /* Only eDP pipes is joining the MBUS */ + graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_A), PIPE_A, GFX_MBUS_JOIN | hashing_mode); + } else if (type == DUAL_DISPLAY) { + /* All pipes are joining the MBUS */ + graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_A), PIPE_A, GFX_MBUS_JOIN | hashing_mode); + graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_B), PIPE_B, GFX_MBUS_JOIN | hashing_mode); + graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_C), PIPE_C, GFX_MBUS_JOIN | hashing_mode); +#if CONFIG(INTEL_GMA_VERSION_2) + graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_D), PIPE_D, GFX_MBUS_JOIN | hashing_mode); +#endif + } else { + /* No pipe joins the MBUS */ + graphics_gtt_rmw(GFX_MBUS_CTL, GFX_MBUS_JOIN_PIPE_SEL, + GFX_MBUS_JOIN | hashing_mode); + } +} + static void graphics_dev_final(struct device *dev) { pci_dev_request_bus_master(dev); - if (CONFIG(SOC_INTEL_GFX_MBUS_JOIN)) { - enum display_type type = get_external_display_status(); - uint32_t hashing_mode = 0; /* 2x2 */ - if (type == INTERNAL_DISPLAY_ONLY) { - hashing_mode = GFX_MBUS_HASHING_MODE; /* 1x4 */ - /* Only eDP pipes is joining the MBUS */ - graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_A), PIPE_A, GFX_MBUS_JOIN | hashing_mode); - } else if (type == DUAL_DISPLAY) { - /* All pipes are joining the MBUS */ - graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_A), PIPE_A, GFX_MBUS_JOIN | hashing_mode); - graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_B), PIPE_B, GFX_MBUS_JOIN | hashing_mode); - graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_C), PIPE_C, GFX_MBUS_JOIN | hashing_mode); -#if CONFIG(INTEL_GMA_VERSION_2) - graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_D), PIPE_D, GFX_MBUS_JOIN | hashing_mode); -#endif - } else { - /* No pipe joins the MBUS */ - graphics_gtt_rmw(GFX_MBUS_CTL, GFX_MBUS_JOIN_PIPE_SEL, - GFX_MBUS_JOIN | hashing_mode); - } - } + /* + * Call function to join the MBUS if GFX PEIM module inside FSP + * binary is taking care of graphics initialization based on + * RUN_FSP_GOP config option. + * + * Skip FW joining the MBUS in case of non-FSP solution. + */ + if (CONFIG(RUN_FSP_GOP) && CONFIG(SOC_INTEL_GFX_MBUS_JOIN) && display_init_required()) + graphics_join_mbus(); } const struct device_operations graphics_ops = { -- To view, visit https://review.coreboot.org/c/coreboot/+/78802?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I50d719a286722f5aafbad48ab4ca60500c836dd6 Gerrit-Change-Number: 78802 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: soc/intel/mtl: Display Sign-of-Life message using FSP-M
by Jérémy Compostella (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Eran Mitrani, Jérémy Compostella, Kapil Porwal, Shelley Chen, Subrata Banik, Tarun. Hello Eran Mitrani, Kapil Porwal, Shelley Chen, Subrata Banik, Tarun, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78244?usp=email to look at the new patch set (#14). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/intel/mtl: Display Sign-of-Life message using FSP-M ...................................................................... soc/intel/mtl: Display Sign-of-Life message using FSP-M Meteor Lake FSP-M, compiled with the appropriate option, allows to display a text message on screen. If the newly introduced SOC_INTEL_METEORLAKE_SIGN_OF_LIFE flag is set coreboot configures the appropriate UPD to display a text message during memory training and CSME update. BUG=b:279173035 TEST=Text message is displayed during memory training on rex Change-Id: I8e7772582b1895fa8e38780932346683be998558 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/soc/intel/meteorlake/Kconfig M src/soc/intel/meteorlake/romstage/fsp_params.c 2 files changed, 69 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/78244/14 -- To view, visit https://review.coreboot.org/c/coreboot/+/78244?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8e7772582b1895fa8e38780932346683be998558 Gerrit-Change-Number: 78244 Gerrit-PatchSet: 14 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2023