Yu-Ping Wu has submitted this change. ( https://review.coreboot.org/c/coreboot/+/73176 )
Change subject: soc/mediatek: Add config to control DRAM scramble
......................................................................
soc/mediatek: Add config to control DRAM scramble
The DRAM scramble feature enhances DRAM data protection. When it's
enabled, the written DRAM data will be scrambled and hence can prevent
the data from being hacked.
This feature would make debugging more difficult (for example ramoops
would be lost after reset). Therefore, add a new config to allow
enabling or disabling the feature from coreboot, without having to
maintain two versions of the DRAM calibration blob.
BUG=b:269049451
TEST=build pass and check scramble enable or disable successfully
Signed-off-by: Xi Chen <xixi.chen(a)mediatek.corp-partner.google.com>
Change-Id: Ib4279bc1cc960fae9c9f5da39f4448a5627288d4
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73176
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Yidi Lin <yidilin(a)google.com>
Reviewed-by: Yu-Ping Wu <yupingso(a)google.com>
---
M src/soc/mediatek/common/Kconfig
M src/soc/mediatek/common/include/soc/dramc_param_common.h
M src/soc/mediatek/common/memory.c
3 files changed, 37 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Yu-Ping Wu: Looks good to me, approved
Yidi Lin: Looks good to me, but someone else must approve
diff --git a/src/soc/mediatek/common/Kconfig b/src/soc/mediatek/common/Kconfig
index 2fdeda9..1ca0b98 100644
--- a/src/soc/mediatek/common/Kconfig
+++ b/src/soc/mediatek/common/Kconfig
@@ -30,6 +30,13 @@
This option allows performing fast calibration through different
open-source policy.
+config MEDIATEK_DRAM_SCRAMBLE
+ bool
+ default y
+ help
+ This option enables DRAM data scramble, which can prevent DRAM data from
+ being hacked.
+
config MEMORY_TEST
bool
default y
diff --git a/src/soc/mediatek/common/include/soc/dramc_param_common.h b/src/soc/mediatek/common/include/soc/dramc_param_common.h
index 09b89cb..429f3c6 100644
--- a/src/soc/mediatek/common/include/soc/dramc_param_common.h
+++ b/src/soc/mediatek/common/include/soc/dramc_param_common.h
@@ -31,6 +31,8 @@
DRAMC_CONFIG_EMCP = 0x0001,
DRAMC_CONFIG_DVFS = 0x0002,
DRAMC_CONFIG_FAST_K = 0x0004,
+ /* Security configs */
+ DRAMC_CONFIG_SCRAMBLE = 0x0100,
};
struct dramc_param_header {
diff --git a/src/soc/mediatek/common/memory.c b/src/soc/mediatek/common/memory.c
index fc559e2..b6f7dde 100644
--- a/src/soc/mediatek/common/memory.c
+++ b/src/soc/mediatek/common/memory.c
@@ -244,6 +244,8 @@
if (CONFIG(MEDIATEK_DRAM_DVFS))
dparam->dramc_datas.ddr_info.config_dvfs = DRAMC_ENABLE_DVFS;
+ if (CONFIG(MEDIATEK_DRAM_SCRAMBLE))
+ dparam->header.config |= DRAMC_CONFIG_SCRAMBLE;
dparam->dramc_datas.ddr_info.sdram.ddr_geometry = geometry;
--
To view, visit https://review.coreboot.org/c/coreboot/+/73176
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib4279bc1cc960fae9c9f5da39f4448a5627288d4
Gerrit-Change-Number: 73176
Gerrit-PatchSet: 5
Gerrit-Owner: Xixi Chen <xixi.chen(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Subrata Banik, Caveh Jalali, Nick Vaccaro, Boris Mittelberg.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73325 )
Change subject: ec/google/chromeec: Use host command API
......................................................................
Patch Set 1:
(3 comments)
File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/73325/comment/477a13b9_30646dba
PS1, Line 20: (h) == NULL
How about we define a macro for `NULL`
```
/* Required for ec_cmd_api.h but unused in coreboot */
#define H ((CROS_EC_COMMAND_INFO *)NULL)
```
and use it in API calls below? Then I think we can simply ignore `h` here.
https://review.coreboot.org/c/coreboot/+/73325/comment/f6475d95_78a13363
PS1, Line 469: _v1
How do we expect to update the cmd version? Before this patch we use macros such as `EC_VER_FLASH_PROTECT` defined in ec_commands.h. Therefore whenever the version is updated in ec_commands.h (for example CL:2929340), the version coreboot uses gets updated automatically (by syncing ec_commands.h in coreboot).
Now if we hardcode the version in ec.c, the process of syncing ec_commands.h will become much more complicated and error-prone. I wonder if we can make `_CROS_EC_C0_F_PF_RF` auto-decide the version from the `EC_VER_*` macros.
https://review.coreboot.org/c/coreboot/+/73325/comment/58c50d69_59b2dcdc
PS1, Line 523: struct chromeec_command cmd = {
Why isn't there an API for this command? I'd expect with this patch every chromeec_command declaration would be replaced.
--
To view, visit https://review.coreboot.org/c/coreboot/+/73325
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0317405b1ed0c58568078133c17c8cfbc7c21d80
Gerrit-Change-Number: 73325
Gerrit-PatchSet: 1
Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com>
Gerrit-Comment-Date: Thu, 02 Mar 2023 09:19:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment