Change in coreboot[master]: cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46917 )

Change subject: cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR
......................................................................

cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR

This MSR only needs to be programmed when IO MWAIT is to be enabled. The
code came from Sandy Bridge, which already showed this inconsistency.

Change-Id: I424333afd654db9a7e180e9a2c31d369e3d92fd6
Signed-off-by: Angel Pons 
---
M src/cpu/intel/haswell/haswell_init.c
1 file changed, 0 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/46917/1

diff --git a/src/cpu/intel/haswell/haswell_init.c b/src/cpu/intel/haswell/haswell_init.c
index 21f3f4a..6f483a5 100644
--- a/src/cpu/intel/haswell/haswell_init.c
+++ b/src/cpu/intel/haswell/haswell_init.c
@@ -452,12 +452,6 @@
    /* The deepest package c-state defaults to factory-configured value. */
    wrmsr(MSR_PKG_CST_CONFIG_CONTROL, msr);

-	msr = rdmsr(MSR_PMG_IO_CAPTURE_BASE);
-	msr.lo &= ~0xffff;
-	msr.lo |= (get_pmbase() + 0x14);	// LVL_2 base address
-	/* The deepest package c-state defaults to factory-configured value. */
-	wrmsr(MSR_PMG_IO_CAPTURE_BASE, msr);
-
    msr = rdmsr(MSR_MISC_PWR_MGMT);
    msr.lo &= ~(1 << 0);	// Enable P-state HW_ALL coordination
    wrmsr(MSR_MISC_PWR_MGMT, msr);

[Angel Pons (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph,

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

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

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

Change subject: cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR
......................................................................

cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR

This MSR only needs to be programmed when IO MWAIT is to be enabled. The
code came from Sandy Bridge, which already showed this inconsistency.

Change-Id: I424333afd654db9a7e180e9a2c31d369e3d92fd6
Signed-off-by: Angel Pons 
---
M src/cpu/intel/haswell/haswell_init.c
1 file changed, 0 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/46917/6

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46917 )

Change subject: cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR
......................................................................

Patch Set 14: Code-Review-2

[Nico Huber (Code Review)]

Attention is currently required from: Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46917 )

Change subject: cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR
......................................................................

Patch Set 14: Code-Review+2

[Nico Huber (Code Review)]

Attention is currently required from: Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46917 )

Change subject: cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR
......................................................................

Patch Set 14:

(1 comment)

File src/cpu/intel/haswell/haswell_init.c:

https://review.coreboot.org/c/coreboot/+/46917/comment/46318e5d_cc569fc8 
PS14, Line 445: 	msr.lo &= ~(1 << 10);	// Disable IO MWAIT redirection
I guess this is what the commit message refers to, but it says "IO MWAIT"
not "IO MWAIT redirection" which made me look for users of the IO ports.

[Angel Pons (Code Review)]

Attention is currently required from: Angel Pons.
Hello build bot (Jenkins), Nico Huber, Patrick Rudolph,

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

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

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

Change subject: cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR
......................................................................

cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR

The MSR only needs to be set when IO MWAIT redirection is to be enabled.
This was copied from Sandy Bridge, which already had this inconsistency.

Change-Id: I424333afd654db9a7e180e9a2c31d369e3d92fd6
Signed-off-by: Angel Pons 
---
M src/cpu/intel/haswell/haswell_init.c
1 file changed, 0 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/46917/15

[Angel Pons (Code Review)]

Angel Pons has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/46917 )

Change subject: cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR
......................................................................

Removed Code-Review-2 by Angel Pons

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46917 )

Change subject: cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR
......................................................................

Patch Set 15:

(1 comment)

File src/cpu/intel/haswell/haswell_init.c:

https://review.coreboot.org/c/coreboot/+/46917/comment/6113fa84_413c39fd 
PS14, Line 445: 	msr.lo &= ~(1 << 10);	// Disable IO MWAIT redirection
> I guess this is what the commit message refers to, but it says "IO MWAIT" […]

Thanks for making me remember

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46917 )

Change subject: cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR
......................................................................

cpu/intel/haswell: Do not set PMG_IO_CAPTURE_BASE MSR

The MSR only needs to be set when IO MWAIT redirection is to be enabled.
This was copied from Sandy Bridge, which already had this inconsistency.

Change-Id: I424333afd654db9a7e180e9a2c31d369e3d92fd6
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46917
Reviewed-by: Nico Huber 
Tested-by: build bot (Jenkins) 
---
M src/cpu/intel/haswell/haswell_init.c
1 file changed, 0 insertions(+), 6 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/src/cpu/intel/haswell/haswell_init.c b/src/cpu/intel/haswell/haswell_init.c
index 44bbbfd..ff9573f 100644
--- a/src/cpu/intel/haswell/haswell_init.c
+++ b/src/cpu/intel/haswell/haswell_init.c
@@ -446,12 +446,6 @@
    /* The deepest package c-state defaults to factory-configured value. */
    wrmsr(MSR_PKG_CST_CONFIG_CONTROL, msr);

-	msr = rdmsr(MSR_PMG_IO_CAPTURE_BASE);
-	msr.lo &= ~0xffff;
-	msr.lo |= (get_pmbase() + 0x14);	// LVL_2 base address
-	/* The deepest package c-state defaults to factory-configured value. */
-	wrmsr(MSR_PMG_IO_CAPTURE_BASE, msr);
-
    msr = rdmsr(MSR_MISC_PWR_MGMT);
    msr.lo &= ~(1 << 0);	// Enable P-state HW_ALL coordination
    wrmsr(MSR_MISC_PWR_MGMT, msr);