| Message ID | 20240725194716.32232-5-ddrokosov@salutedevices.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Mattijs Korpershoek |
| Headers | show |
| Series | android_ab: fix slot_suffix issue and introduce ab_dump command | expand |
Hi Dmitry, On Thu, 25 Jul 2024 at 14:55, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote: > > To align with the official Android BCB (Boot Control Block) > specifications, it's important to note that the slot_suffix should start > with an underscore symbol. > > For a comprehensive understanding of the expected slot_suffix format in > userspace, please refer to the provided reference [1]. > > Links: > [1] - https://source.android.com/docs/core/architecture/bootloader/updating#slots > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > --- > boot/android_ab.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> (can the test cover this?) > > diff --git a/boot/android_ab.c b/boot/android_ab.c > index 359cc1a00428..45c154b10f1a 100644 > --- a/boot/android_ab.c > +++ b/boot/android_ab.c > @@ -53,7 +53,7 @@ static int ab_control_default(struct bootloader_control *abc) > if (!abc) > return -EFAULT; > > - memcpy(abc->slot_suffix, "a\0\0\0", 4); > + memcpy(abc->slot_suffix, "_a\0\0", 4); > abc->magic = BOOT_CTRL_MAGIC; > abc->version = BOOT_CTRL_VERSION; > abc->nb_slot = NUM_SLOTS; > @@ -319,7 +319,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, > * or the device tree. > */ > memset(slot_suffix, 0, sizeof(slot_suffix)); > - slot_suffix[0] = BOOT_SLOT_NAME(slot); > + slot_suffix[0] = '_'; > + slot_suffix[1] = BOOT_SLOT_NAME(slot); > if (memcmp(abc->slot_suffix, slot_suffix, > sizeof(slot_suffix))) { > memcpy(abc->slot_suffix, slot_suffix, > -- > 2.43.0 >
On Sun, Jul 28, 2024 at 01:36:06PM -0600, Simon Glass wrote: > Hi Dmitry, > > On Thu, 25 Jul 2024 at 14:55, Dmitry Rokosov > <ddrokosov@salutedevices.com> wrote: > > > > To align with the official Android BCB (Boot Control Block) > > specifications, it's important to note that the slot_suffix should start > > with an underscore symbol. > > > > For a comprehensive understanding of the expected slot_suffix format in > > userspace, please refer to the provided reference [1]. > > > > Links: > > [1] - https://source.android.com/docs/core/architecture/bootloader/updating#slots > > > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > > --- > > boot/android_ab.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > (can the test cover this?) I understand your point. I will try to think about the test that could potentially uncover this problem. Will send it in the next version. > > > > diff --git a/boot/android_ab.c b/boot/android_ab.c > > index 359cc1a00428..45c154b10f1a 100644 > > --- a/boot/android_ab.c > > +++ b/boot/android_ab.c > > @@ -53,7 +53,7 @@ static int ab_control_default(struct bootloader_control *abc) > > if (!abc) > > return -EFAULT; > > > > - memcpy(abc->slot_suffix, "a\0\0\0", 4); > > + memcpy(abc->slot_suffix, "_a\0\0", 4); > > abc->magic = BOOT_CTRL_MAGIC; > > abc->version = BOOT_CTRL_VERSION; > > abc->nb_slot = NUM_SLOTS; > > @@ -319,7 +319,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, > > * or the device tree. > > */ > > memset(slot_suffix, 0, sizeof(slot_suffix)); > > - slot_suffix[0] = BOOT_SLOT_NAME(slot); > > + slot_suffix[0] = '_'; > > + slot_suffix[1] = BOOT_SLOT_NAME(slot); > > if (memcmp(abc->slot_suffix, slot_suffix, > > sizeof(slot_suffix))) { > > memcpy(abc->slot_suffix, slot_suffix, > > -- > > 2.43.0 > >
Hi Dmitry, Thank you for the patch. On jeu., juil. 25, 2024 at 22:47, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote: > To align with the official Android BCB (Boot Control Block) > specifications, it's important to note that the slot_suffix should start > with an underscore symbol. > > For a comprehensive understanding of the expected slot_suffix format in > userspace, please refer to the provided reference [1]. > > Links: > [1] - https://source.android.com/docs/core/architecture/bootloader/updating#slots > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> Can we point to the AOSP change on which this is based in the commit message? https://android-review.googlesource.com/c/platform/external/u-boot/+/1446439 With that added: Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> > --- > boot/android_ab.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/boot/android_ab.c b/boot/android_ab.c > index 359cc1a00428..45c154b10f1a 100644 > --- a/boot/android_ab.c > +++ b/boot/android_ab.c > @@ -53,7 +53,7 @@ static int ab_control_default(struct bootloader_control *abc) > if (!abc) > return -EFAULT; > > - memcpy(abc->slot_suffix, "a\0\0\0", 4); > + memcpy(abc->slot_suffix, "_a\0\0", 4); > abc->magic = BOOT_CTRL_MAGIC; > abc->version = BOOT_CTRL_VERSION; > abc->nb_slot = NUM_SLOTS; > @@ -319,7 +319,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, > * or the device tree. > */ > memset(slot_suffix, 0, sizeof(slot_suffix)); > - slot_suffix[0] = BOOT_SLOT_NAME(slot); > + slot_suffix[0] = '_'; > + slot_suffix[1] = BOOT_SLOT_NAME(slot); > if (memcmp(abc->slot_suffix, slot_suffix, > sizeof(slot_suffix))) { > memcpy(abc->slot_suffix, slot_suffix, > -- > 2.43.0
On Tue, Jul 30, 2024 at 10:43:13AM +0200, Mattijs Korpershoek wrote: > Hi Dmitry, > > Thank you for the patch. > > On jeu., juil. 25, 2024 at 22:47, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote: > > > To align with the official Android BCB (Boot Control Block) > > specifications, it's important to note that the slot_suffix should start > > with an underscore symbol. > > > > For a comprehensive understanding of the expected slot_suffix format in > > userspace, please refer to the provided reference [1]. > > > > Links: > > [1] - https://source.android.com/docs/core/architecture/bootloader/updating#slots > > > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > > Can we point to the AOSP change on which this is based in the commit message? > > https://android-review.googlesource.com/c/platform/external/u-boot/+/1446439 > > With that added: > > Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> > Sure, no problem. I will add the reference in the next version. > > --- > > boot/android_ab.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/boot/android_ab.c b/boot/android_ab.c > > index 359cc1a00428..45c154b10f1a 100644 > > --- a/boot/android_ab.c > > +++ b/boot/android_ab.c > > @@ -53,7 +53,7 @@ static int ab_control_default(struct bootloader_control *abc) > > if (!abc) > > return -EFAULT; > > > > - memcpy(abc->slot_suffix, "a\0\0\0", 4); > > + memcpy(abc->slot_suffix, "_a\0\0", 4); > > abc->magic = BOOT_CTRL_MAGIC; > > abc->version = BOOT_CTRL_VERSION; > > abc->nb_slot = NUM_SLOTS; > > @@ -319,7 +319,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, > > * or the device tree. > > */ > > memset(slot_suffix, 0, sizeof(slot_suffix)); > > - slot_suffix[0] = BOOT_SLOT_NAME(slot); > > + slot_suffix[0] = '_'; > > + slot_suffix[1] = BOOT_SLOT_NAME(slot); > > if (memcmp(abc->slot_suffix, slot_suffix, > > sizeof(slot_suffix))) { > > memcpy(abc->slot_suffix, slot_suffix, > > -- > > 2.43.0
diff --git a/boot/android_ab.c b/boot/android_ab.c index 359cc1a00428..45c154b10f1a 100644 --- a/boot/android_ab.c +++ b/boot/android_ab.c @@ -53,7 +53,7 @@ static int ab_control_default(struct bootloader_control *abc) if (!abc) return -EFAULT; - memcpy(abc->slot_suffix, "a\0\0\0", 4); + memcpy(abc->slot_suffix, "_a\0\0", 4); abc->magic = BOOT_CTRL_MAGIC; abc->version = BOOT_CTRL_VERSION; abc->nb_slot = NUM_SLOTS; @@ -319,7 +319,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, * or the device tree. */ memset(slot_suffix, 0, sizeof(slot_suffix)); - slot_suffix[0] = BOOT_SLOT_NAME(slot); + slot_suffix[0] = '_'; + slot_suffix[1] = BOOT_SLOT_NAME(slot); if (memcmp(abc->slot_suffix, slot_suffix, sizeof(slot_suffix))) { memcpy(abc->slot_suffix, slot_suffix,
To align with the official Android BCB (Boot Control Block) specifications, it's important to note that the slot_suffix should start with an underscore symbol. For a comprehensive understanding of the expected slot_suffix format in userspace, please refer to the provided reference [1]. Links: [1] - https://source.android.com/docs/core/architecture/bootloader/updating#slots Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> --- boot/android_ab.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)