Re: [PATCH 1/2] arm: mach-k3: am625: copy bootindex to OCRAM for main domain SPL

Bryan Brattlof
2024-03-04T20:27:24Z

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