Change in coreboot[master]: cpu/x86: Test the GNVS pointer before actually setting it

[Arthur Heymans (Code Review)]

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

Change subject: cpu/x86: Test the GNVS pointer before actually setting it
......................................................................

cpu/x86: Test the GNVS pointer before actually setting it

Change-Id: I15f67a12dbdd2529572eebf354cfbcf67017ca6d
Signed-off-by: Arthur Heymans 
---
M src/soc/intel/baytrail/smihandler.c
M src/soc/intel/braswell/smihandler.c
M src/soc/intel/broadwell/pch/smihandler.c
M src/soc/intel/common/block/smm/smihandler.c
M src/soc/intel/denverton_ns/smihandler.c
M src/southbridge/intel/bd82x6x/smihandler.c
M src/southbridge/intel/ibexpeak/smihandler.c
M src/southbridge/intel/lynxpoint/smihandler.c
8 files changed, 27 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/47014/1

diff --git a/src/soc/intel/baytrail/smihandler.c b/src/soc/intel/baytrail/smihandler.c
index 1810821..e1387b2 100644
--- a/src/soc/intel/baytrail/smihandler.c
+++ b/src/soc/intel/baytrail/smihandler.c
@@ -320,11 +320,12 @@
    	state = smi_apmc_find_state_save(reg8);
    	if (state) {
    		/* EBX in the state save contains the GNVS pointer */
-			gnvs = (struct global_nvs *)((uint32_t)state->rbx);
-			if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+			void *test_gnvs = (void *)((u32)state->rbx);
+			if (smm_points_to_smram(test_gnvs, sizeof(*gnvs))) {
    			printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
    			return;
    		}
+			gnvs = test_gnvs;
    		smm_initialized = 1;
    		printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
    	}
diff --git a/src/soc/intel/braswell/smihandler.c b/src/soc/intel/braswell/smihandler.c
index 6e250d8..e422777 100644
--- a/src/soc/intel/braswell/smihandler.c
+++ b/src/soc/intel/braswell/smihandler.c
@@ -299,11 +299,12 @@
    	state = smi_apmc_find_state_save(reg8);
    	if (state) {
    		/* EBX in the state save contains the GNVS pointer */
-			gnvs = (struct global_nvs *)((uint32_t)state->rbx);
-			if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+			void *test_gnvs = (void *)((u32)state->rbx);
+			if (smm_points_to_smram(test_gnvs, sizeof(*gnvs))) {
    			printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
    			return;
    		}
+			gnvs = test_gnvs;
    		smm_initialized = 1;
    		printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
    	}
diff --git a/src/soc/intel/broadwell/pch/smihandler.c b/src/soc/intel/broadwell/pch/smihandler.c
index fd5d452..f3b0cd2 100644
--- a/src/soc/intel/broadwell/pch/smihandler.c
+++ b/src/soc/intel/broadwell/pch/smihandler.c
@@ -342,11 +342,12 @@
    	state = smi_apmc_find_state_save(reg8);
    	if (state) {
    		/* EBX in the state save contains the GNVS pointer */
-			gnvs = (struct global_nvs *)((u32)state->rbx);
-			if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+			void *test_gnvs = (void *)((u32)state->rbx);
+			if (smm_points_to_smram(test_gnvs, sizeof(*gnvs))) {
    			printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
    			return;
    		}
+			gnvs = test_gnvs;
    		smm_initialized = 1;
    		printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
    	}
diff --git a/src/soc/intel/common/block/smm/smihandler.c b/src/soc/intel/common/block/smm/smihandler.c
index 270b1aa..141f084 100644
--- a/src/soc/intel/common/block/smm/smihandler.c
+++ b/src/soc/intel/common/block/smm/smihandler.c
@@ -370,12 +370,12 @@
    	state = find_save_state(save_state_ops, reg8);
    	if (state) {
    		/* EBX in the state save contains the GNVS pointer */
-			uint32_t reg_ebx = save_state_ops->get_reg(state, RBX);
-			gnvs = (struct global_nvs *)(uintptr_t)reg_ebx;
-			if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+			void *test_gnvs = (void *)((u32)state->rbx);
+			if (smm_points_to_smram(test_gnvs, sizeof(*gnvs))) {
    			printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
    			return;
    		}
+			gnvs = test_gnvs;
    		smm_initialized = 1;
    		printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
    	}
diff --git a/src/soc/intel/denverton_ns/smihandler.c b/src/soc/intel/denverton_ns/smihandler.c
index 5eecba7..1e7d1e3 100644
--- a/src/soc/intel/denverton_ns/smihandler.c
+++ b/src/soc/intel/denverton_ns/smihandler.c
@@ -240,7 +240,12 @@
    	state = smi_apmc_find_state_save(reg8);
    	if (state) {
    		/* EBX in the state save contains the GNVS pointer */
-			gnvs = (struct global_nvs *)((uint32_t)state->rbx);
+			void *test_gnvs = (void *)((u32)state->rbx);
+			if (smm_points_to_smram(test_gnvs, sizeof(*gnvs))) {
+				printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
+				return;
+			}
+			gnvs = test_gnvs;
    		smm_initialized = 1;
    		printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
    	}
diff --git a/src/southbridge/intel/bd82x6x/smihandler.c b/src/southbridge/intel/bd82x6x/smihandler.c
index 40672f8..f390d06 100644
--- a/src/southbridge/intel/bd82x6x/smihandler.c
+++ b/src/southbridge/intel/bd82x6x/smihandler.c
@@ -191,12 +191,12 @@
    	smi_apmc_find_state_save(apm_cnt);
    if (state) {
    	/* EBX in the state save contains the GNVS pointer */
-		gnvs = (struct global_nvs *)((u32)state->rbx);
-		struct region r = {(uintptr_t)gnvs, sizeof(struct global_nvs)};
-		if (smm_region_overlaps_handler(&r)) {
+		void *test_gnvs = (void *)((u32)state->rbx);
+		if (smm_points_to_smram(test_gnvs, sizeof(*gnvs))) {
    		printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
    		return;
    	}
+		gnvs = test_gnvs;
    	*smm_done = 1;
    	printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
    }
diff --git a/src/southbridge/intel/ibexpeak/smihandler.c b/src/southbridge/intel/ibexpeak/smihandler.c
index 0c5e954..e53705a 100644
--- a/src/southbridge/intel/ibexpeak/smihandler.c
+++ b/src/southbridge/intel/ibexpeak/smihandler.c
@@ -153,11 +153,12 @@
    	smi_apmc_find_state_save(apm_cnt);
    if (state) {
    	/* EBX in the state save contains the GNVS pointer */
-		gnvs = (struct global_nvs *)(uintptr_t)((u32)state->rbx);
-		if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+		void *test_gnvs = (void *)((u32)state->rbx);
+		if (smm_points_to_smram(test_gnvs, sizeof(*gnvs))) {
    		printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
    		return;
    	}
+		gnvs = test_gnvs;
    	*smm_done = 1;
    	printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
    }
diff --git a/src/southbridge/intel/lynxpoint/smihandler.c b/src/southbridge/intel/lynxpoint/smihandler.c
index 9a5e5c0..b6fe7bf 100644
--- a/src/southbridge/intel/lynxpoint/smihandler.c
+++ b/src/southbridge/intel/lynxpoint/smihandler.c
@@ -313,11 +313,12 @@
    	state = smi_apmc_find_state_save(reg8);
    	if (state) {
    		/* EBX in the state save contains the GNVS pointer */
-			gnvs = (struct global_nvs *)((u32)state->rbx);
-			if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+			void *test_gnvs = (void *)((u32)state->rbx);
+			if (smm_points_to_smram(test_gnvs, sizeof(*gnvs))) {
    			printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
    			return;
    		}
+			gnvs = test_gnvs;
    		smm_initialized = 1;
    		printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
    	}

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

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

Change subject: cpu/x86: Test the GNVS pointer before actually setting it
......................................................................

Patch Set 1:

(1 comment)

Patchset:

PS1: 
Obsolete for master?

[Arthur Heymans (Code Review)]

Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/47014 )

Change subject: cpu/x86: Test the GNVS pointer before actually setting it
......................................................................

Abandoned