Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81087?usp=email )
Change subject: cbfs: Remove broken remnants of PAYLOAD_INFO feature ......................................................................
cbfs: Remove broken remnants of PAYLOAD_INFO feature
PAYLOAD_INFO is a very old feature that can add an key/value information section to a payload file. It seems to have only ever been generated by coreinfo and never really read by anything.
Since CB:1721 in 2012, the feature has been inadvertently broken in practice since the `.note.pinfo` sections that contain the information get discarded from the payload before cbfstool gets to see them. Since CB:28647 in 2018, support for the section in the SELF loader was (inadvertently?) dropped, so if someone actually fed cbfstool a payload ELF that did have a `.note.pinfo` section, modern coreboot would refuse to boot the payload entirely (which is probably not a good state to leave things in).
This patch removes the code to generate PAYLOAD_INFO entries entirely, but leaves the support to parse and extract those sections from old payloads in place in cbfstool.
Change-Id: I40d8e9b76a171ebcdaa2eae02d54a1ca5e592c85 Signed-off-by: Julius Werner jwerner@chromium.org --- M payloads/coreinfo/Kconfig M payloads/coreinfo/coreinfo.c M payloads/libpayload/include/libpayload.h M src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h M util/cbfstool/cbfs-mkpayload.c M util/cbfstool/cbfs_image.c 6 files changed, 12 insertions(+), 83 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/81087/1
diff --git a/payloads/coreinfo/Kconfig b/payloads/coreinfo/Kconfig index 2c1f91c..c0e314e 100644 --- a/payloads/coreinfo/Kconfig +++ b/payloads/coreinfo/Kconfig @@ -18,25 +18,13 @@
This option will increase the ELF file size by ca. 250 bytes.
-config PAYLOAD_INFO_NAME +config COREINFO_NAME string "Payload name" default "coreinfo" help The name of this payload for use in (e.g.) Bayou.
-config PAYLOAD_INFO_LISTNAME - string "Payload menu entry name" - default "System Information" - help - The name of this payload's menu entry for use in (e.g.) Bayou. - -config PAYLOAD_INFO_DESC - string "Payload description" - default "Display information about the system" - help - The description of this payload for use in (e.g.) Bayou. - -config PAYLOAD_INFO_VERSION +config COREINFO_VERSION string "Payload version" default "0.1" help diff --git a/payloads/coreinfo/coreinfo.c b/payloads/coreinfo/coreinfo.c index b357f97..b01dfeb 100644 --- a/payloads/coreinfo/coreinfo.c +++ b/payloads/coreinfo/coreinfo.c @@ -229,7 +229,7 @@ { int key;
- center(0, CONFIG_PAYLOAD_INFO_NAME " " CONFIG_PAYLOAD_INFO_VERSION); + center(0, CONFIG_COREINFO_NAME " " CONFIG_COREINFO_VERSION); print_no_modules_selected(); refresh();
@@ -319,7 +319,3 @@ halt(); return 0; } - -PAYLOAD_INFO(name, CONFIG_PAYLOAD_INFO_NAME); -PAYLOAD_INFO(listname, CONFIG_PAYLOAD_INFO_LISTNAME); -PAYLOAD_INFO(desc, CONFIG_PAYLOAD_INFO_DESC); diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index 61dbd27..05164e5 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -84,18 +84,6 @@
#define MAX_ARGC_COUNT 32
-/* - * Payload information parameters - these are used to pass information - * to the entity loading the payload. - * Usage: PAYLOAD_INFO(key, value) - * Example: PAYLOAD_INFO(name, "CoreInfo!") - */ -#define _pstruct(key) __pinfo_ ##key -#define PAYLOAD_INFO(key, value) \ -static const char _pstruct(key)[] \ - __attribute__((__used__)) \ - __attribute__((section(".note.pinfo"),unused)) = #key "=" value - /** * @defgroup nvram NVRAM and RTC functions * @{ diff --git a/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h b/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h index cebaf83..3d721f5 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h @@ -198,7 +198,8 @@ PAYLOAD_SEGMENT_CODE = 0x434F4445, /* BE: 'CODE' */ PAYLOAD_SEGMENT_DATA = 0x44415441, /* BE: 'DATA' */ PAYLOAD_SEGMENT_BSS = 0x42535320, /* BE: 'BSS ' */ - PAYLOAD_SEGMENT_PARAMS = 0x50415241, /* BE: 'PARA' */ + /* PARAMS for PAYLOAD_INFO feature. Broken since 2012, removed 2024. */ + PAYLOAD_SEGMENT_DEPRECATED_PARAMS = 0x50415241, /* BE: 'PARA' */ PAYLOAD_SEGMENT_ENTRY = 0x454E5452, /* BE: 'ENTR' */ };
diff --git a/util/cbfstool/cbfs-mkpayload.c b/util/cbfstool/cbfs-mkpayload.c index e9bcfba..a04d673 100644 --- a/util/cbfstool/cbfs-mkpayload.c +++ b/util/cbfstool/cbfs-mkpayload.c @@ -79,28 +79,8 @@
strtab = &header[shdr[ehdr.e_shstrndx].sh_offset];
- /* Count the number of headers - look for the .notes.pinfo - * section */ - - for (i = 0; i < ehdr.e_shnum; i++) { - char *name; - - if (i == ehdr.e_shstrndx) - continue; - - if (shdr[i].sh_size == 0) - continue; - - name = (char *)(strtab + shdr[i].sh_name); - - if (!strcmp(name, ".note.pinfo")) { - segments++; - isize += (unsigned int)shdr[i].sh_size; - } - } - - /* Now, regular headers - we only care about PT_LOAD headers, - * because that's what we're actually going to load + /* Count the number of segment headers - we only care about PT_LOAD + * headers, because that's what we're actually going to load */
for (i = 0; i < headers; i++) { @@ -144,30 +124,6 @@ */ segments = 0;
- for (i = 0; i < ehdr.e_shnum; i++) { - char *name; - if (i == ehdr.e_shstrndx) - continue; - - if (shdr[i].sh_size == 0) - continue; - name = (char *)(strtab + shdr[i].sh_name); - if (!strcmp(name, ".note.pinfo")) { - segs[segments].type = PAYLOAD_SEGMENT_PARAMS; - segs[segments].load_addr = 0; - segs[segments].len = (unsigned int)shdr[i].sh_size; - segs[segments].offset = doffset; - - memcpy((unsigned long *)(output->data + doffset), - &header[shdr[i].sh_offset], shdr[i].sh_size); - - doffset += segs[segments].len; - osize += segs[segments].len; - - segments++; - } - } - for (i = 0; i < headers; i++) { if (phdr[i].p_type != PT_LOAD) continue; diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 1fde8cd..740a3b2 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -864,7 +864,7 @@ if (segments[i].type == PAYLOAD_SEGMENT_BSS || segments[i].type == PAYLOAD_SEGMENT_ENTRY) { continue; - } else if (segments[i].type == PAYLOAD_SEGMENT_PARAMS) { + } else if (segments[i].type == PAYLOAD_SEGMENT_DEPRECATED_PARAMS) { memcpy(out_ptr, in_ptr, segments[i].len); segments[i].offset = new_offset; new_offset += segments[i].len; @@ -1090,7 +1090,7 @@ segments++; } else if (payload_type == PAYLOAD_SEGMENT_BSS) { segments++; - } else if (payload_type == PAYLOAD_SEGMENT_PARAMS) { + } else if (payload_type == PAYLOAD_SEGMENT_DEPRECATED_PARAMS) { segments++; } else if (payload_type == PAYLOAD_SEGMENT_ENTRY) { /* The last segment in a payload is always ENTRY as @@ -1162,7 +1162,7 @@ shdr.sh_size = segs[i].len; name = strdup(".bss"); buffer_splice(&tbuff, buff, 0, 0); - } else if (segs[i].type == PAYLOAD_SEGMENT_PARAMS) { + } else if (segs[i].type == PAYLOAD_SEGMENT_DEPRECATED_PARAMS) { shdr.sh_type = SHT_NOTE; shdr.sh_flags = 0; shdr.sh_size = segs[i].len; @@ -1398,8 +1398,8 @@ seg->load_addr, seg->len); break;
- case PAYLOAD_SEGMENT_PARAMS: - fprintf(fp, " parameters\n"); + case PAYLOAD_SEGMENT_DEPRECATED_PARAMS: + fprintf(fp, " parameters (deprecated)\n"); break;
default:
-
Julius Werner (Code Review)