[S] Change in coreboot[main]: amdfwtool: Set the table size for L1 separately

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78274?usp=email )

Change subject: amdfwtool: Set the table size for L1 separately
......................................................................

amdfwtool: Set the table size for L1 separately

The space defined by size of the L1 table can not overlap with ISH
header. For other cases, the size defines the directory and its
content.

The PSP spec does not say it quite clearly. This change is partly
based on guess and can make extraction tool work so far.

Change-Id: Id4fbc6d57d7ea070a9478649a96af92be9441289
Signed-off-by: Zheng Bao 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78274
Tested-by: build bot (Jenkins) 
Reviewed-by: Felix Held 
---
M util/amdfwtool/amdfwtool.c
1 file changed, 23 insertions(+), 13 deletions(-)

Approvals:
  Felix Held: Looks good to me, approved
  build bot (Jenkins): Verified

diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c
index e37c635..61ccefb 100644
--- a/util/amdfwtool/amdfwtool.c
+++ b/util/amdfwtool/amdfwtool.c
@@ -566,16 +566,17 @@
    set_current_pointer(ctx, ALIGN_UP(ctx->current + add, align));
 }

-static void *new_psp_dir(context *ctx, int multi)
+static void *new_psp_dir(context *ctx, amd_cb_config *cb_config)
 {
    void *ptr;
+	uint32_t align_end = cb_config->need_ish ? TABLE_ALIGNMENT : 1;

/*
     * Force both onto boundary when multi.  Primary table is after
     * updatable table, so alignment ensures primary can stay intact
     * if secondary is reprogrammed.
     */
-	if (multi)
+	if (cb_config->multi_level)
    	adjust_current_pointer(ctx, 0, TABLE_ERASE_ALIGNMENT);
    else
    	adjust_current_pointer(ctx, 0, TABLE_ALIGNMENT);
@@ -586,7 +587,7 @@
    ((psp_directory_header *)ptr)->additional_info_fields.address_mode = ctx->address_mode;
    adjust_current_pointer(ctx,
    	sizeof(psp_directory_header) + MAX_PSP_ENTRIES * sizeof(psp_directory_entry),
-		1);
+		align_end);
    return ptr;
 }

@@ -612,7 +613,8 @@
    return ptr;
 }

-static void fill_dir_header(void *directory, uint32_t count, uint32_t cookie, context *ctx)
+static void fill_dir_header(void *directory, uint32_t count, uint32_t cookie,
+	context *ctx, amd_cb_config *cb_config)
 {
    psp_combo_directory *cdir = directory;
    psp_directory_table *dir = directory;
@@ -647,7 +649,14 @@
    	break;
    case PSP_COOKIE:
    case PSPL2_COOKIE:
-		table_size = ctx->current - ctx->current_table;
+		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);
@@ -1116,7 +1125,7 @@
    	count++;
    }

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

@@ -1479,7 +1488,7 @@
    	count++;
    }

-	fill_dir_header(biosdir, count, cookie, ctx);
+	fill_dir_header(biosdir, count, cookie, ctx, cb_config);
 }

enum {
@@ -2347,13 +2356,13 @@

if (cb_config.multi_level) {
    		/* Do 2nd PSP directory followed by 1st */
-			pspdir2 = new_psp_dir(&ctx, cb_config.multi_level);
+			pspdir2 = new_psp_dir(&ctx, &cb_config);
    		integrate_psp_firmwares(&ctx, pspdir2, NULL, NULL,
    					amd_psp_fw_table, PSPL2_COOKIE, &cb_config);
    		if (cb_config.recovery_ab && !cb_config.recovery_ab_single_copy) {
    			/* Create a copy of PSP Directory 2 in the backup slot B.
    			   Related biosdir2_b copy will be created later. */
-				pspdir2_b = new_psp_dir(&ctx, cb_config.multi_level);
+				pspdir2_b = new_psp_dir(&ctx, &cb_config);
    			integrate_psp_firmwares(&ctx, pspdir2_b, NULL, NULL,
    					amd_psp_fw_table, PSPL2_COOKIE, &cb_config);
    		} else {
@@ -2367,12 +2376,12 @@
    			 */
    			pspdir2_b = NULL; /* More explicitly */
    		}
-			pspdir = new_psp_dir(&ctx, cb_config.multi_level);
+			pspdir = new_psp_dir(&ctx, &cb_config);
    		integrate_psp_firmwares(&ctx, pspdir, pspdir2, pspdir2_b,
    				amd_psp_fw_table, PSP_COOKIE, &cb_config);
    	} else {
    		/* flat: PSP 1 cookie and no pointer to 2nd table */
-			pspdir = new_psp_dir(&ctx, cb_config.multi_level);
+			pspdir = new_psp_dir(&ctx, &cb_config);
    		integrate_psp_firmwares(&ctx, pspdir, NULL, NULL,
    				amd_psp_fw_table, PSP_COOKIE, &cb_config);
    	}
@@ -2388,7 +2397,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);
+			fill_dir_header(psp_combo_dir, combo_index + 1,
+				PSP2_COOKIE, &ctx, &cb_config);
    	}

if (have_bios_tables(amd_bios_table)) {
@@ -2441,7 +2451,7 @@
    				BUFF_TO_RUN_MODE(ctx, biosdir, AMD_ADDR_REL_BIOS);

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