Change in coreboot[master]: soc/intel/xeon_sp: Add read CPU PPIN MSR function

[Johnny Lin (Code Review)]

Johnny Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42901 )

Change subject: soc/intel/xeon_sp: Add read CPU PPIN MSR function
......................................................................

soc/intel/xeon_sp: Add read CPU PPIN MSR function

Tested on OCP Tioga Pass and Delta Lake.

Change-Id: I8c2eac055a065c06859a3cb7b48ed59f15ae2fc4
Signed-off-by: Johnny Lin 
---
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/skx/cpu.c
M src/soc/intel/xeon_sp/skx/include/soc/cpu.h
4 files changed, 66 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/42901/1

diff --git a/src/soc/intel/xeon_sp/cpx/cpu.c b/src/soc/intel/xeon_sp/cpx/cpu.c
index 386401f..1df5c13 100644
--- a/src/soc/intel/xeon_sp/cpx/cpu.c
+++ b/src/soc/intel/xeon_sp/cpx/cpu.c
@@ -188,3 +188,34 @@
    /* 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 2d12699..1458000 100644
--- a/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h
+++ b/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h
@@ -4,6 +4,7 @@
 #define _SOC_CPU_H

#include <device/device.h>
+#include <cpu/x86/msr.h>

#define CPUID_COOPERLAKE_SP_A0			0x05065a

@@ -11,5 +12,6 @@
 #define CPU_BCLK 100

void cpx_init_cpus(struct device *dev);
+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 f97c37d..c59edab 100644
--- a/src/soc/intel/xeon_sp/skx/cpu.c
+++ b/src/soc/intel/xeon_sp/skx/cpu.c
@@ -244,3 +244,34 @@

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 0221c7d..c2af265 100644
--- a/src/soc/intel/xeon_sp/skx/include/soc/cpu.h
+++ b/src/soc/intel/xeon_sp/skx/include/soc/cpu.h
@@ -4,6 +4,7 @@
 #define _SOC_CPU_H_

#include <device/device.h>
+#include <cpu/x86/msr.h>

/* SKXSP CPUID */
 #define CPUID_SKYLAKE_SP_A0_A1 0x506f0
@@ -15,5 +16,6 @@

int get_cpu_count(void);
 void xeon_sp_init_cpus(struct device *dev);
+msr_t read_msr_ppin(void);

#endif

[Johnny Lin (Code Review)]

Hello Jonathan Zhang, Jingle Hsu, Morgan Jang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/42901

to look at the new patch set (#2).

Change subject: soc/intel/xeon_sp: Add read CPU PPIN MSR function
......................................................................

soc/intel/xeon_sp: Add read CPU PPIN MSR function

These changes are in accordance with the documentation:
[*] page 208-209
    Intel(R) 64 and IA-32 Architectures, Software Developer’s Manual,
    Volume 4: Model-Specific Registers. May 2019.
    Order Number: 335592-070US

Tested on OCP Tioga Pass and Delta Lake.

Change-Id: I8c2eac055a065c06859a3cb7b48ed59f15ae2fc4
Signed-off-by: Johnny Lin 
---
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/skx/cpu.c
M src/soc/intel/xeon_sp/skx/include/soc/cpu.h
4 files changed, 66 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/42901/2

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42901 )

Change subject: soc/intel/xeon_sp: Add read CPU PPIN MSR function
......................................................................

Patch Set 4:

a

[Jonathan Zhang (Code Review)]

Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42901 )

Change subject: soc/intel/xeon_sp: Add read CPU PPIN MSR function
......................................................................

Patch Set 4: Code-Review+1

Ideally we do not want to duplicate the code in skx and cpx folders. That being said, we do have a plan to merge skx/cpx code base. One idea is to introduce processor family concept, similar like how mainboard variants are designed. So we are good for now.

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42901 )

Change subject: soc/intel/xeon_sp: Add read CPU PPIN MSR function
......................................................................

Patch Set 5: Code-Review+2

[Philipp Deppenwiese (Code Review)]

Philipp Deppenwiese has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42901 )

Change subject: soc/intel/xeon_sp: Add read CPU PPIN MSR function
......................................................................

soc/intel/xeon_sp: Add read CPU PPIN MSR function

These changes are in accordance with the documentation:
[*] page 208-209
    Intel(R) 64 and IA-32 Architectures, Software Developer’s Manual,
    Volume 4: Model-Specific Registers. May 2019.
    Order Number: 335592-070US

Tested on OCP Tioga Pass and Delta Lake.

Change-Id: I8c2eac055a065c06859a3cb7b48ed59f15ae2fc4
Signed-off-by: Johnny Lin 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42901
Reviewed-by: Patrick Rudolph 
Reviewed-by: Jonathan Zhang 
Tested-by: build bot (Jenkins) 
---
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/skx/cpu.c
M src/soc/intel/xeon_sp/skx/include/soc/cpu.h
4 files changed, 66 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Rudolph: Looks good to me, approved
  Jonathan Zhang: Looks good to me, but someone else must approve

diff --git a/src/soc/intel/xeon_sp/cpx/cpu.c b/src/soc/intel/xeon_sp/cpx/cpu.c
index 386401f..1df5c13 100644
--- a/src/soc/intel/xeon_sp/cpx/cpu.c
+++ b/src/soc/intel/xeon_sp/cpx/cpu.c
@@ -188,3 +188,34 @@
    /* 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 2d12699..1458000 100644
--- a/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h
+++ b/src/soc/intel/xeon_sp/cpx/include/soc/cpu.h
@@ -4,6 +4,7 @@
 #define _SOC_CPU_H

#include <device/device.h>
+#include <cpu/x86/msr.h>

#define CPUID_COOPERLAKE_SP_A0			0x05065a

@@ -11,5 +12,6 @@
 #define CPU_BCLK 100

void cpx_init_cpus(struct device *dev);
+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 f97c37d..c59edab 100644
--- a/src/soc/intel/xeon_sp/skx/cpu.c
+++ b/src/soc/intel/xeon_sp/skx/cpu.c
@@ -244,3 +244,34 @@

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 0221c7d..c2af265 100644
--- a/src/soc/intel/xeon_sp/skx/include/soc/cpu.h
+++ b/src/soc/intel/xeon_sp/skx/include/soc/cpu.h
@@ -4,6 +4,7 @@
 #define _SOC_CPU_H_

#include <device/device.h>
+#include <cpu/x86/msr.h>

/* SKXSP CPUID */
 #define CPUID_SKYLAKE_SP_A0_A1 0x506f0
@@ -15,5 +16,6 @@

int get_cpu_count(void);
 void xeon_sp_init_cpus(struct device *dev);
+msr_t read_msr_ppin(void);

#endif