On Sun, Jul 28, 2024 at 01:36:04PM -0600, Simon Glass wrote:
> Hi Dmitry,
>
> On Thu, 25 Jul 2024 at 14:55, Dmitry Rokosov
> <ddroko...@salutedevices.com> wrote:
> >
> > It's really helpful to have the ability to dump BCB block for debugging
> > A/B logic on the board supported this partition schema.
> >
> > Command 'ab_dump' prints all fields of bootloader_control struct
> > including slot_metadata for all presented slots.
> >
> > Output example:
> > =====
> > > board# ab_dump ubi 0#misc
> > > Read 512 bytes from volume misc to 000000000bf51900
> > > Bootloader Control: [misc]
> > > Active Slot: _a
> > > Magic Number: 0x42414342
> > > Version: 1
> > > Number of Slots: 2
> > > Recovery Tries Remaining: 7
> > > CRC: 0x61378F6F (Valid)
> > >
> > > Slot[0] Metadata:
> > > - Priority: 15
> > > - Tries Remaining: 4
> > > - Successful Boot: 0
> > > - Verity Corrupted: 0
> > >
> > > Slot[1] Metadata:
> > > - Priority: 15
> > > - Tries Remaining: 5
> > > - Successful Boot: 0
> > > - Verity Corrupted: 0
> > ====
> >
> > Signed-off-by: Dmitry Rokosov <ddroko...@salutedevices.com>
> > ---
> > boot/android_ab.c | 68 ++++++++++++++++++++++++++++++++++++++++++++
> > cmd/ab_select.c | 30 +++++++++++++++++++
> > include/android_ab.h | 9 ++++++
> > 3 files changed, 107 insertions(+)
> >
> > diff --git a/boot/android_ab.c b/boot/android_ab.c
> > index 1e5aa81b7503..359cc1a00428 100644
> > --- a/boot/android_ab.c
> > +++ b/boot/android_ab.c
> > @@ -363,3 +363,71 @@ int ab_select_slot(struct blk_desc *dev_desc, struct
> > disk_partition *part_info,
> >
> > return slot;
> > }
> > +
> > +int ab_dump_abc(struct blk_desc *dev_desc, struct disk_partition
> > *part_info)
> > +{
> > + struct bootloader_control *abc;
> > + u32 crc32_le;
> > + int i, ret;
> > + struct slot_metadata *slot;
> > +
> > + if (!dev_desc || !part_info) {
> > + log_err("ANDROID: Empty device descriptor or partition
> > info\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = ab_control_create_from_disk(dev_desc, part_info, &abc, 0);
> > + if (ret < 0) {
> > + log_err("ANDROID: Cannot create bcb from disk %d\n", ret);
> > + return ret;
> > + }
> > +
> > + if (abc->magic != BOOT_CTRL_MAGIC) {
> > + log_err("ANDROID: Unknown A/B metadata: %.8x\n",
> > abc->magic);
> > + ret = -ENODATA;
> > + goto error;
> > + }
> > +
> > + if (abc->version > BOOT_CTRL_VERSION) {
> > + log_err("ANDROID: Unsupported A/B metadata version: %.8x\n",
> > + abc->version);
> > + ret = -ENODATA;
> > + goto error;
> > + }
> > +
> > + if (abc->nb_slot > ARRAY_SIZE(abc->slot_info)) {
> > + log_err("ANDROID: Wrong number of slots %u, expected %zu\n",
> > + abc->nb_slot, ARRAY_SIZE(abc->slot_info));
> > + ret = -ENODATA;
> > + goto error;
> > + }
> > +
> > + printf("Bootloader Control: \t[%s]\n", part_info->name);
> > + printf("Active Slot: \t\t%s\n", abc->slot_suffix);
> > + printf("Magic Number: \t\t0x%X\n", abc->magic);
> > + printf("Version: \t\t%u\n", abc->version);
> > + printf("Number of Slots: \t%u\n", abc->nb_slot);
> > + printf("Recovery Tries Remaining: %u\n",
> > abc->recovery_tries_remaining);
> > +
> > + printf("CRC: \t\t\t0x%.8X", abc->crc32_le);
> > +
> > + crc32_le = ab_control_compute_crc(abc);
> > + if (abc->crc32_le != crc32_le)
> > + printf(" (Invalid, Expected: \t0x%.8X)\n", crc32_le);
> > + else
> > + printf(" (Valid)\n");
> > +
> > + for (i = 0; i < abc->nb_slot; ++i) {
> > + slot = &abc->slot_info[i];
> > + printf("\nSlot[%d] Metadata:\n", i);
> > + printf("\t- Priority: \t\t%u\n", slot->priority);
> > + printf("\t- Tries Remaining: \t%u\n",
> > slot->tries_remaining);
> > + printf("\t- Successful Boot: \t%u\n",
> > slot->successful_boot);
> > + printf("\t- Verity Corrupted: \t%u\n",
> > slot->verity_corrupted);
> > + }
> > +
> > +error:
> > + free(abc);
> > +
> > + return ret;
> > +}
> > diff --git a/cmd/ab_select.c b/cmd/ab_select.c
> > index 9e2f74573c22..1d34150ceea9 100644
> > --- a/cmd/ab_select.c
> > +++ b/cmd/ab_select.c
> > @@ -51,6 +51,31 @@ static int do_ab_select(struct cmd_tbl *cmdtp, int flag,
> > int argc,
> > return CMD_RET_SUCCESS;
> > }
> >
> > +static int do_ab_dump(struct cmd_tbl *cmdtp, int flag, int argc,
> > + char *const argv[])
> > +{
> > + int ret;
> > + struct blk_desc *dev_desc;
> > + struct disk_partition part_info;
> > +
> > + if (argc < 3)
> > + return CMD_RET_USAGE;
> > +
> > + if (part_get_info_by_dev_and_name_or_num(argv[1], argv[2],
> > + &dev_desc, &part_info,
> > + false) < 0) {
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + ret = ab_dump_abc(dev_desc, &part_info);
> > + if (ret < 0) {
> > + printf("Cannot dump ABC data, error %d.\n", ret);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + return CMD_RET_SUCCESS;
> > +}
> > +
> > U_BOOT_CMD(ab_select, 5, 0, do_ab_select,
> > "Select the slot used to boot from and register the boot
> > attempt.",
> > "<slot_var_name> <interface> <dev[:part|#part_name]>
> > [--no-dec]\n"
> > @@ -66,3 +91,8 @@ U_BOOT_CMD(ab_select, 5, 0, do_ab_select,
> > " - If '--no-dec' is set, the number of tries remaining will
> > not\n"
> > " decremented for the selected boot slot\n"
> > );
> > +
> > +U_BOOT_CMD(ab_dump, 3, 0, do_ab_dump,
> > + "Dump boot_control information from specific partition.",
> > + "<interface> <dev[:part|#part_name]>\n"
> > +);
> > diff --git a/include/android_ab.h b/include/android_ab.h
> > index 1fee7582b90a..e53bf7eb6a02 100644
> > --- a/include/android_ab.h
> > +++ b/include/android_ab.h
> > @@ -33,4 +33,13 @@ struct disk_partition;
> > int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition
> > *part_info,
> > bool dec_tries);
> >
> > +/**
> > + * Dump ABC information for specific partition.
> > + *
> > + * @param[in] dev_desc Device description pointer
> > + * @param[in] part_info Partition information
>
> We have moved to the @ notation now:
>
> @dev_desc: ...
>
> > + * Return: 0 on success, or a negative on error
> > + */
> > +int ab_dump_abc(struct blk_desc *dev_desc, struct disk_partition
> > *part_info);
> > +
> > #endif /* __ANDROID_AB_H */
> > --
> > 2.43.0
> >
>
> Rather than creating a new command I think this should be a subcommand
> of abootimg.
>
> Can you please create some docs in doc/usage/cmd/abootimg for the command?
>
> I also wonder if ab_select should move under that command?
I believe it's an excellent idea to utilize abootimg for modifying and
dumping Android related stuff, as this tool is specifically designed for
such purposes. Moreover, the A/B approach is predominantly an
Android-based feature.
--
Thank you,
Dmitry
