Change in coreboot[master]: ACPI: Final APM_CNT_GNVS_UPDATE cleanup

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

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

Change subject: ACPI: Final APM_CNT_GNVS_UPDATE cleanup
......................................................................

ACPI: Final APM_CNT_GNVS_UPDATE cleanup

All platforms moved to initialise GNVS at the time
of SMM module loading.

Change-Id: I31b5652a946b0d9bd1909ff8bde53b43e06e2cd9
Signed-off-by: Kyösti Mälkki 
---
M src/arch/x86/acpi_s3.c
M src/cpu/x86/smm/smi_trigger.c
M src/include/cpu/x86/smm.h
3 files changed, 1 insertion(+), 41 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/48699/1

diff --git a/src/arch/x86/acpi_s3.c b/src/arch/x86/acpi_s3.c
index 43a68f9..ec1fafc 100644
--- a/src/arch/x86/acpi_s3.c
+++ b/src/arch/x86/acpi_s3.c
@@ -5,7 +5,6 @@
 #include <acpi/acpi.h>
 #include <arch/cpu.h>
 #include <commonlib/helpers.h>
-#include <cpu/x86/smm.h>
 #include <fallback.h>
 #include <timestamp.h>

@@ -18,9 +17,6 @@

void __noreturn acpi_resume(void *wake_vec)
 {
-	/* Restore GNVS pointer in SMM if found. */
-	apm_control(APM_CNT_GNVS_UPDATE);
-
    /* Call mainboard resume handler first, if defined. */
    mainboard_suspend_resume();

diff --git a/src/cpu/x86/smm/smi_trigger.c b/src/cpu/x86/smm/smi_trigger.c
index 4b63745..7e3ddb1 100644
--- a/src/cpu/x86/smm/smi_trigger.c
+++ b/src/cpu/x86/smm/smi_trigger.c
@@ -1,12 +1,9 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

-#include <acpi/acpi_gnvs.h>
 #include <arch/io.h>
 #include <console/console.h>
 #include <cpu/x86/smm.h>

-static void set_smm_gnvs_ptr(void);
-
 int apm_control(u8 cmd)
 {
    if (!CONFIG(HAVE_SMI_HANDLER))
@@ -23,9 +20,6 @@
    case APM_CNT_ACPI_ENABLE:
    	printk(BIOS_DEBUG, "Enabling ACPI via APMC.\n");
    	break;
-	case APM_CNT_GNVS_UPDATE:
-		set_smm_gnvs_ptr();
-		return 0;
    case APM_CNT_FINALIZE:
    	printk(BIOS_DEBUG, "Finalizing SMM.\n");
    	break;
@@ -45,32 +39,3 @@
    printk(BIOS_DEBUG, "APMC done.\n");
    return 0;
 }
-
-static void set_smm_gnvs_ptr(void)
-{
-	uintptr_t gnvs_address;
-
-	if (CONFIG(ACPI_NO_SMI_GNVS)) {
-		printk(BIOS_WARNING, "%s() is not implemented\n", __func__);
-		return;
-	}
-
-	gnvs_address = (uintptr_t)acpi_get_gnvs();
-	if (!gnvs_address)
-		return;
-
-	/*
-	 * Issue SMI to set the gnvs pointer in SMM.
-	 *
-	 * EAX = APM_CNT_GNVS_UPDATE
-	 * EBX = gnvs pointer
-	 * EDX = APM_CNT
-	 */
-	asm volatile (
-		"outb %%al, %%dx\n\t"
-		: /* ignore result */
-		: "a" (APM_CNT_GNVS_UPDATE),
-		  "b" (gnvs_address),
-		  "d" (APM_CNT)
-	);
-}
diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h
index 6dfe08d..9106842 100644
--- a/src/include/cpu/x86/smm.h
+++ b/src/include/cpu/x86/smm.h
@@ -22,11 +22,10 @@
 #define APM_CNT_PST_CONTROL	0x80
 #define APM_CNT_ACPI_DISABLE	0x1e
 #define APM_CNT_ACPI_ENABLE	0xe1
-#define APM_CNT_MBI_UPDATE	0xeb
-#define APM_CNT_GNVS_UPDATE	0xea
 #define APM_CNT_ROUTE_ALL_XHCI	0xca
 #define APM_CNT_FINALIZE	0xcb
 #define APM_CNT_LEGACY		0xcc
+#define APM_CNT_MBI_UPDATE	0xeb
 #define APM_CNT_SMMINFO		0xec
 #define APM_CNT_SMMSTORE	0xed
 #define APM_CNT_ELOG_GSMI	0xef

[Angel Pons (Code Review)]

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

Change subject: ACPI: Final APM_CNT_GNVS_UPDATE cleanup
......................................................................

Patch Set 2: Code-Review+2

[Patrick Georgi (Code Review)]

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

Change subject: ACPI: Final APM_CNT_GNVS_UPDATE cleanup
......................................................................

ACPI: Final APM_CNT_GNVS_UPDATE cleanup

All platforms moved to initialise GNVS at the time
of SMM module loading.

Change-Id: I31b5652a946b0d9bd1909ff8bde53b43e06e2cd9
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48699
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/arch/x86/acpi_s3.c
M src/cpu/x86/smm/smi_trigger.c
M src/include/cpu/x86/smm.h
3 files changed, 1 insertion(+), 41 deletions(-)

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

diff --git a/src/arch/x86/acpi_s3.c b/src/arch/x86/acpi_s3.c
index 43a68f9..ec1fafc 100644
--- a/src/arch/x86/acpi_s3.c
+++ b/src/arch/x86/acpi_s3.c
@@ -5,7 +5,6 @@
 #include <acpi/acpi.h>
 #include <arch/cpu.h>
 #include <commonlib/helpers.h>
-#include <cpu/x86/smm.h>
 #include <fallback.h>
 #include <timestamp.h>

@@ -18,9 +17,6 @@

void __noreturn acpi_resume(void *wake_vec)
 {
-	/* Restore GNVS pointer in SMM if found. */
-	apm_control(APM_CNT_GNVS_UPDATE);
-
    /* Call mainboard resume handler first, if defined. */
    mainboard_suspend_resume();

diff --git a/src/cpu/x86/smm/smi_trigger.c b/src/cpu/x86/smm/smi_trigger.c
index 4b63745..7e3ddb1 100644
--- a/src/cpu/x86/smm/smi_trigger.c
+++ b/src/cpu/x86/smm/smi_trigger.c
@@ -1,12 +1,9 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

-#include <acpi/acpi_gnvs.h>
 #include <arch/io.h>
 #include <console/console.h>
 #include <cpu/x86/smm.h>

-static void set_smm_gnvs_ptr(void);
-
 int apm_control(u8 cmd)
 {
    if (!CONFIG(HAVE_SMI_HANDLER))
@@ -23,9 +20,6 @@
    case APM_CNT_ACPI_ENABLE:
    	printk(BIOS_DEBUG, "Enabling ACPI via APMC.\n");
    	break;
-	case APM_CNT_GNVS_UPDATE:
-		set_smm_gnvs_ptr();
-		return 0;
    case APM_CNT_FINALIZE:
    	printk(BIOS_DEBUG, "Finalizing SMM.\n");
    	break;
@@ -45,32 +39,3 @@
    printk(BIOS_DEBUG, "APMC done.\n");
    return 0;
 }
-
-static void set_smm_gnvs_ptr(void)
-{
-	uintptr_t gnvs_address;
-
-	if (CONFIG(ACPI_NO_SMI_GNVS)) {
-		printk(BIOS_WARNING, "%s() is not implemented\n", __func__);
-		return;
-	}
-
-	gnvs_address = (uintptr_t)acpi_get_gnvs();
-	if (!gnvs_address)
-		return;
-
-	/*
-	 * Issue SMI to set the gnvs pointer in SMM.
-	 *
-	 * EAX = APM_CNT_GNVS_UPDATE
-	 * EBX = gnvs pointer
-	 * EDX = APM_CNT
-	 */
-	asm volatile (
-		"outb %%al, %%dx\n\t"
-		: /* ignore result */
-		: "a" (APM_CNT_GNVS_UPDATE),
-		  "b" (gnvs_address),
-		  "d" (APM_CNT)
-	);
-}
diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h
index 6dfe08d..9106842 100644
--- a/src/include/cpu/x86/smm.h
+++ b/src/include/cpu/x86/smm.h
@@ -22,11 +22,10 @@
 #define APM_CNT_PST_CONTROL	0x80
 #define APM_CNT_ACPI_DISABLE	0x1e
 #define APM_CNT_ACPI_ENABLE	0xe1
-#define APM_CNT_MBI_UPDATE	0xeb
-#define APM_CNT_GNVS_UPDATE	0xea
 #define APM_CNT_ROUTE_ALL_XHCI	0xca
 #define APM_CNT_FINALIZE	0xcb
 #define APM_CNT_LEGACY		0xcc
+#define APM_CNT_MBI_UPDATE	0xeb
 #define APM_CNT_SMMINFO		0xec
 #define APM_CNT_SMMSTORE	0xed
 #define APM_CNT_ELOG_GSMI	0xef