* [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream
@ 2024-02-19 10:15 Igor Opaniuk
2024-02-20 18:29 ` Sam Protsenko
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Igor Opaniuk @ 2024-02-19 10:15 UTC (permalink / raw)
To: u-boot
Cc: Eugeniu Rosca, Tom Rini, Sam Protsenko, Simon Glass, Alex Deymo,
Igor Opaniuk
This takes the latest changes from AOSP from [1][2] (as this
header was split on two) with minimal changes (this could lead
to warnings reported by checkpatch).
Some local changes have been applied:
1. Enable static_assert() for defined structures to be sure
that all of them have correct sizes.
2. Adjuste types in bootloader_control structure with bitfields
(uint8_t -> uint16_t). It seems that gcc just doesn't like bitfields
that cross the width of the type. Changing the type doesn't change
the layout though.
This addresses this gcc note:
In file included from boot/android_ab.c:7:
include/android_bootloader_message.h:230:1: note: offset of packed bit-field ‘merge_status’ has changed in GCC 4.4
230 | } __attribute__((packed));
[1] https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h
[2] https://android.googlesource.com/platform/hardware/interfaces/+/main/boot/1.1/default/boot_control/include/private/boot_control_definition.h
CC: Alex Deymo <deymo@google.com>
CC: Sam Protsenko <semen.protsenko@linaro.org>
CC: Eugeniu Rosca <erosca@de.adit-jv.com>
CC: Simon Glass <sjg@chromium.org>
Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>
---
include/android_bootloader_message.h | 104 +++++++++++++++++++++++----
1 file changed, 92 insertions(+), 12 deletions(-)
diff --git a/include/android_bootloader_message.h b/include/android_bootloader_message.h
index 286d7ab0f31..75198fc9dc2 100644
--- a/include/android_bootloader_message.h
+++ b/include/android_bootloader_message.h
@@ -21,17 +21,22 @@
* stddef.h
*/
#include <compiler.h>
+#include <linux/build_bug.h>
#endif
// Spaces used by misc partition are as below:
// 0 - 2K For bootloader_message
// 2K - 16K Used by Vendor's bootloader (the 2K - 4K range may be optionally used
// as bootloader_message_ab struct)
-// 16K - 64K Used by uncrypt and recovery to store wipe_package for A/B devices
+// 16K - 32K Used by uncrypt and recovery to store wipe_package for A/B devices
+// 32K - 64K System space, used for miscellanious AOSP features. See below.
// Note that these offsets are admitted by bootloader,recovery and uncrypt, so they
// are not configurable without changing all of them.
static const size_t BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0;
+static const size_t VENDOR_SPACE_OFFSET_IN_MISC = 2 * 1024;
static const size_t WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;
+static const size_t SYSTEM_SPACE_OFFSET_IN_MISC = 32 * 1024;
+static const size_t SYSTEM_SPACE_SIZE_IN_MISC = 32 * 1024;
/* Bootloader Message (2-KiB)
*
@@ -81,24 +86,67 @@ struct bootloader_message {
char reserved[1184];
};
+// Holds Virtual A/B merge status information. Current version is 1. New fields
+// must be added to the end.
+struct misc_virtual_ab_message {
+ uint8_t version;
+ uint32_t magic;
+ uint8_t merge_status; // IBootControl 1.1, MergeStatus enum.
+ uint8_t source_slot; // Slot number when merge_status was written.
+ uint8_t reserved[57];
+} __attribute__((packed));
+
+struct misc_memtag_message {
+ uint8_t version;
+ uint32_t magic; // magic string for treble compat
+ uint32_t memtag_mode;
+ uint8_t reserved[55];
+} __attribute__((packed));
+
+struct misc_kcmdline_message {
+ uint8_t version;
+ uint32_t magic;
+ uint64_t kcmdline_flags;
+ uint8_t reserved[51];
+} __attribute__((packed));
+
+#define MISC_VIRTUAL_AB_MESSAGE_VERSION 2
+#define MISC_VIRTUAL_AB_MAGIC_HEADER 0x56740AB0
+
+#define MISC_MEMTAG_MESSAGE_VERSION 1
+#define MISC_MEMTAG_MAGIC_HEADER 0x5afefe5a
+#define MISC_MEMTAG_MODE_MEMTAG 0x1
+#define MISC_MEMTAG_MODE_MEMTAG_ONCE 0x2
+#define MISC_MEMTAG_MODE_MEMTAG_KERNEL 0x4
+#define MISC_MEMTAG_MODE_MEMTAG_KERNEL_ONCE 0x8
+#define MISC_MEMTAG_MODE_MEMTAG_OFF 0x10
+// This is set when the state was overridden forcibly. This does not need to be
+// interpreted by the bootloader but is only for bookkeeping purposes so
+// userspace knows what to do when the override is undone.
+// See system/extras/mtectrl in AOSP for more information.
+#define MISC_MEMTAG_MODE_FORCED 0x20
+
+#define MISC_KCMDLINE_MESSAGE_VERSION 1
+#define MISC_KCMDLINE_MAGIC_HEADER 0x6ab5110c
+#define MISC_KCMDLINE_BINDER_RUST 0x1
/**
* We must be cautious when changing the bootloader_message struct size,
* because A/B-specific fields may end up with different offsets.
*/
-#ifndef __UBOOT__
+
#if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
static_assert(sizeof(struct bootloader_message) == 2048,
"struct bootloader_message size changes, which may break A/B devices");
#endif
-#endif /* __UBOOT__ */
/**
* The A/B-specific bootloader message structure (4-KiB).
*
* We separate A/B boot control metadata from the regular bootloader
* message struct and keep it here. Everything that's A/B-specific
- * stays after struct bootloader_message, which should be managed by
- * the A/B-bootloader or boot control HAL.
+ * stays after struct bootloader_message, which belongs to the vendor
+ * space of /misc partition. Also, the A/B-specific contents should be
+ * managed by the A/B-bootloader or boot control HAL.
*
* The slot_suffix field is used for A/B implementations where the
* bootloader does not set the androidboot.ro.boot.slot_suffix kernel
@@ -124,12 +172,10 @@ struct bootloader_message_ab {
* Be cautious about the struct size change, in case we put anything post
* bootloader_message_ab struct (b/29159185).
*/
-#ifndef __UBOOT__
#if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
static_assert(sizeof(struct bootloader_message_ab) == 4096,
"struct bootloader_message_ab size changes");
#endif
-#endif /* __UBOOT__ */
#define BOOT_CTRL_MAGIC 0x42414342 /* Bootloader Control AB */
#define BOOT_CTRL_VERSION 1
@@ -165,11 +211,13 @@ struct bootloader_control {
// Version of struct being used (see BOOT_CTRL_VERSION).
uint8_t version;
// Number of slots being managed.
- uint8_t nb_slot : 3;
+ uint16_t nb_slot : 3;
// Number of times left attempting to boot recovery.
- uint8_t recovery_tries_remaining : 3;
+ uint16_t recovery_tries_remaining : 3;
+ // Status of any pending snapshot merge of dynamic partitions.
+ uint16_t merge_status : 3;
// Ensure 4-bytes alignment for slot_info field.
- uint8_t reserved0[2];
+ uint8_t reserved0[1];
// Per-slot information. Up to 4 slots.
struct slot_metadata slot_info[4];
// Reserved for further use.
@@ -179,25 +227,46 @@ struct bootloader_control {
uint32_t crc32_le;
} __attribute__((packed));
-#ifndef __UBOOT__
#if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
static_assert(sizeof(struct bootloader_control) ==
sizeof(((struct bootloader_message_ab *)0)->slot_suffix),
"struct bootloader_control has wrong size");
+static_assert(sizeof(struct misc_virtual_ab_message) == 64,
+ "struct misc_virtual_ab_message has wrong size");
+static_assert(sizeof(struct misc_memtag_message) == 64,
+ "struct misc_memtag_message has wrong size");
+static_assert(sizeof(struct misc_kcmdline_message) == 64,
+ "struct misc_kcmdline_message has wrong size");
#endif
-#endif /* __UBOOT__ */
#ifndef __UBOOT__
+
+// This struct is not meant to be used directly, rather, it is to make
+// computation of offsets easier. New fields must be added to the end.
+struct misc_system_space_layout {
+ misc_virtual_ab_message virtual_ab_message;
+ misc_memtag_message memtag_message;
+ misc_kcmdline_message kcmdline_message;
+} __attribute__((packed));
+
#ifdef __cplusplus
#include <string>
#include <vector>
+// Gets the block device name of /misc partition.
+std::string get_misc_blk_device(std::string* err);
+
// Return the block device name for the bootloader message partition and waits
// for the device for up to 10 seconds. In case of error returns the empty
// string.
std::string get_bootloader_message_blk_device(std::string* err);
+// Writes |size| bytes of data from buffer |p| to |misc_blk_device| at |offset|. If the write fails,
+// sets the error message in |err|.
+bool write_misc_partition(const void* p, size_t size, const std::string& misc_blk_device,
+ size_t offset, std::string* err);
+
// Read bootloader message into boot. Error message will be set in err.
bool read_bootloader_message(bootloader_message* boot, std::string* err);
@@ -242,6 +311,17 @@ bool read_wipe_package(std::string* package_data, size_t size, std::string* err)
// Write the wipe package into BCB (to offset WIPE_PACKAGE_OFFSET_IN_MISC).
bool write_wipe_package(const std::string& package_data, std::string* err);
+// Read or write the Virtual A/B message from system space in /misc.
+bool ReadMiscVirtualAbMessage(misc_virtual_ab_message* message, std::string* err);
+bool WriteMiscVirtualAbMessage(const misc_virtual_ab_message& message, std::string* err);
+
+// Read or write the memtag message from system space in /misc.
+bool ReadMiscMemtagMessage(misc_memtag_message* message, std::string* err);
+bool WriteMiscMemtagMessage(const misc_memtag_message& message, std::string* err);
+
+// Read or write the kcmdline message from system space in /misc.
+bool ReadMiscKcmdlineMessage(misc_kcmdline_message* message, std::string* err);
+bool WriteMiscKcmdlineMessage(const misc_kcmdline_message& message, std::string* err);
#else
#include <stdbool.h>
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream
2024-02-19 10:15 [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream Igor Opaniuk
@ 2024-02-20 18:29 ` Sam Protsenko
2024-02-20 19:08 ` Igor Opaniuk
2024-02-29 19:05 ` Igor Opaniuk
2024-03-07 9:29 ` Mattijs Korpershoek
2 siblings, 1 reply; 6+ messages in thread
From: Sam Protsenko @ 2024-02-20 18:29 UTC (permalink / raw)
To: Igor Opaniuk; +Cc: u-boot, Eugeniu Rosca, Tom Rini, Simon Glass, Alex Deymo
On Mon, Feb 19, 2024 at 4:16 AM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
>
> This takes the latest changes from AOSP from [1][2] (as this
> header was split on two) with minimal changes (this could lead
> to warnings reported by checkpatch).
Do we want to maybe follow that and also carry two different headers
in U-Boot? Or it doesn't make much sense? I'm thinking in terms of
future portability mostly: how easy it's to update this header right
now, and how easy it's going to be further. I didn't form any opinion
on that, hence asking.
Another thing: are you sure that changing only the header won't break
anything in U-Boot .c files that use this header?
>
> Some local changes have been applied:
Is it possible to split this work into two patches:
1. Bring the original changes only
2. Apply all necessary changes for U-Boot
Or does it break the build, etc? Again, thinking in terms of
portability easiness, and not sure which approach is better -- just
asking basically.
> 1. Enable static_assert() for defined structures to be sure
> that all of them have correct sizes.
> 2. Adjuste types in bootloader_control structure with bitfields
Adjuste -> adjust
> (uint8_t -> uint16_t). It seems that gcc just doesn't like bitfields
I wonder if all those extra changes can be upstreamed back to AOSP?
Ideally we'd want to just copy those headers over from AOSP to U-Boot
with no changes, would make the porting work easier. What are your
thoughts on that?
> that cross the width of the type. Changing the type doesn't change
> the layout though.
> This addresses this gcc note:
> In file included from boot/android_ab.c:7:
> include/android_bootloader_message.h:230:1: note: offset of packed bit-field ‘merge_status’ has changed in GCC 4.4
> 230 | } __attribute__((packed));
>
> [1] https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h
> [2] https://android.googlesource.com/platform/hardware/interfaces/+/main/boot/1.1/default/boot_control/include/private/boot_control_definition.h
>
> CC: Alex Deymo <deymo@google.com>
> CC: Sam Protsenko <semen.protsenko@linaro.org>
> CC: Eugeniu Rosca <erosca@de.adit-jv.com>
> CC: Simon Glass <sjg@chromium.org>
> Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>
> ---
>
> include/android_bootloader_message.h | 104 +++++++++++++++++++++++----
> 1 file changed, 92 insertions(+), 12 deletions(-)
>
> diff --git a/include/android_bootloader_message.h b/include/android_bootloader_message.h
> index 286d7ab0f31..75198fc9dc2 100644
> --- a/include/android_bootloader_message.h
> +++ b/include/android_bootloader_message.h
> @@ -21,17 +21,22 @@
> * stddef.h
> */
> #include <compiler.h>
> +#include <linux/build_bug.h>
> #endif
>
> // Spaces used by misc partition are as below:
> // 0 - 2K For bootloader_message
> // 2K - 16K Used by Vendor's bootloader (the 2K - 4K range may be optionally used
> // as bootloader_message_ab struct)
> -// 16K - 64K Used by uncrypt and recovery to store wipe_package for A/B devices
> +// 16K - 32K Used by uncrypt and recovery to store wipe_package for A/B devices
> +// 32K - 64K System space, used for miscellanious AOSP features. See below.
> // Note that these offsets are admitted by bootloader,recovery and uncrypt, so they
> // are not configurable without changing all of them.
> static const size_t BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0;
> +static const size_t VENDOR_SPACE_OFFSET_IN_MISC = 2 * 1024;
> static const size_t WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;
> +static const size_t SYSTEM_SPACE_OFFSET_IN_MISC = 32 * 1024;
> +static const size_t SYSTEM_SPACE_SIZE_IN_MISC = 32 * 1024;
>
> /* Bootloader Message (2-KiB)
> *
> @@ -81,24 +86,67 @@ struct bootloader_message {
> char reserved[1184];
> };
>
> +// Holds Virtual A/B merge status information. Current version is 1. New fields
> +// must be added to the end.
> +struct misc_virtual_ab_message {
> + uint8_t version;
> + uint32_t magic;
> + uint8_t merge_status; // IBootControl 1.1, MergeStatus enum.
> + uint8_t source_slot; // Slot number when merge_status was written.
> + uint8_t reserved[57];
> +} __attribute__((packed));
> +
> +struct misc_memtag_message {
> + uint8_t version;
> + uint32_t magic; // magic string for treble compat
> + uint32_t memtag_mode;
> + uint8_t reserved[55];
> +} __attribute__((packed));
> +
> +struct misc_kcmdline_message {
> + uint8_t version;
> + uint32_t magic;
> + uint64_t kcmdline_flags;
> + uint8_t reserved[51];
> +} __attribute__((packed));
> +
> +#define MISC_VIRTUAL_AB_MESSAGE_VERSION 2
> +#define MISC_VIRTUAL_AB_MAGIC_HEADER 0x56740AB0
> +
> +#define MISC_MEMTAG_MESSAGE_VERSION 1
> +#define MISC_MEMTAG_MAGIC_HEADER 0x5afefe5a
> +#define MISC_MEMTAG_MODE_MEMTAG 0x1
> +#define MISC_MEMTAG_MODE_MEMTAG_ONCE 0x2
> +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL 0x4
> +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL_ONCE 0x8
> +#define MISC_MEMTAG_MODE_MEMTAG_OFF 0x10
> +// This is set when the state was overridden forcibly. This does not need to be
> +// interpreted by the bootloader but is only for bookkeeping purposes so
> +// userspace knows what to do when the override is undone.
> +// See system/extras/mtectrl in AOSP for more information.
> +#define MISC_MEMTAG_MODE_FORCED 0x20
> +
> +#define MISC_KCMDLINE_MESSAGE_VERSION 1
> +#define MISC_KCMDLINE_MAGIC_HEADER 0x6ab5110c
> +#define MISC_KCMDLINE_BINDER_RUST 0x1
> /**
> * We must be cautious when changing the bootloader_message struct size,
> * because A/B-specific fields may end up with different offsets.
> */
> -#ifndef __UBOOT__
> +
> #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> static_assert(sizeof(struct bootloader_message) == 2048,
> "struct bootloader_message size changes, which may break A/B devices");
> #endif
> -#endif /* __UBOOT__ */
>
> /**
> * The A/B-specific bootloader message structure (4-KiB).
> *
> * We separate A/B boot control metadata from the regular bootloader
> * message struct and keep it here. Everything that's A/B-specific
> - * stays after struct bootloader_message, which should be managed by
> - * the A/B-bootloader or boot control HAL.
> + * stays after struct bootloader_message, which belongs to the vendor
> + * space of /misc partition. Also, the A/B-specific contents should be
> + * managed by the A/B-bootloader or boot control HAL.
> *
> * The slot_suffix field is used for A/B implementations where the
> * bootloader does not set the androidboot.ro.boot.slot_suffix kernel
> @@ -124,12 +172,10 @@ struct bootloader_message_ab {
> * Be cautious about the struct size change, in case we put anything post
> * bootloader_message_ab struct (b/29159185).
> */
> -#ifndef __UBOOT__
> #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> static_assert(sizeof(struct bootloader_message_ab) == 4096,
> "struct bootloader_message_ab size changes");
> #endif
> -#endif /* __UBOOT__ */
>
> #define BOOT_CTRL_MAGIC 0x42414342 /* Bootloader Control AB */
> #define BOOT_CTRL_VERSION 1
> @@ -165,11 +211,13 @@ struct bootloader_control {
> // Version of struct being used (see BOOT_CTRL_VERSION).
> uint8_t version;
> // Number of slots being managed.
> - uint8_t nb_slot : 3;
> + uint16_t nb_slot : 3;
> // Number of times left attempting to boot recovery.
> - uint8_t recovery_tries_remaining : 3;
> + uint16_t recovery_tries_remaining : 3;
> + // Status of any pending snapshot merge of dynamic partitions.
> + uint16_t merge_status : 3;
> // Ensure 4-bytes alignment for slot_info field.
> - uint8_t reserved0[2];
> + uint8_t reserved0[1];
> // Per-slot information. Up to 4 slots.
> struct slot_metadata slot_info[4];
> // Reserved for further use.
> @@ -179,25 +227,46 @@ struct bootloader_control {
> uint32_t crc32_le;
> } __attribute__((packed));
>
> -#ifndef __UBOOT__
> #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> static_assert(sizeof(struct bootloader_control) ==
> sizeof(((struct bootloader_message_ab *)0)->slot_suffix),
> "struct bootloader_control has wrong size");
> +static_assert(sizeof(struct misc_virtual_ab_message) == 64,
> + "struct misc_virtual_ab_message has wrong size");
> +static_assert(sizeof(struct misc_memtag_message) == 64,
> + "struct misc_memtag_message has wrong size");
> +static_assert(sizeof(struct misc_kcmdline_message) == 64,
> + "struct misc_kcmdline_message has wrong size");
> #endif
> -#endif /* __UBOOT__ */
>
> #ifndef __UBOOT__
> +
> +// This struct is not meant to be used directly, rather, it is to make
> +// computation of offsets easier. New fields must be added to the end.
> +struct misc_system_space_layout {
> + misc_virtual_ab_message virtual_ab_message;
> + misc_memtag_message memtag_message;
> + misc_kcmdline_message kcmdline_message;
> +} __attribute__((packed));
> +
> #ifdef __cplusplus
>
> #include <string>
> #include <vector>
>
> +// Gets the block device name of /misc partition.
> +std::string get_misc_blk_device(std::string* err);
> +
> // Return the block device name for the bootloader message partition and waits
> // for the device for up to 10 seconds. In case of error returns the empty
> // string.
> std::string get_bootloader_message_blk_device(std::string* err);
>
> +// Writes |size| bytes of data from buffer |p| to |misc_blk_device| at |offset|. If the write fails,
> +// sets the error message in |err|.
> +bool write_misc_partition(const void* p, size_t size, const std::string& misc_blk_device,
> + size_t offset, std::string* err);
> +
> // Read bootloader message into boot. Error message will be set in err.
> bool read_bootloader_message(bootloader_message* boot, std::string* err);
>
> @@ -242,6 +311,17 @@ bool read_wipe_package(std::string* package_data, size_t size, std::string* err)
> // Write the wipe package into BCB (to offset WIPE_PACKAGE_OFFSET_IN_MISC).
> bool write_wipe_package(const std::string& package_data, std::string* err);
>
> +// Read or write the Virtual A/B message from system space in /misc.
> +bool ReadMiscVirtualAbMessage(misc_virtual_ab_message* message, std::string* err);
> +bool WriteMiscVirtualAbMessage(const misc_virtual_ab_message& message, std::string* err);
> +
> +// Read or write the memtag message from system space in /misc.
> +bool ReadMiscMemtagMessage(misc_memtag_message* message, std::string* err);
> +bool WriteMiscMemtagMessage(const misc_memtag_message& message, std::string* err);
> +
> +// Read or write the kcmdline message from system space in /misc.
> +bool ReadMiscKcmdlineMessage(misc_kcmdline_message* message, std::string* err);
> +bool WriteMiscKcmdlineMessage(const misc_kcmdline_message& message, std::string* err);
AFAIU, these new prototypes brought by this patch don't have actual
implementation yet. Does it affect the build somehow, e.g. maybe
causing some build warnings? Or is the build log clean?
> #else
>
> #include <stdbool.h>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream
2024-02-20 18:29 ` Sam Protsenko
@ 2024-02-20 19:08 ` Igor Opaniuk
2024-03-07 9:32 ` Mattijs Korpershoek
0 siblings, 1 reply; 6+ messages in thread
From: Igor Opaniuk @ 2024-02-20 19:08 UTC (permalink / raw)
To: Sam Protsenko
Cc: Igor Opaniuk, u-boot, Eugeniu Rosca, Tom Rini, Simon Glass, Alex Deymo
Hi Sam,
On Tue, Feb 20, 2024 at 7:29 PM Sam Protsenko
<semen.protsenko@linaro.org> wrote:
>
> On Mon, Feb 19, 2024 at 4:16 AM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
> >
> > This takes the latest changes from AOSP from [1][2] (as this
> > header was split on two) with minimal changes (this could lead
> > to warnings reported by checkpatch).
>
> Do we want to maybe follow that and also carry two different headers
> in U-Boot? Or it doesn't make much sense? I'm thinking in terms of
> future portability mostly: how easy it's to update this header right
> now, and how easy it's going to be further. I didn't form any opinion
> on that, hence asking.
The problem is licensing. android_bootloader_message.h was
re-licensed by Alex Deymo from Google under BSD-3-Clause,
which is GPLv2 compatible. I'm not sure it's legally correct to pull
boot_control_definition.h from AOSP licensed under Apache as a
separate file.
>
> Another thing: are you sure that changing only the header won't break
> anything in U-Boot .c files that use this header?
I've tested both ab_select and avb verify in QEMU. Or do you mean
something else additionally should be tested?
>
> >
> > Some local changes have been applied:
>
> Is it possible to split this work into two patches:
> 1. Bring the original changes only
> 2. Apply all necessary changes for U-Boot
>
> Or does it break the build, etc? Again, thinking in terms of
> portability easiness, and not sure which approach is better -- just
> asking basically.
Yeah, that's the problem, as splitting this on two commits
will lead to the first one reporting warnings/notes.
>
> > 1. Enable static_assert() for defined structures to be sure
> > that all of them have correct sizes.
> > 2. Adjuste types in bootloader_control structure with bitfields
>
> Adjuste -> adjust
Will fix, thanks!
>
> > (uint8_t -> uint16_t). It seems that gcc just doesn't like bitfields
>
> I wonder if all those extra changes can be upstreamed back to AOSP?
> Ideally we'd want to just copy those headers over from AOSP to U-Boot
> with no changes, would make the porting work easier. What are your
> thoughts on that?
Technically we can, I was planning to do that.
>
> > that cross the width of the type. Changing the type doesn't change
> > the layout though.
> > This addresses this gcc note:
> > In file included from boot/android_ab.c:7:
> > include/android_bootloader_message.h:230:1: note: offset of packed bit-field ‘merge_status’ has changed in GCC 4.4
> > 230 | } __attribute__((packed));
> >
> > [1] https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h
> > [2] https://android.googlesource.com/platform/hardware/interfaces/+/main/boot/1.1/default/boot_control/include/private/boot_control_definition.h
> >
> > CC: Alex Deymo <deymo@google.com>
> > CC: Sam Protsenko <semen.protsenko@linaro.org>
> > CC: Eugeniu Rosca <erosca@de.adit-jv.com>
> > CC: Simon Glass <sjg@chromium.org>
> > Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>
> > ---
> >
> > include/android_bootloader_message.h | 104 +++++++++++++++++++++++----
> > 1 file changed, 92 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/android_bootloader_message.h b/include/android_bootloader_message.h
> > index 286d7ab0f31..75198fc9dc2 100644
> > --- a/include/android_bootloader_message.h
> > +++ b/include/android_bootloader_message.h
> > @@ -21,17 +21,22 @@
> > * stddef.h
> > */
> > #include <compiler.h>
> > +#include <linux/build_bug.h>
> > #endif
> >
> > // Spaces used by misc partition are as below:
> > // 0 - 2K For bootloader_message
> > // 2K - 16K Used by Vendor's bootloader (the 2K - 4K range may be optionally used
> > // as bootloader_message_ab struct)
> > -// 16K - 64K Used by uncrypt and recovery to store wipe_package for A/B devices
> > +// 16K - 32K Used by uncrypt and recovery to store wipe_package for A/B devices
> > +// 32K - 64K System space, used for miscellanious AOSP features. See below.
> > // Note that these offsets are admitted by bootloader,recovery and uncrypt, so they
> > // are not configurable without changing all of them.
> > static const size_t BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0;
> > +static const size_t VENDOR_SPACE_OFFSET_IN_MISC = 2 * 1024;
> > static const size_t WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;
> > +static const size_t SYSTEM_SPACE_OFFSET_IN_MISC = 32 * 1024;
> > +static const size_t SYSTEM_SPACE_SIZE_IN_MISC = 32 * 1024;
> >
> > /* Bootloader Message (2-KiB)
> > *
> > @@ -81,24 +86,67 @@ struct bootloader_message {
> > char reserved[1184];
> > };
> >
> > +// Holds Virtual A/B merge status information. Current version is 1. New fields
> > +// must be added to the end.
> > +struct misc_virtual_ab_message {
> > + uint8_t version;
> > + uint32_t magic;
> > + uint8_t merge_status; // IBootControl 1.1, MergeStatus enum.
> > + uint8_t source_slot; // Slot number when merge_status was written.
> > + uint8_t reserved[57];
> > +} __attribute__((packed));
> > +
> > +struct misc_memtag_message {
> > + uint8_t version;
> > + uint32_t magic; // magic string for treble compat
> > + uint32_t memtag_mode;
> > + uint8_t reserved[55];
> > +} __attribute__((packed));
> > +
> > +struct misc_kcmdline_message {
> > + uint8_t version;
> > + uint32_t magic;
> > + uint64_t kcmdline_flags;
> > + uint8_t reserved[51];
> > +} __attribute__((packed));
> > +
> > +#define MISC_VIRTUAL_AB_MESSAGE_VERSION 2
> > +#define MISC_VIRTUAL_AB_MAGIC_HEADER 0x56740AB0
> > +
> > +#define MISC_MEMTAG_MESSAGE_VERSION 1
> > +#define MISC_MEMTAG_MAGIC_HEADER 0x5afefe5a
> > +#define MISC_MEMTAG_MODE_MEMTAG 0x1
> > +#define MISC_MEMTAG_MODE_MEMTAG_ONCE 0x2
> > +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL 0x4
> > +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL_ONCE 0x8
> > +#define MISC_MEMTAG_MODE_MEMTAG_OFF 0x10
> > +// This is set when the state was overridden forcibly. This does not need to be
> > +// interpreted by the bootloader but is only for bookkeeping purposes so
> > +// userspace knows what to do when the override is undone.
> > +// See system/extras/mtectrl in AOSP for more information.
> > +#define MISC_MEMTAG_MODE_FORCED 0x20
> > +
> > +#define MISC_KCMDLINE_MESSAGE_VERSION 1
> > +#define MISC_KCMDLINE_MAGIC_HEADER 0x6ab5110c
> > +#define MISC_KCMDLINE_BINDER_RUST 0x1
> > /**
> > * We must be cautious when changing the bootloader_message struct size,
> > * because A/B-specific fields may end up with different offsets.
> > */
> > -#ifndef __UBOOT__
> > +
> > #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> > static_assert(sizeof(struct bootloader_message) == 2048,
> > "struct bootloader_message size changes, which may break A/B devices");
> > #endif
> > -#endif /* __UBOOT__ */
> >
> > /**
> > * The A/B-specific bootloader message structure (4-KiB).
> > *
> > * We separate A/B boot control metadata from the regular bootloader
> > * message struct and keep it here. Everything that's A/B-specific
> > - * stays after struct bootloader_message, which should be managed by
> > - * the A/B-bootloader or boot control HAL.
> > + * stays after struct bootloader_message, which belongs to the vendor
> > + * space of /misc partition. Also, the A/B-specific contents should be
> > + * managed by the A/B-bootloader or boot control HAL.
> > *
> > * The slot_suffix field is used for A/B implementations where the
> > * bootloader does not set the androidboot.ro.boot.slot_suffix kernel
> > @@ -124,12 +172,10 @@ struct bootloader_message_ab {
> > * Be cautious about the struct size change, in case we put anything post
> > * bootloader_message_ab struct (b/29159185).
> > */
> > -#ifndef __UBOOT__
> > #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> > static_assert(sizeof(struct bootloader_message_ab) == 4096,
> > "struct bootloader_message_ab size changes");
> > #endif
> > -#endif /* __UBOOT__ */
> >
> > #define BOOT_CTRL_MAGIC 0x42414342 /* Bootloader Control AB */
> > #define BOOT_CTRL_VERSION 1
> > @@ -165,11 +211,13 @@ struct bootloader_control {
> > // Version of struct being used (see BOOT_CTRL_VERSION).
> > uint8_t version;
> > // Number of slots being managed.
> > - uint8_t nb_slot : 3;
> > + uint16_t nb_slot : 3;
> > // Number of times left attempting to boot recovery.
> > - uint8_t recovery_tries_remaining : 3;
> > + uint16_t recovery_tries_remaining : 3;
> > + // Status of any pending snapshot merge of dynamic partitions.
> > + uint16_t merge_status : 3;
> > // Ensure 4-bytes alignment for slot_info field.
> > - uint8_t reserved0[2];
> > + uint8_t reserved0[1];
> > // Per-slot information. Up to 4 slots.
> > struct slot_metadata slot_info[4];
> > // Reserved for further use.
> > @@ -179,25 +227,46 @@ struct bootloader_control {
> > uint32_t crc32_le;
> > } __attribute__((packed));
> >
> > -#ifndef __UBOOT__
> > #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> > static_assert(sizeof(struct bootloader_control) ==
> > sizeof(((struct bootloader_message_ab *)0)->slot_suffix),
> > "struct bootloader_control has wrong size");
> > +static_assert(sizeof(struct misc_virtual_ab_message) == 64,
> > + "struct misc_virtual_ab_message has wrong size");
> > +static_assert(sizeof(struct misc_memtag_message) == 64,
> > + "struct misc_memtag_message has wrong size");
> > +static_assert(sizeof(struct misc_kcmdline_message) == 64,
> > + "struct misc_kcmdline_message has wrong size");
> > #endif
> > -#endif /* __UBOOT__ */
> >
> > #ifndef __UBOOT__
> > +
> > +// This struct is not meant to be used directly, rather, it is to make
> > +// computation of offsets easier. New fields must be added to the end.
> > +struct misc_system_space_layout {
> > + misc_virtual_ab_message virtual_ab_message;
> > + misc_memtag_message memtag_message;
> > + misc_kcmdline_message kcmdline_message;
> > +} __attribute__((packed));
> > +
> > #ifdef __cplusplus
> >
> > #include <string>
> > #include <vector>
> >
> > +// Gets the block device name of /misc partition.
> > +std::string get_misc_blk_device(std::string* err);
> > +
> > // Return the block device name for the bootloader message partition and waits
> > // for the device for up to 10 seconds. In case of error returns the empty
> > // string.
> > std::string get_bootloader_message_blk_device(std::string* err);
> >
> > +// Writes |size| bytes of data from buffer |p| to |misc_blk_device| at |offset|. If the write fails,
> > +// sets the error message in |err|.
> > +bool write_misc_partition(const void* p, size_t size, const std::string& misc_blk_device,
> > + size_t offset, std::string* err);
> > +
> > // Read bootloader message into boot. Error message will be set in err.
> > bool read_bootloader_message(bootloader_message* boot, std::string* err);
> >
> > @@ -242,6 +311,17 @@ bool read_wipe_package(std::string* package_data, size_t size, std::string* err)
> > // Write the wipe package into BCB (to offset WIPE_PACKAGE_OFFSET_IN_MISC).
> > bool write_wipe_package(const std::string& package_data, std::string* err);
> >
> > +// Read or write the Virtual A/B message from system space in /misc.
> > +bool ReadMiscVirtualAbMessage(misc_virtual_ab_message* message, std::string* err);
> > +bool WriteMiscVirtualAbMessage(const misc_virtual_ab_message& message, std::string* err);
> > +
> > +// Read or write the memtag message from system space in /misc.
> > +bool ReadMiscMemtagMessage(misc_memtag_message* message, std::string* err);
> > +bool WriteMiscMemtagMessage(const misc_memtag_message& message, std::string* err);
> > +
> > +// Read or write the kcmdline message from system space in /misc.
> > +bool ReadMiscKcmdlineMessage(misc_kcmdline_message* message, std::string* err);
> > +bool WriteMiscKcmdlineMessage(const misc_kcmdline_message& message, std::string* err);
>
> AFAIU, these new prototypes brought by this patch don't have actual
> implementation yet. Does it affect the build somehow, e.g. maybe
> causing some build warnings? Or is the build log clean?
They are all wrapped with #ifndef __UBOOT__.
>
> > #else
> >
> > #include <stdbool.h>
> > --
> > 2.34.1
> >
Thanks for your comments!
--
Best regards - Freundliche Grüsse - Meilleures salutations
Igor Opaniuk
Senior Software Engineer, Embedded & Security
E: igor.opaniuk@foundries.io
W: www.foundries.io
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream
2024-02-19 10:15 [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream Igor Opaniuk
2024-02-20 18:29 ` Sam Protsenko
@ 2024-02-29 19:05 ` Igor Opaniuk
2024-03-07 9:29 ` Mattijs Korpershoek
2 siblings, 0 replies; 6+ messages in thread
From: Igor Opaniuk @ 2024-02-29 19:05 UTC (permalink / raw)
To: u-boot
Cc: Eugeniu Rosca, Tom Rini, Sam Protsenko, Simon Glass, Alex Deymo,
Mattijs Korpershoek
+CC Mattijs
On Mon, Feb 19, 2024 at 11:16 AM Igor Opaniuk <igor.opaniuk@gmail.com>
wrote:
> This takes the latest changes from AOSP from [1][2] (as this
> header was split on two) with minimal changes (this could lead
> to warnings reported by checkpatch).
>
> Some local changes have been applied:
> 1. Enable static_assert() for defined structures to be sure
> that all of them have correct sizes.
> 2. Adjuste types in bootloader_control structure with bitfields
> (uint8_t -> uint16_t). It seems that gcc just doesn't like bitfields
> that cross the width of the type. Changing the type doesn't change
> the layout though.
> This addresses this gcc note:
> In file included from boot/android_ab.c:7:
> include/android_bootloader_message.h:230:1: note: offset of packed
> bit-field ‘merge_status’ has changed in GCC 4.4
> 230 | } __attribute__((packed));
>
> [1]
> https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h
> [2]
> https://android.googlesource.com/platform/hardware/interfaces/+/main/boot/1.1/default/boot_control/include/private/boot_control_definition.h
>
> CC: Alex Deymo <deymo@google.com>
> CC: Sam Protsenko <semen.protsenko@linaro.org>
> CC: Eugeniu Rosca <erosca@de.adit-jv.com>
> CC: Simon Glass <sjg@chromium.org>
> Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>
> ---
>
> include/android_bootloader_message.h | 104 +++++++++++++++++++++++----
> 1 file changed, 92 insertions(+), 12 deletions(-)
>
> diff --git a/include/android_bootloader_message.h
> b/include/android_bootloader_message.h
> index 286d7ab0f31..75198fc9dc2 100644
> --- a/include/android_bootloader_message.h
> +++ b/include/android_bootloader_message.h
> @@ -21,17 +21,22 @@
> * stddef.h
> */
> #include <compiler.h>
> +#include <linux/build_bug.h>
> #endif
>
> // Spaces used by misc partition are as below:
> // 0 - 2K For bootloader_message
> // 2K - 16K Used by Vendor's bootloader (the 2K - 4K range may be
> optionally used
> // as bootloader_message_ab struct)
> -// 16K - 64K Used by uncrypt and recovery to store wipe_package for
> A/B devices
> +// 16K - 32K Used by uncrypt and recovery to store wipe_package for
> A/B devices
> +// 32K - 64K System space, used for miscellanious AOSP features. See
> below.
> // Note that these offsets are admitted by bootloader,recovery and
> uncrypt, so they
> // are not configurable without changing all of them.
> static const size_t BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0;
> +static const size_t VENDOR_SPACE_OFFSET_IN_MISC = 2 * 1024;
> static const size_t WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;
> +static const size_t SYSTEM_SPACE_OFFSET_IN_MISC = 32 * 1024;
> +static const size_t SYSTEM_SPACE_SIZE_IN_MISC = 32 * 1024;
>
> /* Bootloader Message (2-KiB)
> *
> @@ -81,24 +86,67 @@ struct bootloader_message {
> char reserved[1184];
> };
>
> +// Holds Virtual A/B merge status information. Current version is 1. New
> fields
> +// must be added to the end.
> +struct misc_virtual_ab_message {
> + uint8_t version;
> + uint32_t magic;
> + uint8_t merge_status; // IBootControl 1.1, MergeStatus enum.
> + uint8_t source_slot; // Slot number when merge_status was written.
> + uint8_t reserved[57];
> +} __attribute__((packed));
> +
> +struct misc_memtag_message {
> + uint8_t version;
> + uint32_t magic; // magic string for treble compat
> + uint32_t memtag_mode;
> + uint8_t reserved[55];
> +} __attribute__((packed));
> +
> +struct misc_kcmdline_message {
> + uint8_t version;
> + uint32_t magic;
> + uint64_t kcmdline_flags;
> + uint8_t reserved[51];
> +} __attribute__((packed));
> +
> +#define MISC_VIRTUAL_AB_MESSAGE_VERSION 2
> +#define MISC_VIRTUAL_AB_MAGIC_HEADER 0x56740AB0
> +
> +#define MISC_MEMTAG_MESSAGE_VERSION 1
> +#define MISC_MEMTAG_MAGIC_HEADER 0x5afefe5a
> +#define MISC_MEMTAG_MODE_MEMTAG 0x1
> +#define MISC_MEMTAG_MODE_MEMTAG_ONCE 0x2
> +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL 0x4
> +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL_ONCE 0x8
> +#define MISC_MEMTAG_MODE_MEMTAG_OFF 0x10
> +// This is set when the state was overridden forcibly. This does not need
> to be
> +// interpreted by the bootloader but is only for bookkeeping purposes so
> +// userspace knows what to do when the override is undone.
> +// See system/extras/mtectrl in AOSP for more information.
> +#define MISC_MEMTAG_MODE_FORCED 0x20
> +
> +#define MISC_KCMDLINE_MESSAGE_VERSION 1
> +#define MISC_KCMDLINE_MAGIC_HEADER 0x6ab5110c
> +#define MISC_KCMDLINE_BINDER_RUST 0x1
> /**
> * We must be cautious when changing the bootloader_message struct size,
> * because A/B-specific fields may end up with different offsets.
> */
> -#ifndef __UBOOT__
> +
> #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> static_assert(sizeof(struct bootloader_message) == 2048,
> "struct bootloader_message size changes, which may break
> A/B devices");
> #endif
> -#endif /* __UBOOT__ */
>
> /**
> * The A/B-specific bootloader message structure (4-KiB).
> *
> * We separate A/B boot control metadata from the regular bootloader
> * message struct and keep it here. Everything that's A/B-specific
> - * stays after struct bootloader_message, which should be managed by
> - * the A/B-bootloader or boot control HAL.
> + * stays after struct bootloader_message, which belongs to the vendor
> + * space of /misc partition. Also, the A/B-specific contents should be
> + * managed by the A/B-bootloader or boot control HAL.
> *
> * The slot_suffix field is used for A/B implementations where the
> * bootloader does not set the androidboot.ro.boot.slot_suffix kernel
> @@ -124,12 +172,10 @@ struct bootloader_message_ab {
> * Be cautious about the struct size change, in case we put anything post
> * bootloader_message_ab struct (b/29159185).
> */
> -#ifndef __UBOOT__
> #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> static_assert(sizeof(struct bootloader_message_ab) == 4096,
> "struct bootloader_message_ab size changes");
> #endif
> -#endif /* __UBOOT__ */
>
> #define BOOT_CTRL_MAGIC 0x42414342 /* Bootloader Control AB */
> #define BOOT_CTRL_VERSION 1
> @@ -165,11 +211,13 @@ struct bootloader_control {
> // Version of struct being used (see BOOT_CTRL_VERSION).
> uint8_t version;
> // Number of slots being managed.
> - uint8_t nb_slot : 3;
> + uint16_t nb_slot : 3;
> // Number of times left attempting to boot recovery.
> - uint8_t recovery_tries_remaining : 3;
> + uint16_t recovery_tries_remaining : 3;
> + // Status of any pending snapshot merge of dynamic partitions.
> + uint16_t merge_status : 3;
> // Ensure 4-bytes alignment for slot_info field.
> - uint8_t reserved0[2];
> + uint8_t reserved0[1];
> // Per-slot information. Up to 4 slots.
> struct slot_metadata slot_info[4];
> // Reserved for further use.
> @@ -179,25 +227,46 @@ struct bootloader_control {
> uint32_t crc32_le;
> } __attribute__((packed));
>
> -#ifndef __UBOOT__
> #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> static_assert(sizeof(struct bootloader_control) ==
> sizeof(((struct bootloader_message_ab *)0)->slot_suffix),
> "struct bootloader_control has wrong size");
> +static_assert(sizeof(struct misc_virtual_ab_message) == 64,
> + "struct misc_virtual_ab_message has wrong size");
> +static_assert(sizeof(struct misc_memtag_message) == 64,
> + "struct misc_memtag_message has wrong size");
> +static_assert(sizeof(struct misc_kcmdline_message) == 64,
> + "struct misc_kcmdline_message has wrong size");
> #endif
> -#endif /* __UBOOT__ */
>
> #ifndef __UBOOT__
> +
> +// This struct is not meant to be used directly, rather, it is to make
> +// computation of offsets easier. New fields must be added to the end.
> +struct misc_system_space_layout {
> + misc_virtual_ab_message virtual_ab_message;
> + misc_memtag_message memtag_message;
> + misc_kcmdline_message kcmdline_message;
> +} __attribute__((packed));
> +
> #ifdef __cplusplus
>
> #include <string>
> #include <vector>
>
> +// Gets the block device name of /misc partition.
> +std::string get_misc_blk_device(std::string* err);
> +
> // Return the block device name for the bootloader message partition and
> waits
> // for the device for up to 10 seconds. In case of error returns the empty
> // string.
> std::string get_bootloader_message_blk_device(std::string* err);
>
> +// Writes |size| bytes of data from buffer |p| to |misc_blk_device| at
> |offset|. If the write fails,
> +// sets the error message in |err|.
> +bool write_misc_partition(const void* p, size_t size, const std::string&
> misc_blk_device,
> + size_t offset, std::string* err);
> +
> // Read bootloader message into boot. Error message will be set in err.
> bool read_bootloader_message(bootloader_message* boot, std::string* err);
>
> @@ -242,6 +311,17 @@ bool read_wipe_package(std::string* package_data,
> size_t size, std::string* err)
> // Write the wipe package into BCB (to offset
> WIPE_PACKAGE_OFFSET_IN_MISC).
> bool write_wipe_package(const std::string& package_data, std::string*
> err);
>
> +// Read or write the Virtual A/B message from system space in /misc.
> +bool ReadMiscVirtualAbMessage(misc_virtual_ab_message* message,
> std::string* err);
> +bool WriteMiscVirtualAbMessage(const misc_virtual_ab_message& message,
> std::string* err);
> +
> +// Read or write the memtag message from system space in /misc.
> +bool ReadMiscMemtagMessage(misc_memtag_message* message, std::string*
> err);
> +bool WriteMiscMemtagMessage(const misc_memtag_message& message,
> std::string* err);
> +
> +// Read or write the kcmdline message from system space in /misc.
> +bool ReadMiscKcmdlineMessage(misc_kcmdline_message* message, std::string*
> err);
> +bool WriteMiscKcmdlineMessage(const misc_kcmdline_message& message,
> std::string* err);
> #else
>
> #include <stdbool.h>
> --
> 2.34.1
>
>
--
Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opaniuk@gmail.com
skype: igor.opanyuk
http://ua.linkedin.com/in/iopaniuk
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream
2024-02-19 10:15 [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream Igor Opaniuk
2024-02-20 18:29 ` Sam Protsenko
2024-02-29 19:05 ` Igor Opaniuk
@ 2024-03-07 9:29 ` Mattijs Korpershoek
2 siblings, 0 replies; 6+ messages in thread
From: Mattijs Korpershoek @ 2024-03-07 9:29 UTC (permalink / raw)
To: Igor Opaniuk, u-boot
Cc: Eugeniu Rosca, Tom Rini, Sam Protsenko, Simon Glass, Alex Deymo,
Igor Opaniuk
Hi Igor,
Thank you for the patch.
On lun., févr. 19, 2024 at 11:15, Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
> This takes the latest changes from AOSP from [1][2] (as this
> header was split on two) with minimal changes (this could lead
> to warnings reported by checkpatch).
>
> Some local changes have been applied:
> 1. Enable static_assert() for defined structures to be sure
> that all of them have correct sizes.
> 2. Adjuste types in bootloader_control structure with bitfields
> (uint8_t -> uint16_t). It seems that gcc just doesn't like bitfields
> that cross the width of the type. Changing the type doesn't change
> the layout though.
> This addresses this gcc note:
> In file included from boot/android_ab.c:7:
> include/android_bootloader_message.h:230:1: note: offset of packed bit-field ‘merge_status’ has changed in GCC 4.4
> 230 | } __attribute__((packed));
>
> [1] https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h
> [2] https://android.googlesource.com/platform/hardware/interfaces/+/main/boot/1.1/default/boot_control/include/private/boot_control_definition.h
nitpick: Can you please put in the commit message the sha-1 from when this was synced?
Looking at platform/bootable/recovery, I can see that there are some new
structures introduced (misc_control_message) in
commit 5a4a41ee2e4b ("misctrl: read message, incl 16kb flag")
To do the review, I used:
* //bootable/recovery: a4aec2f2ceac ("Add kcmdline bootloader message")
* //hardware/interfaces: c4b2f5b564e3 ("Merge "Merge Android 14 QPR2 to AOSP main" into main")
>
> CC: Alex Deymo <deymo@google.com>
> CC: Sam Protsenko <semen.protsenko@linaro.org>
> CC: Eugeniu Rosca <erosca@de.adit-jv.com>
> CC: Simon Glass <sjg@chromium.org>
> Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>
Tested on Khadas VIM3 that I could reboot into fastbootd (recovery)
and into fastboot(u-boot) from Android using:
$ adb reboot fastboot
$ adb reboot bootloader
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # on vim3
> ---
>
> include/android_bootloader_message.h | 104 +++++++++++++++++++++++----
> 1 file changed, 92 insertions(+), 12 deletions(-)
>
> diff --git a/include/android_bootloader_message.h b/include/android_bootloader_message.h
> index 286d7ab0f31..75198fc9dc2 100644
> --- a/include/android_bootloader_message.h
> +++ b/include/android_bootloader_message.h
> @@ -21,17 +21,22 @@
> * stddef.h
> */
> #include <compiler.h>
> +#include <linux/build_bug.h>
> #endif
>
> // Spaces used by misc partition are as below:
> // 0 - 2K For bootloader_message
> // 2K - 16K Used by Vendor's bootloader (the 2K - 4K range may be optionally used
> // as bootloader_message_ab struct)
> -// 16K - 64K Used by uncrypt and recovery to store wipe_package for A/B devices
> +// 16K - 32K Used by uncrypt and recovery to store wipe_package for A/B devices
> +// 32K - 64K System space, used for miscellanious AOSP features. See below.
> // Note that these offsets are admitted by bootloader,recovery and uncrypt, so they
> // are not configurable without changing all of them.
> static const size_t BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0;
> +static const size_t VENDOR_SPACE_OFFSET_IN_MISC = 2 * 1024;
> static const size_t WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;
> +static const size_t SYSTEM_SPACE_OFFSET_IN_MISC = 32 * 1024;
> +static const size_t SYSTEM_SPACE_SIZE_IN_MISC = 32 * 1024;
>
> /* Bootloader Message (2-KiB)
> *
> @@ -81,24 +86,67 @@ struct bootloader_message {
> char reserved[1184];
> };
>
> +// Holds Virtual A/B merge status information. Current version is 1. New fields
> +// must be added to the end.
> +struct misc_virtual_ab_message {
> + uint8_t version;
> + uint32_t magic;
> + uint8_t merge_status; // IBootControl 1.1, MergeStatus enum.
> + uint8_t source_slot; // Slot number when merge_status was written.
> + uint8_t reserved[57];
> +} __attribute__((packed));
> +
> +struct misc_memtag_message {
> + uint8_t version;
> + uint32_t magic; // magic string for treble compat
> + uint32_t memtag_mode;
> + uint8_t reserved[55];
> +} __attribute__((packed));
> +
> +struct misc_kcmdline_message {
> + uint8_t version;
> + uint32_t magic;
> + uint64_t kcmdline_flags;
> + uint8_t reserved[51];
> +} __attribute__((packed));
> +
> +#define MISC_VIRTUAL_AB_MESSAGE_VERSION 2
> +#define MISC_VIRTUAL_AB_MAGIC_HEADER 0x56740AB0
> +
> +#define MISC_MEMTAG_MESSAGE_VERSION 1
> +#define MISC_MEMTAG_MAGIC_HEADER 0x5afefe5a
> +#define MISC_MEMTAG_MODE_MEMTAG 0x1
> +#define MISC_MEMTAG_MODE_MEMTAG_ONCE 0x2
> +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL 0x4
> +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL_ONCE 0x8
> +#define MISC_MEMTAG_MODE_MEMTAG_OFF 0x10
> +// This is set when the state was overridden forcibly. This does not need to be
> +// interpreted by the bootloader but is only for bookkeeping purposes so
> +// userspace knows what to do when the override is undone.
> +// See system/extras/mtectrl in AOSP for more information.
> +#define MISC_MEMTAG_MODE_FORCED 0x20
> +
> +#define MISC_KCMDLINE_MESSAGE_VERSION 1
> +#define MISC_KCMDLINE_MAGIC_HEADER 0x6ab5110c
> +#define MISC_KCMDLINE_BINDER_RUST 0x1
> /**
> * We must be cautious when changing the bootloader_message struct size,
> * because A/B-specific fields may end up with different offsets.
> */
> -#ifndef __UBOOT__
> +
> #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> static_assert(sizeof(struct bootloader_message) == 2048,
> "struct bootloader_message size changes, which may break A/B devices");
> #endif
> -#endif /* __UBOOT__ */
>
> /**
> * The A/B-specific bootloader message structure (4-KiB).
> *
> * We separate A/B boot control metadata from the regular bootloader
> * message struct and keep it here. Everything that's A/B-specific
> - * stays after struct bootloader_message, which should be managed by
> - * the A/B-bootloader or boot control HAL.
> + * stays after struct bootloader_message, which belongs to the vendor
> + * space of /misc partition. Also, the A/B-specific contents should be
> + * managed by the A/B-bootloader or boot control HAL.
> *
> * The slot_suffix field is used for A/B implementations where the
> * bootloader does not set the androidboot.ro.boot.slot_suffix kernel
> @@ -124,12 +172,10 @@ struct bootloader_message_ab {
> * Be cautious about the struct size change, in case we put anything post
> * bootloader_message_ab struct (b/29159185).
> */
> -#ifndef __UBOOT__
> #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> static_assert(sizeof(struct bootloader_message_ab) == 4096,
> "struct bootloader_message_ab size changes");
> #endif
> -#endif /* __UBOOT__ */
>
> #define BOOT_CTRL_MAGIC 0x42414342 /* Bootloader Control AB */
> #define BOOT_CTRL_VERSION 1
> @@ -165,11 +211,13 @@ struct bootloader_control {
> // Version of struct being used (see BOOT_CTRL_VERSION).
> uint8_t version;
> // Number of slots being managed.
> - uint8_t nb_slot : 3;
> + uint16_t nb_slot : 3;
> // Number of times left attempting to boot recovery.
> - uint8_t recovery_tries_remaining : 3;
> + uint16_t recovery_tries_remaining : 3;
> + // Status of any pending snapshot merge of dynamic partitions.
> + uint16_t merge_status : 3;
> // Ensure 4-bytes alignment for slot_info field.
> - uint8_t reserved0[2];
> + uint8_t reserved0[1];
> // Per-slot information. Up to 4 slots.
> struct slot_metadata slot_info[4];
> // Reserved for further use.
> @@ -179,25 +227,46 @@ struct bootloader_control {
> uint32_t crc32_le;
> } __attribute__((packed));
>
> -#ifndef __UBOOT__
> #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
> static_assert(sizeof(struct bootloader_control) ==
> sizeof(((struct bootloader_message_ab *)0)->slot_suffix),
> "struct bootloader_control has wrong size");
> +static_assert(sizeof(struct misc_virtual_ab_message) == 64,
> + "struct misc_virtual_ab_message has wrong size");
> +static_assert(sizeof(struct misc_memtag_message) == 64,
> + "struct misc_memtag_message has wrong size");
> +static_assert(sizeof(struct misc_kcmdline_message) == 64,
> + "struct misc_kcmdline_message has wrong size");
> #endif
> -#endif /* __UBOOT__ */
>
> #ifndef __UBOOT__
> +
> +// This struct is not meant to be used directly, rather, it is to make
> +// computation of offsets easier. New fields must be added to the end.
> +struct misc_system_space_layout {
> + misc_virtual_ab_message virtual_ab_message;
> + misc_memtag_message memtag_message;
> + misc_kcmdline_message kcmdline_message;
> +} __attribute__((packed));
> +
> #ifdef __cplusplus
>
> #include <string>
> #include <vector>
>
> +// Gets the block device name of /misc partition.
> +std::string get_misc_blk_device(std::string* err);
> +
> // Return the block device name for the bootloader message partition and waits
> // for the device for up to 10 seconds. In case of error returns the empty
> // string.
> std::string get_bootloader_message_blk_device(std::string* err);
>
> +// Writes |size| bytes of data from buffer |p| to |misc_blk_device| at |offset|. If the write fails,
> +// sets the error message in |err|.
> +bool write_misc_partition(const void* p, size_t size, const std::string& misc_blk_device,
> + size_t offset, std::string* err);
> +
> // Read bootloader message into boot. Error message will be set in err.
> bool read_bootloader_message(bootloader_message* boot, std::string* err);
>
> @@ -242,6 +311,17 @@ bool read_wipe_package(std::string* package_data, size_t size, std::string* err)
> // Write the wipe package into BCB (to offset WIPE_PACKAGE_OFFSET_IN_MISC).
> bool write_wipe_package(const std::string& package_data, std::string* err);
>
> +// Read or write the Virtual A/B message from system space in /misc.
> +bool ReadMiscVirtualAbMessage(misc_virtual_ab_message* message, std::string* err);
> +bool WriteMiscVirtualAbMessage(const misc_virtual_ab_message& message, std::string* err);
> +
> +// Read or write the memtag message from system space in /misc.
> +bool ReadMiscMemtagMessage(misc_memtag_message* message, std::string* err);
> +bool WriteMiscMemtagMessage(const misc_memtag_message& message, std::string* err);
> +
> +// Read or write the kcmdline message from system space in /misc.
> +bool ReadMiscKcmdlineMessage(misc_kcmdline_message* message, std::string* err);
> +bool WriteMiscKcmdlineMessage(const misc_kcmdline_message& message, std::string* err);
> #else
>
> #include <stdbool.h>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream
2024-02-20 19:08 ` Igor Opaniuk
@ 2024-03-07 9:32 ` Mattijs Korpershoek
0 siblings, 0 replies; 6+ messages in thread
From: Mattijs Korpershoek @ 2024-03-07 9:32 UTC (permalink / raw)
To: Igor Opaniuk, Sam Protsenko
Cc: Igor Opaniuk, u-boot, Eugeniu Rosca, Tom Rini, Simon Glass, Alex Deymo
Hi Igor, Sam,
On mar., févr. 20, 2024 at 20:08, Igor Opaniuk <igor.opaniuk@foundries.io> wrote:
> Hi Sam,
>
> On Tue, Feb 20, 2024 at 7:29 PM Sam Protsenko
> <semen.protsenko@linaro.org> wrote:
>>
>> On Mon, Feb 19, 2024 at 4:16 AM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
>> >
>> > This takes the latest changes from AOSP from [1][2] (as this
>> > header was split on two) with minimal changes (this could lead
>> > to warnings reported by checkpatch).
>>
>> Do we want to maybe follow that and also carry two different headers
>> in U-Boot? Or it doesn't make much sense? I'm thinking in terms of
>> future portability mostly: how easy it's to update this header right
>> now, and how easy it's going to be further. I didn't form any opinion
>> on that, hence asking.
> The problem is licensing. android_bootloader_message.h was
> re-licensed by Alex Deymo from Google under BSD-3-Clause,
> which is GPLv2 compatible. I'm not sure it's legally correct to pull
> boot_control_definition.h from AOSP licensed under Apache as a
> separate file.
I'd prefer a separate file as well but I'm not familiar enough with
licensing to see if that's a problem.
I'm ok to keep as is.
>
>>
>> Another thing: are you sure that changing only the header won't break
>> anything in U-Boot .c files that use this header?
>
> I've tested both ab_select and avb verify in QEMU. Or do you mean
> something else additionally should be tested?
I've boot tested this on VIM3 and tested adb reboot bootloader, adb
reboot fastboot (which writes things into the misc partition)
>
>>
>> >
>> > Some local changes have been applied:
>>
>> Is it possible to split this work into two patches:
>> 1. Bring the original changes only
>> 2. Apply all necessary changes for U-Boot
>>
>> Or does it break the build, etc? Again, thinking in terms of
>> portability easiness, and not sure which approach is better -- just
>> asking basically.
> Yeah, that's the problem, as splitting this on two commits
> will lead to the first one reporting warnings/notes.
>
>>
>> > 1. Enable static_assert() for defined structures to be sure
>> > that all of them have correct sizes.
>> > 2. Adjuste types in bootloader_control structure with bitfields
>>
>> Adjuste -> adjust
> Will fix, thanks!
>>
>> > (uint8_t -> uint16_t). It seems that gcc just doesn't like bitfields
>>
>> I wonder if all those extra changes can be upstreamed back to AOSP?
>> Ideally we'd want to just copy those headers over from AOSP to U-Boot
>> with no changes, would make the porting work easier. What are your
>> thoughts on that?
> Technically we can, I was planning to do that.
I'm not sure this change will be of interest to AOSP, since they moved
to the AIDL implementation here:
https://android.googlesource.com/platform/hardware/interfaces/+/refs/heads/main/boot/aidl/default/
If you submit it on gerrit, can you please cc me? (using this email address)
>
>>
>> > that cross the width of the type. Changing the type doesn't change
>> > the layout though.
>> > This addresses this gcc note:
>> > In file included from boot/android_ab.c:7:
>> > include/android_bootloader_message.h:230:1: note: offset of packed bit-field ‘merge_status’ has changed in GCC 4.4
>> > 230 | } __attribute__((packed));
>> >
>> > [1] https://android.googlesource.com/platform/bootable/recovery/+/main/bootloader_message/include/bootloader_message/bootloader_message.h
>> > [2] https://android.googlesource.com/platform/hardware/interfaces/+/main/boot/1.1/default/boot_control/include/private/boot_control_definition.h
>> >
>> > CC: Alex Deymo <deymo@google.com>
>> > CC: Sam Protsenko <semen.protsenko@linaro.org>
>> > CC: Eugeniu Rosca <erosca@de.adit-jv.com>
>> > CC: Simon Glass <sjg@chromium.org>
>> > Signed-off-by: Igor Opaniuk <igor.opaniuk@gmail.com>
>> > ---
>> >
>> > include/android_bootloader_message.h | 104 +++++++++++++++++++++++----
>> > 1 file changed, 92 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/include/android_bootloader_message.h b/include/android_bootloader_message.h
>> > index 286d7ab0f31..75198fc9dc2 100644
>> > --- a/include/android_bootloader_message.h
>> > +++ b/include/android_bootloader_message.h
>> > @@ -21,17 +21,22 @@
>> > * stddef.h
>> > */
>> > #include <compiler.h>
>> > +#include <linux/build_bug.h>
>> > #endif
>> >
>> > // Spaces used by misc partition are as below:
>> > // 0 - 2K For bootloader_message
>> > // 2K - 16K Used by Vendor's bootloader (the 2K - 4K range may be optionally used
>> > // as bootloader_message_ab struct)
>> > -// 16K - 64K Used by uncrypt and recovery to store wipe_package for A/B devices
>> > +// 16K - 32K Used by uncrypt and recovery to store wipe_package for A/B devices
>> > +// 32K - 64K System space, used for miscellanious AOSP features. See below.
>> > // Note that these offsets are admitted by bootloader,recovery and uncrypt, so they
>> > // are not configurable without changing all of them.
>> > static const size_t BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0;
>> > +static const size_t VENDOR_SPACE_OFFSET_IN_MISC = 2 * 1024;
>> > static const size_t WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;
>> > +static const size_t SYSTEM_SPACE_OFFSET_IN_MISC = 32 * 1024;
>> > +static const size_t SYSTEM_SPACE_SIZE_IN_MISC = 32 * 1024;
>> >
>> > /* Bootloader Message (2-KiB)
>> > *
>> > @@ -81,24 +86,67 @@ struct bootloader_message {
>> > char reserved[1184];
>> > };
>> >
>> > +// Holds Virtual A/B merge status information. Current version is 1. New fields
>> > +// must be added to the end.
>> > +struct misc_virtual_ab_message {
>> > + uint8_t version;
>> > + uint32_t magic;
>> > + uint8_t merge_status; // IBootControl 1.1, MergeStatus enum.
>> > + uint8_t source_slot; // Slot number when merge_status was written.
>> > + uint8_t reserved[57];
>> > +} __attribute__((packed));
>> > +
>> > +struct misc_memtag_message {
>> > + uint8_t version;
>> > + uint32_t magic; // magic string for treble compat
>> > + uint32_t memtag_mode;
>> > + uint8_t reserved[55];
>> > +} __attribute__((packed));
>> > +
>> > +struct misc_kcmdline_message {
>> > + uint8_t version;
>> > + uint32_t magic;
>> > + uint64_t kcmdline_flags;
>> > + uint8_t reserved[51];
>> > +} __attribute__((packed));
>> > +
>> > +#define MISC_VIRTUAL_AB_MESSAGE_VERSION 2
>> > +#define MISC_VIRTUAL_AB_MAGIC_HEADER 0x56740AB0
>> > +
>> > +#define MISC_MEMTAG_MESSAGE_VERSION 1
>> > +#define MISC_MEMTAG_MAGIC_HEADER 0x5afefe5a
>> > +#define MISC_MEMTAG_MODE_MEMTAG 0x1
>> > +#define MISC_MEMTAG_MODE_MEMTAG_ONCE 0x2
>> > +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL 0x4
>> > +#define MISC_MEMTAG_MODE_MEMTAG_KERNEL_ONCE 0x8
>> > +#define MISC_MEMTAG_MODE_MEMTAG_OFF 0x10
>> > +// This is set when the state was overridden forcibly. This does not need to be
>> > +// interpreted by the bootloader but is only for bookkeeping purposes so
>> > +// userspace knows what to do when the override is undone.
>> > +// See system/extras/mtectrl in AOSP for more information.
>> > +#define MISC_MEMTAG_MODE_FORCED 0x20
>> > +
>> > +#define MISC_KCMDLINE_MESSAGE_VERSION 1
>> > +#define MISC_KCMDLINE_MAGIC_HEADER 0x6ab5110c
>> > +#define MISC_KCMDLINE_BINDER_RUST 0x1
>> > /**
>> > * We must be cautious when changing the bootloader_message struct size,
>> > * because A/B-specific fields may end up with different offsets.
>> > */
>> > -#ifndef __UBOOT__
>> > +
>> > #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
>> > static_assert(sizeof(struct bootloader_message) == 2048,
>> > "struct bootloader_message size changes, which may break A/B devices");
>> > #endif
>> > -#endif /* __UBOOT__ */
>> >
>> > /**
>> > * The A/B-specific bootloader message structure (4-KiB).
>> > *
>> > * We separate A/B boot control metadata from the regular bootloader
>> > * message struct and keep it here. Everything that's A/B-specific
>> > - * stays after struct bootloader_message, which should be managed by
>> > - * the A/B-bootloader or boot control HAL.
>> > + * stays after struct bootloader_message, which belongs to the vendor
>> > + * space of /misc partition. Also, the A/B-specific contents should be
>> > + * managed by the A/B-bootloader or boot control HAL.
>> > *
>> > * The slot_suffix field is used for A/B implementations where the
>> > * bootloader does not set the androidboot.ro.boot.slot_suffix kernel
>> > @@ -124,12 +172,10 @@ struct bootloader_message_ab {
>> > * Be cautious about the struct size change, in case we put anything post
>> > * bootloader_message_ab struct (b/29159185).
>> > */
>> > -#ifndef __UBOOT__
>> > #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
>> > static_assert(sizeof(struct bootloader_message_ab) == 4096,
>> > "struct bootloader_message_ab size changes");
>> > #endif
>> > -#endif /* __UBOOT__ */
>> >
>> > #define BOOT_CTRL_MAGIC 0x42414342 /* Bootloader Control AB */
>> > #define BOOT_CTRL_VERSION 1
>> > @@ -165,11 +211,13 @@ struct bootloader_control {
>> > // Version of struct being used (see BOOT_CTRL_VERSION).
>> > uint8_t version;
>> > // Number of slots being managed.
>> > - uint8_t nb_slot : 3;
>> > + uint16_t nb_slot : 3;
>> > // Number of times left attempting to boot recovery.
>> > - uint8_t recovery_tries_remaining : 3;
>> > + uint16_t recovery_tries_remaining : 3;
>> > + // Status of any pending snapshot merge of dynamic partitions.
>> > + uint16_t merge_status : 3;
>> > // Ensure 4-bytes alignment for slot_info field.
>> > - uint8_t reserved0[2];
>> > + uint8_t reserved0[1];
>> > // Per-slot information. Up to 4 slots.
>> > struct slot_metadata slot_info[4];
>> > // Reserved for further use.
>> > @@ -179,25 +227,46 @@ struct bootloader_control {
>> > uint32_t crc32_le;
>> > } __attribute__((packed));
>> >
>> > -#ifndef __UBOOT__
>> > #if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus)
>> > static_assert(sizeof(struct bootloader_control) ==
>> > sizeof(((struct bootloader_message_ab *)0)->slot_suffix),
>> > "struct bootloader_control has wrong size");
>> > +static_assert(sizeof(struct misc_virtual_ab_message) == 64,
>> > + "struct misc_virtual_ab_message has wrong size");
>> > +static_assert(sizeof(struct misc_memtag_message) == 64,
>> > + "struct misc_memtag_message has wrong size");
>> > +static_assert(sizeof(struct misc_kcmdline_message) == 64,
>> > + "struct misc_kcmdline_message has wrong size");
>> > #endif
>> > -#endif /* __UBOOT__ */
>> >
>> > #ifndef __UBOOT__
>> > +
>> > +// This struct is not meant to be used directly, rather, it is to make
>> > +// computation of offsets easier. New fields must be added to the end.
>> > +struct misc_system_space_layout {
>> > + misc_virtual_ab_message virtual_ab_message;
>> > + misc_memtag_message memtag_message;
>> > + misc_kcmdline_message kcmdline_message;
>> > +} __attribute__((packed));
>> > +
>> > #ifdef __cplusplus
>> >
>> > #include <string>
>> > #include <vector>
>> >
>> > +// Gets the block device name of /misc partition.
>> > +std::string get_misc_blk_device(std::string* err);
>> > +
>> > // Return the block device name for the bootloader message partition and waits
>> > // for the device for up to 10 seconds. In case of error returns the empty
>> > // string.
>> > std::string get_bootloader_message_blk_device(std::string* err);
>> >
>> > +// Writes |size| bytes of data from buffer |p| to |misc_blk_device| at |offset|. If the write fails,
>> > +// sets the error message in |err|.
>> > +bool write_misc_partition(const void* p, size_t size, const std::string& misc_blk_device,
>> > + size_t offset, std::string* err);
>> > +
>> > // Read bootloader message into boot. Error message will be set in err.
>> > bool read_bootloader_message(bootloader_message* boot, std::string* err);
>> >
>> > @@ -242,6 +311,17 @@ bool read_wipe_package(std::string* package_data, size_t size, std::string* err)
>> > // Write the wipe package into BCB (to offset WIPE_PACKAGE_OFFSET_IN_MISC).
>> > bool write_wipe_package(const std::string& package_data, std::string* err);
>> >
>> > +// Read or write the Virtual A/B message from system space in /misc.
>> > +bool ReadMiscVirtualAbMessage(misc_virtual_ab_message* message, std::string* err);
>> > +bool WriteMiscVirtualAbMessage(const misc_virtual_ab_message& message, std::string* err);
>> > +
>> > +// Read or write the memtag message from system space in /misc.
>> > +bool ReadMiscMemtagMessage(misc_memtag_message* message, std::string* err);
>> > +bool WriteMiscMemtagMessage(const misc_memtag_message& message, std::string* err);
>> > +
>> > +// Read or write the kcmdline message from system space in /misc.
>> > +bool ReadMiscKcmdlineMessage(misc_kcmdline_message* message, std::string* err);
>> > +bool WriteMiscKcmdlineMessage(const misc_kcmdline_message& message, std::string* err);
>>
>> AFAIU, these new prototypes brought by this patch don't have actual
>> implementation yet. Does it affect the build somehow, e.g. maybe
>> causing some build warnings? Or is the build log clean?
>
> They are all wrapped with #ifndef __UBOOT__.
>
>
>>
>> > #else
>> >
>> > #include <stdbool.h>
>> > --
>> > 2.34.1
>> >
> Thanks for your comments!
>
>
> --
> Best regards - Freundliche Grüsse - Meilleures salutations
>
> Igor Opaniuk
> Senior Software Engineer, Embedded & Security
> E: igor.opaniuk@foundries.io
> W: www.foundries.io
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-07 9:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-19 10:15 [PATCH v1] include: android_bootloader_message.h: sync with AOSP upstream Igor Opaniuk
2024-02-20 18:29 ` Sam Protsenko
2024-02-20 19:08 ` Igor Opaniuk
2024-03-07 9:32 ` Mattijs Korpershoek
2024-02-29 19:05 ` Igor Opaniuk
2024-03-07 9:29 ` Mattijs Korpershoek
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.