[coreboot-gerrit] [M] Change in coreboot[master]: amdfwtool: Allow the amdfw.rom to be split into two parts

[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/+/69857

to review the following change.

Change subject: amdfwtool: Allow the amdfw.rom to be split into two parts
......................................................................

amdfwtool: Allow the amdfw.rom to be split into two parts

Split the big PSP FW data into two parts, head and body. The head
needs to be located at specific location. The body address is more
flexible. So the EC FW can be put to the address like 0x80000.

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

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/69857/1

diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c
index 1d98e68..272e55c 100644
--- a/util/amdfwtool/amdfwtool.c
+++ b/util/amdfwtool/amdfwtool.c
@@ -193,11 +193,13 @@
    printf("--bios-bin-dest <HEX_VAL>      Destination for uncompressed BIOS\n");
    printf("--bios-uncomp-size <HEX>       Uncompressed size of BIOS image\n");
    printf("--output <filename>            output filename\n");
+	printf("--body-output <filename>       output of FW body filename\n");
    printf("--flashsize <HEX_VAL>          ROM size in bytes\n");
    printf("                               size must be larger than %dKB\n",
    	MIN_ROM_KB);
    printf("                               and must a multiple of 1024\n");
    printf("--location                     Location of Directory\n");
+	printf("--body-location                Location of body of firmwares\n");
    printf("--anywhere                     Use any 64-byte aligned addr for Directory\n");
    printf("--sharedmem                    Location of PSP/FW shared memory\n");
    printf("--sharedmem-size               Maximum size of the PSP/FW shared memory\n");
@@ -1724,8 +1726,10 @@
    AMDFW_OPT_APOB_NVSIZE,

AMDFW_OPT_OUTPUT,
+	AMDFW_OPT_BODY_OUTPUT,
    AMDFW_OPT_FLASHSIZE,
    AMDFW_OPT_LOCATION,
+	AMDFW_OPT_FW_BODY_LOCATION,
    AMDFW_OPT_ANYWHERE,
    AMDFW_OPT_SHAREDMEM,
    AMDFW_OPT_SHAREDMEM_SIZE,
@@ -1785,8 +1789,10 @@
    {"spi-micron-flag",  required_argument, 0, LONGOPT_SPI_MICRON_FLAG },
    /* other */
    {"output",           required_argument, 0, AMDFW_OPT_OUTPUT },
+	{"body-output",      required_argument, 0, AMDFW_OPT_BODY_OUTPUT },
    {"flashsize",        required_argument, 0, AMDFW_OPT_FLASHSIZE },
    {"location",         required_argument, 0, AMDFW_OPT_LOCATION },
+	{"body-location",    required_argument, 0, AMDFW_OPT_FW_BODY_LOCATION },
    {"anywhere",         no_argument,       0, AMDFW_OPT_ANYWHERE },
    {"sharedmem",        required_argument, 0, AMDFW_OPT_SHAREDMEM },
    {"sharedmem-size",   required_argument, 0, AMDFW_OPT_SHAREDMEM_SIZE },
@@ -2041,7 +2047,7 @@
    bool comboable = false;
    int fuse_defined = 0;
    int targetfd;
-	char *output = NULL, *config = NULL;
+	char *output = NULL, *body_output = NULL, *config = NULL;
    FILE *config_handle;
    context ctx = { 0 };
    /* Values cleared after each firmware or parameter, regardless if N/A */
@@ -2050,6 +2056,7 @@
    bool any_location = 0;
    uint32_t romsig_offset;
    uint32_t rom_base_address;
+	uint32_t romsig_head_size, fw_body_offset = 0xFFFFFFFF;
    uint8_t soc_id = PLATFORM_UNKNOWN;
    uint8_t efs_spi_readmode = 0xff;
    uint8_t efs_spi_speed = 0xff;
@@ -2225,6 +2232,9 @@
    	case AMDFW_OPT_OUTPUT:
    		output = optarg;
    		break;
+		case AMDFW_OPT_BODY_OUTPUT:
+			body_output = optarg;
+			break;
    	case AMDFW_OPT_FLASHSIZE:
    		ctx.rom_size = (uint32_t)strtoul(optarg, &tmp, 16);
    		if (*tmp != '\0') {
@@ -2241,6 +2251,14 @@
    			retval = 1;
    		}
    		break;
+		case AMDFW_OPT_FW_BODY_LOCATION:
+			fw_body_offset = (uint32_t)strtoul(optarg, &tmp, 16);
+			if (*tmp != '\0') {
+				fprintf(stderr, "Error: Directory Location specified"
+					" incorrectly (%s)\n\n", optarg);
+				retval = 1;
+			}
+			break;
    	case AMDFW_OPT_ANYWHERE:
    		any_location = 1;
    		break;
@@ -2418,6 +2436,16 @@
    		ctx.address_mode == AMD_ADDR_PHYSICAL ? "absolute" : "relative",
    		RUN_CURRENT(ctx));

+	romsig_head_size = 0x1000; /* Hardcode */
+	if (fw_body_offset != 0xFFFFFFFF) {
+		ctx.current += romsig_head_size;
+
+		if (ctx.current <= fw_body_offset)
+			ctx.current = fw_body_offset;
+		else
+			return 1;
+	}
+
    integrate_firmwares(&ctx, amd_romsig, amd_fw_table);

ctx.current = ALIGN_UP(ctx.current, 0x10000U); /* TODO: is it necessary? */
@@ -2535,9 +2563,12 @@

targetfd = open(output, O_RDWR | O_CREAT | O_TRUNC, 0666);
    if (targetfd >= 0) {
-		ssize_t bytes;
-		bytes = write(targetfd, amd_romsig, ctx.current - romsig_offset);
-		if (bytes != ctx.current - romsig_offset) {
+		ssize_t bytes, write_bytes;
+		write_bytes = fw_body_offset == 0xFFFFFFFF ?
+				ctx.current - romsig_offset :
+				romsig_head_size;
+		bytes = write(targetfd, amd_romsig, write_bytes);
+		if (bytes != write_bytes) {
    		fprintf(stderr, "Error: Writing to file %s failed\n", output);
    		retval = 1;
    	}
@@ -2546,6 +2577,22 @@
    	fprintf(stderr, "Error: could not open file: %s\n", output);
    	retval = 1;
    }
+	if (fw_body_offset != 0xFFFFFFFF && body_output != NULL) {
+		targetfd = open(body_output, O_RDWR | O_CREAT | O_TRUNC, 0666);
+		if (targetfd >= 0) {
+			ssize_t bytes;
+
+			bytes = write(targetfd, BUFF_OFFSET(ctx, fw_body_offset), ctx.current - fw_body_offset);
+			if (bytes != ctx.current - fw_body_offset) {
+				fprintf(stderr, "Error: Writing to file %s failed\n", output);
+				retval = 1;
+			}
+			close(targetfd);
+		} else {
+			fprintf(stderr, "Error: could not open file: %s\n", body_output);
+			retval = 1;
+		}
+	}

free(rom);
    return retval;