Change in coreboot[master]: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes have been done in the send_hmrfpo_enable_msg(): 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode. 2. Add check for response status. 3. Add description for the send_hmrfpo_enable_msg()
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perfom updates to it.This command is only valid before EOP.
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE to boot from BP1. When CSE boots from BP1, it will have opmode: Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allow CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 24 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/1
diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index 46ad8ce..b60d888 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -659,15 +659,13 @@
printk(BIOS_DEBUG, "HECI: Send HMRFPO Enable Command\n"); hfs1.data = me_read_config32(PCI_ME_HFSTS1); + /* * This command can be run only if: - * - Working state is normal and - * - Operation mode is normal or temporary disable mode. + * - Operation mode is temporary disable mode. */ - if (hfs1.fields.working_state != ME_HFS_CWS_NORMAL || - (hfs1.fields.operation_mode != ME_HFS_MODE_NORMAL && - hfs1.fields.operation_mode != ME_HFS_MODE_TEMP_DISABLE)) { - printk(BIOS_ERR, "HECI: ME not in required Mode\n"); + if (hfs1.fields.operation_mode != ME_HFS_MODE_TEMP_DISABLE) { + printk(BIOS_ERR, "HECI: ME is not in expected mode/state(0x%x)\n", hfs1.data); goto failed; }
@@ -679,6 +677,12 @@ printk(BIOS_ERR, "HECI: Resp Failed:%d\n", resp.hdr.result); goto failed; } + + if (resp.status) { + printk(BIOS_ERR, "HECI: Enable Failed, resp status:%d\n", resp.status); + goto failed; + } + return 1;
failed: diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h index 3c00b87..53f5493 100644 --- a/src/soc/intel/common/block/include/intelblocks/cse.h +++ b/src/soc/intel/common/block/include/intelblocks/cse.h @@ -128,8 +128,20 @@ int send_heci_reset_req_message(uint8_t rst_type);
/* - * Send HMRFPO_ENABLE command. - * returns 0 on failure and 1 on success. + * Sends HMRFPO_ENABLE command. + * HMRFPO - Host ME Region Flash Protection Override. + * When MRP feature is enabled, procedure to place CSE in HMRFPO (SECOVER_MEI_MSG) mode: + * 1. Ensure CSE boots from BP1(RO). + * - Send set_next_boot_partition(BP1) + * - Issue CSE Only Reset + * 2. Send HMRFPO_ENABLE command to CSE. Further, no reset is required. + * + * The HMRFPO mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks + * the CSE region to perfom updates to it. + * This command is only valid before EOP. + * + * Returns 0 on failure to send heci command and to enable hmrfpo mode, and 1 on success. + * */ int send_hmrfpo_enable_msg(void);
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Paul Menzel, Rizwan Qureshi, V Sowmya, build bot (Jenkins), Nico Huber, Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#2).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes have been done in the send_hmrfpo_enable_msg(): 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode. 2. Add check for response status. 3. Add description for the send_hmrfpo_enable_msg()
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perfom updates to it.This command is only valid before EOP.
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE to boot from BP1. When CSE boots from BP1, it will have opmode: Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allow CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 24 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/2
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Paul Menzel, Rizwan Qureshi, V Sowmya, build bot (Jenkins), Nico Huber, Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#3).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes have been done in the send_hmrfpo_enable_msg(): 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode. 2. Add check for response status. 3. Add description for the send_hmrfpo_enable_msg()
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perfom updates to it.This command is only valid before EOP.
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE to boot from BP1. When CSE boots from BP1, it will have opmode: Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allow CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 24 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/3
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Paul Menzel, Rizwan Qureshi, V Sowmya, build bot (Jenkins), Nico Huber, Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#4).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes have been done in send_hmrfpo_enable_msg(): 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode. 2. Add check for response status. 3. Add description for the send_hmrfpo_enable_msg()
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP.
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE to boot from BP1. When CSE boots from BP1, it will have opmode- Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allow CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 24 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/4
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 4:
(10 comments)
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@9 PS4, Line 9: Below changes have been done in send_hmrfpo_enable_msg(): Maybe:
Change send_hmrfpo_enable_msg() as below:
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@11 PS4, Line 11: operation mode is Temp Disable Mode. Please remove the dot/period at the end, or at one to the third item.
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@12 PS4, Line 12: 2. Add check for response status. Check response status
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@13 PS4, Line 13: 3. Add description for the send_hmrfpo_enable_msg() Describe/document send_hmrfpo_enable_msg()
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@17 PS4, Line 17: EOP What is that?
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@20 PS4, Line 20: 1. Ensure CSE to boot from BP1. When CSE boots from BP1, it will have
Ensure CSE boots from BP1. …
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@21 PS4, Line 21: opmode- Remove -.
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@22 PS4, Line 22: 2. Send HMRFPO_ENABLE command to CSE. Remove line break.
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@26 PS4, Line 26: allow allows
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@39 PS4, Line 39: TEST=Verfied on hatch board. How did you verify this?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37283/4/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/4/src/soc/intel/common/block/... PS4, Line 682: printk(BIOS_ERR, "HECI: Enable Failed, resp status:%d\n", resp.status); Please add a space after the colon. Please add a more elaborate the error message, so a user knows about the effects.
HECI: Enable failed (response status: %d), you won’t be able to …\n
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Christian Walter, Paul Menzel, Rizwan Qureshi, V Sowmya, build bot (Jenkins), Nico Huber, Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#5).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Modify send_hmrfpo_enable_msg() as below: 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode 2. Check response status 3. Describe send_hmrfpo_enable_msg()
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allows CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 24 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/5
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 4:
(11 comments)
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@9 PS4, Line 9: Below changes have been done in send_hmrfpo_enable_msg():
Maybe: […]
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@11 PS4, Line 11: operation mode is Temp Disable Mode.
Please remove the dot/period at the end, or at one to the third item.
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@12 PS4, Line 12: 2. Add check for response status.
Check response status
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@13 PS4, Line 13: 3. Add description for the send_hmrfpo_enable_msg()
Describe/document send_hmrfpo_enable_msg()
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@17 PS4, Line 17: EOP
What is that?
CSE does not honour HMRFPO_ENABLE command after it receives EOP message from CSE. Typically, Coreboot sends EOP(End of Post) message to CSE to indicate end of POST & start of OS load.
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@20 PS4, Line 20: 1. Ensure CSE to boot from BP1. When CSE boots from BP1, it will have
Ensure CSE boots from BP1. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@21 PS4, Line 21: opmode-
Remove -.
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@22 PS4, Line 22: 2. Send HMRFPO_ENABLE command to CSE.
Remove line break.
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@26 PS4, Line 26: allow
allows
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@39 PS4, Line 39: TEST=Verfied on hatch board.
How did you verify this?
This is verified through CSE FW upgrade procedure. Please refer the patch:https://review.coreboot.org/c/coreboot/+/35403
https://review.coreboot.org/c/coreboot/+/37283/4/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/4/src/soc/intel/common/block/... PS4, Line 682: printk(BIOS_ERR, "HECI: Enable Failed, resp status:%d\n", resp.status);
Please add a space after the colon. […]
Done
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@17 PS4, Line 17: EOP
CSE does not honour HMRFPO_ENABLE command after it receives EOP message from CSE. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@39 PS4, Line 39: TEST=Verfied on hatch board.
This is verified through CSE FW upgrade procedure. Please refer the patch:https://review.coreboot. […]
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG@11 PS5, Line 11: operation mode is Temp Disable Mode Why? Is this a new requirement for a specific ME firmware version? Or is it just part of your RO/RW update mechanism? If the latter, please keep the CSE API as is and handle the check in a higher level.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG@21 PS5, Line 21: opmode Temp Disable Mode. I still have not seen any documentation from Intel about this. It doesn't seem to reflect how it usually works. Please don't mix information about specific firmware versions with the generic `cse.c`, unless all future firmware versions work like this. Then, maybe we should give this some versioning? new/old CSE?
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG@21 PS5, Line 21: opmode Temp Disable Mode.
I still have not seen any documentation from Intel about this. It […]
The patch is to update the prerequisites for sending HMRFPO command. In the current FWs, the prerequisite is that the CSME working mode and and operation mode should be 'normal'. However when the MRP mode is enabled HMRFPO enable will be allowed when CSME operation mode is "soft temporary disable". The MRP is a new option supported in CML, documentation update is WIP. Meanwhile we have explained it in the commit message.
I agree on versioning part and my suggestion is to link it with the MRP rather than the CSME version. 1. Check CSME working state and operation mode is "Normal" 2. If yes, then send HMRFPO command 3. If no, then check if MRP is enabled and operation mode is "soft temp disabled" 3.1 if yes send HMRFPO command. If not, don't send the command.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG@21 PS5, Line 21: opmode Temp Disable Mode.
The patch is to update the prerequisites for sending HMRFPO command. […]
Thanks for the explanation. SGTM
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#7).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Modify send_hmrfpo_enable_msg() as below: 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode 2. Check response status 3. Describe send_hmrfpo_enable_msg()
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allows CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 24 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/7
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 7:
(6 comments)
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@10 PS7, Line 10: Allow execution of HMRFPO_ENABLE command only when CSE's : operation mode is Temp Disable Mode Why? This does not match my reading of BWG. Please point me to the doc # that captures this condition.
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@19 PS7, Line 19: MRP MRP is ... ?
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@20 PS7, Line 20: Ensure CSE boots from BP1. When CSE boots from BP1, it will have : opmode Temp Disable Mode. Where is this captured?
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 667: if (hfs1.fields.operation_mode != ME_HFS_MODE_TEMP_DISABLE) { : printk(BIOS_ERR, "HECI: ME is not in expected mode/state(0x%x)\n", hfs1.data); : goto failed; : } This does not match my reading of CML BWG. Please point me to the doc where this is captured.
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 133: MRP What is MRP?
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 134: 1. Ensure CSE boots from BP1(RO). : * - Send set_next_boot_partition(BP1) : * - Issue CSE Only Reset : * 2. Send HMRFPO_ENABLE command to CSE. Further, no reset is required. Please point me to the doc # that captures this.
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#8).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Modify send_hmrfpo_enable_msg() as below: 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode 2. Check response status 3. Describe send_hmrfpo_enable_msg()
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allows CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 24 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/8
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#9).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Modify cse_hmrfpo_enable() as below: 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode 2. Check response status 3. Describe send_hmrfpo_enable_msg() 4. Rename padding field of hmrfpo_enable_resp to reserved.
In addition to above changes, in cse_hmrfpo_status(), rename padding field of hmrfpo_get_status_resp struct to reserved to match with ME BWG Guide.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allows CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 26 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/9
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#10).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Modify cse_hmrfpo_enable() as below: 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode 2. Check response status 3. Describe send_hmrfpo_enable_msg() 4. Rename padding field of hmrfpo_enable_resp to reserved.
In addition to above changes, in cse_hmrfpo_status(), rename padding field of hmrfpo_get_status_resp struct to reserved to match with ME BWG Guide.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allows CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 26 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/10
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#13).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Modify cse_hmrfpo_enable() as below: 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode 2. Check response status 3. Describe send_hmrfpo_enable_msg() 4. Rename padding field of hmrfpo_enable_resp to reserved.
In addition to above changes, in cse_hmrfpo_status(), rename padding field of hmrfpo_get_status_resp struct to reserved to match with ME BWG Guide.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allows CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 26 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/13
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 15:
This change is ready for review.
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#17).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Modify cse_hmrfpo_enable() as below: 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode 2. Check response status 3. Describe send_hmrfpo_enable_msg() 4. Rename padding field of hmrfpo_enable_resp to reserved.
In addition to above changes, in cse_hmrfpo_status(), rename padding field of hmrfpo_get_status_resp struct to reserved to match with ME BWG Guide.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allows CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 26 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/17
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#18).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Modify cse_hmrfpo_enable() as below: 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode 2. Check response status 3. Describe send_hmrfpo_enable_msg() 4. Rename padding field of hmrfpo_enable_resp to reserved.
In addition to above changes, in cse_hmrfpo_status(), fix typo & rename 'padding' field of hmrfpo_get_status_resp struct to 'reserved' to match with ME BWG Guide.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allows CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 27 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/18
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#20).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command only when CSE's operation mode is Temp Disable Mode 2. Check response status 3. Add documentation for send_hmrfpo_enable_msg() 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
When MRP feature is enabled, procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
The MRP feature enables CSE FW to support redundant boot partitions, and allows CSE to operate with 'Temporary Disable Mode'. The feature is enabled through FIT settings during build phase. Also, CSE can boot from either BP1 or BP2.
CSE Image Layout with MRP enabled: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 is replica of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition.
Existing CSE Image Layout without MRP feature: = BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 28 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/20
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 20:
(4 comments)
https://review.coreboot.org/c/coreboot/+/37283/20//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/20//COMMIT_MSG@36 PS20, Line 36: Existing CSE Image Layout without MRP feature: : = BP2 + BP3 + DATA PART In this case, what is the expectation for the command? Do the lines 20-23 still hold true?
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 669: !cse_is_hfs1_com_soft_temp_disable() This doesn't match BWG for CML or TGL. I see expected Current Operating Mode as Normal. Also, Current Working State is supposed to be normal.
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 687: This command requires a global reset after it is issued(that is what I see in BWG). I think that should be done as part of this command.
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 131: When MRP feature is enabled, Is this command supposed to have different conditions based on whether MRP is enabled or not?
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#22).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
Existing CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 41 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/22
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 22:
(12 comments)
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG@11 PS5, Line 11: operation mode is Temp Disable Mode
Why? Is this a new requirement for a specific ME firmware version? Or […]
cse_check_hmrfpo_enable_prerequisites() function takes care prerequisites of original flow, and latest fw (Custom SKU) as well.
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG@21 PS5, Line 21: opmode Temp Disable Mode.
Thanks for the explanation. […]
Instead of MRP, it checks ME SKU type. If ME SKU type is Custom, then code will check required for current working state Normal and operation mode "soft temporary disable". Otherwise, it checks for current operation mode and operation mode is 'normal'.
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@10 PS7, Line 10: Allow execution of HMRFPO_ENABLE command only when CSE's : operation mode is Temp Disable Mode
Why? This does not match my reading of BWG. […]
Addendum will be available soon.
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@19 PS7, Line 19: MRP
MRP is ... […]
MRP refers Minimal Recovery Partition.
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@20 PS7, Line 20: Ensure CSE boots from BP1. When CSE boots from BP1, it will have : opmode Temp Disable Mode.
Where is this captured?
Planned Addendum will cover it.
https://review.coreboot.org/c/coreboot/+/37283/20//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/20//COMMIT_MSG@36 PS20, Line 36: Existing CSE Image Layout without MRP feature: : = BP2 + BP3 + DATA PART
In this case, what is the expectation for the command? Do the lines 20-23 still hold true?
No, lines 20-23 are not applicable in this case. In this scenario, after issuing HMRFPO_ENABLE command to CSE, a GLOBAL_RESET command must be issued to place CSE in HMRFPO mode.
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 667: if (hfs1.fields.operation_mode != ME_HFS_MODE_TEMP_DISABLE) { : printk(BIOS_ERR, "HECI: ME is not in expected mode/state(0x%x)\n", hfs1.data); : goto failed; : }
This does not match my reading of CML BWG. Please point me to the doc where this is captured.
The change is made considering ME Custom SKU only. I have addressed prerequisite for this command in the latest patch.
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 669: !cse_is_hfs1_com_soft_temp_disable()
This doesn't match BWG for CML or TGL. I see expected Current Operating Mode as Normal. […]
Yes you are right. A separate Addendum will address the prerequisite for this command considering a new case as well( when CSE boots from BP1_RO).
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 687:
This command requires a global reset after it is issued(that is what I see in BWG). […]
Slight change in HMRFPO mode implementation as defined in current BWG. Hence, GLOBAL RESET is not issued in the handler. This will be covered in our planned document( Addendum to BWG).
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 131: When MRP feature is enabled,
Is this command supposed to have different conditions based on whether MRP is enabled or not?
Yes, Command has support different conditions based on the SKU type(MRP which is a default feature in Custom SKU).
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 133: MRP
What is MRP?
MRP refers Minimal Recovery Partition. This is implemented in Custom SKU.With this feature, ME will boot in Soft Disable Mode from BP1(RO), and allows update of BP2(RW) via HMRFPO mode as explained in the commit message.
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 134: 1. Ensure CSE boots from BP1(RO). : * - Send set_next_boot_partition(BP1) : * - Issue CSE Only Reset : * 2. Send HMRFPO_ENABLE command to CSE. Further, no reset is required.
Please point me to the doc # that captures this.
This info is not yet captured in any doc as of now. We are woring on the documentation. It will be available soon.
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 22:
(12 comments)
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG@11 PS5, Line 11: operation mode is Temp Disable Mode
cse_check_hmrfpo_enable_prerequisites() function takes care prerequisites of original flow, and late […]
Done
https://review.coreboot.org/c/coreboot/+/37283/5//COMMIT_MSG@21 PS5, Line 21: opmode Temp Disable Mode.
Instead of MRP, it checks ME SKU type. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@10 PS7, Line 10: Allow execution of HMRFPO_ENABLE command only when CSE's : operation mode is Temp Disable Mode
Addendum will be available soon.
Done
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@19 PS7, Line 19: MRP
MRP refers Minimal Recovery Partition.
Done
https://review.coreboot.org/c/coreboot/+/37283/7//COMMIT_MSG@20 PS7, Line 20: Ensure CSE boots from BP1. When CSE boots from BP1, it will have : opmode Temp Disable Mode.
Planned Addendum will cover it.
Done
https://review.coreboot.org/c/coreboot/+/37283/20//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/20//COMMIT_MSG@36 PS20, Line 36: Existing CSE Image Layout without MRP feature: : = BP2 + BP3 + DATA PART
No, lines 20-23 are not applicable in this case. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 667: if (hfs1.fields.operation_mode != ME_HFS_MODE_TEMP_DISABLE) { : printk(BIOS_ERR, "HECI: ME is not in expected mode/state(0x%x)\n", hfs1.data); : goto failed; : }
The change is made considering ME Custom SKU only. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 669: !cse_is_hfs1_com_soft_temp_disable()
Yes you are right. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 687:
Slight change in HMRFPO mode implementation as defined in current BWG. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37283/20/src/soc/intel/common/block... PS20, Line 131: When MRP feature is enabled,
Yes, Command has support different conditions based on the SKU type(MRP which is a default feature i […]
Done
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 133: MRP
MRP refers Minimal Recovery Partition. This is implemented in Custom SKU. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/7/src/soc/intel/common/block/... PS7, Line 134: 1. Ensure CSE boots from BP1(RO). : * - Send set_next_boot_partition(BP1) : * - Issue CSE Only Reset : * 2. Send HMRFPO_ENABLE command to CSE. Further, no reset is required.
This info is not yet captured in any doc as of now. We are woring on the documentation. […]
Done
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#24).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
Existing CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 41 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/24
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 25:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37283/25/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37283/25/src/soc/intel/common/block... PS25, Line 143: * the CSE region to perfom updates to it. 'perfom' may be misspelled - perhaps 'perform'?
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#26).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
Existing CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 41 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/26
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#27).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
Existing CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 41 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/27
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#28).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + BP3 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + BP3 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
Existing CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 41 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/28
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#29).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Firmware Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 47 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/29
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#30).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Firmware Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 48 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/30
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#32).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Firmware Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 48 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/32
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 32:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37283/32/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/32/src/soc/intel/common/block... PS32, Line 657: cse nit: capitalized `CSE` ?
https://review.coreboot.org/c/coreboot/+/37283/32/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37283/32/src/soc/intel/common/block... PS32, Line 149: heci HECI
https://review.coreboot.org/c/coreboot/+/37283/32/src/soc/intel/common/block... PS32, Line 149: hmrfpo HMRFPO
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Christian Walter, V Sowmya, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#33).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Firmware Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 48 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/33
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 33:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37283/32/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/32/src/soc/intel/common/block... PS32, Line 657: cse
nit: capitalized `CSE` ?
Done
https://review.coreboot.org/c/coreboot/+/37283/32/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37283/32/src/soc/intel/common/block... PS32, Line 149: hmrfpo
HMRFPO
Done
https://review.coreboot.org/c/coreboot/+/37283/32/src/soc/intel/common/block... PS32, Line 149: heci
HECI
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 33: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... PS33, Line 653: static int cse_check_hmrfpo_enable_prerequisites(void) Maybe return a bool? Also, the function name can be worded as a question:
static bool cse_is_hmrfpo_enable_allowed(void)
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... PS33, Line 661: cse_is_hfs1_cws_normal() Since this needs to be true for both cases, how about factoring it out?
if (!cse_is_hfs1_cws_normal()) return false;
if (cse_is_hfs1_com_normal()) return true;
if (cse_is_hfs3_fw_sku_custom() && cse_is_hfs1_com_soft_temp_disable()) return true;
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... PS33, Line 704: /* : * This command can be run only if: : * - ME's current working state is Normal and current operation mode is Normal : * - (or) current working state is Normal and current operating mode is : * Soft Temp Disable while CSE Firmware SKU is Custom. : */ Isn't this comment the same as in cse_check_hmrfpo_enable_prerequisites() ?
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, Angel Pons, Subrata Banik, Aamir Bohra, Patrick Rudolph, V Sowmya, Nico Huber, Christian Walter,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#34).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Firmware Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 44 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/34
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 34:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... PS33, Line 653: static int cse_check_hmrfpo_enable_prerequisites(void)
Maybe return a bool? Also, the function name can be worded as a question: […]
Done
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... PS33, Line 661: cse_is_hfs1_cws_normal()
Since this needs to be true for both cases, how about factoring it out? […]
Done
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... PS33, Line 704: /* : * This command can be run only if: : * - ME's current working state is Normal and current operation mode is Normal : * - (or) current working state is Normal and current operating mode is : * Soft Temp Disable while CSE Firmware SKU is Custom. : */
Isn't this comment the same as in cse_check_hmrfpo_enable_prerequisites() ?
Redundant comment.Removed comment from here.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 34:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... PS33, Line 661: cse_is_hfs1_cws_normal()
Done
Um, not really. The 'cse_is_hfs1_cws_normal()' check will not be taken into account for the custom FW path.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Sridhar Siricilla, Rizwan Qureshi, Paul Menzel, Angel Pons, Subrata Banik, Aamir Bohra, Patrick Rudolph, V Sowmya, Nico Huber, Christian Walter,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37283
to look at the new patch set (#35).
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Firmware Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 47 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/37283/35
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 35:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/33/src/soc/intel/common/block... PS33, Line 661: cse_is_hfs1_cws_normal()
Um, not really. […]
Ack
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 35: Code-Review+2
Thanks!
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command
Below changes are done: 1. Allow execution of HMRFPO_ENABLE command if CSE meets below prerequisites: - Current operation mode(COM) is Normal and Curret working state(CWS) is Normal. -(or) COM is Soft Temp Disable and CWS is Normal if ME's Firmware SKU is Custom. 2. Check response status. 3. Add documentation for send_hmrfpo_enable_msg(). 4. Rename padding field of hmrfpo_enable_resp to reserved.
The HMRFPO (Host ME Region Flash Protection Override) mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks the CSE region to perform updates to it. This command is only valid before EOP(End of Post).
For Custom SKU, follow below procedure to place CSE in HMRFPO mode: 1. Ensure CSE boots from BP1. When CSE boots from BP1, it will have opmode Temp Disable Mode. 2. Send HMRFPO_ENABLE command to CSE. Then, CSE enters HMRFPO mode.
CSE Firmware Custom SKU Image Layout: = [RO] + [RW + DATA PART] = [BP1] + [BP2 + DATA PART]
Here, BP1 will have reduced functionality of BP2, and the BP1 will be CSE's RO partition and [BP2 + DATA PART] together will represent CSE's RW partition. CSE can boot from either BP1(RO) or BP2(RW).
CSE Image Layout in Consumer SKU: BP2 + BP3 + DATA PART
TEST=Verfied on hatch board.
Change-Id: I7c87998fa105947e5ba4638a8e68625e46703448 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37283 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 47 insertions(+), 17 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index c9712dbb..041656a 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -650,6 +650,26 @@ return status; }
+static bool cse_is_hmrfpo_enable_allowed(void) +{ + /* + * Allow sending HMRFPO ENABLE command only if: + * - CSE's current working state is Normal and current operation mode is Normal + * - (or) cse's current working state is normal and current operation mode is + * Soft Temp Disable if CSE's Firmware SKU is Custom + */ + if (!cse_is_hfs1_cws_normal()) + return false; + + if (cse_is_hfs1_com_normal()) + return true; + + if (cse_is_hfs3_fw_sku_custom() && cse_is_hfs1_com_soft_temp_disable()) + return true; + + return false; +} + /* Sends HMRFPO Enable command to CSE */ int cse_hmrfpo_enable(void) { @@ -675,36 +695,34 @@ /* Length of factory data area, not relevant for client SKUs */ uint32_t fct_limit; uint8_t status; - uint8_t padding[3]; + uint8_t reserved[3]; } __packed;
struct hmrfpo_enable_resp resp; size_t resp_size = sizeof(struct hmrfpo_enable_resp);
printk(BIOS_DEBUG, "HECI: Send HMRFPO Enable Command\n"); - /* - * This command can be run only if: - * - Working state is normal and - * - Operation mode is normal or temporary disable mode. - */ - if (!cse_is_hfs1_cws_normal() || - (!cse_is_hfs1_com_normal() && !cse_is_hfs1_com_soft_temp_disable())) { - printk(BIOS_ERR, "HECI: ME not in required Mode\n"); - goto failed; + + if (!cse_is_hmrfpo_enable_allowed()) { + printk(BIOS_ERR, "HECI: CSE does not meet required prerequisites\n"); + return 0; }
if (!heci_send_receive(&msg, sizeof(struct hmrfpo_enable_msg), &resp, &resp_size)) - goto failed; + return 0;
if (resp.hdr.result) { printk(BIOS_ERR, "HECI: Resp Failed:%d\n", resp.hdr.result); - goto failed; + return 0; } - return 1;
-failed: - return 0; + if (resp.status) { + printk(BIOS_ERR, "HECI: HMRFPO_Enable Failed (resp status: %d)\n", resp.status); + return 0; + } + + return 1; }
/* diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h index 93d1ce1..595c7d8 100644 --- a/src/soc/intel/common/block/include/intelblocks/cse.h +++ b/src/soc/intel/common/block/include/intelblocks/cse.h @@ -136,8 +136,20 @@ int cse_request_global_reset(enum rst_req_type rst_type);
/* - * Send HMRFPO_ENABLE command. - * returns 0 on failure and 1 on success. + * Sends HMRFPO_ENABLE command. + * HMRFPO - Host ME Region Flash Protection Override. + * For CSE Firmware SKU Custom, procedure to place CSE in HMRFPO (SECOVER_MEI_MSG) mode: + * 1. Ensure CSE boots from BP1(RO). + * - Send set_next_boot_partition(BP1) + * - Issue CSE Only Reset + * 2. Send HMRFPO_ENABLE command to CSE. Further, no reset is required. + * + * The HMRFPO mode prevents CSE to execute SPI I/O cycles to CSE region, and unlocks + * the CSE region to perform updates to it. + * This command is only valid before EOP. + * + * Returns 0 on failure to send HECI command and to enable HMRFPO mode, and 1 on success. + * */ int cse_hmrfpo_enable(void);
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Furquan Shaikh (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Rizwan Qureshi (Code Review)
-
Sridhar Siricilla (Code Review)