soc/amd/cezanne,picasso,sabrina: Fix incorrect values of CBFS amdfw position makefile variables
Currently apu/amdfw_a-position and apu/amdfw_b-position currently depend on CEZANNE_FW_A_POSITION and CEZANNE_FW_B_POSITION. This causes error messages from awk as these variables are sourced from fmap_config.h and these variables are expanded before fmap_config.h is built. However these variables should not be set to CEZANNE_FW_*_POSITION. These files end up in the FW_MAIN_* fmap regions. These regions are placed at the proper locations through the chromeos.fmd file. The apu/amdfw_*-position variables are the positions within these regions where the files end up. These variables should be set to 0x40 to coincide with the beginning of the FW_MAIN_* regions, accounting for the size of struct cbfs_file + filename + metadata, aligned to 64 bytes. Currently they end up in the correct locations only because fmap_config.h does not exist when the apu/amdfw_*-position variables are expanded.
This change explicity sets the value of these variables to 0x40, removing the errors from awk and ensuring that these files end up in the correct location in the resulting image. These changes are also applied to the Picasso and Sabrina makefiles as well.
BUG=b:198322933
TEST=Verified that the apu/amdfw_* files end up in the correct locations as reported by cbfstool during the build, did timeless builds and confirmed that coreboot.rom images were identical, tested AP firmware on guybrush and zork devices
Signed-off-by: Robert Zieba <robertzieba@google.com>
Change-Id: If1c2b61c5be0bcab52e19349dacbcc391e8aa909
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61349
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Rob Barnes <robbarnes@google.com>
diff --git a/src/mainboard/google/zork/Kconfig b/src/mainboard/google/zork/Kconfig
index 96f40e4..dfc87bf 100644
--- a/src/mainboard/google/zork/Kconfig
+++ b/src/mainboard/google/zork/Kconfig
@@ -133,22 +133,6 @@
hex
default 0x50
-config PICASSO_FW_A_POSITION
- hex
- default 0xFF012040
- depends on VBOOT_SLOTS_RW_AB
- help
- Location of the AMD firmware in the RW_A region. This is the
- start of the RW-A region + 64 bytes for the cbfs header.
-
-config PICASSO_FW_B_POSITION
- hex
- default 0xFF312040
- depends on VBOOT_SLOTS_RW_AB
- help
- Location of the AMD firmware in the RW_B region. This is the
- start of the RW-A region + 64 bytes for the cbfs header.
-
config VARIANT_SUPPORTS_PRE_V3_SCHEMATICS
bool
default y if BOARD_GOOGLE_TREMBYLE
diff --git a/src/soc/amd/cezanne/Makefile.inc b/src/soc/amd/cezanne/Makefile.inc
index 5708baa0..332268c 100644
--- a/src/soc/amd/cezanne/Makefile.inc
+++ b/src/soc/amd/cezanne/Makefile.inc
@@ -73,13 +73,17 @@
$(call int-shift-left, \
0x80000 $(CONFIG_AMD_FWM_POSITION_INDEX))) 0x20000 1)
+# 0x40 accounts for the cbfs_file struct + filename + metadata structs, aligned to 64 bytes
+# Building the cbfs image will fail if the offset isn't large enough
+AMD_FW_AB_POSITION := 0x40
+
CEZANNE_FW_A_POSITION=$(call int-add, \
$(shell awk '$$2 == "FMAP_SECTION_FW_MAIN_A_START" {print $$3}' $(obj)/fmap_config.h) \
- 0x40)
+ $(AMD_FW_AB_POSITION))
CEZANNE_FW_B_POSITION=$(call int-add, \
$(shell awk '$$2 == "FMAP_SECTION_FW_MAIN_B_START" {print $$3}' $(obj)/fmap_config.h) \
- 0x40)
+ $(AMD_FW_AB_POSITION))
#
# PSP Directory Table items
#
@@ -278,12 +282,14 @@
ifeq ($(CONFIG_VBOOT_SLOTS_RW_AB)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy)
cbfs-files-y += apu/amdfw_a
apu/amdfw_a-file := $(obj)/amdfw_a.rom
-apu/amdfw_a-position := $(call strip_quotes, $(CEZANNE_FW_A_POSITION))
+# Ensure this ends up at the beginning of the FW_MAIN_A fmap region
+apu/amdfw_a-position := $(AMD_FW_AB_POSITION)
apu/amdfw_a-type := raw
cbfs-files-y += apu/amdfw_b
apu/amdfw_b-file := $(obj)/amdfw_b.rom
-apu/amdfw_b-position := $(call strip_quotes, $(CEZANNE_FW_B_POSITION))
+# Ensure this ends up at the beginning of the FW_MAIN_B fmap region
+apu/amdfw_b-position := $(AMD_FW_AB_POSITION)
apu/amdfw_b-type := raw
endif
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig
index ebcbd62..558f301 100644
--- a/src/soc/amd/picasso/Kconfig
+++ b/src/soc/amd/picasso/Kconfig
@@ -517,16 +517,6 @@
Add a space-delimited list of filenames that should only be in the
RW-B section.
-config PICASSO_FW_A_POSITION
- hex
- help
- Location of the AMD firmware in the RW_A region
-
-config PICASSO_FW_B_POSITION
- hex
- help
- Location of the AMD firmware in the RW_B region
-
endif # VBOOT_SLOTS_RW_AB && VBOOT_STARTS_BEFORE_BOOTBLOCK
endif # SOC_AMD_PICASSO
diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc
index de16da4..8eef153 100644
--- a/src/soc/amd/picasso/Makefile.inc
+++ b/src/soc/amd/picasso/Makefile.inc
@@ -77,6 +77,18 @@
$(call int-shift-left, \
0x80000 $(CONFIG_AMD_FWM_POSITION_INDEX))) 0x20000 1)
+# 0x40 accounts for the cbfs_file struct + filename + metadata structs, aligned to 64 bytes
+# Building the cbfs image will fail if the offset isn't large enough
+AMD_FW_AB_POSITION := 0x40
+
+PICASSO_FW_A_POSITION=$(call int-add, \
+ $(shell awk '$$2 == "FMAP_SECTION_FW_MAIN_A_START" {print $$3}' $(obj)/fmap_config.h) \
+ $(AMD_FW_AB_POSITION))
+
+PICASSO_FW_B_POSITION=$(call int-add, \
+ $(shell awk '$$2 == "FMAP_SECTION_FW_MAIN_B_START" {print $$3}' $(obj)/fmap_config.h) \
+ $(AMD_FW_AB_POSITION))
+
#
# PSP Directory Table items
#
@@ -248,7 +260,7 @@
$(AMDFW_COMMON_ARGS) \
$(OPT_APOB_NV_SIZE) \
$(OPT_APOB_NV_BASE) \
- --location $(shell printf "%#x" $(CONFIG_PICASSO_FW_A_POSITION)) \
+ --location $(shell printf "%#x" $(PICASSO_FW_A_POSITION)) \
--anywhere \
--output $@
@@ -259,7 +271,7 @@
$(AMDFW_COMMON_ARGS) \
$(OPT_APOB_NV_SIZE) \
$(OPT_APOB_NV_BASE) \
- --location $(shell printf "%#x" $(CONFIG_PICASSO_FW_B_POSITION)) \
+ --location $(shell printf "%#x" $(PICASSO_FW_B_POSITION)) \
--anywhere \
--output $@
@@ -271,12 +283,12 @@
ifeq ($(CONFIG_VBOOT_SLOTS_RW_AB)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy)
cbfs-files-y += apu/amdfw_a
apu/amdfw_a-file := $(obj)/amdfw_a.rom
-apu/amdfw_a-position := $(call strip_quotes, $(CONFIG_PICASSO_FW_A_POSITION))
+apu/amdfw_a-position := $(AMD_FW_AB_POSITION)
apu/amdfw_a-type := raw
cbfs-files-y += apu/amdfw_b
apu/amdfw_b-file := $(obj)/amdfw_b.rom
-apu/amdfw_b-position := $(call strip_quotes, $(CONFIG_PICASSO_FW_B_POSITION))
+apu/amdfw_b-position := $(AMD_FW_AB_POSITION)
apu/amdfw_b-type := raw
endif
diff --git a/src/soc/amd/sabrina/Makefile.inc b/src/soc/amd/sabrina/Makefile.inc
index 22ba2cc..74124d2 100644
--- a/src/soc/amd/sabrina/Makefile.inc
+++ b/src/soc/amd/sabrina/Makefile.inc
@@ -72,13 +72,17 @@
$(call int-shift-left, \
0x80000 $(CONFIG_AMD_FWM_POSITION_INDEX))) 0x20000 1)
+# 0x40 accounts for the cbfs_file struct + filename + metadata structs, aligned to 64 bytes
+# Building the cbfs image will fail if the offset isn't large enough
+AMD_FW_AB_POSITION := 0x40
+
SABRINA_FW_A_POSITION=$(call int-add, \
$(shell awk '$$2 == "FMAP_SECTION_FW_MAIN_A_START" {print $$3}' $(obj)/fmap_config.h) \
- 0x40)
+ $(AMD_FW_AB_POSITION))
SABRINA_FW_B_POSITION=$(call int-add, \
$(shell awk '$$2 == "FMAP_SECTION_FW_MAIN_B_START" {print $$3}' $(obj)/fmap_config.h) \
- 0x40)
+ $(AMD_FW_AB_POSITION))
#
# PSP Directory Table items
#
@@ -277,12 +281,12 @@
ifeq ($(CONFIG_VBOOT_SLOTS_RW_AB)$(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),yy)
cbfs-files-y += apu/amdfw_a
apu/amdfw_a-file := $(obj)/amdfw_a.rom
-apu/amdfw_a-position := $(call strip_quotes, $(SABRINA_FW_A_POSITION))
+apu/amdfw_a-position := $(AMD_FW_AB_POSITION)
apu/amdfw_a-type := raw
cbfs-files-y += apu/amdfw_b
apu/amdfw_b-file := $(obj)/amdfw_b.rom
-apu/amdfw_b-position := $(call strip_quotes, $(SABRINA_FW_B_POSITION))
+apu/amdfw_b-position := $(AMD_FW_AB_POSITION)
apu/amdfw_b-type := raw
endif