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=i...
On 15 May 2018 at 18:31, Eugeniu Rosca erosca@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=60e14a6a07...
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.