[M] Change in coreboot[master]: amdfwtool: Change .rom.efs to .rom and .rom to .rom.body
Attention is currently required from: Jason Glenesk, Raul Rangel, Matt DeVillier, Zheng Bao, Fred Reitberger, Felix Held.
Hello Zheng Bao,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/72702
to review the following change.
Change subject: amdfwtool: Change .rom.efs to .rom and .rom to .rom.body ......................................................................
amdfwtool: Change .rom.efs to .rom and .rom to .rom.body
The non-vboot also need to split amdfw. Keep the default filename which is located at EFS.
Change-Id: Id77b11422d4549cf57a1cd8980c7a9cf3597d1bc Signed-off-by: Zheng Bao fishbaozi@gmail.com --- M src/soc/amd/mendocino/Makefile.inc M util/amdfwtool/amdfwtool.c 2 files changed, 48 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/72702/1
diff --git a/src/soc/amd/mendocino/Makefile.inc b/src/soc/amd/mendocino/Makefile.inc index 7d7033a..4408b98 100644 --- a/src/soc/amd/mendocino/Makefile.inc +++ b/src/soc/amd/mendocino/Makefile.inc @@ -165,8 +165,8 @@ SIGNED_AMDFW_B_POSITION=$(call int-subtract, \ $(shell awk '$$2 == "FMAP_SECTION_SIGNED_AMDFW_B_START" {print $$3}' $(obj)/fmap_config.h) \ $(shell awk '$$2 == "FMAP_SECTION_FLASH_START" {print $$3}' $(obj)/fmap_config.h)) -SIGNED_AMDFW_A_FILE=$(obj)/amdfw_a.rom.signed -SIGNED_AMDFW_B_FILE=$(obj)/amdfw_b.rom.signed +SIGNED_AMDFW_A_FILE=$(obj)/amdfw_a.rom.body.signed +SIGNED_AMDFW_B_FILE=$(obj)/amdfw_b.rom.body.signed endif # CONFIG_SEPARATE_SIGNED_PSPFW
# Helper function to return a value with given bit set @@ -301,41 +301,41 @@ --anywhere \ --output $@
-$(obj)/amdfw_a.rom.efs: $(obj)/amdfw_a.rom -$(obj)/amdfw_b.rom.efs: $(obj)/amdfw_b.rom +$(obj)/amdfw_a.rom.body: $(obj)/amdfw_a.rom +$(obj)/amdfw_b.rom.body: $(obj)/amdfw_b.rom
ifeq ($(CONFIG_VBOOT_SLOTS_RW_A)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy) cbfs-files-y += apu/amdfw_a -apu/amdfw_a-file := $(obj)/amdfw_a.rom.efs +apu/amdfw_a-file := $(obj)/amdfw_a.rom apu/amdfw_a-position := $(AMD_FW_AB_POSITION) apu/amdfw_a-type := raw
cbfs-files-y += apu/amdfw_a_body -apu/amdfw_a_body-file := $(obj)/amdfw_a.rom +apu/amdfw_a_body-file := $(obj)/amdfw_a.rom.body apu/amdfw_a_body-position := $(call int-add, $(AMD_FW_AB_POSITION) $(MENDOCINO_FW_BODY_OFFSET)) apu/amdfw_a_body-type := raw endif
ifeq ($(CONFIG_VBOOT_SLOTS_RW_AB)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy) cbfs-files-y += apu/amdfw_b -apu/amdfw_b-file := $(obj)/amdfw_b.rom.efs +apu/amdfw_b-file := $(obj)/amdfw_b.rom apu/amdfw_b-position := $(AMD_FW_AB_POSITION) apu/amdfw_b-type := raw
cbfs-files-y += apu/amdfw_b_body -apu/amdfw_b_body-file := $(obj)/amdfw_b.rom +apu/amdfw_b_body-file := $(obj)/amdfw_b.rom.body apu/amdfw_b_body-position := $(call int-add, $(AMD_FW_AB_POSITION) $(MENDOCINO_FW_BODY_OFFSET)) apu/amdfw_b_body-type := raw endif
ifeq ($(CONFIG_SEPARATE_SIGNED_PSPFW)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy) -build_complete:: $(obj)/amdfw_a.rom $(obj)/amdfw_b.rom +build_complete:: $(obj)/amdfw_a.rom.body $(obj)/amdfw_b.rom.body @printf " Adding Signed ROM and HASH\n" - $(CBFSTOOL) $(obj)/coreboot.rom write -u -r SIGNED_AMDFW_A -i 0 -f $(obj)/amdfw_a.rom.signed - $(CBFSTOOL) $(obj)/coreboot.rom write -u -r SIGNED_AMDFW_B -i 0 -f $(obj)/amdfw_b.rom.signed - $(CBFSTOOL) $(obj)/coreboot.rom add -r FW_MAIN_A -f $(obj)/amdfw_a.rom.signed.hash \ + $(CBFSTOOL) $(obj)/coreboot.rom write -u -r SIGNED_AMDFW_A -i 0 -f $(obj)/amdfw_a.rom.body.signed + $(CBFSTOOL) $(obj)/coreboot.rom write -u -r SIGNED_AMDFW_B -i 0 -f $(obj)/amdfw_b.rom.body.signed + $(CBFSTOOL) $(obj)/coreboot.rom add -r FW_MAIN_A -f $(obj)/amdfw_a.rom.body.signed.hash \ -n apu/amdfw_a_hash -t raw - $(CBFSTOOL) $(obj)/coreboot.rom add -r FW_MAIN_B -f $(obj)/amdfw_b.rom.signed.hash \ + $(CBFSTOOL) $(obj)/coreboot.rom add -r FW_MAIN_B -f $(obj)/amdfw_b.rom.body.signed.hash \ -n apu/amdfw_b_hash -t raw endif
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 7605f30..562ed22 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -111,7 +111,7 @@ #define HASH_FILE_SUFFIX ".hash" #define EFS_FILE_SUFFIX ".efs" #define TMP_FILE_SUFFIX ".tmp" - +#define BODY_FILE_SUFFIX ".body" /* * Beginning with Family 15h Models 70h-7F, a.k.a Stoney Ridge, the PSP * can support an optional "combo" implementation. If the PSP sees the @@ -2019,48 +2019,48 @@ return 0; }
-static ssize_t write_efs(char *output, embedded_firmware *amd_romsig) +static ssize_t write_body(char *output, embedded_firmware *amd_romsig, ssize_t body_size) { - char efs_name[PATH_MAX], efs_tmp_name[PATH_MAX]; + char body_name[PATH_MAX], body_tmp_name[PATH_MAX]; int ret; int fd; ssize_t bytes = -1;
/* Create a tmp file and rename it at the end so that make does not get confused if amdfwtool is killed for some unexpected reasons. */ - ret = snprintf(efs_tmp_name, sizeof(efs_tmp_name), "%s%s%s", - output, EFS_FILE_SUFFIX, TMP_FILE_SUFFIX); + ret = snprintf(body_tmp_name, sizeof(body_tmp_name), "%s%s%s", + output, BODY_FILE_SUFFIX, TMP_FILE_SUFFIX); if (ret < 0) { - fprintf(stderr, "Error %s forming EFS tmp file name: %d\n", + fprintf(stderr, "Error %s forming BODY tmp file name: %d\n", strerror(errno), ret); exit(1); - } else if ((unsigned int)ret >= sizeof(efs_tmp_name)) { - fprintf(stderr, "EFS File name %d > %zu\n", ret, sizeof(efs_tmp_name)); + } else if ((unsigned int)ret >= sizeof(body_tmp_name)) { + fprintf(stderr, "BODY File name %d > %zu\n", ret, sizeof(body_tmp_name)); exit(1); }
- fd = open(efs_tmp_name, O_RDWR | O_CREAT | O_TRUNC, 0666); + fd = open(body_tmp_name, O_RDWR | O_CREAT | O_TRUNC, 0666); if (fd < 0) { - fprintf(stderr, "Error: Opening %s file: %s\n", efs_tmp_name, strerror(errno)); + fprintf(stderr, "Error: Opening %s file: %s\n", body_tmp_name, strerror(errno)); exit(1); }
- bytes = write_from_buf_to_file(fd, amd_romsig, sizeof(*amd_romsig)); - if (bytes != sizeof(*amd_romsig)) { - fprintf(stderr, "Error: Writing to file %s failed\n", efs_tmp_name); + bytes = write_from_buf_to_file(fd, amd_romsig, body_size); + if (bytes != body_size) { + fprintf(stderr, "Error: Writing to file %s failed\n", body_tmp_name); exit(1); } close(fd);
/* Rename the tmp file */ - ret = snprintf(efs_name, sizeof(efs_name), "%s%s", output, EFS_FILE_SUFFIX); + ret = snprintf(body_name, sizeof(body_name), "%s%s", output, BODY_FILE_SUFFIX); if (ret < 0) { - fprintf(stderr, "Error %s forming EFS file name: %d\n", strerror(errno), ret); + fprintf(stderr, "Error %s forming BODY file name: %d\n", strerror(errno), ret); exit(1); }
- if (rename(efs_tmp_name, efs_name)) { - fprintf(stderr, "Error: renaming file %s to %s\n", efs_tmp_name, efs_name); + if (rename(body_tmp_name, body_name)) { + fprintf(stderr, "Error: renaming file %s to %s\n", body_tmp_name, body_name); exit(1); }
@@ -2677,11 +2677,11 @@
targetfd = open(output, O_RDWR | O_CREAT | O_TRUNC, 0666); if (targetfd >= 0) { - ssize_t bytes; - uint32_t offset = body_location ? body_location : AMD_ROMSIG_OFFSET; + uint32_t offset = efs_location; + uint32_t bytes = efs_location == body_location ? ctx.current - offset : sizeof(*amd_romsig), ret_bytes;
- bytes = write(targetfd, BUFF_OFFSET(ctx, offset), ctx.current - offset); - if (bytes != ctx.current - offset) { + ret_bytes = write(targetfd, BUFF_OFFSET(ctx, offset), bytes); + if (bytes != ret_bytes) { fprintf(stderr, "Error: Writing to file %s failed\n", output); retval = 1; } @@ -2692,13 +2692,7 @@ }
if (efs_location != body_location) { - ssize_t bytes; - - bytes = write_efs(output, amd_romsig); - if (bytes != sizeof(*amd_romsig)) { - fprintf(stderr, "Error: Writing EFS\n"); - retval = 1; - } + write_body(output, BUFF_OFFSET(ctx, body_location), ctx.current - body_location); }
free(rom);
-
Bao Zheng (Code Review)