[PATCH v3 0/8] Suspend to RAM support for K3 J7200

[PATCH v3 0/8] Suspend to RAM support for K3 J7200

[Thomas Richard]

This series is the U-Boot part of the work to add the suspend to RAM
support for the K3 J7200 EVM board.

During the boot R5 SPL makes a copy of DM-Firmware and TF-A in memory.
Resume detection is done by reading a magic value in a pmic register
(set by DM-Firmware).

If a resume is detected, R5 SPL run the exit retention sequence of the
DDR. Then it load TF-A and DM-Firmware using the copies done during the boot
(fit image processing is skipped).
Before to start TF-A, R5 SPL writes a magic value in scratchpad ram. This
will be used by TF-A to detect that the board is resuming.

The copy of TF-A/DM-Firmware, the SPL stack and malloc are located in a
reserved memory region (for the kernel point of view) to avoid any
memory corruption.

This version is mostly to test the firewall protection with the suspend
to ram.
Some comments (for the v2) were not fixed in this version.
This series has been tested with the series [1] to enable the firewall.
At the end of the resume sequence, TF-A is well protected by the
firewall, but OP-TEE remains unprotected.

[1] https://lore.kernel.org/all/20231229-binman-firewalling-v7-0-47ed4af303fe@ti.com/


Changes in v3:
- At resume, R5 SPL doesn't restore TF-A anymore.
  TF-A is started like during a cold boot. R5 SPL will notify that the board is
  resuming using a magic value written in the scratchpad ram.
  TF-A will restore itself.
- Link to v2:
  https://lore.kernel.org/u-boot/20231107161802.855154-1-thomas.richard@bootlin.com/

Changes in v2:
- Set exit retention code for DDR behind CONFIG_K3_J721E_DDRSS
- Check if TF-A is running in DRAM, if yes no need to restore it
- Remove BL31_START macro, and get TF-A start address from the fit image
- Remove the test_enter_suspend command
- Link to v1:
  https://lore.kernel.org/u-boot/20231016141135.325698-1-thomas.richard@bootlin.com/

Gowtham Tammana (1):
  DO NOT MERGE: arm: dts: k3-j7200-r5-common: Add pmic node for esm

Gregory CLEMENT (2):
  configs: j7200_evm_r5: Used reserved memory in DDR for stack
  configs: j7200_evm_r5: Move address used for allocation in the
    reserved space

Thomas Richard (5):
  board: ti: j721e: Add resume detection for J7200
  ram: k3-ddrss: Add exit retention support
  board: ti: j721e: Add the missing part of exit retention for k3-ddrss
    (J7200)
  board: ti: j721e: During resume spl notify tf-a that the board is
    resuming
  arm: mach-k3: j7200: Skip fit processing when resuming

 .../arm/dts/k3-j7200-r5-common-proc-board.dts |  17 +-
 arch/arm/mach-k3/common.c                     |  65 +++-
 .../arm/mach-k3/include/mach/j721e_hardware.h |   1 +
 arch/arm/mach-k3/include/mach/j721e_spl.h     |  30 ++
 arch/arm/mach-k3/sysfw-loader.c               |  16 +-
 board/ti/j721e/evm.c                          |  77 +++++
 configs/j7200_evm_r5_defconfig                |   4 +-
 drivers/ram/k3-ddrss/k3-ddrss.c               | 307 ++++++++++++++++++
 8 files changed, 507 insertions(+), 10 deletions(-)

-- 
2.39.2

[PATCH v3 1/8] DO NOT MERGE: arm: dts: k3-j7200-r5-common: Add pmic node for esm

[Thomas Richard]

From: Gowtham Tammana <g-tammana@ti.com>

Add pmic tps659413 node needed for ESM error event handling.

Signed-off-by: Gowtham Tammana <g-tammana@ti.com>
Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
---

(no changes since v1)

 arch/arm/dts/k3-j7200-r5-common-proc-board.dts | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/arm/dts/k3-j7200-r5-common-proc-board.dts b/arch/arm/dts/k3-j7200-r5-common-proc-board.dts
index e62f9218e8..2bf0d5e3c0 100644
--- a/arch/arm/dts/k3-j7200-r5-common-proc-board.dts
+++ b/arch/arm/dts/k3-j7200-r5-common-proc-board.dts
@@ -249,13 +249,24 @@
 
 &wkup_i2c0 {
 	bootph-pre-ram;
+	pinctrl-names = "default";
+	pinctrl-0 = <&wkup_i2c0_pins_default>;
+	clock-frequency = <400000>;
+
+	tps659413: tps659413@48 {
+		compatible = "ti,tps659413";
+		reg = <0x48>;
+		bootph-pre-ram;
+
+		regulators_a: regulators {
+			bootph-pre-ram;
+		};
+	};
+
 	lp876441: lp876441@4c {
 		compatible = "ti,lp876441";
 		reg = <0x4c>;
 		bootph-pre-ram;
-		pinctrl-names = "default";
-		pinctrl-0 = <&wkup_i2c0_pins_default>;
-		clock-frequency = <400000>;
 
 		regulators: regulators {
 			bootph-pre-ram;
-- 
2.39.2

[PATCH v3 2/8] configs: j7200_evm_r5: Used reserved memory in DDR for stack

[Thomas Richard]

From: Gregory CLEMENT <gregory.clement@bootlin.com>

When resuming from suspend to ram, we load again the DM firmware.
However, we have to be sure to not modify the memory used by
Linux.

Currently the SPL stack in DDR was in place that could be used by
Linux. Instead of it use a memory address that is located in a
reserved location that won't be used by Linux.

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
---

(no changes since v1)

 configs/j7200_evm_r5_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configs/j7200_evm_r5_defconfig b/configs/j7200_evm_r5_defconfig
index c4dd33627b..7450529d66 100644
--- a/configs/j7200_evm_r5_defconfig
+++ b/configs/j7200_evm_r5_defconfig
@@ -20,7 +20,7 @@ CONFIG_DM_RESET=y
 CONFIG_SPL_MMC=y
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL_DRIVERS_MISC=y
-CONFIG_SPL_STACK_R_ADDR=0x82000000
+CONFIG_SPL_STACK_R_ADDR=0xa5300000
 CONFIG_SPL_FS_FAT=y
 CONFIG_SPL_LIBDISK_SUPPORT=y
 CONFIG_SPL_SPI_FLASH_SUPPORT=y
-- 
2.39.2

[PATCH v3 3/8] configs: j7200_evm_r5: Move address used for allocation in the reserved space

[Thomas Richard]

From: Gregory CLEMENT <gregory.clement@bootlin.com>

Else it could corrupt the memory of Linux when exiting suspend to ram.

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
---

(no changes since v1)

 configs/j7200_evm_r5_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configs/j7200_evm_r5_defconfig b/configs/j7200_evm_r5_defconfig
index 7450529d66..1e7daf9b4a 100644
--- a/configs/j7200_evm_r5_defconfig
+++ b/configs/j7200_evm_r5_defconfig
@@ -39,7 +39,7 @@ CONFIG_SPL_STACK_R=y
 CONFIG_SPL_SEPARATE_BSS=y
 CONFIG_SYS_SPL_MALLOC=y
 CONFIG_HAS_CUSTOM_SPL_MALLOC_START=y
-CONFIG_CUSTOM_SYS_SPL_MALLOC_ADDR=0x84000000
+CONFIG_CUSTOM_SYS_SPL_MALLOC_ADDR=0xa5400000
 CONFIG_SYS_SPL_MALLOC_SIZE=0x1000000
 CONFIG_SPL_EARLY_BSS=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
-- 
2.39.2

[PATCH v3 4/8] board: ti: j721e: Add resume detection for J7200

[Thomas Richard]

Add the capability to detect a resume.
To detect the resume, SPL searches a magic value (0xBA) in a register
of PMICA.
This value is set by DM-Firmware during the suspend sequence.

Based on the work of Gregory CLEMENT <gregory.clement@bootlin.com>

Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>

---

Changes in v3:
- Remove if statement for pmic.h

 board/ti/j721e/evm.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c
index 38fe447d8f..c5404c014d 100644
--- a/board/ti/j721e/evm.c
+++ b/board/ti/j721e/evm.c
@@ -22,6 +22,7 @@
 #include <spl.h>
 #include <dm.h>
 #include <dm/uclass-internal.h>
+#include <power/pmic.h>
 
 #include "../common/board_detect.h"
 
@@ -528,6 +529,57 @@ err_free_gpio:
 	}
 }
 
+#if (IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_TARGET_J7200_R5_EVM))
+
+#define SCRATCH_PAD_REG_3 0xCB
+
+#define MAGIC_SUSPEND 0xBA
+
+static int resuming = -1;
+
+int board_is_resuming(void)
+{
+	struct udevice *pmica, *pmicb;
+	int ret;
+
+	if (resuming >= 0)
+		goto end;
+
+	ret = uclass_get_device_by_name(UCLASS_PMIC,
+					"tps659413@48", &pmica);
+	if (ret) {
+		printf("Getting PMICA init failed: %d\n", ret);
+		resuming = 0;
+		goto end;
+	}
+	debug("%s: PMICA is detected (%s)\n", __func__, pmica->name);
+
+	ret = uclass_get_device_by_name(UCLASS_PMIC,
+					"lp876441@4c", &pmicb);
+	if (ret) {
+		printf("Getting PMICB init failed: %d\n", ret);
+		resuming = 0;
+		goto end;
+	}
+	debug("%s: PMICB is detected (%s)\n", __func__, pmicb->name);
+
+	if ((pmic_reg_read(pmica, SCRATCH_PAD_REG_3) == MAGIC_SUSPEND)) {
+		resuming = 1;
+		debug("%s: board is resuming\n", __func__);
+
+		/* clean magic suspend */
+		if (pmic_reg_write(pmica, SCRATCH_PAD_REG_3, 0))
+			printf("Failed to clean magic value for suspend detection in PMICA\n");
+	} else {
+		resuming = 0;
+		debug("%s: board is booting (no resume detected)\n", __func__);
+	}
+end:
+	return resuming;
+}
+
+#endif /* CONFIG_SPL_BUILD && CONFIG_TARGET_J7200_R5_EVM */
+
 void spl_board_init(void)
 {
 #if defined(CONFIG_ESM_K3) || defined(CONFIG_ESM_PMIC)
-- 
2.39.2

[PATCH v3 5/8] ram: k3-ddrss: Add exit retention support

[Thomas Richard]

Add the exit retention support.
The enter retention is done by DM-Firmware.
A part of the exit retention sequence is specific to the board.
It is done in board_k3_ddrss_lpddr4_release_retention.

Based on the work of Gregory CLEMENT <gregory.clement@bootlin.com>

Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>

---

(no changes since v2)

Changes in v2:
- Set exit retention code for DDR behind CONFIG_K3_J721E_DDRSS

 drivers/ram/k3-ddrss/k3-ddrss.c | 307 ++++++++++++++++++++++++++++++++
 1 file changed, 307 insertions(+)

diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c b/drivers/ram/k3-ddrss/k3-ddrss.c
index b54557f02c..e47f615032 100644
--- a/drivers/ram/k3-ddrss/k3-ddrss.c
+++ b/drivers/ram/k3-ddrss/k3-ddrss.c
@@ -413,6 +413,306 @@ static int k3_ddrss_ofdata_to_priv(struct udevice *dev)
 	return ret;
 }
 
+#if defined(CONFIG_K3_J721E_DDRSS)
+
+void __weak board_k3_ddrss_lpddr4_release_retention(void) {}
+
+int __weak board_is_resuming(void)
+{
+	return 0;
+}
+
+#define PLL12_CTRL  0x0068C020
+
+#define k3_ddrss_readreg(k3_ddrss, block, shift, reg, pt) do {		\
+		u32 offset = 0U;					\
+		u32 result = 0U;					\
+		TH_OFFSET_FROM_REG(reg, shift, offset);			\
+		result = k3_ddrss->driverdt->readreg(&k3_ddrss->pd, block, offset, pt); \
+		if (result > 0U) {					\
+			printf("%s: Failed to read %s\n", __func__, xstr(reg));	\
+			hang();						\
+		}							\
+	} while (0)
+
+#define k3_ddrss_writereg(k3_ddrss, block, shift, reg, value) do {	\
+		u32 offset = 0U;					\
+		u32 result = 0U;					\
+		TH_OFFSET_FROM_REG(reg, shift, offset);			\
+		result = k3_ddrss->driverdt->writereg(&k3_ddrss->pd, block, offset, value); \
+		if (result > 0U) {					\
+			printf("%s: Failed to write %s\n", __func__, xstr(reg)); \
+			hang();						\
+		}							\
+	} while (0)
+
+#define k3_ddrss_readreg_ctl(ddrss, reg, pt) \
+	k3_ddrss_readreg(ddrss, LPDDR4_CTL_REGS, CTL_SHIFT, reg, pt)
+
+#define k3_ddrss_readreg_pi(ddrss, reg, pt) \
+	k3_ddrss_readreg(ddrss, LPDDR4_PHY_INDEP_REGS, PI_SHIFT, reg, pt)
+
+#define k3_ddrss_readreg_phy(ddrss, reg, pt) \
+	k3_ddrss_readreg(ddrss, LPDDR4_PHY_REGS, PHY_SHIFT, reg, pt)
+
+#define k3_ddrss_writereg_ctl(ddrss, reg, value) \
+	k3_ddrss_writereg(ddrss, LPDDR4_CTL_REGS, CTL_SHIFT, reg, value)
+
+#define k3_ddrss_writereg_pi(ddrss, reg, value) \
+	k3_ddrss_writereg(ddrss, LPDDR4_PHY_INDEP_REGS, PI_SHIFT, reg, value)
+
+#define k3_ddrss_writereg_phy(ddrss, reg, value) \
+	k3_ddrss_writereg(ddrss, LPDDR4_PHY_REGS, PHY_SHIFT, reg, value)
+
+#define k3_ddrss_set_ctl(k3_ddrss, reg, mask) do {	\
+	u32 val;					\
+	k3_ddrss_readreg_ctl(ddrss, reg, &val);		\
+	val |= mask;					\
+	k3_ddrss_writereg_ctl(ddrss, reg, val);		\
+	} while (0)
+
+#define k3_ddrss_clr_ctl(k3_ddrss, reg, mask) do {		\
+		u32 val;					\
+		k3_ddrss_readreg_ctl(ddrss, reg, &val);		\
+		val &= ~(mask);					\
+		k3_ddrss_writereg_ctl(ddrss, reg, val);		\
+	} while (0)
+
+#define k3_ddrss_set_pi(k3_ddrss, reg, mask) do {		\
+		u32 val;					\
+		k3_ddrss_readreg_pi(ddrss, reg, &val);		\
+		val |= mask;					\
+		k3_ddrss_writereg_pi(ddrss, reg, val);		\
+	} while (0)
+
+#define k3_ddrss_clr_pi(k3_ddrss, reg, mask) do {		\
+		u32 val;					\
+		k3_ddrss_readreg_pi(ddrss, reg, &val);		\
+		val &= ~(mask);					\
+		k3_ddrss_writereg_pi(ddrss, reg, val);		\
+	} while (0)
+
+#define k3_ddrss_set_phy(ddrss, reg, mask) do {		\
+	u32 val;					\
+	k3_ddrss_readreg_phy(ddrss, reg, &val);		\
+	val |= mask;					\
+	k3_ddrss_writereg_phy(ddrss, reg, val);		\
+	} while (0)
+
+#define k3_ddrss_clr_phy(ddrss, reg, mask) do {		\
+	u32 val;					\
+	k3_ddrss_readreg_phy(ddrss, reg, &val);		\
+	val &= ~(mask);					\
+	k3_ddrss_writereg_phy(ddrss, reg, val);		\
+	} while (0)
+
+static void k3_ddrss_lpddr4_exit_retention(struct k3_ddrss_desc *ddrss)
+{
+	u32 regval;
+	unsigned long tmo;
+	volatile unsigned int val;
+
+	/* disable auto entry / exit */
+	k3_ddrss_clr_ctl(ddrss, LPDDR4__LP_STATE_CS0__REG, (0xF << 24) | (0xF << 16));
+
+	/* Configure DFI Interface, DDR retention exit occurs through PHY */
+	k3_ddrss_readreg_phy(ddrss, LPDDR4__PHY_SET_DFI_INPUT_0__REG, &regval);
+	regval &= ~0xF0F; // Set DFI_Input_1 = 0
+	regval |= 0x01;   // Set DFI Input_0 = 1
+	k3_ddrss_writereg_phy(ddrss, LPDDR4__PHY_SET_DFI_INPUT_0__REG, regval);
+
+	/* PWRUP_SREFRESH_EXIT = 1 */
+	k3_ddrss_set_ctl(ddrss, LPDDR4__PWRUP_SREFRESH_EXIT__REG, 0x1);
+
+	/* PI_PWRUP_SREFRESH_EXIT = 0 */
+	k3_ddrss_clr_ctl(ddrss, LPDDR4__PI_COL_DIFF__REG, 0x1 << 16);
+
+	/* DFIBUS_BOOT_FREQ = 0 */
+	k3_ddrss_clr_ctl(ddrss, LPDDR4__DFIBUS_BOOT_FREQ__REG, 0x3);
+
+	/* DFIBUS_FREQ_INIT = 2 */
+	k3_ddrss_readreg_ctl(ddrss, LPDDR4__PHY_INDEP_TRAIN_MODE__REG, &regval);
+	regval &= ~(0x3 << 24);
+	regval |= (0x1 << 24);
+	k3_ddrss_writereg_ctl(ddrss, LPDDR4__PHY_INDEP_TRAIN_MODE__REG, regval);
+
+	/* Force Leveling during Initialization, Enable Link Training */
+
+	/* PI_INIT_LVL_EN = 1 */
+	k3_ddrss_set_pi(ddrss, LPDDR4__PI_NORMAL_LVL_SEQ__REG, (1 << 8));
+
+	/* PI_DRAM_INIT_EN = 0 */
+	k3_ddrss_clr_pi(ddrss, LPDDR4__PI_DLL_RST__REG, (0x1 << 8));
+
+	/* PI_DFI_PHYMSTR_STATE_SEL_R = 1  (force memory into self-refresh) */
+	k3_ddrss_set_pi(ddrss, LPDDR4__PI_DFI_VERSION__REG, (1 << 24));
+
+	/*  PHY_INDEP_INIT_MODE = 0 */
+	k3_ddrss_clr_ctl(ddrss, LPDDR4__PHY_INDEP_TRAIN_MODE__REG, (0x1 << 16));
+	/* PHY_INDEP_TRAIN_MODE = 1 */
+	k3_ddrss_set_ctl(ddrss, LPDDR4__PHY_INDEP_TRAIN_MODE__REG, 0x1);
+
+	/* PI_INIT_WORK_FREQ = 1 */
+	k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_INIT_WORK_FREQ__REG, &regval);
+	regval &= ~0x1F;
+	regval |= 0x01;
+	k3_ddrss_writereg_pi(ddrss, LPDDR4__PI_INIT_WORK_FREQ__REG, regval);
+
+	/* PI_FREQ_MAP[2:0] */
+	k3_ddrss_writereg_pi(ddrss, LPDDR4__PI_FREQ_MAP__REG, 0x03);
+
+	/* Training/leveling configurations for different frequency set points */
+
+	/* PI_CALVL_EN_F0 = 01b; PI_CALVL_EN_F1 = 01b; PI_CALVL_EN_F2 = 01b */
+	k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_CALVL_EN_F0__REG, &regval);
+	regval &= ~0x030303;
+	regval |= 0x010101;
+	k3_ddrss_writereg_pi(ddrss, LPDDR4__PI_CALVL_EN_F0__REG, regval);
+
+	/* PI_WRLVL_EN_F0 = 00b; PI_WRLVL_EN_F1 = 01b */
+	k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_TDFI_CTRL_DELAY_F1__REG, &regval);
+	regval &= ~0x03030000;
+	regval |= 0x01000000;
+	k3_ddrss_writereg_pi(ddrss, LPDDR4__PI_TDFI_CTRL_DELAY_F1__REG, regval);
+
+	/* PI_WRLVL_EN_F2 = 01b */
+	k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_WRLVL_EN_F2__REG, &regval);
+	regval &= ~0x03;
+	regval |= 0x01;
+	k3_ddrss_writereg_pi(ddrss, LPDDR4__PI_WRLVL_EN_F2__REG, regval);
+
+	/* PI_RDLVL_EN_F0 = 00b; PI_RDLVL_EN_F1 = 01b */
+	k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_RDLVL_EN_F0__REG, &regval);
+	regval &= ~0x030003;
+	regval |= 0x010000;
+	k3_ddrss_writereg_pi(ddrss, LPDDR4__PI_RDLVL_EN_F0__REG, regval);
+
+	/* PI_RDLVL_EN_F2 = 01b */
+	k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_RDLVL_EN_F2__REG, &regval);
+	regval &= ~0x03;
+	regval |= 0x01;
+	k3_ddrss_writereg_pi(ddrss, LPDDR4__PI_RDLVL_EN_F2__REG, regval);
+
+	/* PI_RDLVL_GATE_EN_F0 = 00b; PI_RDLVL_GATE_EN_F1 = 01b */
+	k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_RDLVL_EN_F0__REG, &regval);
+	regval &= ~0x03000300;
+	regval |= 0x01000000;
+	k3_ddrss_writereg_pi(ddrss, LPDDR4__PI_RDLVL_EN_F0__REG, regval);
+
+	/* PI_RDLVL_GATE_EN_F2 = 01b */
+	k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_RDLVL_EN_F2__REG, &regval);
+	regval &= ~0x0300;
+	regval |= 0x0100;
+	k3_ddrss_writereg_pi(ddrss, LPDDR4__PI_RDLVL_EN_F2__REG, regval);
+
+	/* PI_WDQLVL_EN_F0 = 00b */
+	k3_ddrss_clr_pi(ddrss, LPDDR4__PI_WDQLVL_VREF_DELTA_F0__REG, (0x3 << 8));
+
+	/* PI_WDQLVL_EN_F1 = 01b */
+	k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_WDQLVL_VREF_DELTA_F1__REG, &regval);
+	regval &= ~(0x3 << 24);
+	regval |= (0x1 << 24);
+	k3_ddrss_writereg_pi(ddrss, LPDDR4__PI_WDQLVL_VREF_DELTA_F1__REG, regval);
+
+	/* PI_WDQLVL_EN_F2 = 01b */
+	k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_WDQLVL_VREF_DELTA_F2__REG, &regval);
+	regval &= ~(0x3 << 8);
+	regval |= (0x1 << 8);
+	k3_ddrss_writereg_pi(ddrss, LPDDR4__PI_WDQLVL_VREF_DELTA_F2__REG, regval);
+
+	*(unsigned int *)PLL12_CTRL |= 0x80000000;
+	val = *(volatile unsigned int *)PLL12_CTRL;
+
+	if ((val & 0x80000000) != 0x80000000)
+		val = *(volatile unsigned int *)PLL12_CTRL;
+
+	board_k3_ddrss_lpddr4_release_retention();
+
+	/* LPC_SR_PHYMSTR_EN */
+	k3_ddrss_clr_ctl(ddrss, LPDDR4__LPC_SR_CTRLUPD_EN__REG, (0x1 << 16));
+	/* PI_START=1 */
+	k3_ddrss_set_pi(ddrss, LPDDR4__PI_START__REG, 0x01);
+	/* START=1 */
+	k3_ddrss_set_ctl(ddrss, LPDDR4__START__REG, 0x01);
+
+	debug("%s: PPL12 = %x\n", __func__, *(volatile unsigned int *)PLL12_CTRL);
+	ddrss->ddr_fhs_cnt = 5;
+	k3_lpddr4_freq_update(ddrss);
+	debug("%s: PPL12 = %x\n", __func__, *(volatile unsigned int *)PLL12_CTRL);
+
+	tmo = timer_get_us() + 100000;
+	do {
+		k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_INT_STATUS__REG, &regval);
+		debug("%s: DDRSS_PI_79 = %x\n", __func__, regval);
+		if (timer_get_us() > tmo) {
+			printf("%s:%d timeout error\n", __func__, __LINE__);
+			hang();
+		}
+	} while ((regval & (1 << 0)) == 0x00);
+
+	tmo = timer_get_us() + 4000;
+	do {
+		k3_ddrss_readreg_ctl(ddrss, LPDDR4__INT_STATUS_0__REG, &regval);
+		if (timer_get_us() > tmo) {
+			printf("%s:%d timeout error\n", __func__, __LINE__);
+			hang();
+		}
+	} while ((regval & (1 << 9)) == 0x00);
+
+	k3_ddrss_readreg_pi(ddrss, LPDDR4__PI_INT_STATUS__REG, &regval);
+	debug("%s: PI interrupt status: 0x%08x\n", __func__, regval);
+
+	k3_ddrss_readreg_ctl(ddrss, LPDDR4__INT_STATUS_0__REG, &regval);
+	debug("%s: Controller interrupt status: 0x%08x\n", __func__, regval);
+
+	debug("%s: Successfully exited Retention\n", __func__);
+
+	k3_ddrss_readreg_ctl(ddrss, LPDDR4__LP_STATE_CS0__REG, &regval);
+	debug("%s: LP State: 0x%08x\n", __func__, regval);
+
+	k3_ddrss_writereg_ctl(ddrss, LPDDR4__INT_ACK_0__REG, (0x1 << 10));
+
+	k3_ddrss_readreg_ctl(ddrss, LPDDR4__CKSRX_F1__REG, &regval);
+	regval &= ~(0x7F << 24);
+	regval |= (0x2 << 24); // set low power mode exit
+	k3_ddrss_writereg_ctl(ddrss, LPDDR4__CKSRX_F1__REG, regval);
+
+	k3_ddrss_readreg_ctl(ddrss, LPDDR4__LP_STATE_CS0__REG, &regval);
+	debug("%s: LP State: 0x%08x\n", __func__, regval);
+
+	/* wait until low power operation has been completed */
+	tmo = timer_get_us() + 4000;
+	do {
+		k3_ddrss_readreg_ctl(ddrss, LPDDR4__INT_STATUS_0__REG, &regval);
+		if (timer_get_us() > tmo) {
+			printf("%s:%d timeout error\n", __func__, __LINE__);
+			hang();
+		}
+	} while ((regval & (0x1 << 10)) == 0);
+
+	k3_ddrss_readreg_ctl(ddrss, LPDDR4__LP_STATE_CS0__REG, &regval);
+	debug("%s: LP State: 0x%08x\n", __func__, regval);
+
+	k3_ddrss_writereg_ctl(ddrss, LPDDR4__INT_ACK_0__REG, (0x1 << 10));
+
+	/*
+	 * bit 6 / 14 -- lp_state valid
+	 * bits 13:8 / 5:0 0x0F SRPD Long with Mem and Controller Clk Gating
+	 */
+	tmo = timer_get_us() + 4000;
+	do {
+		k3_ddrss_readreg_ctl(ddrss, LPDDR4__LP_STATE_CS0__REG, &regval);
+		if (timer_get_us() > tmo) {
+			printf("%s:%d timeout error\n", __func__, __LINE__);
+			hang();
+		}
+	} while ((regval & 0x4F4F) != 0x4040);
+
+	k3_ddrss_readreg_ctl(ddrss, LPDDR4__LP_STATE_CS0__REG, &regval);
+	debug("%s: LP State: 0x%08x\n", __func__, regval);
+}
+#endif /* CONFIG_K3_J721E_DDRSS */
+
 void k3_lpddr4_probe(struct k3_ddrss_desc *ddrss)
 {
 	u32 status = 0U;
@@ -637,6 +937,13 @@ static int k3_ddrss_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
+#if defined(CONFIG_K3_J721E_DDRSS)
+	if (board_is_resuming()) {
+		k3_ddrss_lpddr4_exit_retention(ddrss);
+		return 0;
+	}
+#endif
+
 	k3_lpddr4_start(ddrss);
 
 	if (ddrss->ti_ecc_enabled) {
-- 
2.39.2

[PATCH v3 6/8] board: ti: j721e: Add the missing part of exit retention for k3-ddrss (J7200)

[Thomas Richard]

Add the board specific part of the exit retention sequence for k3-ddrss

Based on the work of Gregory CLEMENT <gregory.clement@bootlin.com>

Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---

(no changes since v1)

 board/ti/j721e/evm.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c
index c5404c014d..1814ccd882 100644
--- a/board/ti/j721e/evm.c
+++ b/board/ti/j721e/evm.c
@@ -531,15 +531,22 @@ err_free_gpio:
 
 #if (IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_TARGET_J7200_R5_EVM))
 
+static struct udevice *pmica;
+static struct udevice *pmicb;
+
+#define GPIO_OUT_1        0x3D
+
 #define SCRATCH_PAD_REG_3 0xCB
 
 #define MAGIC_SUSPEND 0xBA
 
+#define DDR_RET_VAL BIT(1)
+#define DDR_RET_CLK BIT(2)
+
 static int resuming = -1;
 
 int board_is_resuming(void)
 {
-	struct udevice *pmica, *pmicb;
 	int ret;
 
 	if (resuming >= 0)
@@ -578,6 +585,24 @@ end:
 	return resuming;
 }
 
+void board_k3_ddrss_lpddr4_release_retention(void)
+{
+	int regval;
+
+	/* Set DDR_RET Signal Low on PMIC B */
+	regval = pmic_reg_read(pmicb, GPIO_OUT_1) & ~DDR_RET_VAL;
+
+	pmic_reg_write(pmicb, GPIO_OUT_1, regval);
+
+	/* Now toggle the CLK of the latch for DDR ret */
+	pmic_reg_write(pmicb, GPIO_OUT_1, regval | DDR_RET_CLK);
+	pmic_reg_write(pmicb, GPIO_OUT_1, regval & ~(DDR_RET_CLK));
+	pmic_reg_write(pmicb, GPIO_OUT_1, regval | DDR_RET_CLK);
+	pmic_reg_write(pmicb, GPIO_OUT_1, regval & ~(DDR_RET_CLK));
+
+	pmic_reg_write(pmica, 0x86, 0x3);
+}
+
 #endif /* CONFIG_SPL_BUILD && CONFIG_TARGET_J7200_R5_EVM */
 
 void spl_board_init(void)
-- 
2.39.2

[PATCH v3 7/8] board: ti: j721e: During resume spl notify tf-a that the board is resuming

[Thomas Richard]

During the boot a copy of DM-Firmware and TF-A is done in a reserved memory
area.
When resuming, R5 SPL uses these copies instead of the fit image.
R5 SPL writes a magic value in the scratchpad ram to notify TF-A that
the board is resuming.

Based on the work of Gregory CLEMENT <gregory.clement@bootlin.com>

Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>

---

Changes in v3:
- At resume, R5 SPL doesn't restore TF-A anymore.
  TF-A is started like during a cold boot. R5 SPL will notify that the board is
  resuming using a magic value written in the scratchpad ram.
  TF-A will restore itself.

Changes in v2:
- Check if TF-A is running in DRAM, if yes no need to restore it
- Remove BL31_START macro, and get TF-A start address from the fit image

 arch/arm/mach-k3/common.c                     | 65 ++++++++++++++++++-
 .../arm/mach-k3/include/mach/j721e_hardware.h |  1 +
 arch/arm/mach-k3/include/mach/j721e_spl.h     | 30 +++++++++
 arch/arm/mach-k3/sysfw-loader.c               |  9 ++-
 4 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
index a35110429b..546ea3cf5e 100644
--- a/arch/arm/mach-k3/common.c
+++ b/arch/arm/mach-k3/common.c
@@ -26,6 +26,7 @@
 #include <env.h>
 #include <elf.h>
 #include <soc.h>
+#include <hang.h>
 
 #if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
 enum {
@@ -221,6 +222,11 @@ void release_resources_for_core_shutdown(void)
 	}
 }
 
+__weak int board_is_resuming(void)
+{
+	return 0;
+}
+
 void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 {
 	typedef void __noreturn (*image_entry_noargs_t)(void);
@@ -235,6 +241,35 @@ void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 	if (ret)
 		panic("rproc failed to be initialized (%d)\n", ret);
 
+	if (board_is_resuming()) {
+#if IS_ENABLED(CONFIG_SOC_K3_J721E)
+		if (!valid_elf_image(LPM_DM))
+			panic("%s: DM-Firmware image is not valid, it cannot be loaded\n",
+			      __func__);
+
+		loadaddr = load_elf_image_phdr(LPM_DM);
+
+		memcpy((void *)*(ulong *)(LPM_BL31_START),
+		       (void *)LPM_BL31, *(ulong *)(LPM_BL31_SIZE));
+
+		ret = rproc_load(1, *(ulong *)(LPM_BL31_START), 0x200);
+		if (ret)
+			panic("%s: ATF failed to load on rproc (%d)\n", __func__, ret);
+
+#if (CONFIG_IS_ENABLED(FIT_IMAGE_POST_PROCESS) && IS_ENABLED(CONFIG_SYS_K3_SPL_ATF))
+		debug("%s: Authenticating image: addr=%lx, size=%ld, os=%s\n", __func__,
+		      *(ulong *)(LPM_BL31_START), *(ulong *)(LPM_BL31_SIZE),
+		      image_os_match[IMAGE_ID_ATF]);
+
+		ti_secure_image_post_process((void *)LPM_BL31_START,
+					     (size_t *)LPM_BL31_SIZE);
+#endif
+
+		debug("%s: jumping to address %x\n", __func__, loadaddr);
+		goto start_arm64;
+#endif /* IS_ENABLED(CONFIG_SOC_K3_J721E) */
+	}
+
 	init_env();
 
 	if (!fit_image_info[IMAGE_ID_DM_FW].image_start) {
@@ -250,6 +285,14 @@ void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 		fit_image_info[IMAGE_ID_ATF].image_start =
 			spl_image->entry_point;
 
+#if IS_ENABLED(CONFIG_SOC_K3_J721E)
+	/* Save TF-A image, as at resume fit image is not processed */
+	memcpy((void *)LPM_BL31, (void *)fit_image_info[IMAGE_ID_ATF].image_start,
+	       fit_image_info[IMAGE_ID_ATF].image_len);
+	*(ulong *)(LPM_BL31_START) = fit_image_info[IMAGE_ID_ATF].image_start;
+	*(ulong *)(LPM_BL31_SIZE) = fit_image_info[IMAGE_ID_ATF].image_len;
+#endif
+
 	ret = rproc_load(1, fit_image_info[IMAGE_ID_ATF].image_start, 0x200);
 	if (ret)
 		panic("%s: ATF failed to load on rproc (%d)\n", __func__, ret);
@@ -289,8 +332,18 @@ void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 		loadaddr = load_elf_image_phdr(loadaddr);
 	} else {
 		loadaddr = fit_image_info[IMAGE_ID_DM_FW].image_start;
-		if (valid_elf_image(loadaddr))
+		if (valid_elf_image(loadaddr)) {
 			loadaddr = load_elf_image_phdr(loadaddr);
+#if IS_ENABLED(CONFIG_SOC_K3_J721E)
+			if (fit_image_info[IMAGE_ID_DM_FW].image_len > (BUFFER_ADDR - LPM_DM))
+				log_warning("%s\n: Not enough space to save DM-Firmware",
+					    __func__);
+			else
+				memcpy((void *)LPM_DM,
+				       (void *)fit_image_info[IMAGE_ID_DM_FW].image_start,
+				       fit_image_info[IMAGE_ID_DM_FW].image_len);
+#endif
+		}
 	}
 
 	debug("%s: jumping to address %x\n", __func__, loadaddr);
@@ -299,6 +352,16 @@ start_arm64:
 	/* Add an extra newline to differentiate the ATF logs from SPL */
 	printf("Starting ATF on ARM64 core...\n\n");
 
+#if IS_ENABLED(CONFIG_SOC_K3_J721E)
+	/*
+	 * Write a magic value in scratchpad ram to notify TF-A that the board
+	 * is resuming
+	 */
+	if (board_is_resuming())
+		*(u32 *)(SCRACTHPAD_RAM_BASE) = BL31_MAGIC_SUSPEND;
+	else
+		*(u32 *)(SCRACTHPAD_RAM_BASE) = 0x00;
+#endif
 	ret = rproc_start(1);
 	if (ret)
 		panic("%s: ATF failed to start on rproc (%d)\n", __func__, ret);
diff --git a/arch/arm/mach-k3/include/mach/j721e_hardware.h b/arch/arm/mach-k3/include/mach/j721e_hardware.h
index 376db389ba..29f4030f5e 100644
--- a/arch/arm/mach-k3/include/mach/j721e_hardware.h
+++ b/arch/arm/mach-k3/include/mach/j721e_hardware.h
@@ -15,6 +15,7 @@
 #define WKUP_CTRL_MMR0_BASE				0x43000000
 #define MCU_CTRL_MMR0_BASE				0x40f00000
 #define CTRL_MMR0_BASE					0x00100000
+#define SCRACTHPAD_RAM_BASE				0x40280000
 
 #define CTRLMMR_MAIN_DEVSTAT				(CTRL_MMR0_BASE + 0x30)
 #define MAIN_DEVSTAT_BOOT_MODE_B_MASK		BIT(0)
diff --git a/arch/arm/mach-k3/include/mach/j721e_spl.h b/arch/arm/mach-k3/include/mach/j721e_spl.h
index e8947917a6..253e36b239 100644
--- a/arch/arm/mach-k3/include/mach/j721e_spl.h
+++ b/arch/arm/mach-k3/include/mach/j721e_spl.h
@@ -42,4 +42,34 @@
 #define K3_PRIMARY_BOOTMODE		0x0
 #define K3_BACKUP_BOOTMODE		0x1
 
+/* Starting buffer address is 1MB before the stack address in DDR */
+#define BUFFER_ADDR (CONFIG_SPL_STACK_R_ADDR - SZ_1M)
+
+/* This is actually the whole size of the SRAM */
+#define BL31_SIZE    0x20000
+
+/* This address belongs to a reserved memory region for the point of view of
+ * Linux, U-boot SPL must use the same address to restore TF-A and resume
+ * entry point address
+ */
+#define LPM_SAVE		0xA5000000
+#define LPM_BL31		LPM_SAVE
+#define LPM_BL31_START		LPM_BL31 + BL31_SIZE
+#define LPM_BL31_SIZE		LPM_BL31_START + 4
+#define LPM_DM			LPM_BL31_SIZE  + 4
+
+/* Check if the copy of TF-A and DM-Firmware in DRAM does not overlap an
+ * over memory section.
+ * The resume address of TF-A is also saved in DRAM.
+ * At build time we don't know the DM-Firmware size, so we keep 512k to
+ * save it.
+ */
+#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_TARGET_J7200_R5_EVM)
+#if ((LPM_DM + SZ_512K) > BUFFER_ADDR)
+#error Not enough space to save DM-Firmware and TF-A for S2R
+#endif
+#endif
+
+#define BL31_MAGIC_SUSPEND 0xA5A5A5A5
+
 #endif
diff --git a/arch/arm/mach-k3/sysfw-loader.c b/arch/arm/mach-k3/sysfw-loader.c
index 9be2d9eaea..e6d452e590 100644
--- a/arch/arm/mach-k3/sysfw-loader.c
+++ b/arch/arm/mach-k3/sysfw-loader.c
@@ -84,13 +84,16 @@ static bool sysfw_loaded;
 static void *sysfw_load_address;
 
 /*
- * Populate SPL hook to override the default load address used by the SPL
- * loader function with a custom address for SYSFW loading.
+ * Populate SPL hook to override the default load address used by the
+ * SPL loader function with a custom address for SYSFW loading. In
+ * other case use also a custom address located in a reserved memory
+ * region. It ensures that Linux memory won't be corrupted by SPL during
+ * suspend to ram.
  */
 struct legacy_img_hdr *spl_get_load_buffer(ssize_t offset, size_t size)
 {
 	if (sysfw_loaded)
-		return (struct legacy_img_hdr *)(CONFIG_TEXT_BASE + offset);
+		return (struct legacy_img_hdr *)(BUFFER_ADDR + offset);
 	else if (sysfw_load_address)
 		return sysfw_load_address;
 	else
-- 
2.39.2

[PATCH v3 8/8] arm: mach-k3: j7200: Skip fit processing when resuming

[Thomas Richard]

No need to process fit image during resume.
All needed parts are already available in DRAM.

Based on the work of Gregory CLEMENT <gregory.clement@bootlin.com>

Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>

---

(no changes since v1)

 arch/arm/mach-k3/sysfw-loader.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-k3/sysfw-loader.c b/arch/arm/mach-k3/sysfw-loader.c
index e6d452e590..89daf9b7f8 100644
--- a/arch/arm/mach-k3/sysfw-loader.c
+++ b/arch/arm/mach-k3/sysfw-loader.c
@@ -100,6 +100,11 @@ struct legacy_img_hdr *spl_get_load_buffer(ssize_t offset, size_t size)
 		panic("SYSFW load address not defined!");
 }
 
+__weak int board_is_resuming(void)
+{
+	return 0;
+}
+
 /*
  * Populate SPL hook to skip the default SPL loader FIT post-processing steps
  * during SYSFW loading and return to the calling function so we can perform
@@ -107,7 +112,7 @@ struct legacy_img_hdr *spl_get_load_buffer(ssize_t offset, size_t size)
  */
 bool spl_load_simple_fit_skip_processing(void)
 {
-	return !sysfw_loaded;
+	return board_is_resuming() ? true : !sysfw_loaded;
 }
 
 static int fit_get_data_by_name(const void *fit, int images, const char *name,
-- 
2.39.2

Re: [PATCH v3 0/8] Suspend to RAM support for K3 J7200

[Nishanth Menon]

On 17:56-20240108, Thomas Richard wrote:
> This series is the U-Boot part of the work to add the suspend to RAM
> support for the K3 J7200 EVM board.
> 
> During the boot R5 SPL makes a copy of DM-Firmware and TF-A in memory.
> Resume detection is done by reading a magic value in a pmic register
> (set by DM-Firmware).
> 
> If a resume is detected, R5 SPL run the exit retention sequence of the
> DDR. Then it load TF-A and DM-Firmware using the copies done during the boot
> (fit image processing is skipped).
> Before to start TF-A, R5 SPL writes a magic value in scratchpad ram. This
> will be used by TF-A to detect that the board is resuming.
> 
> The copy of TF-A/DM-Firmware, the SPL stack and malloc are located in a
> reserved memory region (for the kernel point of view) to avoid any
> memory corruption.
> 
> This version is mostly to test the firewall protection with the suspend
> to ram.
> Some comments (for the v2) were not fixed in this version.
> This series has been tested with the series [1] to enable the firewall.
> At the end of the resume sequence, TF-A is well protected by the
> firewall, but OP-TEE remains unprotected.
> 
> [1] https://lore.kernel.org/all/20231229-binman-firewalling-v7-0-47ed4af303fe@ti.com/
> 
> 

And as usual, as I have already responded on https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/25120

My objection to this series mirrors what I have mentioned previously for
TFA as well - I am looking for some common sequence to be defined
between am62x and J7200 family rather than each go completely
tangentially, until that happens, please consider my standing NAK.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v3 0/8] Suspend to RAM support for K3 J7200

[Andrew Davis]

On 1/8/24 10:56 AM, Thomas Richard wrote:
> This series is the U-Boot part of the work to add the suspend to RAM
> support for the K3 J7200 EVM board.
> 
> During the boot R5 SPL makes a copy of DM-Firmware and TF-A in memory.
> Resume detection is done by reading a magic value in a pmic register
> (set by DM-Firmware).
> 
> If a resume is detected, R5 SPL run the exit retention sequence of the
> DDR. Then it load TF-A and DM-Firmware using the copies done during the boot
> (fit image processing is skipped).
> Before to start TF-A, R5 SPL writes a magic value in scratchpad ram. This
> will be used by TF-A to detect that the board is resuming.
> 
> The copy of TF-A/DM-Firmware, the SPL stack and malloc are located in a
> reserved memory region (for the kernel point of view) to avoid any
> memory corruption.
> 
> This version is mostly to test the firewall protection with the suspend
> to ram.

Seems to show the opposite, if you are able to access and load TF-A
back to its spot after resume from un-trusted SPL then the firewall
did not survive suspend to RAM. That is a huge security gap if TIFS
is forgetting to put back the firewalls on resume.. What is the
point of firewalls on these systems if I can just knock them all
out by doing a simple suspend/resume cycle?

> Some comments (for the v2) were not fixed in this version.

Why not?

> This series has been tested with the series [1] to enable the firewall.
> At the end of the resume sequence, TF-A is well protected by the
> firewall, but OP-TEE remains unprotected.
> 

Then why post this? If it is just to get some eyes on it, then label
it as an "RFC" so our silence isn't considered acceptance, otherwise we
have to manually NAK these each time.

Andrew

> [1] https://lore.kernel.org/all/20231229-binman-firewalling-v7-0-47ed4af303fe@ti.com/
> 
> 
> Changes in v3:
> - At resume, R5 SPL doesn't restore TF-A anymore.
>    TF-A is started like during a cold boot. R5 SPL will notify that the board is
>    resuming using a magic value written in the scratchpad ram.
>    TF-A will restore itself.
> - Link to v2:
>    https://lore.kernel.org/u-boot/20231107161802.855154-1-thomas.richard@bootlin.com/
> 
> Changes in v2:
> - Set exit retention code for DDR behind CONFIG_K3_J721E_DDRSS
> - Check if TF-A is running in DRAM, if yes no need to restore it
> - Remove BL31_START macro, and get TF-A start address from the fit image
> - Remove the test_enter_suspend command
> - Link to v1:
>    https://lore.kernel.org/u-boot/20231016141135.325698-1-thomas.richard@bootlin.com/
> 
> Gowtham Tammana (1):
>    DO NOT MERGE: arm: dts: k3-j7200-r5-common: Add pmic node for esm
> 
> Gregory CLEMENT (2):
>    configs: j7200_evm_r5: Used reserved memory in DDR for stack
>    configs: j7200_evm_r5: Move address used for allocation in the
>      reserved space
> 
> Thomas Richard (5):
>    board: ti: j721e: Add resume detection for J7200
>    ram: k3-ddrss: Add exit retention support
>    board: ti: j721e: Add the missing part of exit retention for k3-ddrss
>      (J7200)
>    board: ti: j721e: During resume spl notify tf-a that the board is
>      resuming
>    arm: mach-k3: j7200: Skip fit processing when resuming
> 
>   .../arm/dts/k3-j7200-r5-common-proc-board.dts |  17 +-
>   arch/arm/mach-k3/common.c                     |  65 +++-
>   .../arm/mach-k3/include/mach/j721e_hardware.h |   1 +
>   arch/arm/mach-k3/include/mach/j721e_spl.h     |  30 ++
>   arch/arm/mach-k3/sysfw-loader.c               |  16 +-
>   board/ti/j721e/evm.c                          |  77 +++++
>   configs/j7200_evm_r5_defconfig                |   4 +-
>   drivers/ram/k3-ddrss/k3-ddrss.c               | 307 ++++++++++++++++++
>   8 files changed, 507 insertions(+), 10 deletions(-)
> 

Re: [PATCH v3 0/8] Suspend to RAM support for K3 J7200

[Thomas Richard]

On 1/9/24 18:32, Andrew Davis wrote:
> On 1/8/24 10:56 AM, Thomas Richard wrote:
>> This series is the U-Boot part of the work to add the suspend to RAM
>> support for the K3 J7200 EVM board.
>>
>> During the boot R5 SPL makes a copy of DM-Firmware and TF-A in memory.
>> Resume detection is done by reading a magic value in a pmic register
>> (set by DM-Firmware).
>>
>> If a resume is detected, R5 SPL run the exit retention sequence of the
>> DDR. Then it load TF-A and DM-Firmware using the copies done during
>> the boot
>> (fit image processing is skipped).
>> Before to start TF-A, R5 SPL writes a magic value in scratchpad ram. This
>> will be used by TF-A to detect that the board is resuming.
>>
>> The copy of TF-A/DM-Firmware, the SPL stack and malloc are located in a
>> reserved memory region (for the kernel point of view) to avoid any
>> memory corruption.
>>
>> This version is mostly to test the firewall protection with the suspend
>> to ram.
> 
> Seems to show the opposite, if you are able to access and load TF-A
> back to its spot after resume from un-trusted SPL then the firewall
> did not survive suspend to RAM. That is a huge security gap if TIFS
> is forgetting to put back the firewalls on resume.. What is the
> point of firewalls on these systems if I can just knock them all
> out by doing a simple suspend/resume cycle?

Hello Andrew,

Why are you talking about un-trusted SPL, how R5 SPL can be un-trusted ?
Our resume sequence starts like a power-on: ROM code is started, it
loads TIFS and R5 SPL.
As defined in the chain of trust [1], ROM authenticates TIFS and R5 SPL.
So I don't understand how R5 SPL can be un-trusted.

Then R5 SPL detects that the board is resuming (and does some specific
operations on the DDR).
But even if the board is resuming, TF-A is authenticated (using
ti_secure_image_post_process) like during the boot.

So once TF-A is loaded, firewall is active.
TF-A restores its context, then jump to its warm entrypoint.

I guess a weak point could be TF-A context (stored in DDR).

[1] https://docs.u-boot.org/en/latest/board/ti/k3.html#chain-of-trust

> 
>> Some comments (for the v2) were not fixed in this version.
> 
> Why not?
> 
>> This series has been tested with the series [1] to enable the firewall.
>> At the end of the resume sequence, TF-A is well protected by the
>> firewall, but OP-TEE remains unprotected.
>>
> 
> Then why post this? If it is just to get some eyes on it, then label
> it as an "RFC" so our silence isn't considered acceptance, otherwise we
> have to manually NAK these each time.

Yes sorry, I totally forgot to label it as RFC.

Regards,

Thomas

Re: [PATCH v3 0/8] Suspend to RAM support for K3 J7200

[Andrew Davis]

On 1/10/24 3:34 AM, Thomas Richard wrote:
> On 1/9/24 18:32, Andrew Davis wrote:
>> On 1/8/24 10:56 AM, Thomas Richard wrote:
>>> This series is the U-Boot part of the work to add the suspend to RAM
>>> support for the K3 J7200 EVM board.
>>>
>>> During the boot R5 SPL makes a copy of DM-Firmware and TF-A in memory.
>>> Resume detection is done by reading a magic value in a pmic register
>>> (set by DM-Firmware).
>>>
>>> If a resume is detected, R5 SPL run the exit retention sequence of the
>>> DDR. Then it load TF-A and DM-Firmware using the copies done during
>>> the boot
>>> (fit image processing is skipped).
>>> Before to start TF-A, R5 SPL writes a magic value in scratchpad ram. This
>>> will be used by TF-A to detect that the board is resuming.
>>>
>>> The copy of TF-A/DM-Firmware, the SPL stack and malloc are located in a
>>> reserved memory region (for the kernel point of view) to avoid any
>>> memory corruption.
>>>
>>> This version is mostly to test the firewall protection with the suspend
>>> to ram.
>>
>> Seems to show the opposite, if you are able to access and load TF-A
>> back to its spot after resume from un-trusted SPL then the firewall
>> did not survive suspend to RAM. That is a huge security gap if TIFS
>> is forgetting to put back the firewalls on resume.. What is the
>> point of firewalls on these systems if I can just knock them all
>> out by doing a simple suspend/resume cycle?
> 
> Hello Andrew,
> 
> Why are you talking about un-trusted SPL, how R5 SPL can be un-trusted ?

Very easily, when was the last time it had a proper security audit,
was it written from the ground up with security in mind, what secure
software development standards does it follow, etc.. No offense to
SPL, it is very good at what it was designed for, but it was not
designed for security like OP-TEE or TF-A.

> Our resume sequence starts like a power-on: ROM code is started, it
> loads TIFS and R5 SPL.
> As defined in the chain of trust [1], ROM authenticates TIFS and R5 SPL.
> So I don't understand how R5 SPL can be un-trusted.
> 

You are making a common mistake here, just because some code was
authenticated before it started, doesn't mean it is invulnerable to
attack and cannot be compromised. Authentication only protects
against modification on the boot media.

> Then R5 SPL detects that the board is resuming (and does some specific
> operations on the DDR).
> But even if the board is resuming, TF-A is authenticated (using
> ti_secure_image_post_process) like during the boot.

Right, and this is the only thing that saves us here. TF-A being
authenticated by a higher trust entity like TIFS. My comment
was more about the context in DDR as you point out below. I'm
assuming TF-A will gain some way to authenticate that while
loading it in. Right now R5 SPL could read it out or modify
it (and as above, we don't trust R5 SPL), so having that in
plantext and un-firewalled at that point is a problem that
needs fixed for this solution to be complete.

To be fair that is a TF-A problem, but both need to be in place.

Andrew

> 
> So once TF-A is loaded, firewall is active.
> TF-A restores its context, then jump to its warm entrypoint.
> 
> I guess a weak point could be TF-A context (stored in DDR).
> 
> [1] https://docs.u-boot.org/en/latest/board/ti/k3.html#chain-of-trust
> 
>>
>>> Some comments (for the v2) were not fixed in this version.
>>
>> Why not?
>>
>>> This series has been tested with the series [1] to enable the firewall.
>>> At the end of the resume sequence, TF-A is well protected by the
>>> firewall, but OP-TEE remains unprotected.
>>>
>>
>> Then why post this? If it is just to get some eyes on it, then label
>> it as an "RFC" so our silence isn't considered acceptance, otherwise we
>> have to manually NAK these each time.
> 
> Yes sorry, I totally forgot to label it as RFC.
> 
> Regards,
> 
> Thomas