[coreboot-gerrit] New patch to review for coreboot: cpu/intel: use modulo 32 to set msr.hi bits

Arthur Heymans (arthur@aheymans.xyz) gerrit at coreboot.org
Sat Oct 8 02:25:39 CEST 2016


Arthur Heymans (arthur at aheymans.xyz) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16937

-gerrit

commit fad346aa7c78beb0f7fd1448c599648fccd1733f
Author: Arthur Heymans <arthur at aheymans.xyz>
Date:   Sat Oct 8 02:22:44 2016 +0200

    cpu/intel: use modulo 32 to set msr.hi bits
    
    Change-Id: I91c5a52385c49b6175084f6aa54b05a3d0e2e397
    Signed-off-by: Arthur Heymans <arthur at aheymans.xyz>
---
 src/cpu/intel/fsp_model_206ax/finalize.c     |  4 ++--
 src/cpu/intel/fsp_model_406dx/bootblock.c    |  2 +-
 src/cpu/intel/haswell/bootblock.c            |  2 +-
 src/cpu/intel/haswell/finalize.c             |  4 ++--
 src/cpu/intel/haswell/haswell_init.c         | 22 +++++++++++-----------
 src/cpu/intel/haswell/romstage.c             |  2 +-
 src/cpu/intel/haswell/smmrelocate.c          |  6 +++---
 src/cpu/intel/model_1067x/model_1067x_init.c | 14 +++++++-------
 src/cpu/intel/model_2065x/bootblock.c        |  2 +-
 src/cpu/intel/model_2065x/finalize.c         |  4 ++--
 src/cpu/intel/model_206ax/bootblock.c        |  2 +-
 src/cpu/intel/model_206ax/finalize.c         |  4 ++--
 src/cpu/intel/model_6ex/model_6ex_init.c     |  4 ++--
 src/cpu/intel/model_6fx/model_6fx_init.c     |  6 +++---
 src/cpu/intel/speedstep/speedstep.c          |  4 ++--
 15 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/src/cpu/intel/fsp_model_206ax/finalize.c b/src/cpu/intel/fsp_model_206ax/finalize.c
index f027e80..b155fe3 100644
--- a/src/cpu/intel/fsp_model_206ax/finalize.c
+++ b/src/cpu/intel/fsp_model_206ax/finalize.c
@@ -34,9 +34,9 @@ static void msr_set_bit(unsigned reg, unsigned bit)
 			return;
 		msr.lo |= 1 << bit;
 	} else {
-		if (msr.hi & (1 << (bit - 32)))
+		if (msr.hi & (1 << (bit % 32)))
 			return;
-		msr.hi |= 1 << (bit - 32);
+		msr.hi |= 1 << (bit % 32);
 	}
 
 	wrmsr(reg, msr);
diff --git a/src/cpu/intel/fsp_model_406dx/bootblock.c b/src/cpu/intel/fsp_model_406dx/bootblock.c
index 99a27a7..9616356 100644
--- a/src/cpu/intel/fsp_model_406dx/bootblock.c
+++ b/src/cpu/intel/fsp_model_406dx/bootblock.c
@@ -51,7 +51,7 @@ static void set_var_mtrr(int reg, uint32_t base, uint32_t size, int type)
 	basem.hi = 0;
 	wrmsr(MTRR_PHYS_BASE(reg), basem);
 	maskm.lo = ~(size - 1) | MTRR_PHYS_MASK_VALID;
-	maskm.hi = (1 << (CONFIG_CPU_ADDR_BITS - 32)) - 1;
+	maskm.hi = (1 << (CONFIG_CPU_ADDR_BITS % 32)) - 1;
 	wrmsr(MTRR_PHYS_MASK(reg), maskm);
 }
 
diff --git a/src/cpu/intel/haswell/bootblock.c b/src/cpu/intel/haswell/bootblock.c
index 204a86b..93521db 100644
--- a/src/cpu/intel/haswell/bootblock.c
+++ b/src/cpu/intel/haswell/bootblock.c
@@ -42,7 +42,7 @@ static void set_var_mtrr(
 	basem.hi = 0;
 	wrmsr(MTRR_PHYS_BASE(reg), basem);
 	maskm.lo = ~(size - 1) | MTRR_PHYS_MASK_VALID;
-	maskm.hi = (1 << (CONFIG_CPU_ADDR_BITS - 32)) - 1;
+	maskm.hi = (1 << (CONFIG_CPU_ADDR_BITS % 32)) - 1;
 	wrmsr(MTRR_PHYS_MASK(reg), maskm);
 }
 
diff --git a/src/cpu/intel/haswell/finalize.c b/src/cpu/intel/haswell/finalize.c
index 743b900..1e01d5f 100644
--- a/src/cpu/intel/haswell/finalize.c
+++ b/src/cpu/intel/haswell/finalize.c
@@ -35,9 +35,9 @@ static void msr_set_bit(unsigned reg, unsigned bit)
 			return;
 		msr.lo |= 1 << bit;
 	} else {
-		if (msr.hi & (1 << (bit - 32)))
+		if (msr.hi & (1 << (bit % 32)))
 			return;
-		msr.hi |= 1 << (bit - 32);
+		msr.hi |= 1 << (bit % 32);
 	}
 
 	wrmsr(reg, msr);
diff --git a/src/cpu/intel/haswell/haswell_init.c b/src/cpu/intel/haswell/haswell_init.c
index 2fac879..9b4e0c2 100644
--- a/src/cpu/intel/haswell/haswell_init.c
+++ b/src/cpu/intel/haswell/haswell_init.c
@@ -351,31 +351,31 @@ static void initialize_vr_config(void)
 	/* Preserve bits 63 and 62. Bit 62 is PSI4 enable, but it is only valid
 	 * on ULT systems. */
 	msr.hi &= 0xc0000000;
-	msr.hi |= (0x01 << (52 - 32)); /* PSI3 threshold -  1A. */
-	msr.hi |= (0x05 << (42 - 32)); /* PSI2 threshold -  5A. */
-	msr.hi |= (0x0f << (32 - 32)); /* PSI1 threshold - 15A. */
+	msr.hi |= (0x01 << (52 % 32)); /* PSI3 threshold -  1A. */
+	msr.hi |= (0x05 << (42 % 32)); /* PSI2 threshold -  5A. */
+	msr.hi |= (0x0f << (32 % 32)); /* PSI1 threshold - 15A. */
 
 	if (haswell_is_ult())
-		msr.hi |= (1 <<  (62 - 32)); /* Enable PSI4 */
+		msr.hi |= (1 <<  (62 % 32)); /* Enable PSI4 */
 	/* Leave the max instantaneous current limit (12:0) to default. */
 	wrmsr(MSR_VR_CURRENT_CONFIG, msr);
 
 	/*  Configure VR_MISC_CONFIG MSR. */
 	msr = rdmsr(MSR_VR_MISC_CONFIG);
 	/* Set the IOUT_SLOPE scalar applied to dIout in U10.1.9 format. */
-	msr.hi &= ~(0x3ff << (40 - 32));
-	msr.hi |= (0x200 << (40 - 32)); /* 1.0 */
+	msr.hi &= ~(0x3ff << (40 % 32));
+	msr.hi |= (0x200 << (40 % 32)); /* 1.0 */
 	/* Set IOUT_OFFSET to 0. */
 	msr.hi &= ~0xff;
 	/* Set exit ramp rate to fast. */
-	msr.hi |= (1 << (50 - 32));
+	msr.hi |= (1 << (50 % 32));
 	/* Set entry ramp rate to slow. */
-	msr.hi &= ~(1 << (51 - 32));
+	msr.hi &= ~(1 << (51 % 32));
 	/* Enable decay mode on C-state entry. */
-	msr.hi |= (1 << (52 - 32));
+	msr.hi |= (1 << (52 % 32));
 	/* Set the slow ramp rate to be fast ramp rate / 4 */
-	msr.hi &= ~(0x3 << (53 - 32));
-	msr.hi |= (0x01 << (53 - 32));
+	msr.hi &= ~(0x3 << (53 % 32));
+	msr.hi |= (0x01 << (53 % 32));
 	/* Set MIN_VID (31:24) to allow CPU to have full control. */
 	msr.lo &= ~0xff000000;
 	wrmsr(MSR_VR_MISC_CONFIG, msr);
diff --git a/src/cpu/intel/haswell/romstage.c b/src/cpu/intel/haswell/romstage.c
index 8b62d43..70e051e 100644
--- a/src/cpu/intel/haswell/romstage.c
+++ b/src/cpu/intel/haswell/romstage.c
@@ -82,7 +82,7 @@ static void *setup_romstage_stack_after_car(void)
 
 	/* The upper bits of the MTRR mask need to set according to the number
 	 * of physical address bits. */
-	mtrr_mask_upper = (1 << (cpu_phys_address_size() - 32)) - 1;
+	mtrr_mask_upper = (1 << (cpu_phys_address_size() % 32)) - 1;
 
 	/* The order for each MTRR is value then base with upper 32-bits of
 	 * each value coming before the lower 32-bits. The reasoning for
diff --git a/src/cpu/intel/haswell/smmrelocate.c b/src/cpu/intel/haswell/smmrelocate.c
index 34a3551..acf0ca5 100644
--- a/src/cpu/intel/haswell/smmrelocate.c
+++ b/src/cpu/intel/haswell/smmrelocate.c
@@ -35,7 +35,7 @@
 #define UNCORE_EMRRphysMask_MSR 0x2f5
 #define SMM_MCA_CAP_MSR 0x17d
 #define   SMM_CPU_SVRSTR_BIT 57
-#define   SMM_CPU_SVRSTR_MASK (1 << (SMM_CPU_SVRSTR_BIT - 32))
+#define   SMM_CPU_SVRSTR_MASK (1 << (SMM_CPU_SVRSTR_BIT % 32))
 #define SMM_FEATURE_CONTROL_MSR 0x4e0
 #define   SMM_CPU_SAVE_EN (1 << 1)
 /* SMM save state MSRs */
@@ -267,14 +267,14 @@ static void fill_in_relocation_params(struct device *dev,
 	params->emrr_base.lo = emrr_base | MTRR_TYPE_WRBACK;
 	params->emrr_base.hi = 0;
 	params->emrr_mask.lo = (~(emrr_size - 1) & rmask) | MTRR_PHYS_MASK_VALID;
-	params->emrr_mask.hi = (1 << (phys_bits - 32)) - 1;
+	params->emrr_mask.hi = (1 << (phys_bits % 32)) - 1;
 
 	/* UNCORE_EMRR has 39 bits of valid address aligned to 4KiB. */
 	params->uncore_emrr_base.lo = emrr_base;
 	params->uncore_emrr_base.hi = 0;
 	params->uncore_emrr_mask.lo = (~(emrr_size - 1) & rmask) |
 	                              MTRR_PHYS_MASK_VALID;
-	params->uncore_emrr_mask.hi = (1 << (39 - 32)) - 1;
+	params->uncore_emrr_mask.hi = (1 << (39 % 32)) - 1;
 }
 
 static void setup_ied_area(struct smm_relocation_params *params)
diff --git a/src/cpu/intel/model_1067x/model_1067x_init.c b/src/cpu/intel/model_1067x/model_1067x_init.c
index e28a331..ea6f265 100644
--- a/src/cpu/intel/model_1067x/model_1067x_init.c
+++ b/src/cpu/intel/model_1067x/model_1067x_init.c
@@ -149,7 +149,7 @@ static void configure_p_states(const char stepping, const char cores)
 		msr.lo |= (1 << 28); /* Enable Super LFM. */
 	wrmsr(MSR_EXTENDED_CONFIG, msr);
 
-	if (rdmsr(MSR_FSB_CLOCK_VCC).hi & (1 << (63 - 32))) {
+	if (rdmsr(MSR_FSB_CLOCK_VCC).hi & (1 << (63 % 32))) {
 							/* Turbo supported? */
 		if ((stepping == 0xa) && (cores < 4)) {
 			msr = rdmsr(MSR_FSB_FREQ);
@@ -157,7 +157,7 @@ static void configure_p_states(const char stepping, const char cores)
 			wrmsr(MSR_FSB_FREQ, msr);
 		}
 		msr = rdmsr(IA32_PERF_CTL);
-		msr.hi &= ~(1 << (32 - 32)); /* Clear turbo disable. */
+		msr.hi &= ~(1 << (32 % 32)); /* Clear turbo disable. */
 		wrmsr(IA32_PERF_CTL, msr);
 	}
 
@@ -241,17 +241,17 @@ static void configure_misc(const int eist, const int tm2, const int emttm)
 
 	/* Enable C4E */
 	if (((sub_cstates >> (4 * 4)) & 0xf) >= 2) {
-		msr.hi |= (1 << (32 - 32)); // C4E
-		msr.hi |= (1 << (33 - 32)); // Hard C4E
+		msr.hi |= (1 << (32 % 32)); // C4E
+		msr.hi |= (1 << (33 % 32)); // Hard C4E
 	}
 
 	/* Enable EMTTM */
 	if (emttm)
-		msr.hi |= (1 << (36 - 32));
+		msr.hi |= (1 << (36 % 32));
 
 	/* Enable turbo mode */
-	if (rdmsr(MSR_FSB_CLOCK_VCC).hi & (1 << (63 - 32)))
-		msr.hi &= ~(1 << (38 - 32));
+	if (rdmsr(MSR_FSB_CLOCK_VCC).hi & (1 << (63 % 32)))
+		msr.hi &= ~(1 << (38 % 32));
 
 	wrmsr(IA32_MISC_ENABLES, msr);
 
diff --git a/src/cpu/intel/model_2065x/bootblock.c b/src/cpu/intel/model_2065x/bootblock.c
index 675e6bd..21fdef2 100644
--- a/src/cpu/intel/model_2065x/bootblock.c
+++ b/src/cpu/intel/model_2065x/bootblock.c
@@ -41,7 +41,7 @@ static void set_var_mtrr(
 	basem.hi = 0;
 	wrmsr(MTRR_PHYS_BASE(reg), basem);
 	maskm.lo = ~(size - 1) | MTRR_PHYS_MASK_VALID;
-	maskm.hi = (1 << (CONFIG_CPU_ADDR_BITS - 32)) - 1;
+	maskm.hi = (1 << (CONFIG_CPU_ADDR_BITS % 32)) - 1;
 	wrmsr(MTRR_PHYS_MASK(reg), maskm);
 }
 
diff --git a/src/cpu/intel/model_2065x/finalize.c b/src/cpu/intel/model_2065x/finalize.c
index 6a729b2..91c93cf 100644
--- a/src/cpu/intel/model_2065x/finalize.c
+++ b/src/cpu/intel/model_2065x/finalize.c
@@ -35,9 +35,9 @@ static void msr_set_bit(unsigned reg, unsigned bit)
 			return;
 		msr.lo |= 1 << bit;
 	} else {
-		if (msr.hi & (1 << (bit - 32)))
+		if (msr.hi & (1 << (bit % 32)))
 			return;
-		msr.hi |= 1 << (bit - 32);
+		msr.hi |= 1 << (bit % 32);
 	}
 
 	wrmsr(reg, msr);
diff --git a/src/cpu/intel/model_206ax/bootblock.c b/src/cpu/intel/model_206ax/bootblock.c
index eaa1870..4fadc94 100644
--- a/src/cpu/intel/model_206ax/bootblock.c
+++ b/src/cpu/intel/model_206ax/bootblock.c
@@ -42,7 +42,7 @@ static void set_var_mtrr(
 	basem.hi = 0;
 	wrmsr(MTRR_PHYS_BASE(reg), basem);
 	maskm.lo = ~(size - 1) | MTRR_PHYS_MASK_VALID;
-	maskm.hi = (1 << (CONFIG_CPU_ADDR_BITS - 32)) - 1;
+	maskm.hi = (1 << (CONFIG_CPU_ADDR_BITS % 32)) - 1;
 	wrmsr(MTRR_PHYS_MASK(reg), maskm);
 }
 
diff --git a/src/cpu/intel/model_206ax/finalize.c b/src/cpu/intel/model_206ax/finalize.c
index 870750c..b2b6948 100644
--- a/src/cpu/intel/model_206ax/finalize.c
+++ b/src/cpu/intel/model_206ax/finalize.c
@@ -35,9 +35,9 @@ static void msr_set_bit(unsigned reg, unsigned bit)
 			return;
 		msr.lo |= 1 << bit;
 	} else {
-		if (msr.hi & (1 << (bit - 32)))
+		if (msr.hi & (1 << (bit % 32)))
 			return;
-		msr.hi |= 1 << (bit - 32);
+		msr.hi |= 1 << (bit % 32);
 	}
 
 	wrmsr(reg, msr);
diff --git a/src/cpu/intel/model_6ex/model_6ex_init.c b/src/cpu/intel/model_6ex/model_6ex_init.c
index d42ff69..6ecddc1 100644
--- a/src/cpu/intel/model_6ex/model_6ex_init.c
+++ b/src/cpu/intel/model_6ex/model_6ex_init.c
@@ -107,8 +107,8 @@ static void configure_misc(void)
 	msr.lo |= (1 << 26);
 
 	/* Enable C4E */
-	msr.hi |= (1 << (32 - 32)); // C4E
-	msr.hi |= (1 << (33 - 32)); // Hard C4E
+	msr.hi |= (1 << (32 % 32)); // C4E
+	msr.hi |= (1 << (33 % 32)); // Hard C4E
 
 	wrmsr(IA32_MISC_ENABLE, msr);
 
diff --git a/src/cpu/intel/model_6fx/model_6fx_init.c b/src/cpu/intel/model_6fx/model_6fx_init.c
index 67a7408..91462d2 100644
--- a/src/cpu/intel/model_6fx/model_6fx_init.c
+++ b/src/cpu/intel/model_6fx/model_6fx_init.c
@@ -111,12 +111,12 @@ static void configure_misc(void)
 
 	/* Enable C4E */
 	/* TODO This should only be done on mobile CPUs, see cpuid 5 */
-	msr.hi |= (1 << (32 - 32)); // C4E
-	msr.hi |= (1 << (33 - 32)); // Hard C4E
+	msr.hi |= (1 << (32 % 32)); // C4E
+	msr.hi |= (1 << (33 % 32)); // Hard C4E
 
 	/* Enable EMTTM. */
 	/* NOTE: We leave the EMTTM_CR_TABLE0-5 at their default values */
-	msr.hi |= (1 << (36 - 32));
+	msr.hi |= (1 << (36 % 32));
 
 	wrmsr(IA32_MISC_ENABLE, msr);
 
diff --git a/src/cpu/intel/speedstep/speedstep.c b/src/cpu/intel/speedstep/speedstep.c
index 96ac8e5..8bb7bde 100644
--- a/src/cpu/intel/speedstep/speedstep.c
+++ b/src/cpu/intel/speedstep/speedstep.c
@@ -70,9 +70,9 @@ static void speedstep_get_limits(sst_params_t *const params)
 
 	/* Read turbo parameters. */
 	msr = rdmsr(MSR_FSB_CLOCK_VCC);
-	if ((msr.hi & (1 << (63 - 32))) &&
+	if ((msr.hi & (1 << (63 % 32))) &&
 		/* supported and */
-			!(rdmsr(IA32_MISC_ENABLES).hi & (1 << (38 - 32)))) {
+			!(rdmsr(IA32_MISC_ENABLES).hi & (1 << (38 % 32)))) {
 			/* not disabled */
 		params->turbo = SPEEDSTEP_STATE_FROM_MSR(msr.hi, state_mask);
 		params->turbo.is_turbo = 1;



More information about the coreboot-gerrit mailing list