On Sat Jun 7, 2025 at 12:45 AM IST, Tom Rini wrote:
> On Tue, Jun 03, 2025 at 07:54:48PM +0530, Anshul Dalal wrote:
>> We use the spl_board_prepare_for_boot hook to call k3_falcon_prep which
>> is ran after tispl is loaded but before jump_to_image.
>>
>> In here, we find the boot media and load the image just as std falcon
>> flow (since spl_start_uboot returns 0 now). Once the kernel and args are
>> loaded, we perform fdt fixups on the fdt accompanying the kernel (if
>> loaded as FIT) or the loaded up args and return.
>>
>> Now when the flow goes to jump_to_image, we do the regular pre-jump
>> procedure and jump to ATF which jumps to the kernel directly since we
>> have already loaded the kernel and dtb at their respective addresses
>> (PRELOADED_BL33_BASE and K3_HW_CONFIG_BASE).
>>
>> Signed-off-by: Anshul Dalal <ansh...@ti.com>
>> ---
>> arch/arm/mach-k3/common.c | 107 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 107 insertions(+)
>>
>> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
>> index fc230f180d0..70317fec19c 100644
>> --- a/arch/arm/mach-k3/common.c
>> +++ b/arch/arm/mach-k3/common.c
>> @@ -259,8 +259,115 @@ static __maybe_unused void k3_dma_remove(void)
>> pr_warn("DMA Device not found (err=%d)\n", rc);
>> }
>>
>> +#if IS_ENABLED(CONFIG_SPL_OS_BOOT) && !IS_ENABLED(CONFIG_ARM64)
>> +static bool tispl_loaded;
>> +
>> +int spl_start_uboot(void)
>> +{
>> + if (!tispl_loaded)
>> + return 1;
>> + return 0;
>> +}
>> +
>> +static int k3_falcon_fdt_fixup(void *fdt)
>> +{
>> + struct disk_partition info;
>> + struct blk_desc *dev_desc;
>> + char bootmedia[32];
>> + char bootpart[32];
>> + char str[256];
>> + int ret;
>> +
>> + strcpy(bootmedia, env_get("boot"));
>> + strcpy(bootpart, env_get("bootpart"));
>
> Since we're talking secure parts, strncpy is even more important, and
> being aware of:
> * Note that unlike userspace strncpy, this does not %NUL-pad the buffer.
> * However, the result is not %NUL-terminated if the source exceeds
> * @count bytes.
> from lib/string.c
>
Got it, I'll update the call to strncpy(bootmedia, env_get("boot"), 32)
in the next revision.
>> + ret = blk_get_device_part_str(bootmedia, bootpart, &dev_desc, &info, 0);
>> + if (ret < 0)
>> + printf("Failed to get part details for %s %s [%d]\n", bootmedia,
>> + bootpart, ret);
>
> ... and bail out?
>
That makes sense, in secure world we should not be proceeding in such a
case regardless. I'll propogate the error to the top and hang at
spl_prepare_for_boot if anything fails.
> I feel like all the changes here need a read with "and on failure in secure
> mode what can someone abuse this to do" in mind.
