Change in coreboot[master]: cpu/x86/smm/smihandler.c: Implement smm_get_save_state()

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44320 )

Change subject: cpu/x86/smm/smihandler.c: Implement smm_get_save_state()
......................................................................

cpu/x86/smm/smihandler.c: Implement smm_get_save_state()

This will be used in common save_state handling code.

Change-Id: I4cb3180ec565cee931606e8a8f55b78fdb8932ae
Signed-off-by: Arthur Heymans 
---
M src/cpu/x86/smm/smihandler.c
1 file changed, 18 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/44320/1

diff --git a/src/cpu/x86/smm/smihandler.c b/src/cpu/x86/smm/smihandler.c
index 99594be..ebb7a9b 100644
--- a/src/cpu/x86/smm/smihandler.c
+++ b/src/cpu/x86/smm/smihandler.c
@@ -128,6 +128,24 @@
    return *(uint32_t *)(SMM_BASE + SMM_ENTRY_OFFSET * 2 - SMM_REVISION_OFFSET_FROM_TOP);
 }

+void *smm_get_save_state(int cpu)
+{
+	switch(smm_revision()) {
+	case 0x00030002:
+	case 0x00030007:
+		return smm_save_state(SMM_BASE, SMM_LEGACY_ARCH_OFFSET, cpu);
+	case 0x00030100:
+		return smm_save_state(SMM_BASE, SMM_EM64T100_ARCH_OFFSET, cpu);
+	case 0x00030101: /* SandyBridge, IvyBridge, and Haswell */
+		return smm_save_state(SMM_BASE, SMM_EM64T101_ARCH_OFFSET, cpu);
+	case 0x00020064:
+	case 0x00030064:
+		return smm_save_state(SMM_BASE, SMM_AMD64_ARCH_OFFSET, cpu);
+	}
+
+	return NULL;
+}
+
 bool smm_region_overlaps_handler(const struct region *r)
 {
    const struct region r_smm = {SMM_BASE, SMM_DEFAULT_SIZE};

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44320 )

Change subject: cpu/x86/smm/smihandler.c: Implement smm_get_save_state()
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44320/1/src/cpu/x86/smm/smihandler.... 
File src/cpu/x86/smm/smihandler.c:

https://review.coreboot.org/c/coreboot/+/44320/1/src/cpu/x86/smm/smihandler.... 
PS1, Line 133: 	switch(smm_revision()) {
space required before the open parenthesis '('

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44320 )

Change subject: cpu/x86/smm/smihandler.c: Implement smm_get_save_state()
......................................................................

Patch Set 1: Code-Review+1

(2 comments)

https://review.coreboot.org/c/coreboot/+/44320/1/src/cpu/x86/smm/smihandler.... 
File src/cpu/x86/smm/smihandler.c:

https://review.coreboot.org/c/coreboot/+/44320/1/src/cpu/x86/smm/smihandler.... 
PS1, Line 133: 	switch(smm_revision()) {
> space required before the open parenthesis '('

Would be nice to handle

https://review.coreboot.org/c/coreboot/+/44320/1/src/cpu/x86/smm/smihandler.... 
PS1, Line 155: 
Could we place `smm_get_save_state` here instead? That way, it's closer to the `switch` with the same numbers in `smi_handler`

[Arthur Heymans (Code Review)]

Hello build bot (Jenkins), Angel Pons,

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

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

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

Change subject: cpu/x86/smm/smihandler.c: Implement smm_get_save_state()
......................................................................

cpu/x86/smm/smihandler.c: Implement smm_get_save_state()

This will be used in common save_state handling code.

Change-Id: I4cb3180ec565cee931606e8a8f55b78fdb8932ae
Signed-off-by: Arthur Heymans 
---
M src/cpu/x86/smm/smihandler.c
1 file changed, 18 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/44320/2

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44320 )

Change subject: cpu/x86/smm/smihandler.c: Implement smm_get_save_state()
......................................................................

Patch Set 2: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/44320/1/src/cpu/x86/smm/smihandler.... 
File src/cpu/x86/smm/smihandler.c:

https://review.coreboot.org/c/coreboot/+/44320/1/src/cpu/x86/smm/smihandler.... 
PS1, Line 133: 	switch(smm_revision()) {
> Would be nice to handle

Done

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44320 )

Change subject: cpu/x86/smm/smihandler.c: Implement smm_get_save_state()
......................................................................

Patch Set 3: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/44320/1/src/cpu/x86/smm/smihandler.... 
File src/cpu/x86/smm/smihandler.c:

https://review.coreboot.org/c/coreboot/+/44320/1/src/cpu/x86/smm/smihandler.... 
PS1, Line 155:
> Could we place `smm_get_save_state` here instead? That way, it's closer to the `switch` with the sam […]

Ack

[Patrick Georgi (Code Review)]

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

Change subject: cpu/x86/smm/smihandler.c: Implement smm_get_save_state()
......................................................................

cpu/x86/smm/smihandler.c: Implement smm_get_save_state()

This will be used in common save_state handling code.

Change-Id: I4cb3180ec565cee931606e8a8f55b78fdb8932ae
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44320
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/cpu/x86/smm/smihandler.c
1 file changed, 18 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/cpu/x86/smm/smihandler.c b/src/cpu/x86/smm/smihandler.c
index 99594be..077fa8d 100644
--- a/src/cpu/x86/smm/smihandler.c
+++ b/src/cpu/x86/smm/smihandler.c
@@ -128,6 +128,24 @@
    return *(uint32_t *)(SMM_BASE + SMM_ENTRY_OFFSET * 2 - SMM_REVISION_OFFSET_FROM_TOP);
 }

+void *smm_get_save_state(int cpu)
+{
+	switch (smm_revision()) {
+	case 0x00030002:
+	case 0x00030007:
+		return smm_save_state(SMM_BASE, SMM_LEGACY_ARCH_OFFSET, cpu);
+	case 0x00030100:
+		return smm_save_state(SMM_BASE, SMM_EM64T100_ARCH_OFFSET, cpu);
+	case 0x00030101: /* SandyBridge, IvyBridge, and Haswell */
+		return smm_save_state(SMM_BASE, SMM_EM64T101_ARCH_OFFSET, cpu);
+	case 0x00020064:
+	case 0x00030064:
+		return smm_save_state(SMM_BASE, SMM_AMD64_ARCH_OFFSET, cpu);
+	}
+
+	return NULL;
+}
+
 bool smm_region_overlaps_handler(const struct region *r)
 {
    const struct region r_smm = {SMM_BASE, SMM_DEFAULT_SIZE};