Change in coreboot[master]: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
mb/google/hatch: Enable chipset_lockdown coreboot config for hatch
This patch enables lockdown configuration for hatch family (hatch, kindred, helios and kohaku)
Change-Id: Ia6dc90156dc76fde490b25cf833da3cf80f664f2 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/variants/hatch/overridetree.cb M src/mainboard/google/hatch/variants/helios/overridetree.cb M src/mainboard/google/hatch/variants/kindred/overridetree.cb M src/mainboard/google/hatch/variants/kohaku/overridetree.cb 4 files changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/34514/1
diff --git a/src/mainboard/google/hatch/variants/hatch/overridetree.cb b/src/mainboard/google/hatch/variants/hatch/overridetree.cb index e40a5664..2cde48d 100644 --- a/src/mainboard/google/hatch/variants/hatch/overridetree.cb +++ b/src/mainboard/google/hatch/variants/hatch/overridetree.cb @@ -19,6 +19,7 @@ #+-------------------+---------------------------+ #| Field | Value | #+-------------------+---------------------------+ + #| chipset_lockdown | CHIPSET_LOCKDOWN_COREBOOT | #| GSPI0 | cr50 TPM. Early init is | #| | required to set up a BAR | #| | for TPM communication | @@ -29,6 +30,7 @@ #| I2C4 | Audio | #+-------------------+---------------------------+ register "common_soc_config" = "{ + .chipset_lockdown = CHIPSET_LOCKDOWN_COREBOOT, .gspi[0] = { .speed_mhz = 1, .early_init = 1, diff --git a/src/mainboard/google/hatch/variants/helios/overridetree.cb b/src/mainboard/google/hatch/variants/helios/overridetree.cb index 41c15c8..86a9b4a 100644 --- a/src/mainboard/google/hatch/variants/helios/overridetree.cb +++ b/src/mainboard/google/hatch/variants/helios/overridetree.cb @@ -21,12 +21,14 @@ #+-------------------+---------------------------+ #| Field | Value | #+-------------------+---------------------------+ + #| chipset_lockdown | CHIPSET_LOCKDOWN_COREBOOT | #| GSPI1 | FP MCU | #| I2C0 | Trackpad | #| I2C1 | Touchscreen | #| I2C4 | Audio | #+-------------------+---------------------------+ register "common_soc_config" = "{ + .chipset_lockdown = CHIPSET_LOCKDOWN_COREBOOT, .i2c[0] = { .speed = I2C_SPEED_FAST, }, diff --git a/src/mainboard/google/hatch/variants/kindred/overridetree.cb b/src/mainboard/google/hatch/variants/kindred/overridetree.cb index becfc49..be53f01 100644 --- a/src/mainboard/google/hatch/variants/kindred/overridetree.cb +++ b/src/mainboard/google/hatch/variants/kindred/overridetree.cb @@ -19,6 +19,7 @@ #+-------------------+---------------------------+ #| Field | Value | #+-------------------+---------------------------+ + #| chipset_lockdown | CHIPSET_LOCKDOWN_COREBOOT | #| GSPI0 | cr50 TPM. Early init is | #| | required to set up a BAR | #| | for TPM communication | @@ -29,6 +30,7 @@ #| I2C4 | Audio | #+-------------------+---------------------------+ register "common_soc_config" = "{ + .chipset_lockdown = CHIPSET_LOCKDOWN_COREBOOT, .gspi[0] = { .speed_mhz = 1, .early_init = 1, diff --git a/src/mainboard/google/hatch/variants/kohaku/overridetree.cb b/src/mainboard/google/hatch/variants/kohaku/overridetree.cb index e463b8b..23cc370 100644 --- a/src/mainboard/google/hatch/variants/kohaku/overridetree.cb +++ b/src/mainboard/google/hatch/variants/kohaku/overridetree.cb @@ -21,12 +21,14 @@ #+-------------------+---------------------------+ #| Field | Value | #+-------------------+---------------------------+ + #| chipset_lockdown | CHIPSET_LOCKDOWN_COREBOOT | #| I2C0 | Trackpad | #| I2C1 | Touchscreen | #| I2C2 | Digitizer | #| I2C4 | Audio | #+-------------------+---------------------------+ register "common_soc_config" = "{ + .chipset_lockdown = CHIPSET_LOCKDOWN_COREBOOT, .i2c[0] = { .speed = I2C_SPEED_FAST, },
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG@9 PS1, Line 9: lockdown configuration I believe your intent is to enable configuration lockdown by coreboot instead of FSP? What are the different configs that coreboot will be locking down and skipped by FSP? For KBL, I see following UPDs being set to 0 if coreboot is doing the lockdown: https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/intel/skylake/chi...
But there is no such change in soc/intel/cannonlake that would take effect for hatch. Don't you need a corresponding change for soc/intel/cannonlake?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG@9 PS1, Line 9: lockdown configuration
I believe your intent is to enable configuration lockdown by coreboot instead of FSP? What are the d […]
you are right Furquan. those changes also required
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG@9 PS1, Line 9: lockdown configuration
you are right Furquan. […]
Furquan,
Looks like except "SpiFlashCfgLockDown" UPD, we have others available in CML. Today FSP is unconditionally locking down DLOCK and FLOCK inside FSP.
tconfig->PchLockDownGlobalSmi = config->LockDownConfigGlobalSmi; tconfig->PchLockDownRtcLock = config->LockDownConfigRtcLock; if (get_lockdown_config() == CHIPSET_LOCKDOWN_COREBOOT) { tconfig->PchLockDownBiosInterface = 0; params->PchLockDownBiosLock = 0; params->PchLockDownSpiEiss = 0; }
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG@9 PS1, Line 9: lockdown configuration
Furquan, […]
What happened to SpiFlashCfgLockDown? Is it done by default always by FSP now?
Also, can PchLockDownGlobalSmi and PchLockDownRtcLock be set to 0 if CHIPSET_LOCKDOWN_COREBOOT is set instead of having 2 chip configs for those?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG@9 PS1, Line 9: lockdown configuration
What happened to SpiFlashCfgLockDown? Is it done by default always by FSP now?
yes, looks like FSP is setting DLOCK and FLOCK by its own for CML FSP without honoring dedicated FSP UPD.
Also, can PchLockDownGlobalSmi and PchLockDownRtcLock be set to 0 if CHIPSET_LOCKDOWN_COREBOOT is set instead of having 2 chip configs for those?
yes, thats better
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG@9 PS1, Line 9: lockdown configuration
What happened to SpiFlashCfgLockDown? Is it done by default always by FSP now?
yes, looks like FSP is setting DLOCK and FLOCK by its own for CML FSP without honoring dedicated FSP UPD.
Subrata, would you be following up internally to get this fixed?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG@9 PS1, Line 9: lockdown configuration
What happened to SpiFlashCfgLockDown? Is it done by default always by FSP now? […]
yes on it. will let you know about progress. Please help me to get a bug to track this
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG@9 PS1, Line 9: lockdown configuration
yes on it. will let you know about progress. Please help me to get a bug to track this
Here you go: b/138200201
Hello Paul Fagerburg, Aamir Bohra, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34514
to look at the new patch set (#3).
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
mb/google/hatch: Enable chipset_lockdown coreboot config for hatch
This patch enables lockdown configuration for hatch family (hatch, kindred, helios and kohaku)
BUG=b:138200201
Change-Id: Ia6dc90156dc76fde490b25cf833da3cf80f664f2 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/variants/hatch/overridetree.cb M src/mainboard/google/hatch/variants/helios/overridetree.cb M src/mainboard/google/hatch/variants/kindred/overridetree.cb M src/mainboard/google/hatch/variants/kohaku/overridetree.cb 4 files changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/34514/3
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 4:
Might be better to do the same thing as with the thermal trip parameter; set common_soc_config.chipset_lockdown = CHIPSET_LOCKDOWN_COREBOOT in the baseboard/devicetree.cb, again with a note that variants can override it using the same fully-qualified syntax
Hello Paul Fagerburg, Aamir Bohra, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34514
to look at the new patch set (#5).
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
mb/google/hatch: Enable chipset_lockdown coreboot config for hatch
This patch enables lockdown configuration for hatch family (hatch, kindred, helios and kohaku)
BUG=b:138200201
Change-Id: Ia6dc90156dc76fde490b25cf833da3cf80f664f2 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/34514/5
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 5:
Patch Set 4:
Might be better to do the same thing as with the thermal trip parameter; set common_soc_config.chipset_lockdown = CHIPSET_LOCKDOWN_COREBOOT in the baseboard/devicetree.cb, again with a note that variants can override it using the same fully-qualified syntax
Done.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34514/1//COMMIT_MSG@9 PS1, Line 9: lockdown configuration
yes on it. will let you know about progress. Please help me to get a bug to track this […]
Ack
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 5: Code-Review+1
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
Patch Set 5: Code-Review+2
Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34514 )
Change subject: mb/google/hatch: Enable chipset_lockdown coreboot config for hatch ......................................................................
mb/google/hatch: Enable chipset_lockdown coreboot config for hatch
This patch enables lockdown configuration for hatch family (hatch, kindred, helios and kohaku)
BUG=b:138200201
Change-Id: Ia6dc90156dc76fde490b25cf833da3cf80f664f2 Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34514 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Fagerburg pfagerburg@chromium.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 6 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved Paul Fagerburg: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb index 6ff90f8..6f4adc4 100644 --- a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb @@ -179,6 +179,12 @@ register "gpio_pm[COMM_3]" = "0" register "gpio_pm[COMM_4]" = "0"
+ # chipset_lockdown configuration + # Use below format to override value in overridetree.cb if required + # format: + # register "common_soc_config.<variable_name>" = "value" + register "common_soc_config.chipset_lockdown" = CHIPSET_LOCKDOWN_COREBOOT + device cpu_cluster 0 on device lapic 0 on end end
-
Furquan Shaikh (Code Review) -
Paul Fagerburg (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)