Marc Jones has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46479 )
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
soc/intel/xeon_sp: Move read_msr_ppin() to common util.c
Move CPX and SKX read_msr_ppin() to common util.c file. Update the drive/ocp/smbios #include to match.
Change-Id: I4c4281d2d5ce679f5444a502fa88df04de9f2cd8 Signed-off-by: Marc Jones marcjones@sysproconsulting.com --- M src/drivers/ocp/dmi/smbios.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/cpx/include/soc/cpu.h M src/soc/intel/xeon_sp/include/soc/util.h M src/soc/intel/xeon_sp/skx/cpu.c M src/soc/intel/xeon_sp/skx/include/soc/cpu.h M src/soc/intel/xeon_sp/util.c 7 files changed, 35 insertions(+), 65 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/46479/1
diff --git a/src/drivers/ocp/dmi/smbios.c b/src/drivers/ocp/dmi/smbios.c index 4e54af0..d0ef115 100644 --- a/src/drivers/ocp/dmi/smbios.c +++ b/src/drivers/ocp/dmi/smbios.c @@ -9,7 +9,7 @@ #include <stdio.h> #include <string.h> #include <soc/soc_util.h> -#include <soc/cpu.h> +#include <soc/util.h> #include <smbios.h>
#include "ocp_dmi.h" diff --git a/src/soc/intel/xeon_sp/cpx/cpu.c b/src/soc/intel/xeon_sp/cpx/cpu.c index 4afe47c..5bde819 100644 --- a/src/soc/intel/xeon_sp/cpx/cpu.c +++ b/src/soc/intel/xeon_sp/cpx/cpu.c @@ -205,34 +205,3 @@ /* update numa domain for all cpu devices */ xeonsp_init_cpu_config(); } - -msr_t read_msr_ppin(void) -{ - msr_t ppin = {0}; - msr_t msr; - - /* If MSR_PLATFORM_INFO PPIN_CAP is 0, PPIN capability is not supported */ - msr = rdmsr(MSR_PLATFORM_INFO); - if ((msr.lo & MSR_PPIN_CAP) == 0) { - printk(BIOS_ERR, "MSR_PPIN_CAP is 0, PPIN is not supported\n"); - return ppin; - } - - /* Access to MSR_PPIN is permitted only if MSR_PPIN_CTL LOCK is 0 and ENABLE is 1 */ - msr = rdmsr(MSR_PPIN_CTL); - if (msr.lo & MSR_PPIN_CTL_LOCK) { - printk(BIOS_ERR, "MSR_PPIN_CTL_LOCK is 1, PPIN access is not allowed\n"); - return ppin; - } - - if ((msr.lo & MSR_PPIN_CTL_ENABLE) == 0) { - /* Set MSR_PPIN_CTL ENABLE to 1 */ - msr.lo |= MSR_PPIN_CTL_ENABLE; - wrmsr(MSR_PPIN_CTL, msr); - } - ppin = rdmsr(MSR_PPIN); - /* Set enable to 0 after reading MSR_PPIN */ - msr.lo &= ~MSR_PPIN_CTL_ENABLE; - wrmsr(MSR_PPIN_CTL, msr); - return ppin; -} diff --git a/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h b/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h index 19f6e4c..693de8f 100644 --- a/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h +++ b/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h @@ -10,6 +10,5 @@ #define CPUID_COOPERLAKE_SP_A1 0x05065b
void cpx_init_cpus(struct device *dev); -msr_t read_msr_ppin(void);
#endif diff --git a/src/soc/intel/xeon_sp/include/soc/util.h b/src/soc/intel/xeon_sp/include/soc/util.h index 8c2b597..51e2b69 100644 --- a/src/soc/intel/xeon_sp/include/soc/util.h +++ b/src/soc/intel/xeon_sp/include/soc/util.h @@ -3,10 +3,12 @@ #ifndef _XEON_SP_SOC_UTIL_H_ #define _XEON_SP_SOC_UTIL_H_
+#include <cpu/x86/msr.h> #include <hob_iiouds.h>
void get_cpubusnos(uint32_t *bus0, uint32_t *bus1, uint32_t *bus2, uint32_t *bus3); void unlock_pam_regions(void); void get_stack_busnos(uint32_t *bus); +msr_t read_msr_ppin(void);
#endif diff --git a/src/soc/intel/xeon_sp/skx/cpu.c b/src/soc/intel/xeon_sp/skx/cpu.c index 73d7a73..1a11a42 100644 --- a/src/soc/intel/xeon_sp/skx/cpu.c +++ b/src/soc/intel/xeon_sp/skx/cpu.c @@ -244,34 +244,3 @@
FUNC_EXIT(); } - -msr_t read_msr_ppin(void) -{ - msr_t ppin = {0}; - msr_t msr; - - /* If MSR_PLATFORM_INFO PPIN_CAP is 0, PPIN capability is not supported */ - msr = rdmsr(MSR_PLATFORM_INFO); - if ((msr.lo & MSR_PPIN_CAP) == 0) { - printk(BIOS_ERR, "MSR_PPIN_CAP is 0, PPIN is not supported\n"); - return ppin; - } - - /* Access to MSR_PPIN is permitted only if MSR_PPIN_CTL LOCK is 0 and ENABLE is 1 */ - msr = rdmsr(MSR_PPIN_CTL); - if (msr.lo & MSR_PPIN_CTL_LOCK) { - printk(BIOS_ERR, "MSR_PPIN_CTL_LOCK is 1, PPIN access is not allowed\n"); - return ppin; - } - - if ((msr.lo & MSR_PPIN_CTL_ENABLE) == 0) { - /* Set MSR_PPIN_CTL ENABLE to 1 */ - msr.lo |= MSR_PPIN_CTL_ENABLE; - wrmsr(MSR_PPIN_CTL, msr); - } - ppin = rdmsr(MSR_PPIN); - /* Set enable to 0 after reading MSR_PPIN */ - msr.lo &= ~MSR_PPIN_CTL_ENABLE; - wrmsr(MSR_PPIN_CTL, msr); - return ppin; -} diff --git a/src/soc/intel/xeon_sp/skx/include/soc/cpu.h b/src/soc/intel/xeon_sp/skx/include/soc/cpu.h index c2af265..0e3028d 100644 --- a/src/soc/intel/xeon_sp/skx/include/soc/cpu.h +++ b/src/soc/intel/xeon_sp/skx/include/soc/cpu.h @@ -16,6 +16,5 @@
int get_cpu_count(void); void xeon_sp_init_cpus(struct device *dev); -msr_t read_msr_ppin(void);
#endif diff --git a/src/soc/intel/xeon_sp/util.c b/src/soc/intel/xeon_sp/util.c index 77fc1e4..66b9ef1 100644 --- a/src/soc/intel/xeon_sp/util.c +++ b/src/soc/intel/xeon_sp/util.c @@ -3,6 +3,7 @@ #include <console/console.h> #include <device/pci.h> #include <soc/pci_devs.h> +#include <soc/msr.h> #include <soc/util.h>
void get_stack_busnos(uint32_t *bus) @@ -53,3 +54,34 @@ if (bus3) *bus3 = (bus >> 24) & 0xff; } + +msr_t read_msr_ppin(void) +{ + msr_t ppin = {0}; + msr_t msr; + + /* If MSR_PLATFORM_INFO PPIN_CAP is 0, PPIN capability is not supported */ + msr = rdmsr(MSR_PLATFORM_INFO); + if ((msr.lo & MSR_PPIN_CAP) == 0) { + printk(BIOS_ERR, "MSR_PPIN_CAP is 0, PPIN is not supported\n"); + return ppin; + } + + /* Access to MSR_PPIN is permitted only if MSR_PPIN_CTL LOCK is 0 and ENABLE is 1 */ + msr = rdmsr(MSR_PPIN_CTL); + if (msr.lo & MSR_PPIN_CTL_LOCK) { + printk(BIOS_ERR, "MSR_PPIN_CTL_LOCK is 1, PPIN access is not allowed\n"); + return ppin; + } + + if ((msr.lo & MSR_PPIN_CTL_ENABLE) == 0) { + /* Set MSR_PPIN_CTL ENABLE to 1 */ + msr.lo |= MSR_PPIN_CTL_ENABLE; + wrmsr(MSR_PPIN_CTL, msr); + } + ppin = rdmsr(MSR_PPIN); + /* Set enable to 0 after reading MSR_PPIN */ + msr.lo &= ~MSR_PPIN_CTL_ENABLE; + wrmsr(MSR_PPIN_CTL, msr); + return ppin; +}
Jay Talbott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46479 )
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
Patch Set 1: Code-Review+1
Hello build bot (Jenkins), Jonathan Zhang, Jay Talbott, Stefan Reinauer, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46479
to look at the new patch set (#2).
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
soc/intel/xeon_sp: Move read_msr_ppin() to common util.c
Move CPX and SKX read_msr_ppin() to common util.c file. Update drivers/ocp/smbios #include to match.
Change-Id: I4c4281d2d5ce679f5444a502fa88df04de9f2cd8 Signed-off-by: Marc Jones marcjones@sysproconsulting.com --- M src/drivers/ocp/dmi/smbios.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/cpx/include/soc/cpu.h M src/soc/intel/xeon_sp/include/soc/util.h M src/soc/intel/xeon_sp/skx/cpu.c M src/soc/intel/xeon_sp/skx/include/soc/cpu.h M src/soc/intel/xeon_sp/util.c 7 files changed, 35 insertions(+), 65 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/46479/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46479 )
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
Patch Set 2: Code-Review+2
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46479 )
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
Patch Set 2: Code-Review+2
Jay Talbott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46479 )
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
Patch Set 2: Code-Review+1
Hello build bot (Jenkins), Jonathan Zhang, Jay Talbott, Stefan Reinauer, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46479
to look at the new patch set (#3).
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
soc/intel/xeon_sp: Move read_msr_ppin() to common util.c
Move CPX and SKX read_msr_ppin() to common util.c file. Update drivers/ocp/smbios #include to match.
Change-Id: I4c4281d2d5ce679f5444a502fa88df04de9f2cd8 Signed-off-by: Marc Jones marcjones@sysproconsulting.com --- M src/drivers/ocp/dmi/smbios.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/cpx/include/soc/cpu.h M src/soc/intel/xeon_sp/include/soc/util.h M src/soc/intel/xeon_sp/skx/cpu.c M src/soc/intel/xeon_sp/skx/include/soc/cpu.h M src/soc/intel/xeon_sp/util.c 7 files changed, 35 insertions(+), 65 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/46479/3
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46479 )
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
Patch Set 3: Code-Review+1
Jay Talbott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46479 )
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
Patch Set 3: Code-Review+1
Hello build bot (Jenkins), Jonathan Zhang, Jay Talbott, Johnny Lin, Stefan Reinauer, Angel Pons, Arthur Heymans, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46479
to look at the new patch set (#4).
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
soc/intel/xeon_sp: Move read_msr_ppin() to common util.c
Move CPX and SKX read_msr_ppin() to common util.c file. Update drivers/ocp/smbios #include to match.
Change-Id: I4c4281d2d5ce679f5444a502fa88df04de9f2cd8 Signed-off-by: Marc Jones marcjones@sysproconsulting.com --- M src/drivers/ocp/dmi/smbios.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/cpx/include/soc/cpu.h M src/soc/intel/xeon_sp/include/soc/util.h M src/soc/intel/xeon_sp/skx/cpu.c M src/soc/intel/xeon_sp/skx/include/soc/cpu.h M src/soc/intel/xeon_sp/util.c 7 files changed, 35 insertions(+), 65 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/46479/4
Jay Talbott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46479 )
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
Patch Set 4: Code-Review+1
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46479 )
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
Patch Set 4: Code-Review+2
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46479 )
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
Patch Set 4: Code-Review+2
Marc Jones has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46479 )
Change subject: soc/intel/xeon_sp: Move read_msr_ppin() to common util.c ......................................................................
soc/intel/xeon_sp: Move read_msr_ppin() to common util.c
Move CPX and SKX read_msr_ppin() to common util.c file. Update drivers/ocp/smbios #include to match.
Change-Id: I4c4281d2d5ce679f5444a502fa88df04de9f2cd8 Signed-off-by: Marc Jones marcjones@sysproconsulting.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46479 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Jay Talbott JayTalbott@sysproconsulting.com Reviewed-by: Arthur Heymans arthur@aheymans.xyz Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org --- M src/drivers/ocp/dmi/smbios.c M src/soc/intel/xeon_sp/cpx/cpu.c M src/soc/intel/xeon_sp/cpx/include/soc/cpu.h M src/soc/intel/xeon_sp/include/soc/util.h M src/soc/intel/xeon_sp/skx/cpu.c M src/soc/intel/xeon_sp/skx/include/soc/cpu.h M src/soc/intel/xeon_sp/util.c 7 files changed, 35 insertions(+), 65 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved Arthur Heymans: Looks good to me, approved Jay Talbott: Looks good to me, but someone else must approve
diff --git a/src/drivers/ocp/dmi/smbios.c b/src/drivers/ocp/dmi/smbios.c index 4e54af0..d0ef115 100644 --- a/src/drivers/ocp/dmi/smbios.c +++ b/src/drivers/ocp/dmi/smbios.c @@ -9,7 +9,7 @@ #include <stdio.h> #include <string.h> #include <soc/soc_util.h> -#include <soc/cpu.h> +#include <soc/util.h> #include <smbios.h>
#include "ocp_dmi.h" diff --git a/src/soc/intel/xeon_sp/cpx/cpu.c b/src/soc/intel/xeon_sp/cpx/cpu.c index 4afe47c..5bde819 100644 --- a/src/soc/intel/xeon_sp/cpx/cpu.c +++ b/src/soc/intel/xeon_sp/cpx/cpu.c @@ -205,34 +205,3 @@ /* update numa domain for all cpu devices */ xeonsp_init_cpu_config(); } - -msr_t read_msr_ppin(void) -{ - msr_t ppin = {0}; - msr_t msr; - - /* If MSR_PLATFORM_INFO PPIN_CAP is 0, PPIN capability is not supported */ - msr = rdmsr(MSR_PLATFORM_INFO); - if ((msr.lo & MSR_PPIN_CAP) == 0) { - printk(BIOS_ERR, "MSR_PPIN_CAP is 0, PPIN is not supported\n"); - return ppin; - } - - /* Access to MSR_PPIN is permitted only if MSR_PPIN_CTL LOCK is 0 and ENABLE is 1 */ - msr = rdmsr(MSR_PPIN_CTL); - if (msr.lo & MSR_PPIN_CTL_LOCK) { - printk(BIOS_ERR, "MSR_PPIN_CTL_LOCK is 1, PPIN access is not allowed\n"); - return ppin; - } - - if ((msr.lo & MSR_PPIN_CTL_ENABLE) == 0) { - /* Set MSR_PPIN_CTL ENABLE to 1 */ - msr.lo |= MSR_PPIN_CTL_ENABLE; - wrmsr(MSR_PPIN_CTL, msr); - } - ppin = rdmsr(MSR_PPIN); - /* Set enable to 0 after reading MSR_PPIN */ - msr.lo &= ~MSR_PPIN_CTL_ENABLE; - wrmsr(MSR_PPIN_CTL, msr); - return ppin; -} diff --git a/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h b/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h index 19f6e4c..693de8f 100644 --- a/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h +++ b/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h @@ -10,6 +10,5 @@ #define CPUID_COOPERLAKE_SP_A1 0x05065b
void cpx_init_cpus(struct device *dev); -msr_t read_msr_ppin(void);
#endif diff --git a/src/soc/intel/xeon_sp/include/soc/util.h b/src/soc/intel/xeon_sp/include/soc/util.h index 8c2b597..51e2b69 100644 --- a/src/soc/intel/xeon_sp/include/soc/util.h +++ b/src/soc/intel/xeon_sp/include/soc/util.h @@ -3,10 +3,12 @@ #ifndef _XEON_SP_SOC_UTIL_H_ #define _XEON_SP_SOC_UTIL_H_
+#include <cpu/x86/msr.h> #include <hob_iiouds.h>
void get_cpubusnos(uint32_t *bus0, uint32_t *bus1, uint32_t *bus2, uint32_t *bus3); void unlock_pam_regions(void); void get_stack_busnos(uint32_t *bus); +msr_t read_msr_ppin(void);
#endif diff --git a/src/soc/intel/xeon_sp/skx/cpu.c b/src/soc/intel/xeon_sp/skx/cpu.c index 581378b..874bcfd 100644 --- a/src/soc/intel/xeon_sp/skx/cpu.c +++ b/src/soc/intel/xeon_sp/skx/cpu.c @@ -245,34 +245,3 @@
FUNC_EXIT(); } - -msr_t read_msr_ppin(void) -{ - msr_t ppin = {0}; - msr_t msr; - - /* If MSR_PLATFORM_INFO PPIN_CAP is 0, PPIN capability is not supported */ - msr = rdmsr(MSR_PLATFORM_INFO); - if ((msr.lo & MSR_PPIN_CAP) == 0) { - printk(BIOS_ERR, "MSR_PPIN_CAP is 0, PPIN is not supported\n"); - return ppin; - } - - /* Access to MSR_PPIN is permitted only if MSR_PPIN_CTL LOCK is 0 and ENABLE is 1 */ - msr = rdmsr(MSR_PPIN_CTL); - if (msr.lo & MSR_PPIN_CTL_LOCK) { - printk(BIOS_ERR, "MSR_PPIN_CTL_LOCK is 1, PPIN access is not allowed\n"); - return ppin; - } - - if ((msr.lo & MSR_PPIN_CTL_ENABLE) == 0) { - /* Set MSR_PPIN_CTL ENABLE to 1 */ - msr.lo |= MSR_PPIN_CTL_ENABLE; - wrmsr(MSR_PPIN_CTL, msr); - } - ppin = rdmsr(MSR_PPIN); - /* Set enable to 0 after reading MSR_PPIN */ - msr.lo &= ~MSR_PPIN_CTL_ENABLE; - wrmsr(MSR_PPIN_CTL, msr); - return ppin; -} diff --git a/src/soc/intel/xeon_sp/skx/include/soc/cpu.h b/src/soc/intel/xeon_sp/skx/include/soc/cpu.h index c2af265..0e3028d 100644 --- a/src/soc/intel/xeon_sp/skx/include/soc/cpu.h +++ b/src/soc/intel/xeon_sp/skx/include/soc/cpu.h @@ -16,6 +16,5 @@
int get_cpu_count(void); void xeon_sp_init_cpus(struct device *dev); -msr_t read_msr_ppin(void);
#endif diff --git a/src/soc/intel/xeon_sp/util.c b/src/soc/intel/xeon_sp/util.c index 77fc1e4..66b9ef1 100644 --- a/src/soc/intel/xeon_sp/util.c +++ b/src/soc/intel/xeon_sp/util.c @@ -3,6 +3,7 @@ #include <console/console.h> #include <device/pci.h> #include <soc/pci_devs.h> +#include <soc/msr.h> #include <soc/util.h>
void get_stack_busnos(uint32_t *bus) @@ -53,3 +54,34 @@ if (bus3) *bus3 = (bus >> 24) & 0xff; } + +msr_t read_msr_ppin(void) +{ + msr_t ppin = {0}; + msr_t msr; + + /* If MSR_PLATFORM_INFO PPIN_CAP is 0, PPIN capability is not supported */ + msr = rdmsr(MSR_PLATFORM_INFO); + if ((msr.lo & MSR_PPIN_CAP) == 0) { + printk(BIOS_ERR, "MSR_PPIN_CAP is 0, PPIN is not supported\n"); + return ppin; + } + + /* Access to MSR_PPIN is permitted only if MSR_PPIN_CTL LOCK is 0 and ENABLE is 1 */ + msr = rdmsr(MSR_PPIN_CTL); + if (msr.lo & MSR_PPIN_CTL_LOCK) { + printk(BIOS_ERR, "MSR_PPIN_CTL_LOCK is 1, PPIN access is not allowed\n"); + return ppin; + } + + if ((msr.lo & MSR_PPIN_CTL_ENABLE) == 0) { + /* Set MSR_PPIN_CTL ENABLE to 1 */ + msr.lo |= MSR_PPIN_CTL_ENABLE; + wrmsr(MSR_PPIN_CTL, msr); + } + ppin = rdmsr(MSR_PPIN); + /* Set enable to 0 after reading MSR_PPIN */ + msr.lo &= ~MSR_PPIN_CTL_ENABLE; + wrmsr(MSR_PPIN_CTL, msr); + return ppin; +}
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Jay Talbott (Code Review)
-
Jonathan Zhang (Code Review)
-
Marc Jones (Code Review)
-
Stefan Reinauer (Code Review)