Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support ......................................................................
soc/intel/jasperlake: Enable end of post support
Send end of post message to CSME.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40612/1
diff --git a/src/soc/intel/jasperlake/fsp_params.c b/src/soc/intel/jasperlake/fsp_params.c index 19b9300..f90014c 100644 --- a/src/soc/intel/jasperlake/fsp_params.c +++ b/src/soc/intel/jasperlake/fsp_params.c @@ -97,6 +97,9 @@ /* Unlock upper 8 bytes of RTC RAM */ params->RtcMemoryLock = 0;
+ /* Enable End of Post */ + params->EndOfPostMessage = 0x01; + /* Legacy 8254 timer support */ params->Enable8254ClockGating = !CONFIG_USE_LEGACY_8254_TIMER; params->Enable8254ClockGatingOnS3 = 1;
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40612/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40612/1//COMMIT_MSG@7 PS1, Line 7: Enable end of post support In what phase?
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... PS1, Line 101: 0x01 So, what is the difference between sending EOP in PEI v/s DXE? I believe this is setting it to happen in PEI?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... PS1, Line 101: EndOfPostMessage also isn't EOP is mandatory and why do we have to set such UPDs which is silicon recommended settings. IMHO then UPD coreboot shouldn't even bother to touch default should be true, exposing this might provide chances for security exploitation from bootloader side.
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... PS1, Line 101: EndOfPostMessage
also isn't EOP is mandatory and why do we have to set such UPDs which is silicon recommended setting […]
Current dsc default(value: 2) is set for DXE(not to disable). Need to set it to PEI(value : 1) for EOP routine to be executed under FSP notify.
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... PS1, Line 101: 0x01
So, what is the difference between sending EOP in PEI v/s DXE? I believe this is setting it to happe […]
Yes, this is issue EOP in PEI phase. The DXE EOP implementation does not apply for coreboot. rather is used by UEFI bootloader. This configuration ensures the EOP routine is called in FSP Notify phase.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... PS1, Line 101: EndOfPostMessage
Current dsc default(value: 2) is set for DXE(not to disable). […]
Can you please add these details to commit message and a comment here indicating that?
Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40612
to look at the new patch set (#2).
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
soc/intel/jasperlake: Enable end of post support in FSP
Send end of post message to CSME in FSP, by selecting to send EndOfPost message in PEI phase. Default UPD configuration is to send EndOfPost message in DXE phase and not applicable for coreboot.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40612/2
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... PS1, Line 101: EndOfPostMessage
Can you please add these details to commit message and a comment here indicating that?
Added, please review.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/2/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/2/src/soc/intel/jasperlake/fs... PS2, Line 100: Enable End of Post * Can you please mention that this is done to ensure that EoP is sent in PEI rather than DXE? I haven't looked at the details, but I am guessing this actually happens when we send the Notify message in ramstage?
Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40612
to look at the new patch set (#3).
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
soc/intel/jasperlake: Enable end of post support in FSP
Send end of post message to CSME in FSP, by selecting to send EndOfPost message in PEI phase. Default UPD configuration is to send EndOfPost message in DXE phase and not applicable for coreboot.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40612/3
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/3/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/3/src/soc/intel/jasperlake/fs... PS3, Line 102: * 1 - Send in PEI (Appicable for coreboot) 'Appicable' may be misspelled - perhaps 'Applicable'?
Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40612
to look at the new patch set (#4).
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
soc/intel/jasperlake: Enable end of post support in FSP
Send end of post message to CSME in FSP, by selecting to send EndOfPost message in PEI phase. Default UPD configuration is to send EndOfPost message in DXE phase and not applicable for coreboot.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40612/4
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/4/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/4/src/soc/intel/jasperlake/fs... PS4, Line 103: Send Possible to convert this into enums
enum { CHIPSET_LOCKDOWN_FSP = 0, /* FSP handles locking per UPDs */ CHIPSET_LOCKDOWN_COREBOOT, /* coreboot handles locking */ };
Additionally for security reason, can we add an assert to avoid soc users to pass 0 to skip EOP?
Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40612
to look at the new patch set (#5).
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
soc/intel/jasperlake: Enable end of post support in FSP
Send end of post message to CSME in FSP, by selecting to send EndOfPost message in PEI phase. Default UPD configuration is to send EndOfPost message in DXE phase and not applicable for coreboot.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/fsp_params.c 2 files changed, 17 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40612/5
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40612/5//COMMIT_MSG@11 PS5, Line 11: applicable Why not? Please reference the corresponding document.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/5/src/soc/intel/jasperlake/ch... File src/soc/intel/jasperlake/chip.h:
https://review.coreboot.org/c/coreboot/+/40612/5/src/soc/intel/jasperlake/ch... PS5, Line 158: End_Of_Post_Disabled you can avoid camel case here
Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40612
to look at the new patch set (#6).
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
soc/intel/jasperlake: Enable end of post support in FSP
Send end of post message to CSME in FSP, by selecting to send EndOfPost message in PEI phase. Default UPD configuration is to send EndOfPost message in DXE phase and not applicable for coreboot.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/fsp_params.c 2 files changed, 17 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40612/6
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40612/5/src/soc/intel/jasperlake/ch... File src/soc/intel/jasperlake/chip.h:
https://review.coreboot.org/c/coreboot/+/40612/5/src/soc/intel/jasperlake/ch... PS5, Line 158: End_Of_Post_Disabled
you can avoid camel case here
Ok. Will set it as EOP_Disabled.
https://review.coreboot.org/c/coreboot/+/40612/4/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/4/src/soc/intel/jasperlake/fs... PS4, Line 103: Send
Possible to convert this into enums […]
Sure, Updated as enum.
We can add a runtime assert based on the UPD assignment here. But I think that can be bypassed if UPDs are made to change in coreboot fsp driver directly. Thoughts?
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/5/src/soc/intel/jasperlake/ch... File src/soc/intel/jasperlake/chip.h:
https://review.coreboot.org/c/coreboot/+/40612/5/src/soc/intel/jasperlake/ch... PS5, Line 158: End_Of_Post_Disabled
Ok. Will set it as EOP_Disabled.
Sure ok, Updated.
Done?
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40612/5//COMMIT_MSG@11 PS5, Line 11: applicable
Why not? Please reference the corresponding document.
Pls refer to EOP requirement documented in Doc#619830(section 3.6). Sending EOP via FSP requires EndOfPostMessage to be configured for PEI phase.
Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40612
to look at the new patch set (#7).
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
soc/intel/jasperlake: Enable end of post support in FSP
Send end of post message to CSME in FSP, by selecting to send EndOfPost message in PEI phase. Default UPD configuration is to send EndOfPost message in DXE phase and not applicable for FSP.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/fsp_params.c 2 files changed, 17 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40612/7
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40612/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40612/5//COMMIT_MSG@11 PS5, Line 11: applicable
Pls refer to EOP requirement documented in Doc#619830(section 3.6). […]
My understanding here is that FSP in API mode runs only PEI. There is no DXE. And so it is essential to set the EOP to happen in PEI. Aamir - does this match your understanding? If yes, can you please add this detail to commit message. Right now, it is not very clear "why" this change is being made.
https://review.coreboot.org/c/coreboot/+/40612/7/src/soc/intel/jasperlake/ch... File src/soc/intel/jasperlake/chip.h:
https://review.coreboot.org/c/coreboot/+/40612/7/src/soc/intel/jasperlake/ch... PS7, Line 151: /* : * ME End of Post configuration : * 0 - Disable EOP. : * 1 - Send in PEI (Applicable for coreboot) : * 2 - Send in DXE (Not applicable for coreboot) : */ : enum { : EOP_Disabled, : EOP_PEI, : EOP_DXE, : } EndOfPost; Any reason why this is living in chip.h instead of fsp_params.c? It doesn't seem to be configurable by mainboard anyways.
Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40612
to look at the new patch set (#8).
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
soc/intel/jasperlake: Enable end of post support in FSP
Send end of post message to CSME in FSP, by selecting EndOfPost message in PEI phase. DXE modules are not part of FSP. Sending EndOfPost message in DXE phase is not applicable for FSP.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 15 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40612/8
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40612/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40612/5//COMMIT_MSG@11 PS5, Line 11: applicable
My understanding here is that FSP in API mode runs only PEI. There is no DXE. […]
Yes Furquan. Updated the commit message.
https://review.coreboot.org/c/coreboot/+/40612/7/src/soc/intel/jasperlake/ch... File src/soc/intel/jasperlake/chip.h:
https://review.coreboot.org/c/coreboot/+/40612/7/src/soc/intel/jasperlake/ch... PS7, Line 151: /* : * ME End of Post configuration : * 0 - Disable EOP. : * 1 - Send in PEI (Applicable for coreboot) : * 2 - Send in DXE (Not applicable for coreboot) : */ : enum { : EOP_Disabled, : EOP_PEI, : EOP_DXE, : } EndOfPost;
Any reason why this is living in chip.h instead of fsp_params. […]
No. Moved to fsp_params.c now.
Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40612
to look at the new patch set (#9).
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
soc/intel/jasperlake: Enable end of post support in FSP
Send end of post message to CSME in FSP, by selecting EndOfPost message in PEI phase. DXE modules are not part of FSP. Sending EndOfPost message in DXE phase is not applicable for FSP.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 15 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40612/9
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 9:
(5 comments)
https://review.coreboot.org/c/coreboot/+/40612/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40612/1//COMMIT_MSG@7 PS1, Line 7: Enable end of post support
In what phase?
Done
https://review.coreboot.org/c/coreboot/+/40612/7/src/soc/intel/jasperlake/ch... File src/soc/intel/jasperlake/chip.h:
https://review.coreboot.org/c/coreboot/+/40612/7/src/soc/intel/jasperlake/ch... PS7, Line 151: /* : * ME End of Post configuration : * 0 - Disable EOP. : * 1 - Send in PEI (Applicable for coreboot) : * 2 - Send in DXE (Not applicable for coreboot) : */ : enum { : EOP_Disabled, : EOP_PEI, : EOP_DXE, : } EndOfPost;
No. Moved to fsp_params.c now.
Done
https://review.coreboot.org/c/coreboot/+/40612/5/src/soc/intel/jasperlake/ch... File src/soc/intel/jasperlake/chip.h:
https://review.coreboot.org/c/coreboot/+/40612/5/src/soc/intel/jasperlake/ch... PS5, Line 158: End_Of_Post_Disabled
Sure ok, Updated. […]
Done
https://review.coreboot.org/c/coreboot/+/40612/2/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/2/src/soc/intel/jasperlake/fs... PS2, Line 100: Enable End of Post *
Can you please mention that this is done to ensure that EoP is sent in PEI rather than DXE? I haven' […]
yes, the EOP is send in ready to boot notify phase.
https://review.coreboot.org/c/coreboot/+/40612/3/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/3/src/soc/intel/jasperlake/fs... PS3, Line 102: * 1 - Send in PEI (Appicable for coreboot)
'Appicable' may be misspelled - perhaps 'Applicable'?
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 9: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/40612/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40612/9//COMMIT_MSG@10 PS9, Line 10: DXE modules are not part of FSP nit: in API mode which coreboot currently uses.
https://review.coreboot.org/c/coreboot/+/40612/9/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/9/src/soc/intel/jasperlake/fs... PS9, Line 20: ) nit: in API mode
https://review.coreboot.org/c/coreboot/+/40612/9/src/soc/intel/jasperlake/fs... PS9, Line 21: ) nit: in API mode
Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40612
to look at the new patch set (#10).
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
soc/intel/jasperlake: Enable end of post support in FSP
Send end of post message to CSME in FSP, by selecting EndOfPost message in PEI phase. In API mode which coreboot currently uses, sending EndOfPost message in DXE phase is not applicable.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 15 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40612/10
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 10:
(7 comments)
https://review.coreboot.org/c/coreboot/+/40612/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40612/5//COMMIT_MSG@11 PS5, Line 11: applicable
Yes Furquan. Updated the commit message.
Done
https://review.coreboot.org/c/coreboot/+/40612/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40612/9//COMMIT_MSG@10 PS9, Line 10: DXE modules are not part of FSP
nit: in API mode which coreboot currently uses.
Done
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... PS1, Line 101: EndOfPostMessage
Added, please review.
Done
https://review.coreboot.org/c/coreboot/+/40612/1/src/soc/intel/jasperlake/fs... PS1, Line 101: 0x01
Yes, this is issue EOP in PEI phase. The DXE EOP implementation does not apply for coreboot. […]
Done
https://review.coreboot.org/c/coreboot/+/40612/4/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/4/src/soc/intel/jasperlake/fs... PS4, Line 103: Send
Sure, Updated as enum. […]
Ack
https://review.coreboot.org/c/coreboot/+/40612/9/src/soc/intel/jasperlake/fs... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/9/src/soc/intel/jasperlake/fs... PS9, Line 20: )
nit: in API mode
Done
https://review.coreboot.org/c/coreboot/+/40612/9/src/soc/intel/jasperlake/fs... PS9, Line 21: )
nit: in API mode
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 10: Code-Review+2
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 10: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/10/src/soc/intel/jasperlake/f... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/10/src/soc/intel/jasperlake/f... PS10, Line 24: EOP_Disabled Please do not use CamelCase is coreboot code.
EOP_DISABLED
Hello build bot (Jenkins), Furquan Shaikh, Maulik V Vaghela, Rizwan Qureshi, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40612
to look at the new patch set (#11).
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
soc/intel/jasperlake: Enable end of post support in FSP
Send end of post message to CSME in FSP, by selecting EndOfPost message in PEI phase. In API mode which coreboot currently uses, sending EndOfPost message in DXE phase is not applicable.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 15 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/40612/11
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40612/10/src/soc/intel/jasperlake/f... File src/soc/intel/jasperlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/40612/10/src/soc/intel/jasperlake/f... PS10, Line 24: EOP_Disabled
Please do not use CamelCase is coreboot code. […]
Done
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 11: Code-Review+2
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
soc/intel/jasperlake: Enable end of post support in FSP
Send end of post message to CSME in FSP, by selecting EndOfPost message in PEI phase. In API mode which coreboot currently uses, sending EndOfPost message in DXE phase is not applicable.
Change-Id: Ie21dcfc84d331f036090d01ea3e3925b81eea902 Signed-off-by: Aamir Bohra aamir.bohra@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/40612 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Karthik Ramasubramanian kramasub@google.com --- M src/soc/intel/jasperlake/fsp_params.c 1 file changed, 15 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Karthik Ramasubramanian: Looks good to me, approved
diff --git a/src/soc/intel/jasperlake/fsp_params.c b/src/soc/intel/jasperlake/fsp_params.c index 28dccab..38138be 100644 --- a/src/soc/intel/jasperlake/fsp_params.c +++ b/src/soc/intel/jasperlake/fsp_params.c @@ -14,6 +14,18 @@ #include <soc/soc_chip.h> #include <string.h>
+/* + * ME End of Post configuration + * 0 - Disable EOP. + * 1 - Send in PEI (Applicable for FSP in API mode) + * 2 - Send in DXE (Not applicable for FSP in API mode) + */ +enum { + EOP_DISABLE, + EOP_PEI, + EOP_DXE, +} EndOfPost; + static const pci_devfn_t serial_io_dev[] = { PCH_DEVFN_I2C0, PCH_DEVFN_I2C1, @@ -97,6 +109,9 @@ /* Unlock upper 8 bytes of RTC RAM */ params->RtcMemoryLock = 0;
+ /* Enable End of Post in PEI phase */ + params->EndOfPostMessage = EOP_PEI; + /* Legacy 8254 timer support */ params->Enable8254ClockGating = !CONFIG_USE_LEGACY_8254_TIMER; params->Enable8254ClockGatingOnS3 = 1;
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 12:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3300 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3299 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3298 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3297
Please note: This test is under development and might not be accurate at all!
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 12:
Do we need this for TGL?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40612 )
Change subject: soc/intel/jasperlake: Enable end of post support in FSP ......................................................................
Patch Set 12:
Patch Set 12:
Do we need this for TGL?
Yes, it seems to be required for TGL as well.
+Nick, +Tim, +Wonkyu.
-
9elements QA (Code Review) -
Aamir Bohra (Code Review)
-
build bot (Jenkins) (Code Review)
-
EricR Lai (Code Review)
-
Furquan Shaikh (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Subrata Banik (Code Review)