Change in coreboot[master]: include/cpu/amd/msr: move SMM_LOCK bit right after HWCR_MSR definition
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43309 )
Change subject: include/cpu/amd/msr: move SMM_LOCK bit right after HWCR_MSR definition ......................................................................
include/cpu/amd/msr: move SMM_LOCK bit right after HWCR_MSR definition
The SMM_LOCK bit isn't in SMM_MASK_MSR, but in HWCR_MSR, so move it there. The soc/amd/* code itself uses the bit definition when accessing HWCR_MSR, so SMM_LOCK was just below the wrong MSR definition.
Also remove SMM_LOCK from comment about masking bits in SMM_MASK_MSR, since that bit isn't in that MSR.
TEST=Checked the code and the corresponding BKDG/PPR.
Change-Id: I2df446f5a9e11e1e7c8d10256f3c2803b18f9088 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/include/cpu/amd/msr.h M src/soc/amd/common/block/psp/psp_smm.c 2 files changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/43309/1
diff --git a/src/include/cpu/amd/msr.h b/src/include/cpu/amd/msr.h index e466e7b..8bc00d1 100644 --- a/src/include/cpu/amd/msr.h +++ b/src/include/cpu/amd/msr.h @@ -17,6 +17,7 @@ #define MC4_MISC2 0xC0000409 #define FS_Base 0xC0000100 #define HWCR_MSR 0xC0010015 +#define SMM_LOCK (1 << 0) #define NB_CFG_MSR 0xC001001f #define FidVidStatus 0xC0010042 #define MC1_CTL_MASK 0xC0010045 @@ -53,7 +54,6 @@ #define SMM_BASE_MSR 0xC0010111 #define SMM_ADDR_MSR 0xC0010112 #define SMM_MASK_MSR 0xC0010113 -#define SMM_LOCK (1 << 0) #define SMM_TSEG_VALID (1 << 1) #define SMM_TSEG_WB (6 << 12)
diff --git a/src/soc/amd/common/block/psp/psp_smm.c b/src/soc/amd/common/block/psp/psp_smm.c index b103b3e..f919668 100644 --- a/src/soc/amd/common/block/psp/psp_smm.c +++ b/src/soc/amd/common/block/psp/psp_smm.c @@ -53,7 +53,7 @@ msr = rdmsr(SMM_ADDR_MSR); buffer.req.smm_base = ((uint64_t)msr.hi << 32) | msr.lo; msr = rdmsr(SMM_MASK_MSR); - msr.lo &= 0xffff0000; /* mask SMM_LOCK and SMM_TSEG_VALID and reserved bits */ + msr.lo &= 0xffff0000; /* mask SMM_TSEG_VALID and reserved bits */ buffer.req.smm_mask = ((uint64_t)msr.hi << 32) | msr.lo;
soc_fill_smm_trig_info(&buffer.req.smm_trig_info);
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43309 )
Change subject: include/cpu/amd/msr: move SMM_LOCK bit right after HWCR_MSR definition ......................................................................
Patch Set 1: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43309 )
Change subject: include/cpu/amd/msr: move SMM_LOCK bit right after HWCR_MSR definition ......................................................................
Patch Set 1: Code-Review+1
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43309 )
Change subject: include/cpu/amd/msr: move SMM_LOCK bit right after HWCR_MSR definition ......................................................................
include/cpu/amd/msr: move SMM_LOCK bit right after HWCR_MSR definition
The SMM_LOCK bit isn't in SMM_MASK_MSR, but in HWCR_MSR, so move it there. The soc/amd/* code itself uses the bit definition when accessing HWCR_MSR, so SMM_LOCK was just below the wrong MSR definition.
Also remove SMM_LOCK from comment about masking bits in SMM_MASK_MSR, since that bit isn't in that MSR.
TEST=Checked the code and the corresponding BKDG/PPR.
Change-Id: I2df446f5a9e11e1e7c8d10256f3c2803b18f9088 Signed-off-by: Felix Held felix-coreboot@felixheld.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/43309 Reviewed-by: Raul Rangel rrangel@chromium.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/include/cpu/amd/msr.h M src/soc/amd/common/block/psp/psp_smm.c 2 files changed, 2 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, but someone else must approve Raul Rangel: Looks good to me, approved
diff --git a/src/include/cpu/amd/msr.h b/src/include/cpu/amd/msr.h index e466e7b..8bc00d1 100644 --- a/src/include/cpu/amd/msr.h +++ b/src/include/cpu/amd/msr.h @@ -17,6 +17,7 @@ #define MC4_MISC2 0xC0000409 #define FS_Base 0xC0000100 #define HWCR_MSR 0xC0010015 +#define SMM_LOCK (1 << 0) #define NB_CFG_MSR 0xC001001f #define FidVidStatus 0xC0010042 #define MC1_CTL_MASK 0xC0010045 @@ -53,7 +54,6 @@ #define SMM_BASE_MSR 0xC0010111 #define SMM_ADDR_MSR 0xC0010112 #define SMM_MASK_MSR 0xC0010113 -#define SMM_LOCK (1 << 0) #define SMM_TSEG_VALID (1 << 1) #define SMM_TSEG_WB (6 << 12)
diff --git a/src/soc/amd/common/block/psp/psp_smm.c b/src/soc/amd/common/block/psp/psp_smm.c index b103b3e..f919668 100644 --- a/src/soc/amd/common/block/psp/psp_smm.c +++ b/src/soc/amd/common/block/psp/psp_smm.c @@ -53,7 +53,7 @@ msr = rdmsr(SMM_ADDR_MSR); buffer.req.smm_base = ((uint64_t)msr.hi << 32) | msr.lo; msr = rdmsr(SMM_MASK_MSR); - msr.lo &= 0xffff0000; /* mask SMM_LOCK and SMM_TSEG_VALID and reserved bits */ + msr.lo &= 0xffff0000; /* mask SMM_TSEG_VALID and reserved bits */ buffer.req.smm_mask = ((uint64_t)msr.hi << 32) | msr.lo;
soc_fill_smm_trig_info(&buffer.req.smm_trig_info);
-
Felix Held (Code Review) -
Kyösti Mälkki (Code Review)
-
Raul Rangel (Code Review)