[PATCH 0/2] board: ti: am6x: Switch to standard boot

[PATCH 0/2] board: ti: am6x: Switch to standard boot

[Roger Quadros]

Hi,

This series switches am64x to standard boot flow.
Also minor fix to am62x boot targets.

NOTE:
Tested on AM64x GP EVM but I had to hand edit the grub.cfg
file provided with the TI release [1]

from,

menuentry 'boot'{
linux /Image root=PARTUUID=3a4a99a8-02 rootwait rootfstype=ext4
}

to,

menuentry 'boot'{
linux /Image console=ttyS2,115200n8 root=PARTUUID=3a4a99a8-02 rootwait rootfstype=ext4
}

else there is no console output.


[1] https://dr-download.ti.com/software-development/software-development-kit-sdk/MD-yXgchBCk98/09.00.00.03/tisdk-default-image-am64xx-evm.wic.xz

cheers,
-roger

Roger Quadros (2):
  board: ti: am62x: am62x.env: Fix boot_targets
  board: ti: am64x: Switch to standard boot flow

 board/ti/am62x/am62x.env        | 2 +-
 board/ti/am64x/am64x.env        | 1 +
 configs/am64x_evm_a53_defconfig | 5 +++--
 include/configs/am64x_evm.h     | 2 --
 4 files changed, 5 insertions(+), 5 deletions(-)


base-commit: 90c81f407dd4a7747385b10f9b8f732202c45cde
prerequisite-patch-id: afb49f33f198747451687f83936f039049365924
prerequisite-patch-id: 1d8297eb0a3b44d8578cae785cc22f0fb6077239
prerequisite-patch-id: 4ae9fdc0b3737e55289a78a59d546a08c03d62e5
prerequisite-patch-id: 2be0e0caf153bec2c453b2f21342ba207c1ee13d
prerequisite-patch-id: 7710703ce9a41b72ff3fb89abf0823625a5b5454
prerequisite-patch-id: 60d61440bf0ab8c0bea8b971ef18aa6d0d26e113
prerequisite-patch-id: c974e6d80489cbb0ee8cc1f3e19bcc9cf47f25b6
-- 
2.34.1

[PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Roger Quadros]

ti_mmc is not a valid boot_target for standard boot flow so
remove it. Prefer mmc1 (sd-card) over mmc0 (emmc).

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 board/ti/am62x/am62x.env | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/ti/am62x/am62x.env b/board/ti/am62x/am62x.env
index 22a6c2c91b..e53a55c38f 100644
--- a/board/ti/am62x/am62x.env
+++ b/board/ti/am62x/am62x.env
@@ -8,7 +8,7 @@ args_all=setenv optargs ${optargs} earlycon=ns16550a,mmio32,0x02800000
 	${mtdparts}
 run_kern=booti ${loadaddr} ${rd_spec} ${fdtaddr}
 
-boot_targets=ti_mmc mmc0 mmc1 usb pxe dhcp
+boot_targets=mmc1 mmc0 usb pxe dhcp
 boot=mmc
 mmcdev=1
 bootpart=1:2
-- 
2.34.1

[PATCH 2/2] board: ti: am64x: Switch to standard boot flow

[Roger Quadros]

Switch to using bootstd. Note with this change, we will stop using
distro_bootcmd and instead depend entirely on bootflow method of
starting the system up.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 board/ti/am64x/am64x.env        | 1 +
 configs/am64x_evm_a53_defconfig | 5 +++--
 include/configs/am64x_evm.h     | 2 --
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/board/ti/am64x/am64x.env b/board/ti/am64x/am64x.env
index 68e42222b7..efd736b99b 100644
--- a/board/ti/am64x/am64x.env
+++ b/board/ti/am64x/am64x.env
@@ -15,6 +15,7 @@ console=ttyS2,115200n8
 args_all=setenv optargs earlycon=ns16550a,mmio32,0x02800000 ${mtdparts}
 run_kern=booti ${loadaddr} ${rd_spec} ${fdtaddr}
 
+boot_targets=mmc1 mmc0 usb pxe dhcp
 boot=mmc
 mmcdev=1
 bootpart=1:2
diff --git a/configs/am64x_evm_a53_defconfig b/configs/am64x_evm_a53_defconfig
index 718ad176cb..43bfcf957a 100644
--- a/configs/am64x_evm_a53_defconfig
+++ b/configs/am64x_evm_a53_defconfig
@@ -31,8 +31,9 @@ CONFIG_SPL_SPI=y
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
 CONFIG_SPL_LOAD_FIT=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x81000000
-CONFIG_DISTRO_DEFAULTS=y
-CONFIG_BOOTCOMMAND="run envboot; run distro_bootcmd;"
+CONFIG_BOOTSTD_FULL=y
+CONFIG_BOOTSTD_DEFAULTS=y
+CONFIG_BOOTCOMMAND="run envboot; bootflow scan -lb"
 CONFIG_BOARD_LATE_INIT=y
 CONFIG_SPL_MAX_SIZE=0x180000
 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
diff --git a/include/configs/am64x_evm.h b/include/configs/am64x_evm.h
index 062102a610..d3d9eac8b6 100644
--- a/include/configs/am64x_evm.h
+++ b/include/configs/am64x_evm.h
@@ -10,8 +10,6 @@
 #define __CONFIG_AM642_EVM_H
 
 #include <linux/sizes.h>
-#include <config_distro_bootcmd.h>
-#include <env/ti/mmc.h>
 #include <asm/arch/am64_hardware.h>
 #include <env/ti/k3_dfu.h>
 
-- 
2.34.1

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Andrew Davis]

On 10/4/23 8:23 AM, Roger Quadros wrote:
> ti_mmc is not a valid boot_target for standard boot flow so

Is there some way to make it into a valid boot_target? Otherwise
how do we use uEnv.txt files, or boot from FIT images with overlays?

Andrew

> remove it. Prefer mmc1 (sd-card) over mmc0 (emmc).
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>   board/ti/am62x/am62x.env | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/board/ti/am62x/am62x.env b/board/ti/am62x/am62x.env
> index 22a6c2c91b..e53a55c38f 100644
> --- a/board/ti/am62x/am62x.env
> +++ b/board/ti/am62x/am62x.env
> @@ -8,7 +8,7 @@ args_all=setenv optargs ${optargs} earlycon=ns16550a,mmio32,0x02800000
>   	${mtdparts}
>   run_kern=booti ${loadaddr} ${rd_spec} ${fdtaddr}
>   
> -boot_targets=ti_mmc mmc0 mmc1 usb pxe dhcp
> +boot_targets=mmc1 mmc0 usb pxe dhcp
>   boot=mmc
>   mmcdev=1
>   bootpart=1:2

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Nishanth Menon]

On 08:48-20231004, Andrew Davis wrote:
> On 10/4/23 8:23 AM, Roger Quadros wrote:
> > ti_mmc is not a valid boot_target for standard boot flow so
> 
> Is there some way to make it into a valid boot_target? Otherwise
> how do we use uEnv.txt files, or boot from FIT images with overlays?

envboot takes care of uEnv.txt file (see
https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)

Early remote proc loading and FIT image is a question for stdboot itself.

> 
> Andrew
> 
> > remove it. Prefer mmc1 (sd-card) over mmc0 (emmc).
> > 
> > Signed-off-by: Roger Quadros <rogerq@kernel.org>
> > ---
> >   board/ti/am62x/am62x.env | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/board/ti/am62x/am62x.env b/board/ti/am62x/am62x.env
> > index 22a6c2c91b..e53a55c38f 100644
> > --- a/board/ti/am62x/am62x.env
> > +++ b/board/ti/am62x/am62x.env
> > @@ -8,7 +8,7 @@ args_all=setenv optargs ${optargs} earlycon=ns16550a,mmio32,0x02800000
> >   	${mtdparts}
> >   run_kern=booti ${loadaddr} ${rd_spec} ${fdtaddr}
> > -boot_targets=ti_mmc mmc0 mmc1 usb pxe dhcp
> > +boot_targets=mmc1 mmc0 usb pxe dhcp
> >   boot=mmc
> >   mmcdev=1
> >   bootpart=1:2

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 2/2] board: ti: am64x: Switch to standard boot flow

[Nishanth Menon]

On 16:23-20231004, Roger Quadros wrote:
> Switch to using bootstd. Note with this change, we will stop using
> distro_bootcmd and instead depend entirely on bootflow method of
> starting the system up.
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>


> ---
>  board/ti/am64x/am64x.env        | 1 +
>  configs/am64x_evm_a53_defconfig | 5 +++--
>  include/configs/am64x_evm.h     | 2 --
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/board/ti/am64x/am64x.env b/board/ti/am64x/am64x.env
> index 68e42222b7..efd736b99b 100644
> --- a/board/ti/am64x/am64x.env
> +++ b/board/ti/am64x/am64x.env
> @@ -15,6 +15,7 @@ console=ttyS2,115200n8
>  args_all=setenv optargs earlycon=ns16550a,mmio32,0x02800000 ${mtdparts}
>  run_kern=booti ${loadaddr} ${rd_spec} ${fdtaddr}
>  
> +boot_targets=mmc1 mmc0 usb pxe dhcp
>  boot=mmc
>  mmcdev=1
>  bootpart=1:2
> diff --git a/configs/am64x_evm_a53_defconfig b/configs/am64x_evm_a53_defconfig
> index 718ad176cb..43bfcf957a 100644
> --- a/configs/am64x_evm_a53_defconfig
> +++ b/configs/am64x_evm_a53_defconfig
> @@ -31,8 +31,9 @@ CONFIG_SPL_SPI=y
>  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
>  CONFIG_SPL_LOAD_FIT=y
>  CONFIG_SPL_LOAD_FIT_ADDRESS=0x81000000
> -CONFIG_DISTRO_DEFAULTS=y
> -CONFIG_BOOTCOMMAND="run envboot; run distro_bootcmd;"
> +CONFIG_BOOTSTD_FULL=y
> +CONFIG_BOOTSTD_DEFAULTS=y
> +CONFIG_BOOTCOMMAND="run envboot; bootflow scan -lb"
>  CONFIG_BOARD_LATE_INIT=y
>  CONFIG_SPL_MAX_SIZE=0x180000
>  CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
> diff --git a/include/configs/am64x_evm.h b/include/configs/am64x_evm.h
> index 062102a610..d3d9eac8b6 100644
> --- a/include/configs/am64x_evm.h
> +++ b/include/configs/am64x_evm.h
> @@ -10,8 +10,6 @@
>  #define __CONFIG_AM642_EVM_H
>  
>  #include <linux/sizes.h>
Do we really need this?
> -#include <config_distro_bootcmd.h>
> -#include <env/ti/mmc.h>
>  #include <asm/arch/am64_hardware.h>

OR this?

>  #include <env/ti/k3_dfu.h>
you dont need k3_dfu.h either. the env setup is already in
board/ti/am64x/am64x.env (k3_dfu.env)

and we dont seem to be using CFG_SYS_SDRAM_BASE1

might be that we could reduce it down to am62x_evm.h ?

>  
> -- 
> 2.34.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Nishanth Menon]

On 16:23-20231004, Roger Quadros wrote:
> ti_mmc is not a valid boot_target for standard boot flow so
> remove it. Prefer mmc1 (sd-card) over mmc0 (emmc).
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>

Reviewed-by: Nishanth Menon <nm@ti.com>

> ---
>  board/ti/am62x/am62x.env | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/board/ti/am62x/am62x.env b/board/ti/am62x/am62x.env
> index 22a6c2c91b..e53a55c38f 100644
> --- a/board/ti/am62x/am62x.env
> +++ b/board/ti/am62x/am62x.env
> @@ -8,7 +8,7 @@ args_all=setenv optargs ${optargs} earlycon=ns16550a,mmio32,0x02800000
>  	${mtdparts}
>  run_kern=booti ${loadaddr} ${rd_spec} ${fdtaddr}
>  
> -boot_targets=ti_mmc mmc0 mmc1 usb pxe dhcp
> +boot_targets=mmc1 mmc0 usb pxe dhcp
>  boot=mmc
>  mmcdev=1
>  bootpart=1:2
> -- 
> 2.34.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Andrew Davis]

On 10/4/23 8:54 AM, Nishanth Menon wrote:
> On 08:48-20231004, Andrew Davis wrote:
>> On 10/4/23 8:23 AM, Roger Quadros wrote:
>>> ti_mmc is not a valid boot_target for standard boot flow so
>>
>> Is there some way to make it into a valid boot_target? Otherwise
>> how do we use uEnv.txt files, or boot from FIT images with overlays?
> 
> envboot takes care of uEnv.txt file (see
> https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> 
> Early remote proc loading and FIT image is a question for stdboot itself.
> 

If stdboot is missing these features then we shouldn't switch until it
has them. I'm all for switching to this, but only if it is complete.

Andrew

>>
>> Andrew
>>
>>> remove it. Prefer mmc1 (sd-card) over mmc0 (emmc).
>>>
>>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>>> ---
>>>    board/ti/am62x/am62x.env | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/board/ti/am62x/am62x.env b/board/ti/am62x/am62x.env
>>> index 22a6c2c91b..e53a55c38f 100644
>>> --- a/board/ti/am62x/am62x.env
>>> +++ b/board/ti/am62x/am62x.env
>>> @@ -8,7 +8,7 @@ args_all=setenv optargs ${optargs} earlycon=ns16550a,mmio32,0x02800000
>>>    	${mtdparts}
>>>    run_kern=booti ${loadaddr} ${rd_spec} ${fdtaddr}
>>> -boot_targets=ti_mmc mmc0 mmc1 usb pxe dhcp
>>> +boot_targets=mmc1 mmc0 usb pxe dhcp
>>>    boot=mmc
>>>    mmcdev=1
>>>    bootpart=1:2
> 

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]

On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > On 08:48-20231004, Andrew Davis wrote:
> > > On 10/4/23 8:23 AM, Roger Quadros wrote:
> > > > ti_mmc is not a valid boot_target for standard boot flow so
> > > 
> > > Is there some way to make it into a valid boot_target? Otherwise
> > > how do we use uEnv.txt files, or boot from FIT images with overlays?
> > 
> > envboot takes care of uEnv.txt file (see
> > https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > 
> > Early remote proc loading and FIT image is a question for stdboot itself.
> > 
> 
> If stdboot is missing these features then we shouldn't switch until it
> has them. I'm all for switching to this, but only if it is complete.

Depends on what you mean?  Did you mean an option to run scripts
(exists) or an option to do what TI needs done, via
boot/bootmeth_something.c ?  If the latter, someone from TI needs to
figure out what that should be and do (but plumbing-wise everything it
needs should exist).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Nishanth Menon]

On 12:36-20231005, Tom Rini wrote:
> On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> > On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > > On 08:48-20231004, Andrew Davis wrote:
> > > > On 10/4/23 8:23 AM, Roger Quadros wrote:
> > > > > ti_mmc is not a valid boot_target for standard boot flow so
> > > > 
> > > > Is there some way to make it into a valid boot_target? Otherwise
> > > > how do we use uEnv.txt files, or boot from FIT images with overlays?
> > > 
> > > envboot takes care of uEnv.txt file (see
> > > https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > > 
> > > Early remote proc loading and FIT image is a question for stdboot itself.
> > > 
> > 
> > If stdboot is missing these features then we shouldn't switch until it
> > has them. I'm all for switching to this, but only if it is complete.
> 
> Depends on what you mean?  Did you mean an option to run scripts
> (exists) or an option to do what TI needs done, via
> boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> figure out what that should be and do (but plumbing-wise everything it
> needs should exist).

Andrew is generalizing here (on the wrong patch though).

On am62x platforms, there is nothing regressing with this series. The
challenge is early remote_proc loading which is done for J7* platforms.

How that is initiated as part of bootmethods is something of a gap.

The other gap has been support for uEnv.txt -> which we can workaround
at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
-lb" in defconfig (This series from Roger already does that - hence I am
saying that Andrew is complaining on the wrong series).

Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
uEnv.txt remoteproc loads and the various standard bootmethods should
"just work".

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Nishanth Menon]

On 12:10-20231005, Nishanth Menon wrote:
> On 12:36-20231005, Tom Rini wrote:
> > On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> > > On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > > > On 08:48-20231004, Andrew Davis wrote:
> > > > > On 10/4/23 8:23 AM, Roger Quadros wrote:
> > > > > > ti_mmc is not a valid boot_target for standard boot flow so
> > > > > 
> > > > > Is there some way to make it into a valid boot_target? Otherwise
> > > > > how do we use uEnv.txt files, or boot from FIT images with overlays?
> > > > 
> > > > envboot takes care of uEnv.txt file (see
> > > > https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > > > 
> > > > Early remote proc loading and FIT image is a question for stdboot itself.
> > > > 
> > > 
> > > If stdboot is missing these features then we shouldn't switch until it
> > > has them. I'm all for switching to this, but only if it is complete.
> > 
> > Depends on what you mean?  Did you mean an option to run scripts
> > (exists) or an option to do what TI needs done, via
> > boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> > figure out what that should be and do (but plumbing-wise everything it
> > needs should exist).
> 
> Andrew is generalizing here (on the wrong patch though).
> 
> On am62x platforms, there is nothing regressing with this series. The
> challenge is early remote_proc loading which is done for J7* platforms.
> 
> How that is initiated as part of bootmethods is something of a gap.
> 
> The other gap has been support for uEnv.txt -> which we can workaround
> at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> -lb" in defconfig (This series from Roger already does that - hence I am
> saying that Andrew is complaining on the wrong series).
> 
> Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
> uEnv.txt remoteproc loads and the various standard bootmethods should
> "just work".


I forgot to add: FIT image authenticated boot flow. That is really what
ti_mmc distroboot method was trying to solve.

Maybe Simon or someone know how the stdboot flow handles authenticated
kernel image and dtb boot flow with FIT image?

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Andrew Davis]

On 10/5/23 12:16 PM, Nishanth Menon wrote:
> On 12:10-20231005, Nishanth Menon wrote:
>> On 12:36-20231005, Tom Rini wrote:
>>> On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
>>>> On 10/4/23 8:54 AM, Nishanth Menon wrote:
>>>>> On 08:48-20231004, Andrew Davis wrote:
>>>>>> On 10/4/23 8:23 AM, Roger Quadros wrote:
>>>>>>> ti_mmc is not a valid boot_target for standard boot flow so
>>>>>>
>>>>>> Is there some way to make it into a valid boot_target? Otherwise
>>>>>> how do we use uEnv.txt files, or boot from FIT images with overlays?
>>>>>
>>>>> envboot takes care of uEnv.txt file (see
>>>>> https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
>>>>>
>>>>> Early remote proc loading and FIT image is a question for stdboot itself.
>>>>>
>>>>
>>>> If stdboot is missing these features then we shouldn't switch until it
>>>> has them. I'm all for switching to this, but only if it is complete.
>>>
>>> Depends on what you mean?  Did you mean an option to run scripts
>>> (exists) or an option to do what TI needs done, via
>>> boot/bootmeth_something.c ?  If the latter, someone from TI needs to
>>> figure out what that should be and do (but plumbing-wise everything it
>>> needs should exist).
>>
>> Andrew is generalizing here (on the wrong patch though).
>>
>> On am62x platforms, there is nothing regressing with this series. The
>> challenge is early remote_proc loading which is done for J7* platforms.
>>
>> How that is initiated as part of bootmethods is something of a gap.
>>
>> The other gap has been support for uEnv.txt -> which we can workaround
>> at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
>> -lb" in defconfig (This series from Roger already does that - hence I am
>> saying that Andrew is complaining on the wrong series).
>>
>> Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
>> uEnv.txt remoteproc loads and the various standard bootmethods should
>> "just work".
> 
> 
> I forgot to add: FIT image authenticated boot flow. That is really what
> ti_mmc distroboot method was trying to solve.
> 

Right, FIT (and remote proc loading) are handled in ti_mmc, and this
is the patch removing that (so I do believe I am complaining on the right
patch here). I know that needs removed before we switch to stdboot, but
we cant remove it until someone has the the alternative in place.

Simply dropping it and hoping someone else comes along later and re-adds
the features isn't a good idea. Even if, as said above, the plumbing we
need should already exist, it needs done first.

Andrew

> Maybe Simon or someone know how the stdboot flow handles authenticated
> kernel image and dtb boot flow with FIT image?
> 

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Simon Glass]

Hi Nishanth,

On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
>
> On 12:10-20231005, Nishanth Menon wrote:
> > On 12:36-20231005, Tom Rini wrote:
> > > On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> > > > On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > > > > On 08:48-20231004, Andrew Davis wrote:
> > > > > > On 10/4/23 8:23 AM, Roger Quadros wrote:
> > > > > > > ti_mmc is not a valid boot_target for standard boot flow so
> > > > > >
> > > > > > Is there some way to make it into a valid boot_target? Otherwise
> > > > > > how do we use uEnv.txt files, or boot from FIT images with overlays?
> > > > >
> > > > > envboot takes care of uEnv.txt file (see
> > > > > https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > > > >
> > > > > Early remote proc loading and FIT image is a question for stdboot itself.
> > > > >
> > > >
> > > > If stdboot is missing these features then we shouldn't switch until it
> > > > has them. I'm all for switching to this, but only if it is complete.
> > >
> > > Depends on what you mean?  Did you mean an option to run scripts
> > > (exists) or an option to do what TI needs done, via
> > > boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> > > figure out what that should be and do (but plumbing-wise everything it
> > > needs should exist).
> >
> > Andrew is generalizing here (on the wrong patch though).
> >
> > On am62x platforms, there is nothing regressing with this series. The
> > challenge is early remote_proc loading which is done for J7* platforms.
> >
> > How that is initiated as part of bootmethods is something of a gap.
> >
> > The other gap has been support for uEnv.txt -> which we can workaround
> > at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> > -lb" in defconfig (This series from Roger already does that - hence I am
> > saying that Andrew is complaining on the wrong series).
> >
> > Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
> > uEnv.txt remoteproc loads and the various standard bootmethods should
> > "just work".
>
>
> I forgot to add: FIT image authenticated boot flow. That is really what
> ti_mmc distroboot method was trying to solve.
>
> Maybe Simon or someone know how the stdboot flow handles authenticated
> kernel image and dtb boot flow with FIT image?

Yes you can use FIT configuration verification and things should work as normal.

Regards,
Simon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Nishanth Menon]

On 12:22-20231005, Andrew Davis wrote:
> On 10/5/23 12:16 PM, Nishanth Menon wrote:
> > On 12:10-20231005, Nishanth Menon wrote:
> > > On 12:36-20231005, Tom Rini wrote:
> > > > On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> > > > > On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > > > > > On 08:48-20231004, Andrew Davis wrote:
> > > > > > > On 10/4/23 8:23 AM, Roger Quadros wrote:
> > > > > > > > ti_mmc is not a valid boot_target for standard boot flow so
> > > > > > > 
> > > > > > > Is there some way to make it into a valid boot_target? Otherwise
> > > > > > > how do we use uEnv.txt files, or boot from FIT images with overlays?
> > > > > > 
> > > > > > envboot takes care of uEnv.txt file (see
> > > > > > https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > > > > > 
> > > > > > Early remote proc loading and FIT image is a question for stdboot itself.
> > > > > > 
> > > > > 
> > > > > If stdboot is missing these features then we shouldn't switch until it
> > > > > has them. I'm all for switching to this, but only if it is complete.
> > > > 
> > > > Depends on what you mean?  Did you mean an option to run scripts
> > > > (exists) or an option to do what TI needs done, via
> > > > boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> > > > figure out what that should be and do (but plumbing-wise everything it
> > > > needs should exist).
> > > 
> > > Andrew is generalizing here (on the wrong patch though).
> > > 
> > > On am62x platforms, there is nothing regressing with this series. The
> > > challenge is early remote_proc loading which is done for J7* platforms.
> > > 
> > > How that is initiated as part of bootmethods is something of a gap.
> > > 
> > > The other gap has been support for uEnv.txt -> which we can workaround
> > > at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> > > -lb" in defconfig (This series from Roger already does that - hence I am
> > > saying that Andrew is complaining on the wrong series).
> > > 
> > > Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
> > > uEnv.txt remoteproc loads and the various standard bootmethods should
> > > "just work".
> > 
> > 
> > I forgot to add: FIT image authenticated boot flow. That is really what
> > ti_mmc distroboot method was trying to solve.
> > 
> 
> Right, FIT (and remote proc loading) are handled in ti_mmc, and this
> is the patch removing that (so I do believe I am complaining on the right
> patch here). I know that needs removed before we switch to stdboot, but
> we cant remove it until someone has the the alternative in place.
> 
> Simply dropping it and hoping someone else comes along later and re-adds
> the features isn't a good idea. Even if, as said above, the plumbing we
> need should already exist, it needs done first.

As simon stated:
https://lore.kernel.org/all/CAPnjgZ1fnqgk5W_8-jkJL1keXGM4CfnK-jPnFtQcFcV+y89+cA@mail.gmail.com/
FIT is already supported.

remoteproc support was not yet added for am62.. so there is no real
regression, that said, even then, it can still be handled by envboot
till we solve that aspect. There is no real reason to hold back stdboot
as the standardization is very valuable at this point.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Roger Quadros]

Hi Simon,

On 05/10/2023 20:22, Simon Glass wrote:
> Hi Nishanth,
> 
> On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
>>
>> On 12:10-20231005, Nishanth Menon wrote:
>>> On 12:36-20231005, Tom Rini wrote:
>>>> On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
>>>>> On 10/4/23 8:54 AM, Nishanth Menon wrote:
>>>>>> On 08:48-20231004, Andrew Davis wrote:
>>>>>>> On 10/4/23 8:23 AM, Roger Quadros wrote:
>>>>>>>> ti_mmc is not a valid boot_target for standard boot flow so
>>>>>>>
>>>>>>> Is there some way to make it into a valid boot_target? Otherwise
>>>>>>> how do we use uEnv.txt files, or boot from FIT images with overlays?
>>>>>>
>>>>>> envboot takes care of uEnv.txt file (see
>>>>>> https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
>>>>>>
>>>>>> Early remote proc loading and FIT image is a question for stdboot itself.
>>>>>>
>>>>>
>>>>> If stdboot is missing these features then we shouldn't switch until it
>>>>> has them. I'm all for switching to this, but only if it is complete.
>>>>
>>>> Depends on what you mean?  Did you mean an option to run scripts
>>>> (exists) or an option to do what TI needs done, via
>>>> boot/bootmeth_something.c ?  If the latter, someone from TI needs to
>>>> figure out what that should be and do (but plumbing-wise everything it
>>>> needs should exist).
>>>
>>> Andrew is generalizing here (on the wrong patch though).
>>>
>>> On am62x platforms, there is nothing regressing with this series. The
>>> challenge is early remote_proc loading which is done for J7* platforms.
>>>
>>> How that is initiated as part of bootmethods is something of a gap.
>>>
>>> The other gap has been support for uEnv.txt -> which we can workaround
>>> at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
>>> -lb" in defconfig (This series from Roger already does that - hence I am
>>> saying that Andrew is complaining on the wrong series).
>>>
>>> Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
>>> uEnv.txt remoteproc loads and the various standard bootmethods should
>>> "just work".
>>
>>
>> I forgot to add: FIT image authenticated boot flow. That is really what
>> ti_mmc distroboot method was trying to solve.
>>
>> Maybe Simon or someone know how the stdboot flow handles authenticated
>> kernel image and dtb boot flow with FIT image?
> 
> Yes you can use FIT configuration verification and things should work as normal.
> 

It there any reference to look at on how this is done? Thanks.

-- 
cheers,
-roger

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Manorit Chawdhry]

Hi Simon,

On 11:22-20231005, Simon Glass wrote:
> Hi Nishanth,
> 
> On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
> >
> > On 12:10-20231005, Nishanth Menon wrote:
> > > On 12:36-20231005, Tom Rini wrote:
> > > > On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> > > > > On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > > > > > On 08:48-20231004, Andrew Davis wrote:
> > > > > > > On 10/4/23 8:23 AM, Roger Quadros wrote:
> > > > > > > > ti_mmc is not a valid boot_target for standard boot flow so
> > > > > > >
> > > > > > > Is there some way to make it into a valid boot_target? Otherwise
> > > > > > > how do we use uEnv.txt files, or boot from FIT images with overlays?
> > > > > >
> > > > > > envboot takes care of uEnv.txt file (see
> > > > > > https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > > > > >
> > > > > > Early remote proc loading and FIT image is a question for stdboot itself.
> > > > > >
> > > > >
> > > > > If stdboot is missing these features then we shouldn't switch until it
> > > > > has them. I'm all for switching to this, but only if it is complete.
> > > >
> > > > Depends on what you mean?  Did you mean an option to run scripts
> > > > (exists) or an option to do what TI needs done, via
> > > > boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> > > > figure out what that should be and do (but plumbing-wise everything it
> > > > needs should exist).
> > >
> > > Andrew is generalizing here (on the wrong patch though).
> > >
> > > On am62x platforms, there is nothing regressing with this series. The
> > > challenge is early remote_proc loading which is done for J7* platforms.
> > >
> > > How that is initiated as part of bootmethods is something of a gap.
> > >
> > > The other gap has been support for uEnv.txt -> which we can workaround
> > > at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> > > -lb" in defconfig (This series from Roger already does that - hence I am
> > > saying that Andrew is complaining on the wrong series).
> > >
> > > Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
> > > uEnv.txt remoteproc loads and the various standard bootmethods should
> > > "just work".
> >
> >
> > I forgot to add: FIT image authenticated boot flow. That is really what
> > ti_mmc distroboot method was trying to solve.
> >
> > Maybe Simon or someone know how the stdboot flow handles authenticated
> > kernel image and dtb boot flow with FIT image?
> 
> Yes you can use FIT configuration verification and things should work as normal.
> 

Could you give any reference documentation for this?

Regards,
Manorit

> Regards,
> Simon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2980 bytes --]

On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
> Hi Simon,
> 
> On 11:22-20231005, Simon Glass wrote:
> > Hi Nishanth,
> > 
> > On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
> > >
> > > On 12:10-20231005, Nishanth Menon wrote:
> > > > On 12:36-20231005, Tom Rini wrote:
> > > > > On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> > > > > > On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > > > > > > On 08:48-20231004, Andrew Davis wrote:
> > > > > > > > On 10/4/23 8:23 AM, Roger Quadros wrote:
> > > > > > > > > ti_mmc is not a valid boot_target for standard boot flow so
> > > > > > > >
> > > > > > > > Is there some way to make it into a valid boot_target? Otherwise
> > > > > > > > how do we use uEnv.txt files, or boot from FIT images with overlays?
> > > > > > >
> > > > > > > envboot takes care of uEnv.txt file (see
> > > > > > > https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > > > > > >
> > > > > > > Early remote proc loading and FIT image is a question for stdboot itself.
> > > > > > >
> > > > > >
> > > > > > If stdboot is missing these features then we shouldn't switch until it
> > > > > > has them. I'm all for switching to this, but only if it is complete.
> > > > >
> > > > > Depends on what you mean?  Did you mean an option to run scripts
> > > > > (exists) or an option to do what TI needs done, via
> > > > > boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> > > > > figure out what that should be and do (but plumbing-wise everything it
> > > > > needs should exist).
> > > >
> > > > Andrew is generalizing here (on the wrong patch though).
> > > >
> > > > On am62x platforms, there is nothing regressing with this series. The
> > > > challenge is early remote_proc loading which is done for J7* platforms.
> > > >
> > > > How that is initiated as part of bootmethods is something of a gap.
> > > >
> > > > The other gap has been support for uEnv.txt -> which we can workaround
> > > > at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> > > > -lb" in defconfig (This series from Roger already does that - hence I am
> > > > saying that Andrew is complaining on the wrong series).
> > > >
> > > > Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
> > > > uEnv.txt remoteproc loads and the various standard bootmethods should
> > > > "just work".
> > >
> > >
> > > I forgot to add: FIT image authenticated boot flow. That is really what
> > > ti_mmc distroboot method was trying to solve.
> > >
> > > Maybe Simon or someone know how the stdboot flow handles authenticated
> > > kernel image and dtb boot flow with FIT image?
> > 
> > Yes you can use FIT configuration verification and things should work as normal.
> > 
> 
> Could you give any reference documentation for this?

I suspect you should start with doc/usage/fit/beaglebone_vboot.rst

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Andrew Davis]

On 11/6/23 9:31 AM, Tom Rini wrote:
> On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
>> Hi Simon,
>>
>> On 11:22-20231005, Simon Glass wrote:
>>> Hi Nishanth,
>>>
>>> On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
>>>>
>>>> On 12:10-20231005, Nishanth Menon wrote:
>>>>> On 12:36-20231005, Tom Rini wrote:
>>>>>> On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
>>>>>>> On 10/4/23 8:54 AM, Nishanth Menon wrote:
>>>>>>>> On 08:48-20231004, Andrew Davis wrote:
>>>>>>>>> On 10/4/23 8:23 AM, Roger Quadros wrote:
>>>>>>>>>> ti_mmc is not a valid boot_target for standard boot flow so
>>>>>>>>>
>>>>>>>>> Is there some way to make it into a valid boot_target? Otherwise
>>>>>>>>> how do we use uEnv.txt files, or boot from FIT images with overlays?
>>>>>>>>
>>>>>>>> envboot takes care of uEnv.txt file (see
>>>>>>>> https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
>>>>>>>>
>>>>>>>> Early remote proc loading and FIT image is a question for stdboot itself.
>>>>>>>>
>>>>>>>
>>>>>>> If stdboot is missing these features then we shouldn't switch until it
>>>>>>> has them. I'm all for switching to this, but only if it is complete.
>>>>>>
>>>>>> Depends on what you mean?  Did you mean an option to run scripts
>>>>>> (exists) or an option to do what TI needs done, via
>>>>>> boot/bootmeth_something.c ?  If the latter, someone from TI needs to
>>>>>> figure out what that should be and do (but plumbing-wise everything it
>>>>>> needs should exist).
>>>>>
>>>>> Andrew is generalizing here (on the wrong patch though).
>>>>>
>>>>> On am62x platforms, there is nothing regressing with this series. The
>>>>> challenge is early remote_proc loading which is done for J7* platforms.
>>>>>
>>>>> How that is initiated as part of bootmethods is something of a gap.
>>>>>
>>>>> The other gap has been support for uEnv.txt -> which we can workaround
>>>>> at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
>>>>> -lb" in defconfig (This series from Roger already does that - hence I am
>>>>> saying that Andrew is complaining on the wrong series).
>>>>>
>>>>> Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
>>>>> uEnv.txt remoteproc loads and the various standard bootmethods should
>>>>> "just work".
>>>>
>>>>
>>>> I forgot to add: FIT image authenticated boot flow. That is really what
>>>> ti_mmc distroboot method was trying to solve.
>>>>
>>>> Maybe Simon or someone know how the stdboot flow handles authenticated
>>>> kernel image and dtb boot flow with FIT image?
>>>
>>> Yes you can use FIT configuration verification and things should work as normal.
>>>
>>
>> Could you give any reference documentation for this?
> 
> I suspect you should start with doc/usage/fit/beaglebone_vboot.rst
> 

 From that doc:

```
Boot the board using the commands below::

     setenv bootargs console=ttyO0,115200n8 quiet root=/dev/mmcblk0p2 ro rootfstype=ext4 rootwait
     ext2load mmc 0:2 82000000 /boot/image.fit
     bootm 82000000
```

This is very much not stdboot :( This doc has some good and current info on building
a secure FIT image, but much of it is showing its age. Stdboot is still missing

  * ability to limit boot mode search to only one image (FIT)
  * fetching and applying FIT overlay/config strings
  * remote proc loading
  * uEnv.txt processing

Last two can be solved by adding to CONFIG_BOOTCOMMAND before calling `bootflow scan`
as suggested above. Until we figure out the first two I don't see us ready to switch.

Andrew

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Simon Glass]

Hi Andrew,

On Mon, 6 Nov 2023 at 10:27, Andrew Davis <afd@ti.com> wrote:
>
> On 11/6/23 9:31 AM, Tom Rini wrote:
> > On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
> >> Hi Simon,
> >>
> >> On 11:22-20231005, Simon Glass wrote:
> >>> Hi Nishanth,
> >>>
> >>> On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
> >>>>
> >>>> On 12:10-20231005, Nishanth Menon wrote:
> >>>>> On 12:36-20231005, Tom Rini wrote:
> >>>>>> On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> >>>>>>> On 10/4/23 8:54 AM, Nishanth Menon wrote:
> >>>>>>>> On 08:48-20231004, Andrew Davis wrote:
> >>>>>>>>> On 10/4/23 8:23 AM, Roger Quadros wrote:
> >>>>>>>>>> ti_mmc is not a valid boot_target for standard boot flow so
> >>>>>>>>>
> >>>>>>>>> Is there some way to make it into a valid boot_target? Otherwise
> >>>>>>>>> how do we use uEnv.txt files, or boot from FIT images with overlays?
> >>>>>>>>
> >>>>>>>> envboot takes care of uEnv.txt file (see
> >>>>>>>> https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> >>>>>>>>
> >>>>>>>> Early remote proc loading and FIT image is a question for stdboot itself.
> >>>>>>>>
> >>>>>>>
> >>>>>>> If stdboot is missing these features then we shouldn't switch until it
> >>>>>>> has them. I'm all for switching to this, but only if it is complete.
> >>>>>>
> >>>>>> Depends on what you mean?  Did you mean an option to run scripts
> >>>>>> (exists) or an option to do what TI needs done, via
> >>>>>> boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> >>>>>> figure out what that should be and do (but plumbing-wise everything it
> >>>>>> needs should exist).
> >>>>>
> >>>>> Andrew is generalizing here (on the wrong patch though).
> >>>>>
> >>>>> On am62x platforms, there is nothing regressing with this series. The
> >>>>> challenge is early remote_proc loading which is done for J7* platforms.
> >>>>>
> >>>>> How that is initiated as part of bootmethods is something of a gap.
> >>>>>
> >>>>> The other gap has been support for uEnv.txt -> which we can workaround
> >>>>> at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> >>>>> -lb" in defconfig (This series from Roger already does that - hence I am
> >>>>> saying that Andrew is complaining on the wrong series).
> >>>>>
> >>>>> Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
> >>>>> uEnv.txt remoteproc loads and the various standard bootmethods should
> >>>>> "just work".
> >>>>
> >>>>
> >>>> I forgot to add: FIT image authenticated boot flow. That is really what
> >>>> ti_mmc distroboot method was trying to solve.
> >>>>
> >>>> Maybe Simon or someone know how the stdboot flow handles authenticated
> >>>> kernel image and dtb boot flow with FIT image?
> >>>
> >>> Yes you can use FIT configuration verification and things should work as normal.
> >>>
> >>
> >> Could you give any reference documentation for this?
> >
> > I suspect you should start with doc/usage/fit/beaglebone_vboot.rst
> >
>
>  From that doc:
>
> ```
> Boot the board using the commands below::
>
>      setenv bootargs console=ttyO0,115200n8 quiet root=/dev/mmcblk0p2 ro rootfstype=ext4 rootwait
>      ext2load mmc 0:2 82000000 /boot/image.fit
>      bootm 82000000
> ```
>
> This is very much not stdboot :( This doc has some good and current info on building
> a secure FIT image, but much of it is showing its age. Stdboot is still missing
>
>   * ability to limit boot mode search to only one image (FIT)

What does this mean? Can you please be more specific or give an example?

>   * fetching and applying FIT overlay/config strings

Re overlays, is this the 'extension' command? What are config strings?

>   * remote proc loading

What command can do this?

>   * uEnv.txt processing

What command can do this?

>
> Last two can be solved by adding to CONFIG_BOOTCOMMAND before calling `bootflow scan`
> as suggested above. Until we figure out the first two I don't see us ready to switch.

I'm happy to help figure this out.

Regards,
SImon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Andrew Davis]

On 11/6/23 11:47 AM, Simon Glass wrote:
> Hi Andrew,
> 
> On Mon, 6 Nov 2023 at 10:27, Andrew Davis <afd@ti.com> wrote:
>>
>> On 11/6/23 9:31 AM, Tom Rini wrote:
>>> On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
>>>> Hi Simon,
>>>>
>>>> On 11:22-20231005, Simon Glass wrote:
>>>>> Hi Nishanth,
>>>>>
>>>>> On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
>>>>>>
>>>>>> On 12:10-20231005, Nishanth Menon wrote:
>>>>>>> On 12:36-20231005, Tom Rini wrote:
>>>>>>>> On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
>>>>>>>>> On 10/4/23 8:54 AM, Nishanth Menon wrote:
>>>>>>>>>> On 08:48-20231004, Andrew Davis wrote:
>>>>>>>>>>> On 10/4/23 8:23 AM, Roger Quadros wrote:
>>>>>>>>>>>> ti_mmc is not a valid boot_target for standard boot flow so
>>>>>>>>>>>
>>>>>>>>>>> Is there some way to make it into a valid boot_target? Otherwise
>>>>>>>>>>> how do we use uEnv.txt files, or boot from FIT images with overlays?
>>>>>>>>>>
>>>>>>>>>> envboot takes care of uEnv.txt file (see
>>>>>>>>>> https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
>>>>>>>>>>
>>>>>>>>>> Early remote proc loading and FIT image is a question for stdboot itself.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> If stdboot is missing these features then we shouldn't switch until it
>>>>>>>>> has them. I'm all for switching to this, but only if it is complete.
>>>>>>>>
>>>>>>>> Depends on what you mean?  Did you mean an option to run scripts
>>>>>>>> (exists) or an option to do what TI needs done, via
>>>>>>>> boot/bootmeth_something.c ?  If the latter, someone from TI needs to
>>>>>>>> figure out what that should be and do (but plumbing-wise everything it
>>>>>>>> needs should exist).
>>>>>>>
>>>>>>> Andrew is generalizing here (on the wrong patch though).
>>>>>>>
>>>>>>> On am62x platforms, there is nothing regressing with this series. The
>>>>>>> challenge is early remote_proc loading which is done for J7* platforms.
>>>>>>>
>>>>>>> How that is initiated as part of bootmethods is something of a gap.
>>>>>>>
>>>>>>> The other gap has been support for uEnv.txt -> which we can workaround
>>>>>>> at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
>>>>>>> -lb" in defconfig (This series from Roger already does that - hence I am
>>>>>>> saying that Andrew is complaining on the wrong series).
>>>>>>>
>>>>>>> Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
>>>>>>> uEnv.txt remoteproc loads and the various standard bootmethods should
>>>>>>> "just work".
>>>>>>
>>>>>>
>>>>>> I forgot to add: FIT image authenticated boot flow. That is really what
>>>>>> ti_mmc distroboot method was trying to solve.
>>>>>>
>>>>>> Maybe Simon or someone know how the stdboot flow handles authenticated
>>>>>> kernel image and dtb boot flow with FIT image?
>>>>>
>>>>> Yes you can use FIT configuration verification and things should work as normal.
>>>>>
>>>>
>>>> Could you give any reference documentation for this?
>>>
>>> I suspect you should start with doc/usage/fit/beaglebone_vboot.rst
>>>
>>
>>   From that doc:
>>
>> ```
>> Boot the board using the commands below::
>>
>>       setenv bootargs console=ttyO0,115200n8 quiet root=/dev/mmcblk0p2 ro rootfstype=ext4 rootwait
>>       ext2load mmc 0:2 82000000 /boot/image.fit
>>       bootm 82000000
>> ```
>>
>> This is very much not stdboot :( This doc has some good and current info on building
>> a secure FIT image, but much of it is showing its age. Stdboot is still missing
>>
>>    * ability to limit boot mode search to only one image (FIT)
> 
> What does this mean? Can you please be more specific or give an example?
> 

Sure, for instance with secure boot we only want to search for FIT images
and if for each media this fails we do not want to fall back to other
image types or boot modes (like UART boot or net boot which would bypass
the signature checks).

Something similar is needed for searching for EFI compatible boot across
each enabled boot media first, before trying other methods on each media.
Basically breadth-first search on each media type not depth-first.

>>    * fetching and applying FIT overlay/config strings
> 
> Re overlays, is this the 'extension' command? What are config strings?
> 

When we have DT overlays in our FIT image, we build a string to pass to
bootm to apply all the selected overlays[0]. If there is now another
mechanism for this please let me know (building this string today is messy).

https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L18

>>    * remote proc loading
> 
> What command can do this?
> 

https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L28

>>    * uEnv.txt processing
> 
> What command can do this?
> 

run envboot;

https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/mmc.env?ref_type=heads#L20

>>
>> Last two can be solved by adding to CONFIG_BOOTCOMMAND before calling `bootflow scan`
>> as suggested above. Until we figure out the first two I don't see us ready to switch.
> 
> I'm happy to help figure this out.
> 

Thanks, we are eager to standardize our boot flows and stdboot seems like the
right way forward, very happy to have your guidance on these last few items :)

Andrew

> Regards,
> SImon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Manorit Chawdhry]

Hi Simon,

On 12:05-20231106, Andrew Davis wrote:
> On 11/6/23 11:47 AM, Simon Glass wrote:
> > Hi Andrew,
> > 
> > On Mon, 6 Nov 2023 at 10:27, Andrew Davis <afd@ti.com> wrote:
> > > 
> > > On 11/6/23 9:31 AM, Tom Rini wrote:
> > > > On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
> > > > > Hi Simon,
> > > > > 
> > > > > On 11:22-20231005, Simon Glass wrote:
> > > > > > Hi Nishanth,
> > > > > > 
> > > > > > On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
> > > > > > > 
> > > > > > > On 12:10-20231005, Nishanth Menon wrote:
> > > > > > > > On 12:36-20231005, Tom Rini wrote:
> > > > > > > > > On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> > > > > > > > > > On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > > > > > > > > > > On 08:48-20231004, Andrew Davis wrote:
> > > > > > > > > > > > On 10/4/23 8:23 AM, Roger Quadros wrote:
> > > > > > > > > > > > > ti_mmc is not a valid boot_target for standard boot flow so
> > > > > > > > > > > > 
> > > > > > > > > > > > Is there some way to make it into a valid boot_target? Otherwise
> > > > > > > > > > > > how do we use uEnv.txt files, or boot from FIT images with overlays?
> > > > > > > > > > > 
> > > > > > > > > > > envboot takes care of uEnv.txt file (see
> > > > > > > > > > > https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > > > > > > > > > > 
> > > > > > > > > > > Early remote proc loading and FIT image is a question for stdboot itself.
> > > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > If stdboot is missing these features then we shouldn't switch until it
> > > > > > > > > > has them. I'm all for switching to this, but only if it is complete.
> > > > > > > > > 
> > > > > > > > > Depends on what you mean?  Did you mean an option to run scripts
> > > > > > > > > (exists) or an option to do what TI needs done, via
> > > > > > > > > boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> > > > > > > > > figure out what that should be and do (but plumbing-wise everything it
> > > > > > > > > needs should exist).
> > > > > > > > 
> > > > > > > > Andrew is generalizing here (on the wrong patch though).
> > > > > > > > 
> > > > > > > > On am62x platforms, there is nothing regressing with this series. The
> > > > > > > > challenge is early remote_proc loading which is done for J7* platforms.
> > > > > > > > 
> > > > > > > > How that is initiated as part of bootmethods is something of a gap.
> > > > > > > > 
> > > > > > > > The other gap has been support for uEnv.txt -> which we can workaround
> > > > > > > > at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> > > > > > > > -lb" in defconfig (This series from Roger already does that - hence I am
> > > > > > > > saying that Andrew is complaining on the wrong series).
> > > > > > > > 
> > > > > > > > Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
> > > > > > > > uEnv.txt remoteproc loads and the various standard bootmethods should
> > > > > > > > "just work".
> > > > > > > 
> > > > > > > 
> > > > > > > I forgot to add: FIT image authenticated boot flow. That is really what
> > > > > > > ti_mmc distroboot method was trying to solve.
> > > > > > > 
> > > > > > > Maybe Simon or someone know how the stdboot flow handles authenticated
> > > > > > > kernel image and dtb boot flow with FIT image?
> > > > > > 
> > > > > > Yes you can use FIT configuration verification and things should work as normal.
> > > > > > 
> > > > > 
> > > > > Could you give any reference documentation for this?
> > > > 
> > > > I suspect you should start with doc/usage/fit/beaglebone_vboot.rst
> > > > 
> > > 
> > >   From that doc:
> > > 
> > > ```
> > > Boot the board using the commands below::
> > > 
> > >       setenv bootargs console=ttyO0,115200n8 quiet root=/dev/mmcblk0p2 ro rootfstype=ext4 rootwait
> > >       ext2load mmc 0:2 82000000 /boot/image.fit
> > >       bootm 82000000
> > > ```
> > > 
> > > This is very much not stdboot :( This doc has some good and current info on building
> > > a secure FIT image, but much of it is showing its age. Stdboot is still missing
> > > 
> > >    * ability to limit boot mode search to only one image (FIT)
> > 
> > What does this mean? Can you please be more specific or give an example?
> > 
> 
> Sure, for instance with secure boot we only want to search for FIT images
> and if for each media this fails we do not want to fall back to other
> image types or boot modes (like UART boot or net boot which would bypass
> the signature checks).
> 
> Something similar is needed for searching for EFI compatible boot across
> each enabled boot media first, before trying other methods on each media.
> Basically breadth-first search on each media type not depth-first.
> 
> > >    * fetching and applying FIT overlay/config strings
> > 
> > Re overlays, is this the 'extension' command? What are config strings?
> > 
> 
> When we have DT overlays in our FIT image, we build a string to pass to
> bootm to apply all the selected overlays[0]. If there is now another
> mechanism for this please let me know (building this string today is messy).
> 
> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L18
> 
> > >    * remote proc loading
> > 
> > What command can do this?
> > 
> 
> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L28
> 
> > >    * uEnv.txt processing
> > 
> > What command can do this?
> > 
> 
> run envboot;
> 
> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/mmc.env?ref_type=heads#L20
> 
> > > 
> > > Last two can be solved by adding to CONFIG_BOOTCOMMAND before calling `bootflow scan`
> > > as suggested above. Until we figure out the first two I don't see us ready to switch.
> > 
> > I'm happy to help figure this out.
> > 
> 
> Thanks, we are eager to standardize our boot flows and stdboot seems like the
> right way forward, very happy to have your guidance on these last few items :)
> 
> Andrew

Any nudges on the Andrew's response as to where we can look for all
this or how can we maybe move towards all this?

Regards,
Manorit

> 
> > Regards,
> > SImon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Simon Glass]

Hi Manorit,

On Tue, 28 Nov 2023 at 02:32, Manorit Chawdhry <m-chawdhry@ti.com> wrote:
>
> Hi Simon,
>
> On 12:05-20231106, Andrew Davis wrote:
> > On 11/6/23 11:47 AM, Simon Glass wrote:
> > > Hi Andrew,
> > >
> > > On Mon, 6 Nov 2023 at 10:27, Andrew Davis <afd@ti.com> wrote:
> > > >
> > > > On 11/6/23 9:31 AM, Tom Rini wrote:
> > > > > On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
> > > > > > Hi Simon,
> > > > > >
> > > > > > On 11:22-20231005, Simon Glass wrote:
> > > > > > > Hi Nishanth,
> > > > > > >
> > > > > > > On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
> > > > > > > >
> > > > > > > > On 12:10-20231005, Nishanth Menon wrote:
> > > > > > > > > On 12:36-20231005, Tom Rini wrote:
> > > > > > > > > > On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> > > > > > > > > > > On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > > > > > > > > > > > On 08:48-20231004, Andrew Davis wrote:
> > > > > > > > > > > > > On 10/4/23 8:23 AM, Roger Quadros wrote:
> > > > > > > > > > > > > > ti_mmc is not a valid boot_target for standard boot flow so
> > > > > > > > > > > > >
> > > > > > > > > > > > > Is there some way to make it into a valid boot_target? Otherwise
> > > > > > > > > > > > > how do we use uEnv.txt files, or boot from FIT images with overlays?
> > > > > > > > > > > >
> > > > > > > > > > > > envboot takes care of uEnv.txt file (see
> > > > > > > > > > > > https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > > > > > > > > > > >
> > > > > > > > > > > > Early remote proc loading and FIT image is a question for stdboot itself.
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > If stdboot is missing these features then we shouldn't switch until it
> > > > > > > > > > > has them. I'm all for switching to this, but only if it is complete.
> > > > > > > > > >
> > > > > > > > > > Depends on what you mean?  Did you mean an option to run scripts
> > > > > > > > > > (exists) or an option to do what TI needs done, via
> > > > > > > > > > boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> > > > > > > > > > figure out what that should be and do (but plumbing-wise everything it
> > > > > > > > > > needs should exist).
> > > > > > > > >
> > > > > > > > > Andrew is generalizing here (on the wrong patch though).
> > > > > > > > >
> > > > > > > > > On am62x platforms, there is nothing regressing with this series. The
> > > > > > > > > challenge is early remote_proc loading which is done for J7* platforms.
> > > > > > > > >
> > > > > > > > > How that is initiated as part of bootmethods is something of a gap.
> > > > > > > > >
> > > > > > > > > The other gap has been support for uEnv.txt -> which we can workaround
> > > > > > > > > at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> > > > > > > > > -lb" in defconfig (This series from Roger already does that - hence I am
> > > > > > > > > saying that Andrew is complaining on the wrong series).
> > > > > > > > >
> > > > > > > > > Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
> > > > > > > > > uEnv.txt remoteproc loads and the various standard bootmethods should
> > > > > > > > > "just work".
> > > > > > > >
> > > > > > > >
> > > > > > > > I forgot to add: FIT image authenticated boot flow. That is really what
> > > > > > > > ti_mmc distroboot method was trying to solve.
> > > > > > > >
> > > > > > > > Maybe Simon or someone know how the stdboot flow handles authenticated
> > > > > > > > kernel image and dtb boot flow with FIT image?
> > > > > > >
> > > > > > > Yes you can use FIT configuration verification and things should work as normal.
> > > > > > >
> > > > > >
> > > > > > Could you give any reference documentation for this?
> > > > >
> > > > > I suspect you should start with doc/usage/fit/beaglebone_vboot.rst
> > > > >
> > > >
> > > >   From that doc:
> > > >
> > > > ```
> > > > Boot the board using the commands below::
> > > >
> > > >       setenv bootargs console=ttyO0,115200n8 quiet root=/dev/mmcblk0p2 ro rootfstype=ext4 rootwait
> > > >       ext2load mmc 0:2 82000000 /boot/image.fit
> > > >       bootm 82000000
> > > > ```
> > > >
> > > > This is very much not stdboot :( This doc has some good and current info on building
> > > > a secure FIT image, but much of it is showing its age. Stdboot is still missing
> > > >
> > > >    * ability to limit boot mode search to only one image (FIT)
> > >
> > > What does this mean? Can you please be more specific or give an example?
> > >
> >
> > Sure, for instance with secure boot we only want to search for FIT images
> > and if for each media this fails we do not want to fall back to other
> > image types or boot modes (like UART boot or net boot which would bypass
> > the signature checks).
> >
> > Something similar is needed for searching for EFI compatible boot across
> > each enabled boot media first, before trying other methods on each media.
> > Basically breadth-first search on each media type not depth-first.
> >
> > > >    * fetching and applying FIT overlay/config strings
> > >
> > > Re overlays, is this the 'extension' command? What are config strings?
> > >
> >
> > When we have DT overlays in our FIT image, we build a string to pass to
> > bootm to apply all the selected overlays[0]. If there is now another
> > mechanism for this please let me know (building this string today is messy).
> >
> > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L18
> >
> > > >    * remote proc loading
> > >
> > > What command can do this?
> > >
> >
> > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L28
> >
> > > >    * uEnv.txt processing
> > >
> > > What command can do this?
> > >
> >
> > run envboot;
> >
> > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/mmc.env?ref_type=heads#L20
> >
> > > >
> > > > Last two can be solved by adding to CONFIG_BOOTCOMMAND before calling `bootflow scan`
> > > > as suggested above. Until we figure out the first two I don't see us ready to switch.
> > >
> > > I'm happy to help figure this out.
> > >
> >
> > Thanks, we are eager to standardize our boot flows and stdboot seems like the
> > right way forward, very happy to have your guidance on these last few items :)
> >
> > Andrew
>
> Any nudges on the Andrew's response as to where we can look for all
> this or how can we maybe move towards all this?

Yes I have taken a look at a possible design...I will reply soon.

Regards,
Simon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Simon Glass]

Hi Andrew,

On Mon, 6 Nov 2023 at 11:05, Andrew Davis <afd@ti.com> wrote:
>
> On 11/6/23 11:47 AM, Simon Glass wrote:
> > Hi Andrew,
> >
> > On Mon, 6 Nov 2023 at 10:27, Andrew Davis <afd@ti.com> wrote:
> >>
> >> On 11/6/23 9:31 AM, Tom Rini wrote:
> >>> On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
> >>>> Hi Simon,
> >>>>
> >>>> On 11:22-20231005, Simon Glass wrote:
> >>>>> Hi Nishanth,
> >>>>>
> >>>>> On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
> >>>>>>
> >>>>>> On 12:10-20231005, Nishanth Menon wrote:
> >>>>>>> On 12:36-20231005, Tom Rini wrote:
> >>>>>>>> On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> >>>>>>>>> On 10/4/23 8:54 AM, Nishanth Menon wrote:
> >>>>>>>>>> On 08:48-20231004, Andrew Davis wrote:
> >>>>>>>>>>> On 10/4/23 8:23 AM, Roger Quadros wrote:
> >>>>>>>>>>>> ti_mmc is not a valid boot_target for standard boot flow so
> >>>>>>>>>>>
> >>>>>>>>>>> Is there some way to make it into a valid boot_target? Otherwise
> >>>>>>>>>>> how do we use uEnv.txt files, or boot from FIT images with overlays?
> >>>>>>>>>>
> >>>>>>>>>> envboot takes care of uEnv.txt file (see
> >>>>>>>>>> https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> >>>>>>>>>>
> >>>>>>>>>> Early remote proc loading and FIT image is a question for stdboot itself.
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> If stdboot is missing these features then we shouldn't switch until it
> >>>>>>>>> has them. I'm all for switching to this, but only if it is complete.
> >>>>>>>>
> >>>>>>>> Depends on what you mean?  Did you mean an option to run scripts
> >>>>>>>> (exists) or an option to do what TI needs done, via
> >>>>>>>> boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> >>>>>>>> figure out what that should be and do (but plumbing-wise everything it
> >>>>>>>> needs should exist).
> >>>>>>>
> >>>>>>> Andrew is generalizing here (on the wrong patch though).
> >>>>>>>
> >>>>>>> On am62x platforms, there is nothing regressing with this series. The
> >>>>>>> challenge is early remote_proc loading which is done for J7* platforms.
> >>>>>>>
> >>>>>>> How that is initiated as part of bootmethods is something of a gap.
> >>>>>>>
> >>>>>>> The other gap has been support for uEnv.txt -> which we can workaround
> >>>>>>> at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> >>>>>>> -lb" in defconfig (This series from Roger already does that - hence I am
> >>>>>>> saying that Andrew is complaining on the wrong series).
> >>>>>>>
> >>>>>>> Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
> >>>>>>> uEnv.txt remoteproc loads and the various standard bootmethods should
> >>>>>>> "just work".
> >>>>>>
> >>>>>>
> >>>>>> I forgot to add: FIT image authenticated boot flow. That is really what
> >>>>>> ti_mmc distroboot method was trying to solve.
> >>>>>>
> >>>>>> Maybe Simon or someone know how the stdboot flow handles authenticated
> >>>>>> kernel image and dtb boot flow with FIT image?
> >>>>>
> >>>>> Yes you can use FIT configuration verification and things should work as normal.
> >>>>>
> >>>>
> >>>> Could you give any reference documentation for this?
> >>>
> >>> I suspect you should start with doc/usage/fit/beaglebone_vboot.rst
> >>>
> >>
> >>   From that doc:
> >>
> >> ```
> >> Boot the board using the commands below::
> >>
> >>       setenv bootargs console=ttyO0,115200n8 quiet root=/dev/mmcblk0p2 ro rootfstype=ext4 rootwait
> >>       ext2load mmc 0:2 82000000 /boot/image.fit
> >>       bootm 82000000
> >> ```
> >>
> >> This is very much not stdboot :( This doc has some good and current info on building
> >> a secure FIT image, but much of it is showing its age. Stdboot is still missing
> >>
> >>    * ability to limit boot mode search to only one image (FIT)
> >
> > What does this mean? Can you please be more specific or give an example?
> >
>
> Sure, for instance with secure boot we only want to search for FIT images
> and if for each media this fails we do not want to fall back to other
> image types or boot modes (like UART boot or net boot which would bypass
> the signature checks).

We could have something like:

bootstd {
   image-types = "fit", "uimage";
}

to limit the supported types.

>
> Something similar is needed for searching for EFI compatible boot across
> each enabled boot media first, before trying other methods on each media.
> Basically breadth-first search on each media type not depth-first.

How about:

bootstd {
   scan-order = "bootmeth,bootdev";   // instead of default "bootdev,bootmeth";
};


>
> >>    * fetching and applying FIT overlay/config strings
> >
> > Re overlays, is this the 'extension' command? What are config strings?
> >
>
> When we have DT overlays in our FIT image, we build a string to pass to
> bootm to apply all the selected overlays[0]. If there is now another
> mechanism for this please let me know (building this string today is messy).
>
> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L18

My proposal for this is to implement extensions in FIT. This would
have a new extensions nodes, so you can specify what extensions are
available for each FIT configuration.

configurations {
   conf-1 {
      compatible = ...
      extensions = "ext1", "ext-2";
   };

extensions {
   ext-1 {
      fdto = "fdt-1";   // fdt overlay for this 'cape'
      compatible = "vendor,combined-device1";
      extensions = "ext3";
   };

   ext-2 {
      fdto = "fdto-2";   // fdt overlay for this 'cape'
      compatible = "vendor,device2";
   };

   ext-3 {
      fdto = "fdto-3";
      compatible = "vendor,device3";
   };

So FIT configurations now have a list of supported extensions. The
extensions are hierarchical so that you can have ext1 which can
optionally have ext-2 as well. This allows boards which share a lot of
'capes' to be specified only ones.

Extensions actually present are declared by a sysinfo driver for the
board, with new methods:

get_compat() - determine the compatible strings for the current platform
get_ext() - get a list of compatible strings for extensions which are
actually present

The extension compatible strings are used to select the correct things
from the FIT, apply the overlays and produce the final DT.

>
> >>    * remote proc loading
> >
> > What command can do this?
> >
>
> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L28

This could be a new property in bootstd:

bootstd: {
   probe-before = <&phandle of remoteproc>, ...
};

>
> >>    * uEnv.txt processing
> >
> > What command can do this?
> >
>
> run envboot;
>
> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/mmc.env?ref_type=heads#L20

This could be a new bootmeth which looks for uenv.txt on available
devices. It might be better to put the env in a FIT or something with
a checksum.

>
> >>
> >> Last two can be solved by adding to CONFIG_BOOTCOMMAND before calling `bootflow scan`
> >> as suggested above. Until we figure out the first two I don't see us ready to switch.
> >
> > I'm happy to help figure this out.
> >
>
> Thanks, we are eager to standardize our boot flows and stdboot seems like the
> right way forward, very happy to have your guidance on these last few items :)

Regards,
Simon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Simon Glass]

Hi,

On Wed, Nov 29, 2023 at 7:48 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Andrew,
>
> On Mon, 6 Nov 2023 at 11:05, Andrew Davis <afd@ti.com> wrote:
> >
> > On 11/6/23 11:47 AM, Simon Glass wrote:
> > > Hi Andrew,
> > >
> > > On Mon, 6 Nov 2023 at 10:27, Andrew Davis <afd@ti.com> wrote:
> > >>
> > >> On 11/6/23 9:31 AM, Tom Rini wrote:
> > >>> On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
> > >>>> Hi Simon,
> > >>>>
> > >>>> On 11:22-20231005, Simon Glass wrote:
> > >>>>> Hi Nishanth,
> > >>>>>
> > >>>>> On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
> > >>>>>>
> > >>>>>> On 12:10-20231005, Nishanth Menon wrote:
> > >>>>>>> On 12:36-20231005, Tom Rini wrote:
> > >>>>>>>> On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> > >>>>>>>>> On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > >>>>>>>>>> On 08:48-20231004, Andrew Davis wrote:
> > >>>>>>>>>>> On 10/4/23 8:23 AM, Roger Quadros wrote:
> > >>>>>>>>>>>> ti_mmc is not a valid boot_target for standard boot flow so
> > >>>>>>>>>>>
> > >>>>>>>>>>> Is there some way to make it into a valid boot_target? Otherwise
> > >>>>>>>>>>> how do we use uEnv.txt files, or boot from FIT images with overlays?
> > >>>>>>>>>>
> > >>>>>>>>>> envboot takes care of uEnv.txt file (see
> > >>>>>>>>>> https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > >>>>>>>>>>
> > >>>>>>>>>> Early remote proc loading and FIT image is a question for stdboot itself.
> > >>>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>> If stdboot is missing these features then we shouldn't switch until it
> > >>>>>>>>> has them. I'm all for switching to this, but only if it is complete.
> > >>>>>>>>
> > >>>>>>>> Depends on what you mean?  Did you mean an option to run scripts
> > >>>>>>>> (exists) or an option to do what TI needs done, via
> > >>>>>>>> boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> > >>>>>>>> figure out what that should be and do (but plumbing-wise everything it
> > >>>>>>>> needs should exist).
> > >>>>>>>
> > >>>>>>> Andrew is generalizing here (on the wrong patch though).
> > >>>>>>>
> > >>>>>>> On am62x platforms, there is nothing regressing with this series. The
> > >>>>>>> challenge is early remote_proc loading which is done for J7* platforms.
> > >>>>>>>
> > >>>>>>> How that is initiated as part of bootmethods is something of a gap.
> > >>>>>>>
> > >>>>>>> The other gap has been support for uEnv.txt -> which we can workaround
> > >>>>>>> at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> > >>>>>>> -lb" in defconfig (This series from Roger already does that - hence I am
> > >>>>>>> saying that Andrew is complaining on the wrong series).
> > >>>>>>>
> > >>>>>>> Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
> > >>>>>>> uEnv.txt remoteproc loads and the various standard bootmethods should
> > >>>>>>> "just work".
> > >>>>>>
> > >>>>>>
> > >>>>>> I forgot to add: FIT image authenticated boot flow. That is really what
> > >>>>>> ti_mmc distroboot method was trying to solve.
> > >>>>>>
> > >>>>>> Maybe Simon or someone know how the stdboot flow handles authenticated
> > >>>>>> kernel image and dtb boot flow with FIT image?
> > >>>>>
> > >>>>> Yes you can use FIT configuration verification and things should work as normal.
> > >>>>>
> > >>>>
> > >>>> Could you give any reference documentation for this?
> > >>>
> > >>> I suspect you should start with doc/usage/fit/beaglebone_vboot.rst
> > >>>
> > >>
> > >>   From that doc:
> > >>
> > >> ```
> > >> Boot the board using the commands below::
> > >>
> > >>       setenv bootargs console=ttyO0,115200n8 quiet root=/dev/mmcblk0p2 ro rootfstype=ext4 rootwait
> > >>       ext2load mmc 0:2 82000000 /boot/image.fit
> > >>       bootm 82000000
> > >> ```
> > >>
> > >> This is very much not stdboot :( This doc has some good and current info on building
> > >> a secure FIT image, but much of it is showing its age. Stdboot is still missing
> > >>
> > >>    * ability to limit boot mode search to only one image (FIT)
> > >
> > > What does this mean? Can you please be more specific or give an example?
> > >
> >
> > Sure, for instance with secure boot we only want to search for FIT images
> > and if for each media this fails we do not want to fall back to other
> > image types or boot modes (like UART boot or net boot which would bypass
> > the signature checks).
>
> We could have something like:
>
> bootstd {
>    image-types = "fit", "uimage";
> }
>
> to limit the supported types.
>
> >
> > Something similar is needed for searching for EFI compatible boot across
> > each enabled boot media first, before trying other methods on each media.
> > Basically breadth-first search on each media type not depth-first.
>
> How about:
>
> bootstd {
>    scan-order = "bootmeth,bootdev";   // instead of default "bootdev,bootmeth";
> };
>
>
> >
> > >>    * fetching and applying FIT overlay/config strings
> > >
> > > Re overlays, is this the 'extension' command? What are config strings?
> > >
> >
> > When we have DT overlays in our FIT image, we build a string to pass to
> > bootm to apply all the selected overlays[0]. If there is now another
> > mechanism for this please let me know (building this string today is messy).
> >
> > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L18
>
> My proposal for this is to implement extensions in FIT. This would
> have a new extensions nodes, so you can specify what extensions are
> available for each FIT configuration.
>
> configurations {
>    conf-1 {
>       compatible = ...
>       extensions = "ext1", "ext-2";
>    };
>
> extensions {
>    ext-1 {
>       fdto = "fdt-1";   // fdt overlay for this 'cape'
>       compatible = "vendor,combined-device1";
>       extensions = "ext3";
>    };
>
>    ext-2 {
>       fdto = "fdto-2";   // fdt overlay for this 'cape'
>       compatible = "vendor,device2";
>    };
>
>    ext-3 {
>       fdto = "fdto-3";
>       compatible = "vendor,device3";
>    };
>
> So FIT configurations now have a list of supported extensions. The
> extensions are hierarchical so that you can have ext1 which can
> optionally have ext-2 as well. This allows boards which share a lot of
> 'capes' to be specified only ones.
>
> Extensions actually present are declared by a sysinfo driver for the
> board, with new methods:
>
> get_compat() - determine the compatible strings for the current platform
> get_ext() - get a list of compatible strings for extensions which are
> actually present
>
> The extension compatible strings are used to select the correct things
> from the FIT, apply the overlays and produce the final DT.
>
> >
> > >>    * remote proc loading
> > >
> > > What command can do this?
> > >
> >
> > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L28
>
> This could be a new property in bootstd:
>
> bootstd: {
>    probe-before = <&phandle of remoteproc>, ...
> };
>
> >
> > >>    * uEnv.txt processing
> > >
> > > What command can do this?
> > >
> >
> > run envboot;
> >
> > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/mmc.env?ref_type=heads#L20
>
> This could be a new bootmeth which looks for uenv.txt on available
> devices. It might be better to put the env in a FIT or something with
> a checksum.
>
> >
> > >>
> > >> Last two can be solved by adding to CONFIG_BOOTCOMMAND before calling `bootflow scan`
> > >> as suggested above. Until we figure out the first two I don't see us ready to switch.
> > >
> > > I'm happy to help figure this out.
> > >
> >
> > Thanks, we are eager to standardize our boot flows and stdboot seems like the
> > right way forward, very happy to have your guidance on these last few items :)
>

I'm just checking whether there is any feedback on the above? I sent
the FIT part as a separate proposal.

Regards,
Simon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Andrew Davis]

On 12/31/23 6:48 AM, Simon Glass wrote:
> Hi,
> 
> On Wed, Nov 29, 2023 at 7:48 PM Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Andrew,
>>
>> On Mon, 6 Nov 2023 at 11:05, Andrew Davis <afd@ti.com> wrote:
>>>
>>> On 11/6/23 11:47 AM, Simon Glass wrote:
>>>> Hi Andrew,
>>>>
>>>> On Mon, 6 Nov 2023 at 10:27, Andrew Davis <afd@ti.com> wrote:
>>>>>
>>>>> On 11/6/23 9:31 AM, Tom Rini wrote:
>>>>>> On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
>>>>>>> Hi Simon,
>>>>>>>
>>>>>>> On 11:22-20231005, Simon Glass wrote:
>>>>>>>> Hi Nishanth,
>>>>>>>>
>>>>>>>> On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
>>>>>>>>>
>>>>>>>>> On 12:10-20231005, Nishanth Menon wrote:
>>>>>>>>>> On 12:36-20231005, Tom Rini wrote:
>>>>>>>>>>> On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
>>>>>>>>>>>> On 10/4/23 8:54 AM, Nishanth Menon wrote:
>>>>>>>>>>>>> On 08:48-20231004, Andrew Davis wrote:
>>>>>>>>>>>>>> On 10/4/23 8:23 AM, Roger Quadros wrote:
>>>>>>>>>>>>>>> ti_mmc is not a valid boot_target for standard boot flow so
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Is there some way to make it into a valid boot_target? Otherwise
>>>>>>>>>>>>>> how do we use uEnv.txt files, or boot from FIT images with overlays?
>>>>>>>>>>>>>
>>>>>>>>>>>>> envboot takes care of uEnv.txt file (see
>>>>>>>>>>>>> https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
>>>>>>>>>>>>>
>>>>>>>>>>>>> Early remote proc loading and FIT image is a question for stdboot itself.
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> If stdboot is missing these features then we shouldn't switch until it
>>>>>>>>>>>> has them. I'm all for switching to this, but only if it is complete.
>>>>>>>>>>>
>>>>>>>>>>> Depends on what you mean?  Did you mean an option to run scripts
>>>>>>>>>>> (exists) or an option to do what TI needs done, via
>>>>>>>>>>> boot/bootmeth_something.c ?  If the latter, someone from TI needs to
>>>>>>>>>>> figure out what that should be and do (but plumbing-wise everything it
>>>>>>>>>>> needs should exist).
>>>>>>>>>>
>>>>>>>>>> Andrew is generalizing here (on the wrong patch though).
>>>>>>>>>>
>>>>>>>>>> On am62x platforms, there is nothing regressing with this series. The
>>>>>>>>>> challenge is early remote_proc loading which is done for J7* platforms.
>>>>>>>>>>
>>>>>>>>>> How that is initiated as part of bootmethods is something of a gap.
>>>>>>>>>>
>>>>>>>>>> The other gap has been support for uEnv.txt -> which we can workaround
>>>>>>>>>> at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
>>>>>>>>>> -lb" in defconfig (This series from Roger already does that - hence I am
>>>>>>>>>> saying that Andrew is complaining on the wrong series).
>>>>>>>>>>
>>>>>>>>>> Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan-lb" and
>>>>>>>>>> uEnv.txt remoteproc loads and the various standard bootmethods should
>>>>>>>>>> "just work".
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I forgot to add: FIT image authenticated boot flow. That is really what
>>>>>>>>> ti_mmc distroboot method was trying to solve.
>>>>>>>>>
>>>>>>>>> Maybe Simon or someone know how the stdboot flow handles authenticated
>>>>>>>>> kernel image and dtb boot flow with FIT image?
>>>>>>>>
>>>>>>>> Yes you can use FIT configuration verification and things should work as normal.
>>>>>>>>
>>>>>>>
>>>>>>> Could you give any reference documentation for this?
>>>>>>
>>>>>> I suspect you should start with doc/usage/fit/beaglebone_vboot.rst
>>>>>>
>>>>>
>>>>>    From that doc:
>>>>>
>>>>> ```
>>>>> Boot the board using the commands below::
>>>>>
>>>>>        setenv bootargs console=ttyO0,115200n8 quiet root=/dev/mmcblk0p2 ro rootfstype=ext4 rootwait
>>>>>        ext2load mmc 0:2 82000000 /boot/image.fit
>>>>>        bootm 82000000
>>>>> ```
>>>>>
>>>>> This is very much not stdboot :( This doc has some good and current info on building
>>>>> a secure FIT image, but much of it is showing its age. Stdboot is still missing
>>>>>
>>>>>     * ability to limit boot mode search to only one image (FIT)
>>>>
>>>> What does this mean? Can you please be more specific or give an example?
>>>>
>>>
>>> Sure, for instance with secure boot we only want to search for FIT images
>>> and if for each media this fails we do not want to fall back to other
>>> image types or boot modes (like UART boot or net boot which would bypass
>>> the signature checks).
>>
>> We could have something like:
>>
>> bootstd {
>>     image-types = "fit", "uimage";
>> }
>>
>> to limit the supported types.
>>
>>>
>>> Something similar is needed for searching for EFI compatible boot across
>>> each enabled boot media first, before trying other methods on each media.
>>> Basically breadth-first search on each media type not depth-first.
>>
>> How about:
>>
>> bootstd {
>>     scan-order = "bootmeth,bootdev";   // instead of default "bootdev,bootmeth";
>> };
>>

This along with the image-types above seem reasonable. Might also help with
our EFI search order issue (we need to search all boot media for EFI parts
first before trying all the other types of boot on each media, basically
breadth-first search vs depth for EFI). We should be able to describe that
pattern here if we think this through.

>>
>>>
>>>>>     * fetching and applying FIT overlay/config strings
>>>>
>>>> Re overlays, is this the 'extension' command? What are config strings?
>>>>
>>>
>>> When we have DT overlays in our FIT image, we build a string to pass to
>>> bootm to apply all the selected overlays[0]. If there is now another
>>> mechanism for this please let me know (building this string today is messy).
>>>
>>> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L18
>>
>> My proposal for this is to implement extensions in FIT. This would
>> have a new extensions nodes, so you can specify what extensions are
>> available for each FIT configuration.
>>
>> configurations {
>>     conf-1 {
>>        compatible = ...
>>        extensions = "ext1", "ext-2";
>>     };
>>
>> extensions {
>>     ext-1 {
>>        fdto = "fdt-1";   // fdt overlay for this 'cape'
>>        compatible = "vendor,combined-device1";
>>        extensions = "ext3";
>>     };
>>
>>     ext-2 {
>>        fdto = "fdto-2";   // fdt overlay for this 'cape'
>>        compatible = "vendor,device2";
>>     };
>>
>>     ext-3 {
>>        fdto = "fdto-3";
>>        compatible = "vendor,device3";
>>     };
>>
>> So FIT configurations now have a list of supported extensions. The
>> extensions are hierarchical so that you can have ext1 which can
>> optionally have ext-2 as well. This allows boards which share a lot of
>> 'capes' to be specified only ones.
>>
>> Extensions actually present are declared by a sysinfo driver for the
>> board, with new methods:
>>
>> get_compat() - determine the compatible strings for the current platform
>> get_ext() - get a list of compatible strings for extensions which are
>> actually present
>>
>> The extension compatible strings are used to select the correct things
>> from the FIT, apply the overlays and produce the final DT.
>>

If we have a list of compatible strings and extensions, then do we need
the extensions nodes? Couldn't each extension string simply be the name
of the overlay ftdo-x node to apply?

>>>
>>>>>     * remote proc loading
>>>>
>>>> What command can do this?
>>>>
>>>
>>> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L28
>>
>> This could be a new property in bootstd:
>>
>> bootstd: {
>>     probe-before = <&phandle of remoteproc>, ...
>> };
>>
>>>
>>>>>     * uEnv.txt processing
>>>>
>>>> What command can do this?
>>>>
>>>
>>> run envboot;
>>>
>>> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/mmc.env?ref_type=heads#L20
>>
>> This could be a new bootmeth which looks for uenv.txt on available
>> devices. It might be better to put the env in a FIT or something with
>> a checksum.
>>
>>>
>>>>>
>>>>> Last two can be solved by adding to CONFIG_BOOTCOMMAND before calling`bootflow scan`
>>>>> as suggested above. Until we figure out the first two I don't see us ready to switch.
>>>>
>>>> I'm happy to help figure this out.
>>>>
>>>
>>> Thanks, we are eager to standardize our boot flows and stdboot seems like the
>>> right way forward, very happy to have your guidance on these last few items :)
>>
> 
> I'm just checking whether there is any feedback on the above? I sent
> the FIT part as a separate proposal.
> 

Manorit,

Does the above all work for you?

Andrew

> Regards,
> Simon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Manorit Chawdhry]

Hi Andrew, Simon,

On 08:58-20240102, Andrew Davis wrote:
> On 12/31/23 6:48 AM, Simon Glass wrote:
> > Hi,
> > 
> > On Wed, Nov 29, 2023 at 7:48 PM Simon Glass <sjg@chromium.org> wrote:
> > > 
> > > Hi Andrew,
> > > 
> > > On Mon, 6 Nov 2023 at 11:05, Andrew Davis <afd@ti.com> wrote:
> > > > 
> > > > On 11/6/23 11:47 AM, Simon Glass wrote:
> > > > > Hi Andrew,
> > > > > 
> > > > > On Mon, 6 Nov 2023 at 10:27, Andrew Davis <afd@ti.com> wrote:
> > > > > > 
> > > > > > On 11/6/23 9:31 AM, Tom Rini wrote:
> > > > > > > On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
> > > > > > > > Hi Simon,
> > > > > > > > 
> > > > > > > > On 11:22-20231005, Simon Glass wrote:
> > > > > > > > > Hi Nishanth,
> > > > > > > > > 
> > > > > > > > > On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
> > > > > > > > > > 
> > > > > > > > > > On 12:10-20231005, Nishanth Menon wrote:
> > > > > > > > > > > On 12:36-20231005, Tom Rini wrote:
> > > > > > > > > > > > On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> > > > > > > > > > > > > On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > > > > > > > > > > > > > On 08:48-20231004, Andrew Davis wrote:
> > > > > > > > > > > > > > > On 10/4/23 8:23 AM, Roger Quadros wrote:
> > > > > > > > > > > > > > > > ti_mmc is not a valid boot_target for standard boot flow so
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Is there some way to make it into a valid boot_target? Otherwise
> > > > > > > > > > > > > > > how do we use uEnv.txt files, or boot from FIT images with overlays?
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > envboot takes care of uEnv.txt file (see
> > > > > > > > > > > > > > https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Early remote proc loading and FIT image is a question for stdboot itself.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > If stdboot is missing these features then we shouldn't switch until it
> > > > > > > > > > > > > has them. I'm all for switching to this, but only if it is complete.
> > > > > > > > > > > > 
> > > > > > > > > > > > Depends on what you mean?  Did you mean an option to run scripts
> > > > > > > > > > > > (exists) or an option to do what TI needs done, via
> > > > > > > > > > > > boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> > > > > > > > > > > > figure out what that should be and do (but plumbing-wise everything it
> > > > > > > > > > > > needs should exist).
> > > > > > > > > > > 
> > > > > > > > > > > Andrew is generalizing here (on the wrong patch though).
> > > > > > > > > > > 
> > > > > > > > > > > On am62x platforms, there is nothing regressing with this series. The
> > > > > > > > > > > challenge is early remote_proc loading which is done for J7* platforms.
> > > > > > > > > > > 
> > > > > > > > > > > How that is initiated as part of bootmethods is something of a gap.
> > > > > > > > > > > 
> > > > > > > > > > > The other gap has been support for uEnv.txt -> which we can workaround
> > > > > > > > > > > at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> > > > > > > > > > > -lb" in defconfig (This series from Roger already does that - hence I am
> > > > > > > > > > > saying that Andrew is complaining on the wrong series).
> > > > > > > > > > > 
> > > > > > > > > > > Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan-lb" and
> > > > > > > > > > > uEnv.txt remoteproc loads and the various standard bootmethods should
> > > > > > > > > > > "just work".
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > I forgot to add: FIT image authenticated boot flow. That is really what
> > > > > > > > > > ti_mmc distroboot method was trying to solve.
> > > > > > > > > > 
> > > > > > > > > > Maybe Simon or someone know how the stdboot flow handles authenticated
> > > > > > > > > > kernel image and dtb boot flow with FIT image?
> > > > > > > > > 
> > > > > > > > > Yes you can use FIT configuration verification and things should work as normal.
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > Could you give any reference documentation for this?
> > > > > > > 
> > > > > > > I suspect you should start with doc/usage/fit/beaglebone_vboot.rst
> > > > > > > 
> > > > > > 
> > > > > >    From that doc:
> > > > > > 
> > > > > > ```
> > > > > > Boot the board using the commands below::
> > > > > > 
> > > > > >        setenv bootargs console=ttyO0,115200n8 quiet root=/dev/mmcblk0p2 ro rootfstype=ext4 rootwait
> > > > > >        ext2load mmc 0:2 82000000 /boot/image.fit
> > > > > >        bootm 82000000
> > > > > > ```
> > > > > > 
> > > > > > This is very much not stdboot :( This doc has some good and current info on building
> > > > > > a secure FIT image, but much of it is showing its age. Stdboot is still missing
> > > > > > 
> > > > > >     * ability to limit boot mode search to only one image (FIT)
> > > > > 
> > > > > What does this mean? Can you please be more specific or give an example?
> > > > > 
> > > > 
> > > > Sure, for instance with secure boot we only want to search for FIT images
> > > > and if for each media this fails we do not want to fall back to other
> > > > image types or boot modes (like UART boot or net boot which would bypass
> > > > the signature checks).
> > > 
> > > We could have something like:
> > > 
> > > bootstd {
> > >     image-types = "fit", "uimage";
> > > }
> > > 
> > > to limit the supported types.
> > > 
> > > > 
> > > > Something similar is needed for searching for EFI compatible boot across
> > > > each enabled boot media first, before trying other methods on each media.
> > > > Basically breadth-first search on each media type not depth-first.
> > > 
> > > How about:
> > > 
> > > bootstd {
> > >     scan-order = "bootmeth,bootdev";   // instead of default "bootdev,bootmeth";
> > > };
> > > 
> 
> This along with the image-types above seem reasonable. Might also help with
> our EFI search order issue (we need to search all boot media for EFI parts
> first before trying all the other types of boot on each media, basically
> breadth-first search vs depth for EFI). We should be able to describe that
> pattern here if we think this through.
> 

This seems okay and will solve the problem of the FIT Image issues and
fallback that we were having with stdboot. 

> > > 
> > > > 
> > > > > >     * fetching and applying FIT overlay/config strings
> > > > > 
> > > > > Re overlays, is this the 'extension' command? What are config strings?
> > > > > 
> > > > 
> > > > When we have DT overlays in our FIT image, we build a string to pass to
> > > > bootm to apply all the selected overlays[0]. If there is now another
> > > > mechanism for this please let me know (building this string today is messy).
> > > > 
> > > > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L18
> > > 
> > > My proposal for this is to implement extensions in FIT. This would
> > > have a new extensions nodes, so you can specify what extensions are
> > > available for each FIT configuration.
> > > 
> > > configurations {
> > >     conf-1 {
> > >        compatible = ...
> > >        extensions = "ext1", "ext-2";
> > >     };
> > > 
> > > extensions {
> > >     ext-1 {
> > >        fdto = "fdt-1";   // fdt overlay for this 'cape'
> > >        compatible = "vendor,combined-device1";
> > >        extensions = "ext3";
> > >     };
> > > 
> > >     ext-2 {
> > >        fdto = "fdto-2";   // fdt overlay for this 'cape'
> > >        compatible = "vendor,device2";
> > >     };
> > > 
> > >     ext-3 {
> > >        fdto = "fdto-3";
> > >        compatible = "vendor,device3";
> > >     };
> > > 
> > > So FIT configurations now have a list of supported extensions. The
> > > extensions are hierarchical so that you can have ext1 which can
> > > optionally have ext-2 as well. This allows boards which share a lot of
> > > 'capes' to be specified only ones.
> > > 
> > > Extensions actually present are declared by a sysinfo driver for the
> > > board, with new methods:
> > > 
> > > get_compat() - determine the compatible strings for the current platform
> > > get_ext() - get a list of compatible strings for extensions which are
> > > actually present
> > > 
> > > The extension compatible strings are used to select the correct things
> > > from the FIT, apply the overlays and produce the final DT.
> > > 

This is something that is still concerning me, so previously we used to
have two methods of applying overlays, one was through FIT Image where
we have the overlays embedded and in another we used to use non FIT
boot where the overlays were present in the filesystem only and we used
to apply them, this might solve the problem with FIT Image booting overlays but
normal kernel boot and dtb with overlays is still something that I don't
know would be possible or not with this. Is there any way to do that?

> 
> If we have a list of compatible strings and extensions, then do we need
> the extensions nodes? Couldn't each extension string simply be the name
> of the overlay ftdo-x node to apply?
> 
> > > > 
> > > > > >     * remote proc loading
> > > > > 
> > > > > What command can do this?
> > > > > 
> > > > 
> > > > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L28
> > > 
> > > This could be a new property in bootstd:
> > > 
> > > bootstd: {
> > >     probe-before = <&phandle of remoteproc>, ...
> > > };

I think this can also work, the firmware names we can get from the dt
nodes and I think modifying the names to signed firmwares that we used
to do in the scripts can be handled in the driver itself. 

Andrew, do you see anything else in this that cannot work?

> > > 
> > > > 
> > > > > >     * uEnv.txt processing
> > > > > 
> > > > > What command can do this?
> > > > > 
> > > > 
> > > > run envboot;
> > > > 
> > > > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/mmc.env?ref_type=heads#L20
> > > 
> > > This could be a new bootmeth which looks for uenv.txt on available
> > > devices. It might be better to put the env in a FIT or something with
> > > a checksum.

We use non-FIT boot as well for our non HS platforms, I think the new
bootmeth might be helpful as you mentioned, would have to look at this
more as I thought bootmeth is supposed to boot a platform at the end
only but it seems like here we'll be combining this bootmeth ( that'll
just import the env ) and then other bootmeth will actually boot Linux.

Regards,
Manorit

> > > 
> > > > 
> > > > > > 
> > > > > > Last two can be solved by adding to CONFIG_BOOTCOMMAND before calling`bootflow scan`
> > > > > > as suggested above. Until we figure out the first two I don't see us ready to switch.
> > > > > 
> > > > > I'm happy to help figure this out.
> > > > > 
> > > > 
> > > > Thanks, we are eager to standardize our boot flows and stdboot seems like the
> > > > right way forward, very happy to have your guidance on these last few items :)
> > > 
> > 
> > I'm just checking whether there is any feedback on the above? I sent
> > the FIT part as a separate proposal.
> > 
> 
> Manorit,
> 
> Does the above all work for you?
> 
> Andrew
> 
> > Regards,
> > Simon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Andrew Davis]

On 1/4/24 1:52 AM, Manorit Chawdhry wrote:
> Hi Andrew, Simon,
> 
> On 08:58-20240102, Andrew Davis wrote:
>> On 12/31/23 6:48 AM, Simon Glass wrote:
>>> Hi,
>>>
>>> On Wed, Nov 29, 2023 at 7:48 PM Simon Glass <sjg@chromium.org> wrote:
>>>>
>>>> Hi Andrew,
>>>>
>>>> On Mon, 6 Nov 2023 at 11:05, Andrew Davis <afd@ti.com> wrote:
>>>>>
>>>>> On 11/6/23 11:47 AM, Simon Glass wrote:
>>>>>> Hi Andrew,
>>>>>>
>>>>>> On Mon, 6 Nov 2023 at 10:27, Andrew Davis <afd@ti.com> wrote:
>>>>>>>
>>>>>>> On 11/6/23 9:31 AM, Tom Rini wrote:
>>>>>>>> On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
>>>>>>>>> Hi Simon,
>>>>>>>>>
>>>>>>>>> On 11:22-20231005, Simon Glass wrote:
>>>>>>>>>> Hi Nishanth,
>>>>>>>>>>
>>>>>>>>>> On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On 12:10-20231005, Nishanth Menon wrote:
>>>>>>>>>>>> On 12:36-20231005, Tom Rini wrote:
>>>>>>>>>>>>> On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
>>>>>>>>>>>>>> On 10/4/23 8:54 AM, Nishanth Menon wrote:
>>>>>>>>>>>>>>> On 08:48-20231004, Andrew Davis wrote:
>>>>>>>>>>>>>>>> On 10/4/23 8:23 AM, Roger Quadros wrote:
>>>>>>>>>>>>>>>>> ti_mmc is not a valid boot_target for standard boot flow so
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Is there some way to make it into a valid boot_target? Otherwise
>>>>>>>>>>>>>>>> how do we use uEnv.txt files, or boot from FIT images with overlays?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> envboot takes care of uEnv.txt file (see
>>>>>>>>>>>>>>> https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Early remote proc loading and FIT image is a question for stdboot itself.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> If stdboot is missing these features then we shouldn't switch until it
>>>>>>>>>>>>>> has them. I'm all for switching to this, but only if it is complete.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Depends on what you mean?  Did you mean an option to run scripts
>>>>>>>>>>>>> (exists) or an option to do what TI needs done, via
>>>>>>>>>>>>> boot/bootmeth_something.c ?  If the latter, someone from TI needs to
>>>>>>>>>>>>> figure out what that should be and do (but plumbing-wise everything it
>>>>>>>>>>>>> needs should exist).
>>>>>>>>>>>>
>>>>>>>>>>>> Andrew is generalizing here (on the wrong patch though).
>>>>>>>>>>>>
>>>>>>>>>>>> On am62x platforms, there is nothing regressing with this series. The
>>>>>>>>>>>> challenge is early remote_proc loading which is done for J7* platforms.
>>>>>>>>>>>>
>>>>>>>>>>>> How that is initiated as part of bootmethods is something of a gap.
>>>>>>>>>>>>
>>>>>>>>>>>> The other gap has been support for uEnv.txt -> which we can workaround
>>>>>>>>>>>> at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
>>>>>>>>>>>> -lb" in defconfig (This series from Roger already does that - hence I am
>>>>>>>>>>>> saying that Andrew is complaining on the wrong series).
>>>>>>>>>>>>
>>>>>>>>>>>> Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan-lb" and
>>>>>>>>>>>> uEnv.txt remoteproc loads and the various standard bootmethods should
>>>>>>>>>>>> "just work".
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I forgot to add: FIT image authenticated boot flow. That is really what
>>>>>>>>>>> ti_mmc distroboot method was trying to solve.
>>>>>>>>>>>
>>>>>>>>>>> Maybe Simon or someone know how the stdboot flow handles authenticated
>>>>>>>>>>> kernel image and dtb boot flow with FIT image?
>>>>>>>>>>
>>>>>>>>>> Yes you can use FIT configuration verification and things should work as normal.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Could you give any reference documentation for this?
>>>>>>>>
>>>>>>>> I suspect you should start with doc/usage/fit/beaglebone_vboot.rst
>>>>>>>>
>>>>>>>
>>>>>>>     From that doc:
>>>>>>>
>>>>>>> ```
>>>>>>> Boot the board using the commands below::
>>>>>>>
>>>>>>>         setenv bootargs console=ttyO0,115200n8 quiet root=/dev/mmcblk0p2 ro rootfstype=ext4 rootwait
>>>>>>>         ext2load mmc 0:2 82000000 /boot/image.fit
>>>>>>>         bootm 82000000
>>>>>>> ```
>>>>>>>
>>>>>>> This is very much not stdboot :( This doc has some good and current info on building
>>>>>>> a secure FIT image, but much of it is showing its age. Stdboot is still missing
>>>>>>>
>>>>>>>      * ability to limit boot mode search to only one image (FIT)
>>>>>>
>>>>>> What does this mean? Can you please be more specific or give an example?
>>>>>>
>>>>>
>>>>> Sure, for instance with secure boot we only want to search for FIT images
>>>>> and if for each media this fails we do not want to fall back to other
>>>>> image types or boot modes (like UART boot or net boot which would bypass
>>>>> the signature checks).
>>>>
>>>> We could have something like:
>>>>
>>>> bootstd {
>>>>      image-types = "fit", "uimage";
>>>> }
>>>>
>>>> to limit the supported types.
>>>>
>>>>>
>>>>> Something similar is needed for searching for EFI compatible boot across
>>>>> each enabled boot media first, before trying other methods on each media.
>>>>> Basically breadth-first search on each media type not depth-first.
>>>>
>>>> How about:
>>>>
>>>> bootstd {
>>>>      scan-order = "bootmeth,bootdev";   // instead of default "bootdev,bootmeth";
>>>> };
>>>>
>>
>> This along with the image-types above seem reasonable. Might also help with
>> our EFI search order issue (we need to search all boot media for EFI parts
>> first before trying all the other types of boot on each media, basically
>> breadth-first search vs depth for EFI). We should be able to describe that
>> pattern here if we think this through.
>>
> 
> This seems okay and will solve the problem of the FIT Image issues and
> fallback that we were having with stdboot.
> 
>>>>
>>>>>
>>>>>>>      * fetching and applying FIT overlay/config strings
>>>>>>
>>>>>> Re overlays, is this the 'extension' command? What are config strings?
>>>>>>
>>>>>
>>>>> When we have DT overlays in our FIT image, we build a string to pass to
>>>>> bootm to apply all the selected overlays[0]. If there is now another
>>>>> mechanism for this please let me know (building this string today is messy).
>>>>>
>>>>> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L18
>>>>
>>>> My proposal for this is to implement extensions in FIT. This would
>>>> have a new extensions nodes, so you can specify what extensions are
>>>> available for each FIT configuration.
>>>>
>>>> configurations {
>>>>      conf-1 {
>>>>         compatible = ...
>>>>         extensions = "ext1", "ext-2";
>>>>      };
>>>>
>>>> extensions {
>>>>      ext-1 {
>>>>         fdto = "fdt-1";   // fdt overlay for this 'cape'
>>>>         compatible = "vendor,combined-device1";
>>>>         extensions = "ext3";
>>>>      };
>>>>
>>>>      ext-2 {
>>>>         fdto = "fdto-2";   // fdt overlay for this 'cape'
>>>>         compatible = "vendor,device2";
>>>>      };
>>>>
>>>>      ext-3 {
>>>>         fdto = "fdto-3";
>>>>         compatible = "vendor,device3";
>>>>      };
>>>>
>>>> So FIT configurations now have a list of supported extensions. The
>>>> extensions are hierarchical so that you can have ext1 which can
>>>> optionally have ext-2 as well. This allows boards which share a lot of
>>>> 'capes' to be specified only ones.
>>>>
>>>> Extensions actually present are declared by a sysinfo driver for the
>>>> board, with new methods:
>>>>
>>>> get_compat() - determine the compatible strings for the current platform
>>>> get_ext() - get a list of compatible strings for extensions which are
>>>> actually present
>>>>
>>>> The extension compatible strings are used to select the correct things
>>>> from the FIT, apply the overlays and produce the final DT.
>>>>
> 
> This is something that is still concerning me, so previously we used to
> have two methods of applying overlays, one was through FIT Image where
> we have the overlays embedded and in another we used to use non FIT
> boot where the overlays were present in the filesystem only and we used
> to apply them, this might solve the problem with FIT Image booting overlays but
> normal kernel boot and dtb with overlays is still something that I don't
> know would be possible or not with this. Is there any way to do that?
> 
>>
>> If we have a list of compatible strings and extensions, then do we need
>> the extensions nodes? Couldn't each extension string simply be the name
>> of the overlay ftdo-x node to apply?
>>
>>>>>
>>>>>>>      * remote proc loading
>>>>>>
>>>>>> What command can do this?
>>>>>>
>>>>>
>>>>> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L28
>>>>
>>>> This could be a new property in bootstd:
>>>>
>>>> bootstd: {
>>>>      probe-before = <&phandle of remoteproc>, ...
>>>> };
> 
> I think this can also work, the firmware names we can get from the dt
> nodes and I think modifying the names to signed firmwares that we used
> to do in the scripts can be handled in the driver itself.
> 

We need to also think about where the firmware blobs themselves come
from in this case. In fact we are having that discussion in a different
thread right now[0].

My suggestion would be to have the firmware inside the FIT. This is
already supported to some degree for normal firmware (TFA/OP-TEE/etc.).
This way drivers do not need to deal with all the complexity of finding
and loading blobs (they would have to replicate all the logic of stdboot
to find the blobs correctly in all cases if the blobs are not in the
same FIT that stdboot finds for us).

> Andrew, do you see anything else in this that cannot work?
> 
>>>>
>>>>>
>>>>>>>      * uEnv.txt processing
>>>>>>
>>>>>> What command can do this?
>>>>>>
>>>>>
>>>>> run envboot;
>>>>>
>>>>> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/mmc.env?ref_type=heads#L20
>>>>
>>>> This could be a new bootmeth which looks for uenv.txt on available
>>>> devices. It might be better to put the env in a FIT or something with
>>>> a checksum.
> 
> We use non-FIT boot as well for our non HS platforms, I think the new
> bootmeth might be helpful as you mentioned, would have to look at this
> more as I thought bootmeth is supposed to boot a platform at the end
> only but it seems like here we'll be combining this bootmeth ( that'll
> just import the env ) and then other bootmeth will actually boot Linux.
> 

As long as bootmeths can perform an action then continue with other
bootmeths that could work. This might require complex logic though.
For instance the uenv.txt bootmeth would only need run on some subset
of available bootdevs, and it should scan all those devs for uenv.txt
first, before the Linux finding bootmeths start.

Andrew

[0] https://lore.kernel.org/all/64a18d0a-294d-4e11-a867-4191e99f2437@ti.com/

> Regards,
> Manorit
> 
>>>>
>>>>>
>>>>>>>
>>>>>>> Last two can be solved by adding to CONFIG_BOOTCOMMAND before calling`bootflow scan`
>>>>>>> as suggested above. Until we figure out the first two I don't see us ready to switch.
>>>>>>
>>>>>> I'm happy to help figure this out.
>>>>>>
>>>>>
>>>>> Thanks, we are eager to standardize our boot flows and stdboot seems like the
>>>>> right way forward, very happy to have your guidance on these last few items :)
>>>>
>>>
>>> I'm just checking whether there is any feedback on the above? I sent
>>> the FIT part as a separate proposal.
>>>
>>
>> Manorit,
>>
>> Does the above all work for you?
>>
>> Andrew
>>
>>> Regards,
>>> Simon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Jon Humphreys]

Andrew Davis <afd@ti.com> writes:

>>>>>>>>      * uEnv.txt processing
>>>>>>>
>>>>>>> What command can do this?
>>>>>>>
>>>>>>
>>>>>> run envboot;
>>>>>>
>>>>>> https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/mmc.env?ref_type=heads#L20
>>>>>
>>>>> This could be a new bootmeth which looks for uenv.txt on available
>>>>> devices. It might be better to put the env in a FIT or something with
>>>>> a checksum.
>> 
>> We use non-FIT boot as well for our non HS platforms, I think the new
>> bootmeth might be helpful as you mentioned, would have to look at this
>> more as I thought bootmeth is supposed to boot a platform at the end
>> only but it seems like here we'll be combining this bootmeth ( that'll
>> just import the env ) and then other bootmeth will actually boot Linux.
>> 
>
> As long as bootmeths can perform an action then continue with other
> bootmeths that could work. This might require complex logic though.
> For instance the uenv.txt bootmeth would only need run on some subset
> of available bootdevs, and it should scan all those devs for uenv.txt
> first, before the Linux finding bootmeths start.
>

It doesn't make sense to treat envboot as a bootmeth because it's really
not about a way of booting but more often just used to modify
environment variables before going through another boot flow (although
you could define/run env variables that performs it's own boot).  It may be
better to think of it as a preprocessing step before a bootflow scan.

Some other thoughts - envboot is more of a hacky way to solve things:
1) As we move more towards secure boot as the standard, we should
   understand what use cases the hacks are working around and properly
   support those use cases.
2) But I don't think we should just remove it altogether because it is a
   very powerful 'feature' during the development phases to try out
   changes without modifying/rebuilding/reflashing u-boot.
3) This is a TI feature.  Are others solving this need in a better way?

Jon

Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

[Manorit Chawdhry]

Hi Andrew,

On 09:55-20240104, Andrew Davis wrote:
> On 1/4/24 1:52 AM, Manorit Chawdhry wrote:
> > Hi Andrew, Simon,
> > 
> > On 08:58-20240102, Andrew Davis wrote:
> > > On 12/31/23 6:48 AM, Simon Glass wrote:
> > > > Hi,
> > > > 
> > > > On Wed, Nov 29, 2023 at 7:48 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > 
> > > > > Hi Andrew,
> > > > > 
> > > > > On Mon, 6 Nov 2023 at 11:05, Andrew Davis <afd@ti.com> wrote:
> > > > > > 
> > > > > > On 11/6/23 11:47 AM, Simon Glass wrote:
> > > > > > > Hi Andrew,
> > > > > > > 
> > > > > > > On Mon, 6 Nov 2023 at 10:27, Andrew Davis <afd@ti.com> wrote:
> > > > > > > > 
> > > > > > > > On 11/6/23 9:31 AM, Tom Rini wrote:
> > > > > > > > > On Mon, Nov 06, 2023 at 11:23:51AM +0530, Manorit Chawdhry wrote:
> > > > > > > > > > Hi Simon,
> > > > > > > > > > 
> > > > > > > > > > On 11:22-20231005, Simon Glass wrote:
> > > > > > > > > > > Hi Nishanth,
> > > > > > > > > > > 
> > > > > > > > > > > On Thu, 5 Oct 2023 at 11:16, Nishanth Menon <nm@ti.com> wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > On 12:10-20231005, Nishanth Menon wrote:
> > > > > > > > > > > > > On 12:36-20231005, Tom Rini wrote:
> > > > > > > > > > > > > > On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> > > > > > > > > > > > > > > On 10/4/23 8:54 AM, Nishanth Menon wrote:
> > > > > > > > > > > > > > > > On 08:48-20231004, Andrew Davis wrote:
> > > > > > > > > > > > > > > > > On 10/4/23 8:23 AM, Roger Quadros wrote:
> > > > > > > > > > > > > > > > > > ti_mmc is not a valid boot_target for standard boot flow so
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > Is there some way to make it into a valid boot_target? Otherwise
> > > > > > > > > > > > > > > > > how do we use uEnv.txt files, or boot from FIT images with overlays?
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > envboot takes care of uEnv.txt file (see
> > > > > > > > > > > > > > > > https://lore.kernel.org/all/20231004132324.44198-3-rogerq@kernel.org/)
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > Early remote proc loading and FIT image is a question for stdboot itself.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > If stdboot is missing these features then we shouldn't switch until it
> > > > > > > > > > > > > > > has them. I'm all for switching to this, but only if it is complete.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Depends on what you mean?  Did you mean an option to run scripts
> > > > > > > > > > > > > > (exists) or an option to do what TI needs done, via
> > > > > > > > > > > > > > boot/bootmeth_something.c ?  If the latter, someone from TI needs to
> > > > > > > > > > > > > > figure out what that should be and do (but plumbing-wise everything it
> > > > > > > > > > > > > > needs should exist).
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Andrew is generalizing here (on the wrong patch though).
> > > > > > > > > > > > > 
> > > > > > > > > > > > > On am62x platforms, there is nothing regressing with this series. The
> > > > > > > > > > > > > challenge is early remote_proc loading which is done for J7* platforms.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > How that is initiated as part of bootmethods is something of a gap.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > The other gap has been support for uEnv.txt -> which we can workaround
> > > > > > > > > > > > > at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
> > > > > > > > > > > > > -lb" in defconfig (This series from Roger already does that - hence I am
> > > > > > > > > > > > > saying that Andrew is complaining on the wrong series).
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan-lb" and
> > > > > > > > > > > > > uEnv.txt remoteproc loads and the various standard bootmethods should
> > > > > > > > > > > > > "just work".
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > I forgot to add: FIT image authenticated boot flow. That is really what
> > > > > > > > > > > > ti_mmc distroboot method was trying to solve.
> > > > > > > > > > > > 
> > > > > > > > > > > > Maybe Simon or someone know how the stdboot flow handles authenticated
> > > > > > > > > > > > kernel image and dtb boot flow with FIT image?
> > > > > > > > > > > 
> > > > > > > > > > > Yes you can use FIT configuration verification and things should work as normal.
> > > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > Could you give any reference documentation for this?
> > > > > > > > > 
> > > > > > > > > I suspect you should start with doc/usage/fit/beaglebone_vboot.rst
> > > > > > > > > 
> > > > > > > > 
> > > > > > > >     From that doc:
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > Boot the board using the commands below::
> > > > > > > > 
> > > > > > > >         setenv bootargs console=ttyO0,115200n8 quiet root=/dev/mmcblk0p2 ro rootfstype=ext4 rootwait
> > > > > > > >         ext2load mmc 0:2 82000000 /boot/image.fit
> > > > > > > >         bootm 82000000
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > This is very much not stdboot :( This doc has some good and current info on building
> > > > > > > > a secure FIT image, but much of it is showing its age. Stdboot is still missing
> > > > > > > > 
> > > > > > > >      * ability to limit boot mode search to only one image (FIT)
> > > > > > > 
> > > > > > > What does this mean? Can you please be more specific or give an example?
> > > > > > > 
> > > > > > 
> > > > > > Sure, for instance with secure boot we only want to search for FIT images
> > > > > > and if for each media this fails we do not want to fall back to other
> > > > > > image types or boot modes (like UART boot or net boot which would bypass
> > > > > > the signature checks).
> > > > > 
> > > > > We could have something like:
> > > > > 
> > > > > bootstd {
> > > > >      image-types = "fit", "uimage";
> > > > > }
> > > > > 
> > > > > to limit the supported types.
> > > > > 
> > > > > > 
> > > > > > Something similar is needed for searching for EFI compatible boot across
> > > > > > each enabled boot media first, before trying other methods on each media.
> > > > > > Basically breadth-first search on each media type not depth-first.
> > > > > 
> > > > > How about:
> > > > > 
> > > > > bootstd {
> > > > >      scan-order = "bootmeth,bootdev";   // instead of default "bootdev,bootmeth";
> > > > > };
> > > > > 
> > > 
> > > This along with the image-types above seem reasonable. Might also help with
> > > our EFI search order issue (we need to search all boot media for EFI parts
> > > first before trying all the other types of boot on each media, basically
> > > breadth-first search vs depth for EFI). We should be able to describe that
> > > pattern here if we think this through.
> > > 
> > 
> > This seems okay and will solve the problem of the FIT Image issues and
> > fallback that we were having with stdboot.
> > 
> > > > > 
> > > > > > 
> > > > > > > >      * fetching and applying FIT overlay/config strings
> > > > > > > 
> > > > > > > Re overlays, is this the 'extension' command? What are config strings?
> > > > > > > 
> > > > > > 
> > > > > > When we have DT overlays in our FIT image, we build a string to pass to
> > > > > > bootm to apply all the selected overlays[0]. If there is now another
> > > > > > mechanism for this please let me know (building this string today is messy).
> > > > > > 
> > > > > > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L18
> > > > > 
> > > > > My proposal for this is to implement extensions in FIT. This would
> > > > > have a new extensions nodes, so you can specify what extensions are
> > > > > available for each FIT configuration.
> > > > > 
> > > > > configurations {
> > > > >      conf-1 {
> > > > >         compatible = ...
> > > > >         extensions = "ext1", "ext-2";
> > > > >      };
> > > > > 
> > > > > extensions {
> > > > >      ext-1 {
> > > > >         fdto = "fdt-1";   // fdt overlay for this 'cape'
> > > > >         compatible = "vendor,combined-device1";
> > > > >         extensions = "ext3";
> > > > >      };
> > > > > 
> > > > >      ext-2 {
> > > > >         fdto = "fdto-2";   // fdt overlay for this 'cape'
> > > > >         compatible = "vendor,device2";
> > > > >      };
> > > > > 
> > > > >      ext-3 {
> > > > >         fdto = "fdto-3";
> > > > >         compatible = "vendor,device3";
> > > > >      };
> > > > > 
> > > > > So FIT configurations now have a list of supported extensions. The
> > > > > extensions are hierarchical so that you can have ext1 which can
> > > > > optionally have ext-2 as well. This allows boards which share a lot of
> > > > > 'capes' to be specified only ones.
> > > > > 
> > > > > Extensions actually present are declared by a sysinfo driver for the
> > > > > board, with new methods:
> > > > > 
> > > > > get_compat() - determine the compatible strings for the current platform
> > > > > get_ext() - get a list of compatible strings for extensions which are
> > > > > actually present
> > > > > 
> > > > > The extension compatible strings are used to select the correct things
> > > > > from the FIT, apply the overlays and produce the final DT.
> > > > > 
> > 
> > This is something that is still concerning me, so previously we used to
> > have two methods of applying overlays, one was through FIT Image where
> > we have the overlays embedded and in another we used to use non FIT
> > boot where the overlays were present in the filesystem only and we used
> > to apply them, this might solve the problem with FIT Image booting overlays but
> > normal kernel boot and dtb with overlays is still something that I don't
> > know would be possible or not with this. Is there any way to do that?
> > 
> > > 
> > > If we have a list of compatible strings and extensions, then do we need
> > > the extensions nodes? Couldn't each extension string simply be the name
> > > of the overlay ftdo-x node to apply?
> > > 
> > > > > > 
> > > > > > > >      * remote proc loading
> > > > > > > 
> > > > > > > What command can do this?
> > > > > > > 
> > > > > > 
> > > > > > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/ti_common.env?ref_type=heads#L28
> > > > > 
> > > > > This could be a new property in bootstd:
> > > > > 
> > > > > bootstd: {
> > > > >      probe-before = <&phandle of remoteproc>, ...
> > > > > };
> > 
> > I think this can also work, the firmware names we can get from the dt
> > nodes and I think modifying the names to signed firmwares that we used
> > to do in the scripts can be handled in the driver itself.
> > 
> 
> We need to also think about where the firmware blobs themselves come
> from in this case. In fact we are having that discussion in a different
> thread right now[0].
> 
> My suggestion would be to have the firmware inside the FIT. This is
> already supported to some degree for normal firmware (TFA/OP-TEE/etc.).
> This way drivers do not need to deal with all the complexity of finding
> and loading blobs (they would have to replicate all the logic of stdboot
> to find the blobs correctly in all cases if the blobs are not in the
> same FIT that stdboot finds for us).
> 

How would we handle people who use non-FIT boot? If you are telling us
to pack these in tispl.bin/u-boot.img then am not sure if size could be
an issue otherwise it does seem scalable but I would be against packing
these in the Kernel FIT image as that would make no sense at all to me
with the complexity that FIT building already has and having additional
things to pack with multiple projects involved while generating the FIT.
Also, I don't want people to depend on FIT image and the old method of
using Kernel + DTB should still work and this approach won't help in
solving that.

Regards,
Manorit

> > Andrew, do you see anything else in this that cannot work?
> > 
> > > > > 
> > > > > > 
> > > > > > > >      * uEnv.txt processing
> > > > > > > 
> > > > > > > What command can do this?
> > > > > > > 
> > > > > > 
> > > > > > run envboot;
> > > > > > 
> > > > > > https://source.denx.de/u-boot/u-boot/-/blob/master/include/env/ti/mmc.env?ref_type=heads#L20
> > > > > 
> > > > > This could be a new bootmeth which looks for uenv.txt on available
> > > > > devices. It might be better to put the env in a FIT or something with
> > > > > a checksum.
> > 
> > We use non-FIT boot as well for our non HS platforms, I think the new
> > bootmeth might be helpful as you mentioned, would have to look at this
> > more as I thought bootmeth is supposed to boot a platform at the end
> > only but it seems like here we'll be combining this bootmeth ( that'll
> > just import the env ) and then other bootmeth will actually boot Linux.
> > 
> 
> As long as bootmeths can perform an action then continue with other
> bootmeths that could work. This might require complex logic though.
> For instance the uenv.txt bootmeth would only need run on some subset
> of available bootdevs, and it should scan all those devs for uenv.txt
> first, before the Linux finding bootmeths start.
> 
> Andrew
> 
> [0] https://lore.kernel.org/all/64a18d0a-294d-4e11-a867-4191e99f2437@ti.com/
> 
> > Regards,
> > Manorit
> > 
> > > > > 
> > > > > > 
> > > > > > > > 
> > > > > > > > Last two can be solved by adding to CONFIG_BOOTCOMMAND before calling`bootflow scan`
> > > > > > > > as suggested above. Until we figure out the first two I don't see us ready to switch.
> > > > > > > 
> > > > > > > I'm happy to help figure this out.
> > > > > > > 
> > > > > > 
> > > > > > Thanks, we are eager to standardize our boot flows and stdboot seems like the
> > > > > > right way forward, very happy to have your guidance on these last few items :)
> > > > > 
> > > > 
> > > > I'm just checking whether there is any feedback on the above? I sent
> > > > the FIT part as a separate proposal.
> > > > 
> > > 
> > > Manorit,
> > > 
> > > Does the above all work for you?
> > > 
> > > Andrew
> > > 
> > > > Regards,
> > > > Simon