Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu.

Shuo Liu has uploaded this change for review.

View Change

soc/intel/xeon_sp: Remove usage of VPD in SoC codes

Configuration variable implementation (VPD, et al) is regarded to
be mainboard specific and should not be bounded to SoC codes.
Remove the usage of VPD in SoC codes and move the logics to
mainboard codes accordingly.

Change-Id: Iefea72eec6e52f8d1ae2d10e1edbabdebf4dff91
Signed-off-by: Shuo Liu <shuo.liu@intel.com>
---
M src/mainboard/intel/archercity_crb/Makefile.mk
M src/mainboard/intel/archercity_crb/romstage.c
A src/mainboard/intel/archercity_crb/util.c
M src/mainboard/inventec/transformers/Makefile.mk
M src/mainboard/inventec/transformers/romstage.c
A src/mainboard/inventec/transformers/util.c
M src/soc/intel/xeon_sp/include/soc/chip_common.h
M src/soc/intel/xeon_sp/include/soc/util.h
M src/soc/intel/xeon_sp/spr/romstage.c
M src/soc/intel/xeon_sp/uncore.c
M src/soc/intel/xeon_sp/util.c
11 files changed, 106 insertions(+), 103 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/82090/1
diff --git a/src/mainboard/intel/archercity_crb/Makefile.mk b/src/mainboard/intel/archercity_crb/Makefile.mk
index 4c7a7be..b28d73c 100644
--- a/src/mainboard/intel/archercity_crb/Makefile.mk
+++ b/src/mainboard/intel/archercity_crb/Makefile.mk
@@ -2,5 +2,7 @@

bootblock-y += bootblock.c
romstage-y += romstage.c
+romstage-y += util.c
ramstage-y += ramstage.c
+ramstage-y += util.c
CPPFLAGS_common += -I$(src)/mainboard/$(MAINBOARDDIR)/include
diff --git a/src/mainboard/intel/archercity_crb/romstage.c b/src/mainboard/intel/archercity_crb/romstage.c
index 6e4bd8e..ff56f59 100644
--- a/src/mainboard/intel/archercity_crb/romstage.c
+++ b/src/mainboard/intel/archercity_crb/romstage.c
@@ -35,17 +35,23 @@

void mainboard_memory_init_params(FSPM_UPD *mupd)
{
- uint8_t val;
-
- /* Send FSP log message to SOL */
- if (CONFIG(VPD) && vpd_get_bool(FSP_LOG, VPD_RW_THEN_RO, &val))
- mupd->FspmConfig.SerialIoUartDebugEnable = val;
- else {
- printk(BIOS_INFO, "Not able to get VPD %s, default set SerialIoUartDebugEnable to %d\n",
- FSP_LOG, FSP_LOG_DEFAULT);
- mupd->FspmConfig.SerialIoUartDebugEnable = FSP_LOG_DEFAULT;
+ /* Setup FSP log */
+ mupd->FspmConfig.SerialIoUartDebugEnable = get_bool_from_vpd(FSP_LOG,
+ FSP_LOG_DEFAULT);
+ if (mupd->FspmConfig.SerialIoUartDebugEnable) {
+ mupd->FspmConfig.serialDebugMsgLvl = get_int_from_vpd_range(
+ FSP_MEM_LOG_LEVEL, FSP_MEM_LOG_LEVEL_DEFAULT, 0, 4);
+ /* If serialDebugMsgLvl less than 1, disable FSP memory train results */
+ if (mupd->FspmConfig.serialDebugMsgLvl <= 1) {
+ printk(BIOS_DEBUG, "Setting serialDebugMsgLvlTrainResults to 0\n");
+ mupd->FspmConfig.serialDebugMsgLvlTrainResults = 0x0;
+ }
}

+ /* FSP Dfx PMIC Secure mode */
+ mupd->FspmConfig.DfxPmicSecureMode = get_int_from_vpd_range(
+ FSP_PMIC_SECURE_MODE, FSP_PMIC_SECURE_MODE_DEFAULT, 0, 2);
+
/* Set Rank Margin Tool to disable. */
mupd->FspmConfig.EnableRMT = 0x0;
/* Enable - Portions of memory reference code will be skipped
diff --git a/src/mainboard/intel/archercity_crb/util.c b/src/mainboard/intel/archercity_crb/util.c
new file mode 100644
index 0000000..b371fd0
--- /dev/null
+++ b/src/mainboard/intel/archercity_crb/util.c
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <drivers/ocp/include/vpd.h>
+#include <soc/chip_common.h>
+#include <soc/util.h>
+
+#if CONFIG(SOC_INTEL_HAS_CXL)
+int get_cxl_mode(void)
+{
+ int ocp_cxl_mode = get_cxl_mode_from_vpd();
+ switch (ocp_cxl_mode) {
+ case CXL_SYSTEM_MEMORY:
+ return XEONSP_CXL_SYS_MEM;
+ case CXL_SPM:
+ return XEONSP_CXL_SP_MEM;
+ default:
+ return XEONSP_CXL_DISABLED;
+ }
+}
+#endif
diff --git a/src/mainboard/inventec/transformers/Makefile.mk b/src/mainboard/inventec/transformers/Makefile.mk
index ecb6ef2..eb859d3 100644
--- a/src/mainboard/inventec/transformers/Makefile.mk
+++ b/src/mainboard/inventec/transformers/Makefile.mk
@@ -2,5 +2,7 @@

bootblock-y += bootblock.c
romstage-y += romstage.c
+romstage-y += util.c
romstage-$(CONFIG_IPMI_KCS_ROMSTAGE) += ipmi.c
+ramstage-y += util.c
CPPFLAGS_common += -I$(src)/mainboard/$(MAINBOARDDIR)/include
diff --git a/src/mainboard/inventec/transformers/romstage.c b/src/mainboard/inventec/transformers/romstage.c
index 9299c7d..1abcf70 100644
--- a/src/mainboard/inventec/transformers/romstage.c
+++ b/src/mainboard/inventec/transformers/romstage.c
@@ -98,22 +98,28 @@

void mainboard_memory_init_params(FSPM_UPD *mupd)
{
- uint8_t val;
-
/* Since it's the first IPMI command, it's better to run get BMC selftest result first */
if (ipmi_premem_init(CONFIG_BMC_KCS_BASE, 0) == CB_SUCCESS) {
init_frb2_wdt();
}

- /* Send FSP log message to SOL */
- if (CONFIG(VPD) && vpd_get_bool(FSP_LOG, VPD_RW_THEN_RO, &val))
- mupd->FspmConfig.SerialIoUartDebugEnable = val;
- else {
- printk(BIOS_INFO, "Not able to get VPD %s, default set "
- "SerialIoUartDebugEnable to %d\n", FSP_LOG, FSP_LOG_DEFAULT);
- mupd->FspmConfig.SerialIoUartDebugEnable = FSP_LOG_DEFAULT;
+ /* Setup FSP log */
+ mupd->FspmConfig.SerialIoUartDebugEnable = get_bool_from_vpd(FSP_LOG,
+ FSP_LOG_DEFAULT);
+ if (mupd->FspmConfig.SerialIoUartDebugEnable) {
+ mupd->FspmConfig.serialDebugMsgLvl = get_int_from_vpd_range(
+ FSP_MEM_LOG_LEVEL, FSP_MEM_LOG_LEVEL_DEFAULT, 0, 4);
+ /* If serialDebugMsgLvl less than 1, disable FSP memory train results */
+ if (mupd->FspmConfig.serialDebugMsgLvl <= 1) {
+ printk(BIOS_DEBUG, "Setting serialDebugMsgLvlTrainResults to 0\n");
+ mupd->FspmConfig.serialDebugMsgLvlTrainResults = 0x0;
+ }
}

+ /* FSP Dfx PMIC Secure mode */
+ mupd->FspmConfig.DfxPmicSecureMode = get_int_from_vpd_range(
+ FSP_PMIC_SECURE_MODE, FSP_PMIC_SECURE_MODE_DEFAULT, 0, 2);
+
/* Set Rank Margin Tool to disable. */
mupd->FspmConfig.EnableRMT = 0x0;
/* Enable - Portions of memory reference code will be skipped when possible to increase boot speed on warm boots */
diff --git a/src/mainboard/inventec/transformers/util.c b/src/mainboard/inventec/transformers/util.c
new file mode 100644
index 0000000..b371fd0
--- /dev/null
+++ b/src/mainboard/inventec/transformers/util.c
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <drivers/ocp/include/vpd.h>
+#include <soc/chip_common.h>
+#include <soc/util.h>
+
+#if CONFIG(SOC_INTEL_HAS_CXL)
+int get_cxl_mode(void)
+{
+ int ocp_cxl_mode = get_cxl_mode_from_vpd();
+ switch (ocp_cxl_mode) {
+ case CXL_SYSTEM_MEMORY:
+ return XEONSP_CXL_SYS_MEM;
+ case CXL_SPM:
+ return XEONSP_CXL_SP_MEM;
+ default:
+ return XEONSP_CXL_DISABLED;
+ }
+}
+#endif
diff --git a/src/soc/intel/xeon_sp/include/soc/chip_common.h b/src/soc/intel/xeon_sp/include/soc/chip_common.h
index 5fd5dc6..54b5069 100644
--- a/src/soc/intel/xeon_sp/include/soc/chip_common.h
+++ b/src/soc/intel/xeon_sp/include/soc/chip_common.h
@@ -33,6 +33,10 @@
path->domain.domain = dp.domain_path;
};

+#define XEONSP_CXL_DISABLED 0
+#define XEONSP_CXL_SYS_MEM 1
+#define XEONSP_CXL_SP_MEM 2
+
/*
* Every STACK can have multiple PCI domains with an unique domain type.
* This is only of cosmetic nature and generates more readable ACPI code,
diff --git a/src/soc/intel/xeon_sp/include/soc/util.h b/src/soc/intel/xeon_sp/include/soc/util.h
index 177d6d5..3ad6683 100644
--- a/src/soc/intel/xeon_sp/include/soc/util.h
+++ b/src/soc/intel/xeon_sp/include/soc/util.h
@@ -28,4 +28,6 @@
bool is_iio_cxl_stack_res(const xSTACK_RES *res);
void bios_done_msr(void *unused);

+int get_cxl_mode(void);
+
#endif
diff --git a/src/soc/intel/xeon_sp/spr/romstage.c b/src/soc/intel/xeon_sp/spr/romstage.c
index 3e16608..bdb9886 100644
--- a/src/soc/intel/xeon_sp/spr/romstage.c
+++ b/src/soc/intel/xeon_sp/spr/romstage.c
@@ -13,12 +13,14 @@
#include <fsp/util.h>
#include <hob_iiouds.h>
#include <hob_memmap.h>
+#include <soc/chip_common.h>
#include <soc/romstage.h>
#include <soc/pci_devs.h>
#include <soc/soc_pch.h>
#include <soc/intel/common/smbios.h>
#include <string.h>
#include <soc/soc_util.h>
+#include <soc/util.h>
#include <soc/ddr.h>

#include "chip.h"
@@ -32,73 +34,10 @@
/* Default weak implementation */
}

-/*
- * Search from VPD_RW first then VPD_RO for UPD config variables,
- * overwrites them from VPD if it's found.
- */
-static void config_upd_from_vpd(FSPM_UPD *mupd)
+static void config_upd(FSPM_UPD *mupd)
{
- uint8_t val;
- int val_int, cxl_mode;
-
- /* Send FSP log message to SOL */
- if (vpd_get_bool(FSP_LOG, VPD_RW_THEN_RO, &val))
- mupd->FspmConfig.SerialIoUartDebugEnable = val;
- else {
- printk(BIOS_INFO,
- "Not able to get VPD %s, default set "
- "SerialIoUartDebugEnable to %d\n",
- FSP_LOG, FSP_LOG_DEFAULT);
- mupd->FspmConfig.SerialIoUartDebugEnable = FSP_LOG_DEFAULT;
- }
-
- if (mupd->FspmConfig.SerialIoUartDebugEnable) {
- /* FSP memory debug log level */
- if (vpd_get_int(FSP_MEM_LOG_LEVEL, VPD_RW_THEN_RO, &val_int)) {
- if (val_int < 0 || val_int > 4) {
- printk(BIOS_DEBUG,
- "Invalid serialDebugMsgLvl value from VPD: "
- "%d\n",
- val_int);
- val_int = FSP_MEM_LOG_LEVEL_DEFAULT;
- }
- printk(BIOS_DEBUG, "Setting serialDebugMsgLvl to %d\n", val_int);
- mupd->FspmConfig.serialDebugMsgLvl = (uint8_t)val_int;
- } else {
- printk(BIOS_INFO,
- "Not able to get VPD %s, default set "
- "DebugPrintLevel to %d\n",
- FSP_MEM_LOG_LEVEL, FSP_MEM_LOG_LEVEL_DEFAULT);
- mupd->FspmConfig.serialDebugMsgLvl = FSP_MEM_LOG_LEVEL_DEFAULT;
- }
- /* If serialDebugMsgLvl less than 1, disable FSP memory train results */
- if (mupd->FspmConfig.serialDebugMsgLvl <= 1) {
- printk(BIOS_DEBUG, "Setting serialDebugMsgLvlTrainResults to 0\n");
- mupd->FspmConfig.serialDebugMsgLvlTrainResults = 0x0;
- }
- }
-
- /* FSP Dfx PMIC Secure mode */
- if (vpd_get_int(FSP_PMIC_SECURE_MODE, VPD_RW_THEN_RO, &val_int)) {
- if (val_int < 0 || val_int > 2) {
- printk(BIOS_DEBUG,
- "Invalid PMIC secure mode value from VPD: "
- "%d\n",
- val_int);
- val_int = FSP_PMIC_SECURE_MODE_DEFAULT;
- }
- printk(BIOS_DEBUG, "Setting PMIC secure mode to %d\n", val_int);
- mupd->FspmConfig.DfxPmicSecureMode = (uint8_t)val_int;
- } else {
- printk(BIOS_INFO,
- "Not able to get VPD %s, default set "
- "PMIC secure mode to %d\n",
- FSP_PMIC_SECURE_MODE, FSP_PMIC_SECURE_MODE_DEFAULT);
- mupd->FspmConfig.DfxPmicSecureMode = FSP_PMIC_SECURE_MODE_DEFAULT;
- }
-
- cxl_mode = get_cxl_mode_from_vpd();
- if (cxl_mode == CXL_SYSTEM_MEMORY || cxl_mode == CXL_SPM)
+ int cxl_mode = get_cxl_mode();
+ if (cxl_mode == XEONSP_CXL_SYS_MEM || cxl_mode == XEONSP_CXL_SP_MEM)
mupd->FspmConfig.DfxCxlType3LegacyEn = 1;
else /* Disable CXL */
mupd->FspmConfig.DfxCxlType3LegacyEn = 0;
@@ -272,9 +211,8 @@
printk(BIOS_DEBUG, "CPU is D stepping, setting package C state to C0/C1\n");
mupd->FspmConfig.CpuPmPackageCState = 0;
}
- /* Set some common UPDs from VPD, mainboard can still override them if needed */
- if (CONFIG(VPD))
- config_upd_from_vpd(mupd);
+
+ config_upd(mupd);
initialize_iio_upd(mupd);
mainboard_memory_init_params(mupd);

diff --git a/src/soc/intel/xeon_sp/uncore.c b/src/soc/intel/xeon_sp/uncore.c
index a177a89..c5d1d78 100644
--- a/src/soc/intel/xeon_sp/uncore.c
+++ b/src/soc/intel/xeon_sp/uncore.c
@@ -6,8 +6,8 @@
#include <cpu/x86/lapic_def.h>
#include <device/pci.h>
#include <device/pci_ids.h>
-#include <drivers/ocp/include/vpd.h>
#include <soc/acpi.h>
+#include <soc/chip_common.h>
#include <soc/iomap.h>
#include <soc/pci_devs.h>
#include <soc/ramstage.h>
@@ -286,22 +286,20 @@
if (pds.pds[i].pd_type == PD_TYPE_PROCESSOR)
continue;

- if (CONFIG(OCP_VPD)) {
- unsigned long flags = IORESOURCE_CACHEABLE;
- int cxl_mode = get_cxl_mode_from_vpd();
- if (cxl_mode == CXL_SPM)
- flags |= IORESOURCE_SOFT_RESERVE;
- else
- flags |= IORESOURCE_STORED;
+ unsigned long flags = IORESOURCE_CACHEABLE;
+ int cxl_mode = get_cxl_mode();
+ if (cxl_mode == XEONSP_CXL_SP_MEM)
+ flags |= IORESOURCE_SOFT_RESERVE;
+ else
+ flags |= IORESOURCE_STORED;

- res = fixed_mem_range_flags(dev, index++,
- (uint64_t)pds.pds[i].base << 26,
- (uint64_t)pds.pds[i].size << 26, flags);
- if (cxl_mode == CXL_SPM)
- LOG_RESOURCE("specific_purpose_memory", dev, res);
- else
- LOG_RESOURCE("CXL_memory", dev, res);
- }
+ res = fixed_mem_range_flags(dev, index++,
+ (uint64_t)pds.pds[i].base << 26,
+ (uint64_t)pds.pds[i].size << 26, flags);
+ if (cxl_mode == XEONSP_CXL_SP_MEM)
+ LOG_RESOURCE("specific_purpose_memory", dev, res);
+ else
+ LOG_RESOURCE("CXL_memory", dev, res);
}
} else {
/* 4GiB -> TOHM */
diff --git a/src/soc/intel/xeon_sp/util.c b/src/soc/intel/xeon_sp/util.c
index 81dc77d..2556969 100644
--- a/src/soc/intel/xeon_sp/util.c
+++ b/src/soc/intel/xeon_sp/util.c
@@ -236,4 +236,9 @@
/* And finally, take care of the SBSP */
set_bios_init_completion_for_package(sbsp_socket_id);
}
+
+__weak int get_cxl_mode(void)
+{
+ return XEONSP_CXL_DISABLED;
+}
#endif

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

Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iefea72eec6e52f8d1ae2d10e1edbabdebf4dff91
Gerrit-Change-Number: 82090
Gerrit-PatchSet: 1
Gerrit-Owner: Shuo Liu <shuo.liu@intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter@9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin@wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang@gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu@quantatw.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang@gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin@wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter@9elements.com>
Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan@9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu@quantatw.com>
Gerrit-MessageType: newchange