Re: [PATCH v7 08/10] mach-k3: common: enable falcon mode for 62 platform

[Anshul Dalal <anshuld@ti.com>]

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