Changed header structure and added script

Signed-off-by: Yonatan Schachter <yonatan.schachter@gmail.com>

include/zephyr/bin_descriptors.h

@@ -12,18 +12,7 @@ extern "C" {
 #endif
 
 #define BIN_DESCRIPTOR_MAGIC 0x2c31b9b9
-
-#ifndef _LINKER
-/**
- * @brief A union for holding the value of the binary descriptor, which
- * can be either an integer or a string.
- */
-union bin_descriptor_value {
-	/** Value as an integer */
-	uint32_t integer;
-	/** Value as a pointer to a char */
-	const char *str;
-};
+#define BIN_DESCRIPTOR_ALIGNMENT 4
 
 /** The app version string such as "1.2.3" */
 #define BIN_DESC_TYPE_APP_VERSION_STRING 0x1000
@@ -61,15 +50,21 @@ union bin_descriptor_value {
 /** The time of compilation such as "00:07:04" */
 #define BIN_DESC_TYPE_BUILD_TIME_STRING 0x100b
 
+#define BIN_DESC_TYPE_DESCRIPTORS_END 0xffffffff
+
+#ifndef _LINKER
+
 /**
  * @brief An entry of the binary descriptor header. Each descriptor is
  * described by one of these entry.
  */
-struct bin_descriptor_entry {
+struct bin_descriptor {
 	/** Identifier of the entry */
 	uint32_t id;
+	/** Length of the descriptor data */
+	uint32_t len;
 	/** Value of the entry. This is either an integer or a char pointer */
-	union bin_descriptor_value value;
+	uint8_t data[];
 };
 
 /**
@@ -79,8 +74,8 @@ struct bin_descriptor_entry {
  * Convenience helper for declaring a binary description entry.
  */
 #define DECLARE_BIN_DESC_ENTRY(name) \
-	Z_DECL_ALIGN(struct bin_descriptor_entry) _##name \
-	__in_section(_##bin_descriptor_entry, static, name) __used __noasan
+	Z_DECL_ALIGN(struct bin_descriptor) _##name \
+	__in_section(_##bin_descriptor, static, name) __used __noasan
 
 /**
  * @brief Declare a binary descriptor of type string.
@@ -93,11 +88,12 @@ struct bin_descriptor_entry {
  * @note The declared string is not static, so its name must not collide with
  * any other symbol in the executable.
  */
-#define DECLARE_BIN_DESC_STRING(name, _id, _value) \
-	const char *name = _value; \
+#define DECLARE_BIN_DESC_STRING(name, _id, value) \
+	const char *name = value; \
 	DECLARE_BIN_DESC_ENTRY(name) = { \
 		.id = _id, \
-		.value = { .str = _value } \
+		.len = sizeof(value), \
+		.data = value \
 	}
 
 /**
@@ -111,11 +107,12 @@ struct bin_descriptor_entry {
  * @note The declared integer is not static, so its name must not collide with
  * any other symbol in the executable.
  */
-#define DECLARE_BIN_DESC_INTEGER(name, _id, _value) \
-	const uint32_t name = _value; \
+#define DECLARE_BIN_DESC_INTEGER(name, _id, value) \
+	const uint32_t name = (uint32_t)value; \
 	DECLARE_BIN_DESC_ENTRY(name) = { \
 		.id = _id, \
-		.value = { .integer = _value } \
+		.len = sizeof(uint32_t), \
+		.data = (uint8_t[sizeof(uint32_t)])value \
 	}
 
 /**
@@ -126,7 +123,7 @@ struct bin_descriptor_entry {
  * @param value The value of the binary descriptor
  * @param user_data The same user data that was passed to @ref bin_descriptors_foreach
  */
-typedef void (*bin_descriptors_foreach_cb)(uint32_t id, union bin_descriptor_value value,
+typedef void (*bin_descriptors_foreach_cb)(uint32_t id, void *value,
 		void *user_data);
 
 /**
@@ -147,6 +144,8 @@ typedef void (*bin_descriptors_foreach_cb)(uint32_t id, union bin_descriptor_val
 int bin_descriptors_foreach(uint32_t *address, bin_descriptors_foreach_cb callback,
 		void *user_data);
 
+int bin_descriptors_get_by_id(uint32_t *address, uint32_t id, struct bin_descriptor **entry);
+
 int bin_descriptors_search_header(uint32_t *start_address, size_t count, uint32_t **result);
 
 #endif /* _LINKER */

samples/subsys/bin_descriptors/self_version/src/main.c

@@ -6,16 +6,16 @@ struct app_descriptors {
 	const char *version;
 };
 
-void callback(uint32_t id, union bin_descriptor_value value, void *user_data)
+void callback(uint32_t id, void *value, void *user_data)
 {
 	struct app_descriptors *result = (struct app_descriptors *)user_data;
 
 	switch (id) {
 	case BIN_DESC_TYPE_BUILD_DATE_TIME_STRING:
-		result->build_timestamp = value.str;
+		result->build_timestamp = (const char *)value;
 		break;
 	case BIN_DESC_TYPE_KERNEL_VERSION_STRING:
-		result->version = value.str;
+		result->version = (const char *)value;
 		break;
 	default:
 		break;
@@ -40,6 +40,7 @@ void main(void)
 {
 	uint32_t *address;
 	struct app_descriptors descriptors;
+	struct bin_descriptor *entry;
 
 	if (bin_descriptors_search_header(SEARCH_ADDRESS, SEARCH_LENGTH, &address)) {
 		printk("Not found!\n");
@@ -52,4 +53,10 @@ void main(void)
 	}
 	printk("Application build time: %s\n", descriptors.build_timestamp);
 	printk("Application kernel version: %s\n", descriptors.version);
+
+	if (bin_descriptors_get_by_id(address, BIN_DESC_TYPE_BUILD_DATE_TIME_STRING, &entry)) {
+		printk("Invalid magic!\n");
+		return;
+	}
+	printk("ID: %x len: %x data: %s\n", entry->id, entry->len, (const char *)(entry->data));
 }

scripts/bin_descriptors/inspect_image.py

@@ -0,0 +1,75 @@
+import argparse
+import struct
+
+BIN_DESCRIPTOR_MAGIC = 0x2c31b9b9
+BIN_DESCRIPTOR_ALIGNMENT = 4
+
+TYPE_STR = 0
+TYPE_UINT = 1
+
+BIN_DESCRIPTOR_IDS = {
+    0x1000: ("APP_VERSION_STRING", TYPE_STR),
+    0x1001: ("APP_VERSION_MAJOR", TYPE_UINT),
+    0x1002: ("APP_VERSION_MINOR", TYPE_UINT),
+    0x1003: ("APP_VERSION_REVISION", TYPE_UINT),
+    0x1004: ("KERNEL_VERSION_STRING", TYPE_STR),
+    0x1005: ("KERNEL_VERSION_MAJOR", TYPE_UINT),
+    0x1006: ("KERNEL_VERSION_MINOR", TYPE_UINT),
+    0x1007: ("KERNEL_VERSION_PATCHLEVEL", TYPE_UINT),
+    0x1008: ("KERNEL_VERSION_NUMBER", TYPE_UINT),
+    0x1009: ("BUILD_DATE_TIME_STRING", TYPE_STR),
+    0x100a: ("BUILD_DATE_STRING", TYPE_STR),
+    0x100b: ("BUILD_TIME_STRING", TYPE_STR),
+}
+
+BIN_DESC_TYPE_DESCRIPTORS_END = 0xffffffff
+
+def bytes_to_uint(b):
+    return struct.unpack("I", b)[0]
+
+def align(x, alignment):
+    return (x + alignment - 1) & (~(alignment - 1))
+
+def read_descriptors(filename):
+    with open(filename, "rb") as f:
+        binary_data = f.read()
+
+    index = binary_data.find(struct.pack("I", BIN_DESCRIPTOR_MAGIC))
+    if index == -1:
+        print("Could not find binary descriptor magic")
+
+    descriptors = []
+
+    index += 4 # index points to first descriptor
+    current_id = bytes_to_uint(binary_data[index:index+4])
+    while current_id != BIN_DESC_TYPE_DESCRIPTORS_END:
+        index += 4 # index points to length
+        length = bytes_to_uint(binary_data[index:index+4])
+        index += 4 # index points to data
+        data = binary_data[index:index+length]
+
+        if BIN_DESCRIPTOR_IDS[current_id][1] == TYPE_STR:
+            parsed_data = data[:-1].decode("ascii")
+        else:
+            parsed_data = bytes_to_uint(data)
+        descriptors.append((BIN_DESCRIPTOR_IDS[current_id][0], parsed_data))
+        index += length
+        index = align(index, 4)
+        current_id = bytes_to_uint(binary_data[index:index+4])
+
+    return descriptors
+
+
+def main():
+    parser = argparse.ArgumentParser()
+
+    parser.add_argument('filename',
+                        help='Binary image'
+                        )
+    args = parser.parse_args()
+    descriptors = read_descriptors(args.filename)
+    print(descriptors)
+
+
+if __name__ == "__main__":
+    main()

subsys/bin_descriptors/bin_descriptors.c

@@ -1,39 +1,68 @@
 #include <zephyr/kernel.h>
 #include <zephyr/bin_descriptors.h>
+#include <zephyr/sys/util.h>
 
-int bin_descriptors_foreach(uint32_t *address, bin_descriptors_foreach_cb callback,
-				void *user_data)
+#define BIN_DESC_ALIGN(x) (struct bin_descriptor *)ROUND_UP((x), BIN_DESCRIPTOR_ALIGNMENT)
+
+static int is_valid_range(uint32_t *address, size_t count)
+{
+#ifdef CONFIG_QEMU_TARGET
+	return true;
+#else
+	return !((address < (uint32_t *)CONFIG_FLASH_BASE_ADDRESS) ||
+		(address >= (uint32_t *)CONFIG_FLASH_BASE_ADDRESS + CONFIG_FLASH_SIZE) ||
+		(address + count >= (uint32_t *)CONFIG_FLASH_BASE_ADDRESS + CONFIG_FLASH_SIZE));
+#endif
+}
+
+static int bin_descriptors_walk(uint32_t *address, bin_descriptors_foreach_cb callback,
+				void *user_data, bool search, uint32_t search_id,
+				struct bin_descriptor **bd)
 {
-	struct bin_descriptor_entry *iterator;
-	size_t descriptor_count;
+	struct bin_descriptor *iterator;
 	uint32_t magic;
-	size_t i;
 
 	magic = (uint32_t)*address++;
 	if (magic != BIN_DESCRIPTOR_MAGIC) {
 		return -EINVAL;
 	}
 
-	descriptor_count = (uint32_t)*address++;
-
-	for (i = 0; i < descriptor_count; i++) {
-		iterator = ((struct bin_descriptor_entry *)address) + i;
-		callback(iterator->id, iterator->value, user_data);
+	iterator = ((struct bin_descriptor *)address);
+	while (iterator->id != BIN_DESC_TYPE_DESCRIPTORS_END) {
+		if (search) {
+			if (iterator->id == search_id) {
+				*bd = iterator;
+				return 0;
+			}
+		} else {
+			callback(iterator->id, &(iterator->data), user_data);
+		}
+		iterator = BIN_DESC_ALIGN((uint8_t *)iterator +
+				sizeof(struct bin_descriptor) + iterator->len);
 	}
 
-	return 0;
+	return search ? -1 : 0;
+}
+
+int bin_descriptors_foreach(uint32_t *address, bin_descriptors_foreach_cb callback,
+				void *user_data)
+{
+	return bin_descriptors_walk(address, callback, user_data, false, 0, NULL);
+}
+
+int bin_descriptors_get_by_id(uint32_t *address, uint32_t id, struct bin_descriptor **entry)
+{
+	return bin_descriptors_walk(address, NULL, NULL, true, id, entry);
 }
 
+
 int bin_descriptors_search_header(uint32_t *start_address, size_t count, uint32_t **result)
 {
 	size_t i;
 
-	// if ((start_address < (uint32_t *)CONFIG_FLASH_BASE_ADDRESS) ||
-	// 	(start_address >= (uint32_t *)CONFIG_FLASH_BASE_ADDRESS + CONFIG_FLASH_SIZE) ||
-	// 	(start_address + count >= (uint32_t *)CONFIG_FLASH_BASE_ADDRESS + CONFIG_FLASH_SIZE)) {
-	// 	printk("invalid range\n");
-	// 	return -EINVAL;
-	// }
+	if (!is_valid_range(start_address, count)) {
+		return -EINVAL;
+	}
 
 	for (i = 0; i < count; i++) {
 		if (*(start_address + i) == BIN_DESCRIPTOR_MAGIC) {
@@ -42,6 +71,5 @@ int bin_descriptors_search_header(uint32_t *start_address, size_t count, uint32_
 		}
 	}
 
-	printk("not found\n");
 	return -1;
 }

subsys/bin_descriptors/bin_descriptors.ld

@@ -1,7 +1,6 @@
 #include <zephyr/bin_descriptors.h>
 
 LONG(BIN_DESCRIPTOR_MAGIC);
-LONG((_bin_desc_end - _bin_desc_start) / 8);
-_bin_desc_start = .;
-Z_LINK_ITERABLE(bin_descriptor_entry);
-_bin_desc_end = .;
+Z_LINK_ITERABLE(bin_descriptor);
+. = ALIGN(BIN_DESCRIPTOR_ALIGNMENT);
+LONG(BIN_DESC_TYPE_DESCRIPTORS_END)