Change in coreboot[master]: soc/amd/picasso: Move APM_CNT_SMMINFO callsite

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

soc/amd/picasso: Move APM_CNT_SMMINFO callsite

Triggering SMI is not part of the semantics of
global_smi_enable().

Change-Id: Id0e7723c2bb9811f80fe36c38199a01445dc1d7d
Signed-off-by: Kyösti Mälkki 
---
M src/soc/amd/picasso/cpu.c
M src/soc/amd/picasso/smi.c
2 files changed, 7 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/42987/1

diff --git a/src/soc/amd/picasso/cpu.c b/src/soc/amd/picasso/cpu.c
index c42f400..6c6149f 100644
--- a/src/soc/amd/picasso/cpu.c
+++ b/src/soc/amd/picasso/cpu.c
@@ -85,12 +85,18 @@
    smm_state->smbase = staggered_smbase;
 }

+static void post_mp_init(void)
+{
+	global_smi_enable();
+	apm_control(APM_CNT_SMMINFO);
+}
+
 static const struct mp_ops mp_ops = {
    .pre_mp_init = pre_mp_init,
    .get_cpu_count = get_cpu_count,
    .get_smm_info = get_smm_info,
    .relocation_handler = relocation_handler,
-	.post_mp_init = global_smi_enable,
+	.post_mp_init = post_mp_init,
 };

void mp_init_cpus(struct bus *cpu_bus)
diff --git a/src/soc/amd/picasso/smi.c b/src/soc/amd/picasso/smi.c
index 125dde6..fb6d348 100644
--- a/src/soc/amd/picasso/smi.c
+++ b/src/soc/amd/picasso/smi.c
@@ -4,7 +4,6 @@
  * Utilities for SMM setup
  */

-#include <arch/io.h>
 #include <console/console.h>
 #include <cpu/x86/smm.h>
 #include <amdblocks/acpimmio.h>
@@ -18,6 +17,4 @@
    reg &= ~SMITRG0_SMIENB;	/* Enable SMI generation */
    reg |= SMITRG0_EOS;	/* Set EOS bit */
    smi_write32(SMI_REG_SMITRIG0, reg);
-
-	outb(APM_CNT_SMMINFO, APM_CNT);
 }

[Angel Pons (Code Review)]

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

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

Patch Set 3: Code-Review+2

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

Abandoned

[Felix Held (Code Review)]

Attention is currently required from: Kyösti Mälkki.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

Patch Set 3:

(1 comment)

Patchset:

PS3: 
it would be good if you can un-abandon this one and i'll do the manual rebase, rework and testing. there was a somewhat related change in https://review.coreboot.org/c/coreboot/+/44398 will need to have a closer look into that

[Kyösti Mälkki (Code Review)]

Attention is currently required from: Kyösti Mälkki.
Kyösti Mälkki has restored this change. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

Restored

[Kyösti Mälkki (Code Review)]

Attention is currently required from: Kyösti Mälkki.
Kyösti Mälkki has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

Removed Code-Review+2 by Angel Pons

[Angel Pons (Code Review)]

Attention is currently required from: Kyösti Mälkki.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

Patch Set 3: Code-Review+1

[Felix Held (Code Review)]

Attention is currently required from: Kyösti Mälkki.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

Patch Set 3:

(1 comment)

Patchset:

PS3: 
i'll push a rebased and updated patch as soon as it's tested

[Felix Held (Code Review)]

Attention is currently required from: Kyösti Mälkki, Felix Held.
Felix Held has uploaded a new patch set (#4) to the change originally created by Kyösti Mälkki. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

soc/amd/picasso: Move APM_CNT_SMMINFO callsite

Triggering SMI is not part of the semantics of global_smi_enable(), so
move it to the post_mp_init handler. Even without the !acpi_is_wakeup_s3
check we don't get PSP warnings/errors during resume, so we can drop the
workaround introduced in commit 5dbe45e0f5608794e03634aed28530ddb2ab9ac5
in this patch.

Change-Id: Id0e7723c2bb9811f80fe36c38199a01445dc1d7d
Signed-off-by: Kyösti Mälkki 
Signed-off-by: Felix Held 
---
M src/soc/amd/picasso/cpu.c
M src/soc/amd/picasso/smi.c
2 files changed, 7 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/42987/4

[Marshall Dawson (Code Review)]

Attention is currently required from: Jason Glenesk, Kyösti Mälkki, Felix Held.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

Patch Set 4: Code-Review+2

[Angel Pons (Code Review)]

Attention is currently required from: Jason Glenesk, Kyösti Mälkki, Felix Held.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

Patch Set 4: Code-Review+2

[Kyösti Mälkki (Code Review)]

Attention is currently required from: Jason Glenesk, Felix Held.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

Patch Set 4: Code-Review+1

(1 comment)

Patchset:

PS4: 
Felix, thanks for the update and test.

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42987 )

Change subject: soc/amd/picasso: Move APM_CNT_SMMINFO callsite
......................................................................

soc/amd/picasso: Move APM_CNT_SMMINFO callsite

Triggering SMI is not part of the semantics of global_smi_enable(), so
move it to the post_mp_init handler. Even without the !acpi_is_wakeup_s3
check we don't get PSP warnings/errors during resume, so we can drop the
workaround introduced in commit 5dbe45e0f5608794e03634aed28530ddb2ab9ac5
in this patch.

Change-Id: Id0e7723c2bb9811f80fe36c38199a01445dc1d7d
Signed-off-by: Kyösti Mälkki 
Signed-off-by: Felix Held 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42987
Reviewed-by: Marshall Dawson 
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/soc/amd/picasso/cpu.c
M src/soc/amd/picasso/smi.c
2 files changed, 7 insertions(+), 6 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, but someone else must approve
  Marshall Dawson: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/soc/amd/picasso/cpu.c b/src/soc/amd/picasso/cpu.c
index 9460485..8ee065d 100644
--- a/src/soc/amd/picasso/cpu.c
+++ b/src/soc/amd/picasso/cpu.c
@@ -81,12 +81,18 @@
    smm_state->smbase = staggered_smbase;
 }

+static void post_mp_init(void)
+{
+	global_smi_enable();
+	apm_control(APM_CNT_SMMINFO);
+}
+
 static const struct mp_ops mp_ops = {
    .pre_mp_init = pre_mp_init,
    .get_cpu_count = get_cpu_count,
    .get_smm_info = get_smm_info,
    .relocation_handler = relocation_handler,
-	.post_mp_init = global_smi_enable,
+	.post_mp_init = post_mp_init,
 };

void mp_init_cpus(struct bus *cpu_bus)
diff --git a/src/soc/amd/picasso/smi.c b/src/soc/amd/picasso/smi.c
index 5822151..fb6d348 100644
--- a/src/soc/amd/picasso/smi.c
+++ b/src/soc/amd/picasso/smi.c
@@ -4,8 +4,6 @@
  * Utilities for SMM setup
  */

-#include <acpi/acpi.h>
-#include <arch/io.h>
 #include <console/console.h>
 #include <cpu/x86/smm.h>
 #include <amdblocks/acpimmio.h>
@@ -19,7 +17,4 @@
    reg &= ~SMITRG0_SMIENB;	/* Enable SMI generation */
    reg |= SMITRG0_EOS;	/* Set EOS bit */
    smi_write32(SMI_REG_SMITRIG0, reg);
-
-	if (!acpi_is_wakeup_s3())
-		outb(APM_CNT_SMMINFO, APM_CNT);
 }