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

[Michael Walle <michael@walle.cc>]

Am 17. Dezember 2021 11:19:42 GMT+05:00 schrieb "Sahil Malhotra (OSS)" <sahil.malhotra@oss.nxp.com>:
>Hi Michael/Andrey
>
>Gentle reminder for comment on this patch series.
>
>Regards,
>Sahil Malhotra
>
>> -----Original Message-----
>> From: Sahil Malhotra (OSS) <sahil.malhotra@oss.nxp.com>
>> Sent: Friday, December 10, 2021 12:03 PM
>> To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; 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
>> 
>> 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
>> 
>> 
>> 
>

hey sahil,

I'm on vacation until the end of this week. will comment beginning of next week.

-michael