Change in coreboot[master]: soc/intel/tigerlake: Print the value of HPR_CAUSE0 register

[Derek Huang (Code Review)]

Derek Huang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40648 )

Change subject: soc/intel/tigerlake: Print the value of HPR_CAUSE0 register
......................................................................

soc/intel/tigerlake: Print the value of HPR_CAUSE0 register

In addition to GBLRST_CAUSE0 and GBLRST_CAUSE1, print the value of
HPR_CAUSE0 to understand the causes of host reset

Change-Id: Idc57c3cd6a8d156c5544640898e8e7147d34c535
Signed-off-by: derek.huang 
---
M src/soc/intel/tigerlake/include/soc/pm.h
M src/soc/intel/tigerlake/include/soc/pmc.h
M src/soc/intel/tigerlake/pmutil.c
3 files changed, 5 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/40648/1

diff --git a/src/soc/intel/tigerlake/include/soc/pm.h b/src/soc/intel/tigerlake/include/soc/pm.h
index 3004b4a..2a93149 100644
--- a/src/soc/intel/tigerlake/include/soc/pm.h
+++ b/src/soc/intel/tigerlake/include/soc/pm.h
@@ -150,6 +150,7 @@
    uint32_t gen_pmcon_a;
    uint32_t gen_pmcon_b;
    uint32_t gblrst_cause[2];
+	uint32_t hpr_cause0;
    uint32_t prev_sleep_state;
 } __packed;

diff --git a/src/soc/intel/tigerlake/include/soc/pmc.h b/src/soc/intel/tigerlake/include/soc/pmc.h
index 88c6f61..4faba8f 100644
--- a/src/soc/intel/tigerlake/include/soc/pmc.h
+++ b/src/soc/intel/tigerlake/include/soc/pmc.h
@@ -119,6 +119,7 @@
 #define GBLRST_CAUSE0			0x1924
 #define   GBLRST_CAUSE0_THERMTRIP	(1 << 5)
 #define GBLRST_CAUSE1			0x1928
+#define HPR_CAUSE0			0x192C

#define CPPMVRIC			0x1B1C
 #define   XTALSDQDIS			(1 << 22)
diff --git a/src/soc/intel/tigerlake/pmutil.c b/src/soc/intel/tigerlake/pmutil.c
index 4482b1e..554932b 100644
--- a/src/soc/intel/tigerlake/pmutil.c
+++ b/src/soc/intel/tigerlake/pmutil.c
@@ -260,12 +260,15 @@
    ps->gen_pmcon_b = read32(pmc + GEN_PMCON_B);
    ps->gblrst_cause[0] = read32(pmc + GBLRST_CAUSE0);
    ps->gblrst_cause[1] = read32(pmc + GBLRST_CAUSE1);
+	ps->hpr_cause0 = read32(pmc + HPR_CAUSE0);

printk(BIOS_DEBUG, "GEN_PMCON: %08x %08x\n",
    	ps->gen_pmcon_a, ps->gen_pmcon_b);

printk(BIOS_DEBUG, "GBLRST_CAUSE: %08x %08x\n",
    	ps->gblrst_cause[0], ps->gblrst_cause[1]);
+
+	printk(BIOS_DEBUG, "HPR_CAUSE0: %08x\n", ps->hpr_cause0);
 }

/* STM Support */

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40648 )

Change subject: soc/intel/tigerlake: Print the value of HPR_CAUSE0 register
......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/40648/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40648/1//COMMIT_MSG@10 
PS1, Line 10: HPR_CAUSE0 to understand the causes of host reset
Please add a dot/period at the end of sentences.

[Derek Huang (Code Review)]

Hello build bot (Jenkins), Paul Menzel, Kane Chen, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40648

to look at the new patch set (#2).

Change subject: soc/intel/tigerlake: Print the value of HPR_CAUSE0 register
......................................................................

soc/intel/tigerlake: Print the value of HPR_CAUSE0 register

In addition to GBLRST_CAUSE0 and GBLRST_CAUSE1, print the value of
HPR_CAUSE0 to understand the causes of host reset.

Change-Id: Idc57c3cd6a8d156c5544640898e8e7147d34c535
Signed-off-by: derek.huang 
---
M src/soc/intel/tigerlake/include/soc/pm.h
M src/soc/intel/tigerlake/include/soc/pmc.h
M src/soc/intel/tigerlake/pmutil.c
3 files changed, 5 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/40648/2

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40648 )

Change subject: soc/intel/tigerlake: Print the value of HPR_CAUSE0 register
......................................................................

Patch Set 2:

(1 comment)

I'm wondering

https://review.coreboot.org/c/coreboot/+/40648/2/src/soc/intel/tigerlake/inc... 
File src/soc/intel/tigerlake/include/soc/pmc.h:

https://review.coreboot.org/c/coreboot/+/40648/2/src/soc/intel/tigerlake/inc... 
PS2, Line 123: 
I think it may be useful to call out the CSME-initiated bits here. And we should also update the elog to include reset causes from this register as well.

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog

[Derek Huang (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Tim Wawrzynczak, Alex Levin, Paul Menzel, Nick Vaccaro, Kane Chen, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40648

to look at the new patch set (#3).

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog
......................................................................

soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog

In addition to GBLRST_CAUSE0 and GBLRST_CAUSE1, print the value
of HPR_CAUSE0. Also call out the CSME-initiated bits from this
register and update the elog to include reset causes

Change-Id: Idc57c3cd6a8d156c5544640898e8e7147d34c535
Signed-off-by: derek.huang 
---
M src/include/elog.h
M src/soc/intel/tigerlake/elog.c
M src/soc/intel/tigerlake/include/soc/pm.h
M src/soc/intel/tigerlake/include/soc/pmc.h
M src/soc/intel/tigerlake/pmutil.c
5 files changed, 25 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/40648/3

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog

[Kane Chen (Code Review)]

Kane Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40648 )

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40648/3/src/include/elog.h 
File src/include/elog.h:

https://review.coreboot.org/c/coreboot/+/40648/3/src/include/elog.h@198 
PS3, Line 198: /* ME-Initiated Host Reset */
             : #define ELOG_TYPE_MI_HRPD                 0xb3
             : #define ELOG_TYPE_MI_HRPC                 0xb4
             : #define ELOG_TYPE_MI_HR                   0xb5
Should we create separate patch for this header change?
I meant.
1. elog header change
2. print rst cause and save elog.

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog

[Derek Huang (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Tim Wawrzynczak, Alex Levin, Paul Menzel, Nick Vaccaro, Kane Chen, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40648

to look at the new patch set (#4).

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog
......................................................................

soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog

In addition to GBLRST_CAUSE0 and GBLRST_CAUSE1, print the value
of HPR_CAUSE0. Also call out the CSME-initiated bits from this
register and update the elog to include reset causes

Change-Id: Idc57c3cd6a8d156c5544640898e8e7147d34c535
Signed-off-by: derek.huang 
---
M src/soc/intel/tigerlake/elog.c
M src/soc/intel/tigerlake/include/soc/pm.h
M src/soc/intel/tigerlake/include/soc/pmc.h
M src/soc/intel/tigerlake/pmutil.c
4 files changed, 20 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/40648/4

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40648 )

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40648/5/src/soc/intel/tigerlake/elo... 
File src/soc/intel/tigerlake/elog.c:

https://review.coreboot.org/c/coreboot/+/40648/5/src/soc/intel/tigerlake/elo... 
PS5, Line 65: downi
down

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40648 )

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog
......................................................................

Patch Set 5:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40648/3/src/include/elog.h 
File src/include/elog.h:

https://review.coreboot.org/c/coreboot/+/40648/3/src/include/elog.h@198 
PS3, Line 198: /* ME-Initiated Host Reset */
             : #define ELOG_TYPE_MI_HRPD                 0xb3
             : #define ELOG_TYPE_MI_HRPC                 0xb4
             : #define ELOG_TYPE_MI_HR                   0xb5
> Should we create separate patch for this header change? […]

+1

https://review.coreboot.org/c/coreboot/+/40648/5/src/soc/intel/tigerlake/elo... 
File src/soc/intel/tigerlake/elog.c:

https://review.coreboot.org/c/coreboot/+/40648/5/src/soc/intel/tigerlake/elo... 
PS5, Line 67: ELOG_TYPE_MI_HRPD
Where are these ELOG_TYPE_MI_* defined? I don't see them... We will also need to update mosys on the chromeos side to match these new ELOG entries with descriptive strings for the eventlog.

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog

[Derek Huang (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Rizwan Qureshi, Tim Wawrzynczak, Paul Menzel, Alex Levin, Nick Vaccaro, Subrata Banik, Sridhar Siricilla, Kane Chen, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40648

to look at the new patch set (#6).

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog
......................................................................

soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog

In addition to GBLRST_CAUSE0 and GBLRST_CAUSE1, print the value
of HPR_CAUSE0. Also call out the CSME-initiated bits from this
register and update the elog to include reset causes

Change-Id: Idc57c3cd6a8d156c5544640898e8e7147d34c535
Signed-off-by: derek.huang 
---
M src/soc/intel/tigerlake/elog.c
M src/soc/intel/tigerlake/include/soc/pm.h
M src/soc/intel/tigerlake/include/soc/pmc.h
M src/soc/intel/tigerlake/pmutil.c
4 files changed, 20 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/40648/6

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40648 )

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register and update elog
......................................................................

Patch Set 6:

Would you mind splitting out the elog.c change into a different CL?

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register

[Derek Huang (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Tim Wawrzynczak, Rizwan Qureshi, Paul Menzel, Alex Levin, Nick Vaccaro, Subrata Banik, Sridhar Siricilla, Kane Chen, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40648

to look at the new patch set (#7).

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register
......................................................................

soc/intel/tigerlake: Print HPR_CAUSE0 register

In addition to GBLRST_CAUSE0 and GBLRST_CAUSE1, print the value
of HPR_CAUSE0.

Change-Id: Idc57c3cd6a8d156c5544640898e8e7147d34c535
Signed-off-by: derek.huang 
---
M src/soc/intel/tigerlake/include/soc/pm.h
M src/soc/intel/tigerlake/include/soc/pmc.h
M src/soc/intel/tigerlake/pmutil.c
3 files changed, 8 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/40648/7

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register

[Derek Huang (Code Review)]

Derek Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40648 )

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register
......................................................................

Patch Set 7:

(5 comments)
> Patch Set 6:
>
> Would you mind splitting out the elog.c change into a different CL?

Done

https://review.coreboot.org/c/coreboot/+/40648/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40648/1//COMMIT_MSG@10 
PS1, Line 10: HPR_CAUSE0 to understand the causes of host reset
> Please add a dot/period at the end of sentences.

Done

https://review.coreboot.org/c/coreboot/+/40648/3/src/include/elog.h 
File src/include/elog.h:

https://review.coreboot.org/c/coreboot/+/40648/3/src/include/elog.h@198 
PS3, Line 198: /* ME-Initiated Host Reset */
             : #define ELOG_TYPE_MI_HRPD                 0xb3
             : #define ELOG_TYPE_MI_HRPC                 0xb4
             : #define ELOG_TYPE_MI_HR                   0xb5
> Should we create separate patch for this header change? […]

OK. I will create a separate patch for header change

https://review.coreboot.org/c/coreboot/+/40648/5/src/soc/intel/tigerlake/elo... 
File src/soc/intel/tigerlake/elog.c:

https://review.coreboot.org/c/coreboot/+/40648/5/src/soc/intel/tigerlake/elo... 
PS5, Line 65: downi
> down

Done

https://review.coreboot.org/c/coreboot/+/40648/5/src/soc/intel/tigerlake/elo... 
PS5, Line 67: ELOG_TYPE_MI_HRPD
> Where are these ELOG_TYPE_MI_* defined? I don't see them... […]

Done. A separate patch is created for header file. will submit mosys CL once the changes are merged.

https://review.coreboot.org/c/coreboot/+/40648/2/src/soc/intel/tigerlake/inc... 
File src/soc/intel/tigerlake/include/soc/pmc.h:

https://review.coreboot.org/c/coreboot/+/40648/2/src/soc/intel/tigerlake/inc... 
PS2, Line 123:
> I think it may be useful to call out the CSME-initiated bits here. […]

Done

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40648 )

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register
......................................................................

Patch Set 7: Code-Review+2

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40648 )

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register
......................................................................

soc/intel/tigerlake: Print HPR_CAUSE0 register

In addition to GBLRST_CAUSE0 and GBLRST_CAUSE1, print the value
of HPR_CAUSE0.

Change-Id: Idc57c3cd6a8d156c5544640898e8e7147d34c535
Signed-off-by: derek.huang 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40648
Tested-by: build bot (Jenkins) 
Reviewed-by: Tim Wawrzynczak 
---
M src/soc/intel/tigerlake/include/soc/pm.h
M src/soc/intel/tigerlake/include/soc/pmc.h
M src/soc/intel/tigerlake/pmutil.c
3 files changed, 8 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Tim Wawrzynczak: Looks good to me, approved

diff --git a/src/soc/intel/tigerlake/include/soc/pm.h b/src/soc/intel/tigerlake/include/soc/pm.h
index c42e280..c69fe3e 100644
--- a/src/soc/intel/tigerlake/include/soc/pm.h
+++ b/src/soc/intel/tigerlake/include/soc/pm.h
@@ -150,6 +150,7 @@
    uint32_t gen_pmcon_a;
    uint32_t gen_pmcon_b;
    uint32_t gblrst_cause[2];
+	uint32_t hpr_cause0;
    uint32_t prev_sleep_state;
 } __packed;

diff --git a/src/soc/intel/tigerlake/include/soc/pmc.h b/src/soc/intel/tigerlake/include/soc/pmc.h
index 88c6f61..9ad3391 100644
--- a/src/soc/intel/tigerlake/include/soc/pmc.h
+++ b/src/soc/intel/tigerlake/include/soc/pmc.h
@@ -119,6 +119,10 @@
 #define GBLRST_CAUSE0			0x1924
 #define   GBLRST_CAUSE0_THERMTRIP	(1 << 5)
 #define GBLRST_CAUSE1			0x1928
+#define HPR_CAUSE0			0x192C
+#define   HPR_CAUSE0_MI_HRPD		(1 << 10)
+#define   HPR_CAUSE0_MI_HRPC		(1 << 9)
+#define   HPR_CAUSE0_MI_HR		(1 << 8)

#define CPPMVRIC			0x1B1C
 #define   XTALSDQDIS			(1 << 22)
diff --git a/src/soc/intel/tigerlake/pmutil.c b/src/soc/intel/tigerlake/pmutil.c
index 4482b1e..554932b 100644
--- a/src/soc/intel/tigerlake/pmutil.c
+++ b/src/soc/intel/tigerlake/pmutil.c
@@ -260,12 +260,15 @@
    ps->gen_pmcon_b = read32(pmc + GEN_PMCON_B);
    ps->gblrst_cause[0] = read32(pmc + GBLRST_CAUSE0);
    ps->gblrst_cause[1] = read32(pmc + GBLRST_CAUSE1);
+	ps->hpr_cause0 = read32(pmc + HPR_CAUSE0);

printk(BIOS_DEBUG, "GEN_PMCON: %08x %08x\n",
    	ps->gen_pmcon_a, ps->gen_pmcon_b);

printk(BIOS_DEBUG, "GBLRST_CAUSE: %08x %08x\n",
    	ps->gblrst_cause[0], ps->gblrst_cause[1]);
+
+	printk(BIOS_DEBUG, "HPR_CAUSE0: %08x\n", ps->hpr_cause0);
 }

/* STM Support */

Change in coreboot[master]: soc/intel/tigerlake: Print HPR_CAUSE0 register

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40648 )

Change subject: soc/intel/tigerlake: Print HPR_CAUSE0 register
......................................................................

Patch Set 8:

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/3227
"QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3226
"QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3225
"QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3224

Please note: This test is under development and might not be accurate at all!