coreboot-gerrit November 2024

coreboot-gerrit@coreboot.org

1 participants
1933 discussions

[XS] Change in coreboot[main]: soc/amd/common/psp/psp_smi: report errors in 'handle_psp_command'
by Felix Held (Code Review) 21 Nov '24

21 Nov '24

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85238?usp=email ) Change subject: soc/amd/common/psp/psp_smi: report errors in 'handle_psp_command' ...................................................................... soc/amd/common/psp/psp_smi: report errors in 'handle_psp_command' To see if things went wrong in the 'handle_psp_command' function, print the status code in case it's not MBOX_PSP_SUCCESS. Change-Id: I8c02e8e29ab5619282e5b864a8cea6f0703f6ef2 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/soc/amd/common/block/psp/psp_smi.c 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/85238/1 diff --git a/src/soc/amd/common/block/psp/psp_smi.c b/src/soc/amd/common/block/psp/psp_smi.c index 5a1a4b9..762c8ee 100644 --- a/src/soc/amd/common/block/psp/psp_smi.c +++ b/src/soc/amd/common/block/psp/psp_smi.c @@ -173,6 +173,9 @@ if (status == MBOX_PSP_SUCCESS && rd_bios_mbox_checksum_en()) wr_bios_mbox_checksum(calc_psp_buffer_checksum8()); + + if (status != MBOX_PSP_SUCCESS) + printk(BIOS_ERR, "PSP: SMI processing error. staus code %#x\n", status); } /* TODO: check if all wbinvd() calls are necessary */ -- To view, visit https://review.coreboot.org/c/coreboot/+/85238?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8c02e8e29ab5619282e5b864a8cea6f0703f6ef2 Gerrit-Change-Number: 85238 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[L] Change in coreboot[main]: drivers/spi: add RPMC support
by Felix Held (Code Review) 21 Nov '24

21 Nov '24

Felix Held has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/84837?usp=email ) Change subject: drivers/spi: add RPMC support ...................................................................... Patch Set 8: (2 comments) File src/drivers/spi/spi_flash_rpmc.c: https://review.coreboot.org/c/coreboot/+/84837/comment/b902ffb0_3544f4b8?us… : PS2, Line 259: uint32_t counter_data > i'm still unsure if it's better to use a uint32_t or a uint8_t * here. […] should be uint8_t * File src/drivers/spi/spi_flash_rpmc.c: https://review.coreboot.org/c/coreboot/+/84837/comment/8564291d_1f14d3d4?us… : PS7, Line 194: return counter_addr < flash->rpmc_caps.number_of_counters; > enum cb_err vs bool... Done -- To view, visit https://review.coreboot.org/c/coreboot/+/84837?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia9bd69d0105c66bf5ecb6c90e8c782a81912bd40 Gerrit-Change-Number: 84837 Gerrit-PatchSet: 8 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 21 Nov 2024 14:56:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[S] Change in coreboot[main]: soc/amd/common/psp_smi_flash: implement SPI flash RPMC command handling
by Felix Held (Code Review) 21 Nov '24

21 Nov '24

Attention is currently required from: Fred Reitberger, Jason Glenesk, Matt DeVillier. Felix Held has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/84906?usp=email ) Change subject: soc/amd/common/psp_smi_flash: implement SPI flash RPMC command handling ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/84906?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I20e4f60d4e35d33e560fc43212b320e817e13004 Gerrit-Change-Number: 84906 Gerrit-PatchSet: 7 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Thu, 21 Nov 2024 14:56:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No

1 0

[L] Change in coreboot[main]: drivers/spi: add RPMC support
by Felix Held (Code Review) 21 Nov '24

21 Nov '24

1 0

[S] Change in coreboot[main]: soc/amd/common/psp: add delay after boot done command in PSP SMI case
by Felix Held (Code Review) 21 Nov '24

21 Nov '24

Attention is currently required from: Fred Reitberger, Jason Glenesk, Matt DeVillier. Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85237?usp=email ) Change subject: soc/amd/common/psp: add delay after boot done command in PSP SMI case ...................................................................... soc/amd/common/psp: add delay after boot done command in PSP SMI case When the PSP SMI support is enabled, the PSP might send PSP SMI commands to the SMI handler to access the SPI flash. This seems to interfere with some Windows kernel driver, causing the OS to get stuck early in the boot process somewhere between when it sends the enter ACPI mode SMI and when it starts the boot animation. Adding a 5s delay after the boot done PSP mailbox command in this case makes sure that this will be done before the OS takes over which avoids the problem. TEST=Now Windows 10 boots successfully every time when the PSP SMI support is enabled. Before that the first boot after power on failed due to PSP SMI flash access activity or possibly due to the PSP not being done with its flash write and RPMC activity. In subsequent boots no flash access PSP SMIs were sent from the PSP and the system booted successfully. Change-Id: Icd4b76af829cfaa48e02da2ed224599b863af3b0 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/soc/amd/common/block/psp/psp.c 1 file changed, 13 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/85237/1 diff --git a/src/soc/amd/common/block/psp/psp.c b/src/soc/amd/common/block/psp/psp.c index 9ad04e1..822887e 100644 --- a/src/soc/amd/common/block/psp/psp.c +++ b/src/soc/amd/common/block/psp/psp.c @@ -3,6 +3,7 @@ #include <device/mmio.h> #include <bootstate.h> #include <console/console.h> +#include <delay.h> #include <amdblocks/psp.h> #include <soc/iomap.h> #include "psp_def.h" @@ -121,6 +122,18 @@ /* buffer's status shouldn't change but report it if it does */ psp_print_cmd_status(cmd_status, &buffer.header); + + /* + * When SOC_AMD_COMMON_BLOCK_PSP_SMI is selected, the PSP might send some PSP SMI + * commands after the MBOX_BIOS_CMD_BOOT_DONE command has been sent to the PSP. Since + * coreboot is rather fast, this happens after coreboot handed over to the payload and + * OS. This caused symptoms like the Windows kernel getting stuck very early during + * boot shortly after the kernel has sent the SMI to enter ACPI mode. Adding a delay in + * this case, makes the problem go away. Not too happy about that, but I'm not sure yet + * how to handle that in a better way. + */ + if (CONFIG(SOC_AMD_COMMON_BLOCK_PSP_SMI)) + delay(5); } BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_BOOT, BS_ON_ENTRY, psp_notify_boot_done, NULL); -- To view, visit https://review.coreboot.org/c/coreboot/+/85237?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Icd4b76af829cfaa48e02da2ed224599b863af3b0 Gerrit-Change-Number: 85237 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>

1 0

[XS] Change in coreboot[main]: soc/amd/common/block/psp/psp_smi_flash.h: fix struct element types
by Felix Held (Code Review) 21 Nov '24

21 Nov '24

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85236?usp=email ) Change subject: soc/amd/common/block/psp/psp_smi_flash.h: fix struct element types ...................................................................... soc/amd/common/block/psp/psp_smi_flash.h: fix struct element types Commit ee93b35bc3dd ("soc/amd/common/psp_smi_flash: add RPMC command- specific data structures") added the 'psp_spi_rpmc_inc_mc' and 'psp_smi_rpmc_req_mc' structs, but added the counter data as uint32_t while it should have been an array of 4 uint8_t, since the bytes in that buffer are already in the order in which they need to be sent over to the SPI flash which is different than the byte order of a uint32_t. This was only noticed after getting the code that uses these structs was tested. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I6c290535a1896c080b74d892cb289e6e122d4525 --- M src/soc/amd/common/block/psp/psp_smi_flash.h 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/85236/1 diff --git a/src/soc/amd/common/block/psp/psp_smi_flash.h b/src/soc/amd/common/block/psp/psp_smi_flash.h index 79593b5..789e28d 100644 --- a/src/soc/amd/common/block/psp/psp_smi_flash.h +++ b/src/soc/amd/common/block/psp/psp_smi_flash.h @@ -53,7 +53,7 @@ struct psp_spi_rpmc_inc_mc { uint32_t counter_address; - uint32_t counter_data; + uint8_t counter_data[SPI_RPMC_COUNTER_DATA_LEN]; uint8_t signature[SPI_RPMC_SIG_LEN]; } __packed; @@ -66,7 +66,7 @@ uint32_t counter_address; uint8_t tag[SPI_RPMC_TAG_LEN]; uint8_t signature[SPI_RPMC_SIG_LEN]; - uint32_t output_counter_data; + uint8_t output_counter_data[SPI_RPMC_COUNTER_DATA_LEN]; uint8_t output_signature[SPI_RPMC_SIG_LEN]; } __packed; -- To view, visit https://review.coreboot.org/c/coreboot/+/85236?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6c290535a1896c080b74d892cb289e6e122d4525 Gerrit-Change-Number: 85236 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[XS] Change in coreboot[main]: doc/util/ifdtool: Update instructions
by Maximilian Brune (Code Review) 21 Nov '24

21 Nov '24

Maximilian Brune has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/85235?usp=email ) Change subject: doc/util/ifdtool: Update instructions ...................................................................... doc/util/ifdtool: Update instructions - Add step for building ifdtool (might not be obvious) - Remove "./ifdtool COREBOOT_NAME" because it does nothing - Add a small comment explaining what the -d and -x args do. Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: I868ea8918a1566cfade3bc161117f2ca8dfed31d --- M Documentation/util/ifdtool/binary_extraction.md 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/85235/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85235?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I868ea8918a1566cfade3bc161117f2ca8dfed31d Gerrit-Change-Number: 85235 Gerrit-PatchSet: 2 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>

1 0

[XS] Change in coreboot[main]: doc/util/ifdtool: Add step for building ifdtool
by Maximilian Brune (Code Review) 21 Nov '24

21 Nov '24

Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85235?usp=email ) Change subject: doc/util/ifdtool: Add step for building ifdtool ...................................................................... doc/util/ifdtool: Add step for building ifdtool It might not be as obvious as one might think. Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: I868ea8918a1566cfade3bc161117f2ca8dfed31d --- M Documentation/util/ifdtool/binary_extraction.md 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/85235/1 diff --git a/Documentation/util/ifdtool/binary_extraction.md b/Documentation/util/ifdtool/binary_extraction.md index fe8c25c..0663bd6 100644 --- a/Documentation/util/ifdtool/binary_extraction.md +++ b/Documentation/util/ifdtool/binary_extraction.md @@ -9,6 +9,7 @@ **Note:** Make sure you are in the root coreboot directory. cd /path/to/coreboot/util/ifdtool + make ./ifdtool COREBOOT_IMAGE ./ifdtool -d COREBOOT_IMAGE ./ifdtool -x COREBOOT_IMAGE -- To view, visit https://review.coreboot.org/c/coreboot/+/85235?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I868ea8918a1566cfade3bc161117f2ca8dfed31d Gerrit-Change-Number: 85235 Gerrit-PatchSet: 1 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>

1 0

[XS] Change in coreboot[main]: soc/intel/meteorlake: Hook up public microcode
by Michał Kopeć (Code Review) 21 Nov '24

21 Nov '24

Michał Kopeć has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/82686?usp=email ) Change subject: soc/intel/meteorlake: Hook up public microcode ...................................................................... Abandoned Superseded by https://review.coreboot.org/c/coreboot/+/84125 -- To view, visit https://review.coreboot.org/c/coreboot/+/82686?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I16f20956a1490da02acc24156360aef235111494 Gerrit-Change-Number: 82686 Gerrit-PatchSet: 2 Gerrit-Owner: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.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>

1 0

[XS] Change in coreboot[main]: mb/google/fatcat: Adjust EC host command range for fatcat_ish variant
by Jayvik Desai (Code Review) 21 Nov '24

21 Nov '24

Attention is currently required from: Paul Menzel. Jayvik Desai has posted comments on this change by Jayvik Desai. ( https://review.coreboot.org/c/coreboot/+/85221?usp=email ) Change subject: mb/google/fatcat: Adjust EC host command range for fatcat_ish variant ...................................................................... Patch Set 2: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/85221/comment/c56b5093_783654b5?us… : PS1, Line 7: for fatcat_ish variant > Make this part of the prefix? Maintaining the code path in prefix https://review.coreboot.org/c/coreboot/+/85221/comment/0dd1a514_199f10b3?us… : PS1, Line 9: This commit adjusts the EC host command range for the fatcat_ish variant > “This commit” is redundant. […] Acknowledged https://review.coreboot.org/c/coreboot/+/85221/comment/de99f3f6_cd33bbe8?us… : PS1, Line 9: This commit adjusts the EC host command range for the fatcat_ish variant : to 0x800-0x807 & 0x200-0x20f. > According to what source? Added some more details -- To view, visit https://review.coreboot.org/c/coreboot/+/85221?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0d726d60d2a15d2dfaff35f570de479fdc6d15aa Gerrit-Change-Number: 85221 Gerrit-PatchSet: 2 Gerrit-Owner: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(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-Comment-Date: Thu, 21 Nov 2024 13:48:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2024