Change in coreboot[master]: cpu/intel/common: Fix invalid MSR access

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35548 )

Change subject: cpu/intel/common: Fix invalid MSR access
......................................................................

cpu/intel/common: Fix invalid MSR access

Fix regression from commit ecea916

cpu/intel/common: Extend FSB detection to cover TSC

MSR_EBC_FREQUENCY_ID (0x2c) was not defined for affected
CPU models and rdmsr() caused reset loops. Implementations
deviate from public documentation.

Change to IA32_PERF_STATUS (0x198) already used in i945/udelay.c
to detect FSB to TSC multiplier.

Change-Id: I7a91da221920a7e7bfccb98d76115b5c89e3b52e
Signed-off-by: Kyösti Mälkki 
---
M src/cpu/intel/common/fsb.c
1 file changed, 2 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/35548/1

diff --git a/src/cpu/intel/common/fsb.c b/src/cpu/intel/common/fsb.c
index 5ad98d4..c32bc23 100644
--- a/src/cpu/intel/common/fsb.c
+++ b/src/cpu/intel/common/fsb.c
@@ -42,12 +42,12 @@
    	case 0xe:  /* Core Solo/Duo */
    	case 0x1c: /* Atom */
    		*fsb = core_fsb[rdmsr(MSR_FSB_FREQ).lo & 7];
-			*ratio = rdmsr(MSR_EBC_FREQUENCY_ID).lo >> 24;
+			*ratio = (rdmsr(IA32_PERF_STATUS).hi >> 8) & 0x1f;
    		break;
    	case 0xf:  /* Core 2 or Xeon */
    	case 0x17: /* Enhanced Core */
    		*fsb = core2_fsb[rdmsr(MSR_FSB_FREQ).lo & 7];
-			*ratio = rdmsr(MSR_EBC_FREQUENCY_ID).lo >> 24;
+			*ratio = (rdmsr(IA32_PERF_STATUS).hi >> 8) & 0x1f;
    		break;
    	case 0x25: /* Nehalem BCLK fixed at 133MHz */
    		*fsb = 133;

[Patrick Georgi (Code Review)]

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

Change subject: cpu/intel/common: Fix invalid MSR access
......................................................................

Patch Set 1: Code-Review+2

looks good and makes my Getac boot again without ugly hacks

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35548 )

Change subject: cpu/intel/common: Fix invalid MSR access
......................................................................

Patch Set 1: Code-Review+1

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35548 )

Change subject: cpu/intel/common: Fix invalid MSR access
......................................................................

Patch Set 1:
> Patch Set 1: Code-Review+2
>
> looks good and makes my Getac boot again without ugly hacks

Thanks. Could I have CB:34200 tested as well? Seems like we have given TSC precedence over LAPIC for timebase, once TSC_CONSTANT_RATE and tsc_freq_mhz() were introduced.

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35548 )

Change subject: cpu/intel/common: Fix invalid MSR access
......................................................................

Patch Set 1: Code-Review+2

Fixes booting on x60 with model_6ex.

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35548 )

Change subject: cpu/intel/common: Fix invalid MSR access
......................................................................

cpu/intel/common: Fix invalid MSR access

Fix regression from commit ecea916

cpu/intel/common: Extend FSB detection to cover TSC

MSR_EBC_FREQUENCY_ID (0x2c) was not defined for affected
CPU models and rdmsr() caused reset loops. Implementations
deviate from public documentation.

Change to IA32_PERF_STATUS (0x198) already used in i945/udelay.c
to detect FSB to TSC multiplier.

Change-Id: I7a91da221920a7e7bfccb98d76115b5c89e3b52e
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35548
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Georgi 
Reviewed-by: HAOUAS Elyes 
Reviewed-by: Arthur Heymans 
---
M src/cpu/intel/common/fsb.c
1 file changed, 2 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved
  HAOUAS Elyes: Looks good to me, but someone else must approve
  Arthur Heymans: Looks good to me, approved

diff --git a/src/cpu/intel/common/fsb.c b/src/cpu/intel/common/fsb.c
index 5ad98d4..c32bc23 100644
--- a/src/cpu/intel/common/fsb.c
+++ b/src/cpu/intel/common/fsb.c
@@ -42,12 +42,12 @@
    	case 0xe:  /* Core Solo/Duo */
    	case 0x1c: /* Atom */
    		*fsb = core_fsb[rdmsr(MSR_FSB_FREQ).lo & 7];
-			*ratio = rdmsr(MSR_EBC_FREQUENCY_ID).lo >> 24;
+			*ratio = (rdmsr(IA32_PERF_STATUS).hi >> 8) & 0x1f;
    		break;
    	case 0xf:  /* Core 2 or Xeon */
    	case 0x17: /* Enhanced Core */
    		*fsb = core2_fsb[rdmsr(MSR_FSB_FREQ).lo & 7];
-			*ratio = rdmsr(MSR_EBC_FREQUENCY_ID).lo >> 24;
+			*ratio = (rdmsr(IA32_PERF_STATUS).hi >> 8) & 0x1f;
    		break;
    	case 0x25: /* Nehalem BCLK fixed at 133MHz */
    		*fsb = 133;