coreboot-gerrit December 2024

coreboot-gerrit@coreboot.org

1 participants
3018 discussions

[XS] Change in coreboot[main]: superio/ite: Add missing pnp_set_logical_device in ite_kill_watchdog
by Michał Żygowski (Code Review) 03 Dec '24

03 Dec '24

Michał Żygowski has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/85453?usp=email ) Change subject: superio/ite: Add missing pnp_set_logical_device in ite_kill_watchdog ...................................................................... superio/ite: Add missing pnp_set_logical_device in ite_kill_watchdog The watchdog registers are available in the GPIO LDN. Global LDN may not be sufficient to access the watchdog registers. Switch to GPIO LDN before disabling watchdog. Change-Id: Id88472d474e947987dd6a63873057551cdcc2e31 Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> --- M src/superio/ite/common/early_serial.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/85453/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85453?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: Id88472d474e947987dd6a63873057551cdcc2e31 Gerrit-Change-Number: 85453 Gerrit-PatchSet: 2 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>

1 0

[XS] Change in coreboot[main]: superio/ite/common: Add missing pnp_set_logical_device in ite_kill_wa...
by Michał Żygowski (Code Review) 03 Dec '24

03 Dec '24

Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85453?usp=email ) Change subject: superio/ite/common: Add missing pnp_set_logical_device in ite_kill_watchdog ...................................................................... superio/ite/common: Add missing pnp_set_logical_device in ite_kill_watchdog The watchdog registers are available in the GPIO LDN. Global LDN may not be sufficient to access the watchdog registers. Switch to GPIO LDN before disabling watchdog. Change-Id: Id88472d474e947987dd6a63873057551cdcc2e31 Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> --- M src/superio/ite/common/early_serial.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/85453/1 diff --git a/src/superio/ite/common/early_serial.c b/src/superio/ite/common/early_serial.c index ec218a6..66c4de9 100644 --- a/src/superio/ite/common/early_serial.c +++ b/src/superio/ite/common/early_serial.c @@ -137,6 +137,7 @@ void ite_kill_watchdog(pnp_devfn_t dev) { pnp_enter_conf_state(dev); + pnp_set_logical_device(dev); ite_sio_write(dev, ITE_CONFIG_REG_WATCHDOG, 0x00); ite_sio_write(dev, ITE_CONFIG_REG_WDT_TIMEOUT_LSB, 0x00); ite_sio_write(dev, ITE_CONFIG_REG_WDT_TIMEOUT_MSB, 0x00); -- To view, visit https://review.coreboot.org/c/coreboot/+/85453?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: Id88472d474e947987dd6a63873057551cdcc2e31 Gerrit-Change-Number: 85453 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>

1 0

[XS] Change in coreboot[main]: soc/intel/pantherlake: Bind SoC config VR settings to respective UPD
by Jérémy Compostella (Code Review) 03 Dec '24

03 Dec '24

Jérémy Compostella has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/85131?usp=email ) Change subject: soc/intel/pantherlake: Bind SoC config VR settings to respective UPD ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/85131?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: Ie72e4725cb97b4af7843a43eeaedd687d28b6752 Gerrit-Change-Number: 85131 Gerrit-PatchSet: 4 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 03 Dec 2024 17:51:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: soc/intel/pantherlake: Remove unnecessary cep_enable SoC chip field
by Jérémy Compostella (Code Review) 03 Dec '24

03 Dec '24

Jérémy Compostella has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85452?usp=email ) Change subject: soc/intel/pantherlake: Remove unnecessary cep_enable SoC chip field ...................................................................... soc/intel/pantherlake: Remove unnecessary cep_enable SoC chip field The cep_enable was used on previous platforms to fill the CepEnable UPD, which is not available on Panther Lake because the Current Excursion Protection no longer exists. BUG=b:357011633 TEST=fatcat board build successfully Change-Id: I42d8c793ac3f33eca212320605b16f3b92c60b9c Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/soc/intel/pantherlake/chip.h 1 file changed, 0 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/85452/1 diff --git a/src/soc/intel/pantherlake/chip.h b/src/soc/intel/pantherlake/chip.h index 5846037..59653bd 100644 --- a/src/soc/intel/pantherlake/chip.h +++ b/src/soc/intel/pantherlake/chip.h @@ -295,12 +295,6 @@ bool enable_fast_vmode[NUM_VR_DOMAINS]; /* - * Current Excursion Protection needs to be set for each VR domain - * in order to be able to enable fast Vmode. - */ - bool cep_enable[NUM_VR_DOMAINS]; - - /* * VR Fast Vmode I_TRIP threshold. * 0-255A in 1/4 A units. Example: 400 = 100A * This setting overrides the default value set by FSPs when Fast VMode -- To view, visit https://review.coreboot.org/c/coreboot/+/85452?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: I42d8c793ac3f33eca212320605b16f3b92c60b9c Gerrit-Change-Number: 85452 Gerrit-PatchSet: 1 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

[XS] Change in coreboot[main]: soc/amd/common/psp/psp_def.h: increase P2C_BUFFER_MAXSIZE
by Felix Held (Code Review) 03 Dec '24

03 Dec '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85357?usp=email ) ( 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/amd/common/psp/psp_def.h: increase P2C_BUFFER_MAXSIZE ...................................................................... soc/amd/common/psp/psp_def.h: increase P2C_BUFFER_MAXSIZE This matches the size used in the reference code and required by the corresponding document #55758 Rev. 2.04. This doesn't seem to make any difference in runtime behavior, but I'd rather waste a kilobyte of SMM RAM, than debugging possible problems caused from not following the corresponding specification. Change-Id: I2ee30d6d1255317efcd3960016069dfe50885aa7 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85357 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> --- M src/soc/amd/common/block/psp/psp_def.h 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Marshall Dawson: Looks good to me, approved diff --git a/src/soc/amd/common/block/psp/psp_def.h b/src/soc/amd/common/block/psp/psp_def.h index d11199d..369ef08 100644 --- a/src/soc/amd/common/block/psp/psp_def.h +++ b/src/soc/amd/common/block/psp/psp_def.h @@ -133,7 +133,7 @@ #define PSP_CMD_TIMEOUT 1000 /* 1 second */ #define C2P_BUFFER_MAXSIZE 0xc00 /* Core-to-PSP buffer */ -#define P2C_BUFFER_MAXSIZE 0xc00 /* PSP-to-core buffer */ +#define P2C_BUFFER_MAXSIZE 0x1000 /* PSP-to-core buffer */ /* PSP to x86 status */ enum mbox_p2c_status { -- To view, visit https://review.coreboot.org/c/coreboot/+/85357?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2ee30d6d1255317efcd3960016069dfe50885aa7 Gerrit-Change-Number: 85357 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Marshall Dawson <marshalldawson3rd(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>

1 0

[XS] Change in coreboot[main]: soc/amd/common/psp/rpmc: fix printk format string
by Felix Held (Code Review) 03 Dec '24

03 Dec '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85251?usp=email ) Change subject: soc/amd/common/psp/rpmc: fix printk format string ...................................................................... soc/amd/common/psp/rpmc: fix printk format string While gcc didn't seem to care about that mismatch, clang didn't like that '%ld' was used in the printk format string to print a size_t variable. Replace the correct '%zu' instead of '%ld' to fix that. Change-Id: I32bc584abe835c9c1d732c12311881345b8f0cdf Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85251 Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/psp/rpmc.c 1 file changed, 3 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Marshall Dawson: Looks good to me, approved diff --git a/src/soc/amd/common/block/psp/rpmc.c b/src/soc/amd/common/block/psp/rpmc.c index dd097c5..99e166a 100644 --- a/src/soc/amd/common/block/psp/rpmc.c +++ b/src/soc/amd/common/block/psp/rpmc.c @@ -78,12 +78,12 @@ static void print_spi_rpmc_usage(uint8_t available, uint8_t used) { for (size_t i = 0; i < SPI_RPMC_COUNTER_COUNT; i++) { - printk(BIOS_SPEW, "SPI flash RPMC counter %ld %s provisioned\n", i, + printk(BIOS_SPEW, "SPI flash RPMC counter %zu %s provisioned\n", i, available & BIT(i) ? "can still be" : "has already been"); } for (size_t i = 0; i < SPI_RPMC_COUNTER_COUNT; i++) { - printk(BIOS_SPEW, "SPI flash RPMC counter %ld is%s in use\n", i, + printk(BIOS_SPEW, "SPI flash RPMC counter %zu is%s in use\n", i, used & BIT(i) ? "" : " not"); } } @@ -97,7 +97,7 @@ print_spi_rpmc_usage(psp_caps.r0.spi_rpmc_slots_available, psp_caps.r0.spi_rpmc_slot_used); for (size_t i = 0; i < PSP_RPMC_R0_SLOT_COUNT; i++) { - printk(BIOS_SPEW, "SoC RPMC slot %ld %s provisioned\n", i, + printk(BIOS_SPEW, "SoC RPMC slot %zu %s provisioned\n", i, psp_caps.r0.psp_rpmc_slot_available & BIT(i) ? "can still be" : "has already been"); } -- To view, visit https://review.coreboot.org/c/coreboot/+/85251?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I32bc584abe835c9c1d732c12311881345b8f0cdf Gerrit-Change-Number: 85251 Gerrit-PatchSet: 6 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Marshall Dawson <marshalldawson3rd(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>

1 0

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

03 Dec '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85238?usp=email ) ( 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )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> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85238 Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/psp/psp_smi.c 1 file changed, 3 insertions(+), 0 deletions(-) Approvals: Marshall Dawson: Looks good to me, approved build bot (Jenkins): Verified Matt DeVillier: Looks good to me, approved 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: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8c02e8e29ab5619282e5b864a8cea6f0703f6ef2 Gerrit-Change-Number: 85238 Gerrit-PatchSet: 7 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

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

03 Dec '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84906?usp=email ) ( 11 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/amd/common/psp_smi_flash: implement SPI flash RPMC command handling ...................................................................... soc/amd/common/psp_smi_flash: implement SPI flash RPMC command handling Extend the 'psp_smi_spi_rpmc_inc_mc' and 'psp_smi_spi_rpmc_req_mc' function stubs that now implement the actual functionality by calling 'spi_flash_rpmc_increment' and 'spi_flash_rpmc_request' after doing some sanity checks. TEST=When selecting both 'SOC_AMD_COMMON_BLOCK_PSP_RPMC' and 'SOC_AMD_COMMON_BLOCK_PSP_SMI' Kconfig options on a board with an RPMC- capable SPI flash, the PSP SMI handler can successfully service not only the already working SPI flash write command, but also the increment monotonic counter RPMC command: [NOTE ] coreboot-[...] x86_32 smm starting (log level: 8)... [SPEW ] SMI# #0 [SPEW ] PSP: SPI write request [DEBUG] FMAP: area PSP_NVRAM found @ f20000 (131072 bytes) [SPEW ] PSP: SPI write 0x400 bytes at 0x0 [NOTE ] coreboot-[...] x86_32 smm starting (log level: 8)... [SPEW ] SMI# #0 [SPEW ] PSP: SPI write request [DEBUG] FMAP: area PSP_NVRAM found @ f20000 (131072 bytes) [SPEW ] PSP: SPI write 0x400 bytes at 0x400 [NOTE ] coreboot-[...] x86_32 smm starting (log level: 8)... [SPEW ] SMI# #8 [SPEW ] PSP: SPI write request [DEBUG] FMAP: area PSP_NVRAM found @ f20000 (131072 bytes) [SPEW ] PSP: SPI write 0x310 bytes at 0x800 [NOTE ] coreboot-[...] x86_32 smm starting (log level: 8)... [SPEW ] SMI# #1 [SPEW ] PSP: SPI write request [DEBUG] FMAP: area PSP_RPMC_NVRAM found @ f40000 (262144 bytes) [SPEW ] PSP: SPI write 0x70 bytes at 0x100 [NOTE ] coreboot-[...] x86_32 smm starting (log level: 8)... [SPEW ] SMI# #0 [SPEW ] PSP: SPI RPMC increment monotonic counter request This requires the PSP_RPMC_NVRAM FMAP section to have the correct size which in case of Renoir is 256 kByte. Having this large enough size also makes the PSP report that the PSP RPMC NVRAM is healthy which wasn't the case in previous tests when the region was too small. Change-Id: I20e4f60d4e35d33e560fc43212b320e817e13004 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/84906 Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> --- M src/soc/amd/common/block/psp/Kconfig M src/soc/amd/common/block/psp/psp_smi_flash.c 2 files changed, 41 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Marshall Dawson: Looks good to me, approved Matt DeVillier: Looks good to me, but someone else must approve diff --git a/src/soc/amd/common/block/psp/Kconfig b/src/soc/amd/common/block/psp/Kconfig index 3ae5e43..07ee0fa 100644 --- a/src/soc/amd/common/block/psp/Kconfig +++ b/src/soc/amd/common/block/psp/Kconfig @@ -34,6 +34,7 @@ config SOC_AMD_COMMON_BLOCK_PSP_RPMC bool depends on SOC_AMD_COMMON_BLOCK_PSP_GEN2 + select SPI_FLASH_RPMC help Select this option in the SoC's Kconfig to include the support for the replay-protected monotonic counter (RPMC) feature. diff --git a/src/soc/amd/common/block/psp/psp_smi_flash.c b/src/soc/amd/common/block/psp/psp_smi_flash.c index abddcdf..a32ba21 100644 --- a/src/soc/amd/common/block/psp/psp_smi_flash.c +++ b/src/soc/amd/common/block/psp/psp_smi_flash.c @@ -281,14 +281,52 @@ enum mbox_p2c_status psp_smi_spi_rpmc_inc_mc(struct mbox_default_buffer *buffer) { + struct mbox_psp_cmd_spi_rpmc_inc_mc *const cmd_buf = + (struct mbox_psp_cmd_spi_rpmc_inc_mc *)buffer; + const struct spi_flash *flash; + printk(BIOS_SPEW, "PSP: SPI RPMC increment monotonic counter request\n"); - return MBOX_PSP_UNSUPPORTED; + if (!CONFIG(SOC_AMD_COMMON_BLOCK_PSP_RPMC)) + return MBOX_PSP_UNSUPPORTED; + + if (spi_controller_busy()) { + return MBOX_PSP_SPI_BUSY; + } + + if (get_flash_device(&flash) != MBOX_PSP_SUCCESS) + return MBOX_PSP_COMMAND_PROCESS_ERROR; + + if (spi_flash_rpmc_increment(flash, cmd_buf->req.counter_address, + cmd_buf->req.counter_data, cmd_buf->req.signature) + != CB_SUCCESS) + return MBOX_PSP_COMMAND_PROCESS_ERROR; + + return MBOX_PSP_SUCCESS; } enum mbox_p2c_status psp_smi_spi_rpmc_req_mc(struct mbox_default_buffer *buffer) { + struct mbox_psp_cmd_spi_rpmc_req_mc *const cmd_buf = + (struct mbox_psp_cmd_spi_rpmc_req_mc *)buffer; + const struct spi_flash *flash; + printk(BIOS_SPEW, "PSP: SPI RPMC request monotonic counter request\n"); - return MBOX_PSP_UNSUPPORTED; + if (!CONFIG(SOC_AMD_COMMON_BLOCK_PSP_RPMC)) + return MBOX_PSP_UNSUPPORTED; + + if (spi_controller_busy()) { + return MBOX_PSP_SPI_BUSY; + } + + if (get_flash_device(&flash) != MBOX_PSP_SUCCESS) + return MBOX_PSP_COMMAND_PROCESS_ERROR; + + if (spi_flash_rpmc_request(flash, cmd_buf->req.counter_address, cmd_buf->req.tag, + cmd_buf->req.signature, cmd_buf->req.output_counter_data, + cmd_buf->req.output_signature) != CB_SUCCESS) + return MBOX_PSP_COMMAND_PROCESS_ERROR; + + return MBOX_PSP_SUCCESS; } -- 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: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I20e4f60d4e35d33e560fc43212b320e817e13004 Gerrit-Change-Number: 84906 Gerrit-PatchSet: 13 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

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) 03 Dec '24

03 Dec '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85236?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )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 Reviewed-on: https://review.coreboot.org/c/coreboot/+/85236 Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/psp/psp_smi_flash.h 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Marshall Dawson: Looks good to me, approved Matt DeVillier: Looks good to me, approved 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: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6c290535a1896c080b74d892cb289e6e122d4525 Gerrit-Change-Number: 85236 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: 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: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

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

03 Dec '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84837?usp=email ) Change subject: drivers/spi: add RPMC support ...................................................................... drivers/spi: add RPMC support Add functions to send the RPMC-related commands to a RPMC-capable SPI flash to provision the root key, update the HMAC key, increment the monotonic counter, and request the monotonic counter data. To talk to the flash chip, the command bytes and polling mechanism described in the SFDP data structure of the flash are used. The JESD260 specification was used as a reference. Some of inspiration was taken from github.com/teslamotors/coreboot/tree/tesla-4.12-amd TEST=When used with the later patch that calls some of the functions added in this patch, sending the RPMC increment monotonic counter command to the RPMC-capable SPI flash, in this case W74M12JW, is successful. Change-Id: Ia9bd69d0105c66bf5ecb6c90e8c782a81912bd40 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/84837 Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> Reviewed-by: Marshall Dawson <marshalldawson3rd(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/drivers/spi/spi_flash_rpmc.c M src/include/spi_flash.h 2 files changed, 357 insertions(+), 0 deletions(-) Approvals: Matt DeVillier: Looks good to me, approved Marshall Dawson: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/drivers/spi/spi_flash_rpmc.c b/src/drivers/spi/spi_flash_rpmc.c index 148e3bb..27342ab 100644 --- a/src/drivers/spi/spi_flash_rpmc.c +++ b/src/drivers/spi/spi_flash_rpmc.c @@ -30,3 +30,308 @@ flash->rpmc_caps.op2_read_cmd = rpmc_info.op2_read_command; flash->rpmc_caps.rpmc_available = true; } + +static bool spi_flash_rpmc_is_available(const struct spi_flash *flash) +{ + return flash->rpmc_caps.rpmc_available; +} + +#define RPMC_OP2_READ_BUF_EXT_STATUS_OFFSET 0 +#define RPMC_OP2_READ_BUF_TAG_OFFSET 1 +#define RPMC_OP2_READ_BUF_COUNTER_DATA_OFFSET (1 + SPI_RPMC_TAG_LEN) +#define RPMC_OP2_READ_BUF_SIGNATURE_OFFSET (1 + SPI_RPMC_TAG_LEN + SPI_RPMC_COUNTER_DATA_LEN) + +/* + * 'extended_status', 'tag', 'counter_data', and 'signature' may be NULL if the caller + * doesn't need the value. 'tag' must be a SPI_RPMC_TAG_LEN bytes long buffer, 'signature' + * must be a SPI_RPMC_SIG_LEN bytes long buffer. When only 'extended_status' is non-NULL, only + * the extended status is read back. + */ +static enum cb_err spi_flash_rpmc_op2_read(const struct spi_flash *flash, + uint8_t *extended_status, uint8_t *tag, + uint8_t *counter_data, uint8_t *signature) +{ + int cmd_ret; + const bool only_check_extended_status = extended_status != NULL && tag == NULL && + counter_data == NULL && signature == NULL; + + /* command and dummy byte */ + uint8_t write_buf[1 + 1]; + /* extended status, tag, counter data, signature */ + uint8_t read_buf[1 + SPI_RPMC_TAG_LEN + SPI_RPMC_COUNTER_DATA_LEN + SPI_RPMC_SIG_LEN]; + + write_buf[0] = flash->rpmc_caps.op2_read_cmd; + write_buf[1] = 0x00; /* dummy byte */ + + cmd_ret = spi_flash_cmd_multi(&flash->spi, write_buf, sizeof(write_buf), read_buf, + only_check_extended_status ? 1 : sizeof(read_buf)); + + if (cmd_ret) + return CB_ERR; + + if (extended_status != NULL) + *extended_status = read_buf[RPMC_OP2_READ_BUF_EXT_STATUS_OFFSET]; + + if (tag != NULL) + memcpy(tag, &read_buf[RPMC_OP2_READ_BUF_TAG_OFFSET], SPI_RPMC_TAG_LEN); + + if (counter_data != NULL) + memcpy(counter_data, &read_buf[RPMC_OP2_READ_BUF_COUNTER_DATA_OFFSET], + SPI_RPMC_COUNTER_DATA_LEN); + + if (signature != NULL) + memcpy(signature, &read_buf[RPMC_OP2_READ_BUF_SIGNATURE_OFFSET], + SPI_RPMC_SIG_LEN); + + return CB_SUCCESS; +} + +static enum cb_err spi_flash_rpmc_op2_get_exteded_status(const struct spi_flash *flash, + uint8_t *extended_status) +{ + return spi_flash_rpmc_op2_read(flash, extended_status, NULL, NULL, NULL); +} + +#define RPMC_OP2_EXT_STATUS_SUCCESS BIT(7) +#define RPMC_OP2_EXT_STATUS_ERR_FATAL BIT(5) +#define RPMC_OP2_EXT_STATUS_ERR_BAD_COUNTER BIT(4) +#define RPMC_OP2_EXT_STATUS_ERR_UNINITIALIZED BIT(3) +#define RPMC_OP2_EXT_STATUS_ERR_INVALID BIT(2) +#define RPMC_OP2_EXT_STATUS_ERR_OTHER BIT(1) +#define RPMC_OP2_EXT_STATUS_POLL_BUSY BIT(0) + +static bool spi_flash_rpmc_is_extended_status_successful(uint8_t extended_status) +{ + return extended_status & RPMC_OP2_EXT_STATUS_SUCCESS; +} + +static void spi_flash_rpmc_print_extended_status_error(uint8_t extended_status) +{ + if (extended_status & RPMC_OP2_EXT_STATUS_POLL_BUSY) + printk(BIOS_WARNING, "SPI flash RPMC is busy\n"); + + if (extended_status & RPMC_OP2_EXT_STATUS_ERR_OTHER) + printk(BIOS_ERR, "SPI flash RPMC other error\n"); + + if (extended_status & RPMC_OP2_EXT_STATUS_ERR_INVALID) + printk(BIOS_ERR, "SPI flash RPMC invalid input\n"); + + if (extended_status & RPMC_OP2_EXT_STATUS_ERR_UNINITIALIZED) + printk(BIOS_ERR, "SPI flash RPMC uninitialized\n"); + + if (extended_status & RPMC_OP2_EXT_STATUS_ERR_BAD_COUNTER) + printk(BIOS_ERR, "SPI flash RPMC counter mismatch\n"); + + if (extended_status & RPMC_OP2_EXT_STATUS_ERR_FATAL) + printk(BIOS_ERR, "SPI flash RPMC fatal error\n"); +} + +static enum cb_err spi_flash_rpmc_check_extended_status(const struct spi_flash *flash) +{ + uint8_t extended_status; + + if (spi_flash_rpmc_op2_get_exteded_status(flash, &extended_status) != CB_SUCCESS) + return CB_ERR; + + if (spi_flash_rpmc_is_extended_status_successful(extended_status)) + return CB_SUCCESS; + + spi_flash_rpmc_print_extended_status_error(extended_status); + + return CB_ERR; +} + +static enum cb_err spi_flash_rpmc_poll_op2(const struct spi_flash *flash, uint64_t timeout_ms) +{ + unsigned int attempt = 0; + uint8_t extended_status; + struct stopwatch sw; + + stopwatch_init_msecs_expire(&sw, timeout_ms); + do { + attempt++; + + if (spi_flash_rpmc_op2_get_exteded_status(flash, &extended_status)) { + printk(BIOS_WARNING, + "SPI command failed on attempt %d\n", attempt); + return CB_ERR; + } + + if ((extended_status & RPMC_OP2_EXT_STATUS_POLL_BUSY) == 0) + return CB_SUCCESS; + } while (!stopwatch_expired(&sw)); + + printk(BIOS_WARNING, "SPI RPMC poll timeout at %lld msec after %d attempts\n", + stopwatch_duration_msecs(&sw), attempt); + + return CB_ERR; +} + +static enum cb_err spi_flash_rpmc_wait_ready(const struct spi_flash *flash, + uint64_t timeout_ms) +{ + if (!flash->rpmc_caps.poll_op2_ext_stat) { + /* poll read status */ + if (spi_flash_cmd_wait_ready(flash, timeout_ms)) + return CB_ERR; + } else { + /* poll RPMC OP2 extended status */ + if (spi_flash_rpmc_poll_op2(flash, timeout_ms) != CB_SUCCESS) + return CB_ERR; + } + + return CB_SUCCESS; +} + +static bool spi_flash_rpmc_is_valid_counter_addr(const struct spi_flash *flash, + uint8_t counter_addr) +{ + return counter_addr < flash->rpmc_caps.number_of_counters; +} + +/* TODO: replace with proper value read from SFDP?! */ +#define SPI_FLASH_RPMC_TIMEOUT_MS 200 + +#define RPMC_OP1_CMD_WRITE_ROOT_KEY_REGISTER 0x00 +#define RPMC_OP1_CMD_UPDATE_HMAC_KEY_REGISTER 0x01 +#define RPMC_OP1_CMD_INCREMENT_MONOTONIC_COUNTER 0x02 +#define RPMC_OP1_CMD_REQUEST_MONOTONIC_COUNTER 0x03 + +enum cb_err spi_flash_rpmc_write_root_key(const struct spi_flash *flash, uint8_t counter_addr, + const uint8_t *root_key, + const uint8_t *truncated_signature) +{ + /* command, command type, counter address, reserved, root key, truncated signature */ + uint8_t buf[1 + 1 + 1 + 1 + SPI_RPMC_ROOT_KEY_LEN + SPI_RPMC_TRUNCTAED_SIG_LEN]; + + if (!spi_flash_rpmc_is_available(flash)) + return CB_ERR; + + if (!spi_flash_rpmc_is_valid_counter_addr(flash, counter_addr)) + return CB_ERR; + + buf[0] = flash->rpmc_caps.op1_write_cmd; + buf[1] = RPMC_OP1_CMD_WRITE_ROOT_KEY_REGISTER; + buf[2] = counter_addr; + buf[3] = 0x00; /* reserved */ + memcpy(&buf[4], root_key, SPI_RPMC_ROOT_KEY_LEN); + memcpy(&buf[4 + SPI_RPMC_ROOT_KEY_LEN], truncated_signature, + SPI_RPMC_TRUNCTAED_SIG_LEN); + + if (spi_flash_cmd_multi(&flash->spi, buf, sizeof(buf), NULL, 0)) + return CB_ERR; + + if (spi_flash_rpmc_wait_ready(flash, SPI_FLASH_RPMC_TIMEOUT_MS) != CB_SUCCESS) + return CB_ERR; + + if (spi_flash_rpmc_check_extended_status(flash) != CB_SUCCESS) + return CB_ERR; + + return CB_SUCCESS; +} + +enum cb_err spi_flash_rpmc_update_hmac_key(const struct spi_flash *flash, uint8_t counter_addr, + uint8_t *key_data, const uint8_t *signature) +{ + /* command, command type, counter address, reserved, key data, signature */ + uint8_t buf[1 + 1 + 1 + 1 + SPI_RPMC_KEY_DATA_LEN + SPI_RPMC_SIG_LEN]; + + if (!spi_flash_rpmc_is_available(flash)) + return CB_ERR; + + if (!spi_flash_rpmc_is_valid_counter_addr(flash, counter_addr)) + return CB_ERR; + + buf[0] = flash->rpmc_caps.op1_write_cmd; + buf[1] = RPMC_OP1_CMD_UPDATE_HMAC_KEY_REGISTER; + buf[2] = counter_addr; + buf[3] = 0x00; /* reserved */ + memcpy(&buf[4], key_data, SPI_RPMC_KEY_DATA_LEN); + memcpy(&buf[4 + SPI_RPMC_KEY_DATA_LEN], signature, SPI_RPMC_SIG_LEN); + + if (spi_flash_cmd_multi(&flash->spi, buf, sizeof(buf), NULL, 0)) + return CB_ERR; + + if (spi_flash_rpmc_wait_ready(flash, SPI_FLASH_RPMC_TIMEOUT_MS) != CB_SUCCESS) + return CB_ERR; + + if (spi_flash_rpmc_check_extended_status(flash) != CB_SUCCESS) + return CB_ERR; + + return CB_SUCCESS; +} + +enum cb_err spi_flash_rpmc_increment(const struct spi_flash *flash, uint8_t counter_addr, + const uint8_t *counter_data, const uint8_t *signature) +{ + /* command, command type, counter address, reserved, counter data, signature */ + uint8_t buf[1 + 1 + 1 + 1 + SPI_RPMC_COUNTER_DATA_LEN + SPI_RPMC_SIG_LEN]; + + if (!spi_flash_rpmc_is_available(flash)) + return CB_ERR; + + if (!spi_flash_rpmc_is_valid_counter_addr(flash, counter_addr)) + return CB_ERR; + + buf[0] = flash->rpmc_caps.op1_write_cmd; + buf[1] = RPMC_OP1_CMD_INCREMENT_MONOTONIC_COUNTER; + buf[2] = counter_addr; + buf[3] = 0x00; /* reserved */ + memcpy(&buf[4], counter_data, SPI_RPMC_COUNTER_DATA_LEN); + memcpy(&buf[4 + SPI_RPMC_COUNTER_DATA_LEN], signature, SPI_RPMC_SIG_LEN); + + if (spi_flash_cmd_multi(&flash->spi, buf, sizeof(buf), NULL, 0)) + return CB_ERR; + + if (spi_flash_rpmc_wait_ready(flash, SPI_FLASH_RPMC_TIMEOUT_MS) != CB_SUCCESS) + return CB_ERR; + + if (spi_flash_rpmc_check_extended_status(flash) != CB_SUCCESS) + return CB_ERR; + + return CB_SUCCESS; +} + +enum cb_err spi_flash_rpmc_request(const struct spi_flash *flash, uint8_t counter_addr, + const uint8_t *tag, const uint8_t *signature, + uint8_t *counter_data, uint8_t *signature_out) +{ + /* command, command type, counter address, reserved, tag, signature */ + uint8_t buf[1 + 1 + 1 + 1 + SPI_RPMC_TAG_LEN + SPI_RPMC_SIG_LEN]; + uint8_t extended_status; + + if (!spi_flash_rpmc_is_available(flash)) + return CB_ERR; + + if (!spi_flash_rpmc_is_valid_counter_addr(flash, counter_addr)) + return CB_ERR; + + buf[0] = flash->rpmc_caps.op1_write_cmd; + buf[1] = RPMC_OP1_CMD_REQUEST_MONOTONIC_COUNTER; + buf[2] = counter_addr; + buf[3] = 0x00; /* reserved */ + memcpy(&buf[4], tag, SPI_RPMC_TAG_LEN); + memcpy(&buf[4 + SPI_RPMC_TAG_LEN], signature, SPI_RPMC_SIG_LEN); + + if (spi_flash_cmd_multi(&flash->spi, buf, sizeof(buf), NULL, 0)) + return CB_ERR; + + if (spi_flash_rpmc_wait_ready(flash, SPI_FLASH_RPMC_TIMEOUT_MS) != CB_SUCCESS) + return CB_ERR; + + /* + * Since we need to read back more than just the extended status for this command and + * want to avoid unnecessary SPI transactions, spi_flash_rpmc_check_extended_status + * isn't used here. + */ + if (spi_flash_rpmc_op2_read(flash, &extended_status, NULL, counter_data, signature_out) + != CB_SUCCESS) + return CB_ERR; + + if (!spi_flash_rpmc_is_extended_status_successful(extended_status)) { + spi_flash_rpmc_print_extended_status_error(extended_status); + return CB_ERR; + } + + return CB_SUCCESS; +} diff --git a/src/include/spi_flash.h b/src/include/spi_flash.h index 76c33be..9ba5d1f 100644 --- a/src/include/spi_flash.h +++ b/src/include/spi_flash.h @@ -15,6 +15,10 @@ /* SPI RPMC field lengths in bytes */ #define SPI_RPMC_TAG_LEN 12 #define SPI_RPMC_SIG_LEN 32 +#define SPI_RPMC_ROOT_KEY_LEN 32 +#define SPI_RPMC_TRUNCTAED_SIG_LEN 28 /* only used in write RPMC root key command */ +#define SPI_RPMC_KEY_DATA_LEN 4 +#define SPI_RPMC_COUNTER_DATA_LEN 4 struct spi_flash; @@ -261,4 +265,52 @@ /* Print the SFDP headers read from the SPI flash */ void spi_flash_print_sfdp_headers(const struct spi_flash *flash); +/* + * Write RPMC root key + * + * root_key: pointer to 32 bytes long buffer with root key + * truncated_signature: pointer to 28 bytes long buffer with truncated signature + * + * The buffers have the bytes in the order in which they will be sent to the SPI flash + */ +enum cb_err spi_flash_rpmc_write_root_key(const struct spi_flash *flash, uint8_t counter_addr, + const uint8_t *root_key, + const uint8_t *truncated_signature); + +/* + * Update HMAC key + * + * key_data: pointer to 4 bytes long buffer with key data + * signature: pointer to 32 bytes long buffer with signature + * + * The buffers have the bytes in the order in which they will be sent to the SPI flash + */ +enum cb_err spi_flash_rpmc_update_hmac_key(const struct spi_flash *flash, uint8_t counter_addr, + uint8_t *key_data, const uint8_t *signature); + +/* + * Increment monotonic counter + * + * counter_data: pointer to 4 bytes long buffer with counter data + * signature: pointer to 32 bytes long buffer with signature + * + * The buffers have the bytes in the order in which they will be sent to the SPI flash + */ +enum cb_err spi_flash_rpmc_increment(const struct spi_flash *flash, uint8_t counter_addr, + const uint8_t *counter_data, const uint8_t *signature); + +/* + * Request monotonic counter + * + * tag: pointer to 12 bytes long buffer with tag + * signature: pointer to 32 bytes long buffer with signature + * counter_data: pointer to 4 bytes long buffer with counter data + * signature_out: pointer to 32 bytes long buffer with signature + * + * The buffers have the bytes in the order in which they will be sent to the SPI flash + */ +enum cb_err spi_flash_rpmc_request(const struct spi_flash *flash, uint8_t counter_addr, + const uint8_t *tag, const uint8_t *signature, + uint8_t *counter_data, uint8_t *signature_out); + #endif /* _SPI_FLASH_H_ */ -- 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: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia9bd69d0105c66bf5ecb6c90e8c782a81912bd40 Gerrit-Change-Number: 84837 Gerrit-PatchSet: 11 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2024