public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Eugeniu Rosca <roscaeugeniu@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 0/8] Initial integration of AVB2.0
Date: Sun, 6 May 2018 13:31:18 +0200	[thread overview]
Message-ID: <20180506113118.GB20434@example.com> (raw)
In-Reply-To: <1524662285-19617-1-git-send-email-igor.opaniuk@linaro.org>

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

  parent reply	other threads:[~2018-05-06 11:31 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-25 13:17 [U-Boot] [PATCH 0/8] Initial integration of AVB2.0 Igor Opaniuk
2018-04-25 13:17 ` [U-Boot] [PATCH 1/8] avb2.0: add Android Verified Boot 2.0 libraries Igor Opaniuk
2018-04-25 13:17 ` [U-Boot] [PATCH 2/8] avb2.0: integrate avb 2.0 into the build system Igor Opaniuk
2018-04-25 13:18 ` [U-Boot] [PATCH 3/8] avb2.0: implement AVB ops Igor Opaniuk
2018-04-25 13:18 ` [U-Boot] [PATCH 4/8] cmd: avb2.0: avb command for performing verification Igor Opaniuk
2018-05-02 18:52   ` Sam Protsenko
2018-05-03  2:31   ` Simon Glass
2018-05-15 15:44     ` Igor Opaniuk
2018-05-15 16:26       ` Simon Glass
2018-05-15 17:31         ` Igor Opaniuk
2018-05-15 18:28           ` Simon Glass
2018-05-16  8:20             ` Igor Opaniuk
2018-05-16 15:40               ` Simon Glass
2018-04-25 13:18 ` [U-Boot] [PATCH 5/8] avb2.0: add boot states and dm-verity support Igor Opaniuk
2018-05-02 18:59   ` Sam Protsenko
2018-04-25 13:18 ` [U-Boot] [PATCH 6/8] am57xx_hs: avb2.0: add support of AVB 2.0 Igor Opaniuk
2018-05-02 19:06   ` Sam Protsenko
2018-04-25 13:18 ` [U-Boot] [PATCH 7/8] test/py: avb2.0: add tests for avb commands Igor Opaniuk
2018-04-25 13:18 ` [U-Boot] [PATCH 8/8] doc: avb2.0: add README about AVB2.0 integration Igor Opaniuk
2018-05-02 19:12   ` Sam Protsenko
2018-05-16  9:20     ` Igor Opaniuk
2018-04-26  3:05 ` [U-Boot] [PATCH 0/8] Initial integration of AVB2.0 Kever Yang
2018-04-26 13:00   ` Igor Opaniuk
2018-04-26 18:35   ` Alex Deymo
2018-04-27  9:53     ` Igor Opaniuk
2018-04-30 10:47       ` Alex Deymo
2018-05-06 11:31 ` Eugeniu Rosca [this message]
2018-05-15 15:31   ` Eugeniu Rosca
2018-05-15 16:58     ` Igor Opaniuk
2018-05-15 17:10       ` Eugeniu Rosca

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180506113118.GB20434@example.com \
    --to=roscaeugeniu@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox