Arthur Heymans has uploaded this change for review.

View Change

cpu/x86/smm: Add a function for the amount of support CPUs

CONFIG_MAX_CPUS might be larger than the amount for which an SMI
handler is installed. So when looping over CONFIG_MAX_CPUS to fetch a
SMM save state one might access memory that does not belong to a CPU
node save state and get invalid results.

Change-Id: Ibc17602aa2817d910292c37d0a0095d3d9dc0aae
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
---
M src/cpu/amd/smm/amd64_save_state.c
M src/cpu/intel/smm/em64t100_save_state.c
M src/cpu/intel/smm/em64t101_save_state.c
M src/cpu/x86/smm/legacy_save_state.c
M src/cpu/x86/smm/save_state.c
M src/cpu/x86/smm/smihandler.c
M src/cpu/x86/smm/smm_module_handler.c
M src/include/cpu/x86/smm.h
8 files changed, 20 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/47016/1
diff --git a/src/cpu/amd/smm/amd64_save_state.c b/src/cpu/amd/smm/amd64_save_state.c
index db329c0..a274fd1 100644
--- a/src/cpu/amd/smm/amd64_save_state.c
+++ b/src/cpu/amd/smm/amd64_save_state.c
@@ -84,7 +84,7 @@
int node;
u8 reg_al;

- for (node = 0; node < CONFIG_MAX_CPUS; node++) {
+ for (node = 0; node < smm_max_cpus(); node++) {
state = smm_get_save_state(node);
smm_io_trap = state->smm_io_trap_offset;

diff --git a/src/cpu/intel/smm/em64t100_save_state.c b/src/cpu/intel/smm/em64t100_save_state.c
index 4fd7d76..fe5f65e 100644
--- a/src/cpu/intel/smm/em64t100_save_state.c
+++ b/src/cpu/intel/smm/em64t100_save_state.c
@@ -72,7 +72,7 @@
em64t100_smm_state_save_area_t *state;
int node;

- for (node = 0; node < CONFIG_MAX_CPUS; node++) {
+ for (node = 0; node < smm_max_cpus(); node++) {
state = smm_get_save_state(node);

/* Check for Synchronous IO (bit0 == 1) */
diff --git a/src/cpu/intel/smm/em64t101_save_state.c b/src/cpu/intel/smm/em64t101_save_state.c
index c6aa397..70112df 100644
--- a/src/cpu/intel/smm/em64t101_save_state.c
+++ b/src/cpu/intel/smm/em64t101_save_state.c
@@ -72,7 +72,7 @@
em64t101_smm_state_save_area_t *state;
int node;

- for (node = 0; node < CONFIG_MAX_CPUS; node++) {
+ for (node = 0; node < smm_max_cpus(); node++) {
state = smm_get_save_state(node);

/* Check for Synchronous IO (bit0 == 1) */
diff --git a/src/cpu/x86/smm/legacy_save_state.c b/src/cpu/x86/smm/legacy_save_state.c
index 8e996ee..0b07155 100644
--- a/src/cpu/x86/smm/legacy_save_state.c
+++ b/src/cpu/x86/smm/legacy_save_state.c
@@ -71,7 +71,7 @@
legacy_smm_state_save_area_t *state;
int node;

- for (node = 0; node < CONFIG_MAX_CPUS; node++) {
+ for (node = 0; node < smm_max_cpus(); node++) {
state = smm_get_save_state(node);

/* Check AL against the requested command */
diff --git a/src/cpu/x86/smm/save_state.c b/src/cpu/x86/smm/save_state.c
index bb08f86..c860b03 100644
--- a/src/cpu/x86/smm/save_state.c
+++ b/src/cpu/x86/smm/save_state.c
@@ -59,7 +59,7 @@
if (init_save_state())
return -1;

- if (node > CONFIG_MAX_CPUS)
+ if (node > smm_max_cpus())
return -1;

return save_state->get_reg(reg, node, out, length);
@@ -70,7 +70,7 @@
if (init_save_state())
return -1;

- if (node > CONFIG_MAX_CPUS)
+ if (node > smm_max_cpus())
return -1;

return save_state->set_reg(reg, node, in, length);
diff --git a/src/cpu/x86/smm/smihandler.c b/src/cpu/x86/smm/smihandler.c
index 0d9131e..21b2b5e 100644
--- a/src/cpu/x86/smm/smihandler.c
+++ b/src/cpu/x86/smm/smihandler.c
@@ -136,6 +136,11 @@
return region_overlap(&r_smm, r);
}

+uint32_t smm_max_cpus(void)
+{
+ return CONFIG_MAX_CPUS;
+}
+
/**
* @brief Interrupt handler for SMI#
*
diff --git a/src/cpu/x86/smm/smm_module_handler.c b/src/cpu/x86/smm/smm_module_handler.c
index 3ba5684..89864c4 100644
--- a/src/cpu/x86/smm/smm_module_handler.c
+++ b/src/cpu/x86/smm/smm_module_handler.c
@@ -119,6 +119,11 @@
return region_overlap(&r_smm, r) || region_overlap(&r_aseg, r);
}

+uint32_t smm_max_cpus(void)
+{
+ return MIN(smm_runtime->num_cpus, CONFIG_MAX_CPUS);
+}
+
asmlinkage void smm_handler_start(void *arg)
{
const struct smm_module_params *p;
@@ -137,7 +142,7 @@
if (smm_runtime == NULL)
smm_runtime = runtime;

- if (cpu >= CONFIG_MAX_CPUS) {
+ if (cpu >= smm_max_cpus()) {
console_init();
printk(BIOS_CRIT,
"Invalid CPU number assigned in SMM stub: %d\n", cpu);
diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h
index 6cf6f82..8bdd695 100644
--- a/src/include/cpu/x86/smm.h
+++ b/src/include/cpu/x86/smm.h
@@ -98,6 +98,9 @@
* account CPUs which are configured to not save their state to RAM. */
void *smm_get_save_state(int cpu);

+/* Return the amount of CPUs the smihandler is set up to handle. */
+uint32_t smm_max_cpus(void);
+
/* Returns true if the region overlaps with the SMM */
bool smm_region_overlaps_handler(const struct region *r);


To view, visit change 47016. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibc17602aa2817d910292c37d0a0095d3d9dc0aae
Gerrit-Change-Number: 47016
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-MessageType: newchange