All of lore.kernel.org
 help / color / mirror / Atom feed
* [cbootimage PATCH V2 0/5] Re-enable jtag function for Tegra124
@ 2014-04-08 14:30 Penny Chiu
       [not found] ` <1396967422-6018-1-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Penny Chiu @ 2014-04-08 14:30 UTC (permalink / raw)
  To: swarren-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: Penny Chiu

This patch series is used to update BCT configs to re-enable jtag function
for Tegra124.

Since the secure jtag is disabled by default once SECURITY_MODE fuse is
blown. BOOTROM will re-enable secure jtag function if BCT has
SecureJTAGControl=1 and a matching chip UID included.

After applying these changes, cbootimage can access jtag control and chip uid
fields. It can also read the BCT data from pre-built image, update the BCT
configs based on config file, and generate a new image file.

Changes from V1:
 - Improvement based on reviewed comments.

Penny Chiu (5):
  Add format functions to express BCT and bootloader data value
  Accept void pointer as input data type for get/set_value functions
  Add token_supported function
  Add Tegra124 bct data access for jtag control and chip uid
  Add update BCT configs feature

 src/bct_dump.c           | 103 ++++++++++++++++++++++++++++++++++-------------
 src/cbootimage.c         |  91 ++++++++++++++++++++++++++++++++++-------
 src/cbootimage.h         |  12 +++++-
 src/context.c            |  10 +++--
 src/data_layout.c        |  94 ++++++++++++++++++++++++++++++++++--------
 src/data_layout.h        |   9 +++++
 src/parse.c              |  72 +++++++++++++++++++++++++++++++++
 src/parse.h              |  16 ++++++--
 src/set.c                |  72 ++++++++++++++++++++++-----------
 src/set.h                |   4 +-
 src/t114/nvbctlib_t114.c |  47 ++++++++++++++++-----
 src/t124/nvbctlib_t124.c |  60 ++++++++++++++++++++++-----
 src/t20/nvbctlib_t20.c   |  47 ++++++++++++++++-----
 src/t30/nvbctlib_t30.c   |  47 ++++++++++++++++-----
 14 files changed, 552 insertions(+), 132 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [cbootimage PATCH V2 1/5] Add format functions to express BCT and bootloader data value
       [not found] ` <1396967422-6018-1-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2014-04-08 14:30   ` Penny Chiu
  2014-04-08 14:30   ` [cbootimage PATCH V2 2/5] Accept void pointer as input data type for get/set_value functions Penny Chiu
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Penny Chiu @ 2014-04-08 14:30 UTC (permalink / raw)
  To: swarren-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: Penny Chiu

Add a new field to the value_data table, which is the function to
use to format the data value.

Signed-off-by: Penny Chiu <pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 src/bct_dump.c | 59 +++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/src/bct_dump.c b/src/bct_dump.c
index dbef913..6d99214 100644
--- a/src/bct_dump.c
+++ b/src/bct_dump.c
@@ -27,37 +27,54 @@
 int enable_debug;
 cbootimage_soc_config * g_soc_config;
 
+static void format_u32_hex8(char const * message, void * data);
+static void format_u32(char const * message, void * data);
+
+typedef void (*format_function)(char const * message, void * data);
+
 typedef struct {
 	parse_token id;
 	char const * message;
+	format_function format;
 } value_data;
 
 static value_data const values[] = {
-	{ token_boot_data_version,   "Version       = 0x%08x;\n" },
-	{ token_block_size_log2,     "BlockSize     = 0x%08x;\n" },
-	{ token_page_size_log2,      "PageSize      = 0x%08x;\n" },
-	{ token_partition_size,      "PartitionSize = 0x%08x;\n" },
-	{ token_odm_data,            "OdmData       = 0x%08x;\n\n" },
-	{ token_bootloader_used,     "# Bootloader used       = %d;\n" },
-	{ token_bootloaders_max,     "# Bootloaders max       = %d;\n" },
-	{ token_bct_size,            "# BCT size              = %d;\n" },
-	{ token_hash_size,           "# Hash size             = %d;\n" },
-	{ token_crypto_offset,       "# Crypto offset         = %d;\n" },
-	{ token_crypto_length,       "# Crypto length         = %d;\n" },
-	{ token_max_bct_search_blks, "# Max BCT search blocks = %d;\n" },
+	{ token_boot_data_version,   "Version       = ", format_u32_hex8 },
+	{ token_block_size_log2,     "BlockSize     = ", format_u32_hex8 },
+	{ token_page_size_log2,      "PageSize      = ", format_u32_hex8 },
+	{ token_partition_size,      "PartitionSize = ", format_u32_hex8 },
+	{ token_odm_data,            "OdmData       = ", format_u32_hex8 },
+	{ token_bootloader_used,     "# Bootloader used       = ", format_u32 },
+	{ token_bootloaders_max,     "# Bootloaders max       = ", format_u32 },
+	{ token_bct_size,            "# BCT size              = ", format_u32 },
+	{ token_hash_size,           "# Hash size             = ", format_u32 },
+	{ token_crypto_offset,       "# Crypto offset         = ", format_u32 },
+	{ token_crypto_length,       "# Crypto length         = ", format_u32 },
+	{ token_max_bct_search_blks, "# Max BCT search blocks = ", format_u32 },
 };
 
 static value_data const bl_values[] = {
-	{ token_bl_version,     "Version      = 0x%08x;\n" },
-	{ token_bl_start_blk,   "Start block  = %d;\n" },
-	{ token_bl_start_page,  "Start page   = %d;\n" },
-	{ token_bl_length,      "Length       = %d;\n" },
-	{ token_bl_load_addr,   "Load address = 0x%08x;\n" },
-	{ token_bl_entry_point, "Entry point  = 0x%08x;\n" },
-	{ token_bl_attribute,   "Attributes   = 0x%08x;\n" },
+	{ token_bl_version,     "Version      = ", format_u32_hex8 },
+	{ token_bl_start_blk,   "Start block  = ", format_u32 },
+	{ token_bl_start_page,  "Start page   = ", format_u32 },
+	{ token_bl_length,      "Length       = ", format_u32 },
+	{ token_bl_load_addr,   "Load address = ", format_u32_hex8 },
+	{ token_bl_entry_point, "Entry point  = ", format_u32_hex8 },
+	{ token_bl_attribute,   "Attributes   = ", format_u32_hex8 },
 };
 
 /*****************************************************************************/
+static void format_u32_hex8(char const * message, void * data)
+{
+	printf("%s0x%08x;\n", message, *((u_int32_t *) data));
+}
+
+static void format_u32(char const * message, void * data)
+{
+	printf("%s%d;\n", message, *((u_int32_t *) data));
+}
+
+/*****************************************************************************/
 static void usage(void)
 {
 	printf("Usage: bct_dump bctfile\n");
@@ -164,7 +181,7 @@ int main(int argc, char *argv[])
 			 values[i].id == token_page_size_log2)
 			data = 1 << data;
 
-		printf(values[i].message, data);
+		values[i].format(values[i].message, &data);
 	}
 
 	/* Display bootloader values */
@@ -192,7 +209,7 @@ int main(int argc, char *argv[])
 				if (e != 0)
 					data = -1;
 
-				printf(bl_values[j].message, data);
+				bl_values[j].format(bl_values[j].message, &data);
 			}
 		}
 	}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [cbootimage PATCH V2 2/5] Accept void pointer as input data type for get/set_value functions
       [not found] ` <1396967422-6018-1-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2014-04-08 14:30   ` [cbootimage PATCH V2 1/5] Add format functions to express BCT and bootloader data value Penny Chiu
@ 2014-04-08 14:30   ` Penny Chiu
       [not found]     ` <1396967422-6018-3-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2014-04-08 14:30   ` [cbootimage PATCH V2 3/5] Add token_supported function Penny Chiu
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Penny Chiu @ 2014-04-08 14:30 UTC (permalink / raw)
  To: swarren-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: Penny Chiu

This change uses void * as input data type for
cbootimage_soc_config.get/set_value and context_set_value functions.
This makes the functions can accept various data types based on
different tokens.

Signed-off-by: Penny Chiu <pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 src/context.c            | 10 +++++++---
 src/data_layout.c        |  8 +++++---
 src/parse.c              |  2 +-
 src/parse.h              |  6 +++---
 src/set.c                | 32 ++++++++++++++++----------------
 src/set.h                |  2 +-
 src/t114/nvbctlib_t114.c | 20 ++++++++++----------
 src/t124/nvbctlib_t124.c | 22 +++++++++++-----------
 src/t20/nvbctlib_t20.c   | 20 ++++++++++----------
 src/t30/nvbctlib_t30.c   | 20 ++++++++++----------
 10 files changed, 74 insertions(+), 68 deletions(-)

diff --git a/src/context.c b/src/context.c
index 25b322f..f485334 100644
--- a/src/context.c
+++ b/src/context.c
@@ -31,12 +31,16 @@ cleanup_context(build_image_context *context)
 int
 init_context(build_image_context *context)
 {
+	u_int32_t value;
+
 	/* Set defaults */
 	context->memory = new_block_list();
 	context->next_bct_blk = 0; /* Default to block 0 */
-	context_set_value(context, token_redundancy, 1);
-	context_set_value(context, token_version, 1);
-	context_set_value(context, token_bct_copy, 2);
+	value = 1;
+	context_set_value(context, token_redundancy, &value);
+	context_set_value(context, token_version, &value);
+	value = 2;
+	context_set_value(context, token_bct_copy, &value);
 
 	return 0;
 }
diff --git a/src/data_layout.c b/src/data_layout.c
index ba3361a..0a64ec2 100644
--- a/src/data_layout.c
+++ b/src/data_layout.c
@@ -366,6 +366,7 @@ write_bootloaders(build_image_context *context)
 	u_int32_t current_blk;
 	u_int32_t current_page;
 	u_int32_t  pages_in_bl;
+	u_int32_t bootloader_used;
 	u_int8_t  *bl_storage; /* Holds the Bl after reading */
 	u_int8_t  *buffer;	/* Holds the Bl for writing */
 	u_int8_t  *src;	/* Scans through the Bl during writing */
@@ -554,8 +555,9 @@ write_bootloaders(build_image_context *context)
 		free(buffer);
 	}
 
+	bootloader_used = context->redundancy + bl_move_count;
 	g_soc_config->set_value(token_bootloader_used,
-			context->redundancy + bl_move_count,
+			&bootloader_used,
 			context->bct);
 
 	if (enable_debug) {
@@ -752,7 +754,7 @@ begin_update(build_image_context *context)
 	}
 
 	g_soc_config->set_value(token_boot_data_version,
-			context->boot_data_version, context->bct);
+			&(context->boot_data_version), context->bct);
 	g_soc_config->get_value(token_hash_size,
 			&hash_size, context->bct);
 	g_soc_config->get_value(token_reserved_size,
@@ -761,7 +763,7 @@ begin_update(build_image_context *context)
 			&reserved_offset, context->bct);
 	/* Set the odm data */
 	g_soc_config->set_value(token_odm_data,
-			context->odm_data, context->bct);
+			&(context->odm_data), context->bct);
 
 	/* Initialize the bad block table field. */
 	g_soc_config->init_bad_block_table(context);
diff --git a/src/parse.c b/src/parse.c
index 464ee8f..f82c008 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -482,7 +482,7 @@ static int parse_value_u32(build_image_context *context,
 	if (rest == NULL)
 		return 1;
 
-	return context_set_value(context, token, value);
+	return context_set_value(context, token, &value);
 }
 
 /*
diff --git a/src/parse.h b/src/parse.h
index 80f42c4..18c2a87 100644
--- a/src/parse.h
+++ b/src/parse.h
@@ -717,12 +717,12 @@ typedef struct cbootimage_soc_config_rec {
 	 * Set the specified bct value stored in context bct data structure.
 	 *
 	 * @param id  	The parse token value
-	 * @param data	Value to set
+	 * @param data	Pointer of value to set
 	 * @param bct 	Bct pointer
 	 * @return 0 and -ENODATA for success and failure
 	 */
 	int (*set_value)(parse_token id,
-			u_int32_t  data,
+			void *data,
 			u_int8_t *bct);
 	/*
 	 * Get the specified bct value or some constant value of clocks and
@@ -734,7 +734,7 @@ typedef struct cbootimage_soc_config_rec {
 	 * @return 0 and -ENODATA for success and failure
 	 */
 	int (*get_value)(parse_token id,
-			u_int32_t *data,
+			void *data,
 			u_int8_t *bct);
 	/*
 	 * Set the bct crypto hash data.
diff --git a/src/set.c b/src/set.c
index 08c9fb6..3d62254 100644
--- a/src/set.c
+++ b/src/set.c
@@ -122,37 +122,37 @@ set_bootloader(build_image_context	*context,
  *
  * @param context	The main context pointer
  * @param token  	The parse token value
- * @param value  	The value to set
+ * @param value  	The pointer of value to set
  * @return 0 for success
  */
 int context_set_value(build_image_context *context,
 		parse_token token,
-		u_int32_t value)
+		void *value)
 {
 	assert(context != NULL);
 
 	switch (token) {
 	case token_attribute:
-		context->newbl_attr = value;
+		context->newbl_attr = *((u_int32_t *)value);
 		break;
 
 	case token_block_size:
-		context->block_size = value;
-		context->block_size_log2 = log2(value);
+		context->block_size = *((u_int32_t *)value);
+		context->block_size_log2 = log2(*((u_int32_t *)value));
 
 		if (context->memory != NULL) {
 			printf("Error: Too late to change block size.\n");
 			return 1;
 		}
 
-		if (value != (u_int32_t)(1 << context->block_size_log2)) {
+		if (*((u_int32_t *)value) != (u_int32_t)(1 << context->block_size_log2)) {
 			printf("Error: Block size must be a power of 2.\n");
 			return 1;
 		}
 		context->pages_per_blk= 1 << (context->block_size_log2- 
 				context->page_size_log2);
 		g_soc_config->set_value(token_block_size_log2,
-			context->block_size_log2, context->bct);
+			&(context->block_size_log2), context->bct);
 		break;
 
 	case token_partition_size:
@@ -161,34 +161,34 @@ int context_set_value(build_image_context *context,
 			return 1;
 		}
 
-		context->partition_size= value;
+		context->partition_size= *((u_int32_t *)value);
 		g_soc_config->set_value(token_partition_size,
 			value, context->bct);
 		break;
 
 	case token_page_size:
-		context->page_size = value;
-		context->page_size_log2 = log2(value);
+		context->page_size = *((u_int32_t *)value);
+		context->page_size_log2 = log2(*((u_int32_t *)value));
 		context->pages_per_blk= 1 << (context->block_size_log2- 
 			context->page_size_log2);
 
 		g_soc_config->set_value(token_page_size_log2,
-			context->page_size_log2, context->bct);
+			&(context->page_size_log2), context->bct);
 		break;
 	case token_redundancy:
-		context->redundancy = value;
+		context->redundancy = *((u_int32_t *)value);
 		break;
 
 	case token_version:
-		context->version = value;
+		context->version = *((u_int32_t *)value);
 		break;
 
 	case token_bct_copy:
-		context->bct_copy = value;
+		context->bct_copy = *((u_int32_t *)value);
 		break;
 
 	case token_odm_data:
-		context->odm_data = value;
+		context->odm_data = *((u_int32_t *)value);
 		break;
 
 	case token_pre_bct_pad_blocks:
@@ -196,7 +196,7 @@ int context_set_value(build_image_context *context,
 			printf("Error: Too late to pre-BCT pad.\n");
 			return 1;
 		}
-		context->pre_bct_pad_blocks = value;
+		context->pre_bct_pad_blocks = *((u_int32_t *)value);
 		break;
 
 	DEFAULT();
diff --git a/src/set.h b/src/set.h
index 1515fcd..754ed7a 100644
--- a/src/set.h
+++ b/src/set.h
@@ -38,7 +38,7 @@ set_bootloader(build_image_context	*context,
 int
 context_set_value(build_image_context	*context,
 		parse_token	token,
-		u_int32_t	value);
+		void		*value);
 
 int
 read_from_image(char *filename,
diff --git a/src/t114/nvbctlib_t114.c b/src/t114/nvbctlib_t114.c
index f7e449a..29878c1 100644
--- a/src/t114/nvbctlib_t114.c
+++ b/src/t114/nvbctlib_t114.c
@@ -59,22 +59,22 @@ case token_bl_##x:\
 #define CASE_GET_NVU32(id) \
 case token_##id:\
 	if (bct == NULL) return -ENODATA; \
-	*data = bct_ptr->id; \
+	*((u_int32_t *)data) = bct_ptr->id; \
 	break
 
 #define CASE_GET_CONST(id, val) \
 case token_##id:\
-	*data = val; \
+	*((u_int32_t *)data) = val; \
 	break
 
 #define CASE_GET_CONST_PREFIX(id, val_prefix) \
 case token_##id:\
-	*data = val_prefix##_##id; \
+	*((u_int32_t *)data) = val_prefix##_##id; \
 	break
 
 #define CASE_SET_NVU32(id) \
 case token_##id:\
-	bct_ptr->id = data; \
+	bct_ptr->id = *((u_int32_t *)data); \
 	break
 
 #define CASE_GET_DATA(id, size) \
@@ -901,7 +901,7 @@ t114_setbl_param(u_int32_t set,
 }
 
 int
-t114_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
+t114_bct_get_value(parse_token id, void *data, u_int8_t *bct)
 {
 	nvboot_config_table *bct_ptr = (nvboot_config_table *)bct;
 	nvboot_config_table  samplebct; /* Used for computing offsets. */
@@ -940,25 +940,25 @@ t114_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
 		break;
 
 	case token_reserved_offset:
-		*data = (u_int8_t *)&(samplebct.reserved)
+		*((u_int32_t *)data) = (u_int8_t *)&(samplebct.reserved)
 				- (u_int8_t *)&samplebct;
 		break;
 
 	case token_bct_size:
-		*data = sizeof(nvboot_config_table);
+		*((u_int32_t *)data) = sizeof(nvboot_config_table);
 		break;
 
 	CASE_GET_CONST(hash_size, sizeof(nvboot_hash));
 
 	case token_crypto_offset:
 		/* Offset to region in BCT to encrypt & sign */
-		*data = (u_int8_t *)&(samplebct.random_aes_blk)
+		*((u_int32_t *)data) = (u_int8_t *)&(samplebct.random_aes_blk)
 				- (u_int8_t *)&samplebct;
 		break;
 
 	case token_crypto_length:
 		/* size of region in BCT to encrypt & sign */
-		*data = (u_int8_t *)bct_ptr + sizeof(nvboot_config_table)
+		*((u_int32_t *)data) = (u_int8_t *)bct_ptr + sizeof(nvboot_config_table)
 				- (u_int8_t *)&(bct_ptr->random_aes_blk);
 		break;
 
@@ -985,7 +985,7 @@ t114_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
 }
 
 int
-t114_bct_set_value(parse_token id, u_int32_t data, u_int8_t *bct)
+t114_bct_set_value(parse_token id, void *data, u_int8_t *bct)
 {
 	nvboot_config_table *bct_ptr = (nvboot_config_table *)bct;
 
diff --git a/src/t124/nvbctlib_t124.c b/src/t124/nvbctlib_t124.c
index 27e5a62..ec5c3a2 100644
--- a/src/t124/nvbctlib_t124.c
+++ b/src/t124/nvbctlib_t124.c
@@ -60,22 +60,22 @@ case token_bl_##x:\
 case token_##id:\
 	if (bct == NULL) \
 		return -ENODATA; \
-	*data = bct_ptr->id; \
+	*((u_int32_t *)data) = bct_ptr->id; \
 	break
 
 #define CASE_GET_CONST(id, val) \
 case token_##id:\
-	*data = val; \
+	*((u_int32_t *)data) = val; \
 	break
 
 #define CASE_GET_CONST_PREFIX(id, val_prefix) \
 case token_##id:\
-	*data = val_prefix##_##id; \
+	*((u_int32_t *)data) = val_prefix##_##id; \
 	break
 
 #define CASE_SET_NVU32(id) \
 case token_##id:\
-	bct_ptr->id = data; \
+	bct_ptr->id = *((u_int32_t *)data); \
 	break
 
 #define CASE_GET_DATA(id, size) \
@@ -902,7 +902,7 @@ t124_setbl_param(u_int32_t set,
 }
 
 int
-t124_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
+t124_bct_get_value(parse_token id, void *data, u_int8_t *bct)
 {
 	nvboot_config_table *bct_ptr = (nvboot_config_table *)bct;
 	nvboot_config_table  samplebct; /* Used for computing offsets. */
@@ -941,25 +941,25 @@ t124_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
 		break;
 
 	case token_reserved_offset:
-		*data = (u_int8_t *)&(samplebct.reserved)
+		*((u_int32_t *)data) = (u_int8_t *)&(samplebct.reserved)
 				- (u_int8_t *)&samplebct;
 		break;
 
 	case token_bct_size:
-		*data = sizeof(nvboot_config_table);
+		*((u_int32_t *)data) = sizeof(nvboot_config_table);
 		break;
 
 	CASE_GET_CONST(hash_size, sizeof(nvboot_hash));
 
 	case token_crypto_offset:
 		/* Offset to region in BCT to encrypt & sign */
-		*data = (u_int8_t *)&(samplebct.random_aes_blk)
+		*((u_int32_t *)data) = (u_int8_t *)&(samplebct.random_aes_blk)
 				- (u_int8_t *)&samplebct;
 		break;
 
 	case token_crypto_length:
 		/* size of region in BCT to encrypt & sign */
-		*data = (u_int8_t *)bct_ptr + sizeof(nvboot_config_table)
+		*((u_int32_t *)data) = (u_int8_t *)bct_ptr + sizeof(nvboot_config_table)
 				- (u_int8_t *)&(bct_ptr->random_aes_blk);
 		break;
 
@@ -986,11 +986,11 @@ t124_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
 }
 
 int
-t124_bct_set_value(parse_token id, u_int32_t data, u_int8_t *bct)
+t124_bct_set_value(parse_token id, void *data, u_int8_t *bct)
 {
 	nvboot_config_table *bct_ptr = (nvboot_config_table *)bct;
 
-	if (bct == NULL)
+	if (data == NULL || bct == NULL)
 		return -ENODATA;
 
 	switch (id) {
diff --git a/src/t20/nvbctlib_t20.c b/src/t20/nvbctlib_t20.c
index c145d6d..91fe9e6 100644
--- a/src/t20/nvbctlib_t20.c
+++ b/src/t20/nvbctlib_t20.c
@@ -59,22 +59,22 @@ case token_bl_##x:\
 #define CASE_GET_NVU32(id) \
 case token_##id:\
 	if (bct == NULL) return -ENODATA; \
-	*data = bct_ptr->id; \
+	*((u_int32_t *)data) = bct_ptr->id; \
 	break
 
 #define CASE_GET_CONST(id, val) \
 case token_##id:\
-	*data = val; \
+	*((u_int32_t *)data) = val; \
 	break
 
 #define CASE_GET_CONST_PREFIX(id, val_prefix) \
 case token_##id:\
-	*data = val_prefix##_##id; \
+	*((u_int32_t *)data) = val_prefix##_##id; \
 	break
 
 #define CASE_SET_NVU32(id) \
 case token_##id:\
-	bct_ptr->id = data; \
+	bct_ptr->id = *((u_int32_t *)data); \
 	break
 
 #define CASE_GET_DATA(id, size) \
@@ -490,7 +490,7 @@ t20_setbl_param(u_int32_t set,
 }
 
 int
-t20_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
+t20_bct_get_value(parse_token id, void *data, u_int8_t *bct)
 {
 	nvboot_config_table *bct_ptr = (nvboot_config_table *)bct;
 	nvboot_config_table  samplebct; /* Used for computing offsets. */
@@ -523,25 +523,25 @@ t20_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
 	CASE_GET_CONST(reserved_size,     NVBOOT_BCT_RESERVED_SIZE);
 
 	case token_reserved_offset:
-		*data = (u_int8_t *)&(samplebct.reserved)
+		*((u_int32_t *)data) = (u_int8_t *)&(samplebct.reserved)
 				- (u_int8_t *)&samplebct;
 		break;
 
 	case token_bct_size:
-		*data = sizeof(nvboot_config_table);
+		*((u_int32_t *)data) = sizeof(nvboot_config_table);
 		break;
 
 	CASE_GET_CONST(hash_size, sizeof(nvboot_hash));
 
 	case token_crypto_offset:
 		/* Offset to region in BCT to encrypt & sign */
-		*data = (u_int8_t *)&(samplebct.random_aes_blk)
+		*((u_int32_t *)data) = (u_int8_t *)&(samplebct.random_aes_blk)
 				- (u_int8_t *)&samplebct;
 		break;
 
 	case token_crypto_length:
 		/* size   of region in BCT to encrypt & sign */
-		*data = sizeof(nvboot_config_table) - sizeof(nvboot_hash);
+		*((u_int32_t *)data) = sizeof(nvboot_config_table) - sizeof(nvboot_hash);
 	break;
 
 	CASE_GET_CONST(max_bct_search_blks, NVBOOT_MAX_BCT_SEARCH_BLOCKS);
@@ -569,7 +569,7 @@ t20_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
 }
 
 int
-t20_bct_set_value(parse_token id, u_int32_t  data, u_int8_t *bct)
+t20_bct_set_value(parse_token id, void *data, u_int8_t *bct)
 {
 	nvboot_config_table *bct_ptr = (nvboot_config_table *)bct;
 
diff --git a/src/t30/nvbctlib_t30.c b/src/t30/nvbctlib_t30.c
index 59b0246..a84b7c7 100644
--- a/src/t30/nvbctlib_t30.c
+++ b/src/t30/nvbctlib_t30.c
@@ -59,22 +59,22 @@ case token_bl_##x:\
 #define CASE_GET_NVU32(id) \
 case token_##id:\
 	if (bct == NULL) return -ENODATA; \
-	*data = bct_ptr->id; \
+	*((u_int32_t *)data) = bct_ptr->id; \
 	break
 
 #define CASE_GET_CONST(id, val) \
 case token_##id:\
-	*data = val; \
+	*((u_int32_t *)data) = val; \
 	break
 
 #define CASE_GET_CONST_PREFIX(id, val_prefix) \
 case token_##id:\
-	*data = val_prefix##_##id; \
+	*((u_int32_t *)data) = val_prefix##_##id; \
 	break
 
 #define CASE_SET_NVU32(id) \
 case token_##id:\
-	bct_ptr->id = data; \
+	bct_ptr->id = *((u_int32_t *)data); \
 	break
 
 #define CASE_GET_DATA(id, size) \
@@ -697,7 +697,7 @@ t30_setbl_param(u_int32_t set,
 }
 
 int
-t30_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
+t30_bct_get_value(parse_token id, void *data, u_int8_t *bct)
 {
 	nvboot_config_table *bct_ptr = (nvboot_config_table *)bct;
 	nvboot_config_table  samplebct; /* Used for computing offsets. */
@@ -730,25 +730,25 @@ t30_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
 	CASE_GET_CONST(reserved_size,     NVBOOT_BCT_RESERVED_SIZE);
 
 	case token_reserved_offset:
-		*data = (u_int8_t *)&(samplebct.reserved)
+		*((u_int32_t *)data) = (u_int8_t *)&(samplebct.reserved)
 				- (u_int8_t *)&samplebct;
 		break;
 
 	case token_bct_size:
-		*data = sizeof(nvboot_config_table);
+		*((u_int32_t *)data) = sizeof(nvboot_config_table);
 		break;
 
 	CASE_GET_CONST(hash_size, sizeof(nvboot_hash));
 
 	case token_crypto_offset:
 		/* Offset to region in BCT to encrypt & sign */
-		*data = (u_int8_t *)&(samplebct.random_aes_blk)
+		*((u_int32_t *)data) = (u_int8_t *)&(samplebct.random_aes_blk)
 				- (u_int8_t *)&samplebct;
 		break;
 
 	case token_crypto_length:
 		/* size   of region in BCT to encrypt & sign */
-		*data = sizeof(nvboot_config_table) - sizeof(nvboot_hash);
+		*((u_int32_t *)data) = sizeof(nvboot_config_table) - sizeof(nvboot_hash);
 	break;
 
 	CASE_GET_CONST(max_bct_search_blks, NVBOOT_MAX_BCT_SEARCH_BLOCKS);
@@ -776,7 +776,7 @@ t30_bct_get_value(parse_token id, u_int32_t *data, u_int8_t *bct)
 }
 
 int
-t30_bct_set_value(parse_token id, u_int32_t  data, u_int8_t *bct)
+t30_bct_set_value(parse_token id, void *data, u_int8_t *bct)
 {
 	nvboot_config_table *bct_ptr = (nvboot_config_table *)bct;
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [cbootimage PATCH V2 3/5] Add token_supported function
       [not found] ` <1396967422-6018-1-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2014-04-08 14:30   ` [cbootimage PATCH V2 1/5] Add format functions to express BCT and bootloader data value Penny Chiu
  2014-04-08 14:30   ` [cbootimage PATCH V2 2/5] Accept void pointer as input data type for get/set_value functions Penny Chiu
@ 2014-04-08 14:30   ` Penny Chiu
       [not found]     ` <1396967422-6018-4-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2014-04-08 14:30   ` [cbootimage PATCH V2 4/5] Add Tegra124 bct data access for jtag control and chip uid Penny Chiu
  2014-04-08 14:30   ` [cbootimage PATCH V2 5/5] Add update BCT configs feature Penny Chiu
  4 siblings, 1 reply; 10+ messages in thread
From: Penny Chiu @ 2014-04-08 14:30 UTC (permalink / raw)
  To: swarren-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: Penny Chiu

Add a function called token_supported in cbootimage_soc_config.
It is used to check if the input token is supported in specific
tegra soc.

Signed-off-by: Penny Chiu <pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 src/bct_dump.c           |  3 +++
 src/parse.h              |  8 ++++++++
 src/t114/nvbctlib_t114.c | 27 +++++++++++++++++++++++++++
 src/t124/nvbctlib_t124.c | 27 +++++++++++++++++++++++++++
 src/t20/nvbctlib_t20.c   | 27 +++++++++++++++++++++++++++
 src/t30/nvbctlib_t30.c   | 27 +++++++++++++++++++++++++++
 6 files changed, 119 insertions(+)

diff --git a/src/bct_dump.c b/src/bct_dump.c
index 6d99214..a8e3479 100644
--- a/src/bct_dump.c
+++ b/src/bct_dump.c
@@ -171,6 +171,9 @@ int main(int argc, char *argv[])
 
 	/* Display root values */
 	for (i = 0; i < sizeof(values) / sizeof(values[0]); ++i) {
+		if (!g_soc_config->token_supported(values[i].id))
+			continue;
+
 		e = g_soc_config->get_value(values[i].id,
 						&data,
 						context.bct);
diff --git a/src/parse.h b/src/parse.h
index 18c2a87..d9d873f 100644
--- a/src/parse.h
+++ b/src/parse.h
@@ -750,6 +750,14 @@ typedef struct cbootimage_soc_config_rec {
 			u_int32_t  length,
 			u_int8_t *bct);
 
+	/*
+	 * Check if the token is supported to dump
+	 *
+	 * @param id  	The parse token value
+	 * @return 0 and 1 for unsupported and supported
+	 */
+	int (*token_supported)(parse_token id);
+
 	void (*init_bad_block_table)(build_image_context *context);
 
 	enum_item *devtype_table;
diff --git a/src/t114/nvbctlib_t114.c b/src/t114/nvbctlib_t114.c
index 29878c1..56051d2 100644
--- a/src/t114/nvbctlib_t114.c
+++ b/src/t114/nvbctlib_t114.c
@@ -96,6 +96,21 @@ default :                                           \
 		token, __LINE__);                   \
 	return 1
 
+parse_token t114_root_token_list[] = {
+	token_boot_data_version,
+	token_block_size_log2,
+	token_page_size_log2,
+	token_partition_size,
+	token_odm_data,
+	token_bootloader_used,
+	token_bootloaders_max,
+	token_bct_size,
+	token_hash_size,
+	token_crypto_offset,
+	token_crypto_length,
+	token_max_bct_search_blks
+};
+
 int
 t114_set_dev_param(build_image_context *context,
 	u_int32_t index,
@@ -1037,6 +1052,17 @@ t114_bct_set_data(parse_token id,
 	return 0;
 }
 
+int t114_bct_token_supported(parse_token token)
+{
+	int index;
+
+	for (index = 0; index < sizeof(t114_root_token_list)/sizeof(t114_root_token_list[0]); index++)
+		if (t114_root_token_list[index] == token)
+			return 1;
+
+	return 0;
+}
+
 void t114_init_bad_block_table(build_image_context *context)
 {
 	u_int32_t bytes_per_entry;
@@ -1070,6 +1096,7 @@ cbootimage_soc_config tegra114_config = {
 	.set_value					= t114_bct_set_value,
 	.get_value					= t114_bct_get_value,
 	.set_data					= t114_bct_set_data,
+	.token_supported			= t114_bct_token_supported,
 
 	.devtype_table				= s_devtype_table_t114,
 	.sdmmc_data_width_table		= s_sdmmc_data_width_table_t114,
diff --git a/src/t124/nvbctlib_t124.c b/src/t124/nvbctlib_t124.c
index ec5c3a2..b1ec445 100644
--- a/src/t124/nvbctlib_t124.c
+++ b/src/t124/nvbctlib_t124.c
@@ -99,6 +99,21 @@ default :                                           \
 		token, __LINE__);                   \
 	return 1
 
+parse_token t124_root_token_list[] = {
+	token_boot_data_version,
+	token_block_size_log2,
+	token_page_size_log2,
+	token_partition_size,
+	token_odm_data,
+	token_bootloader_used,
+	token_bootloaders_max,
+	token_bct_size,
+	token_hash_size,
+	token_crypto_offset,
+	token_crypto_length,
+	token_max_bct_search_blks
+};
+
 int
 t124_set_dev_param(build_image_context *context,
 	u_int32_t index,
@@ -1039,6 +1054,17 @@ t124_bct_set_data(parse_token id,
 	return 0;
 }
 
+int t124_bct_token_supported(parse_token token)
+{
+	int index;
+
+	for (index = 0; index < sizeof(t124_root_token_list)/sizeof(t124_root_token_list[0]); index++)
+		if (t124_root_token_list[index] == token)
+			return 1;
+
+	return 0;
+}
+
 void t124_init_bad_block_table(build_image_context *context)
 {
 	u_int32_t bytes_per_entry;
@@ -1072,6 +1098,7 @@ cbootimage_soc_config tegra124_config = {
 	.set_value					= t124_bct_set_value,
 	.get_value					= t124_bct_get_value,
 	.set_data					= t124_bct_set_data,
+	.token_supported			= t124_bct_token_supported,
 
 	.devtype_table				= s_devtype_table_t124,
 	.sdmmc_data_width_table		= s_sdmmc_data_width_table_t124,
diff --git a/src/t20/nvbctlib_t20.c b/src/t20/nvbctlib_t20.c
index 91fe9e6..2f54173 100644
--- a/src/t20/nvbctlib_t20.c
+++ b/src/t20/nvbctlib_t20.c
@@ -96,6 +96,21 @@ default :                                                 \
 		token, __LINE__);                         \
 	return 1
 
+parse_token t20_root_token_list[] = {
+	token_boot_data_version,
+	token_block_size_log2,
+	token_page_size_log2,
+	token_partition_size,
+	token_odm_data,
+	token_bootloader_used,
+	token_bootloaders_max,
+	token_bct_size,
+	token_hash_size,
+	token_crypto_offset,
+	token_crypto_length,
+	token_max_bct_search_blks
+};
+
 int
 t20_set_dev_param(build_image_context *context,
 	u_int32_t index,
@@ -618,6 +633,17 @@ t20_bct_set_data(parse_token id,
 	return 0;
 }
 
+int t20_bct_token_supported(parse_token token)
+{
+	int index;
+
+	for (index = 0; index < sizeof(t20_root_token_list)/sizeof(t20_root_token_list[0]); index++)
+		if (t20_root_token_list[index] == token)
+			return 1;
+
+	return 0;
+}
+
 void t20_init_bad_block_table(build_image_context *context)
 {
 	u_int32_t bytes_per_entry;
@@ -651,6 +677,7 @@ cbootimage_soc_config tegra20_config = {
 	.set_value					= t20_bct_set_value,
 	.get_value					= t20_bct_get_value,
 	.set_data					= t20_bct_set_data,
+	.token_supported			= t20_bct_token_supported,
 
 	.devtype_table				= s_devtype_table_t20,
 	.sdmmc_data_width_table		= s_sdmmc_data_width_table_t20,
diff --git a/src/t30/nvbctlib_t30.c b/src/t30/nvbctlib_t30.c
index a84b7c7..e0a7d0d 100644
--- a/src/t30/nvbctlib_t30.c
+++ b/src/t30/nvbctlib_t30.c
@@ -96,6 +96,21 @@ default :                                           \
 		token, __LINE__);                   \
 	return 1
 
+parse_token t30_root_token_list[] = {
+	token_boot_data_version,
+	token_block_size_log2,
+	token_page_size_log2,
+	token_partition_size,
+	token_odm_data,
+	token_bootloader_used,
+	token_bootloaders_max,
+	token_bct_size,
+	token_hash_size,
+	token_crypto_offset,
+	token_crypto_length,
+	token_max_bct_search_blks
+};
+
 int
 t30_set_dev_param(build_image_context *context,
 	u_int32_t index,
@@ -825,6 +840,17 @@ t30_bct_set_data(parse_token id,
 	return 0;
 }
 
+int t30_bct_token_supported(parse_token token)
+{
+	int index;
+
+	for (index = 0; index < sizeof(t30_root_token_list)/sizeof(t30_root_token_list[0]); index++)
+		if (t30_root_token_list[index] == token)
+			return 1;
+
+	return 0;
+}
+
 void t30_init_bad_block_table(build_image_context *context)
 {
 	u_int32_t bytes_per_entry;
@@ -858,6 +884,7 @@ cbootimage_soc_config tegra30_config = {
 	.set_value					= t30_bct_set_value,
 	.get_value					= t30_bct_get_value,
 	.set_data					= t30_bct_set_data,
+	.token_supported			= t30_bct_token_supported,
 
 	.devtype_table				= s_devtype_table_t30,
 	.sdmmc_data_width_table		= s_sdmmc_data_width_table_t30,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [cbootimage PATCH V2 4/5] Add Tegra124 bct data access for jtag control and chip uid
       [not found] ` <1396967422-6018-1-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
                     ` (2 preceding siblings ...)
  2014-04-08 14:30   ` [cbootimage PATCH V2 3/5] Add token_supported function Penny Chiu
@ 2014-04-08 14:30   ` Penny Chiu
       [not found]     ` <1396967422-6018-5-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2014-04-08 14:30   ` [cbootimage PATCH V2 5/5] Add update BCT configs feature Penny Chiu
  4 siblings, 1 reply; 10+ messages in thread
From: Penny Chiu @ 2014-04-08 14:30 UTC (permalink / raw)
  To: swarren-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: Penny Chiu

Add support for read secure_jtag_control and unique_chip_id from
cfg file and write them into BCT structure, and bct_dump can also
parse the two fields and show the data.

Signed-off-by: Penny Chiu <pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 src/bct_dump.c           | 43 ++++++++++++++++++++++++-----
 src/cbootimage.h         |  2 ++
 src/parse.c              | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/parse.h              |  2 ++
 src/set.c                | 12 ++++++++
 src/t124/nvbctlib_t124.c | 13 ++++++++-
 6 files changed, 136 insertions(+), 8 deletions(-)

diff --git a/src/bct_dump.c b/src/bct_dump.c
index a8e3479..1af2c07 100644
--- a/src/bct_dump.c
+++ b/src/bct_dump.c
@@ -29,6 +29,7 @@ cbootimage_soc_config * g_soc_config;
 
 static void format_u32_hex8(char const * message, void * data);
 static void format_u32(char const * message, void * data);
+static void format_chipuid(char const * message, void * data);
 
 typedef void (*format_function)(char const * message, void * data);
 
@@ -44,6 +45,8 @@ static value_data const values[] = {
 	{ token_page_size_log2,      "PageSize      = ", format_u32_hex8 },
 	{ token_partition_size,      "PartitionSize = ", format_u32_hex8 },
 	{ token_odm_data,            "OdmData       = ", format_u32_hex8 },
+	{ token_secure_jtag_control, "JtagCtrl      = ", format_u32_hex8 },
+	{ token_unique_chip_id,      "ChipUid       = ", format_chipuid },
 	{ token_bootloader_used,     "# Bootloader used       = ", format_u32 },
 	{ token_bootloaders_max,     "# Bootloaders max       = ", format_u32 },
 	{ token_bct_size,            "# BCT size              = ", format_u32 },
@@ -74,6 +77,19 @@ static void format_u32(char const * message, void * data)
 	printf("%s%d;\n", message, *((u_int32_t *) data));
 }
 
+static void format_chipuid(char const * message, void * data)
+{
+	u_int8_t *uid = (u_int8_t *)data;
+	int byte_index;
+	char uid_str[35] = "0x";
+	char *s = &uid_str[2];
+
+	for (byte_index = 15; byte_index >= 0; byte_index--, s += 2)
+		sprintf(s, "%02x", uid[byte_index]);
+
+	printf("%s%s;\n", message, uid_str);
+}
+
 /*****************************************************************************/
 static void usage(void)
 {
@@ -174,17 +190,30 @@ int main(int argc, char *argv[])
 		if (!g_soc_config->token_supported(values[i].id))
 			continue;
 
-		e = g_soc_config->get_value(values[i].id,
+		if (values[i].id == token_unique_chip_id) {
+			u_int8_t uid[16];
+
+			e = g_soc_config->get_value(values[i].id,
+						uid,
+						context.bct);
+			if (e != 0)
+				for (j = 0; j < 16; j++)
+					uid[j] = -1;
+
+			values[i].format(values[i].message, uid);
+		} else {
+			e = g_soc_config->get_value(values[i].id,
 						&data,
 						context.bct);
 
-		if (e != 0)
-			data = -1;
-		else if (values[i].id == token_block_size_log2 ||
-			 values[i].id == token_page_size_log2)
-			data = 1 << data;
+			if (e != 0)
+				data = -1;
+			else if (values[i].id == token_block_size_log2 ||
+				 values[i].id == token_page_size_log2)
+				data = 1 << data;
 
-		values[i].format(values[i].message, &data);
+			values[i].format(values[i].message, &data);
+		}
 	}
 
 	/* Display bootloader values */
diff --git a/src/cbootimage.h b/src/cbootimage.h
index 46e3b8b..252c41e 100644
--- a/src/cbootimage.h
+++ b/src/cbootimage.h
@@ -96,6 +96,8 @@ typedef struct build_image_context_rec
 	u_int32_t boot_data_version; /* The boot data version of BCT */
 	u_int8_t bct_init; /* The flag for the memory allocation of bct */
 	u_int32_t odm_data; /* The odm data value */
+	u_int8_t unique_chip_id[16]; /* The unique chip uid */
+	u_int8_t secure_jtag_control; /* The flag for enabling jtag control */
 } build_image_context;
 
 /* Function prototypes */
diff --git a/src/parse.c b/src/parse.c
index f82c008..8e5daed 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -45,6 +45,7 @@ set_array(build_image_context *context,
 			u_int32_t value);
 static char *parse_u32(char *str, u_int32_t *val);
 static char *parse_u8(char *str, u_int32_t *val);
+static char *parse_chipuid(char *str, u_int8_t *val);
 static char *parse_filename(char *str, char *name, int chars_remaining);
 static char *parse_enum(build_image_context *context,
 			char *str,
@@ -64,6 +65,10 @@ parse_bootloader(build_image_context *context, parse_token token, char *rest);
 static int
 parse_value_u32(build_image_context *context, parse_token token, char *rest);
 static int
+parse_value_chipuid(build_image_context *context,
+			parse_token token,
+			char *rest);
+static int
 parse_bct_file(build_image_context *context, parse_token token, char *rest);
 static char
 *parse_end_state(char *str, char *uname, int chars_remaining);
@@ -102,6 +107,8 @@ static parse_item s_top_level_items[] = {
 	{ "Bctcopy=",       token_bct_copy,		parse_value_u32 },
 	{ "Version=",       token_version,		parse_value_u32 },
 	{ "OdmData=",       token_odm_data,		parse_value_u32 },
+	{ "ChipUid=",       token_unique_chip_id,	parse_value_chipuid },
+	{ "JtagCtrl=",	    token_secure_jtag_control,	parse_value_u32 },
 	{ NULL, 0, NULL } /* Must be last */
 };
 
@@ -165,6 +172,46 @@ parse_u8(char *str, u_int32_t *val)
 	return retval;
 }
 
+/*
+ * Parse the given string and transfer to chip uid.
+ *
+ * @param str		String to parse
+ * @param chipuid	Returns chip uid that was parsed
+ * @return the remainder of the string after the number was parsed
+ */
+static char *
+parse_chipuid(char *str, u_int8_t *chipuid)
+{
+	int byte_index = 0;
+	int paddings = 0;
+	char byte_str[3];
+
+	if (*str++ != '0')
+		return NULL;
+
+	if (*str++ != 'x')
+		return NULL;
+
+	paddings = strlen(str) % 2;
+	byte_index = strlen(str) / 2 + paddings;
+
+	while (*str != '\0' && byte_index > 0) {
+		char *endptr;
+
+		strncpy(byte_str, str, 2 - paddings);
+		byte_str[2-paddings] = '\0';
+		str += 2 - paddings;
+
+		chipuid[byte_index - 1] = strtoul(byte_str, &endptr, 16);
+		if (*endptr)
+			return NULL;
+
+		byte_index--;
+		paddings = 0;
+	}
+
+	return str;
+}
 
 /*
  * Parse the given string and find the file name then
@@ -486,6 +533,31 @@ static int parse_value_u32(build_image_context *context,
 }
 
 /*
+ * General handler for setting chip uid in config files.
+ *
+ * @param context	The main context pointer
+ * @param token		The parse token value
+ * @param rest		String to parse
+ * @return 0 and 1 for success and failure
+ */
+static int parse_value_chipuid(build_image_context *context,
+			parse_token token,
+			char *rest)
+{
+	u_int8_t value[16];
+
+	assert(context != NULL);
+	assert(rest != NULL);
+
+	memset(value, 0, sizeof(value));
+	rest = parse_chipuid(rest, value);
+	if (rest == NULL)
+		return 1;
+
+	return context_set_value(context, token, value);
+}
+
+/*
  * Parse the given string and find the bct file name.
  *
  * @param context	The main context pointer
diff --git a/src/parse.h b/src/parse.h
index d9d873f..114168c 100644
--- a/src/parse.h
+++ b/src/parse.h
@@ -108,6 +108,8 @@ typedef enum
 	token_dev_type_spi,
 	token_num_sdram_sets,
 	token_pre_bct_pad_blocks,
+	token_unique_chip_id,
+	token_secure_jtag_control,
 
 	token_nand_clock_divider,
 	token_nand_nand_timing,
diff --git a/src/set.c b/src/set.c
index 3d62254..4a78b71 100644
--- a/src/set.c
+++ b/src/set.c
@@ -199,6 +199,18 @@ int context_set_value(build_image_context *context,
 		context->pre_bct_pad_blocks = *((u_int32_t *)value);
 		break;
 
+	case token_secure_jtag_control:
+		context->secure_jtag_control = *((u_int32_t *)value);
+		g_soc_config->set_value(token_secure_jtag_control,
+			value, context->bct);
+		break;
+
+	case token_unique_chip_id:
+		memcpy(context->unique_chip_id, value, 16);
+		g_soc_config->set_value(token_unique_chip_id,
+			value, context->bct);
+		break;
+
 	DEFAULT();
 	}
 
diff --git a/src/t124/nvbctlib_t124.c b/src/t124/nvbctlib_t124.c
index b1ec445..b103ea7 100644
--- a/src/t124/nvbctlib_t124.c
+++ b/src/t124/nvbctlib_t124.c
@@ -111,7 +111,9 @@ parse_token t124_root_token_list[] = {
 	token_hash_size,
 	token_crypto_offset,
 	token_crypto_length,
-	token_max_bct_search_blks
+	token_max_bct_search_blks,
+	token_unique_chip_id,
+	token_secure_jtag_control
 };
 
 int
@@ -941,6 +943,7 @@ t124_bct_get_value(parse_token id, void *data, u_int8_t *bct)
 	CASE_GET_NVU32(num_sdram_sets);
 	CASE_GET_NVU32(bootloader_used);
 	CASE_GET_NVU32(odm_data);
+	CASE_GET_NVU32(secure_jtag_control);
 
 	/*
 	 * Constants.
@@ -955,6 +958,10 @@ t124_bct_get_value(parse_token id, void *data, u_int8_t *bct)
 		sizeof(nvboot_hash));
 		break;
 
+	case token_unique_chip_id:
+		memcpy(data, &(bct_ptr->unique_chip_id), sizeof(nvboot_ecid));
+		break;
+
 	case token_reserved_offset:
 		*((u_int32_t *)data) = (u_int8_t *)&(samplebct.reserved)
 				- (u_int8_t *)&samplebct;
@@ -1020,6 +1027,10 @@ t124_bct_set_value(parse_token id, void *data, u_int8_t *bct)
 	CASE_SET_NVU32(num_sdram_sets);
 	CASE_SET_NVU32(bootloader_used);
 	CASE_SET_NVU32(odm_data);
+	CASE_SET_NVU32(secure_jtag_control);
+	case token_unique_chip_id:
+		memcpy(&bct_ptr->unique_chip_id, data, sizeof(nvboot_ecid));
+		break;
 
 	default:
 		return -ENODATA;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [cbootimage PATCH V2 5/5] Add update BCT configs feature
       [not found] ` <1396967422-6018-1-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
                     ` (3 preceding siblings ...)
  2014-04-08 14:30   ` [cbootimage PATCH V2 4/5] Add Tegra124 bct data access for jtag control and chip uid Penny Chiu
@ 2014-04-08 14:30   ` Penny Chiu
       [not found]     ` <1396967422-6018-6-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  4 siblings, 1 reply; 10+ messages in thread
From: Penny Chiu @ 2014-04-08 14:30 UTC (permalink / raw)
  To: swarren-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: Penny Chiu

This feature reads the BCT data from BCT or BCT with bootloader
appended binary, updates the BCT data based on config file, then
writes to new image file.

Signed-off-by: Penny Chiu <pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 src/cbootimage.c  | 91 ++++++++++++++++++++++++++++++++++++++++++++++---------
 src/cbootimage.h  | 10 +++++-
 src/data_layout.c | 86 +++++++++++++++++++++++++++++++++++++++++++---------
 src/data_layout.h |  9 ++++++
 src/set.c         | 28 ++++++++++++-----
 src/set.h         |  2 ++
 6 files changed, 188 insertions(+), 38 deletions(-)

diff --git a/src/cbootimage.c b/src/cbootimage.c
index 1332c5f..7477414 100644
--- a/src/cbootimage.c
+++ b/src/cbootimage.c
@@ -49,6 +49,7 @@ struct option cbootcmd[] = {
 	{"tegra", 1, NULL, 't'},
 	{"odmdata", 1, NULL, 'o'},
 	{"soc", 1, NULL, 's'},
+	{"update", 0, NULL, 'u'},
 	{0, 0, 0, 0},
 };
 
@@ -63,7 +64,7 @@ write_image_file(build_image_context *context)
 static void
 usage(void)
 {
-	printf("Usage: cbootimage [options] configfile imagename\n");
+	printf("Usage: cbootimage [options] configfile [inputimage] outputimage\n");
 	printf("    options:\n");
 	printf("    -h, --help, -?        Display this message.\n");
 	printf("    -d, --debug           Output debugging information.\n");
@@ -75,18 +76,22 @@ usage(void)
 	printf("    -s|--soc tegraNN      Select target device. Must be one of:\n");
 	printf("                          tegra20, tegra30, tegra114, tegra124.\n");
 	printf("                          Default: tegra20.\n");
+	printf("    -u|--update           Copy input image data and update bct\n");
+	printf("                          configs into new image file.\n");
 	printf("    configfile            File with configuration information\n");
-	printf("    imagename             Output image name\n");
+	printf("    inputimage            Input image name. This is required\n");
+	printf("                          if -u|--update option is used.\n");
+	printf("    outputimage           Output image name\n");
 }
 
 static int
 process_command_line(int argc, char *argv[], build_image_context *context)
 {
-	int c;
+	int c, num_filenames = 2;
 
 	context->generate_bct = 0;
 
-	while ((c = getopt_long(argc, argv, "hdg:t:o:s:", cbootcmd, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, "hdg:t:o:s:u", cbootcmd, NULL)) != -1) {
 		switch (c) {
 		case 'h':
 			help_only = 1;
@@ -131,10 +136,14 @@ process_command_line(int argc, char *argv[], build_image_context *context)
 		case 'o':
 			context->odm_data = strtoul(optarg, NULL, 16);
 			break;
+		case 'u':
+			context->update_image = 1;
+			num_filenames = 3;
+			break;
 		}
 	}
 
-	if (argc - optind != 2) {
+	if (argc - optind != num_filenames) {
 		printf("Missing configuration and/or image file name.\n");
 		usage();
 		return -EINVAL;
@@ -145,14 +154,18 @@ process_command_line(int argc, char *argv[], build_image_context *context)
 		t20_get_soc_config(context, &g_soc_config);
 
 	/* Open the configuration file. */
-	context->config_file = fopen(argv[optind], "r");
+	context->config_file = fopen(argv[optind++], "r");
 	if (context->config_file == NULL) {
 		printf("Error opening config file.\n");
 		return -ENODATA;
 	}
 
+	/* Record the input image filename if update_image is necessary */
+	if (context->update_image)
+		context->input_image_filename = argv[optind++];
+
 	/* Record the output filename */
-	context->image_filename = argv[optind + 1];
+	context->output_image_filename = argv[optind++];
 
 	return 0;
 }
@@ -190,18 +203,66 @@ main(int argc, char *argv[])
 	}
 
 	/* Open the raw output file. */
-	context.raw_file = fopen(context.image_filename,
-		                 "w+");
+	context.raw_file = fopen(context.output_image_filename, "w+");
 	if (context.raw_file == NULL) {
 		printf("Error opening raw file %s.\n",
-			context.image_filename);
+			context.output_image_filename);
 		goto fail;
 	}
 
-	/* first, if we aren't generating the bct, read in config file */
-	if (context.generate_bct == 0) {
-		process_config_file(&context, 1);
+	/* Read the bct data from image if bct configs needs to be updated */
+	if (context.update_image) {
+		u_int32_t offset = 0, bct_size, actual_size;
+		u_int8_t *data_block;
+		struct stat stats;
+
+		if (stat(context.input_image_filename, &stats) != 0) {
+			printf("Error: Unable to query info on input file path %s\n",
+			context.input_image_filename);
+			goto fail;
+		}
+
+		/* Get BCT_SIZE from input image file  */
+		bct_size = get_bct_size_from_image(&context);
+		if (!bct_size) {
+			printf("Error: Invalid input image file %s\n",
+			context.input_image_filename);
+			goto fail;
+		}
+
+		while (stats.st_size > offset) {
+			/* Read a block of data into memory */
+			if (read_from_image(context.input_image_filename, offset, bct_size,
+					&data_block, &actual_size, file_type_bct)) {
+				printf("Error reading image file %s.\n", context.input_image_filename);
+				goto fail;
+			}
+
+			/* Check if memory data is valid BCT */
+			context.bct = data_block;
+			if (data_is_valid_bct(&context)) {
+				fseek(context.config_file, 0, SEEK_SET);
+				process_config_file(&context, 0);
+				e = sign_bct(&context, context.bct);
+				if (e != 0) {
+					printf("Signing BCT failed, error: %d.\n", e);
+					goto fail;
+				}
+			}
+
+			/* Write the block of data to file */
+			write_data_block(context.raw_file, offset, actual_size, data_block);
+
+			offset += bct_size;
+		}
+
+		printf("Image file %s has been successfully generated!\n",
+			context.output_image_filename);
+		goto fail;
 	}
+	/* If we aren't generating the bct, read in config file */
+	else if (context.generate_bct == 0)
+		process_config_file(&context, 1);
 	/* Generate the new bct file */
 	else {
 		/* Initialize the bct memory */
@@ -218,7 +279,7 @@ main(int argc, char *argv[])
 		fwrite(context.bct, 1, context.bct_size,
 			context.raw_file);
 		printf("New BCT file %s has been successfully generated!\n",
-			context.image_filename);
+			context.output_image_filename);
 		goto fail;
 	}
 
@@ -234,7 +295,7 @@ main(int argc, char *argv[])
 		printf("Error writing image file.\n");
 	else
 		printf("Image file %s has been successfully generated!\n",
-				context.image_filename);
+				context.output_image_filename);
 
  fail:
 	/* Close the file(s). */
diff --git a/src/cbootimage.h b/src/cbootimage.h
index 252c41e..121bc95 100644
--- a/src/cbootimage.h
+++ b/src/cbootimage.h
@@ -44,6 +44,12 @@
 #define BOOTDATA_VERSION_T114		NVBOOT_BOOTDATA_VERSION(0x35, 0x1)
 #define BOOTDATA_VERSION_T124		NVBOOT_BOOTDATA_VERSION(0x40, 0x1)
 
+#define NVBOOT_CONFIG_TABLE_SIZE_MAX 8192
+#define NVBOOT_CONFIG_TABLE_SIZE_T20 4080
+#define NVBOOT_CONFIG_TABLE_SIZE_T30 6128
+#define NVBOOT_CONFIG_TABLE_SIZE_T114 8192
+#define NVBOOT_CONFIG_TABLE_SIZE_T124 8192
+
 /*
  * Enumerations
  */
@@ -60,7 +66,8 @@ typedef enum
 typedef struct build_image_context_rec
 {
 	FILE *config_file;
-	char *image_filename;
+	char *output_image_filename;
+	char *input_image_filename;
 	FILE *raw_file;
 	u_int32_t block_size;
 	u_int32_t block_size_log2;
@@ -98,6 +105,7 @@ typedef struct build_image_context_rec
 	u_int32_t odm_data; /* The odm data value */
 	u_int8_t unique_chip_id[16]; /* The unique chip uid */
 	u_int8_t secure_jtag_control; /* The flag for enabling jtag control */
+	u_int8_t update_image; /* The flag for updating image */
 } build_image_context;
 
 /* Function prototypes */
diff --git a/src/data_layout.c b/src/data_layout.c
index 0a64ec2..a7a89fc 100644
--- a/src/data_layout.c
+++ b/src/data_layout.c
@@ -451,6 +451,8 @@ write_bootloaders(build_image_context *context)
 
 	/* Read the BL into memory. */
 	if (read_from_image(context->newbl_filename,
+		0,
+		MAX_BOOTLOADER_SIZE,
 		&bl_storage,
 		&bl_actual_size,
 		bl_filetype) == 1) {
@@ -665,17 +667,20 @@ int
 read_bct_file(struct build_image_context_rec *context)
 {
 	u_int8_t  *bct_storage; /* Holds the Bl after reading */
-	u_int32_t  bct_actual_size; /* In bytes */
+	u_int32_t bct_actual_size; /* In bytes */
 	file_type bct_filetype = file_type_bct;
 	int err = 0;
 
 	if (read_from_image(context->bct_filename,
-		&bct_storage,
-		&bct_actual_size,
-		bct_filetype) == 1) {
+			0,
+			NVBOOT_CONFIG_TABLE_SIZE_MAX,
+			&bct_storage,
+			&bct_actual_size,
+			bct_filetype) == 1) {
 		printf("Error reading bct file %s.\n", context->bct_filename);
 		exit(1);
 	}
+
 	context->bct_size = bct_actual_size;
 	if (context->bct_init != 1)
 		err = init_bct(context);
@@ -686,18 +691,12 @@ read_bct_file(struct build_image_context_rec *context)
 	memcpy(context->bct, bct_storage, context->bct_size);
 	free(bct_storage);
 
-	/* get proper soc_config pointer by polling each supported chip */
-	if (if_bct_is_t20_get_soc_config(context, &g_soc_config))
-		return 0;
-	if (if_bct_is_t30_get_soc_config(context, &g_soc_config))
-		return 0;
-	if (if_bct_is_t114_get_soc_config(context, &g_soc_config))
-		return 0;
-	if (if_bct_is_t124_get_soc_config(context, &g_soc_config))
-		return 0;
+	if (!data_is_valid_bct(context))
+		return ENODATA;
 
-	return ENODATA;
+	return err;
 }
+
 /*
  * Update the next_bct_blk and make it point to the next
  * new blank block according to bct_copy given.
@@ -898,3 +897,62 @@ write_block_raw(build_image_context *context)
 	free(empty_blk);
 	return 0;
 }
+
+int write_data_block(FILE *fp, u_int32_t offset, u_int32_t size, u_int8_t *buffer)
+{
+	if (fseek(fp, offset, 0))
+		return -1;
+
+	fwrite(buffer, 1, size, fp);
+	return 0;
+}
+
+int data_is_valid_bct(build_image_context *context)
+{
+	/* get proper soc_config pointer by polling each supported chip */
+	if (if_bct_is_t20_get_soc_config(context, &g_soc_config))
+		return 1;
+	if (if_bct_is_t30_get_soc_config(context, &g_soc_config))
+		return 1;
+	if (if_bct_is_t114_get_soc_config(context, &g_soc_config))
+		return 1;
+	if (if_bct_is_t124_get_soc_config(context, &g_soc_config))
+		return 1;
+
+	return 0;
+}
+
+int get_bct_size_from_image(build_image_context *context)
+{
+	u_int8_t buffer[NVBOOT_CONFIG_TABLE_SIZE_MAX];
+	u_int32_t bct_size = 0;
+	FILE *fp;
+
+	fp = fopen(context->input_image_filename, "r");
+	if (!fp)
+		return ENODATA;
+
+	if (fread(buffer, 1, NVBOOT_CONFIG_TABLE_SIZE_MAX, fp)) {
+		context->bct = buffer;
+		if (data_is_valid_bct(context)) {
+			switch (context->boot_data_version) {
+			case BOOTDATA_VERSION_T20:
+				bct_size = NVBOOT_CONFIG_TABLE_SIZE_T20;
+				break;
+			case BOOTDATA_VERSION_T30:
+				bct_size = NVBOOT_CONFIG_TABLE_SIZE_T30;
+				break;
+			case BOOTDATA_VERSION_T114:
+				bct_size = NVBOOT_CONFIG_TABLE_SIZE_T114;
+				break;
+			case BOOTDATA_VERSION_T124:
+				bct_size = NVBOOT_CONFIG_TABLE_SIZE_T124;
+				break;
+			}
+		}
+	}
+	fclose(fp);
+
+	context->bct = 0;
+	return bct_size;
+}
diff --git a/src/data_layout.h b/src/data_layout.h
index 9ee8267..12da165 100644
--- a/src/data_layout.h
+++ b/src/data_layout.h
@@ -50,6 +50,15 @@ int
 write_block_raw(struct build_image_context_rec *context);
 
 int
+write_data_block(FILE *fp, u_int32_t offset, u_int32_t size, u_int8_t *buffer);
+
+int
+data_is_valid_bct(build_image_context *context);
+
+int
+get_bct_size_from_image(build_image_context *context);
+
+int
 begin_update(build_image_context *context);
 
 #endif /* #ifndef INCLUDED_DATA_LAYOUT_H */
diff --git a/src/set.c b/src/set.c
index 4a78b71..46dcfdc 100644
--- a/src/set.c
+++ b/src/set.c
@@ -43,6 +43,8 @@
 
 int
 read_from_image(char	*filename,
+		u_int32_t	offset,
+		u_int32_t	max_size,
 		u_int8_t	**image,
 		u_int32_t	*actual_size,
 		file_type	f_type)
@@ -57,6 +59,8 @@ read_from_image(char	*filename,
 		return result;
 	}
 
+	fseek(fp, offset, SEEK_SET);
+
 	if (stat(filename, &stats) != 0) {
 		printf("Error: Unable to query info on bootloader path %s\n",
 			filename);
@@ -64,14 +68,21 @@ read_from_image(char	*filename,
 		goto cleanup;
 	}
 
-	*actual_size  = (u_int32_t)stats.st_size;
-
-	if (f_type == file_type_bl && *actual_size > MAX_BOOTLOADER_SIZE) {
-		printf("Error: Bootloader file %s is too large.\n",
-			filename);
-		result = 1;
-		goto cleanup;
+	if (f_type == file_type_bl) {
+		if ((stats.st_size - offset) > max_size) {
+			printf("Error: Bootloader file %s is too large.\n",
+				filename);
+			result = 1;
+			goto cleanup;
+		}
+		*actual_size = (u_int32_t)stats.st_size;
+	} else {
+		if ((stats.st_size - offset) < max_size)
+			*actual_size = stats.st_size - offset;
+		else
+			*actual_size = max_size;
 	}
+
 	*image = malloc(*actual_size);
 	if (*image == NULL) {
 		result = 1;
@@ -80,7 +91,8 @@ read_from_image(char	*filename,
 
 	memset(*image, 0, *actual_size);
 
-	if (fread(*image, 1, (size_t)stats.st_size, fp) != stats.st_size) {
+	if (fread(*image, 1, (size_t)(*actual_size), fp) !=
+		(size_t)(*actual_size)) {
 		result = 1;
 		goto cleanup;
 	}
diff --git a/src/set.h b/src/set.h
index 754ed7a..97ecc76 100644
--- a/src/set.h
+++ b/src/set.h
@@ -42,6 +42,8 @@ context_set_value(build_image_context	*context,
 
 int
 read_from_image(char *filename,
+		u_int32_t	offset,
+		u_int32_t	max_size,
 		u_int8_t	**Image,
 		u_int32_t	*actual_size,
 		file_type	f_type);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [cbootimage PATCH V2 2/5] Accept void pointer as input data type for get/set_value functions
       [not found]     ` <1396967422-6018-3-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2014-04-08 19:48       ` Stephen Warren
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Warren @ 2014-04-08 19:48 UTC (permalink / raw)
  To: Penny Chiu, swarren-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 04/08/2014 08:30 AM, Penny Chiu wrote:
> This change uses void * as input data type for
> cbootimage_soc_config.get/set_value and context_set_value functions.
> This makes the functions can accept various data types based on
> different tokens.

> diff --git a/src/set.c b/src/set.c

>  int context_set_value(build_image_context *context,

>  	case token_block_size:
> -		context->block_size = value;
> -		context->block_size_log2 = log2(value);
> +		context->block_size = *((u_int32_t *)value);
> +		context->block_size_log2 = log2(*((u_int32_t *)value));
>  
>  		if (context->memory != NULL) {
>  			printf("Error: Too late to change block size.\n");
>  			return 1;
>  		}
>  
> -		if (value != (u_int32_t)(1 << context->block_size_log2)) {
> +		if (*((u_int32_t *)value) != (u_int32_t)(1 << context->block_size_log2)) {

That could be context->block_size rather than *((u_int32_t *)value).

>  	case token_page_size:
> -		context->page_size = value;
> -		context->page_size_log2 = log2(value);
> +		context->page_size = *((u_int32_t *)value);
> +		context->page_size_log2 = log2(*((u_int32_t *)value));
>  		context->pages_per_blk= 1 << (context->block_size_log2- 
>  			context->page_size_log2);

I wonder why this case doesn't validate that page_size is a power of two
like the block_size does? Still, that's a pre-existing issue, so doesn't
block this patch. If you could send a patch to fix that, it would be
awesome.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [cbootimage PATCH V2 3/5] Add token_supported function
       [not found]     ` <1396967422-6018-4-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2014-04-08 19:55       ` Stephen Warren
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Warren @ 2014-04-08 19:55 UTC (permalink / raw)
  To: Penny Chiu, swarren-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 04/08/2014 08:30 AM, Penny Chiu wrote:
> Add a function called token_supported in cbootimage_soc_config.
> It is used to check if the input token is supported in specific
> tegra soc.

> diff --git a/src/t114/nvbctlib_t114.c b/src/t114/nvbctlib_t114.c

> +int t114_bct_token_supported(parse_token token)
> +{
> +	int index;
> +
> +	for (index = 0; index < sizeof(t114_root_token_list)/sizeof(t114_root_token_list[0]); index++)

It'd be nice if we had an ARRAY_SIZE() macro we could use here...

That would also make the line-length a bit more typical.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [cbootimage PATCH V2 4/5] Add Tegra124 bct data access for jtag control and chip uid
       [not found]     ` <1396967422-6018-5-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2014-04-08 20:09       ` Stephen Warren
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Warren @ 2014-04-08 20:09 UTC (permalink / raw)
  To: Penny Chiu, swarren-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 04/08/2014 08:30 AM, Penny Chiu wrote:
> Add support for read secure_jtag_control and unique_chip_id from
> cfg file and write them into BCT structure, and bct_dump can also
> parse the two fields and show the data.

> diff --git a/src/bct_dump.c b/src/bct_dump.c

> @@ -174,17 +190,30 @@ int main(int argc, char *argv[])
>  		if (!g_soc_config->token_supported(values[i].id))
>  			continue;
>  
> +		if (values[i].id == token_unique_chip_id) {
> +			u_int8_t uid[16];
> +
> +			e = g_soc_config->get_value(values[i].id,
> +						uid,
> +						context.bct);
> +			if (e != 0)
> +				for (j = 0; j < 16; j++)
> +					uid[j] = -1;
> +
> +			values[i].format(values[i].message, uid);
> +		} else {
> +			e = g_soc_config->get_value(values[i].id,
>  						&data,
>  						context.bct);
>  
> +			if (e != 0)
> +				data = -1;
> +			else if (values[i].id == token_block_size_log2 ||
> +				 values[i].id == token_page_size_log2)
> +				data = 1 << data;
>  
> +			values[i].format(values[i].message, &data);
> +		}

I was hoping to avoid that outer if condition completely. Wouldn't the
following work:

u_int8_t data[MAX_PARAM_SIZE];
e = g_soc_config->get_value(values[i].id, data, context.bct);
if (e)
    memset data[] to zero
values[i].format(values[i].message, data);

0 seems just as valid in the error case as -1.

I would prefer the special case for token_block_size_log2 adn
token_page_size_log2 to go away too. Why aren't those tokens set to
their actual values?

In order to calculate MAX_PARAM_SIZE, perhaps something like:

union param_types;
    u32 val;
    u_int8_t uuid[16];
};
#define MAX_PARAM_SIZE sizeof(param_types)

> diff --git a/src/parse.c b/src/parse.c

> +parse_chipuid(char *str, u_int8_t *chipuid)

> +	paddings = strlen(str) % 2;
> +	byte_index = strlen(str) / 2 + paddings;

There needs to be error-checking to make sure that byte_index < sizeof
*chipuid.

If byte_index is less than sizeof *chipuid, should the value be left- or
right-aligned within *chipuid? Right now it's left-justified. That may
be fine, but I just wanted to check. Is it an error if the length of the
string doesn't exactly match sizeof *chipuid?

> +	while (*str != '\0' && byte_index > 0) {
> +		char *endptr;
> +
> +		strncpy(byte_str, str, 2 - paddings);
> +		byte_str[2-paddings] = '\0';

Need spaces around the - operator.

> +static int parse_value_chipuid(build_image_context *context,
> +			parse_token token,
> +			char *rest)
> +{
> +	u_int8_t value[16];
> +
> +	assert(context != NULL);
> +	assert(rest != NULL);
> +
> +	memset(value, 0, sizeof(value));

Shouldn't this memset() be inside parse_chipuid(), so that
parse_chipuid() either fails, or succeeds and fills in the entire UID value?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [cbootimage PATCH V2 5/5] Add update BCT configs feature
       [not found]     ` <1396967422-6018-6-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2014-04-08 21:27       ` Stephen Warren
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Warren @ 2014-04-08 21:27 UTC (permalink / raw)
  To: Penny Chiu, swarren-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 04/08/2014 08:30 AM, Penny Chiu wrote:
> This feature reads the BCT data from BCT or BCT with bootloader
> appended binary, updates the BCT data based on config file, then
> writes to new image file.

> diff --git a/src/cbootimage.h b/src/cbootimage.h

> +#define NVBOOT_CONFIG_TABLE_SIZE_MAX 8192
> +#define NVBOOT_CONFIG_TABLE_SIZE_T20 4080
> +#define NVBOOT_CONFIG_TABLE_SIZE_T30 6128
> +#define NVBOOT_CONFIG_TABLE_SIZE_T114 8192
> +#define NVBOOT_CONFIG_TABLE_SIZE_T124 8192

It might at least be nice in this series to add a compile-time-assert to
nvboot_bct_tNN.h to validate that those sizes are OK

> diff --git a/src/cbootimage.c b/src/cbootimage.c

> @@ -190,18 +203,66 @@ main(int argc, char *argv[])

> +		while (stats.st_size > offset) {
> +			/* Read a block of data into memory */
> +			if (read_from_image(context.input_image_filename, offset, bct_size,
> +					&data_block, &actual_size, file_type_bct)) {
...
> +			offset += bct_size;
> +		}

I don't think that handles multiple bootloaders in the image correctly.
bct_size isn't always aligned to the block size of the memory device,
whereas copies of the BCT are. This probably won't be an issue for
Tegra114/124, but for Tegra20/30, the code needs to read the memory
block by block, rather than in BCT-sized chunks.

I'd be fine for now with doing one of the following if determining the
real memory block size is too complicated:

a) Disallow -u on Tegra20/30 (or even anything other than Tegra124).

b) Always read 8KB chunks, since that is the nearest power of 2 above
the BCT size on all platforms.

Related, what happens in the memory block size is less than the size of
a BCT; the code really should read a BCT's worth of data (e.g. 8KB)
starting at each block-sized offset into the file (e.g. 2KB). That's why
I originally suggested reading the entire image into memory (or mmap'ing
it) and updating it in one pass, since that decouples the file IO from
locating the BCTs within the image. Still, we can fix this up later.

> diff --git a/src/data_layout.c b/src/data_layout.c

>  read_bct_file(struct build_image_context_rec *context)
>  {
>  	u_int8_t  *bct_storage; /* Holds the Bl after reading */
> -	u_int32_t  bct_actual_size; /* In bytes */
> +	u_int32_t bct_actual_size; /* In bytes */

That seems like an unrelated change.

> +int write_data_block(FILE *fp, u_int32_t offset, u_int32_t size, u_int8_t *buffer)
> +{
> +	if (fseek(fp, offset, 0))
> +		return -1;
> +
> +	fwrite(buffer, 1, size, fp);
> +	return 0;
> +}

It'd be nice to validate that fwrite() was successful. return fwrite(...);?

> +int get_bct_size_from_image(build_image_context *context)
> +{
> +	u_int8_t buffer[NVBOOT_CONFIG_TABLE_SIZE_MAX];
> +	u_int32_t bct_size = 0;
> +	FILE *fp;
> +
> +	fp = fopen(context->input_image_filename, "r");
> +	if (!fp)
> +		return ENODATA;
> +
> +	if (fread(buffer, 1, NVBOOT_CONFIG_TABLE_SIZE_MAX, fp)) {

Error-handling for read() would be nice here too.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-04-08 21:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-08 14:30 [cbootimage PATCH V2 0/5] Re-enable jtag function for Tegra124 Penny Chiu
     [not found] ` <1396967422-6018-1-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2014-04-08 14:30   ` [cbootimage PATCH V2 1/5] Add format functions to express BCT and bootloader data value Penny Chiu
2014-04-08 14:30   ` [cbootimage PATCH V2 2/5] Accept void pointer as input data type for get/set_value functions Penny Chiu
     [not found]     ` <1396967422-6018-3-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2014-04-08 19:48       ` Stephen Warren
2014-04-08 14:30   ` [cbootimage PATCH V2 3/5] Add token_supported function Penny Chiu
     [not found]     ` <1396967422-6018-4-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2014-04-08 19:55       ` Stephen Warren
2014-04-08 14:30   ` [cbootimage PATCH V2 4/5] Add Tegra124 bct data access for jtag control and chip uid Penny Chiu
     [not found]     ` <1396967422-6018-5-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2014-04-08 20:09       ` Stephen Warren
2014-04-08 14:30   ` [cbootimage PATCH V2 5/5] Add update BCT configs feature Penny Chiu
     [not found]     ` <1396967422-6018-6-git-send-email-pchiu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2014-04-08 21:27       ` Stephen Warren

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.