[U-Boot] [RFC PATCH v2 00/20] Add fastboot UDP support

[Alex Kiernan]

On Wed, May 2, 2018 at 7:34 AM Jocelyn Bohr <bohr at google.com> wrote:

> Hi Alex,

> I think this approach looks really good so far, and will make maintaining
both
> implementations easier going forward. I haven't looked at every change
here
> yet, but when going through the patches should I add a "Reviewed-by:"
line if
> the change looks good to me?


Yes please!

> Thanks,
> Jocelyn

> On Mon, Apr 30, 2018 at 1:33 AM Alex Kiernan <alex.kiernan at gmail.com>
wrote:


>> This series merges the fastboot UDP support from AOSP into mainline
>> U-Boot.

>> The underlying implementations of most commands are now merged between
>> both code paths ('oem format' from the USB side is the only one
remaining).

>> Other changes to command handling so that UDP follows the existing USB
>> behaviour:

>> - 'boot' now follows the USB code and does 'fastboot
CONFIG_FASTBOOT_BUF_ADDR'.
>>    I've added 'fastbootcmd' which if set overrides the boot command and
>>    allows the existing UDP behaviour to be preserved.


> The UDP behavior more correct based on the specification.
> "The previously downloaded data is a boot.img and should be booted
according to
> the normal procedure for a boot.img."

> https://android.googlesource.com/platform/system/core/+/master/fastboot/

> I'm not sure who we will break if we change the USB code...

Making sure we don't break (or at least trying my hardest to) existing
upstream code was my aim in this. If the right thing is to change it, then
do that when the android bootflow gets merged?

> also we probably
> want to make sure image verification happens with "fastboot boot", making
sure
> "fastboot boot" doesn't allow booting an unverified kernel on locked
devices. I
> think we can add that flow later though, upstream U-boot doesn't support
> lock/unlock commands anyway.


I think running a general command here (via 'fastbootcmd') should give
enough options to implement that policy. For my use case fastboot is
actually just a convenient technology and I don't need the Android image
format or bootflow... so I'm getting my signature verification out of a
signed FIT image, and use something like:

fastbootcmd='setenv verify yes; bootm 0x81000800#conf at 0'


>> - 'continue' in UDP now exits the fastboot server rather than executing
>>    'run bootcmd'
>> - 'reboot-bootloader' no longer writes 'reboot-bootloader' to
>>    CONFIG_FASTBOOT_BUF_ADDR as its marker for the subsequent boot. The
code
>>    which is in AOSP common/android_bootloader.c expects this marker, but
>>    we have prior art in the USB code using the weak function
>>    fb_set_reboot_flag
>> - 'getvar' in the UDP path now supports fetching 'fastboot.' prefixed
>>    variables (inherited from the USB path)
>> - 'getvar' in the USB path inherits all the variables from the UDP path

>> Remaining issues:

>> - whilst I've merged NAND support into the UDP code path, I've no way
>>    of testing it. My expectation is it'll work, but will need
timed_send_info
>>    working into the NAND path to avoid timeouts on the network side.


> I don't know of any devices that use NAND and the fastboot UDP code, so
> I'm inclined to say this is okay.


I think so... it's not broken today because there's no UDP and NAND code in
upstream, if someone needs that then the mechanics are all there to wire it
in (and I've no way of testing it).

>> - I still need to fix timed_send_info handling when going through the USB
>>    path.
>> - the protocol part of the fastboot UDP implementation is separated out
>>    and would I expect form the basis of a consolidated implementation, but
>>    I'm inclined to address that as a clearly separate patch so it can be
>>    tested in isolation (I have no USB hardware I can try this on).


> Should this set of patches be tested on USB hardware too? There are some
> changes here to the USB implementation.


My aim thus far was to keep them sufficiently minimal that you get decide
on correctness by inspection, which isn't to say I've actually got it
right, but I'm hopeful. That said you can't beat a real test!

-- 
Alex Kiernan