[U-Boot] [PATCH 0/8] Initial integration of AVB2.0

[Eugeniu Rosca]

Hello Igor, Alex, Kever,

Having these patches in mainline would be great, as this would reduce
the delta between our own and community U-boot trees. After having a
quick look at this series, I have some questions/review findings.

These patches appear to be slightly older than what is available in [1].

More precisely, ignoring:
- change of license
- drop of avb_crc32.c
- updates in avb_sysdeps_posix.c
- 's^#include "^#include "avb/^'

The version of libavb imported in this patch series seems to match
commit b60834f7a4a8 ("uefi: Set both androidboot.slot and
androidboot.slot_suffix.") from [1].

FYI, there are 12 more non-merge commits in the avb repository [2]. Is
there a strong reason not to include them?

I am not an expert in AVB, but IMHO, as suggested by Alex, we shouldn't
assume that the in-tree libavb will diverge from the vanilla one. IMHO
a cleaner workflow would be to contribute any bugfixes to the original
repository and update the U-boot libavb from time to time, similar
to how it's done, as example, for dtc in kernel [3]. Or maybe you see
some obstacles to achieve this in practice?

Assuming that:
1) libavb will undergo regular updates from its original repository.
2) most of libavb headers are not designed to be included from
   non-libavb code, throwing below:
#error "Never include this file directly, include libavb.h instead."

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).

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.

Thank you!

Best regards,
Eugeniu.

[1] https://android.googlesource.com/platform/external/avb/+/master

[2] Postt-b60834f7a4a8 libavb commits in [1]
$> git log --oneline --no-merges b60834f7a4a8..avb_upstream/master -- libavb
30dd8e5a1757 libavb: Add new routine to calculate a digest of all vbmeta images.
fd0ba0d49101 Implement support for on-device persistent digests.
97740e537ad1 Split kernel cmdline code in separate file.
fcadbf1d1a71 Support (boot) partition preloading.
061e33b39e27 Add avb_div_by_10() sysdep operation.
36e5c43f58d2 Fix incorrect variable names in avb_replace
fc2531374e30 Fix AvbAlgorithmType type-limits error
047ecf7d2361 libavb: Avoid conflict with system-provided crc32 symbol.
0922bf8970fd Make it possible to disable verification.
01ca9962bd0d libavb: Only load and verify hash partition if requested.
8221811c5da1 libavb: Allow specifying dm-verity error handling.
27a291fcc194 libavb: Load entire partition if |allow_verification_error| is true.

[3] The way in-tree kernel DTC is aligned to upsteam:
$> git log --oneline --no-merges -i --grep "update.*upstream" -- scripts/dtc
9130ba884640 scripts/dtc: Update to upstream version v1.4.6-9-gaadd0b65c987
e45fe7f788dd scripts/dtc: Update to upstream version v1.4.5-6-gc1e55a5513e9
4201d057ea91 scripts/dtc: Update to upstream version v1.4.5-3-gb1a60033c110
89d123106a97 scripts/dtc: Update to upstream version v1.4.4-8-g756ffc4f52f6

[4] http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=60e14a6a07c5
[5] http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/lib/avb/fsl?h=imx_v2015.04_brillo
[6] 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