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

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

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

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
step cbmem_add() is a no-op here, if reached for some
boards. The memset() here is also redundant and becomes
harmful with followup works, as it would wipe out the
CBMEM console and ChromeOS related fields without them
being set again.

Change-Id: I9b2625af15cae90b9c1eb601e606d0430336609f
Signed-off-by: Kyösti Mälkki 
---
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/pch/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/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
22 files changed, 14 insertions(+), 114 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48701/1

diff --git a/src/mainboard/protectli/vault_bsw/acpi_tables.c b/src/mainboard/protectli/vault_bsw/acpi_tables.c
index c40769a..5267dcd 100644
--- a/src/mainboard/protectli/vault_bsw/acpi_tables.c
+++ b/src/mainboard/protectli/vault_bsw/acpi_tables.c
@@ -7,7 +7,6 @@

void acpi_create_gnvs(struct global_nvs *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 107aa4af..96fb176 100644
--- a/src/soc/amd/common/block/lpc/lpc.c
+++ b/src/soc/amd/common/block/lpc/lpc.c
@@ -100,7 +100,6 @@
 static void lpc_read_resources(struct device *dev)
 {
    struct resource *res;
-	struct global_nvs *gnvs;

/* Get the normal pci resources of this device */
    pci_dev_read_resources(dev);
@@ -134,10 +133,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(struct global_nvs));
-	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 17940e9..46a3cb0 100644
--- a/src/soc/amd/picasso/acpi.c
+++ b/src/soc/amd/picasso/acpi.c
@@ -395,8 +395,6 @@

void acpi_create_gnvs(struct global_nvs *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 e62f235..74a3b2f 100644
--- a/src/soc/amd/stoneyridge/acpi.c
+++ b/src/soc/amd/stoneyridge/acpi.c
@@ -164,8 +164,6 @@

void acpi_create_gnvs(struct global_nvs *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 d0e7a73..61d42a5 100644
--- a/src/soc/intel/apollolake/acpi.c
+++ b/src/soc/intel/apollolake/acpi.c
@@ -77,8 +77,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 a4ed8bd..3d5c1ab 100644
--- a/src/soc/intel/apollolake/chip.c
+++ b/src/soc/intel/apollolake/chip.c
@@ -315,9 +315,6 @@
     */
    p2sb_unhide();

-	/* Allocate ACPI NVS in CBMEM */
-	cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-
    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 b15970e..e199e9f 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>
@@ -143,15 +144,9 @@

static void s3_resume_prepare(void)
 {
-	struct global_nvs *gnvs;
+	struct global_nvs *gnvs = acpi_get_gnvs();

-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-	if (gnvs == NULL)
-		return;
-
-	if (!acpi_is_wakeup_s3())
-		memset(gnvs, 0, sizeof(struct global_nvs));
-	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 2df6410..d8305c6 100644
--- a/src/soc/intel/baytrail/southcluster.c
+++ b/src/soc/intel/baytrail/southcluster.c
@@ -489,11 +489,6 @@
    struct global_nvs *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 9577536..9968520 100644
--- a/src/soc/intel/braswell/acpi.c
+++ b/src/soc/intel/braswell/acpi.c
@@ -381,11 +381,6 @@
    struct global_nvs *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 eba1527..68bddfb 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)
-{
-	struct global_nvs *gnvs;
-
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-	if (!acpi_is_wakeup_s3() && gnvs)
-		memset(gnvs, 0, sizeof(struct global_nvs));
-}
-
 static void set_board_id(void)
 {
-	struct global_nvs *gnvs;
-
-	gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		printk(BIOS_ERR, "Unable to locate Global NVS\n");
+	struct global_nvs *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/pch/lpc.c b/src/soc/intel/broadwell/pch/lpc.c
index 5a29b02..c735a81 100644
--- a/src/soc/intel/broadwell/pch/lpc.c
+++ b/src/soc/intel/broadwell/pch/lpc.c
@@ -593,8 +593,6 @@

static void pch_lpc_read_resources(struct device *dev)
 {
-	struct global_nvs *gnvs;
-
    /* Get the normal PCI resources of this device. */
    pci_dev_read_resources(dev);

@@ -603,11 +601,6 @@

/* Add IO resources. */
    pch_lpc_add_io_resources(dev);
-
-	/* Allocate ACPI NVS in CBMEM */
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-	if (!acpi_is_wakeup_s3() && gnvs)
-		memset(gnvs, 0, sizeof(struct global_nvs));
 }

static void southcluster_inject_dsdt(const struct device *device)
@@ -615,11 +608,6 @@
    struct global_nvs *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 57abf95..214830d 100644
--- a/src/soc/intel/broadwell/ramstage.c
+++ b/src/soc/intel/broadwell/ramstage.c
@@ -63,15 +63,9 @@

static void s3_resume_prepare(void)
 {
-	struct global_nvs *gnvs;
+	struct global_nvs *gnvs = acpi_get_gnvs();

-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-	if (gnvs == NULL)
-		return;
-
-	if (!acpi_is_wakeup_s3())
-		memset(gnvs, 0, sizeof(struct global_nvs));
-	else
+	if (gnvs && acpi_is_wakeup_s3())
    	save_acpi_wake_source(gnvs);
 }

diff --git a/src/soc/intel/common/block/acpi/acpi.c b/src/soc/intel/common/block/acpi/acpi.c
index 69f7a59..52143a2 100644
--- a/src/soc/intel/common/block/acpi/acpi.c
+++ b/src/soc/intel/common/block/acpi/acpi.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */

+#include <acpi/acpi_gnvs.h>
 #include <acpi/acpigen.h>
 #include <arch/cpu.h>
 #include <arch/ioapic.h>
@@ -7,7 +8,6 @@
 #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/intel/common/common.h>
@@ -224,11 +224,6 @@
    struct global_nvs *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 2a847ed..f46f1fc 100644
--- a/src/soc/intel/denverton_ns/acpi.c
+++ b/src/soc/intel/denverton_ns/acpi.c
@@ -250,11 +250,6 @@
    struct global_nvs *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 d654a58..50175aa 100644
--- a/src/soc/intel/skylake/acpi.c
+++ b/src/soc/intel/skylake/acpi.c
@@ -544,11 +544,6 @@
    struct global_nvs *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/acpi.c b/src/soc/intel/xeon_sp/acpi.c
index 184eb6b..14d37f7 100644
--- a/src/soc/intel/xeon_sp/acpi.c
+++ b/src/soc/intel/xeon_sp/acpi.c
@@ -208,11 +208,6 @@
    struct global_nvs *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 4515261..edde09e 100644
--- a/src/southbridge/intel/bd82x6x/lpc.c
+++ b/src/southbridge/intel/bd82x6x/lpc.c
@@ -644,10 +644,9 @@

void southbridge_inject_dsdt(const struct device *dev)
 {
-	struct global_nvs *gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	struct global_nvs *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 4db9351..f6fc384 100644
--- a/src/southbridge/intel/i82801gx/lpc.c
+++ b/src/southbridge/intel/i82801gx/lpc.c
@@ -481,10 +481,9 @@

void southbridge_inject_dsdt(const struct device *dev)
 {
-	struct global_nvs *gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	struct global_nvs *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 821a0b7..f587d01 100644
--- a/src/southbridge/intel/i82801ix/lpc.c
+++ b/src/southbridge/intel/i82801ix/lpc.c
@@ -455,10 +455,9 @@

void southbridge_inject_dsdt(const struct device *dev)
 {
-	struct global_nvs *gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	struct global_nvs *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 ad9bac1..75cc7f4 100644
--- a/src/southbridge/intel/i82801jx/lpc.c
+++ b/src/southbridge/intel/i82801jx/lpc.c
@@ -479,10 +479,9 @@

void southbridge_inject_dsdt(const struct device *dev)
 {
-	struct global_nvs *gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	struct global_nvs *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 0895ddd..5f31c43 100644
--- a/src/southbridge/intel/ibexpeak/lpc.c
+++ b/src/southbridge/intel/ibexpeak/lpc.c
@@ -544,10 +544,9 @@

void southbridge_inject_dsdt(const struct device *dev)
 {
-	struct global_nvs *gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	struct global_nvs *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 586e626..26d7c48 100644
--- a/src/southbridge/intel/lynxpoint/lpc.c
+++ b/src/southbridge/intel/lynxpoint/lpc.c
@@ -661,8 +661,6 @@

static void pch_lpc_read_resources(struct device *dev)
 {
-	struct global_nvs *gnvs;
-
    /* Get the normal PCI resources of this device. */
    pci_dev_read_resources(dev);

@@ -671,11 +669,6 @@

/* Add IO resources. */
    pch_lpc_add_io_resources(dev);
-
-	/* Allocate ACPI NVS in CBMEM */
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-	if (!acpi_is_wakeup_s3() && gnvs)
-		memset(gnvs, 0, sizeof(struct global_nvs));
 }

static void pch_lpc_enable(struct device *dev)
@@ -692,11 +685,6 @@
    struct global_nvs *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);

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

Hello build bot (Jenkins), Jason Glenesk, Michał Żygowski, Mariusz Szafrański, Marshall Dawson, Suresh Bellampalli, Vanessa Eusebio, Michal Motyl, Andrey Petrov, Patrick Rudolph, Piotr Król, Felix Held,

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

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

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

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
step cbmem_add() is a no-op here, if reached for some
boards. The memset() here is also redundant and becomes
harmful with followup works, as it would wipe out the
CBMEM console and ChromeOS related fields without them
being set again.

Change-Id: I9b2625af15cae90b9c1eb601e606d0430336609f
Signed-off-by: Kyösti Mälkki 
---
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/pch/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/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
22 files changed, 15 insertions(+), 114 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48701/2

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

Hello build bot (Jenkins), Jason Glenesk, Michał Żygowski, Mariusz Szafrański, Marshall Dawson, Suresh Bellampalli, Vanessa Eusebio, Michal Motyl, Andrey Petrov, Patrick Rudolph, Piotr Król, Felix Held,

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

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

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

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
step cbmem_add() is a no-op here, if reached for some
boards. The memset() here is also redundant and becomes
harmful with followup works, as it would wipe out the
CBMEM console and ChromeOS related fields without them
being set again.

Change-Id: I9b2625af15cae90b9c1eb601e606d0430336609f
Signed-off-by: Kyösti Mälkki 
---
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/acpi.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/pch/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/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, 89 insertions(+), 113 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48701/3

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

Hello build bot (Jenkins), Jason Glenesk, Michał Żygowski, Mariusz Szafrański, Marshall Dawson, Suresh Bellampalli, Vanessa Eusebio, Michal Motyl, Andrey Petrov, Patrick Rudolph, Piotr Król, Felix Held,

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

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

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

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
step cbmem_add() is a no-op here, if reached for some
boards. The memset() here is also redundant and becomes
harmful with followup works, as it would wipe out the
CBMEM console and ChromeOS related fields without them
being set again.

Change-Id: I9b2625af15cae90b9c1eb601e606d0430336609f
Signed-off-by: Kyösti Mälkki 
---
M src/acpi/Kconfig
M src/acpi/Makefile.inc
M src/acpi/gnvs.c
A src/acpi/nvs.c
M src/drivers/amd/agesa/Makefile.inc
A src/drivers/amd/agesa/nvs.c
M src/mainboard/protectli/vault_bsw/acpi_tables.c
M src/soc/amd/cezanne/Kconfig
M src/soc/amd/common/block/lpc/lpc.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/acpi.c
M src/soc/amd/stoneyridge/Kconfig
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/pch/lpc.c
M src/soc/intel/broadwell/ramstage.c
M src/soc/intel/common/Kconfig.common
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/acpi.c
M src/southbridge/intel/bd82x6x/lpc.c
M src/southbridge/intel/i82371eb/acpi_tables.c
M src/southbridge/intel/i82801dx/fadt.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
34 files changed, 99 insertions(+), 120 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48701/4

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

Hello build bot (Jenkins), Patrick Georgi, Mariusz Szafrański, Andrey Petrov, Patrick Rudolph, Piotr Król, Jason Glenesk, Martin Roth, Michał Żygowski, Marshall Dawson, Suresh Bellampalli, Vanessa Eusebio, Michal Motyl, Felix Held,

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

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

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

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
step cbmem_add() is a no-op here, if reached for some
boards. The memset() here is also redundant and becomes
harmful with followup works, as it would wipe out the
CBMEM console and ChromeOS related fields without them
being set again.

Change-Id: I9b2625af15cae90b9c1eb601e606d0430336609f
Signed-off-by: Kyösti Mälkki 
---
M src/acpi/Kconfig
M src/acpi/Makefile.inc
M src/acpi/gnvs.c
A src/acpi/nvs.c
M src/drivers/amd/agesa/Makefile.inc
A src/drivers/amd/agesa/nvs.c
M src/mainboard/protectli/vault_bsw/acpi_tables.c
M src/soc/amd/cezanne/Kconfig
M src/soc/amd/common/block/lpc/lpc.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/acpi.c
M src/soc/amd/stoneyridge/Kconfig
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/pch/lpc.c
M src/soc/intel/broadwell/ramstage.c
M src/soc/intel/common/Kconfig.common
M src/soc/intel/common/block/acpi/acpi.c
M src/soc/intel/denverton_ns/acpi.c
A src/soc/intel/quark/include/soc/nvs.h
M src/soc/intel/skylake/acpi.c
M src/soc/intel/xeon_sp/acpi.c
M src/southbridge/intel/bd82x6x/lpc.c
M src/southbridge/intel/i82371eb/acpi_tables.c
M src/southbridge/intel/i82801dx/fadt.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
35 files changed, 111 insertions(+), 120 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48701/5

[Angel Pons (Code Review)]

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

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 5: Code-Review+2

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48701 )

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48701/5/src/soc/amd/cezanne/Kconfig 
File src/soc/amd/cezanne/Kconfig:

https://review.coreboot.org/c/coreboot/+/48701/5/src/soc/amd/cezanne/Kconfig... 
PS5, Line 25: 	select ACPI_SOC_NVS
probably not use yet and also not in alphabetical order in the list. the former shouldn't hurt, but i dislike the latter one, since getting those lists back in alphabetical order has the potential to cause unnecessary manual rebases of unrelated patches

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

Hello build bot (Jenkins), Patrick Georgi, Mariusz Szafrański, Angel Pons, Andrey Petrov, Patrick Rudolph, Piotr Król, Jason Glenesk, Martin Roth, Michał Żygowski, Marshall Dawson, Suresh Bellampalli, Vanessa Eusebio, Michal Motyl, Felix Held,

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

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

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

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
step cbmem_add() is a no-op here, if reached for some
boards. The memset() here is also redundant and becomes
harmful with followup works, as it would wipe out the
CBMEM console and ChromeOS related fields without them
being set again.

Change-Id: I9b2625af15cae90b9c1eb601e606d0430336609f
Signed-off-by: Kyösti Mälkki 
---
M src/acpi/Kconfig
M src/acpi/Makefile.inc
M src/acpi/gnvs.c
A src/acpi/nvs.c
M src/drivers/amd/agesa/Makefile.inc
A src/drivers/amd/agesa/nvs.c
M src/mainboard/protectli/vault_bsw/acpi_tables.c
M src/soc/amd/cezanne/Kconfig
M src/soc/amd/common/block/lpc/lpc.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/acpi.c
M src/soc/amd/stoneyridge/Kconfig
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/pch/lpc.c
M src/soc/intel/broadwell/ramstage.c
M src/soc/intel/common/Kconfig.common
M src/soc/intel/common/block/acpi/acpi.c
M src/soc/intel/denverton_ns/acpi.c
A src/soc/intel/quark/include/soc/nvs.h
M src/soc/intel/skylake/acpi.c
M src/southbridge/intel/bd82x6x/lpc.c
M src/southbridge/intel/i82371eb/acpi_tables.c
M src/southbridge/intel/i82801dx/fadt.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
34 files changed, 111 insertions(+), 116 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48701/6

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48701 )

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48701/5/src/soc/amd/cezanne/Kconfig 
File src/soc/amd/cezanne/Kconfig:

https://review.coreboot.org/c/coreboot/+/48701/5/src/soc/amd/cezanne/Kconfig... 
PS5, Line 25: 	select ACPI_SOC_NVS
> probably not use yet and also not in alphabetical order in the list. […]

Done

[Angel Pons (Code Review)]

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

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 8: Code-Review+1

(2 comments)

https://review.coreboot.org/c/coreboot/+/48701/8/src/soc/intel/quark/include... 
File src/soc/intel/quark/include/soc/nvs.h:

PS8: 
Is it easy to keep Quark without GNVS?

https://review.coreboot.org/c/coreboot/+/48701/8/src/southbridge/intel/i8280... 
File src/southbridge/intel/i82801dx/fadt.c:

https://review.coreboot.org/c/coreboot/+/48701/8/src/southbridge/intel/i8280... 
PS8, Line 84: size_t gnvs_size_of_array(void)
I'm not sure if this is the best place to put this in, but it's not a big deal.

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48701 )

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 8: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/48701/8/src/acpi/Kconfig 
File src/acpi/Kconfig:

https://review.coreboot.org/c/coreboot/+/48701/8/src/acpi/Kconfig@27 
PS8, Line 27: ACPI_SOC_NVS
So this is for platforms that have the definition of 'struct global_nvs' in <soc/nvs.h>?

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

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48701 )

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 8:

(2 comments)

https://review.coreboot.org/c/coreboot/+/48701/8/src/acpi/Kconfig 
File src/acpi/Kconfig:

https://review.coreboot.org/c/coreboot/+/48701/8/src/acpi/Kconfig@27 
PS8, Line 27: ACPI_SOC_NVS
> So this is for platforms that have the definition of 'struct global_nvs' in <soc/nvs. […]

That, and a <soc/nvs.h> in the include paths in the first place.

The longterm plan was to move every sb/intel/xxx/nvs.h to match <soc/nvs.h> and have the Kconfig removed.

https://review.coreboot.org/c/coreboot/+/48701/8/src/soc/intel/quark/include... 
File src/soc/intel/quark/include/soc/nvs.h:

PS8:
> Is it easy to keep Quark without GNVS?

Well.. we could move "select ACPI_SOC_NVS" to each soc/intel/*/KConfig separately instead of having it in intel/common/ only.

[Angel Pons (Code Review)]

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

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 8:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48701/8/src/soc/intel/quark/include... 
File src/soc/intel/quark/include/soc/nvs.h:

PS8:
> Well.. […]

I'm fine with having `select ACPI_SOC_NVS if !SOC_INTEL_QUARK`

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

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48701 )

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 8:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48701/8/src/soc/intel/quark/include... 
File src/soc/intel/quark/include/soc/nvs.h:

PS8:
> I'm fine with having `select ACPI_SOC_NVS if !SOC_INTEL_QUARK`

That does not go well together with possible future removal of ACPI_SOC_NVS, so I have no intention of taking that route.

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

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48701 )

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 8:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48701/8/src/acpi/Kconfig 
File src/acpi/Kconfig:

https://review.coreboot.org/c/coreboot/+/48701/8/src/acpi/Kconfig@31 
PS8, Line 31: 	bool
CB.49127 should go in first, I forgot to remove this earlier.

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

Hello build bot (Jenkins), Patrick Georgi, Mariusz Szafrański, Angel Pons, Arthur Heymans, Andrey Petrov, Patrick Rudolph, Piotr Król, Jason Glenesk, Martin Roth, Michał Żygowski, Marshall Dawson, Suresh Bellampalli, Vanessa Eusebio, Michal Motyl, Felix Held,

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

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

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

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
step cbmem_add() is a no-op here, if reached for some
boards. The memset() here is also redundant and becomes
harmful with followup works, as it would wipe out the
CBMEM console and ChromeOS related fields without them
being set again.

Change-Id: I9b2625af15cae90b9c1eb601e606d0430336609f
Signed-off-by: Kyösti Mälkki 
---
M src/acpi/Kconfig
M src/acpi/Makefile.inc
M src/acpi/gnvs.c
A src/acpi/nvs.c
M src/drivers/amd/agesa/Makefile.inc
A src/drivers/amd/agesa/nvs.c
M src/mainboard/protectli/vault_bsw/acpi_tables.c
M src/soc/amd/cezanne/Kconfig
M src/soc/amd/common/block/lpc/lpc.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/acpi.c
M src/soc/amd/stoneyridge/Kconfig
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/pch/lpc.c
M src/soc/intel/broadwell/ramstage.c
M src/soc/intel/common/Kconfig.common
M src/soc/intel/common/block/acpi/acpi.c
M src/soc/intel/denverton_ns/acpi.c
A src/soc/intel/quark/include/soc/nvs.h
M src/soc/intel/skylake/acpi.c
M src/southbridge/intel/bd82x6x/lpc.c
M src/southbridge/intel/i82371eb/acpi_tables.c
M src/southbridge/intel/i82801dx/fadt.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
34 files changed, 114 insertions(+), 116 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48701/9

[Angel Pons (Code Review)]

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

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 9: Code-Review+1

[Nico Huber (Code Review)]

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

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 9: Code-Review+2

(2 comments)

File src/acpi/Kconfig:

https://review.coreboot.org/c/coreboot/+/48701/comment/e2e2508f_5ece6610 
PS8, Line 31: 	bool
> CB.49127 should go in first, I forgot to remove this earlier.

Done

File src/drivers/amd/agesa/nvs.c:

https://review.coreboot.org/c/coreboot/+/48701/comment/c90b94f2_7e8ce313 
PS8, Line 4: #include <stdint.h>
IIRC, `stdint.h` only provides the types with a specific bit width. Also,
if this is meant for `size_t`, I guess types that are part of a forward-
declared function signature don't need an include (but that might just
be me).

[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/+/48701 )

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 9:

(1 comment)

File src/soc/intel/quark/include/soc/nvs.h:

PS8:
> That does not go well together with possible future removal of ACPI_SOC_NVS, so I have no intention  […]

I'll see what can be done afterwards.

[Lance Zhao (Code Review)]

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

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 9:

(1 comment)

File src/acpi/gnvs.c:

https://review.coreboot.org/c/coreboot/+/48701/comment/73d2e40c_a2032124 
PS9, Line 32: /* Needs implementation in platform code.
Did we have example of platform code anywhere for a reference?

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

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

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 9:

(2 comments)

File src/acpi/gnvs.c:

https://review.coreboot.org/c/coreboot/+/48701/comment/17f7f763_6ed39994 
PS9, Line 32: /* Needs implementation in platform code.
> Did we have example of platform code anywhere for a reference?

src/acpi/nvs.c actually implements this for all with existing <soc/nvs.h> include, see the other comments in this change.

The comment is still correct in the sense that src/southbridge/intel/lynxpoint/lpc.c is required.

File src/soc/intel/quark/include/soc/nvs.h:

PS8:
> I'll see what can be done afterwards.

I have some 20-30 more commits around GNVS to push.

[Lance Zhao (Code Review)]

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

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 10: Code-Review+2

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

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

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 10:

(2 comments)

File src/acpi/Kconfig:

https://review.coreboot.org/c/coreboot/+/48701/comment/382a933c_04e4af70 
PS8, Line 27: ACPI_SOC_NVS
> That, and a <soc/nvs.h> in the include paths in the first place. […]

Done

File src/acpi/gnvs.c:

https://review.coreboot.org/c/coreboot/+/48701/comment/23aab9fe_e3e18640 
PS9, Line 32: /* Needs implementation in platform code.
> src/acpi/nvs.c actually implements this for all with existing <soc/nvs. […]

Done

[Nico Huber (Code Review)]

Attention is currently required from: Angel Pons, Arthur Heymans.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48701 )

Change subject: ACPI: Drop redundant CBMEM_ID_ACPI_GNVS allocations
......................................................................

Patch Set 10:

(2 comments)

File src/acpi/Kconfig:

https://review.coreboot.org/c/coreboot/+/48701/comment/de9479fe_0f02fecd 
PS8, Line 27: ACPI_SOC_NVS
> That, and a <soc/nvs.h> in the include paths in the first place. […]

Done, question was answered and help text added.

File src/acpi/gnvs.c:

https://review.coreboot.org/c/coreboot/+/48701/comment/369b1589_548aa050 
PS9, Line 32: /* Needs implementation in platform code.
> src/acpi/nvs.c actually implements this for all with existing <soc/nvs. […]

Done

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48701 )

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
step cbmem_add() is a no-op here, if reached for some
boards. The memset() here is also redundant and becomes
harmful with followup works, as it would wipe out the
CBMEM console and ChromeOS related fields without them
being set again.

Change-Id: I9b2625af15cae90b9c1eb601e606d0430336609f
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48701
Reviewed-by: Lance Zhao
Reviewed-by: Angel Pons 
Reviewed-by: Nico Huber 
Tested-by: build bot (Jenkins) 
---
M src/acpi/Kconfig
M src/acpi/Makefile.inc
M src/acpi/gnvs.c
A src/acpi/nvs.c
M src/drivers/amd/agesa/Makefile.inc
A src/drivers/amd/agesa/nvs.c
M src/mainboard/protectli/vault_bsw/acpi_tables.c
M src/soc/amd/cezanne/Kconfig
M src/soc/amd/common/block/lpc/lpc.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/acpi.c
M src/soc/amd/stoneyridge/Kconfig
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/pch/lpc.c
M src/soc/intel/broadwell/ramstage.c
M src/soc/intel/common/Kconfig.common
M src/soc/intel/common/block/acpi/acpi.c
M src/soc/intel/denverton_ns/acpi.c
A src/soc/intel/quark/include/soc/nvs.h
M src/soc/intel/skylake/acpi.c
M src/southbridge/intel/bd82x6x/lpc.c
M src/southbridge/intel/i82371eb/acpi_tables.c
M src/southbridge/intel/i82801dx/fadt.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
34 files changed, 114 insertions(+), 116 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved
  Lance Zhao: Looks good to me, approved
  Angel Pons: Looks good to me, but someone else must approve

diff --git a/src/acpi/Kconfig b/src/acpi/Kconfig
index 11ef12a3..293c194 100644
--- a/src/acpi/Kconfig
+++ b/src/acpi/Kconfig
@@ -24,6 +24,12 @@
      Provide common definitions for Intel hardware PM1_CNT register sleep
      values.

+config ACPI_SOC_NVS
+	bool
+	help
+	  Set to indicate <soc/nvs.h> exists for the platform with a definition
+	  for global_nvs.
+
 config ACPI_NO_PCAT_8259
    bool
    help
diff --git a/src/acpi/Makefile.inc b/src/acpi/Makefile.inc
index 2f06be1..1cd837d 100644
--- a/src/acpi/Makefile.inc
+++ b/src/acpi/Makefile.inc
@@ -12,6 +12,7 @@
 ramstage-y += device.c
 ramstage-$(CONFIG_CHROMEOS) += chromeos-gnvs.c
 ramstage-y += gnvs.c
+ramstage-$(CONFIG_ACPI_SOC_NVS) += nvs.c
 ramstage-y += pld.c
 ramstage-y += sata.c
 ramstage-y += soundwire.c
diff --git a/src/acpi/gnvs.c b/src/acpi/gnvs.c
index c0a58f3..aed66f9 100644
--- a/src/acpi/gnvs.c
+++ b/src/acpi/gnvs.c
@@ -29,12 +29,7 @@
    	*gnvs_cbmc = (uintptr_t)cbmem_find(CBMEM_ID_CONSOLE);
 }

-/* Platforms that implement GNVS will need to implement these. */
-__weak size_t gnvs_size_of_array(void)
-{
-	return 0;
-}
-
+/* Needs implementation in platform code. */
 __weak uint32_t *gnvs_cbmc_ptr(struct global_nvs *gnvs_)
 {
    return NULL;
diff --git a/src/acpi/nvs.c b/src/acpi/nvs.c
new file mode 100644
index 0000000..0638191
--- /dev/null
+++ b/src/acpi/nvs.c
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <acpi/acpi_gnvs.h>
+#include <soc/nvs.h>
+#include <stdint.h>
+
+size_t gnvs_size_of_array(void)
+{
+	return sizeof(struct global_nvs);
+}
+
+uint32_t *gnvs_cbmc_ptr(struct global_nvs *gnvs)
+{
+	return &gnvs->cbmc;
+}
+
+/* Some <soc/nvs.h> have no chromeos entry. */
+#if CONFIG(MAINBOARD_HAS_CHROMEOS)
+void *gnvs_chromeos_ptr(struct global_nvs *gnvs)
+{
+	return &gnvs->chromeos;
+}
+#endif
diff --git a/src/drivers/amd/agesa/Makefile.inc b/src/drivers/amd/agesa/Makefile.inc
index 6d80c4c..246ed8b 100644
--- a/src/drivers/amd/agesa/Makefile.inc
+++ b/src/drivers/amd/agesa/Makefile.inc
@@ -20,6 +20,7 @@
 ramstage-y += eventlog.c
 ramstage-y += heapmanager.c
 ramstage-y += acpi_tables.c
+ramstage-y += nvs.c

romstage-$(CONFIG_CPU_AMD_AGESA) += oem_s3.c
 ramstage-$(CONFIG_CPU_AMD_AGESA) += oem_s3.c s3_mtrr.c
diff --git a/src/drivers/amd/agesa/nvs.c b/src/drivers/amd/agesa/nvs.c
new file mode 100644
index 0000000..5bde9d5
--- /dev/null
+++ b/src/drivers/amd/agesa/nvs.c
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <acpi/acpi_gnvs.h>
+#include <stdint.h>
+
+size_t gnvs_size_of_array(void)
+{
+	return 0;
+}
diff --git a/src/mainboard/protectli/vault_bsw/acpi_tables.c b/src/mainboard/protectli/vault_bsw/acpi_tables.c
index 3c94603..c1c5f76 100644
--- a/src/mainboard/protectli/vault_bsw/acpi_tables.c
+++ b/src/mainboard/protectli/vault_bsw/acpi_tables.c
@@ -8,7 +8,6 @@

void acpi_create_gnvs(struct global_nvs *gnvs)
 {
-	memset(gnvs, 0, sizeof(*gnvs));

acpi_init_gnvs(gnvs);

diff --git a/src/soc/amd/cezanne/Kconfig b/src/soc/amd/cezanne/Kconfig
index cb613e3..22d0c3a 100644
--- a/src/soc/amd/cezanne/Kconfig
+++ b/src/soc/amd/cezanne/Kconfig
@@ -9,6 +9,7 @@

config SOC_SPECIFIC_OPTIONS
    def_bool y
+	select ACPI_SOC_NVS
    select ARCH_BOOTBLOCK_X86_32
    select ARCH_VERSTAGE_X86_32
    select ARCH_ROMSTAGE_X86_32
diff --git a/src/soc/amd/common/block/lpc/lpc.c b/src/soc/amd/common/block/lpc/lpc.c
index 107aa4af..96fb176 100644
--- a/src/soc/amd/common/block/lpc/lpc.c
+++ b/src/soc/amd/common/block/lpc/lpc.c
@@ -100,7 +100,6 @@
 static void lpc_read_resources(struct device *dev)
 {
    struct resource *res;
-	struct global_nvs *gnvs;

/* Get the normal pci resources of this device */
    pci_dev_read_resources(dev);
@@ -134,10 +133,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(struct global_nvs));
-	printk(BIOS_DEBUG, "ACPI GNVS at %p\n", gnvs);
 }

static void lpc_set_resources(struct device *dev)
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig
index 4d31267..bd36822 100644
--- a/src/soc/amd/picasso/Kconfig
+++ b/src/soc/amd/picasso/Kconfig
@@ -17,6 +17,7 @@
    select X86_AMD_FIXED_MTRRS
    select X86_AMD_INIT_SIPI
    select ACPI_AMD_HARDWARE_SLEEP_VALUES
+	select ACPI_SOC_NVS
    select DRIVERS_I2C_DESIGNWARE
    select DRIVERS_USB_PCI_XHCI
    select GENERIC_GPIO_LIB
diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c
index b89974b..513d9f2 100644
--- a/src/soc/amd/picasso/acpi.c
+++ b/src/soc/amd/picasso/acpi.c
@@ -396,8 +396,6 @@

void acpi_create_gnvs(struct global_nvs *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/Kconfig b/src/soc/amd/stoneyridge/Kconfig
index 7d2ad21..206c95a 100644
--- a/src/soc/amd/stoneyridge/Kconfig
+++ b/src/soc/amd/stoneyridge/Kconfig
@@ -10,6 +10,7 @@
 config CPU_SPECIFIC_OPTIONS
    def_bool y
    select ACPI_AMD_HARDWARE_SLEEP_VALUES
+	select ACPI_SOC_NVS
    select ARCH_ALL_STAGES_X86_32
    select BOOT_DEVICE_SUPPORTS_WRITES if BOOT_DEVICE_SPI_FLASH
    select COLLECT_TIMESTAMPS_NO_TSC
diff --git a/src/soc/amd/stoneyridge/acpi.c b/src/soc/amd/stoneyridge/acpi.c
index e62f235..74a3b2f 100644
--- a/src/soc/amd/stoneyridge/acpi.c
+++ b/src/soc/amd/stoneyridge/acpi.c
@@ -164,8 +164,6 @@

void acpi_create_gnvs(struct global_nvs *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 d0e7a73..61d42a5 100644
--- a/src/soc/intel/apollolake/acpi.c
+++ b/src/soc/intel/apollolake/acpi.c
@@ -77,8 +77,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 fdbeef2..9670a31 100644
--- a/src/soc/intel/apollolake/chip.c
+++ b/src/soc/intel/apollolake/chip.c
@@ -28,7 +28,6 @@
 #include <soc/intel/common/vbt.h>
 #include <soc/iomap.h>
 #include <soc/itss.h>
-#include <soc/nvs.h>
 #include <soc/pci_devs.h>
 #include <soc/pm.h>
 #include <soc/systemagent.h>
@@ -318,9 +317,6 @@
     */
    p2sb_unhide();

-	/* Allocate ACPI NVS in CBMEM */
-	cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-
    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 b15970e..e199e9f 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>
@@ -143,15 +144,9 @@

static void s3_resume_prepare(void)
 {
-	struct global_nvs *gnvs;
+	struct global_nvs *gnvs = acpi_get_gnvs();

-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-	if (gnvs == NULL)
-		return;
-
-	if (!acpi_is_wakeup_s3())
-		memset(gnvs, 0, sizeof(struct global_nvs));
-	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 2df6410..d8305c6 100644
--- a/src/soc/intel/baytrail/southcluster.c
+++ b/src/soc/intel/baytrail/southcluster.c
@@ -489,11 +489,6 @@
    struct global_nvs *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 c9df526..3a82318 100644
--- a/src/soc/intel/braswell/acpi.c
+++ b/src/soc/intel/braswell/acpi.c
@@ -382,11 +382,6 @@
    struct global_nvs *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 eba1527..68bddfb 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)
-{
-	struct global_nvs *gnvs;
-
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-	if (!acpi_is_wakeup_s3() && gnvs)
-		memset(gnvs, 0, sizeof(struct global_nvs));
-}
-
 static void set_board_id(void)
 {
-	struct global_nvs *gnvs;
-
-	gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		printk(BIOS_ERR, "Unable to locate Global NVS\n");
+	struct global_nvs *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/pch/lpc.c b/src/soc/intel/broadwell/pch/lpc.c
index 5a29b02..c735a81 100644
--- a/src/soc/intel/broadwell/pch/lpc.c
+++ b/src/soc/intel/broadwell/pch/lpc.c
@@ -593,8 +593,6 @@

static void pch_lpc_read_resources(struct device *dev)
 {
-	struct global_nvs *gnvs;
-
    /* Get the normal PCI resources of this device. */
    pci_dev_read_resources(dev);

@@ -603,11 +601,6 @@

/* Add IO resources. */
    pch_lpc_add_io_resources(dev);
-
-	/* Allocate ACPI NVS in CBMEM */
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-	if (!acpi_is_wakeup_s3() && gnvs)
-		memset(gnvs, 0, sizeof(struct global_nvs));
 }

static void southcluster_inject_dsdt(const struct device *device)
@@ -615,11 +608,6 @@
    struct global_nvs *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 57abf95..b4f38a5 100644
--- a/src/soc/intel/broadwell/ramstage.c
+++ b/src/soc/intel/broadwell/ramstage.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

#include <acpi/acpi.h>
+#include <acpi/acpi_gnvs.h>
 #include <cbmem.h>
 #include <console/console.h>
 #include <device/device.h>
@@ -63,15 +64,9 @@

static void s3_resume_prepare(void)
 {
-	struct global_nvs *gnvs;
+	struct global_nvs *gnvs = acpi_get_gnvs();

-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-	if (gnvs == NULL)
-		return;
-
-	if (!acpi_is_wakeup_s3())
-		memset(gnvs, 0, sizeof(struct global_nvs));
-	else
+	if (gnvs && acpi_is_wakeup_s3())
    	save_acpi_wake_source(gnvs);
 }

diff --git a/src/soc/intel/common/Kconfig.common b/src/soc/intel/common/Kconfig.common
index b0fbf25..f39571e 100644
--- a/src/soc/intel/common/Kconfig.common
+++ b/src/soc/intel/common/Kconfig.common
@@ -2,6 +2,7 @@
    bool
    select AZALIA_PLUGIN_SUPPORT
    select HAVE_DISPLAY_MTRRS
+	select ACPI_SOC_NVS
    help
      common code for Intel SOCs

diff --git a/src/soc/intel/common/block/acpi/acpi.c b/src/soc/intel/common/block/acpi/acpi.c
index 4a53c55..fc43528 100644
--- a/src/soc/intel/common/block/acpi/acpi.c
+++ b/src/soc/intel/common/block/acpi/acpi.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */

+#include <acpi/acpi_gnvs.h>
 #include <acpi/acpigen.h>
 #include <arch/cpu.h>
 #include <arch/ioapic.h>
@@ -7,7 +8,6 @@
 #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/intel/common/common.h>
@@ -244,11 +244,6 @@
    struct global_nvs *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 993338a..e68e5db 100644
--- a/src/soc/intel/denverton_ns/acpi.c
+++ b/src/soc/intel/denverton_ns/acpi.c
@@ -251,11 +251,6 @@
    struct global_nvs *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/quark/include/soc/nvs.h b/src/soc/intel/quark/include/soc/nvs.h
new file mode 100644
index 0000000..904607f
--- /dev/null
+++ b/src/soc/intel/quark/include/soc/nvs.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef SOC_INTEL_QUARK_NVS_H
+#define SOC_INTEL_QUARK_NVS_H
+
+#include <stdint.h>
+
+struct __packed global_nvs {
+	uint32_t	cbmc; /* 0x00 - 0x03 - coreboot Memory Console */
+};
+
+#endif /* SOC_INTEL_QUARK_NVS_H */
diff --git a/src/soc/intel/skylake/acpi.c b/src/soc/intel/skylake/acpi.c
index 4272954..6b90dc4 100644
--- a/src/soc/intel/skylake/acpi.c
+++ b/src/soc/intel/skylake/acpi.c
@@ -545,11 +545,6 @@
    struct global_nvs *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 4515261..0a99d80 100644
--- a/src/southbridge/intel/bd82x6x/lpc.c
+++ b/src/southbridge/intel/bd82x6x/lpc.c
@@ -642,12 +642,16 @@
    pch_enable(dev);
 }

+size_t gnvs_size_of_array(void)
+{
+	return sizeof(struct global_nvs);
+}
+
 void southbridge_inject_dsdt(const struct device *dev)
 {
-	struct global_nvs *gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	struct global_nvs *gnvs = acpi_get_gnvs();

if (gnvs) {
-		memset(gnvs, 0, sizeof(*gnvs));

acpi_create_gnvs(gnvs);

diff --git a/src/southbridge/intel/i82371eb/acpi_tables.c b/src/southbridge/intel/i82371eb/acpi_tables.c
index 7507cd5..433555d 100644
--- a/src/southbridge/intel/i82371eb/acpi_tables.c
+++ b/src/southbridge/intel/i82371eb/acpi_tables.c
@@ -2,6 +2,7 @@

#include <console/console.h>
 #include <acpi/acpi.h>
+#include <acpi/acpi_gnvs.h>
 #include <acpi/acpigen.h>
 #include <device/device.h>
 #include "i82371eb.h"
@@ -44,3 +45,8 @@
    /* chipset doesn't have mmconfig */
    return current;
 }
+
+size_t gnvs_size_of_array(void)
+{
+	return 0;
+}
diff --git a/src/southbridge/intel/i82801dx/fadt.c b/src/southbridge/intel/i82801dx/fadt.c
index 84ea73a..66aa3f1 100644
--- a/src/southbridge/intel/i82801dx/fadt.c
+++ b/src/southbridge/intel/i82801dx/fadt.c
@@ -2,6 +2,7 @@

#include <device/pci_ops.h>
 #include <acpi/acpi.h>
+#include <acpi/acpi_gnvs.h>
 #include <version.h>

/* FIXME: This needs to go into a separate .h file
@@ -79,3 +80,8 @@
    fadt->x_gpe0_blk.addrl = pmbase + 0x28;
    fadt->x_gpe0_blk.addrh = 0x0;
 }
+
+size_t gnvs_size_of_array(void)
+{
+	return 0;
+}
diff --git a/src/southbridge/intel/i82801gx/lpc.c b/src/southbridge/intel/i82801gx/lpc.c
index 4db9351..3a89fbe 100644
--- a/src/southbridge/intel/i82801gx/lpc.c
+++ b/src/southbridge/intel/i82801gx/lpc.c
@@ -479,12 +479,16 @@
    outb(POST_OS_BOOT, 0x80);
 }

+size_t gnvs_size_of_array(void)
+{
+	return sizeof(struct global_nvs);
+}
+
 void southbridge_inject_dsdt(const struct device *dev)
 {
-	struct global_nvs *gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	struct global_nvs *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 821a0b7..c4712ba 100644
--- a/src/southbridge/intel/i82801ix/lpc.c
+++ b/src/southbridge/intel/i82801ix/lpc.c
@@ -453,12 +453,16 @@
    res->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED;
 }

+size_t gnvs_size_of_array(void)
+{
+	return sizeof(struct global_nvs);
+}
+
 void southbridge_inject_dsdt(const struct device *dev)
 {
-	struct global_nvs *gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	struct global_nvs *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 ad9bac1..0cc147d 100644
--- a/src/southbridge/intel/i82801jx/lpc.c
+++ b/src/southbridge/intel/i82801jx/lpc.c
@@ -477,12 +477,16 @@
    }
 }

+size_t gnvs_size_of_array(void)
+{
+	return sizeof(struct global_nvs);
+}
+
 void southbridge_inject_dsdt(const struct device *dev)
 {
-	struct global_nvs *gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	struct global_nvs *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 0895ddd..4f9a996 100644
--- a/src/southbridge/intel/ibexpeak/lpc.c
+++ b/src/southbridge/intel/ibexpeak/lpc.c
@@ -542,12 +542,16 @@
    pch_enable(dev);
 }

+size_t gnvs_size_of_array(void)
+{
+	return sizeof(struct global_nvs);
+}
+
 void southbridge_inject_dsdt(const struct device *dev)
 {
-	struct global_nvs *gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
+	struct global_nvs *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 586e626..bd424b8 100644
--- a/src/southbridge/intel/lynxpoint/lpc.c
+++ b/src/southbridge/intel/lynxpoint/lpc.c
@@ -661,8 +661,6 @@

static void pch_lpc_read_resources(struct device *dev)
 {
-	struct global_nvs *gnvs;
-
    /* Get the normal PCI resources of this device. */
    pci_dev_read_resources(dev);

@@ -671,11 +669,6 @@

/* Add IO resources. */
    pch_lpc_add_io_resources(dev);
-
-	/* Allocate ACPI NVS in CBMEM */
-	gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(struct global_nvs));
-	if (!acpi_is_wakeup_s3() && gnvs)
-		memset(gnvs, 0, sizeof(struct global_nvs));
 }

static void pch_lpc_enable(struct device *dev)
@@ -687,16 +680,16 @@
    pch_enable(dev);
 }

+size_t gnvs_size_of_array(void)
+{
+	return sizeof(struct global_nvs);
+}
+
 void southbridge_inject_dsdt(const struct device *dev)
 {
    struct global_nvs *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);