[PATCH] android_ab: fix slot selection

[PATCH] android_ab: fix slot selection

[Colin Pinnell McAllister]

The boot selection rules state that a slot is bootable if it is not
corrupted and either has tries remaining or has already booted
successfully. However, slots that have tries_remaining == 0 and
successful_boot == 1 will be disregarded when picking the slot to
attempt.

Update the selection logic so slots marked successful remain eligible
even when their tries counter is zero. Also include the successful_boot
state in the unbootable-slot debug output.

Signed-off-by: Colin Pinnell McAllister <colin.mcallister@garmin.com>
---

An alternative would be to keep the existing behavior and update the
comment in boot/android_ab.c:276 to remove the successful_boot
condition. I do prefer updating the logic to match the documented rules.
Currently, the userspace AB code will need to increment the tries
counter for a slot that has booted successfully but has no tries left.

 boot/android_ab.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/boot/android_ab.c b/boot/android_ab.c
index 13e82dbcb7f..93e74f81d4b 100644
--- a/boot/android_ab.c
+++ b/boot/android_ab.c
@@ -289,11 +289,14 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
        slot = -1;
        for (i = 0; i < abc->nb_slot; ++i) {
                if (abc->slot_info[i].verity_corrupted ||
-                   !abc->slot_info[i].tries_remaining) {
+                   (!abc->slot_info[i].tries_remaining &&
+                        !abc->slot_info[i].successful_boot)) {
                        log_debug("ANDROID: unbootable slot %d tries: %d, ",
                                  i, abc->slot_info[i].tries_remaining);
-                       log_debug("corrupt: %d\n",
+                       log_debug("corrupt: %d, ",
                                  abc->slot_info[i].verity_corrupted);
+                       log_debug("successful: %d\n",
+                                 abc->slot_info[i].successful_boot);
                        continue;
                }
                log_debug("ANDROID: bootable slot %d pri: %d, tries: %d, ",
--
2.53.0


________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.

________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.

Re: [PATCH] android_ab: fix slot selection

[Mattijs Korpershoek]

Hi Colin,

Thank you for the patch.
Sorry for the late review. For some reason I have not been cc'ed so I
missed this.

Please use ./scripts/get_maintainer.pl -f boot/android_ab.c next time
you send a patch.
See: https://docs.u-boot.org/en/latest/CONTRIBUTE.html#contributions

On Fri, Mar 13, 2026 at 15:14, Colin Pinnell McAllister <colin.mcallister@garmin.com> wrote:

> The boot selection rules state that a slot is bootable if it is not
> corrupted and either has tries remaining or has already booted
> successfully. However, slots that have tries_remaining == 0 and
> successful_boot == 1 will be disregarded when picking the slot to
> attempt.

Can you please give me more references to the boot selection rules?
Is this just the code comment that's mentioned (boot/android_ab.c:276) ?

I've checked:
* https://source.android.com/docs/core/ota/ab
* https://source.android.com/docs/core/ota/ab/ab_implement
and it's unclear to me from that.

Then, I've looked a bit at the default bootcontrol AIDL hal at:
https://android.googlesource.com/platform/hardware/interfaces/+/refs/heads/main/boot/aidl/default/Android.bp

It seems to be using libboot_control as a static library.
libboot_control tells us that a slot is considered unbootable when:
tries_remaining != 0;

See:
https://cs.android.com/android/platform/superproject/+/android-latest-release:hardware/interfaces/boot/1.1/default/boot_control/libboot_control.cpp;l=332?q=libboot_control

>
> Update the selection logic so slots marked successful remain eligible
> even when their tries counter is zero. Also include the successful_boot
> state in the unbootable-slot debug output.

What happens if the tries counter is zero and we attempt to boot with
dec_tries=true? In that case, we will decrement tries_remaining which is
an uint8_t (see: android_bootloader_message.h).

>
> Signed-off-by: Colin Pinnell McAllister <colin.mcallister@garmin.com>
> ---
>
> An alternative would be to keep the existing behavior and update the
> comment in boot/android_ab.c:276 to remove the successful_boot
> condition. I do prefer updating the logic to match the documented rules.
> Currently, the userspace AB code will need to increment the tries
> counter for a slot that has booted successfully but has no tries left.
>
>  boot/android_ab.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/boot/android_ab.c b/boot/android_ab.c
> index 13e82dbcb7f..93e74f81d4b 100644
> --- a/boot/android_ab.c
> +++ b/boot/android_ab.c
> @@ -289,11 +289,14 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
>         slot = -1;
>         for (i = 0; i < abc->nb_slot; ++i) {
>                 if (abc->slot_info[i].verity_corrupted ||
> -                   !abc->slot_info[i].tries_remaining) {
> +                   (!abc->slot_info[i].tries_remaining &&
> +                        !abc->slot_info[i].successful_boot)) {
>                         log_debug("ANDROID: unbootable slot %d tries: %d, ",
>                                   i, abc->slot_info[i].tries_remaining);
> -                       log_debug("corrupt: %d\n",
> +                       log_debug("corrupt: %d, ",
>                                   abc->slot_info[i].verity_corrupted);
> +                       log_debug("successful: %d\n",
> +                                 abc->slot_info[i].successful_boot);
>                         continue;
>                 }
>                 log_debug("ANDROID: bootable slot %d pri: %d, tries: %d, ",
> --
> 2.53.0
>
>
> ________________________________
>
> CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.
>
> ________________________________
>
> CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.

Re: [PATCH] android_ab: fix slot selection

[Pinnell McAllister, Colin]

Hi Mattijs,

Thank you for the response! This wasn't a critical change, so no worries about the delay.

Outlook is kind of terrible for replying inline...I should have used my gmail account for this patch. My responses to you message are below.

> Please use ./scripts/get_maintainer.pl -f boot/android_ab.c next time you send a patch.

ack

> Can you please give me more references to the boot selection rules?
> Is this just the code comment that's mentioned (boot/android_ab.c:276) ?

Yes, that is correct. I do not have any other references to cite.

> It seems to be using libboot_control as a static library.
> libboot_control tells us that a slot is considered unbootable when:
> tries_remaining != 0;

I also see that tries_remaining is set to 1 on successful boot:
https://cs.android.com/android/platform/superproject/+/android-latest-release:hardware/interfaces/boot/1.1/default/boot_control/libboot_control.cpp;l=260?q=libboot_control

This is what we also currently do when marking a boot as successful. However, it is unclear if this is a requirement in the spec and what the behavior in u-boot should be if successful_boot == 1 and tries_remaining == 0.

> What happens if the tries counter is zero and we attempt to boot with
> dec_tries=true? In that case, we will decrement tries_remaining which is
> an uint8_t (see: android_bootloader_message.h).

As I read the code, dec_tries only decrements tries_remaining if successful_boot == 0. So in the case where dec_tries == true, tries_remaining == 0, and successful_boot == 1, the decrement would be skipped, which aligns with the behavior for a case where tries_remaining > 0.

I'm not too picky with a solution here. A valid alternative solution would be to simply update the comment at android_ab.c:276. I do think I prefer aligning the code to match the comment as it makes more sense to be able to boot a slot when successful_boot == 1, regardless of tries_remaining. However, if that goes against the spec, simply fixing the comment makes more sense.

Please let me know what you think and what solution is preferred.

Best,
Colin

________________________________________
From: Mattijs Korpershoek <mkorpershoek@kernel.org>
Sent: Thursday, April 9, 2026 05:21
To: Pinnell McAllister, Colin <Colin.McAllister@garmin.com>; u-boot@lists.denx.de <u-boot@lists.denx.de>
Cc: Pinnell McAllister, Colin <Colin.McAllister@garmin.com>
Subject: Re: [PATCH] android_ab: fix slot selection

Hi Colin,

Thank you for the patch.
Sorry for the late review. For some reason I have not been cc'ed so I
missed this.

Please use ./scripts/get_maintainer.pl -f boot/android_ab.c next time
you send a patch.
See: https://urldefense.com/v3/__https://docs.u-boot.org/en/latest/CONTRIBUTE.html*contributions__;Iw!!EJc4YC3iFmQ!Tlm_0_94q_aNcwZ072h7OjCsbyXAaN6bx5DgRMsDkFUXhf8ofhAJ1slRLYnu1EpkNnuaZHqRBDVgEpNOTGCclGX-d_1n$

On Fri, Mar 13, 2026 at 15:14, Colin Pinnell McAllister <colin.mcallister@garmin.com> wrote:

> The boot selection rules state that a slot is bootable if it is not
> corrupted and either has tries remaining or has already booted
> successfully. However, slots that have tries_remaining == 0 and
> successful_boot == 1 will be disregarded when picking the slot to
> attempt.

Can you please give me more references to the boot selection rules?
Is this just the code comment that's mentioned (boot/android_ab.c:276) ?

I've checked:
* https://urldefense.com/v3/__https://source.android.com/docs/core/ota/ab__;!!EJc4YC3iFmQ!Tlm_0_94q_aNcwZ072h7OjCsbyXAaN6bx5DgRMsDkFUXhf8ofhAJ1slRLYnu1EpkNnuaZHqRBDVgEpNOTGCclOK45V17$
* https://urldefense.com/v3/__https://source.android.com/docs/core/ota/ab/ab_implement__;!!EJc4YC3iFmQ!Tlm_0_94q_aNcwZ072h7OjCsbyXAaN6bx5DgRMsDkFUXhf8ofhAJ1slRLYnu1EpkNnuaZHqRBDVgEpNOTGCclDWZu3M8$
and it's unclear to me from that.

Then, I've looked a bit at the default bootcontrol AIDL hal at:
https://urldefense.com/v3/__https://android.googlesource.com/platform/hardware/interfaces/*/refs/heads/main/boot/aidl/default/Android.bp__;Kw!!EJc4YC3iFmQ!Tlm_0_94q_aNcwZ072h7OjCsbyXAaN6bx5DgRMsDkFUXhf8ofhAJ1slRLYnu1EpkNnuaZHqRBDVgEpNOTGCclARJQAuu$

It seems to be using libboot_control as a static library.
libboot_control tells us that a slot is considered unbootable when:
tries_remaining != 0;

See:
https://urldefense.com/v3/__https://cs.android.com/android/platform/superproject/*/android-latest-release:hardware/interfaces/boot/1.1/default/boot_control/libboot_control.cpp;l=332?q=libboot_control__;Kw!!EJc4YC3iFmQ!Tlm_0_94q_aNcwZ072h7OjCsbyXAaN6bx5DgRMsDkFUXhf8ofhAJ1slRLYnu1EpkNnuaZHqRBDVgEpNOTGCclOsUzpyr$

>
> Update the selection logic so slots marked successful remain eligible
> even when their tries counter is zero. Also include the successful_boot
> state in the unbootable-slot debug output.

What happens if the tries counter is zero and we attempt to boot with
dec_tries=true? In that case, we will decrement tries_remaining which is
an uint8_t (see: android_bootloader_message.h).

>
> Signed-off-by: Colin Pinnell McAllister <colin.mcallister@garmin.com>
> ---
>
> An alternative would be to keep the existing behavior and update the
> comment in boot/android_ab.c:276 to remove the successful_boot
> condition. I do prefer updating the logic to match the documented rules.
> Currently, the userspace AB code will need to increment the tries
> counter for a slot that has booted successfully but has no tries left.
>
>  boot/android_ab.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/boot/android_ab.c b/boot/android_ab.c
> index 13e82dbcb7f..93e74f81d4b 100644
> --- a/boot/android_ab.c
> +++ b/boot/android_ab.c
> @@ -289,11 +289,14 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
>         slot = -1;
>         for (i = 0; i < abc->nb_slot; ++i) {
>                 if (abc->slot_info[i].verity_corrupted ||
> -                   !abc->slot_info[i].tries_remaining) {
> +                   (!abc->slot_info[i].tries_remaining &&
> +                        !abc->slot_info[i].successful_boot)) {
>                         log_debug("ANDROID: unbootable slot %d tries: %d, ",
>                                   i, abc->slot_info[i].tries_remaining);
> -                       log_debug("corrupt: %d\n",
> +                       log_debug("corrupt: %d, ",
>                                   abc->slot_info[i].verity_corrupted);
> +                       log_debug("successful: %d\n",
> +                                 abc->slot_info[i].successful_boot);
>                         continue;
>                 }
>                 log_debug("ANDROID: bootable slot %d pri: %d, tries: %d, ",
> --
> 2.53.0
>
>
> ________________________________
>
> CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.
>
> ________________________________
>
> CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.

________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.