Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33193
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM
This patch provides an additional option to skip HECI function disabling using SMM mode for WHL and CML platform, where FSP has dedicated UPD to make HECI function disable.
User to select SKIP_HECI_FUNCTION_DISABLE_USING_SMM if FSP has provided dedicated UPD.
Change-Id: If3b064f3c32877235916f966a01beb525156d188 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/smihandler.c M src/soc/intel/common/block/smm/Kconfig M src/soc/intel/icelake/smihandler.c 4 files changed, 15 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/33193/1
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 76906b2..fd87a5f 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -27,6 +27,7 @@ bool default n select SOC_INTEL_COMMON_CANNONLAKE_BASE + select SKIP_HECI_FUNCTION_DISABLE_USING_SMM if CHROMEOS help Intel Whiskeylake support
@@ -34,6 +35,7 @@ bool default n select SOC_INTEL_COMMON_CANNONLAKE_BASE + select SKIP_HECI_FUNCTION_DISABLE_USING_SMM if CHROMEOS help Intel Cometlake support
diff --git a/src/soc/intel/cannonlake/smihandler.c b/src/soc/intel/cannonlake/smihandler.c index 9af2917..2673bc5 100644 --- a/src/soc/intel/cannonlake/smihandler.c +++ b/src/soc/intel/cannonlake/smihandler.c @@ -88,7 +88,8 @@
config = dev->chip_info;
- if (config->HeciEnabled == 0) + if (!config->HeciEnabled && + !CONFIG(SKIP_HECI_FUNCTION_DISABLE_USING_SMM)) pch_disable_heci(); }
diff --git a/src/soc/intel/common/block/smm/Kconfig b/src/soc/intel/common/block/smm/Kconfig index a58c631..ae522f4 100644 --- a/src/soc/intel/common/block/smm/Kconfig +++ b/src/soc/intel/common/block/smm/Kconfig @@ -23,3 +23,12 @@ Time in milliseconds that SLP_SMI for S5 waits for before enabling sleep. This is required to avoid any race between SLP_SMI and PWRBTN SMI. + +config SKIP_HECI_FUNCTION_DISABLE_USING_SMM + bool + depends on SOC_INTEL_COMMON_BLOCK_SMM + default n + help + This Kconfig will help to skip HECI function disable using + SMM mode. User to only select this option if FSP provide + dedicated UPD to perform HECI function disable. diff --git a/src/soc/intel/icelake/smihandler.c b/src/soc/intel/icelake/smihandler.c index 5c00b63..60e7f80 100644 --- a/src/soc/intel/icelake/smihandler.c +++ b/src/soc/intel/icelake/smihandler.c @@ -86,7 +86,8 @@
config = dev->chip_info;
- if (config->HeciEnabled == 0) + if (!config->HeciEnabled && + !CONFIG(SKIP_HECI_FUNCTION_DISABLE_USING_SMM)) pch_disable_heci(); }
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS Is this really the right check? Calling this code today makes my WHL system hang, regardless of whether CONFIG_CHROMEOS is enabled or not.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
Is this really the right check?
Duncan, i think real use of this config would be like 1. WHL and CML platform will select SKIP_HECI_FUNCTION_DISABLE_USING_SMM from soc kconfig 2. HeciEnable=0 3. Make use of this CL https://review.coreboot.org/c/coreboot/+/32992
I don't think #3 one is merged yet.
Calling this code today makes my WHL system hang,
Do you mean code is still entering into "pch_disable_heci()" for WHL platform ? and this function is causing hang?
I have verified that on WHL and CML, now pch_disable_heci() function won't get call due to below checks
if (!config->HeciEnabled && !CONFIG(SKIP_HECI_FUNCTION_DISABLE_USING_SMM)) pch_disable_heci();
regardless of whether CONFIG_CHROMEOS is enabled or not.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
Patch Set 1:
(2 comments)
Can’t this be checked by the code, if the UPD is provided?
https://review.coreboot.org/#/c/33193/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33193/1//COMMIT_MSG@9 PS1, Line 9: This patch provides an additional option to skip : HECI function disabling using SMM mode for WHL and CML platform, : where FSP has dedicated UPD to make HECI function disable. Please format that paragraph to use the whole text width.
https://review.coreboot.org/#/c/33193/1/src/soc/intel/common/block/smm/Kconf... File src/soc/intel/common/block/smm/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/common/block/smm/Kconf... PS1, Line 32: This Kconfig will help to skip HECI function disable using : SMM mode. User to only select this option if FSP provide : dedicated UPD to perform HECI function disable. Skip HECI function disable using SMM. Only select htis option if FSP provides a dedicated UPD to perform HECI function disable.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
Patch Set 1:
(2 comments)
Patch Set 1:
(2 comments)
Can’t this be checked by the code, if the UPD is provided?
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
Is this really the right check? […]
Hi Duncan, I have tested this code further on hatch CML and WHL, i don't see any issue there as well. My earlier test was on RVP
https://review.coreboot.org/#/c/33193/1/src/soc/intel/common/block/smm/Kconf... File src/soc/intel/common/block/smm/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/common/block/smm/Kconf... PS1, Line 32: This Kconfig will help to skip HECI function disable using : SMM mode. User to only select this option if FSP provide : dedicated UPD to perform HECI function disable.
Skip HECI function disable using SMM. […]
Ack
Hello Aaron Durbin, Aamir Bohra, Duncan Laurie, Rizwan Qureshi, Bora Guvendik, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33193
to look at the new patch set (#2).
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM
This patch provides an additional option to skip HECI function disabling using SMM mode for WHL and CML platform, where FSP has dedicated UPD to make HECI function disable.
User to select SKIP_HECI_FUNCTION_DISABLE_USING_SMM if FSP has provided dedicated UPD.
Change-Id: If3b064f3c32877235916f966a01beb525156d188 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/smihandler.c M src/soc/intel/common/block/smm/Kconfig M src/soc/intel/icelake/smihandler.c 4 files changed, 14 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/33193/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
Hi Duncan, I have tested this code further on hatch CML and WHL, i don't see any issue there as well […]
So, disabling of HECI via SMM works just fine on WHL and CML? In the past, we had understood from Intel that disabling of HECI on WHL/CML has to be done via FSP.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
So, disabling of HECI via SMM works just fine on WHL and CML? In the past, we had understood from In […]
Hi Furquan, isn't the config name is "SKIP_HECI_FUNCTION_DISABLE_USING_SMM" which means WHL and CML is going to skip smm way of disabling HECI?
In the past, we had understood from Intel that disabling of HECI on WHL/CML has to be done via FSP.
SMM mode won't work for WHL and CML, hence we have taken FSP route. ICL and CNL is okay inside SMM mode
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
SMM mode won't work for WHL and CML, hence we have taken FSP route.
Then why is it dependent on CHROMEOS selection? Shouldn't it be independent of that?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
SMM mode won't work for WHL and CML, hence we have taken FSP route. […]
I got your point, the reason i have kept this check is that other WHL and CML users apart from chrome team doesn't wishes to make HECI disable. But i think they will never make Heci1Enable set to 0.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
I got your point, the reason i have kept this check is that other WHL and CML users apart from chrom […]
Correct. If a board does not want to disable Heci, they can set Heci1Enabled to 1.
BTW does this mean that ICL/CNL do not support disabling of HECI using FSP?
https://review.coreboot.org/#/c/33193/5/src/soc/intel/common/block/smm/Kconf... File src/soc/intel/common/block/smm/Kconfig:
https://review.coreboot.org/#/c/33193/5/src/soc/intel/common/block/smm/Kconf... PS5, Line 27: SKIP_HECI_FUNCTION_DISABLE_USING_SMM Does this have to be a negative option? i.e. can it instead be HECI_DISABLE_USING_SMM?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
Correct. If a board does not want to disable Heci, they can set Heci1Enabled to 1.
I will make the change.
BTW does this mean that ICL/CNL do not support disabling of HECI using FSP?
As i understand SMM mode will work for all future SOC is that WHL and CML had older CPU hence SMM mode was not giving other access. Do you prefer to use FSP for HECI disable or SMM mode without FSP?
https://review.coreboot.org/#/c/33193/5/src/soc/intel/common/block/smm/Kconf... File src/soc/intel/common/block/smm/Kconfig:
https://review.coreboot.org/#/c/33193/5/src/soc/intel/common/block/smm/Kconf... PS5, Line 27: SKIP_HECI_FUNCTION_DISABLE_USING_SMM
Does this have to be a negative option? i.e. […]
I can change that as well.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI function disable in SMM ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
As i understand SMM mode will work for all future SOC is that WHL and CML had older CPU hence SMM mode was not giving other access. Do you prefer to use FSP for HECI disable or SMM mode without FSP?
Ideally, I would like to keep it out of FSP and out of SMM too ;). But, since we are stuck with one of the two, I think we can live with doing it in SMM. Need to give this some more thought, but we can go ahead for now with what we have currently.
Hello Aaron Durbin, Aamir Bohra, Duncan Laurie, Rizwan Qureshi, Bora Guvendik, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33193
to look at the new patch set (#6).
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
soc/intel/{cml, whl}: Add option to skip HECI disable in SMM
This patch provides an additional option to skip HECI function disabling using SMM mode for WHL and CML platform, where FSP has dedicated UPD to make HECI function disable.
User to select HECI_DISABLE_USING_SMM if FSP doesn't provided dedicated UPD.
Right now CNL and ICL platform will use HECI_DISABLE_USING_SMM kconfig to make HECI disable and WHL/CML has to rely on FSP to make HECI disable.
Change-Id: If3b064f3c32877235916f966a01beb525156d188 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/smihandler.c M src/soc/intel/common/block/smm/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/smihandler.c 5 files changed, 12 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/33193/6
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
As i understand SMM mode will work for all future SOC is that WHL and CML had older CPU hence SMM […]
What is SMM mode allowing that non-SMM doesn't? Is the ME only able to honor the transaction from the host if the cpu is in SMM?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
What is SMM mode allowing that non-SMM doesn't?
sideband communication.
check this code: https://github.com/coreboot/coreboot/blob/1cf5ea5f1db6f780cd6da052c615a920c7...
after postboot_SAI been implemented at end of FSP-S, it will disable sideband communication. only will only be enable inside SMM mode
Is the ME only able to honor the transaction from the host if the cpu is in SMM?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
What is SMM mode allowing that non-SMM doesn't? […]
That's my point. Once the SA id is changed we require SMM. However, that also means we have libraries linked and persistent in SMM for communicating with ME. This is about turning off HECI, but it's concerning leaving this code around for attack purposes. I think Intel should think about this and how to minimize the exposure.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
That's my point. Once the SA id is changed we require SMM. […]
i guess you are referring at attack surface due to SMM mode. SBI interface lock been applied at end of FSP-notify 1 so after booting to OS, no one can exploit HECI using SBI inside smm mode.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
Patch Set 6: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/33193/6/src/soc/intel/common/block/smm/Kconf... File src/soc/intel/common/block/smm/Kconfig:
https://review.coreboot.org/#/c/33193/6/src/soc/intel/common/block/smm/Kconf... PS6, Line 32: Only select this option if FSP : doesn't provides a dedicated UPD to perform HECI disable. Even if FSP provides a UPD, mainboards could still choose to do it in coreboot, right?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/33193/6/src/soc/intel/common/block/smm/Kconf... File src/soc/intel/common/block/smm/Kconfig:
https://review.coreboot.org/#/c/33193/6/src/soc/intel/common/block/smm/Kconf... PS6, Line 32: Only select this option if FSP : doesn't provides a dedicated UPD to perform HECI disable.
Even if FSP provides a UPD, mainboards could still choose to do it in coreboot, right?
yeah, that what we wish to do even fsp has this support :)
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/33193/6/src/soc/intel/common/block/smm/Kconf... File src/soc/intel/common/block/smm/Kconfig:
https://review.coreboot.org/#/c/33193/6/src/soc/intel/common/block/smm/Kconf... PS6, Line 32: Only select this option if FSP : doesn't provides a dedicated UPD to perform HECI disable.
yeah, that what we wish to do even fsp has this support :)
I meant this option can be selected in more cases than what is indicated here in the help text. "Only select ... if FSP doesn't provide ..." seems to be too restrictive.
Hello Aaron Durbin, Aamir Bohra, Duncan Laurie, Rizwan Qureshi, Bora Guvendik, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33193
to look at the new patch set (#7).
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
soc/intel/{cml, whl}: Add option to skip HECI disable in SMM
This patch provides an additional option to skip HECI function disabling using SMM mode for WHL and CML platform, where FSP has dedicated UPD to make HECI function disable.
User to select HECI_DISABLE_USING_SMM if FSP doesn't provided dedicated UPD.
Right now CNL and ICL platform will use HECI_DISABLE_USING_SMM kconfig to make HECI disable and WHL/CML has to rely on FSP to make HECI disable.
Change-Id: If3b064f3c32877235916f966a01beb525156d188 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/smihandler.c M src/soc/intel/common/block/smm/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/smihandler.c 5 files changed, 12 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/33193/7
Hello Aaron Durbin, Aamir Bohra, Duncan Laurie, Rizwan Qureshi, Bora Guvendik, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33193
to look at the new patch set (#8).
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
soc/intel/{cml, whl}: Add option to skip HECI disable in SMM
This patch provides an additional option to skip HECI function disabling using SMM mode for WHL and CML platform, where FSP has dedicated UPD to make HECI function disable.
User to select HECI_DISABLE_USING_SMM if FSP doesn't provided dedicated UPD.
Right now CNL and ICL platform will use HECI_DISABLE_USING_SMM kconfig to make HECI disable and WHL/CML has to rely on FSP to make HECI disable.
Change-Id: If3b064f3c32877235916f966a01beb525156d188 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/smihandler.c M src/soc/intel/common/block/smm/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/smihandler.c 5 files changed, 12 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/33193/8
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/#/c/33193/6/src/soc/intel/common/block/smm/Kconf... File src/soc/intel/common/block/smm/Kconfig:
https://review.coreboot.org/#/c/33193/6/src/soc/intel/common/block/smm/Kconf... PS6, Line 32: Only select this option if FSP : doesn't provides a dedicated UPD to perform HECI disable.
I meant this option can be selected in more cases than what is indicated here in the help text. […]
Updated help text now.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
Patch Set 8: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
Patch Set 8: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
i guess you are referring at attack surface due to SMM mode. […]
This sounds much like a step backwards. SMM shouldn't be more privileged than normal execution, it only invites people to implement obscurity features in SMM. I guess in the long run it would be much better if coreboot would do the SAI switching.
I'm not sure if this was answered yet: Will ICL FSP have a UPD to disable HECI, so we won't have to do it in SMM?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/33193/1/src/soc/intel/cannonlake/Kconfig@30 PS1, Line 30: CHROMEOS
This sounds much like a step backwards. SMM shouldn't be more privileged than normal execution, it only invites people to implement obscurity features in SMM. I guess in the long run it would be much better if coreboot would do the SAI switching.
I'm not sure if this was answered yet:
Will ICL FSP have a UPD to disable HECI, so we won't have to do it in SMM?
I don't think we have plan to enable FSP UPD for ICL. I agree with you SMM is hackish but as i have explained we don't have any SMI handler which will might get called from OS layer also side access will also get block before booting to OS. But yes this feedback already went into system why we need to move into SMM.
Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33193 )
Change subject: soc/intel/{cml, whl}: Add option to skip HECI disable in SMM ......................................................................
soc/intel/{cml, whl}: Add option to skip HECI disable in SMM
This patch provides an additional option to skip HECI function disabling using SMM mode for WHL and CML platform, where FSP has dedicated UPD to make HECI function disable.
User to select HECI_DISABLE_USING_SMM if FSP doesn't provided dedicated UPD.
Right now CNL and ICL platform will use HECI_DISABLE_USING_SMM kconfig to make HECI disable and WHL/CML has to rely on FSP to make HECI disable.
Change-Id: If3b064f3c32877235916f966a01beb525156d188 Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/33193 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Nico Huber nico.h@gmx.de --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/smihandler.c M src/soc/intel/common/block/smm/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/smihandler.c 5 files changed, 12 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 76906b2..dac3522 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -107,6 +107,7 @@ select UDK_2017_BINDING select DISPLAY_FSP_VERSION_INFO select FSP_T_XIP if FSP_CAR + select HECI_DISABLE_USING_SMM if !SOC_INTEL_COFFEELAKE && !SOC_INTEL_WHISKEYLAKE && !SOC_INTEL_COMETLAKE
config DCACHE_RAM_BASE default 0xfef00000 diff --git a/src/soc/intel/cannonlake/smihandler.c b/src/soc/intel/cannonlake/smihandler.c index 9af2917..e8f0d17 100644 --- a/src/soc/intel/cannonlake/smihandler.c +++ b/src/soc/intel/cannonlake/smihandler.c @@ -88,7 +88,7 @@
config = dev->chip_info;
- if (config->HeciEnabled == 0) + if (!config->HeciEnabled && CONFIG(HECI_DISABLE_USING_SMM)) pch_disable_heci(); }
diff --git a/src/soc/intel/common/block/smm/Kconfig b/src/soc/intel/common/block/smm/Kconfig index a58c631..ab5ee03 100644 --- a/src/soc/intel/common/block/smm/Kconfig +++ b/src/soc/intel/common/block/smm/Kconfig @@ -23,3 +23,11 @@ Time in milliseconds that SLP_SMI for S5 waits for before enabling sleep. This is required to avoid any race between SLP_SMI and PWRBTN SMI. + +config HECI_DISABLE_USING_SMM + bool + depends on SOC_INTEL_COMMON_BLOCK_SMM + default n + help + HECI disable using SMM. Select this option to make HECI disable + using SMM mode, independent of dedicated UPD to perform HECI disable. diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 052e37d..f0b2918 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -62,6 +62,7 @@ select UDELAY_TSC select UDK_2017_BINDING select DISPLAY_FSP_VERSION_INFO + select HECI_DISABLE_USING_SMM
config DCACHE_RAM_BASE default 0xfef00000 diff --git a/src/soc/intel/icelake/smihandler.c b/src/soc/intel/icelake/smihandler.c index 5c00b63..5fb2480 100644 --- a/src/soc/intel/icelake/smihandler.c +++ b/src/soc/intel/icelake/smihandler.c @@ -86,7 +86,7 @@
config = dev->chip_info;
- if (config->HeciEnabled == 0) + if (!config->HeciEnabled && CONFIG(HECI_DISABLE_USING_SMM)) pch_disable_heci(); }