Change in coreboot[master]: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations - coreboot-gerrit

18 Jun 2020

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42491 )
Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................
ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
Allocation now happens prior to device enumeration. The
steps cbmem_add() is a no-op here, if reached for some
boards. The memset() here is also redundant and becomes
harmful with followup works.
Change-Id: I9b2625af15cae90b9c1eb601e606d0430336609f
Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com
---
M src/mainboard/protectli/vault_bsw/acpi_tables.c
M src/soc/amd/common/block/lpc/lpc.c
M src/soc/amd/picasso/acpi.c
M src/soc/amd/stoneyridge/acpi.c
M src/soc/intel/apollolake/acpi.c
M src/soc/intel/apollolake/chip.c
M src/soc/intel/baytrail/ramstage.c
M src/soc/intel/baytrail/southcluster.c
M src/soc/intel/braswell/acpi.c
M src/soc/intel/braswell/ramstage.c
M src/soc/intel/broadwell/lpc.c
M src/soc/intel/broadwell/ramstage.c
M src/soc/intel/common/block/acpi/acpi.c
M src/soc/intel/denverton_ns/acpi.c
M src/soc/intel/skylake/acpi.c
M src/soc/intel/xeon_sp/cpx/acpi.c
M src/soc/intel/xeon_sp/skx/acpi.c
M src/southbridge/intel/bd82x6x/lpc.c
M src/southbridge/intel/i82801gx/lpc.c
M src/southbridge/intel/i82801ix/lpc.c
M src/southbridge/intel/i82801jx/lpc.c
M src/southbridge/intel/ibexpeak/lpc.c
M src/southbridge/intel/lynxpoint/lpc.c
23 files changed, 17 insertions(+), 121 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/42491/1

diff --git a/src/mainboard/protectli/vault_bsw/acpi_tables.c b/src/mainboard/protectli/vault_bsw/acpi_tables.c
index 1724c71..fe66833 100644
--- a/src/mainboard/protectli/vault_bsw/acpi_tables.c
+++ b/src/mainboard/protectli/vault_bsw/acpi_tables.c
@@ -6,7 +6,6 @@
void acpi_create_gnvs(global_nvs_t *gnvs)
 {
-	memset(gnvs, 0, sizeof(*gnvs));
acpi_init_gnvs(gnvs);
diff --git a/src/soc/amd/common/block/lpc/lpc.c b/src/soc/amd/common/block/lpc/lpc.c
index 3ddedce..fcce820 100644
--- a/src/soc/amd/common/block/lpc/lpc.c
+++ b/src/soc/amd/common/block/lpc/lpc.c
@@ -91,7 +91,6 @@
 static void lpc_read_resources(struct device *dev)
 {
    struct resource *res;
-	global_nvs_t *gnvs;
/* Get the normal pci resources of this device */
    pci_dev_read_resources(dev);
@@ -125,10 +124,6 @@
    res->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED;
compact_resources(dev);
-
-	/* Allocate ACPI NVS in CBMEM */
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(global_nvs_t));
-	printk(BIOS_DEBUG, "ACPI GNVS at %p\n", gnvs);
 }
static void lpc_set_resources(struct device *dev)
diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c
index 7704c30..981c8b8 100644
--- a/src/soc/amd/picasso/acpi.c
+++ b/src/soc/amd/picasso/acpi.c
@@ -246,8 +246,6 @@
static void acpi_create_gnvs(struct global_nvs_t *gnvs)
 {
-	/* Clear out GNVS. */
-	memset(gnvs, 0, sizeof(*gnvs));
if (CONFIG(CONSOLE_CBMEM))
    	gnvs->cbmc = (uintptr_t)cbmem_find(CBMEM_ID_CONSOLE);
diff --git a/src/soc/amd/stoneyridge/acpi.c b/src/soc/amd/stoneyridge/acpi.c
index ea67aa3..5559291 100644
--- a/src/soc/amd/stoneyridge/acpi.c
+++ b/src/soc/amd/stoneyridge/acpi.c
@@ -221,8 +221,6 @@
static void acpi_create_gnvs(struct global_nvs_t *gnvs)
 {
-	/* Clear out GNVS. */
-	memset(gnvs, 0, sizeof(*gnvs));
if (CONFIG(CONSOLE_CBMEM))
    	gnvs->cbmc = (uintptr_t)cbmem_find(CBMEM_ID_CONSOLE);
diff --git a/src/soc/intel/apollolake/acpi.c b/src/soc/intel/apollolake/acpi.c
index 595e6a2..be6eb9a 100644
--- a/src/soc/intel/apollolake/acpi.c
+++ b/src/soc/intel/apollolake/acpi.c
@@ -75,8 +75,6 @@
    struct soc_intel_apollolake_config *cfg;
    cfg = config_of_soc();
-	/* Clear out GNVS. */
-	memset(gnvs, 0, sizeof(*gnvs));
if (CONFIG(CONSOLE_CBMEM))
    	gnvs->cbmc = (uintptr_t) cbmem_find(CBMEM_ID_CONSOLE);
diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c
index f9af4f4..7869442 100644
--- a/src/soc/intel/apollolake/chip.c
+++ b/src/soc/intel/apollolake/chip.c
@@ -319,9 +319,6 @@
     */
    p2sb_unhide();
-	/* Allocate ACPI NVS in CBMEM */
-	cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs_t));
-
    if (CONFIG(APL_SKIP_SET_POWER_LIMITS)) {
    	printk(BIOS_INFO, "Skip setting RAPL per configuration\n");
    } else {
diff --git a/src/soc/intel/baytrail/ramstage.c b/src/soc/intel/baytrail/ramstage.c
index d27a17d..b550ee4 100644
--- a/src/soc/intel/baytrail/ramstage.c
+++ b/src/soc/intel/baytrail/ramstage.c
@@ -2,6 +2,7 @@
#include <arch/cpu.h>
 #include <acpi/acpi.h>
+#include <acpi/acpi_gnvs.h>
 #include <cbmem.h>
 #include <console/console.h>
 #include <cpu/intel/microcode.h>
@@ -144,15 +145,9 @@
static void s3_resume_prepare(void)
 {
-	global_nvs_t *gnvs;
+	global_nvs_t *gnvs = acpi_get_gnvs();
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(global_nvs_t));
-	if (gnvs == NULL)
-		return;
-
-	if (!acpi_is_wakeup_s3())
-		memset(gnvs, 0, sizeof(global_nvs_t));
-	else
+	if (gnvs && acpi_is_wakeup_s3())
    	s3_save_acpi_wake_source(gnvs);
 }
diff --git a/src/soc/intel/baytrail/southcluster.c b/src/soc/intel/baytrail/southcluster.c
index 08bd60c..a2d2488 100644
--- a/src/soc/intel/baytrail/southcluster.c
+++ b/src/soc/intel/baytrail/southcluster.c
@@ -485,11 +485,6 @@
    global_nvs_t *gnvs;
gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof (*gnvs));
-		if (gnvs)
-			memset(gnvs, 0, sizeof(*gnvs));
-	}
if (gnvs) {
    	acpi_create_gnvs(gnvs);
diff --git a/src/soc/intel/braswell/acpi.c b/src/soc/intel/braswell/acpi.c
index 9784691..6b29d9a 100644
--- a/src/soc/intel/braswell/acpi.c
+++ b/src/soc/intel/braswell/acpi.c
@@ -496,11 +496,6 @@
    global_nvs_t *gnvs;
gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
-		if (gnvs)
-			memset(gnvs, 0, sizeof(*gnvs));
-	}
if (gnvs) {
    	acpi_create_gnvs(gnvs);
diff --git a/src/soc/intel/braswell/ramstage.c b/src/soc/intel/braswell/ramstage.c
index 893a6b6..59d8b28 100644
--- a/src/soc/intel/braswell/ramstage.c
+++ b/src/soc/intel/braswell/ramstage.c
@@ -2,6 +2,7 @@
#include <arch/cpu.h>
 #include <acpi/acpi.h>
+#include <acpi/acpi_gnvs.h>
 #include <cbmem.h>
 #include <console/console.h>
 #include <cpu/intel/microcode.h>
@@ -135,24 +136,11 @@
    return 1;
 }
-static void s3_resume_prepare(void)
-{
-	global_nvs_t *gnvs;
-
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(global_nvs_t));
-	if (!acpi_is_wakeup_s3() && gnvs)
-		memset(gnvs, 0, sizeof(global_nvs_t));
-}
-
 static void set_board_id(void)
 {
-	global_nvs_t *gnvs;
-
-	gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		printk(BIOS_ERR, "Unable to locate Global NVS\n");
+	global_nvs_t *gnvs = acpi_get_gnvs();
+	if (!gnvs)
    	return;
-	}
    gnvs->bdid = board_id();
 }
@@ -165,9 +153,6 @@
    /* Allow for SSE instructions to be executed. */
    write_cr4(read_cr4() | CR4_OSFXSR | CR4_OSXMMEXCPT);
-	/* Indicate S3 resume to rest of ramstage. */
-	s3_resume_prepare();
-
    /* Perform silicon specific init. */
    intel_silicon_init();
    set_max_freq();
diff --git a/src/soc/intel/broadwell/lpc.c b/src/soc/intel/broadwell/lpc.c
index f01b7fa..aabc23d 100644
--- a/src/soc/intel/broadwell/lpc.c
+++ b/src/soc/intel/broadwell/lpc.c
@@ -553,8 +553,6 @@
static void pch_lpc_read_resources(struct device *dev)
 {
-	global_nvs_t *gnvs;
-
    /* Get the normal PCI resources of this device. */
    pci_dev_read_resources(dev);
@@ -563,11 +561,6 @@
/* Add IO resources. */
    pch_lpc_add_io_resources(dev);
-
-	/* Allocate ACPI NVS in CBMEM */
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(global_nvs_t));
-	if (!acpi_is_wakeup_s3() && gnvs)
-		memset(gnvs, 0, sizeof(global_nvs_t));
 }
static void southcluster_inject_dsdt(const struct device *device)
@@ -575,11 +568,6 @@
    global_nvs_t *gnvs;
gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
-		if (gnvs)
-			memset(gnvs, 0, sizeof(*gnvs));
-	}
if (gnvs) {
    	acpi_create_gnvs(gnvs);
diff --git a/src/soc/intel/broadwell/ramstage.c b/src/soc/intel/broadwell/ramstage.c
index 39550b7..a4dd886 100644
--- a/src/soc/intel/broadwell/ramstage.c
+++ b/src/soc/intel/broadwell/ramstage.c
@@ -30,20 +30,7 @@
    return GPE0_REG_MAX;
 }
-static void s3_resume_prepare(void)
-{
-	global_nvs_t *gnvs;
-
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(global_nvs_t));
-	if (gnvs == NULL)
-		return;
-
-	if (!acpi_is_wakeup_s3())
-		memset(gnvs, 0, sizeof(global_nvs_t));
-}
-
 void broadwell_init_pre_device(void *chip_info)
 {
-	s3_resume_prepare();
    broadwell_run_reference_code();
 }
diff --git a/src/soc/intel/common/block/acpi/acpi.c b/src/soc/intel/common/block/acpi/acpi.c
index 88d8182..c9a56ce 100644
--- a/src/soc/intel/common/block/acpi/acpi.c
+++ b/src/soc/intel/common/block/acpi/acpi.c
@@ -1,12 +1,12 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
+#include <acpi/acpi_gnvs.h>
 #include <acpi/acpigen.h>
 #include <arch/ioapic.h>
 #include <arch/smp/mpspec.h>
 #include <bootstate.h>
 #include <cbmem.h>
 #include <cf9_reset.h>
-#include <acpi/acpi_gnvs.h>
 #include <console/console.h>
 #include <cpu/intel/turbo.h>
 #include <cpu/x86/msr.h>
@@ -231,11 +231,6 @@
    struct global_nvs_t *gnvs;
gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
-		if (gnvs)
-			memset(gnvs, 0, sizeof(*gnvs));
-	}
if (gnvs) {
    	acpi_create_gnvs(gnvs);
diff --git a/src/soc/intel/denverton_ns/acpi.c b/src/soc/intel/denverton_ns/acpi.c
index 0d3f1a6..38913de 100644
--- a/src/soc/intel/denverton_ns/acpi.c
+++ b/src/soc/intel/denverton_ns/acpi.c
@@ -283,11 +283,6 @@
    global_nvs_t *gnvs;
gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
-		if (gnvs)
-			memset(gnvs, 0, sizeof(*gnvs));
-	}
if (gnvs) {
    	acpi_create_gnvs(gnvs);
diff --git a/src/soc/intel/skylake/acpi.c b/src/soc/intel/skylake/acpi.c
index 801ee3f..4a6533d 100644
--- a/src/soc/intel/skylake/acpi.c
+++ b/src/soc/intel/skylake/acpi.c
@@ -646,11 +646,6 @@
    global_nvs_t *gnvs;
gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
-		if (gnvs)
-			memset(gnvs, 0, sizeof(*gnvs));
-	}
if (gnvs) {
    	acpi_create_gnvs(gnvs);
diff --git a/src/soc/intel/xeon_sp/cpx/acpi.c b/src/soc/intel/xeon_sp/cpx/acpi.c
index 3ff946f..ba5128e 100644
--- a/src/soc/intel/xeon_sp/cpx/acpi.c
+++ b/src/soc/intel/xeon_sp/cpx/acpi.c
@@ -111,11 +111,6 @@
    global_nvs_t *gnvs;
gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, 0x2000);
-		if (gnvs)
-			memset(gnvs, 0, sizeof(*gnvs));
-	}
if (gnvs) {
    	acpi_create_gnvs(gnvs);
diff --git a/src/soc/intel/xeon_sp/skx/acpi.c b/src/soc/intel/xeon_sp/skx/acpi.c
index 3ba61ef..9880c44 100644
--- a/src/soc/intel/xeon_sp/skx/acpi.c
+++ b/src/soc/intel/xeon_sp/skx/acpi.c
@@ -959,11 +959,6 @@
    global_nvs_t *gnvs;
gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
-		if (gnvs)
-			memset(gnvs, 0, sizeof(*gnvs));
-	}
if (gnvs) {
    	acpi_create_gnvs(gnvs);
diff --git a/src/southbridge/intel/bd82x6x/lpc.c b/src/southbridge/intel/bd82x6x/lpc.c
index f4aea9f..5195825 100644
--- a/src/southbridge/intel/bd82x6x/lpc.c
+++ b/src/southbridge/intel/bd82x6x/lpc.c
@@ -15,6 +15,7 @@
 #include <acpi/acpigen.h>
 #include <cpu/x86/smm.h>
 #include <cbmem.h>
+#include <acpi/acpi_gnvs.h>
 #include <string.h>
 #include "chip.h"
 #include "pch.h"
@@ -649,10 +650,9 @@
static void southbridge_inject_dsdt(const struct device *dev)
 {
-	global_nvs_t *gnvs = cbmem_add (CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	global_nvs_t *gnvs = acpi_get_gnvs();
if (gnvs) {
-		memset(gnvs, 0, sizeof(*gnvs));
acpi_create_gnvs(gnvs);
diff --git a/src/southbridge/intel/i82801gx/lpc.c b/src/southbridge/intel/i82801gx/lpc.c
index 573c4f9..2cb66da 100644
--- a/src/southbridge/intel/i82801gx/lpc.c
+++ b/src/southbridge/intel/i82801gx/lpc.c
@@ -13,6 +13,7 @@
 #include <arch/ioapic.h>
 #include <acpi/acpi.h>
 #include <cpu/x86/smm.h>
+#include <acpi/acpi_gnvs.h>
 #include <acpi/acpigen.h>
 #include <arch/smp/mpspec.h>
 #include <cbmem.h>
@@ -613,10 +614,9 @@
static void southbridge_inject_dsdt(const struct device *dev)
 {
-	global_nvs_t *gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	global_nvs_t *gnvs = acpi_get_gnvs();
if (gnvs) {
-		memset(gnvs, 0, sizeof(*gnvs));
gnvs->apic = 1;
    	gnvs->mpen = 1; /* Enable Multi Processing */
diff --git a/src/southbridge/intel/i82801ix/lpc.c b/src/southbridge/intel/i82801ix/lpc.c
index 62f7f13..62c47c9 100644
--- a/src/southbridge/intel/i82801ix/lpc.c
+++ b/src/southbridge/intel/i82801ix/lpc.c
@@ -15,6 +15,7 @@
 #include <cpu/x86/smm.h>
 #include <acpi/acpigen.h>
 #include <cbmem.h>
+#include <acpi/acpi_gnvs.h>
 #include <string.h>
 #include "chip.h"
 #include "i82801ix.h"
@@ -457,10 +458,9 @@
static void southbridge_inject_dsdt(const struct device *dev)
 {
-	global_nvs_t *gnvs = cbmem_add (CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	global_nvs_t *gnvs = acpi_get_gnvs();
if (gnvs) {
-		memset(gnvs, 0, sizeof(*gnvs));
    	acpi_create_gnvs(gnvs);
diff --git a/src/southbridge/intel/i82801jx/lpc.c b/src/southbridge/intel/i82801jx/lpc.c
index 23beec5..5f31fca 100644
--- a/src/southbridge/intel/i82801jx/lpc.c
+++ b/src/southbridge/intel/i82801jx/lpc.c
@@ -16,6 +16,7 @@
 #include <acpi/acpigen.h>
 #include <arch/smp/mpspec.h>
 #include <cbmem.h>
+#include <acpi/acpi_gnvs.h>
 #include <string.h>
 #include "chip.h"
 #include "i82801jx.h"
@@ -616,10 +617,9 @@
static void southbridge_inject_dsdt(const struct device *dev)
 {
-	global_nvs_t *gnvs = cbmem_add (CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	global_nvs_t *gnvs = acpi_get_gnvs();
if (gnvs) {
-		memset(gnvs, 0, sizeof(*gnvs));
    	acpi_create_gnvs(gnvs);
diff --git a/src/southbridge/intel/ibexpeak/lpc.c b/src/southbridge/intel/ibexpeak/lpc.c
index 1e7eabc..ae075ce 100644
--- a/src/southbridge/intel/ibexpeak/lpc.c
+++ b/src/southbridge/intel/ibexpeak/lpc.c
@@ -16,6 +16,7 @@
 #include <elog.h>
 #include <acpi/acpigen.h>
 #include <cbmem.h>
+#include <acpi/acpi_gnvs.h>
 #include <string.h>
 #include <cpu/x86/smm.h>
 #include "chip.h"
@@ -554,10 +555,9 @@
static void southbridge_inject_dsdt(const struct device *dev)
 {
-	global_nvs_t *gnvs = cbmem_add (CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	global_nvs_t *gnvs = acpi_get_gnvs();
if (gnvs) {
-		memset(gnvs, 0, sizeof(*gnvs));
acpi_create_gnvs(gnvs);
diff --git a/src/southbridge/intel/lynxpoint/lpc.c b/src/southbridge/intel/lynxpoint/lpc.c
index e5a32fc..342ab5b 100644
--- a/src/southbridge/intel/lynxpoint/lpc.c
+++ b/src/southbridge/intel/lynxpoint/lpc.c
@@ -680,10 +680,6 @@
    /* Add IO resources. */
    pch_lpc_add_io_resources(dev);
-	/* Allocate ACPI NVS in CBMEM */
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(global_nvs_t));
-	if (!acpi_is_wakeup_s3() && gnvs)
-		memset(gnvs, 0, sizeof(global_nvs_t));
 }
static void pch_lpc_enable(struct device *dev)
@@ -700,11 +696,6 @@
    global_nvs_t *gnvs;
gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
-		if (gnvs)
-			memset(gnvs, 0, sizeof(*gnvs));
-	}
if (gnvs) {
    	acpi_create_gnvs(gnvs);
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42491
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9b2625af15cae90b9c1eb601e606d0430336609f
Gerrit-Change-Number: 42491
Gerrit-PatchSet: 1
Gerrit-Owner: Kyösti Mälkki kyosti.malkki@gmail.com
Gerrit-Reviewer: Andrey Petrov andrey.petrov@gmail.com
Gerrit-Reviewer: David Guckian david.guckian@intel.com
Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com
Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org
Gerrit-Reviewer: Piotr Król piotr.krol@3mdeb.com
Gerrit-Reviewer: Vanessa Eusebio vanessa.f.eusebio@intel.com
Gerrit-MessageType: newchange



    