Change in coreboot[master]: soc/intel/common: Use common SMM save state ops - coreboot-gerrit

20 Nov 2019

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37015 )
Change subject: soc/intel/common: Use common SMM save state ops
......................................................................
soc/intel/common: Use common SMM save state ops
Change-Id: I656aad823a96eafb507e8b30d2177d21efcf2416
Signed-off-by: Arthur Heymans arthur@aheymans.xyz
---
M src/soc/intel/apollolake/smihandler.c
M src/soc/intel/cannonlake/smihandler.c
M src/soc/intel/common/block/include/intelblocks/smihandler.h
M src/soc/intel/common/block/smm/smihandler.c
M src/soc/intel/common/block/smm/smitraphandler.c
M src/soc/intel/icelake/smihandler.c
M src/soc/intel/skylake/smihandler.c
M src/soc/intel/tigerlake/smihandler.c
8 files changed, 98 insertions(+), 269 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/37015/1

diff --git a/src/soc/intel/apollolake/smihandler.c b/src/soc/intel/apollolake/smihandler.c
index 53d2b7e..ede073d 100644
--- a/src/soc/intel/apollolake/smihandler.c
+++ b/src/soc/intel/apollolake/smihandler.c
@@ -29,11 +29,6 @@
    return 1;
 }
-const struct smm_save_state_ops *get_smm_save_state_ops(void)
-{
-	return &em64t100_smm_ops;
-}
-
 /* SMI handlers that should be serviced in SCI mode too. */
 uint32_t smihandler_soc_get_sci_mask(void)
 {
diff --git a/src/soc/intel/cannonlake/smihandler.c b/src/soc/intel/cannonlake/smihandler.c
index 4d0b241..31ec9e3 100644
--- a/src/soc/intel/cannonlake/smihandler.c
+++ b/src/soc/intel/cannonlake/smihandler.c
@@ -32,11 +32,6 @@
 #define CSME0_BAR	0x0
 #define CSME0_FID	0xb0
-const struct smm_save_state_ops *get_smm_save_state_ops(void)
-{
-	return &em64t101_smm_ops;
-}
-
 static void pch_disable_heci(void)
 {
    struct pcr_sbi_msg msg = {
diff --git a/src/soc/intel/common/block/include/intelblocks/smihandler.h b/src/soc/intel/common/block/include/intelblocks/smihandler.h
index d8520f1..91bf88f 100644
--- a/src/soc/intel/common/block/include/intelblocks/smihandler.h
+++ b/src/soc/intel/common/block/include/intelblocks/smihandler.h
@@ -22,29 +22,7 @@
 struct gpi_status;
 struct global_nvs_t;
-/*
- * The register value is used with get_reg and set_reg
- */
-enum smm_reg {
-	RAX,
-	RBX,
-	RCX,
-	RDX,
-};
-
-struct smm_save_state_ops {
-	/* return io_misc_info from SMM Save State Area */
-	uint32_t (*get_io_misc_info)(void *state);
-
-	/* return value of the requested register from
-	 * SMM Save State Area
-	 */
-	uint64_t (*get_reg)(void *state, enum smm_reg reg);
-
-	void (*set_reg)(void *state, enum smm_reg reg, uint64_t val);
-};
-
-typedef void (*smi_handler_t)(const struct smm_save_state_ops *save_state_ops);
+typedef void (*smi_handler_t)(void);
/*
  * SOC SMI Handler has to provide this structure which has methods to access
@@ -66,79 +44,69 @@
  * the SMI event on SLP_EN. The default functionality is provided in
  * soc/intel/common/block/smm/smihandler.c
  */
-void smihandler_southbridge_sleep(
-	const struct smm_save_state_ops *save_state_ops);
+void smihandler_southbridge_sleep(void);
/*
  * This function should be implemented in SOC specific code to handle
  * SMI_APM event. The default functionality is provided in
  * soc/intel/common/block/smm/smihandler.c
  */
-void smihandler_southbridge_apmc(
-	const struct smm_save_state_ops *save_state_ops);
+void smihandler_southbridge_apmc(void);
/*
  * This function should be implemented in SOC specific code to handle
  * SMI_PM1 event. The default functionality is provided in
  * soc/intel/common/block/smm/smihandler.c
  */
-void smihandler_southbridge_pm1(
-	const struct smm_save_state_ops *save_state_ops);
+void smihandler_southbridge_pm1(void);
/*
  * This function should be implemented in SOC specific code to handle
  * SMI_GPE0 event. The default functionality is provided in
  * soc/intel/common/block/smm/smihandler.c
  */
-void smihandler_southbridge_gpe0(
-	const struct smm_save_state_ops *save_state_ops);
+void smihandler_southbridge_gpe0(void);
/*
  * This function should be implemented in SOC specific code to handle
  * MC event. The default functionality is provided in
  * soc/intel/common/block/smm/smihandler.c
  */
-void smihandler_southbridge_mc(
-	const struct smm_save_state_ops *save_state_ops);
+void smihandler_southbridge_mc(void);
/*
  * This function should be implemented in SOC specific code to handle
  * minitor event. The default functionality is provided in
  * soc/intel/common/block/smm/smihandler.c
  */
-void smihandler_southbridge_monitor(
-	const struct smm_save_state_ops *save_state_ops);
+void smihandler_southbridge_monitor(void);
 /*
  * This function should be implemented in SOC specific code to handle
  * SMI_TCO event. The default functionality is provided in
  * soc/intel/common/block/smm/smihandler.c
  */
-void smihandler_southbridge_tco(
-	const struct smm_save_state_ops *save_state_ops);
+void smihandler_southbridge_tco(void);
/*
  * This function should be implemented in SOC specific code to handle
  * SMI PERIODIC_STS event. The default functionality is provided in
  * soc/intel/common/block/smm/smihandler.c
  */
-void smihandler_southbridge_periodic(
-	const struct smm_save_state_ops *save_state_ops);
+void smihandler_southbridge_periodic(void);
/*
  * This function should be implemented in SOC specific code to handle
  * SMI GPIO_STS event. The default functionality is provided in
  * soc/intel/common/block/smm/smihandler.c
  */
-void smihandler_southbridge_gpi(
-	const struct smm_save_state_ops *save_state_ops);
+void smihandler_southbridge_gpi(void);
/*
  * This function should be implemented in SOC specific code to handle
  * SMI ESPI_STS event. The default functionality is provided in
  * soc/intel/common/block/smm/smihandler.c
  */
-void smihandler_southbridge_espi(
-	const struct smm_save_state_ops *save_state_ops);
+void smihandler_southbridge_espi(void);
/*
  * Returns gnvs pointer within SMM context
@@ -172,8 +140,4 @@
/* Mainboard handler for ESPI EMIs */
 void mainboard_smi_espi_handler(void);
-
-extern const struct smm_save_state_ops em64t100_smm_ops;
-
-extern const struct smm_save_state_ops em64t101_smm_ops;
 #endif
diff --git a/src/soc/intel/common/block/smm/smihandler.c b/src/soc/intel/common/block/smm/smihandler.c
index 0581d23..d7330df 100644
--- a/src/soc/intel/common/block/smm/smihandler.c
+++ b/src/soc/intel/common/block/smm/smihandler.c
@@ -20,8 +20,6 @@
 #include <console/console.h>
 #include <cpu/x86/cache.h>
 #include <cpu/x86/smm.h>
-#include <cpu/intel/em64t100_save_state.h>
-#include <cpu/intel/em64t101_save_state.h>
 #include <delay.h>
 #include <device/pci_def.h>
 #include <elog.h>
@@ -87,36 +85,44 @@
/* Common Functions */
-static void *find_save_state(const struct smm_save_state_ops *save_state_ops,
-	int cmd)
+/*
+ * Look for Synchronous IO SMI and use save state from that
+ * core in case we are not running on the same core that
+ * initiated the IO transaction.
+ */
+static int smi_apmc_find_state_save_node(u8 cmd)
 {
+	const struct smm_save_state_ops *ops = get_save_state_ops();
    int node;
-	void *state = NULL;
-	uint32_t io_misc_info;
-	uint8_t reg_al;
/* Check all nodes looking for the one that issued the IO */
    for (node = 0; node < CONFIG_MAX_CPUS; node++) {
-		state = smm_get_save_state(node);
+		uint32_t io_misc_info;
+		uint64_t rax;
+		if (ops->get_reg(node, RAX, &rax))
+			continue; /* ??? */
+		/* Check AX against the requested command */
+		if ((rax & 0xff) != cmd)
+			continue;
+		if (ops->get_io_misc_info(node, &io_misc_info)) {
-		io_misc_info = save_state_ops->get_io_misc_info(state);
+			/* Check for Synchronous IO (bit0 == 1) */
+			if (!(io_misc_info & (1 << 0)))
+				continue;
-		/* Check for Synchronous IO (bit0==1) */
-		if (!(io_misc_info & (1 << 0)))
-			continue;
-		/* Make sure it was a write (bit4==0) */
-		if (io_misc_info & (1 << 4))
-			continue;
-		/* Check for APMC IO port */
-		if (((io_misc_info >> 16) & 0xff) != APM_CNT)
-			continue;
-		/* Check AL against the requested command */
-		reg_al = save_state_ops->get_reg(state, RAX);
-		if (reg_al != cmd)
-			continue;
-		break;
+			/* Make sure it was a write (bit4 == 0) */
+			if (io_misc_info & (1 << 4))
+				continue;
+
+			/* Check for APMC IO port */
+			if (((io_misc_info >> 16) & 0xff) != APM_CNT)
+				continue;
+		}
+
+		return node;
    }
-	return state;
+
+	return -1;
 }
/* Inherited from cpu/x86/smm.h resulting in a different signature */
@@ -178,8 +184,7 @@
 }
-void smihandler_southbridge_sleep(
-	const struct smm_save_state_ops *save_state_ops)
+void smihandler_southbridge_sleep(void)
 {
    uint32_t reg32;
    uint8_t slp_typ;
@@ -274,47 +279,56 @@
    }
 }
-static void southbridge_smi_gsmi(
-	const struct smm_save_state_ops *save_state_ops)
+static void southbridge_smi_gsmi(void)
 {
-	u8 sub_command, ret;
-	void *io_smi = NULL;
-	uint32_t reg_ebx;
+	u32 ret, param;
+	u8 sub_command;
+	int node = smi_apmc_find_state_save_node(APM_CNT_ELOG_GSMI);
+	const struct smm_save_state_ops *ops = get_save_state_ops();
-	io_smi = find_save_state(save_state_ops, APM_CNT_ELOG_GSMI);
-	if (!io_smi)
+	if (node < -1)
    	return;
+
    /* Command and return value in EAX */
-	sub_command = (save_state_ops->get_reg(io_smi, RAX) >> 8)
-		& 0xff;
+	uint64_t reg;
+	if (ops->get_reg(node, RAX, &reg))
+		return;
+	sub_command = (u8)(reg >> 8);
/* Parameter buffer in EBX */
-	reg_ebx = save_state_ops->get_reg(io_smi, RBX);
+	if (ops->get_reg(node, RBX, &reg))
+		return;
+	param = reg & UINT32_MAX;
/* drivers/elog/gsmi.c */
-	ret = gsmi_exec(sub_command, &reg_ebx);
-	save_state_ops->set_reg(io_smi, RAX, ret);
+	ret = gsmi_exec(sub_command, (u32 *)param);
+	ops->set_reg(node, RAX, reg);
 }
-static void southbridge_smi_store(
-	const struct smm_save_state_ops *save_state_ops)
+static void southbridge_smi_store(void)
 {
    u8 sub_command, ret;
-	void *io_smi;
-	uint32_t reg_ebx;
+	void *param;
+	int node = smi_apmc_find_state_save_node(APM_CNT_ELOG_GSMI);
+	const struct smm_save_state_ops *ops = get_save_state_ops();
-	io_smi = find_save_state(save_state_ops, APM_CNT_SMMSTORE);
-	if (!io_smi)
+	if (node < -1)
    	return;
+
    /* Command and return value in EAX */
-	sub_command = (save_state_ops->get_reg(io_smi, RAX) >> 8) & 0xff;
+	uint64_t reg;
+	if (ops->get_reg(node, RAX, &reg))
+		return;
+	sub_command = (u8)(reg >> 8);
/* Parameter buffer in EBX */
-	reg_ebx = save_state_ops->get_reg(io_smi, RBX);
+	if (ops->get_reg(node, RBX, &reg))
+		return;
+	param = (void *)(uint32_t)reg;
/* drivers/smmstore/smi.c */
-	ret = smmstore_exec(sub_command, (void *)reg_ebx);
-	save_state_ops->set_reg(io_smi, RAX, ret);
+	ret = smmstore_exec(sub_command, param);
+	ops->set_reg(node, RAX, ret);
 }
static void finalize(void)
@@ -335,11 +349,10 @@
    smihandler_soc_at_finalize();
 }
-void smihandler_southbridge_apmc(
-	const struct smm_save_state_ops *save_state_ops)
+void smihandler_southbridge_apmc(void)
 {
    uint8_t reg8;
-	void *state = NULL;
+	const struct smm_save_state_ops *ops = get_save_state_ops();
    static int smm_initialized = 0;
/* Emulate B2 register as the FADT / Linux expects it */
@@ -376,22 +389,26 @@
    		       "SMI#: SMM structures already initialized!\n");
    		return;
    	}
-		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_t *)(uintptr_t)reg_ebx;
-			smm_initialized = 1;
-			printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
-		}
+		int node = smi_apmc_find_state_save_node(APM_CNT_GNVS_UPDATE);
+		if (node < -1)
+			return;
+
+		uint64_t rbx;
+		if (ops->get_reg(node, RBX, &rbx))
+			return;
+
+		/* EBX in the state save contains the GNVS pointer */
+		gnvs = (global_nvs_t *)(u32)rbx;
+		smm_initialized = 1;
+		printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
    	break;
    case APM_CNT_ELOG_GSMI:
    	if (CONFIG(ELOG_GSMI))
-			southbridge_smi_gsmi(save_state_ops);
+			southbridge_smi_gsmi();
    	break;
    case APM_CNT_SMMSTORE:
    	if (CONFIG(SMMSTORE))
-			southbridge_smi_store(save_state_ops);
+			southbridge_smi_store();
    	break;
    case APM_CNT_FINALIZE:
    	finalize();
@@ -401,8 +418,7 @@
    mainboard_smi_apmc(reg8);
 }
-void smihandler_southbridge_pm1(
-	const struct smm_save_state_ops *save_state_ops)
+void smihandler_southbridge_pm1(void)
 {
    uint16_t pm1_sts = pmc_clear_pm1_status();
    u16 pm1_en = pmc_read_pm1_enable();
@@ -419,14 +435,12 @@
    }
 }
-void smihandler_southbridge_gpe0(
-	const struct smm_save_state_ops *save_state_ops)
+void smihandler_southbridge_gpe0(void)
 {
    pmc_clear_all_gpe_status();
 }
-void smihandler_southbridge_tco(
-	const struct smm_save_state_ops *save_state_ops)
+void smihandler_southbridge_tco(void)
 {
    uint32_t tco_sts = pmc_clear_tco_status();
@@ -442,8 +456,7 @@
    }
 }
-void smihandler_southbridge_periodic(
-	const struct smm_save_state_ops *save_state_ops)
+void smihandler_southbridge_periodic(void)
 {
    uint32_t reg32;
@@ -455,8 +468,7 @@
    printk(BIOS_DEBUG, "Periodic SMI.\n");
 }
-void smihandler_southbridge_gpi(
-	const struct smm_save_state_ops *save_state_ops)
+void smihandler_southbridge_gpi(void)
 {
    struct gpi_status smi_sts;
@@ -467,8 +479,7 @@
    gpi_clear_get_smi_status(&smi_sts);
 }
-void smihandler_southbridge_espi(
-	const struct smm_save_state_ops *save_state_ops)
+void smihandler_southbridge_espi(void)
 {
    mainboard_smi_espi_handler();
 }
@@ -477,7 +488,6 @@
 {
    int i;
    uint32_t smi_sts;
-	const struct smm_save_state_ops *save_state_ops;
/*
     * We need to clear the SMI status registers, or we won't see what's
@@ -496,15 +506,13 @@
    if (!smi_sts)
    	return;
-	save_state_ops = get_smm_save_state_ops();
-
    /* Call SMI sub handler for each of the status bits */
    for (i = 0; i < ARRAY_SIZE(southbridge_smi); i++) {
    	if (!(smi_sts & (1 << i)))
    		continue;
if (southbridge_smi[i] != NULL) {
-			southbridge_smi[i](save_state_ops);
+			southbridge_smi[i]();
    	} else {
    		printk(BIOS_DEBUG,
    		       "SMI_STS[%d] occurred, but no "
@@ -512,119 +520,3 @@
    	}
    }
 }
-
-static uint32_t em64t100_smm_save_state_get_io_misc_info(void *state)
-{
-	em64t100_smm_state_save_area_t *smm_state = state;
-	return smm_state->io_misc_info;
-}
-
-static uint64_t em64t100_smm_save_state_get_reg(void *state, enum smm_reg reg)
-{
-	uintptr_t value = 0;
-	em64t100_smm_state_save_area_t *smm_state = state;
-
-	switch (reg) {
-	case RAX:
-		value = smm_state->rax;
-		break;
-	case RBX:
-		value = smm_state->rbx;
-		break;
-	case RCX:
-		value = smm_state->rcx;
-		break;
-	case RDX:
-		value = smm_state->rdx;
-		break;
-	default:
-		break;
-	}
-	return value;
-}
-
-static void em64t100_smm_save_state_set_reg(void *state, enum smm_reg reg,
-	uint64_t val)
-{
-	em64t100_smm_state_save_area_t *smm_state = state;
-	switch (reg) {
-	case RAX:
-		smm_state->rax = val;
-		break;
-	case RBX:
-		smm_state->rbx = val;
-		break;
-	case RCX:
-		smm_state->rcx = val;
-		break;
-	case RDX:
-		smm_state->rdx = val;
-		break;
-	default:
-		break;
-	}
-}
-
-static uint32_t em64t101_smm_save_state_get_io_misc_info(void *state)
-{
-	em64t101_smm_state_save_area_t *smm_state = state;
-	return smm_state->io_misc_info;
-}
-
-static uint64_t em64t101_smm_save_state_get_reg(void *state, enum smm_reg reg)
-{
-	uintptr_t value = 0;
-	em64t101_smm_state_save_area_t *smm_state = state;
-
-	switch (reg) {
-	case RAX:
-		value = smm_state->rax;
-		break;
-	case RBX:
-		value = smm_state->rbx;
-		break;
-	case RCX:
-		value = smm_state->rcx;
-		break;
-	case RDX:
-		value = smm_state->rdx;
-		break;
-	default:
-		break;
-	}
-	return value;
-}
-
-static void em64t101_smm_save_state_set_reg(void *state, enum smm_reg reg,
-	uint64_t val)
-{
-	em64t101_smm_state_save_area_t *smm_state = state;
-	switch (reg) {
-	case RAX:
-		smm_state->rax = val;
-		break;
-	case RBX:
-		smm_state->rbx = val;
-		break;
-	case RCX:
-		smm_state->rcx = val;
-		break;
-	case RDX:
-		smm_state->rdx = val;
-		break;
-	default:
-		break;
-	}
-}
-
-const struct smm_save_state_ops em64t100_smm_ops = {
-	.get_io_misc_info = em64t100_smm_save_state_get_io_misc_info,
-	.get_reg = em64t100_smm_save_state_get_reg,
-	.set_reg = em64t100_smm_save_state_set_reg,
-};
-
-const struct smm_save_state_ops em64t101_smm_ops = {
-	.get_io_misc_info = em64t101_smm_save_state_get_io_misc_info,
-	.get_reg = em64t101_smm_save_state_get_reg,
-	.set_reg = em64t101_smm_save_state_set_reg,
-};
diff --git a/src/soc/intel/common/block/smm/smitraphandler.c b/src/soc/intel/common/block/smm/smitraphandler.c
index 974c489..3652cde 100644
--- a/src/soc/intel/common/block/smm/smitraphandler.c
+++ b/src/soc/intel/common/block/smm/smitraphandler.c
@@ -55,8 +55,7 @@
    return 0;
 }
-void smihandler_southbridge_mc(
-	const struct smm_save_state_ops *save_state_ops)
+void smihandler_southbridge_mc(void)
 {
    u32 reg32 = inl(ACPI_BASE_ADDRESS + SMI_EN);
@@ -67,8 +66,7 @@
    printk(BIOS_DEBUG, "Microcontroller SMI.\n");
 }
-void smihandler_southbridge_monitor(
-	const struct smm_save_state_ops *save_state_ops)
+void smihandler_southbridge_monitor(void)
 {
 #define IOTRAP(x) (trap_sts & (1 << x))
    u32 trap_cycle;
diff --git a/src/soc/intel/icelake/smihandler.c b/src/soc/intel/icelake/smihandler.c
index b7c37d4..23e0834 100644
--- a/src/soc/intel/icelake/smihandler.c
+++ b/src/soc/intel/icelake/smihandler.c
@@ -29,11 +29,6 @@
 #define CSME0_BAR	0x0
 #define CSME0_FID	0xb0
-const struct smm_save_state_ops *get_smm_save_state_ops(void)
-{
-	return &em64t101_smm_ops;
-}
-
 static void pch_disable_heci(void)
 {
    struct pcr_sbi_msg msg = {
diff --git a/src/soc/intel/skylake/smihandler.c b/src/soc/intel/skylake/smihandler.c
index 2e93075..5d01bae 100644
--- a/src/soc/intel/skylake/smihandler.c
+++ b/src/soc/intel/skylake/smihandler.c
@@ -20,11 +20,6 @@
 #include <intelblocks/smihandler.h>
 #include <soc/pm.h>
-const struct smm_save_state_ops *get_smm_save_state_ops(void)
-{
-	return &em64t101_smm_ops;
-}
-
 void smihandler_soc_check_illegal_access(uint32_t tco_sts)
 {
    if (!((tco_sts & (1 << 8)) && CONFIG(SPI_FLASH_SMM)
diff --git a/src/soc/intel/tigerlake/smihandler.c b/src/soc/intel/tigerlake/smihandler.c
index bf07bea..cbc37f8 100644
--- a/src/soc/intel/tigerlake/smihandler.c
+++ b/src/soc/intel/tigerlake/smihandler.c
@@ -29,11 +29,6 @@
 #define CSME0_BAR	0x0
 #define CSME0_FID	0xb0
-const struct smm_save_state_ops *get_smm_save_state_ops(void)
-{
-	return &em64t101_smm_ops;
-}
-
 static void pch_disable_heci(void)
 {
    struct pcr_sbi_msg msg = {
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/37015
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

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



    