Change in coreboot[master]: util/cbfstool: Add Alignment for CBnT

[Christian Walter (Code Review)]

Christian Walter has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45652 )

Change subject: util/cbfstool: Add Alignment for CBnT
......................................................................

util/cbfstool: Add Alignment for CBnT

Change-Id: Ibd94539a6025e7c3912730a6d90454379d85fb59
Signed-off-by: Christian Walter 
---
M util/cbfstool/cbfstool.c
M util/cbfstool/fit.c
2 files changed, 27 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/45652/1

diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index d2df1cc..ce72835 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -70,6 +70,7 @@
    bool machine_parseable;
    bool unprocessed;
    bool ibb;
+	bool cbnt;
    enum comp_algo compression;
    int precompression;
    enum vb2_hash_algorithm hash;
@@ -551,6 +552,18 @@
    	len_align = 16;
    }

+	if (param.cbnt) {
+		ERROR("MARKING ENTRY AS CBNT\n");
+		/* Mark as Initial Boot Block */
+		struct cbfs_file_attribute *attrs = cbfs_add_file_attr(header,
+				CBFS_FILE_ATTR_TAG_IBB,
+				sizeof(struct cbfs_file_attribute));
+		if (attrs == NULL)
+			return -1;
+		/* For Intel CBNT minimum align is 64 */
+		len_align = 64;
+	}
+
    if (param.padding) {
    	const uint32_t hs = sizeof(struct cbfs_file_attribute);
    	uint32_t size = MAX(hs, param.padding);
@@ -1290,6 +1303,7 @@
    /* begin after ASCII characters */
    LONGOPT_START = 256,
    LONGOPT_IBB = LONGOPT_START,
+	LONGOPT_CBNT = 257,
    LONGOPT_END,
 };

@@ -1333,6 +1347,7 @@
    {"mach-parseable",no_argument,       0, 'k' },
    {"unprocessed",   no_argument,       0, 'U' },
    {"ibb",           no_argument,       0, LONGOPT_IBB },
+	{"cbnt",          no_argument,       0, LONGOPT_CBNT },
    {NULL,            0,                 0,  0  }
 };

@@ -1406,7 +1421,7 @@
         " add [-r image,regions] -f FILE -n NAME -t TYPE [-A hash] \\n"
         "        [-c compression] [-b base-address | -a alignment] \\n"
         "        [-p padding size] [-y|--xip if TYPE is FSP]       \\n"
-	     "        [-j topswap-size] (Intel CPUs only) [--ibb]           "
+	     "        [-j topswap-size] (Intel CPUs only) [--ibb] [--cbnt]  "
    		"Add a component\n"
         "                                                         "
         "    -j valid size: 0x10000 0x20000 0x40000 0x80000 0x100000 \n"
@@ -1417,7 +1432,7 @@
         " add-stage [-r image,regions] -f FILE -n NAME [-A hash] \\n"
         "        [-c compression] [-b base] [-S section-to-ignore] \\n"
         "        [-a alignment] [-P page-size] [-Q|--pow2page] \\n"
-	     "        [-y|--xip] [--ibb]                                   "
+	     "        [-y|--xip] [--ibb] [--cbnt]                           "
    		"Add a stage to the ROM\n"
         " add-flat-binary [-r image,regions] -f FILE -n NAME \\n"
         "        [-A hash] -l load-address -e entry-point \\n"
@@ -1750,6 +1765,9 @@
    		case LONGOPT_IBB:
    			param.ibb = true;
    			break;
+			case LONGOPT_CBNT:
+				param.cbnt = true;
+				break;
    		case 'h':
    		case '?':
    			usage(argv[0]);
diff --git a/util/cbfstool/fit.c b/util/cbfstool/fit.c
index e7aa8d7..b04cf90 100644
--- a/util/cbfstool/fit.c
+++ b/util/cbfstool/fit.c
@@ -463,9 +463,10 @@
    				uint32_t boot_policy_size)
 {
    entry->address = boot_policy_addr;
-	/* Boot Policy Manifest size is not multiple of 16 bytes */
-	fit_entry_update_size(entry, boot_policy_size << 4);
    entry->type_checksum_valid = FIT_TYPE_BOOT_POLICY;
+	/* Boot Policy Manifest size is not multiple of 16 bytes */
+	fit_entry_update_size(entry, boot_policy_size);
+	entry->size_reserved = boot_policy_size;
    entry->version = FIT_TXT_VERSION;
    entry->checksum = 0;
    fit_entry_add_size(&fit->header, sizeof(struct fit_entry));
@@ -482,9 +483,11 @@
    				uint32_t key_manifest_size)
 {
    entry->address = key_manifest_addr;
-	/* Key Manifest size is not multiple of 16 bytes */
-	fit_entry_update_size(entry, key_manifest_size << 4);
+
    entry->type_checksum_valid = FIT_TYPE_KEY_MANIFEST;
+	/* Key Manifest size is not multiple of 16 bytes */
+	fit_entry_update_size(entry, key_manifest_size);
+	entry->size_reserved = key_manifest_size;
    entry->version = FIT_TXT_VERSION;
    entry->checksum = 0;
    fit_entry_add_size(&fit->header, sizeof(struct fit_entry));

[Stefan Reinauer (Code Review)]

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45652 )

Change subject: util/cbfstool: Add Alignment for CBnT
......................................................................

Patch Set 15:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45652/15/util/cbfstool/cbfstool.c 
File util/cbfstool/cbfstool.c:

https://review.coreboot.org/c/coreboot/+/45652/15/util/cbfstool/cbfstool.c@5... 
PS15, Line 556: ERROR("MARKING ENTRY AS CBNT\n");
Why ERROR?

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45652 )

Change subject: util/cbfstool: Add Alignment for CBnT
......................................................................

Patch Set 15:

I don't think this change is really necessary. All this does is mark the bootblock with the IBB CBFS attribute which is unused with CbNT (IBB marking is done via BPM, not via FIT) and align it to 64 bytes which is a CbNT requirement. The alignement is not an issue since the bootblock is a fixed size. If that were to change it would be easy to add this alignment requirement to the linker script.