[U-Boot] [PATCH] fastboot: Fix slot names reported by getvar

Fri Jun 14 14:37:57 UTC 2019

Hi Eugeniu,

On Thu, Jun 13, 2019 at 8:57 PM Eugeniu Rosca <erosca at de.adit-jv.com> wrote:
>
> Hi Sam,
>
> Thanks for the detailed answer. Some comments below.
>
> On Thu, Jun 13, 2019 at 04:59:40PM +0300, Sam Protsenko wrote:
> > Hi Eugeniu,
> >
> > On Thu, Jun 13, 2019 at 12:31 PM Eugeniu Rosca <erosca at de.adit-jv.com> wrote:
> > >
> > > Hi Sam,
> > >
> > > On Thu, Jun 13, 2019 at 12:49:45AM +0300, Sam Protsenko wrote:
> > > > In commit [1] fastboot tool was changed w.r.t. new A/B specification [2],
> > > > and now we should report slot names in "a" format instead of "_a".
> > > > Latter is now considered legacy and we shouldn't rely on that anymore.
> > >
> > > This looks like a change which advantages the users who are always on
> > > the tip/HEAD of all relevant components (fastboot and U-Boot), but that
> > > rarely happens in the industry. Suppliers and hardening vendors often
> > > deliver obsoleted versions because they can't keep up with upstream
> > > development. Can you please document the behavior of 'fastboot flash'
> > > (and anything else relying on
> > > 'fastboot getvar (current-slot|slot-suffixes) in below scenarios:
> > >
> > > A. fastboot >= [1] && U-Boot + this patch
> > > B. fastboot >= [1] && U-Boot - this patch
> > > C. fastboot <  [1] && U-Boot + this patch
> > > D. fastboot <  [1] && U-Boot - this patch
> > >
> > > Would it be possible to keep U-Boot backward-compatible, such that
> > > regardless of the scenario enumerated above, 'fastboot flash' will
> > > always succeed?
> > >
> >
> > I'm afraid in this particular case we weren't given any choice, and we
> > won't be able to provide backward-compatibility for older Android
> > releases. After this commit:
> >
> >     [3] https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f
>
> Thumbs up for pointing out this specific commit.
>
> >
> > they dropped support for "_a" format completely (in fastboot tool). So:
> >   * if user runs new fastboot tool ( >= [3]), then the only way to
> > make "fastboot flash" work is to return slot in "a" format from
> > bootloader
> >   * if user runs old fastboot tool (< [1]), then the only way to make
> > "fastboot flash" work is to return slot in "_a" format from bootloader
>
> This interface change seems to carry no semantic value, so it's
> unfortunate to see such kind of non-backward-compatible update
> contributed by the maintainers of the user-space tool.
>

Yeah, I was pretty frustrated too :) Anyway, let's try to handle this
in U-Boot as good as we can.

> > Good news is that user can basically downgrade or upgrade fastboot
> > tool, to be in sync with U-Boot version in use. Bad news, we need to
> > decide which Android version to break in U-Boot/master.
> >
> > I suggest we track AOSP/master in U-Boot/master. Please let me know if
> > you agree, or maybe there is some better way I'm missing.
>
> I can't come up with a better proposal.
>

Well, theoretically we *can* provide some config option like
AB_NEW/AB_OLD, or check similar variable in U-Boot shell, to figure
out which format to use ("_a" or "a"). But right now A/B is not
implemented in upstream U-Boot anyway, so we probably don't have much
users for that feature yet. So let's implement it for the latest
AOSP/master, and if the need arises, we can handle old format support
later. The crucial thing is, this patch fixes "fastboot flash" in
AOSP/master, when user has slotted partitions.

> Reflecting on fastboot in general, I think there are many more
> questions which lack a straightforward answer:
>  - How accurately the fastboot protocol [4] reflects the AOSP
>    user-space implementation (and viceversa)?
>  - Some basic git grepping reveals that the fastboot protocol [4]
>    was once upgraded from v0.3 to v0.4, but that's pretty much it. It
>    definitely doesn't cover the evolution of fastboot usage/parameters.
>    For example, none of the AB/slot-related parameters (e.g.
>    "slot-suffixes", "current-slot") has ever been documented in [4].
>  - Since the protocol and the actual development are out of sync,
>    staying aligned to AOSP means asserting about the fastboot
>    interface updates by reviewing the AOSP patches (which is time
>    consuming and cannot be easily automated).
>  - OTOH, it is also not clear to which degree U-Boot/master is aligned
>    to AOSP/master even after integrating this patch. As example,
>    commit [3] mentioned by you dropped the support for 'slot-suffixes'
>    altogether while the option is still being processed in U-Boot.
>

Agree. Someone should check AOSP patches and provide corresponding
support for the latest fastboot tool feature. I will probably look
into that later, as we want this for our platforms to be implemented.

I'm gonna send v2 soon, addressing your comments (will remove
slot-suffixes variable and improve commit message). Thanks for your
input!

> In a nutshell, I think we have no other choice but to apply this fix
> (still not clear if w/ or w/o keeping the support for "slot-suffixes").
> I think it's also a must to warn the users that this patch
> assumes/depends on AOSP fastboot version/commit [3] or higher.
>
> > > > [1] https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5
> > > > [2] https://source.android.com/devices/tech/ota/ab/ab_implement#partitions
>
> [4] https://android.googlesource.com/platform/system/core/+/master/fastboot/README.md
>
> --
> Best Regards,
> Eugeniu.