Change in coreboot[master]: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP
Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45012 )
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP
The IA32_MTRR_CAP register has a bit which indicates that the SMRR MSRs can be "locked" and this patch adds the definition for that.
BUG=b:164489598
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I1254fb40c790f2a83dd11c2aabcf9bdf922b9395 --- M src/soc/intel/common/block/include/intelblocks/msr.h 1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/45012/1
diff --git a/src/soc/intel/common/block/include/intelblocks/msr.h b/src/soc/intel/common/block/include/intelblocks/msr.h index 82874f9..4aa069e 100644 --- a/src/soc/intel/common/block/include/intelblocks/msr.h +++ b/src/soc/intel/common/block/include/intelblocks/msr.h @@ -106,8 +106,9 @@ #define MSR_L2_QOS_MASK(reg) (0xd10 + reg)
/* MTRR_CAP_MSR bits */ -#define SMRR_SUPPORTED (1<<11) -#define PRMRR_SUPPORTED (1<<12) +#define SMRR_SUPPORTED (1<<11) +#define PRMRR_SUPPORTED (1<<12) +#define SMRR_LOCK_SUPPORTED (1<<14)
#define SGX_SUPPORTED (1<<2) /* Intel SDM: Table 36-6.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45012 )
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45012/1/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/msr.h:
https://review.coreboot.org/c/coreboot/+/45012/1/src/soc/intel/common/block/... PS1, Line 106: reg ugh, this needs parens.
separate patch 😊
https://review.coreboot.org/c/coreboot/+/45012/1/src/soc/intel/common/block/... PS1, Line 109: (1<<11) : #define PRMRR_SUPPORTED (1<<12) : #define SMRR_LOCK_SUPPORTED (1<<14) could you use the prevailing style and add spaces around the operator?
Hello Furquan Shaikh, Caveh Jalali, Subrata Banik, Kane Chen, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45012
to look at the new patch set (#2).
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP
The IA32_MTRR_CAP register has a bit which indicates that the SMRR MSRs can be "locked" and this patch adds the definition for that.
BUG=b:164489598
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I1254fb40c790f2a83dd11c2aabcf9bdf922b9395 --- M src/soc/intel/common/block/include/intelblocks/msr.h 1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/45012/2
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45012 )
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
Patch Set 2: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45012 )
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45012/1/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/msr.h:
https://review.coreboot.org/c/coreboot/+/45012/1/src/soc/intel/common/block/... PS1, Line 106: reg
ugh, this needs parens. […]
yes, indeedy, good eye.
https://review.coreboot.org/c/coreboot/+/45012/1/src/soc/intel/common/block/... PS1, Line 109: (1<<11) : #define PRMRR_SUPPORTED (1<<12) : #define SMRR_LOCK_SUPPORTED (1<<14)
could you use the prevailing style and add spaces around the operator?
This file is all over the place with spaces or not 😞 but sure I'll do the right thing and add 'em here.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45012 )
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45012/1/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/msr.h:
https://review.coreboot.org/c/coreboot/+/45012/1/src/soc/intel/common/block/... PS1, Line 109: (1<<11) : #define PRMRR_SUPPORTED (1<<12) : #define SMRR_LOCK_SUPPORTED (1<<14)
This file is all over the place with spaces or not 😞 but sure I'll do the right thing and add 'em h […]
So, this MSR is defined in several places (soc/intel/common & soc/intel/broadwell, as examples), and some code includes both header files that define this, so if they're defined differently, the compiler will complain... 😞 so I have to change it back or change the code that's including both... let's go with option #1 for now, and perhaps attempt a cleanup later?
Hello build bot (Jenkins), Furquan Shaikh, Caveh Jalali, Subrata Banik, Kane Chen, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45012
to look at the new patch set (#3).
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP
The IA32_MTRR_CAP register has a bit which indicates that the SMRR MSRs can be "locked" and this patch adds the definition for that.
BUG=b:164489598
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I1254fb40c790f2a83dd11c2aabcf9bdf922b9395 --- M src/soc/intel/common/block/include/intelblocks/msr.h 1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/45012/3
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45012 )
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
Patch Set 3: Code-Review+2
unfortunate full circle. agreed - let's clean up the headers as a separate effort.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45012 )
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45012/1/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/msr.h:
https://review.coreboot.org/c/coreboot/+/45012/1/src/soc/intel/common/block/... PS1, Line 109: (1<<11) : #define PRMRR_SUPPORTED (1<<12) : #define SMRR_LOCK_SUPPORTED (1<<14)
So, this MSR is defined in several places (soc/intel/common & soc/intel/broadwell, as examples), and […]
Done
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45012 )
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
Patch Set 3: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45012 )
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45012 )
Change subject: soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP ......................................................................
soc/intel/common: Add SMRR Lock Supported bit definition for MTRR_CAP
The IA32_MTRR_CAP register has a bit which indicates that the SMRR MSRs can be "locked" and this patch adds the definition for that.
BUG=b:164489598
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I1254fb40c790f2a83dd11c2aabcf9bdf922b9395 Reviewed-on: https://review.coreboot.org/c/coreboot/+/45012 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Caveh Jalali caveh@chromium.org Reviewed-by: Subrata Banik subrata.banik@intel.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/common/block/include/intelblocks/msr.h 1 file changed, 3 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved Caveh Jalali: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/soc/intel/common/block/include/intelblocks/msr.h b/src/soc/intel/common/block/include/intelblocks/msr.h index 82874f9..4aa069e 100644 --- a/src/soc/intel/common/block/include/intelblocks/msr.h +++ b/src/soc/intel/common/block/include/intelblocks/msr.h @@ -106,8 +106,9 @@ #define MSR_L2_QOS_MASK(reg) (0xd10 + reg)
/* MTRR_CAP_MSR bits */ -#define SMRR_SUPPORTED (1<<11) -#define PRMRR_SUPPORTED (1<<12) +#define SMRR_SUPPORTED (1<<11) +#define PRMRR_SUPPORTED (1<<12) +#define SMRR_LOCK_SUPPORTED (1<<14)
#define SGX_SUPPORTED (1<<2) /* Intel SDM: Table 36-6.
-
Angel Pons (Code Review) -
Caveh Jalali (Code Review)
-
Patrick Georgi (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)