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/+/67325
to review the following change.
Change subject: soc/amd/*: Put code about PSP FW into common Makefile.inc ......................................................................
soc/amd/*: Put code about PSP FW into common Makefile.inc
Change the variable XXXX_FWM_POSITION to a generic name AMD_SOC_FWM_POSITION. Put it with definitions of A/B FW position to common Makefile.inc.
Change-Id: I13aa83f57daf24e16ea1900b9920843a3f9b0337 Signed-off-by: Zheng Bao fishbaozi@gmail.com --- M src/soc/amd/cezanne/Makefile.inc M src/soc/amd/common/Makefile.inc M src/soc/amd/mendocino/Makefile.inc M src/soc/amd/picasso/Makefile.inc 4 files changed, 46 insertions(+), 94 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/67325/1
diff --git a/src/soc/amd/cezanne/Makefile.inc b/src/soc/amd/cezanne/Makefile.inc index 205b349..aa495dd 100644 --- a/src/soc/amd/cezanne/Makefile.inc +++ b/src/soc/amd/cezanne/Makefile.inc @@ -64,18 +64,6 @@ # | | PSPDIR ADDR | BIOSDIR ADDR | # +-----------+---------------+----------------+
-$(if $(CONFIG_AMD_FWM_POSITION_INDEX), ,\ - $(error Invalid AMD firmware position index. Check if the board sets a valid ROM size)) - -CEZANNE_FWM_POSITION=$(call int-add, \ - $(call int-subtract, 0xffffffff \ - $(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) \ $(AMD_FW_AB_POSITION)) @@ -240,7 +228,7 @@ $(OPT_APOB_NV_BASE) \ $(OPT_VERSTAGE_FILE) \ $(OPT_VERSTAGE_SIG_FILE) \ - --location $(shell printf "%#x" $(CEZANNE_FWM_POSITION)) \ + --location $(shell printf "%#x" $(AMD_SOC_FWM_POSITION)) \ --multilevel \ --output $@
@@ -274,24 +262,4 @@ --multilevel \ --output $@
- -cbfs-files-y += apu/amdfw -apu/amdfw-file := $(obj)/amdfw.rom -apu/amdfw-position := $(CEZANNE_FWM_POSITION) -apu/amdfw-type := raw - -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 -# 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 -# 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 - endif # ($(CONFIG_SOC_AMD_CEZANNE),y) diff --git a/src/soc/amd/common/Makefile.inc b/src/soc/amd/common/Makefile.inc index a935565..03e57c0 100644 --- a/src/soc/amd/common/Makefile.inc +++ b/src/soc/amd/common/Makefile.inc @@ -5,6 +5,18 @@ subdirs-y += vboot endif
+$(if $(CONFIG_AMD_FWM_POSITION_INDEX), ,\ + $(error Invalid AMD firmware position index. Check if the board sets a valid ROM size)) + +AMD_SOC_FWM_POSITION=$(call int-add, \ + $(call int-subtract, 0xffffffff \ + $(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 + ifneq ($(CONFIG_AMDFW_CONFIG_FILE), ) FIRMWARE_LOCATION=$(shell grep -e FIRMWARE_LOCATION $(CONFIG_AMDFW_CONFIG_FILE) | awk '{print $$2}')
@@ -14,4 +26,21 @@
amd_microcode_bins += $(wildcard ${FIRMWARE_LOCATION}/*UcodePatch_*.bin)
+cbfs-files-y += apu/amdfw +apu/amdfw-file := $(obj)/amdfw.rom +apu/amdfw-position := $(AMD_SOC_FWM_POSITION) +apu/amdfw-type := raw + +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 := $(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 := $(AMD_FW_AB_POSITION) +apu/amdfw_b-type := raw +endif + endif diff --git a/src/soc/amd/mendocino/Makefile.inc b/src/soc/amd/mendocino/Makefile.inc index 225b4a9..5bcbdc2 100644 --- a/src/soc/amd/mendocino/Makefile.inc +++ b/src/soc/amd/mendocino/Makefile.inc @@ -66,18 +66,6 @@ # | | PSPDIR ADDR | BIOSDIR ADDR | # +-----------+---------------+----------------+
-$(if $(CONFIG_AMD_FWM_POSITION_INDEX), ,\ - $(error Invalid AMD firmware position index. Check if the board sets a valid ROM size)) - -MENDOCINO_FWM_POSITION=$(call int-add, \ - $(call int-subtract, 0xffffffff \ - $(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 - MENDOCINO_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)) @@ -237,7 +225,7 @@ $(OPT_APOB_NV_BASE) \ $(OPT_VERSTAGE_FILE) \ $(OPT_VERSTAGE_SIG_FILE) \ - --location $(shell printf "%#x" $(MENDOCINO_FWM_POSITION)) \ + --location $(shell printf "%#x" $(AMD_SOC_FWM_POSITION)) \ --output $@
$(PSP_BIOSBIN_FILE): $(PSP_ELF_FILE) $(AMDCOMPRESS) @@ -268,22 +256,4 @@ --anywhere \ --output $@
- -cbfs-files-y += apu/amdfw -apu/amdfw-file := $(obj)/amdfw.rom -apu/amdfw-position := $(MENDOCINO_FWM_POSITION) -apu/amdfw-type := raw - -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 := $(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 := $(AMD_FW_AB_POSITION) -apu/amdfw_b-type := raw -endif - endif # ($(CONFIG_SOC_AMD_MENDOCINO),y) diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index 4bf1844..018a131 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -69,18 +69,6 @@ # | | PSPDIR ADDR | BIOSDIR ADDR | # +-----------+---------------+----------------+
-$(if $(CONFIG_AMD_FWM_POSITION_INDEX), ,\ - $(error Invalid AMD firmware position index. Check if the board sets a valid ROM size)) - -PICASSO_FWM_POSITION=$(call int-add, \ - $(call int-subtract, 0xffffffff \ - $(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)) @@ -246,7 +234,7 @@ $(OPT_APOB0_NV_BASE) \ $(OPT_VERSTAGE_FILE) \ $(OPT_VERSTAGE_SIG_FILE) \ - --location $(shell printf "%#x" $(PICASSO_FWM_POSITION)) \ + --location $(shell printf "%#x" $(AMD_SOC_FWM_POSITION)) \ --output $@
$(PSP_BIOSBIN_FILE): $(PSP_ELF_FILE) $(AMDCOMPRESS) @@ -277,21 +265,4 @@ --anywhere \ --output $@
-cbfs-files-y += apu/amdfw -apu/amdfw-file := $(obj)/amdfw.rom -apu/amdfw-position := $(PICASSO_FWM_POSITION) -apu/amdfw-type := raw - -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 := $(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 := $(AMD_FW_AB_POSITION) -apu/amdfw_b-type := raw -endif - endif # ($(CONFIG_SOC_AMD_PICASSO),y)