All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] *** Introduce get_boot_device() for K3 platform ***
@ 2024-02-26 13:30 Wadim Egorov
  2024-02-26 13:30 ` [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL Wadim Egorov
  2024-02-26 13:30 ` [PATCH 2/2] arm: mach-k3: am625: Provide a way to obtain boot device for non SPLs Wadim Egorov
  0 siblings, 2 replies; 13+ messages in thread
From: Wadim Egorov @ 2024-02-26 13:30 UTC (permalink / raw)
  To: u-boot; +Cc: nm, upstream, bb

It is handy to have some u-boot environment variables set based on
the current booting device.
Provide a way to obtain the boot device for non SPLs by factoring out
spl_boot_device() into an own function get_boot_device().

Wadim Egorov (2):
  arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  arm: mach-k3: am625: Provide a way to obtain boot device for non SPLs

 arch/arm/mach-k3/Kconfig                      |   3 +-
 arch/arm/mach-k3/Makefile                     |   1 +
 arch/arm/mach-k3/am625_init.c                 | 110 +++---------------
 arch/arm/mach-k3/am62x/Makefile               |   2 +
 arch/arm/mach-k3/am62x/boot.c                 | 103 ++++++++++++++++
 arch/arm/mach-k3/include/mach/am62_hardware.h |  15 +++
 arch/arm/mach-k3/include/mach/hardware.h      |   1 +
 7 files changed, 138 insertions(+), 97 deletions(-)
 create mode 100644 arch/arm/mach-k3/am62x/Makefile
 create mode 100644 arch/arm/mach-k3/am62x/boot.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  2024-02-26 13:30 [PATCH 0/2] *** Introduce get_boot_device() for K3 platform *** Wadim Egorov
@ 2024-02-26 13:30 ` Wadim Egorov
  2024-03-04  5:06   ` Vignesh Raghavendra
  2024-02-26 13:30 ` [PATCH 2/2] arm: mach-k3: am625: Provide a way to obtain boot device for non SPLs Wadim Egorov
  1 sibling, 1 reply; 13+ messages in thread
From: Wadim Egorov @ 2024-02-26 13:30 UTC (permalink / raw)
  To: u-boot; +Cc: nm, upstream, bb

Texas Instruments has begun enabling security settings on the SoCs it
produces to instruct ROM and TIFS to begin protecting the Security
Management Subsystem (SMS) from other binaries we load into the chip by
default.

One way ROM and TIFS do this is by enabling firewalls to protect the
OCSRAM and HSM RAM regions they're using during bootup.

The HSM RAM the wakeup SPL is in is firewalled by TIFS to protect
itself from the main domain applications. This means the 'bootindex'
value in HSM RAM, left by ROM to indicate if we're using the primary
or secondary boot-method, must be moved to OCSRAM (that TIFS has open
for us) before we make the jump to the main domain so the main domain's
bootloaders can keep access to this information.

Based on commit
  b672e8581070 ("arm: mach-k3: copy bootindex to OCRAM for main domain SPL")

Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
---
 arch/arm/mach-k3/Kconfig                      |  3 ++-
 arch/arm/mach-k3/am625_init.c                 | 15 +++++++++++++--
 arch/arm/mach-k3/include/mach/am62_hardware.h | 15 +++++++++++++++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
index 03898424c9..f5d06593f7 100644
--- a/arch/arm/mach-k3/Kconfig
+++ b/arch/arm/mach-k3/Kconfig
@@ -75,7 +75,8 @@ config SYS_K3_BOOT_PARAM_TABLE_INDEX
 	default 0x41cffbfc if SOC_K3_J721E
 	default 0x41cfdbfc if SOC_K3_J721S2
 	default 0x701bebfc if SOC_K3_AM642
-	default 0x43c3f290 if SOC_K3_AM625
+	default 0x43c3f290 if SOC_K3_AM625 && CPU_V7R
+	default 0x7000f290 if SOC_K3_AM625 && ARM64
 	default 0x43c3f290 if SOC_K3_AM62A7 && CPU_V7R
 	default 0x7000f290 if SOC_K3_AM62A7 && ARM64
 	help
diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
index 6c96e88114..67cf63b103 100644
--- a/arch/arm/mach-k3/am625_init.c
+++ b/arch/arm/mach-k3/am625_init.c
@@ -35,8 +35,10 @@ static struct rom_extended_boot_data bootdata __section(".data");
 static void store_boot_info_from_rom(void)
 {
 	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
-	memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
-	       sizeof(struct rom_extended_boot_data));
+	if (IS_ENABLED(CONFIG_CPU_V7R)) {
+		memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
+		       sizeof(struct rom_extended_boot_data));
+	}
 }
 
 static void ctrl_mmr_unlock(void)
@@ -175,6 +177,15 @@ void board_init_f(ulong dummy)
 		k3_sysfw_loader(true, NULL, NULL);
 	}
 
+#if defined(CONFIG_CPU_V7R)
+	/*
+	 * Relocate boot information to OCRAM (after TIFS has opend this
+	 * region for us) so the next bootloader stages can keep access to
+	 * primary vs backup bootmodes.
+	 */
+	writel(bootindex, K3_BOOT_PARAM_TABLE_INDEX_OCRAM);
+#endif
+
 	/*
 	 * Force probe of clk_k3 driver here to ensure basic default clock
 	 * configuration is always done.
diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h
index 54380f36e1..9f504f4642 100644
--- a/arch/arm/mach-k3/include/mach/am62_hardware.h
+++ b/arch/arm/mach-k3/include/mach/am62_hardware.h
@@ -76,8 +76,23 @@
 #define CTRLMMR_MCU_RST_CTRL			(MCU_CTRL_MMR0_BASE + 0x18170)
 
 #define ROM_EXTENDED_BOOT_DATA_INFO		0x43c3f1e0
+#define K3_BOOT_PARAM_TABLE_INDEX_OCRAM         0x7000F290
 
+/*
+ * During the boot process ROM will kill anything that writes to OCSRAM.
+ * This means the wakeup SPL cannot use this region during boot. To
+ * complicate things, TIFS will set a firewall between HSM RAM and the
+ * main domain.
+ *
+ * So, during the wakeup SPL, we will need to store the EEPROM data
+ * somewhere in HSM RAM, and the main domain's SPL will need to store it
+ * somewhere in OCSRAM
+ */
+#ifdef CONFIG_CPU_V7R
 #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x43c30000
+#else
+ #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x70000001
+#endif /* CONFIG_CPU_V7R */
 
 static inline int k3_get_core_nr(void)
 {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] arm: mach-k3: am625: Provide a way to obtain boot device for non SPLs
  2024-02-26 13:30 [PATCH 0/2] *** Introduce get_boot_device() for K3 platform *** Wadim Egorov
  2024-02-26 13:30 ` [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL Wadim Egorov
@ 2024-02-26 13:30 ` Wadim Egorov
  1 sibling, 0 replies; 13+ messages in thread
From: Wadim Egorov @ 2024-02-26 13:30 UTC (permalink / raw)
  To: u-boot; +Cc: nm, upstream, bb

Introduce get_boot_device() to obtain the booting device. Make it also
available for non SPL builds so u-boot can also know the device it is
booting from.

Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
---
 arch/arm/mach-k3/Makefile                |   1 +
 arch/arm/mach-k3/am625_init.c            |  95 +--------------------
 arch/arm/mach-k3/am62x/Makefile          |   2 +
 arch/arm/mach-k3/am62x/boot.c            | 103 +++++++++++++++++++++++
 arch/arm/mach-k3/include/mach/hardware.h |   1 +
 5 files changed, 108 insertions(+), 94 deletions(-)
 create mode 100644 arch/arm/mach-k3/am62x/Makefile
 create mode 100644 arch/arm/mach-k3/am62x/boot.c

diff --git a/arch/arm/mach-k3/Makefile b/arch/arm/mach-k3/Makefile
index 4216137646..61ec4ea13c 100644
--- a/arch/arm/mach-k3/Makefile
+++ b/arch/arm/mach-k3/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_SOC_K3_AM625) += am625_init.o
 obj-$(CONFIG_SOC_K3_AM62A7) += am62a7_init.o
 endif
 obj-y += common.o security.o
+obj-$(CONFIG_SOC_K3_AM625) += am62x/
diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
index 67cf63b103..85cd3f895a 100644
--- a/arch/arm/mach-k3/am625_init.c
+++ b/arch/arm/mach-k3/am625_init.c
@@ -246,100 +246,7 @@ u32 spl_mmc_boot_mode(struct mmc *mmc, const u32 boot_device)
 	}
 }
 
-static u32 __get_backup_bootmedia(u32 devstat)
-{
-	u32 bkup_bootmode = (devstat & MAIN_DEVSTAT_BACKUP_BOOTMODE_MASK) >>
-				MAIN_DEVSTAT_BACKUP_BOOTMODE_SHIFT;
-	u32 bkup_bootmode_cfg =
-			(devstat & MAIN_DEVSTAT_BACKUP_BOOTMODE_CFG_MASK) >>
-				MAIN_DEVSTAT_BACKUP_BOOTMODE_CFG_SHIFT;
-
-	switch (bkup_bootmode) {
-	case BACKUP_BOOT_DEVICE_UART:
-		return BOOT_DEVICE_UART;
-
-	case BACKUP_BOOT_DEVICE_USB:
-		return BOOT_DEVICE_USB;
-
-	case BACKUP_BOOT_DEVICE_ETHERNET:
-		return BOOT_DEVICE_ETHERNET;
-
-	case BACKUP_BOOT_DEVICE_MMC:
-		if (bkup_bootmode_cfg)
-			return BOOT_DEVICE_MMC2;
-		return BOOT_DEVICE_MMC1;
-
-	case BACKUP_BOOT_DEVICE_SPI:
-		return BOOT_DEVICE_SPI;
-
-	case BACKUP_BOOT_DEVICE_I2C:
-		return BOOT_DEVICE_I2C;
-
-	case BACKUP_BOOT_DEVICE_DFU:
-		if (bkup_bootmode_cfg & MAIN_DEVSTAT_BACKUP_USB_MODE_MASK)
-			return BOOT_DEVICE_USB;
-		return BOOT_DEVICE_DFU;
-	};
-
-	return BOOT_DEVICE_RAM;
-}
-
-static u32 __get_primary_bootmedia(u32 devstat)
-{
-	u32 bootmode = (devstat & MAIN_DEVSTAT_PRIMARY_BOOTMODE_MASK) >>
-				MAIN_DEVSTAT_PRIMARY_BOOTMODE_SHIFT;
-	u32 bootmode_cfg = (devstat & MAIN_DEVSTAT_PRIMARY_BOOTMODE_CFG_MASK) >>
-				MAIN_DEVSTAT_PRIMARY_BOOTMODE_CFG_SHIFT;
-
-	switch (bootmode) {
-	case BOOT_DEVICE_OSPI:
-		fallthrough;
-	case BOOT_DEVICE_QSPI:
-		fallthrough;
-	case BOOT_DEVICE_XSPI:
-		fallthrough;
-	case BOOT_DEVICE_SPI:
-		return BOOT_DEVICE_SPI;
-
-	case BOOT_DEVICE_ETHERNET_RGMII:
-		fallthrough;
-	case BOOT_DEVICE_ETHERNET_RMII:
-		return BOOT_DEVICE_ETHERNET;
-
-	case BOOT_DEVICE_EMMC:
-		return BOOT_DEVICE_MMC1;
-
-	case BOOT_DEVICE_MMC:
-		if ((bootmode_cfg & MAIN_DEVSTAT_PRIMARY_MMC_PORT_MASK) >>
-				MAIN_DEVSTAT_PRIMARY_MMC_PORT_SHIFT)
-			return BOOT_DEVICE_MMC2;
-		return BOOT_DEVICE_MMC1;
-
-	case BOOT_DEVICE_DFU:
-		if ((bootmode_cfg & MAIN_DEVSTAT_PRIMARY_USB_MODE_MASK) >>
-		    MAIN_DEVSTAT_PRIMARY_USB_MODE_SHIFT)
-			return BOOT_DEVICE_USB;
-		return BOOT_DEVICE_DFU;
-
-	case BOOT_DEVICE_NOBOOT:
-		return BOOT_DEVICE_RAM;
-	}
-
-	return bootmode;
-}
-
 u32 spl_boot_device(void)
 {
-	u32 devstat = readl(CTRLMMR_MAIN_DEVSTAT);
-	u32 bootmedia;
-
-	if (bootindex == K3_PRIMARY_BOOTMODE)
-		bootmedia = __get_primary_bootmedia(devstat);
-	else
-		bootmedia = __get_backup_bootmedia(devstat);
-
-	debug("am625_init: %s: devstat = 0x%x bootmedia = 0x%x bootindex = %d\n",
-	      __func__, devstat, bootmedia, bootindex);
-
-	return bootmedia;
+	return get_boot_device();
 }
diff --git a/arch/arm/mach-k3/am62x/Makefile b/arch/arm/mach-k3/am62x/Makefile
new file mode 100644
index 0000000000..acf09c3426
--- /dev/null
+++ b/arch/arm/mach-k3/am62x/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier:	GPL-2.0+
+obj-y += boot.o
diff --git a/arch/arm/mach-k3/am62x/boot.c b/arch/arm/mach-k3/am62x/boot.c
new file mode 100644
index 0000000000..966500edc9
--- /dev/null
+++ b/arch/arm/mach-k3/am62x/boot.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0+
+#include <asm/io.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/am62_spl.h>
+
+static u32 __get_backup_bootmedia(u32 devstat)
+{
+	u32 bkup_bootmode = (devstat & MAIN_DEVSTAT_BACKUP_BOOTMODE_MASK) >>
+				MAIN_DEVSTAT_BACKUP_BOOTMODE_SHIFT;
+	u32 bkup_bootmode_cfg =
+			(devstat & MAIN_DEVSTAT_BACKUP_BOOTMODE_CFG_MASK) >>
+				MAIN_DEVSTAT_BACKUP_BOOTMODE_CFG_SHIFT;
+
+	switch (bkup_bootmode) {
+	case BACKUP_BOOT_DEVICE_UART:
+		return BOOT_DEVICE_UART;
+
+	case BACKUP_BOOT_DEVICE_USB:
+		return BOOT_DEVICE_USB;
+
+	case BACKUP_BOOT_DEVICE_ETHERNET:
+		return BOOT_DEVICE_ETHERNET;
+
+	case BACKUP_BOOT_DEVICE_MMC:
+		if (bkup_bootmode_cfg)
+			return BOOT_DEVICE_MMC2;
+		return BOOT_DEVICE_MMC1;
+
+	case BACKUP_BOOT_DEVICE_SPI:
+		return BOOT_DEVICE_SPI;
+
+	case BACKUP_BOOT_DEVICE_I2C:
+		return BOOT_DEVICE_I2C;
+
+	case BACKUP_BOOT_DEVICE_DFU:
+		if (bkup_bootmode_cfg & MAIN_DEVSTAT_BACKUP_USB_MODE_MASK)
+			return BOOT_DEVICE_USB;
+		return BOOT_DEVICE_DFU;
+	};
+
+	return BOOT_DEVICE_RAM;
+}
+
+static u32 __get_primary_bootmedia(u32 devstat)
+{
+	u32 bootmode = (devstat & MAIN_DEVSTAT_PRIMARY_BOOTMODE_MASK) >>
+				MAIN_DEVSTAT_PRIMARY_BOOTMODE_SHIFT;
+	u32 bootmode_cfg = (devstat & MAIN_DEVSTAT_PRIMARY_BOOTMODE_CFG_MASK) >>
+				MAIN_DEVSTAT_PRIMARY_BOOTMODE_CFG_SHIFT;
+
+	switch (bootmode) {
+	case BOOT_DEVICE_OSPI:
+		fallthrough;
+	case BOOT_DEVICE_QSPI:
+		fallthrough;
+	case BOOT_DEVICE_XSPI:
+		fallthrough;
+	case BOOT_DEVICE_SPI:
+		return BOOT_DEVICE_SPI;
+
+	case BOOT_DEVICE_ETHERNET_RGMII:
+		fallthrough;
+	case BOOT_DEVICE_ETHERNET_RMII:
+		return BOOT_DEVICE_ETHERNET;
+
+	case BOOT_DEVICE_EMMC:
+		return BOOT_DEVICE_MMC1;
+
+	case BOOT_DEVICE_MMC:
+		if ((bootmode_cfg & MAIN_DEVSTAT_PRIMARY_MMC_PORT_MASK) >>
+				MAIN_DEVSTAT_PRIMARY_MMC_PORT_SHIFT)
+			return BOOT_DEVICE_MMC2;
+		return BOOT_DEVICE_MMC1;
+
+	case BOOT_DEVICE_DFU:
+		if ((bootmode_cfg & MAIN_DEVSTAT_PRIMARY_USB_MODE_MASK) >>
+		    MAIN_DEVSTAT_PRIMARY_USB_MODE_SHIFT)
+			return BOOT_DEVICE_USB;
+		return BOOT_DEVICE_DFU;
+
+	case BOOT_DEVICE_NOBOOT:
+		return BOOT_DEVICE_RAM;
+	}
+
+	return bootmode;
+}
+
+u32 get_boot_device(void)
+{
+	u32 devstat = readl(CTRLMMR_MAIN_DEVSTAT);
+	u32 bootmode = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
+	u32 bootmedia;
+
+	if (bootmode == K3_PRIMARY_BOOTMODE)
+		bootmedia = __get_primary_bootmedia(devstat);
+	else
+		bootmedia = __get_backup_bootmedia(devstat);
+
+	debug("am625_init: %s: devstat = 0x%x bootmedia = 0x%x bootmode = %d\n",
+	      __func__, devstat, bootmedia, bootmode);
+
+	return bootmedia;
+}
diff --git a/arch/arm/mach-k3/include/mach/hardware.h b/arch/arm/mach-k3/include/mach/hardware.h
index a1a9dfbde6..0511d9fbdf 100644
--- a/arch/arm/mach-k3/include/mach/hardware.h
+++ b/arch/arm/mach-k3/include/mach/hardware.h
@@ -105,4 +105,5 @@ struct k3_qos_data {
 extern struct k3_qos_data am62a_qos_data[];
 extern u32 am62a_qos_count;
 
+u32 get_boot_device(void);
 #endif /* _ASM_ARCH_HARDWARE_H_ */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  2024-02-26 13:30 ` [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL Wadim Egorov
@ 2024-03-04  5:06   ` Vignesh Raghavendra
  2024-03-04 20:27     ` Bryan Brattlof
  2024-03-06 13:44     ` Wadim Egorov
  0 siblings, 2 replies; 13+ messages in thread
From: Vignesh Raghavendra @ 2024-03-04  5:06 UTC (permalink / raw)
  To: Wadim Egorov, u-boot; +Cc: nm, upstream, bb

Hi Wadim,

On 26/02/24 19:00, Wadim Egorov wrote:
> Texas Instruments has begun enabling security settings on the SoCs it
> produces to instruct ROM and TIFS to begin protecting the Security
> Management Subsystem (SMS) from other binaries we load into the chip by
> default.
> 
> One way ROM and TIFS do this is by enabling firewalls to protect the
> OCSRAM and HSM RAM regions they're using during bootup.
> 
> The HSM RAM the wakeup SPL is in is firewalled by TIFS to protect
> itself from the main domain applications. This means the 'bootindex'
> value in HSM RAM, left by ROM to indicate if we're using the primary
> or secondary boot-method, must be moved to OCSRAM (that TIFS has open
> for us) before we make the jump to the main domain so the main domain's
> bootloaders can keep access to this information.
> 
> Based on commit
>   b672e8581070 ("arm: mach-k3: copy bootindex to OCRAM for main domain SPL")
> 

FYI, this is mostly a problem in non SPL flow (TI prosperity SBL for
example) where HSM RAM would be used by HSM firmware. This should be a
issue in R5 SPL flow.  Do you see any issues today? If so, whats the
TIFS firmware being used?

> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
> ---
>  arch/arm/mach-k3/Kconfig                      |  3 ++-
>  arch/arm/mach-k3/am625_init.c                 | 15 +++++++++++++--
>  arch/arm/mach-k3/include/mach/am62_hardware.h | 15 +++++++++++++++
>  3 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> index 03898424c9..f5d06593f7 100644
> --- a/arch/arm/mach-k3/Kconfig
> +++ b/arch/arm/mach-k3/Kconfig
> @@ -75,7 +75,8 @@ config SYS_K3_BOOT_PARAM_TABLE_INDEX
>  	default 0x41cffbfc if SOC_K3_J721E
>  	default 0x41cfdbfc if SOC_K3_J721S2
>  	default 0x701bebfc if SOC_K3_AM642
> -	default 0x43c3f290 if SOC_K3_AM625
> +	default 0x43c3f290 if SOC_K3_AM625 && CPU_V7R
> +	default 0x7000f290 if SOC_K3_AM625 && ARM64
>  	default 0x43c3f290 if SOC_K3_AM62A7 && CPU_V7R
>  	default 0x7000f290 if SOC_K3_AM62A7 && ARM64
>  	help
> diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
> index 6c96e88114..67cf63b103 100644
> --- a/arch/arm/mach-k3/am625_init.c
> +++ b/arch/arm/mach-k3/am625_init.c
> @@ -35,8 +35,10 @@ static struct rom_extended_boot_data bootdata __section(".data");
>  static void store_boot_info_from_rom(void)
>  {
>  	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
> -	memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
> -	       sizeof(struct rom_extended_boot_data));
> +	if (IS_ENABLED(CONFIG_CPU_V7R)) {
> +		memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
> +		       sizeof(struct rom_extended_boot_data));
> +	}
>  }
>  
>  static void ctrl_mmr_unlock(void)
> @@ -175,6 +177,15 @@ void board_init_f(ulong dummy)
>  		k3_sysfw_loader(true, NULL, NULL);
>  	}
>  
> +#if defined(CONFIG_CPU_V7R)
> +	/*
> +	 * Relocate boot information to OCRAM (after TIFS has opend this
> +	 * region for us) so the next bootloader stages can keep access to
> +	 * primary vs backup bootmodes.
> +	 */
> +	writel(bootindex, K3_BOOT_PARAM_TABLE_INDEX_OCRAM);
> +#endif
> +
>  	/*
>  	 * Force probe of clk_k3 driver here to ensure basic default clock
>  	 * configuration is always done.
> diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h
> index 54380f36e1..9f504f4642 100644
> --- a/arch/arm/mach-k3/include/mach/am62_hardware.h
> +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h
> @@ -76,8 +76,23 @@
>  #define CTRLMMR_MCU_RST_CTRL			(MCU_CTRL_MMR0_BASE + 0x18170)
>  
>  #define ROM_EXTENDED_BOOT_DATA_INFO		0x43c3f1e0
> +#define K3_BOOT_PARAM_TABLE_INDEX_OCRAM         0x7000F290
>  
> +/*
> + * During the boot process ROM will kill anything that writes to OCSRAM.

R5 ROM is long gone when R5 SPL starts, how would it kill anything?

> + * This means the wakeup SPL cannot use this region during boot. To
> + * complicate things, TIFS will set a firewall between HSM RAM and the
> + * main domain.
> + *
> + * So, during the wakeup SPL, we will need to store the EEPROM data
> + * somewhere in HSM RAM, and the main domain's SPL will need to store it
> + * somewhere in OCSRAM
> + */
> +#ifdef CONFIG_CPU_V7R
>  #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x43c30000
> +#else
> + #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x70000001

Why not  0x70000000 ?

> +#endif /* CONFIG_CPU_V7R */
>  

Can't we store directly in OCRAM in both stages? This RAM should be
accessible post TIFS is up (ie post k3_sysfw_loader() call)

>  static inline int k3_get_core_nr(void)
>  {

-- 
Regards
Vignesh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  2024-03-04  5:06   ` Vignesh Raghavendra
@ 2024-03-04 20:27     ` Bryan Brattlof
  2024-03-05  4:36       ` Vignesh Raghavendra
  2024-03-06 13:44     ` Wadim Egorov
  1 sibling, 1 reply; 13+ messages in thread
From: Bryan Brattlof @ 2024-03-04 20:27 UTC (permalink / raw)
  To: Vignesh Raghavendra; +Cc: Wadim Egorov, u-boot, nm, upstream

Hey Vignesh!

On March  4, 2024 thus sayeth Vignesh Raghavendra:
> Hi Wadim,
> 
> On 26/02/24 19:00, Wadim Egorov wrote:
> > Texas Instruments has begun enabling security settings on the SoCs it
> > produces to instruct ROM and TIFS to begin protecting the Security
> > Management Subsystem (SMS) from other binaries we load into the chip by
> > default.
> > 
> > One way ROM and TIFS do this is by enabling firewalls to protect the
> > OCSRAM and HSM RAM regions they're using during bootup.
> > 
> > The HSM RAM the wakeup SPL is in is firewalled by TIFS to protect
> > itself from the main domain applications. This means the 'bootindex'
> > value in HSM RAM, left by ROM to indicate if we're using the primary
> > or secondary boot-method, must be moved to OCSRAM (that TIFS has open
> > for us) before we make the jump to the main domain so the main domain's
> > bootloaders can keep access to this information.
> > 
> > Based on commit
> >   b672e8581070 ("arm: mach-k3: copy bootindex to OCRAM for main domain SPL")
> > 
> 
> FYI, this is mostly a problem in non SPL flow (TI prosperity SBL for
> example) where HSM RAM would be used by HSM firmware. This should be a
> issue in R5 SPL flow.  Do you see any issues today? If so, whats the
> TIFS firmware being used?
> 
> > Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
> > ---
> >  arch/arm/mach-k3/Kconfig                      |  3 ++-
> >  arch/arm/mach-k3/am625_init.c                 | 15 +++++++++++++--
> >  arch/arm/mach-k3/include/mach/am62_hardware.h | 15 +++++++++++++++
> >  3 files changed, 30 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> > index 03898424c9..f5d06593f7 100644
> > --- a/arch/arm/mach-k3/Kconfig
> > +++ b/arch/arm/mach-k3/Kconfig
> > @@ -75,7 +75,8 @@ config SYS_K3_BOOT_PARAM_TABLE_INDEX
> >  	default 0x41cffbfc if SOC_K3_J721E
> >  	default 0x41cfdbfc if SOC_K3_J721S2
> >  	default 0x701bebfc if SOC_K3_AM642
> > -	default 0x43c3f290 if SOC_K3_AM625
> > +	default 0x43c3f290 if SOC_K3_AM625 && CPU_V7R
> > +	default 0x7000f290 if SOC_K3_AM625 && ARM64
> >  	default 0x43c3f290 if SOC_K3_AM62A7 && CPU_V7R
> >  	default 0x7000f290 if SOC_K3_AM62A7 && ARM64
> >  	help
> > diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
> > index 6c96e88114..67cf63b103 100644
> > --- a/arch/arm/mach-k3/am625_init.c
> > +++ b/arch/arm/mach-k3/am625_init.c
> > @@ -35,8 +35,10 @@ static struct rom_extended_boot_data bootdata __section(".data");
> >  static void store_boot_info_from_rom(void)
> >  {
> >  	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
> > -	memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
> > -	       sizeof(struct rom_extended_boot_data));
> > +	if (IS_ENABLED(CONFIG_CPU_V7R)) {
> > +		memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
> > +		       sizeof(struct rom_extended_boot_data));
> > +	}
> >  }
> >  
> >  static void ctrl_mmr_unlock(void)
> > @@ -175,6 +177,15 @@ void board_init_f(ulong dummy)
> >  		k3_sysfw_loader(true, NULL, NULL);
> >  	}
> >  
> > +#if defined(CONFIG_CPU_V7R)
> > +	/*
> > +	 * Relocate boot information to OCRAM (after TIFS has opend this
> > +	 * region for us) so the next bootloader stages can keep access to
> > +	 * primary vs backup bootmodes.
> > +	 */
> > +	writel(bootindex, K3_BOOT_PARAM_TABLE_INDEX_OCRAM);
> > +#endif
> > +
> >  	/*
> >  	 * Force probe of clk_k3 driver here to ensure basic default clock
> >  	 * configuration is always done.
> > diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h
> > index 54380f36e1..9f504f4642 100644
> > --- a/arch/arm/mach-k3/include/mach/am62_hardware.h
> > +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h
> > @@ -76,8 +76,23 @@
> >  #define CTRLMMR_MCU_RST_CTRL			(MCU_CTRL_MMR0_BASE + 0x18170)
> >  
> >  #define ROM_EXTENDED_BOOT_DATA_INFO		0x43c3f1e0
> > +#define K3_BOOT_PARAM_TABLE_INDEX_OCRAM         0x7000F290
> >  
> > +/*
> > + * During the boot process ROM will kill anything that writes to OCSRAM.
> 
> R5 ROM is long gone when R5 SPL starts, how would it kill anything?

Looks like this was based on my patch long ago for the AM62Ax family. 
From what little I remember about this was ROM is leaving behind a 
firewall that we need TIFS's help to bring down for us. So I just
blamed ROM ;)

IDK if this is an issue for the AM62x family though.

> 
> > + * This means the wakeup SPL cannot use this region during boot. To
> > + * complicate things, TIFS will set a firewall between HSM RAM and the
> > + * main domain.
> > + *
> > + * So, during the wakeup SPL, we will need to store the EEPROM data
> > + * somewhere in HSM RAM, and the main domain's SPL will need to store it
> > + * somewhere in OCSRAM
> > + */
> > +#ifdef CONFIG_CPU_V7R
> >  #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x43c30000
> > +#else
> > + #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x70000001
> 
> Why not  0x70000000 ?

Ah an off by one by me :) 0x70000000 should be fine

> 
> > +#endif /* CONFIG_CPU_V7R */
> >  
> 
> Can't we store directly in OCRAM in both stages? This RAM should be
> accessible post TIFS is up (ie post k3_sysfw_loader() call)

I'll have to double check but I think this should work.

~Bryan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  2024-03-04 20:27     ` Bryan Brattlof
@ 2024-03-05  4:36       ` Vignesh Raghavendra
  2024-03-05 17:34         ` Bryan Brattlof
  0 siblings, 1 reply; 13+ messages in thread
From: Vignesh Raghavendra @ 2024-03-05  4:36 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Wadim Egorov, u-boot, nm, upstream



On 05/03/24 01:57, Bryan Brattlof wrote:
> Hey Vignesh!
> 
> On March  4, 2024 thus sayeth Vignesh Raghavendra:
>> Hi Wadim,
>>
>> On 26/02/24 19:00, Wadim Egorov wrote:
>>> Texas Instruments has begun enabling security settings on the SoCs it
>>> produces to instruct ROM and TIFS to begin protecting the Security
>>> Management Subsystem (SMS) from other binaries we load into the chip by
>>> default.
>>>
>>> One way ROM and TIFS do this is by enabling firewalls to protect the
>>> OCSRAM and HSM RAM regions they're using during bootup.
>>>
>>> The HSM RAM the wakeup SPL is in is firewalled by TIFS to protect
>>> itself from the main domain applications. This means the 'bootindex'
>>> value in HSM RAM, left by ROM to indicate if we're using the primary
>>> or secondary boot-method, must be moved to OCSRAM (that TIFS has open
>>> for us) before we make the jump to the main domain so the main domain's
>>> bootloaders can keep access to this information.
>>>
>>> Based on commit
>>>   b672e8581070 ("arm: mach-k3: copy bootindex to OCRAM for main domain SPL")
>>>
>>
>> FYI, this is mostly a problem in non SPL flow (TI prosperity SBL for
>> example) where HSM RAM would be used by HSM firmware. This should be a
>> issue in R5 SPL flow.  Do you see any issues today? If so, whats the
>> TIFS firmware being used?
>>
>>> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
>>> ---
>>>  arch/arm/mach-k3/Kconfig                      |  3 ++-
>>>  arch/arm/mach-k3/am625_init.c                 | 15 +++++++++++++--
>>>  arch/arm/mach-k3/include/mach/am62_hardware.h | 15 +++++++++++++++
>>>  3 files changed, 30 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
>>> index 03898424c9..f5d06593f7 100644
>>> --- a/arch/arm/mach-k3/Kconfig
>>> +++ b/arch/arm/mach-k3/Kconfig
>>> @@ -75,7 +75,8 @@ config SYS_K3_BOOT_PARAM_TABLE_INDEX
>>>  	default 0x41cffbfc if SOC_K3_J721E
>>>  	default 0x41cfdbfc if SOC_K3_J721S2
>>>  	default 0x701bebfc if SOC_K3_AM642
>>> -	default 0x43c3f290 if SOC_K3_AM625
>>> +	default 0x43c3f290 if SOC_K3_AM625 && CPU_V7R
>>> +	default 0x7000f290 if SOC_K3_AM625 && ARM64
>>>  	default 0x43c3f290 if SOC_K3_AM62A7 && CPU_V7R
>>>  	default 0x7000f290 if SOC_K3_AM62A7 && ARM64
>>>  	help
>>> diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
>>> index 6c96e88114..67cf63b103 100644
>>> --- a/arch/arm/mach-k3/am625_init.c
>>> +++ b/arch/arm/mach-k3/am625_init.c
>>> @@ -35,8 +35,10 @@ static struct rom_extended_boot_data bootdata __section(".data");
>>>  static void store_boot_info_from_rom(void)
>>>  {
>>>  	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
>>> -	memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
>>> -	       sizeof(struct rom_extended_boot_data));
>>> +	if (IS_ENABLED(CONFIG_CPU_V7R)) {
>>> +		memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
>>> +		       sizeof(struct rom_extended_boot_data));
>>> +	}
>>>  }
>>>  
>>>  static void ctrl_mmr_unlock(void)
>>> @@ -175,6 +177,15 @@ void board_init_f(ulong dummy)
>>>  		k3_sysfw_loader(true, NULL, NULL);
>>>  	}
>>>  
>>> +#if defined(CONFIG_CPU_V7R)
>>> +	/*
>>> +	 * Relocate boot information to OCRAM (after TIFS has opend this
>>> +	 * region for us) so the next bootloader stages can keep access to
>>> +	 * primary vs backup bootmodes.
>>> +	 */
>>> +	writel(bootindex, K3_BOOT_PARAM_TABLE_INDEX_OCRAM);
>>> +#endif
>>> +
>>>  	/*
>>>  	 * Force probe of clk_k3 driver here to ensure basic default clock
>>>  	 * configuration is always done.
>>> diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h
>>> index 54380f36e1..9f504f4642 100644
>>> --- a/arch/arm/mach-k3/include/mach/am62_hardware.h
>>> +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h
>>> @@ -76,8 +76,23 @@
>>>  #define CTRLMMR_MCU_RST_CTRL			(MCU_CTRL_MMR0_BASE + 0x18170)
>>>  
>>>  #define ROM_EXTENDED_BOOT_DATA_INFO		0x43c3f1e0
>>> +#define K3_BOOT_PARAM_TABLE_INDEX_OCRAM         0x7000F290
>>>  
>>> +/*
>>> + * During the boot process ROM will kill anything that writes to OCSRAM.
>>
>> R5 ROM is long gone when R5 SPL starts, how would it kill anything?
> 
> Looks like this was based on my patch long ago for the AM62Ax family. 
> From what little I remember about this was ROM is leaving behind a 
> firewall that we need TIFS's help to bring down for us. So I just
> blamed ROM ;)

Thats true. ROM does bare minimum and so wont open up firewall around
main SRAM. but TIFS does, so you should be able to access this region
post k3_sysfw_loader().

> 
> IDK if this is an issue for the AM62x family though.
> 

It might be if one tries to "select" DT using EEPROM detect before SYSFW
is up. But that's not the case any more right?

>>
>>> + * This means the wakeup SPL cannot use this region during boot. To
>>> + * complicate things, TIFS will set a firewall between HSM RAM and the
>>> + * main domain.
>>> + *
>>> + * So, during the wakeup SPL, we will need to store the EEPROM data
>>> + * somewhere in HSM RAM, and the main domain's SPL will need to store it
>>> + * somewhere in OCSRAM
>>> + */
>>> +#ifdef CONFIG_CPU_V7R
>>>  #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x43c30000
>>> +#else
>>> + #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x70000001
>>
>> Why not  0x70000000 ?
> 
> Ah an off by one by me :) 0x70000000 should be fine
> 
>>
>>> +#endif /* CONFIG_CPU_V7R */
>>>  
>>
>> Can't we store directly in OCRAM in both stages? This RAM should be
>> accessible post TIFS is up (ie post k3_sysfw_loader() call)
> 
> I'll have to double check but I think this should work.
> 
> ~Bryan

-- 
Regards
Vignesh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  2024-03-05  4:36       ` Vignesh Raghavendra
@ 2024-03-05 17:34         ` Bryan Brattlof
  2024-03-05 17:36           ` Raghavendra, Vignesh
  0 siblings, 1 reply; 13+ messages in thread
From: Bryan Brattlof @ 2024-03-05 17:34 UTC (permalink / raw)
  To: Vignesh Raghavendra; +Cc: Wadim Egorov, u-boot, nm, upstream

On March  5, 2024 thus sayeth Vignesh Raghavendra:
> 
> 
> On 05/03/24 01:57, Bryan Brattlof wrote:
> > Hey Vignesh!
> > 
> > On March  4, 2024 thus sayeth Vignesh Raghavendra:
> >> Hi Wadim,
> >>
> >> On 26/02/24 19:00, Wadim Egorov wrote:
> >>> Texas Instruments has begun enabling security settings on the SoCs it
> >>> produces to instruct ROM and TIFS to begin protecting the Security
> >>> Management Subsystem (SMS) from other binaries we load into the chip by
> >>> default.
> >>>
> >>> One way ROM and TIFS do this is by enabling firewalls to protect the
> >>> OCSRAM and HSM RAM regions they're using during bootup.
> >>>
> >>> The HSM RAM the wakeup SPL is in is firewalled by TIFS to protect
> >>> itself from the main domain applications. This means the 'bootindex'
> >>> value in HSM RAM, left by ROM to indicate if we're using the primary
> >>> or secondary boot-method, must be moved to OCSRAM (that TIFS has open
> >>> for us) before we make the jump to the main domain so the main domain's
> >>> bootloaders can keep access to this information.
> >>>
> >>> Based on commit
> >>>   b672e8581070 ("arm: mach-k3: copy bootindex to OCRAM for main domain SPL")
> >>>
> >>
> >> FYI, this is mostly a problem in non SPL flow (TI prosperity SBL for
> >> example) where HSM RAM would be used by HSM firmware. This should be a
> >> issue in R5 SPL flow.  Do you see any issues today? If so, whats the
> >> TIFS firmware being used?
> >>
> >>> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
> >>> ---
> >>>  arch/arm/mach-k3/Kconfig                      |  3 ++-
> >>>  arch/arm/mach-k3/am625_init.c                 | 15 +++++++++++++--
> >>>  arch/arm/mach-k3/include/mach/am62_hardware.h | 15 +++++++++++++++
> >>>  3 files changed, 30 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> >>> index 03898424c9..f5d06593f7 100644
> >>> --- a/arch/arm/mach-k3/Kconfig
> >>> +++ b/arch/arm/mach-k3/Kconfig
> >>> @@ -75,7 +75,8 @@ config SYS_K3_BOOT_PARAM_TABLE_INDEX
> >>>  	default 0x41cffbfc if SOC_K3_J721E
> >>>  	default 0x41cfdbfc if SOC_K3_J721S2
> >>>  	default 0x701bebfc if SOC_K3_AM642
> >>> -	default 0x43c3f290 if SOC_K3_AM625
> >>> +	default 0x43c3f290 if SOC_K3_AM625 && CPU_V7R
> >>> +	default 0x7000f290 if SOC_K3_AM625 && ARM64
> >>>  	default 0x43c3f290 if SOC_K3_AM62A7 && CPU_V7R
> >>>  	default 0x7000f290 if SOC_K3_AM62A7 && ARM64
> >>>  	help
> >>> diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
> >>> index 6c96e88114..67cf63b103 100644
> >>> --- a/arch/arm/mach-k3/am625_init.c
> >>> +++ b/arch/arm/mach-k3/am625_init.c
> >>> @@ -35,8 +35,10 @@ static struct rom_extended_boot_data bootdata __section(".data");
> >>>  static void store_boot_info_from_rom(void)
> >>>  {
> >>>  	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
> >>> -	memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
> >>> -	       sizeof(struct rom_extended_boot_data));
> >>> +	if (IS_ENABLED(CONFIG_CPU_V7R)) {
> >>> +		memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
> >>> +		       sizeof(struct rom_extended_boot_data));
> >>> +	}
> >>>  }
> >>>  
> >>>  static void ctrl_mmr_unlock(void)
> >>> @@ -175,6 +177,15 @@ void board_init_f(ulong dummy)
> >>>  		k3_sysfw_loader(true, NULL, NULL);
> >>>  	}
> >>>  
> >>> +#if defined(CONFIG_CPU_V7R)
> >>> +	/*
> >>> +	 * Relocate boot information to OCRAM (after TIFS has opend this
> >>> +	 * region for us) so the next bootloader stages can keep access to
> >>> +	 * primary vs backup bootmodes.
> >>> +	 */
> >>> +	writel(bootindex, K3_BOOT_PARAM_TABLE_INDEX_OCRAM);
> >>> +#endif
> >>> +
> >>>  	/*
> >>>  	 * Force probe of clk_k3 driver here to ensure basic default clock
> >>>  	 * configuration is always done.
> >>> diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h
> >>> index 54380f36e1..9f504f4642 100644
> >>> --- a/arch/arm/mach-k3/include/mach/am62_hardware.h
> >>> +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h
> >>> @@ -76,8 +76,23 @@
> >>>  #define CTRLMMR_MCU_RST_CTRL			(MCU_CTRL_MMR0_BASE + 0x18170)
> >>>  
> >>>  #define ROM_EXTENDED_BOOT_DATA_INFO		0x43c3f1e0
> >>> +#define K3_BOOT_PARAM_TABLE_INDEX_OCRAM         0x7000F290
> >>>  
> >>> +/*
> >>> + * During the boot process ROM will kill anything that writes to OCSRAM.
> >>
> >> R5 ROM is long gone when R5 SPL starts, how would it kill anything?
> > 
> > Looks like this was based on my patch long ago for the AM62Ax family. 
> > From what little I remember about this was ROM is leaving behind a 
> > firewall that we need TIFS's help to bring down for us. So I just
> > blamed ROM ;)
> 
> Thats true. ROM does bare minimum and so wont open up firewall around
> main SRAM. but TIFS does, so you should be able to access this region
> post k3_sysfw_loader().
> 
> > 
> > IDK if this is an issue for the AM62x family though.
> > 
> 
> It might be if one tries to "select" DT using EEPROM detect before SYSFW
> is up. But that's not the case any more right?

Yep we still need to figure out a plan for multiple DDR configs or see 
if we can move the DDR init to later in the boot as that is the only 
thing left that still needs the board detection this early on.

There is a little race condition here as TIFS can respond to some 
messages before it's finished its init. IDK if we can send it anything 
to act like a fence and stall us until the firewalls are down though.

> 
> >>
> >>> + * This means the wakeup SPL cannot use this region during boot. To
> >>> + * complicate things, TIFS will set a firewall between HSM RAM and the
> >>> + * main domain.
> >>> + *
> >>> + * So, during the wakeup SPL, we will need to store the EEPROM data
> >>> + * somewhere in HSM RAM, and the main domain's SPL will need to store it
> >>> + * somewhere in OCSRAM
> >>> + */
> >>> +#ifdef CONFIG_CPU_V7R
> >>>  #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x43c30000
> >>> +#else
> >>> + #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x70000001
> >>
> >> Why not  0x70000000 ?
> > 
> > Ah an off by one by me :) 0x70000000 should be fine
> > 
> >>
> >>> +#endif /* CONFIG_CPU_V7R */
> >>>  
> >>
> >> Can't we store directly in OCRAM in both stages? This RAM should be
> >> accessible post TIFS is up (ie post k3_sysfw_loader() call)
> > 
> > I'll have to double check but I think this should work.

This should work, using the HSM's RAM was just a short sided work around 
before the HS chips started being produced.

~Bryan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  2024-03-05 17:34         ` Bryan Brattlof
@ 2024-03-05 17:36           ` Raghavendra, Vignesh
  2024-04-01  9:24             ` Wadim Egorov
  0 siblings, 1 reply; 13+ messages in thread
From: Raghavendra, Vignesh @ 2024-03-05 17:36 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Wadim Egorov, u-boot, nm, upstream



On 3/5/2024 11:04 PM, Bryan Brattlof wrote:
> On March  5, 2024 thus sayeth Vignesh Raghavendra:
>>
>> On 05/03/24 01:57, Bryan Brattlof wrote:
>>> Hey Vignesh!
>>>
>>> On March  4, 2024 thus sayeth Vignesh Raghavendra:
>>>> Hi Wadim,
>>>>
>>>> On 26/02/24 19:00, Wadim Egorov wrote:
>>>>> Texas Instruments has begun enabling security settings on the SoCs it
>>>>> produces to instruct ROM and TIFS to begin protecting the Security
>>>>> Management Subsystem (SMS) from other binaries we load into the chip by
>>>>> default.
>>>>>
>>>>> One way ROM and TIFS do this is by enabling firewalls to protect the
>>>>> OCSRAM and HSM RAM regions they're using during bootup.
>>>>>
>>>>> The HSM RAM the wakeup SPL is in is firewalled by TIFS to protect
>>>>> itself from the main domain applications. This means the 'bootindex'
>>>>> value in HSM RAM, left by ROM to indicate if we're using the primary
>>>>> or secondary boot-method, must be moved to OCSRAM (that TIFS has open
>>>>> for us) before we make the jump to the main domain so the main domain's
>>>>> bootloaders can keep access to this information.
>>>>>
>>>>> Based on commit
>>>>>   b672e8581070 ("arm: mach-k3: copy bootindex to OCRAM for main domain SPL")
>>>>>
>>>> FYI, this is mostly a problem in non SPL flow (TI prosperity SBL for
>>>> example) where HSM RAM would be used by HSM firmware. This should be a
>>>> issue in R5 SPL flow.  Do you see any issues today? If so, whats the
>>>> TIFS firmware being used?
>>>>
>>>>> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
>>>>> ---
>>>>>  arch/arm/mach-k3/Kconfig                      |  3 ++-
>>>>>  arch/arm/mach-k3/am625_init.c                 | 15 +++++++++++++--
>>>>>  arch/arm/mach-k3/include/mach/am62_hardware.h | 15 +++++++++++++++
>>>>>  3 files changed, 30 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
>>>>> index 03898424c9..f5d06593f7 100644
>>>>> --- a/arch/arm/mach-k3/Kconfig
>>>>> +++ b/arch/arm/mach-k3/Kconfig
>>>>> @@ -75,7 +75,8 @@ config SYS_K3_BOOT_PARAM_TABLE_INDEX
>>>>>  	default 0x41cffbfc if SOC_K3_J721E
>>>>>  	default 0x41cfdbfc if SOC_K3_J721S2
>>>>>  	default 0x701bebfc if SOC_K3_AM642
>>>>> -	default 0x43c3f290 if SOC_K3_AM625
>>>>> +	default 0x43c3f290 if SOC_K3_AM625 && CPU_V7R
>>>>> +	default 0x7000f290 if SOC_K3_AM625 && ARM64
>>>>>  	default 0x43c3f290 if SOC_K3_AM62A7 && CPU_V7R
>>>>>  	default 0x7000f290 if SOC_K3_AM62A7 && ARM64
>>>>>  	help
>>>>> diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
>>>>> index 6c96e88114..67cf63b103 100644
>>>>> --- a/arch/arm/mach-k3/am625_init.c
>>>>> +++ b/arch/arm/mach-k3/am625_init.c
>>>>> @@ -35,8 +35,10 @@ static struct rom_extended_boot_data bootdata __section(".data");
>>>>>  static void store_boot_info_from_rom(void)
>>>>>  {
>>>>>  	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
>>>>> -	memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
>>>>> -	       sizeof(struct rom_extended_boot_data));
>>>>> +	if (IS_ENABLED(CONFIG_CPU_V7R)) {
>>>>> +		memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
>>>>> +		       sizeof(struct rom_extended_boot_data));
>>>>> +	}
>>>>>  }
>>>>>  
>>>>>  static void ctrl_mmr_unlock(void)
>>>>> @@ -175,6 +177,15 @@ void board_init_f(ulong dummy)
>>>>>  		k3_sysfw_loader(true, NULL, NULL);
>>>>>  	}
>>>>>  
>>>>> +#if defined(CONFIG_CPU_V7R)
>>>>> +	/*
>>>>> +	 * Relocate boot information to OCRAM (after TIFS has opend this
>>>>> +	 * region for us) so the next bootloader stages can keep access to
>>>>> +	 * primary vs backup bootmodes.
>>>>> +	 */
>>>>> +	writel(bootindex, K3_BOOT_PARAM_TABLE_INDEX_OCRAM);
>>>>> +#endif
>>>>> +
>>>>>  	/*
>>>>>  	 * Force probe of clk_k3 driver here to ensure basic default clock
>>>>>  	 * configuration is always done.
>>>>> diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h
>>>>> index 54380f36e1..9f504f4642 100644
>>>>> --- a/arch/arm/mach-k3/include/mach/am62_hardware.h
>>>>> +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h
>>>>> @@ -76,8 +76,23 @@
>>>>>  #define CTRLMMR_MCU_RST_CTRL			(MCU_CTRL_MMR0_BASE + 0x18170)
>>>>>  
>>>>>  #define ROM_EXTENDED_BOOT_DATA_INFO		0x43c3f1e0
>>>>> +#define K3_BOOT_PARAM_TABLE_INDEX_OCRAM         0x7000F290
>>>>>  
>>>>> +/*
>>>>> + * During the boot process ROM will kill anything that writes to OCSRAM.
>>>> R5 ROM is long gone when R5 SPL starts, how would it kill anything?
>>> Looks like this was based on my patch long ago for the AM62Ax family. 
>>> From what little I remember about this was ROM is leaving behind a 
>>> firewall that we need TIFS's help to bring down for us. So I just
>>> blamed ROM 😉
>> Thats true. ROM does bare minimum and so wont open up firewall around
>> main SRAM. but TIFS does, so you should be able to access this region
>> post k3_sysfw_loader().
>>
>>> IDK if this is an issue for the AM62x family though.
>>>
>> It might be if one tries to "select" DT using EEPROM detect before SYSFW
>> is up. But that's not the case any more right?
> Yep we still need to figure out a plan for multiple DDR configs or see 
> if we can move the DDR init to later in the boot as that is the only 
> thing left that still needs the board detection this early on.
> 
> There is a little race condition here as TIFS can respond to some 
> messages before it's finished its init. IDK if we can send it anything 
> to act like a fence and stall us until the firewalls are down though.

Firewall configurations should be done before TIFS posts boot
notification message.

Regards
Vignesh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  2024-03-04  5:06   ` Vignesh Raghavendra
  2024-03-04 20:27     ` Bryan Brattlof
@ 2024-03-06 13:44     ` Wadim Egorov
  2024-03-07  4:04       ` Raghavendra, Vignesh
  1 sibling, 1 reply; 13+ messages in thread
From: Wadim Egorov @ 2024-03-06 13:44 UTC (permalink / raw)
  To: Vignesh Raghavendra, u-boot; +Cc: nm, upstream, bb

Hi Vignesh,

Am 04.03.24 um 06:06 schrieb Vignesh Raghavendra:
> Hi Wadim,
>
> On 26/02/24 19:00, Wadim Egorov wrote:
>> Texas Instruments has begun enabling security settings on the SoCs it
>> produces to instruct ROM and TIFS to begin protecting the Security
>> Management Subsystem (SMS) from other binaries we load into the chip by
>> default.
>>
>> One way ROM and TIFS do this is by enabling firewalls to protect the
>> OCSRAM and HSM RAM regions they're using during bootup.
>>
>> The HSM RAM the wakeup SPL is in is firewalled by TIFS to protect
>> itself from the main domain applications. This means the 'bootindex'
>> value in HSM RAM, left by ROM to indicate if we're using the primary
>> or secondary boot-method, must be moved to OCSRAM (that TIFS has open
>> for us) before we make the jump to the main domain so the main domain's
>> bootloaders can keep access to this information.
>>
>> Based on commit
>>    b672e8581070 ("arm: mach-k3: copy bootindex to OCRAM for main domain SPL")
>>
> FYI, this is mostly a problem in non SPL flow (TI prosperity SBL for
> example) where HSM RAM would be used by HSM firmware. This should be a
> issue in R5 SPL flow.  Do you see any issues today? If so, whats the
> TIFS firmware being used?

I remember I was losing the bootindex using ti/downstream u-boot.
But can't figure out the exact version anymore.
Just did a bit of testing and I can not see the Issue with the current 
u-boot.
Boot index in 0x43c3f290 stays intact.

Would it be okay to drop this patch and keep only the 2nd patch that 
factors out into get_boot_device()?

Regards,
Wadim

>
>> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
>> ---
>>   arch/arm/mach-k3/Kconfig                      |  3 ++-
>>   arch/arm/mach-k3/am625_init.c                 | 15 +++++++++++++--
>>   arch/arm/mach-k3/include/mach/am62_hardware.h | 15 +++++++++++++++
>>   3 files changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
>> index 03898424c9..f5d06593f7 100644
>> --- a/arch/arm/mach-k3/Kconfig
>> +++ b/arch/arm/mach-k3/Kconfig
>> @@ -75,7 +75,8 @@ config SYS_K3_BOOT_PARAM_TABLE_INDEX
>>   	default 0x41cffbfc if SOC_K3_J721E
>>   	default 0x41cfdbfc if SOC_K3_J721S2
>>   	default 0x701bebfc if SOC_K3_AM642
>> -	default 0x43c3f290 if SOC_K3_AM625
>> +	default 0x43c3f290 if SOC_K3_AM625 && CPU_V7R
>> +	default 0x7000f290 if SOC_K3_AM625 && ARM64
>>   	default 0x43c3f290 if SOC_K3_AM62A7 && CPU_V7R
>>   	default 0x7000f290 if SOC_K3_AM62A7 && ARM64
>>   	help
>> diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
>> index 6c96e88114..67cf63b103 100644
>> --- a/arch/arm/mach-k3/am625_init.c
>> +++ b/arch/arm/mach-k3/am625_init.c
>> @@ -35,8 +35,10 @@ static struct rom_extended_boot_data bootdata __section(".data");
>>   static void store_boot_info_from_rom(void)
>>   {
>>   	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
>> -	memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
>> -	       sizeof(struct rom_extended_boot_data));
>> +	if (IS_ENABLED(CONFIG_CPU_V7R)) {
>> +		memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
>> +		       sizeof(struct rom_extended_boot_data));
>> +	}
>>   }
>>   
>>   static void ctrl_mmr_unlock(void)
>> @@ -175,6 +177,15 @@ void board_init_f(ulong dummy)
>>   		k3_sysfw_loader(true, NULL, NULL);
>>   	}
>>   
>> +#if defined(CONFIG_CPU_V7R)
>> +	/*
>> +	 * Relocate boot information to OCRAM (after TIFS has opend this
>> +	 * region for us) so the next bootloader stages can keep access to
>> +	 * primary vs backup bootmodes.
>> +	 */
>> +	writel(bootindex, K3_BOOT_PARAM_TABLE_INDEX_OCRAM);
>> +#endif
>> +
>>   	/*
>>   	 * Force probe of clk_k3 driver here to ensure basic default clock
>>   	 * configuration is always done.
>> diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h
>> index 54380f36e1..9f504f4642 100644
>> --- a/arch/arm/mach-k3/include/mach/am62_hardware.h
>> +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h
>> @@ -76,8 +76,23 @@
>>   #define CTRLMMR_MCU_RST_CTRL			(MCU_CTRL_MMR0_BASE + 0x18170)
>>   
>>   #define ROM_EXTENDED_BOOT_DATA_INFO		0x43c3f1e0
>> +#define K3_BOOT_PARAM_TABLE_INDEX_OCRAM         0x7000F290
>>   
>> +/*
>> + * During the boot process ROM will kill anything that writes to OCSRAM.
> R5 ROM is long gone when R5 SPL starts, how would it kill anything?
>
>> + * This means the wakeup SPL cannot use this region during boot. To
>> + * complicate things, TIFS will set a firewall between HSM RAM and the
>> + * main domain.
>> + *
>> + * So, during the wakeup SPL, we will need to store the EEPROM data
>> + * somewhere in HSM RAM, and the main domain's SPL will need to store it
>> + * somewhere in OCSRAM
>> + */
>> +#ifdef CONFIG_CPU_V7R
>>   #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x43c30000
>> +#else
>> + #define TI_SRAM_SCRATCH_BOARD_EEPROM_START	0x70000001
> Why not  0x70000000 ?
>
>> +#endif /* CONFIG_CPU_V7R */
>>   
> Can't we store directly in OCRAM in both stages? This RAM should be
> accessible post TIFS is up (ie post k3_sysfw_loader() call)
>
>>   static inline int k3_get_core_nr(void)
>>   {

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  2024-03-06 13:44     ` Wadim Egorov
@ 2024-03-07  4:04       ` Raghavendra, Vignesh
  0 siblings, 0 replies; 13+ messages in thread
From: Raghavendra, Vignesh @ 2024-03-07  4:04 UTC (permalink / raw)
  To: Wadim Egorov, u-boot; +Cc: nm, upstream, bb

Hi,

On 3/6/2024 7:14 PM, Wadim Egorov wrote:
> Hi Vignesh,
> 
> Am 04.03.24 um 06:06 schrieb Vignesh Raghavendra:
>> Hi Wadim,
>>
>> On 26/02/24 19:00, Wadim Egorov wrote:
>>> Texas Instruments has begun enabling security settings on the SoCs it
>>> produces to instruct ROM and TIFS to begin protecting the Security
>>> Management Subsystem (SMS) from other binaries we load into the chip by
>>> default.
>>>
>>> One way ROM and TIFS do this is by enabling firewalls to protect the
>>> OCSRAM and HSM RAM regions they're using during bootup.
>>>
>>> The HSM RAM the wakeup SPL is in is firewalled by TIFS to protect
>>> itself from the main domain applications. This means the 'bootindex'
>>> value in HSM RAM, left by ROM to indicate if we're using the primary
>>> or secondary boot-method, must be moved to OCSRAM (that TIFS has open
>>> for us) before we make the jump to the main domain so the main domain's
>>> bootloaders can keep access to this information.
>>>
>>> Based on commit
>>>    b672e8581070 ("arm: mach-k3: copy bootindex to OCRAM for main
>>> domain SPL")
>>>
>> FYI, this is mostly a problem in non SPL flow (TI prosperity SBL for
>> example) where HSM RAM would be used by HSM firmware. This should be a
>> issue in R5 SPL flow.  Do you see any issues today? If so, whats the
>> TIFS firmware being used?
> 
> I remember I was losing the bootindex using ti/downstream u-boot.
> But can't figure out the exact version anymore.
> Just did a bit of testing and I can not see the Issue with the current
> u-boot.
> Boot index in 0x43c3f290 stays intact.
> 
> Would it be okay to drop this patch and keep only the 2nd patch that
> factors out into get_boot_device()?
> 

yeah... 2/2 is still relevant irrespective of this patch.

[...]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  2024-03-05 17:36           ` Raghavendra, Vignesh
@ 2024-04-01  9:24             ` Wadim Egorov
  2024-04-01 14:46               ` Bryan Brattlof
  0 siblings, 1 reply; 13+ messages in thread
From: Wadim Egorov @ 2024-04-01  9:24 UTC (permalink / raw)
  To: Raghavendra, Vignesh, Bryan Brattlof; +Cc: u-boot, nm, upstream

Hi Vignesh, Hi Bryan,


Am 05.03.24 um 18:36 schrieb Raghavendra, Vignesh:
> 
> 
> On 3/5/2024 11:04 PM, Bryan Brattlof wrote:
>> On March  5, 2024 thus sayeth Vignesh Raghavendra:
>>>
>>> On 05/03/24 01:57, Bryan Brattlof wrote:
>>>> Hey Vignesh!
>>>>
>>>> On March  4, 2024 thus sayeth Vignesh Raghavendra:
>>>>> Hi Wadim,
>>>>>
>>>>> On 26/02/24 19:00, Wadim Egorov wrote:
>>>>>> Texas Instruments has begun enabling security settings on the SoCs it
>>>>>> produces to instruct ROM and TIFS to begin protecting the Security
>>>>>> Management Subsystem (SMS) from other binaries we load into the chip by
>>>>>> default.
>>>>>>
>>>>>> One way ROM and TIFS do this is by enabling firewalls to protect the
>>>>>> OCSRAM and HSM RAM regions they're using during bootup.
>>>>>>
>>>>>> The HSM RAM the wakeup SPL is in is firewalled by TIFS to protect
>>>>>> itself from the main domain applications. This means the 'bootindex'
>>>>>> value in HSM RAM, left by ROM to indicate if we're using the primary
>>>>>> or secondary boot-method, must be moved to OCSRAM (that TIFS has open
>>>>>> for us) before we make the jump to the main domain so the main domain's
>>>>>> bootloaders can keep access to this information.
>>>>>>
>>>>>> Based on commit
>>>>>>    b672e8581070 ("arm: mach-k3: copy bootindex to OCRAM for main domain SPL")

I was thinking, even if the reason described here is not right or does 
not apply to the am62x, it is still a valid solution for carrying this 
variable into the context for next stage A53 bootloader.

store_boot_info_from_rom() stores the index to the bootindex (.data) 
variable which makes sure it is valid in R5 SPL context. But the next 
stage bootloader does not know anything about the bootindex variable. So 
from my understanding it needs to be copied to a different region to 
preserve the data for next stage bootloaders.

Or do I miss something?

Regards,
Wadim

>>>>>>
>>>>> FYI, this is mostly a problem in non SPL flow (TI prosperity SBL for
>>>>> example) where HSM RAM would be used by HSM firmware. This should be a
>>>>> issue in R5 SPL flow.  Do you see any issues today? If so, whats the
>>>>> TIFS firmware being used?
>>>>>
>>>>>> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
>>>>>> ---
>>>>>>   arch/arm/mach-k3/Kconfig                      |  3 ++-
>>>>>>   arch/arm/mach-k3/am625_init.c                 | 15 +++++++++++++--
>>>>>>   arch/arm/mach-k3/include/mach/am62_hardware.h | 15 +++++++++++++++
>>>>>>   3 files changed, 30 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
>>>>>> index 03898424c9..f5d06593f7 100644
>>>>>> --- a/arch/arm/mach-k3/Kconfig
>>>>>> +++ b/arch/arm/mach-k3/Kconfig
>>>>>> @@ -75,7 +75,8 @@ config SYS_K3_BOOT_PARAM_TABLE_INDEX
>>>>>>   	default 0x41cffbfc if SOC_K3_J721E
>>>>>>   	default 0x41cfdbfc if SOC_K3_J721S2
>>>>>>   	default 0x701bebfc if SOC_K3_AM642
>>>>>> -	default 0x43c3f290 if SOC_K3_AM625
>>>>>> +	default 0x43c3f290 if SOC_K3_AM625 && CPU_V7R
>>>>>> +	default 0x7000f290 if SOC_K3_AM625 && ARM64
>>>>>>   	default 0x43c3f290 if SOC_K3_AM62A7 && CPU_V7R
>>>>>>   	default 0x7000f290 if SOC_K3_AM62A7 && ARM64
>>>>>>   	help
>>>>>> diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
>>>>>> index 6c96e88114..67cf63b103 100644
>>>>>> --- a/arch/arm/mach-k3/am625_init.c
>>>>>> +++ b/arch/arm/mach-k3/am625_init.c
>>>>>> @@ -35,8 +35,10 @@ static struct rom_extended_boot_data bootdata __section(".data");
>>>>>>   static void store_boot_info_from_rom(void)
>>>>>>   {
>>>>>>   	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
>>>>>> -	memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
>>>>>> -	       sizeof(struct rom_extended_boot_data));
>>>>>> +	if (IS_ENABLED(CONFIG_CPU_V7R)) {
>>>>>> +		memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
>>>>>> +		       sizeof(struct rom_extended_boot_data));
>>>>>> +	}
>>>>>>   }
>>>>>>   
>>>>>>   static void ctrl_mmr_unlock(void)
>>>>>> @@ -175,6 +177,15 @@ void board_init_f(ulong dummy)
>>>>>>   		k3_sysfw_loader(true, NULL, NULL);
>>>>>>   	}
>>>>>>   
>>>>>> +#if defined(CONFIG_CPU_V7R)
>>>>>> +	/*
>>>>>> +	 * Relocate boot information to OCRAM (after TIFS has opend this
>>>>>> +	 * region for us) so the next bootloader stages can keep access to
>>>>>> +	 * primary vs backup bootmodes.
>>>>>> +	 */
>>>>>> +	writel(bootindex, K3_BOOT_PARAM_TABLE_INDEX_OCRAM);
>>>>>> +#endif
>>>>>> +
>>>>>>   	/*
>>>>>>   	 * Force probe of clk_k3 driver here to ensure basic default clock
>>>>>>   	 * configuration is always done.
>>>>>> diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h
>>>>>> index 54380f36e1..9f504f4642 100644
>>>>>> --- a/arch/arm/mach-k3/include/mach/am62_hardware.h
>>>>>> +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h
>>>>>> @@ -76,8 +76,23 @@
>>>>>>   #define CTRLMMR_MCU_RST_CTRL			(MCU_CTRL_MMR0_BASE + 0x18170)
>>>>>>   
>>>>>>   #define ROM_EXTENDED_BOOT_DATA_INFO		0x43c3f1e0
>>>>>> +#define K3_BOOT_PARAM_TABLE_INDEX_OCRAM         0x7000F290
>>>>>>   
>>>>>> +/*
>>>>>> + * During the boot process ROM will kill anything that writes to OCSRAM.
>>>>> R5 ROM is long gone when R5 SPL starts, how would it kill anything?
>>>> Looks like this was based on my patch long ago for the AM62Ax family.
>>>>  From what little I remember about this was ROM is leaving behind a
>>>> firewall that we need TIFS's help to bring down for us. So I just
>>>> blamed ROM 😉
>>> Thats true. ROM does bare minimum and so wont open up firewall around
>>> main SRAM. but TIFS does, so you should be able to access this region
>>> post k3_sysfw_loader().
>>>
>>>> IDK if this is an issue for the AM62x family though.
>>>>
>>> It might be if one tries to "select" DT using EEPROM detect before SYSFW
>>> is up. But that's not the case any more right?
>> Yep we still need to figure out a plan for multiple DDR configs or see
>> if we can move the DDR init to later in the boot as that is the only
>> thing left that still needs the board detection this early on.
>>
>> There is a little race condition here as TIFS can respond to some
>> messages before it's finished its init. IDK if we can send it anything
>> to act like a fence and stall us until the firewalls are down though.
> 
> Firewall configurations should be done before TIFS posts boot
> notification message.
> 
> Regards
> Vignesh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  2024-04-01  9:24             ` Wadim Egorov
@ 2024-04-01 14:46               ` Bryan Brattlof
  2024-04-01 16:53                 ` Wadim Egorov
  0 siblings, 1 reply; 13+ messages in thread
From: Bryan Brattlof @ 2024-04-01 14:46 UTC (permalink / raw)
  To: Wadim Egorov; +Cc: Raghavendra, Vignesh, u-boot, nm, upstream

On April  1, 2024 thus sayeth Wadim Egorov:
> Hi Vignesh, Hi Bryan,
> 
> 
> Am 05.03.24 um 18:36 schrieb Raghavendra, Vignesh:
> > 
> > 
> > On 3/5/2024 11:04 PM, Bryan Brattlof wrote:
> > > On March  5, 2024 thus sayeth Vignesh Raghavendra:
> > > > 
> > > > On 05/03/24 01:57, Bryan Brattlof wrote:
> > > > > Hey Vignesh!
> > > > > 
> > > > > On March  4, 2024 thus sayeth Vignesh Raghavendra:
> > > > > > Hi Wadim,
> > > > > > 
> > > > > > On 26/02/24 19:00, Wadim Egorov wrote:
> > > > > > > Texas Instruments has begun enabling security settings on the SoCs it
> > > > > > > produces to instruct ROM and TIFS to begin protecting the Security
> > > > > > > Management Subsystem (SMS) from other binaries we load into the chip by
> > > > > > > default.
> > > > > > > 
> > > > > > > One way ROM and TIFS do this is by enabling firewalls to protect the
> > > > > > > OCSRAM and HSM RAM regions they're using during bootup.
> > > > > > > 
> > > > > > > The HSM RAM the wakeup SPL is in is firewalled by TIFS to protect
> > > > > > > itself from the main domain applications. This means the 'bootindex'
> > > > > > > value in HSM RAM, left by ROM to indicate if we're using the primary
> > > > > > > or secondary boot-method, must be moved to OCSRAM (that TIFS has open
> > > > > > > for us) before we make the jump to the main domain so the main domain's
> > > > > > > bootloaders can keep access to this information.
> > > > > > > 
> > > > > > > Based on commit
> > > > > > >    b672e8581070 ("arm: mach-k3: copy bootindex to OCRAM for main domain SPL")
> 
> I was thinking, even if the reason described here is not right or does not
> apply to the am62x, it is still a valid solution for carrying this variable
> into the context for next stage A53 bootloader.
> 
> store_boot_info_from_rom() stores the index to the bootindex (.data)
> variable which makes sure it is valid in R5 SPL context. But the next stage
> bootloader does not know anything about the bootindex variable. So from my
> understanding it needs to be copied to a different region to preserve the
> data for next stage bootloaders.
> 
> Or do I miss something?

That's correct. We typically put this bootindex variable in the same 
location for both SPLs.

~Bryan

> > > > > > > 
> > > > > > FYI, this is mostly a problem in non SPL flow (TI prosperity SBL for
> > > > > > example) where HSM RAM would be used by HSM firmware. This should be a
> > > > > > issue in R5 SPL flow.  Do you see any issues today? If so, whats the
> > > > > > TIFS firmware being used?
> > > > > > 
> > > > > > > Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
> > > > > > > ---
> > > > > > >   arch/arm/mach-k3/Kconfig                      |  3 ++-
> > > > > > >   arch/arm/mach-k3/am625_init.c                 | 15 +++++++++++++--
> > > > > > >   arch/arm/mach-k3/include/mach/am62_hardware.h | 15 +++++++++++++++
> > > > > > >   3 files changed, 30 insertions(+), 3 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> > > > > > > index 03898424c9..f5d06593f7 100644
> > > > > > > --- a/arch/arm/mach-k3/Kconfig
> > > > > > > +++ b/arch/arm/mach-k3/Kconfig
> > > > > > > @@ -75,7 +75,8 @@ config SYS_K3_BOOT_PARAM_TABLE_INDEX
> > > > > > >   	default 0x41cffbfc if SOC_K3_J721E
> > > > > > >   	default 0x41cfdbfc if SOC_K3_J721S2
> > > > > > >   	default 0x701bebfc if SOC_K3_AM642
> > > > > > > -	default 0x43c3f290 if SOC_K3_AM625
> > > > > > > +	default 0x43c3f290 if SOC_K3_AM625 && CPU_V7R
> > > > > > > +	default 0x7000f290 if SOC_K3_AM625 && ARM64
> > > > > > >   	default 0x43c3f290 if SOC_K3_AM62A7 && CPU_V7R
> > > > > > >   	default 0x7000f290 if SOC_K3_AM62A7 && ARM64
> > > > > > >   	help
> > > > > > > diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
> > > > > > > index 6c96e88114..67cf63b103 100644
> > > > > > > --- a/arch/arm/mach-k3/am625_init.c
> > > > > > > +++ b/arch/arm/mach-k3/am625_init.c
> > > > > > > @@ -35,8 +35,10 @@ static struct rom_extended_boot_data bootdata __section(".data");
> > > > > > >   static void store_boot_info_from_rom(void)
> > > > > > >   {
> > > > > > >   	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
> > > > > > > -	memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
> > > > > > > -	       sizeof(struct rom_extended_boot_data));
> > > > > > > +	if (IS_ENABLED(CONFIG_CPU_V7R)) {
> > > > > > > +		memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
> > > > > > > +		       sizeof(struct rom_extended_boot_data));
> > > > > > > +	}
> > > > > > >   }
> > > > > > >   static void ctrl_mmr_unlock(void)
> > > > > > > @@ -175,6 +177,15 @@ void board_init_f(ulong dummy)
> > > > > > >   		k3_sysfw_loader(true, NULL, NULL);
> > > > > > >   	}
> > > > > > > +#if defined(CONFIG_CPU_V7R)
> > > > > > > +	/*
> > > > > > > +	 * Relocate boot information to OCRAM (after TIFS has opend this
> > > > > > > +	 * region for us) so the next bootloader stages can keep access to
> > > > > > > +	 * primary vs backup bootmodes.
> > > > > > > +	 */
> > > > > > > +	writel(bootindex, K3_BOOT_PARAM_TABLE_INDEX_OCRAM);
> > > > > > > +#endif
> > > > > > > +
> > > > > > >   	/*
> > > > > > >   	 * Force probe of clk_k3 driver here to ensure basic default clock
> > > > > > >   	 * configuration is always done.
> > > > > > > diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h
> > > > > > > index 54380f36e1..9f504f4642 100644
> > > > > > > --- a/arch/arm/mach-k3/include/mach/am62_hardware.h
> > > > > > > +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h
> > > > > > > @@ -76,8 +76,23 @@
> > > > > > >   #define CTRLMMR_MCU_RST_CTRL			(MCU_CTRL_MMR0_BASE + 0x18170)
> > > > > > >   #define ROM_EXTENDED_BOOT_DATA_INFO		0x43c3f1e0
> > > > > > > +#define K3_BOOT_PARAM_TABLE_INDEX_OCRAM         0x7000F290
> > > > > > > +/*
> > > > > > > + * During the boot process ROM will kill anything that writes to OCSRAM.
> > > > > > R5 ROM is long gone when R5 SPL starts, how would it kill anything?
> > > > > Looks like this was based on my patch long ago for the AM62Ax family.
> > > > >  From what little I remember about this was ROM is leaving behind a
> > > > > firewall that we need TIFS's help to bring down for us. So I just
> > > > > blamed ROM 😉
> > > > Thats true. ROM does bare minimum and so wont open up firewall around
> > > > main SRAM. but TIFS does, so you should be able to access this region
> > > > post k3_sysfw_loader().
> > > > 
> > > > > IDK if this is an issue for the AM62x family though.
> > > > > 
> > > > It might be if one tries to "select" DT using EEPROM detect before SYSFW
> > > > is up. But that's not the case any more right?
> > > Yep we still need to figure out a plan for multiple DDR configs or see
> > > if we can move the DDR init to later in the boot as that is the only
> > > thing left that still needs the board detection this early on.
> > > 
> > > There is a little race condition here as TIFS can respond to some
> > > messages before it's finished its init. IDK if we can send it anything
> > > to act like a fence and stall us until the firewalls are down though.
> > 
> > Firewall configurations should be done before TIFS posts boot
> > notification message.
> > 
> > Regards
> > Vignesh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL
  2024-04-01 14:46               ` Bryan Brattlof
@ 2024-04-01 16:53                 ` Wadim Egorov
  0 siblings, 0 replies; 13+ messages in thread
From: Wadim Egorov @ 2024-04-01 16:53 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Raghavendra, Vignesh, u-boot, nm, upstream



Am 01.04.24 um 16:46 schrieb Bryan Brattlof:
> On April  1, 2024 thus sayeth Wadim Egorov:
>> Hi Vignesh, Hi Bryan,
>>
>>
>> Am 05.03.24 um 18:36 schrieb Raghavendra, Vignesh:
>>>
>>>
>>> On 3/5/2024 11:04 PM, Bryan Brattlof wrote:
>>>> On March  5, 2024 thus sayeth Vignesh Raghavendra:
>>>>>
>>>>> On 05/03/24 01:57, Bryan Brattlof wrote:
>>>>>> Hey Vignesh!
>>>>>>
>>>>>> On March  4, 2024 thus sayeth Vignesh Raghavendra:
>>>>>>> Hi Wadim,
>>>>>>>
>>>>>>> On 26/02/24 19:00, Wadim Egorov wrote:
>>>>>>>> Texas Instruments has begun enabling security settings on the SoCs it
>>>>>>>> produces to instruct ROM and TIFS to begin protecting the Security
>>>>>>>> Management Subsystem (SMS) from other binaries we load into the chip by
>>>>>>>> default.
>>>>>>>>
>>>>>>>> One way ROM and TIFS do this is by enabling firewalls to protect the
>>>>>>>> OCSRAM and HSM RAM regions they're using during bootup.
>>>>>>>>
>>>>>>>> The HSM RAM the wakeup SPL is in is firewalled by TIFS to protect
>>>>>>>> itself from the main domain applications. This means the 'bootindex'
>>>>>>>> value in HSM RAM, left by ROM to indicate if we're using the primary
>>>>>>>> or secondary boot-method, must be moved to OCSRAM (that TIFS has open
>>>>>>>> for us) before we make the jump to the main domain so the main domain's
>>>>>>>> bootloaders can keep access to this information.
>>>>>>>>
>>>>>>>> Based on commit
>>>>>>>>     b672e8581070 ("arm: mach-k3: copy bootindex to OCRAM for main domain SPL")
>>
>> I was thinking, even if the reason described here is not right or does not
>> apply to the am62x, it is still a valid solution for carrying this variable
>> into the context for next stage A53 bootloader.
>>
>> store_boot_info_from_rom() stores the index to the bootindex (.data)
>> variable which makes sure it is valid in R5 SPL context. But the next stage
>> bootloader does not know anything about the bootindex variable. So from my
>> understanding it needs to be copied to a different region to preserve the
>> data for next stage bootloaders.
>>
>> Or do I miss something?
> 
> That's correct. We typically put this bootindex variable in the same
> location for both SPLs.

So basically the patch can stay almost as is, but maybe the misleading 
comments in am62_hardware.h should be removed.


> 
> ~Bryan
> 
>>>>>>>>
>>>>>>> FYI, this is mostly a problem in non SPL flow (TI prosperity SBL for
>>>>>>> example) where HSM RAM would be used by HSM firmware. This should be a
>>>>>>> issue in R5 SPL flow.  Do you see any issues today? If so, whats the
>>>>>>> TIFS firmware being used?
>>>>>>>
>>>>>>>> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
>>>>>>>> ---
>>>>>>>>    arch/arm/mach-k3/Kconfig                      |  3 ++-
>>>>>>>>    arch/arm/mach-k3/am625_init.c                 | 15 +++++++++++++--
>>>>>>>>    arch/arm/mach-k3/include/mach/am62_hardware.h | 15 +++++++++++++++
>>>>>>>>    3 files changed, 30 insertions(+), 3 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
>>>>>>>> index 03898424c9..f5d06593f7 100644
>>>>>>>> --- a/arch/arm/mach-k3/Kconfig
>>>>>>>> +++ b/arch/arm/mach-k3/Kconfig
>>>>>>>> @@ -75,7 +75,8 @@ config SYS_K3_BOOT_PARAM_TABLE_INDEX
>>>>>>>>    	default 0x41cffbfc if SOC_K3_J721E
>>>>>>>>    	default 0x41cfdbfc if SOC_K3_J721S2
>>>>>>>>    	default 0x701bebfc if SOC_K3_AM642
>>>>>>>> -	default 0x43c3f290 if SOC_K3_AM625
>>>>>>>> +	default 0x43c3f290 if SOC_K3_AM625 && CPU_V7R
>>>>>>>> +	default 0x7000f290 if SOC_K3_AM625 && ARM64
>>>>>>>>    	default 0x43c3f290 if SOC_K3_AM62A7 && CPU_V7R
>>>>>>>>    	default 0x7000f290 if SOC_K3_AM62A7 && ARM64
>>>>>>>>    	help
>>>>>>>> diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
>>>>>>>> index 6c96e88114..67cf63b103 100644
>>>>>>>> --- a/arch/arm/mach-k3/am625_init.c
>>>>>>>> +++ b/arch/arm/mach-k3/am625_init.c
>>>>>>>> @@ -35,8 +35,10 @@ static struct rom_extended_boot_data bootdata __section(".data");
>>>>>>>>    static void store_boot_info_from_rom(void)
>>>>>>>>    {
>>>>>>>>    	bootindex = *(u32 *)(CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX);
>>>>>>>> -	memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
>>>>>>>> -	       sizeof(struct rom_extended_boot_data));
>>>>>>>> +	if (IS_ENABLED(CONFIG_CPU_V7R)) {
>>>>>>>> +		memcpy(&bootdata, (uintptr_t *)ROM_EXTENDED_BOOT_DATA_INFO,
>>>>>>>> +		       sizeof(struct rom_extended_boot_data));
>>>>>>>> +	}
>>>>>>>>    }
>>>>>>>>    static void ctrl_mmr_unlock(void)
>>>>>>>> @@ -175,6 +177,15 @@ void board_init_f(ulong dummy)
>>>>>>>>    		k3_sysfw_loader(true, NULL, NULL);
>>>>>>>>    	}
>>>>>>>> +#if defined(CONFIG_CPU_V7R)
>>>>>>>> +	/*
>>>>>>>> +	 * Relocate boot information to OCRAM (after TIFS has opend this
>>>>>>>> +	 * region for us) so the next bootloader stages can keep access to
>>>>>>>> +	 * primary vs backup bootmodes.
>>>>>>>> +	 */
>>>>>>>> +	writel(bootindex, K3_BOOT_PARAM_TABLE_INDEX_OCRAM);
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>>    	/*
>>>>>>>>    	 * Force probe of clk_k3 driver here to ensure basic default clock
>>>>>>>>    	 * configuration is always done.
>>>>>>>> diff --git a/arch/arm/mach-k3/include/mach/am62_hardware.h b/arch/arm/mach-k3/include/mach/am62_hardware.h
>>>>>>>> index 54380f36e1..9f504f4642 100644
>>>>>>>> --- a/arch/arm/mach-k3/include/mach/am62_hardware.h
>>>>>>>> +++ b/arch/arm/mach-k3/include/mach/am62_hardware.h
>>>>>>>> @@ -76,8 +76,23 @@
>>>>>>>>    #define CTRLMMR_MCU_RST_CTRL			(MCU_CTRL_MMR0_BASE + 0x18170)
>>>>>>>>    #define ROM_EXTENDED_BOOT_DATA_INFO		0x43c3f1e0
>>>>>>>> +#define K3_BOOT_PARAM_TABLE_INDEX_OCRAM         0x7000F290
>>>>>>>> +/*
>>>>>>>> + * During the boot process ROM will kill anything that writes to OCSRAM.
>>>>>>> R5 ROM is long gone when R5 SPL starts, how would it kill anything?
>>>>>> Looks like this was based on my patch long ago for the AM62Ax family.
>>>>>>   From what little I remember about this was ROM is leaving behind a
>>>>>> firewall that we need TIFS's help to bring down for us. So I just
>>>>>> blamed ROM 😉
>>>>> Thats true. ROM does bare minimum and so wont open up firewall around
>>>>> main SRAM. but TIFS does, so you should be able to access this region
>>>>> post k3_sysfw_loader().
>>>>>
>>>>>> IDK if this is an issue for the AM62x family though.
>>>>>>
>>>>> It might be if one tries to "select" DT using EEPROM detect before SYSFW
>>>>> is up. But that's not the case any more right?
>>>> Yep we still need to figure out a plan for multiple DDR configs or see
>>>> if we can move the DDR init to later in the boot as that is the only
>>>> thing left that still needs the board detection this early on.
>>>>
>>>> There is a little race condition here as TIFS can respond to some
>>>> messages before it's finished its init. IDK if we can send it anything
>>>> to act like a fence and stall us until the firewalls are down though.
>>>
>>> Firewall configurations should be done before TIFS posts boot
>>> notification message.
>>>
>>> Regards
>>> Vignesh

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-04-01 16:53 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-26 13:30 [PATCH 0/2] *** Introduce get_boot_device() for K3 platform *** Wadim Egorov
2024-02-26 13:30 ` [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL Wadim Egorov
2024-03-04  5:06   ` Vignesh Raghavendra
2024-03-04 20:27     ` Bryan Brattlof
2024-03-05  4:36       ` Vignesh Raghavendra
2024-03-05 17:34         ` Bryan Brattlof
2024-03-05 17:36           ` Raghavendra, Vignesh
2024-04-01  9:24             ` Wadim Egorov
2024-04-01 14:46               ` Bryan Brattlof
2024-04-01 16:53                 ` Wadim Egorov
2024-03-06 13:44     ` Wadim Egorov
2024-03-07  4:04       ` Raghavendra, Vignesh
2024-02-26 13:30 ` [PATCH 2/2] arm: mach-k3: am625: Provide a way to obtain boot device for non SPLs Wadim Egorov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.