Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48500 )
Change subject: arch/x86: Simplify .id section ......................................................................
arch/x86: Simplify .id section
The version string is not required for backwards flashrom compatibility.
TBD: FILO needs to switch to board_info in CBFS first.
Change-Id: I3f4c066b2d914c3cf81275df5080e442bf57d568 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/x86/Makefile.inc M src/arch/x86/id.S 2 files changed, 0 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/48500/1
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 8e8cb56..f61e0e5 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -102,8 +102,6 @@ bootblock-y += id.S bootblock-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
-$(call src-to-obj,bootblock,$(dir)/id.S): $(obj)/build.h - ifeq ($(CONFIG_ARCH_BOOTBLOCK_X86_32),y) $(eval $(call early_x86_stage,bootblock,elf32-i386)) else diff --git a/src/arch/x86/id.S b/src/arch/x86/id.S index 574a7dc..5b660e7 100644 --- a/src/arch/x86/id.S +++ b/src/arch/x86/id.S @@ -1,22 +1,16 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-#include <build.h> - .section ".id", "a", @progbits
-ver: - .asciz COREBOOT_VERSION vendor: .asciz CONFIG_MAINBOARD_VENDOR part: .asciz CONFIG_MAINBOARD_PART_NUMBER
#if ENV_X86_64 -.long 0xffffffff - ver + 1 /* Reverse offset to the version */ .long 0xffffffff - vendor + 1 /* Reverse offset to the vendor id */ .long 0xffffffff - part + 1 /* Reverse offset to the part number */ #else -.long - ver /* Reverse offset to the version */ .long - vendor /* Reverse offset to the vendor id */ .long - part /* Reverse offset to the part number */ #endif
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48500 )
Change subject: arch/x86: Simplify .id section ......................................................................
Patch Set 3:
Nico, the solution in FILO of checking COREBOOT_VERSION in .id section does not work with UPDATE_IMAGE=y, fallback/normal choice or VBOOT? It's not really a problem to maintain .id for x86, but seems to me the feature of FILO flashupdate needs an update anyways.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48500 )
Change subject: arch/x86: Simplify .id section ......................................................................
Patch Set 3:
Nico, the solution in FILO of checking COREBOOT_VERSION in .id section does not work with UPDATE_IMAGE=y, fallback/normal choice or VBOOT? It's not really a problem to maintain .id for x86, but seems to me the feature of FILO flashupdate needs an update anyways.
The upstream FILO code looks stale anyway. I've looked into it though, and our production version does the same annoying check. You are right, it only works for very plain images. Or rather, images with the .id in the right spot. With UPDATE_IMAGE we were used to keep the original bootblock around, so I don't see a problem there. A VBOOT "update" with RW only wouldn't work.
I can look into updating that FILO code when we settled for a CBFS solution. If maintaining compatibility on the coreboot side is making any trouble, I would also not block it. My original concern was mostly about other potential users that don't have a voice here. Having a transition period would be nice, but is not a strong requirement.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48500 )
Change subject: arch/x86: Simplify .id section ......................................................................
Patch Set 3:
I can look into updating that FILO code when we settled for a CBFS solution. If maintaining compatibility on the coreboot side is making any trouble, I would also not block it. My original concern was mostly about other potential users that don't have a voice here. Having a transition period would be nice, but is not a strong requirement.
I moved the .id removal on x86 last on the patchtrain, so hopefully the changes for top-aligning bootblock binary and removal of static C_ENV_BOOTBLOCK_SIZE could proceed.
Kyösti Mälkki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48500 )
Change subject: arch/x86: Simplify .id section ......................................................................
Abandoned
-
Kyösti Mälkki (Code Review) -
Nico Huber (Code Review)