Bora Guvendik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32991
Change subject: soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware ......................................................................
soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware
Dumping ME status displays wrong information if we disable Heci1 because it is called after fsp notifies EndOfFirmware and disables Heci1. This patch moves the ME status dump before fsp notify EndOfFirmware.
TEST=Boot to OS, check ME dump information
Change-Id: Ifd8b18a41c502c4ecfb84698a7669028394589fd Signed-off-by: Bora Guvendik bora.guvendik@intel.com --- M src/soc/intel/cannonlake/finalize.c M src/soc/intel/cannonlake/include/soc/me.h M src/soc/intel/cannonlake/me.c 3 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/32991/1
diff --git a/src/soc/intel/cannonlake/finalize.c b/src/soc/intel/cannonlake/finalize.c index eb4c5c2..4dfd15b 100644 --- a/src/soc/intel/cannonlake/finalize.c +++ b/src/soc/intel/cannonlake/finalize.c @@ -26,7 +26,6 @@ #include <intelblocks/tco.h> #include <reg_script.h> #include <spi-generic.h> -#include <soc/me.h> #include <soc/p2sb.h> #include <soc/pci_devs.h> #include <soc/pcr_ids.h> @@ -95,8 +94,6 @@ { printk(BIOS_DEBUG, "Finalizing chipset.\n");
- dump_me_status(); - pch_finalize();
printk(BIOS_DEBUG, "Finalizing SMM.\n"); diff --git a/src/soc/intel/cannonlake/include/soc/me.h b/src/soc/intel/cannonlake/include/soc/me.h index 1d782c1..9084209 100644 --- a/src/soc/intel/cannonlake/include/soc/me.h +++ b/src/soc/intel/cannonlake/include/soc/me.h @@ -16,6 +16,6 @@ #ifndef _CANNONLAKE_ME_H_ #define _CANNONLAKE_ME_H_
-void dump_me_status(void); +void dump_me_status(void* unused);
#endif /* _CANNONLAKE_ME_H_ */ diff --git a/src/soc/intel/cannonlake/me.c b/src/soc/intel/cannonlake/me.c index 3fedc63..55afbe3 100644 --- a/src/soc/intel/cannonlake/me.c +++ b/src/soc/intel/cannonlake/me.c @@ -235,7 +235,7 @@ } BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_EXIT, print_me_version, NULL);
-void dump_me_status(void) +void dump_me_status(void* unused) { union hfsts1 hfsts1; union hfsts2 hfsts2; @@ -297,3 +297,6 @@ printk(BIOS_DEBUG, "ME: TXT Support : %s\n", hfsts6.fields.txt_support ? "YES" : "NO"); } + +BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_LOAD, BS_ON_ENTRY, dump_me_status, NULL); +BOOT_STATE_INIT_ENTRY(BS_OS_RESUME_CHECK, BS_ON_EXIT, dump_me_status, NULL);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32991 )
Change subject: soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/32991/1/src/soc/intel/cannonlake/include/soc... File src/soc/intel/cannonlake/include/soc/me.h:
https://review.coreboot.org/#/c/32991/1/src/soc/intel/cannonlake/include/soc... PS1, Line 19: void dump_me_status(void* unused); "foo* bar" should be "foo *bar"
https://review.coreboot.org/#/c/32991/1/src/soc/intel/cannonlake/me.c File src/soc/intel/cannonlake/me.c:
https://review.coreboot.org/#/c/32991/1/src/soc/intel/cannonlake/me.c@238 PS1, Line 238: void dump_me_status(void* unused) "foo* bar" should be "foo *bar"
Bora Guvendik has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/32991 )
Change subject: soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware ......................................................................
soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware
Dumping ME status displays wrong information if we disable Heci1 because it is called after fsp notifies EndOfFirmware and disables Heci1. This patch moves the ME status dump before fsp notify EndOfFirmware.
TEST=Boot to OS, check ME dump information
Change-Id: Ifd8b18a41c502c4ecfb84698a7669028394589fd Signed-off-by: Bora Guvendik bora.guvendik@intel.com --- M src/soc/intel/cannonlake/finalize.c M src/soc/intel/cannonlake/include/soc/me.h M src/soc/intel/cannonlake/me.c 3 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/32991/2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32991 )
Change subject: soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware ......................................................................
Patch Set 2: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32991 )
Change subject: soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware ......................................................................
Patch Set 2: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32991 )
Change subject: soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware ......................................................................
Patch Set 2: Code-Review+2
Duncan Laurie has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32991 )
Change subject: soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware ......................................................................
soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware
Dumping ME status displays wrong information if we disable Heci1 because it is called after fsp notifies EndOfFirmware and disables Heci1. This patch moves the ME status dump before fsp notify EndOfFirmware.
TEST=Boot to OS, check ME dump information
Change-Id: Ifd8b18a41c502c4ecfb84698a7669028394589fd Signed-off-by: Bora Guvendik bora.guvendik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32991 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Subrata Banik subrata.banik@intel.com Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/cannonlake/finalize.c M src/soc/intel/cannonlake/include/soc/me.h M src/soc/intel/cannonlake/me.c 3 files changed, 5 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Subrata Banik: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/finalize.c b/src/soc/intel/cannonlake/finalize.c index eb4c5c2..4dfd15b 100644 --- a/src/soc/intel/cannonlake/finalize.c +++ b/src/soc/intel/cannonlake/finalize.c @@ -26,7 +26,6 @@ #include <intelblocks/tco.h> #include <reg_script.h> #include <spi-generic.h> -#include <soc/me.h> #include <soc/p2sb.h> #include <soc/pci_devs.h> #include <soc/pcr_ids.h> @@ -95,8 +94,6 @@ { printk(BIOS_DEBUG, "Finalizing chipset.\n");
- dump_me_status(); - pch_finalize();
printk(BIOS_DEBUG, "Finalizing SMM.\n"); diff --git a/src/soc/intel/cannonlake/include/soc/me.h b/src/soc/intel/cannonlake/include/soc/me.h index 1d782c1..5b411d3 100644 --- a/src/soc/intel/cannonlake/include/soc/me.h +++ b/src/soc/intel/cannonlake/include/soc/me.h @@ -16,6 +16,6 @@ #ifndef _CANNONLAKE_ME_H_ #define _CANNONLAKE_ME_H_
-void dump_me_status(void); +void dump_me_status(void *unused);
#endif /* _CANNONLAKE_ME_H_ */ diff --git a/src/soc/intel/cannonlake/me.c b/src/soc/intel/cannonlake/me.c index 3fedc63..c9748b7 100644 --- a/src/soc/intel/cannonlake/me.c +++ b/src/soc/intel/cannonlake/me.c @@ -235,7 +235,7 @@ } BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_EXIT, print_me_version, NULL);
-void dump_me_status(void) +void dump_me_status(void *unused) { union hfsts1 hfsts1; union hfsts2 hfsts2; @@ -297,3 +297,6 @@ printk(BIOS_DEBUG, "ME: TXT Support : %s\n", hfsts6.fields.txt_support ? "YES" : "NO"); } + +BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_LOAD, BS_ON_ENTRY, dump_me_status, NULL); +BOOT_STATE_INIT_ENTRY(BS_OS_RESUME_CHECK, BS_ON_EXIT, dump_me_status, NULL);
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32991 )
Change subject: soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/32991/4/src/soc/intel/cannonlake/me.c File src/soc/intel/cannonlake/me.c:
https://review.coreboot.org/#/c/32991/4/src/soc/intel/cannonlake/me.c@302 PS4, Line 302: BS_OS_RESUME_CHECK With this, the status would get dumped twice in case of non-resume boot.