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

[Igor Opaniuk]

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 <erosca at 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