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.ego...@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
