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

Raghavendra, Vignesh
2024-03-05T17:36:41Z

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