Hi Eugeniu, You're totally right regarding avb internal headers, they all should remain in lib/libavb. v2 patchset (planning to send it by the end of this week) will include these changes you're talking about (+ will include libavb updates from [1]).
> My final question is what's your opinion about the NXP-specific libavb > wrapper implementation found at `lib/avb/fsl/` in [5]/[6] which seems > to rely on libavb and libavb_ab. Do you see it as a good model to be > followed by other platforms (both from point of view of contents and > placement in the tree)? I am asking because we are in the middle > of some decision-making for similar/alternative implementation on a > different SoC, so your feedback will be extremely helpful and greatly > appreciated. I've taken a quick look at patches from [2], and noticed a few issues (IMHO): 1) It includes custom implementation of RPMB operations, although generic RPMB functionality already exists in the U-boot mainline (check drivers/mmc/rpmb.c) 2) It introduces a brand-new "boota" command for booting Android, although existing bootm does already support Android boot image parsing and booting. 3) It doesn't have any dm-verify enforcement policies in case if the bootloader is in the "locked" state (at least, I didn't manage to grep it) 4) There are a bunch of platform-specific code introduced to fastboot driver, and it's used as a base for working with GPT in avb ops (sounds strange, but this is what I see here), although mainline U-boot does have API for this purposes. Obviously, the root cause of all these issues is a big divergence from the U-boot mainline codebase. Thanks! Regards, Igor [1] https://android.googlesource.com/platform/external/avb/+/master [2] http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/lib/avb/fsl?h=imx_v2017.03_4.9.11_1.0.0_ga On 15 May 2018 at 18:31, Eugeniu Rosca <ero...@de.adit-jv.com> wrote: > On Sun, May 06, 2018 at 01:31:18PM +0200, Eugeniu Rosca wrote: >> Hello Igor, Alex, Kever, >> >> I wonder if it would be possible to keep the internal libavb headers in >> lib/libavb (similar to how it's done by NXP in [4]), since this would >> allow not rewriting the original include paths for such headers in every >> *.c file and hence minimize the delta between in-tree vs out-of-tree >> code, as well as potentially speed up the update process (and/or >> simplify any update script which will take care of it, as also >> mentioned by Alex). >> >> [4] >> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=60e14a6a07c5 > > Here is what I mean. > Three lzma headers are exposed to internal users/consumers: > > $ git grep '"../../lib/' > ---<-snip->--- > include/lzma/LzmaDec.h:#include "../../lib/lzma/LzmaDec.h" > include/lzma/LzmaTools.h:#include "../../lib/lzma/LzmaTools.h" > include/lzma/LzmaTypes.h:#include "../../lib/lzma/Types.h" > ---<-snip->--- > > A few places where lzma headers are used: > > $ git grep 'include <lzma/Lzma' > cmd/lzmadec.c:#include <lzma/LzmaTools.h> > common/bootm.c:#include <lzma/LzmaTypes.h> > common/bootm.c:#include <lzma/LzmaDec.h> > common/bootm.c:#include <lzma/LzmaTools.h> > lib/lzma/LzmaTools.h:#include <lzma/LzmaTypes.h> > test/compression.c:#include <lzma/LzmaTypes.h> > test/compression.c:#include <lzma/LzmaDec.h> > test/compression.c:#include <lzma/LzmaTools.h> > > For libavb this would translate to: > > $ cat include/avb/libavb.h > ---<-snip->--- > #include "../../lib/libavb/avb_chain_partition_descriptor.h" > #include "../../lib/libavb/avb_crypto.h" > #include "../../lib/libavb/avb_descriptor.h" > #include "../../lib/libavb/avb_footer.h" > #include "../../lib/libavb/avb_hash_descriptor.h" > ---<-snip->--- > > And with that, various consumers (mainly libavb_avb?) would do: > #include <avb/libavb.h> > > As said, this would make integration of new libavb versions much easier. > Would appreciate your thoughts. > > Best regards, > Eugeniu. -- Regards, Igor Opaniuk _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
