RE: [PATCH 1/2] fsl-layerscape: add dtb overlay feature

["Sahil Malhotra (OSS)" <sahil.malhotra@oss.nxp.com>]

Hi Andrey,

> -----Original Message-----
> From: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> Sent: Wednesday, December 8, 2021 3:43 PM
> To: Sahil Malhotra (OSS) <sahil.malhotra@oss.nxp.com>; Michael Walle
> <michael@walle.cc>
> Cc: Clément Faure <clement.faure@nxp.com>; Gaurav Jain
> <gaurav.jain@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>; Priyanka
> Jain <priyanka.jain@nxp.com>; u-boot@lists.denx.de; Varun Sethi
> <V.Sethi@nxp.com>; Ye Li <ye.li@nxp.com>
> Subject: RE: [PATCH 1/2] fsl-layerscape: add dtb overlay feature
> 
> Hello Sahil,
> 
> > -----Original Message-----
> > From: Sahil Malhotra (OSS) <sahil.malhotra@oss.nxp.com>
> > Sent: Wednesday, December 8, 2021 7:12 AM
> > To: Michael Walle <michael@walle.cc>; Sahil Malhotra (OSS)
> > <sahil.malhotra@oss.nxp.com>
> > Cc: Clément Faure <clement.faure@nxp.com>; Gaurav Jain
> > <gaurav.jain@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>;
> Priyanka
> > Jain <priyanka.jain@nxp.com>; u- boot@lists.denx.de; Varun Sethi
> > <V.Sethi@nxp.com>; Ye Li <ye.li@nxp.com>; ZHIZHIKIN Andrey
> > <andrey.zhizhikin@leica-geosystems.com>
> > Subject: RE: [PATCH 1/2] fsl-layerscape: add dtb overlay feature
> >
> >
> > Hi Michael,
> >
> > -----Original Message-----
> > From: Michael Walle <michael@walle.cc>
> > Sent: Monday, November 29, 2021 11:17 PM
> > To: Sahil Malhotra (OSS) <sahil.malhotra@oss.nxp.com>
> > Cc: Clément Faure <clement.faure@nxp.com>; Gaurav Jain
> > <gaurav.jain@nxp.com>; Pankaj Gupta <pankaj.gupta@nxp.com>;
> Priyanka
> > Jain <priyanka.jain@nxp.com>; u- boot@lists.denx.de; Varun Sethi
> > <V.Sethi@nxp.com>; Ye Li <ye.li@nxp.com>; ZHIZHIKIN Andrey
> > <andrey.zhizhikin@leica-geosystems.com>
> > Subject: Re: [PATCH 1/2] fsl-layerscape: add dtb overlay feature
> >
> > Hi Sahil,
> >
> > Am 2021-11-29 12:55, schrieb Sahil Malhotra (OSS):
> > > Am 2021-11-17 19:11, schrieb Sahil Malhotra (OSS):
> > >>> Could you please add some description what this is doing and for
> > >>> what this is intended? To have a "DTB overlay feature", it is
> > >>> enough to just enable CONFIG_OF_LIBFDT_OVERLAY.
> > >> I will add some description, and yes for DTB overlay feature, it is
> > >> enough to enable CONFIG_OF_LIBFDT_OVERLAY but we need to do this
> > >> step before booting the kernel that's why also have to enable
> > >> CONFIG_OF_SYSTEM_SETUP.
> > >
> > >> Ok. What will the overlay do? Could you give an example?
> > > This overlay will be disabling the crypto nodes which will be used
> > > by optee in secure world, so that linux should not use it.
> > >
> > >
> > >>> Apparently you're adding an overlay passed by optee. Doesn't this
> > >>> have to be applied to u-boot's control dtb too?
> > >> Yes, we will be applying the overlay passed by optee, yes it will
> > >> be applied to dtb which will be passed to uboot for kernel booting.
> > >
> > >> If I read this patch correctly, you're modifying the DT before you
> > >> jump to linux. But I was asking whether you also have to modify the
> > >> DT which is used by u-boot. Eg. if you disable some kind of crypto
> > >> nodes (because optee will use them in secure world), this also have
> > >> to communicated to u-boot, not only linux, no?
> > > Yes, I got your point now, and is very valid, but as of now for
> > > u-boot we are just using the first available node for communicating
> > > with CAAM leaving other job rings as it is.
> > > So we need not to apply overlay to DTB used by uboot.
> >
> > > But we should do the correct thing, so that u-boot and linux doesn't
> > > see a
> > different version of the device tree.
> >
> > > Also what do you mean with "the first available node"?
> > > There is already a new CAAM driver for u-boot pending, see
> > > https://lore.kernel.org/u-boot/20211115070014.17586-1-gaurav.jain@nx
> > > p.com/
> >
> > Very first CAAM Job Ring is always used by u-boot, OP-TEE does not use
> > this job ring. Same job ring can be used by Linux once it boots up.
> 
> Just for clarification: by saying "Very first CAAM Job Ring" do you actually
> mean JR0?
Yes, Very first CAAM Job Ring means JR0 for Layerscape.

> 
> If the BootROM logic with respect to JR reservation for LS family does not
> differ from i.MX8M family (which I assume does not), then why can't the
> logic be implemented in the same way proposed by Gaurav [1] here as well?
BootROM logic with respect to JR reservation is different for both Layerscape and i.MX8M family.
On Layerscape Platforms we don't have HAB, so we don’t need to reserve JR0.

> 
> DT nodes can be statically disabled if we know that they are held by HAB and
> are not released to NS World.
> 
> OP-TEE does set the status itself via dt_enable_secure_status(), which
> should present the properly configured FDT when U-Boot takes over.
Yes, OP-TEE set the status by dt_enable_secure_status() in DTB overlay which gets merged with DTB provided for Linux bootup and then Linux boots with merged DTB.
But u-boot uses the DTB embedded in its image. How can we modify that DTB or merge DTB overlay passed by OP-TEE with uboot DTB ?

> 
> This is however valid only if OP-TEE implementation for LS matches to one
> from i.MX8M family.
> 
> If it OP-TEE does differ, then I suggest this should be rather addressed there
> before U-Boot, since OP-TEE have all facilities in place to reserve JR nodes as
> they are needed.
OP-TEE reserves the JR node needed by it, but it needs to tell that to Normal World entities and that is being done using DTB overlay for now.

> 
> >
> > Regards,
> > Sahil Malhotra
> 
> -- andrey
> Link: [1]: https://lore.kernel.org/u-boot/20211207074129.10955-9-
> gaurav.jain@nxp.com

Regards,
Sahil Malhotra