[S] Change in coreboot[main]: amdfwtool: Set the table size only for FWs

[Bao Zheng (Code Review)]

Attention is currently required from: Zheng Bao.

Hello Zheng Bao,

I'd like you to do a code review.
Please visit

https://review.coreboot.org/c/coreboot/+/81255?usp=email

to review the following change.

Change subject: amdfwtool: Set the table size only for FWs
......................................................................

amdfwtool: Set the table size only for FWs

The entry in the table has two categaries, file and pointer. For the
pointer, it does not take space. The ISH, PSP level 2, BIOS table are
all the pointer type. So only integration function packs FWs which
increase the table size.

Change-Id: I5dfbbb55912c8e37243c351427a8df89c12e5da8
Signed-off-by: Zheng Bao 
---
M util/amdfwtool/amdfwtool.c
1 file changed, 15 insertions(+), 24 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/81255/1

diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c
index e81059d..d025ba4 100644
--- a/util/amdfwtool/amdfwtool.c
+++ b/util/amdfwtool/amdfwtool.c
@@ -521,23 +521,17 @@
 }

static void fill_dir_header(void *directory, uint32_t count, uint32_t cookie,
-	context *ctx, amd_cb_config *cb_config)
+	uint32_t table_size, context *ctx)
 {
    psp_combo_directory *cdir = directory;
    psp_directory_table *dir = directory;
    bios_directory_table *bdir = directory;
-	uint32_t table_size = 0;

-	if (!count)
-		return;
    if (ctx == NULL || directory == NULL) {
    	fprintf(stderr, "Calling %s with NULL pointers\n", __func__);
    	return;
    }

-	/* The table size needs to be 0x1000 aligned. So align the end of table. */
-	adjust_current_pointer(ctx, 0, TABLE_ALIGNMENT);
-
    switch (cookie) {
    case PSP2_COOKIE:
    case BHD2_COOKIE:
@@ -556,14 +550,6 @@
    	break;
    case PSP_COOKIE:
    case PSPL2_COOKIE:
-		if (cookie == PSP_COOKIE && cb_config->need_ish)
-			/* The ISH header can not be in the space defined by L1 table size.
-			 * The space is allocated when the L1 header is created. */
-			table_size = TABLE_ALIGNMENT;
-		else
-			/* Generally table size not just constains the header,
-			 * but all the FWs. */
-			table_size = ctx->current - ctx->current_table;
    	if ((table_size % TABLE_ALIGNMENT) != 0) {
    		fprintf(stderr, "The PSP table size should be 4K aligned\n");
    		amdfwtool_cleanup(ctx);
@@ -571,7 +557,7 @@
    	}
    	dir->header.cookie = cookie;
    	dir->header.num_entries = count;
-		dir->header.additional_info_fields.dir_size = table_size / TABLE_ALIGNMENT;
+		dir->header.additional_info_fields.dir_size += table_size / TABLE_ALIGNMENT;
    	dir->header.additional_info_fields.spi_block_size = 1;
    	dir->header.additional_info_fields.base_addr = 0;
    	/* checksum everything that comes after the Checksum field */
@@ -582,7 +568,6 @@
    	break;
    case BHD_COOKIE:
    case BHDL2_COOKIE:
-		table_size = ctx->current - ctx->current_table;
    	if ((table_size % TABLE_ALIGNMENT) != 0) {
    		fprintf(stderr, "The BIOS table size should be 4K aligned\n");
    		amdfwtool_cleanup(ctx);
@@ -590,7 +575,7 @@
    	}
    	bdir->header.cookie = cookie;
    	bdir->header.num_entries = count;
-		bdir->header.additional_info_fields.dir_size = table_size / TABLE_ALIGNMENT;
+		bdir->header.additional_info_fields.dir_size += table_size / TABLE_ALIGNMENT;
    	bdir->header.additional_info_fields.spi_block_size = 1;
    	bdir->header.additional_info_fields.base_addr = 0;
    	/* checksum everything that comes after the Checksum field */
@@ -996,6 +981,9 @@
    		/* This APU doesn't have this firmware. */
    	}
    }
+	/* The table size needs to be 0x1000 aligned. So align the end of table. */
+	adjust_current_pointer(ctx, 0, TABLE_ALIGNMENT);
+	fill_dir_header(pspdir, count, cookie, ctx->current - ctx->current_table, ctx);

if (recovery_ab && (pspdir2 != NULL)) {
    	if (cb_config->need_ish) {	/* Need ISH */
@@ -1032,7 +1020,7 @@
    	count++;
    }

-	fill_dir_header(pspdir, count, cookie, ctx, cb_config);
+	fill_dir_header(pspdir, count, cookie, 0, ctx);
    ctx->current_table = current_table_save;
 }

@@ -1397,7 +1385,10 @@
    	count++;
    }

-	fill_dir_header(biosdir, count, cookie, ctx, cb_config);
+	/* The table size needs to be 0x1000 aligned. So align the end of table. */
+	adjust_current_pointer(ctx, 0, TABLE_ALIGNMENT);
+
+	fill_dir_header(biosdir, count, cookie, ctx->current - ctx->current_table, ctx);
    ctx->current_table = current_table_save;
 }

@@ -1685,8 +1676,8 @@
    		psp_combo_dir->entries[combo_index].lvl2_addr =
    			BUFF_TO_RUN_MODE(ctx, pspdir, AMD_ADDR_REL_BIOS);

-			fill_dir_header(psp_combo_dir, combo_index + 1,
-				PSP2_COOKIE, &ctx, &cb_config);
+			fill_dir_header(psp_combo_dir, combo_index + 1, 0,
+				PSP2_COOKIE, &ctx);
    	}

if (have_bios_tables(amd_bios_table)) {
@@ -1738,8 +1729,8 @@
    			bhd_combo_dir->entries[combo_index].lvl2_addr =
    				BUFF_TO_RUN_MODE(ctx, biosdir, AMD_ADDR_REL_BIOS);

-				fill_dir_header(bhd_combo_dir, combo_index + 1,
-					BHD2_COOKIE, &ctx, &cb_config);
+				fill_dir_header(bhd_combo_dir, combo_index + 1, 0,
+					BHD2_COOKIE, &ctx);
    		}
    	}
    } while (cb_config.use_combo && ++combo_index < MAX_COMBO_ENTRIES &&