Hi Rebecca,

Thank you for your patch. The TF-A project uses Gerrit for patch contributions. Please upload this as described at https://trustedfirmware-a.readthedocs.io/en/latest/process/contributing.html... . If it turns out impossible, please let me know and I'll upload it for you.

In the meantime, I have a few comments below.

On Wed, Dec 7, 2022 at 9:45 PM Rebecca Cran rebecca@quicinc.com wrote:

While aarch32 isn't currently supported on qemu, platform.mk for both qemu and qemu_sbsa contain bugs that cause the build to fail earlier than it otherwise should.

Correct usage of ${ARCH} and hardcoded aarch64 strings so that the correct files are built when ARCH=aarch32 is specified.

Signed-off-by: Rebecca Cran rebecca@quicinc.com

plat/qemu/qemu/platform.mk | 12 ++++++------ plat/qemu/qemu_sbsa/platform.mk | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/plat/qemu/qemu/platform.mk b/plat/qemu/qemu/platform.mk index dfc5de2ce43a..7b69ff76f3d6 100644 --- a/plat/qemu/qemu/platform.mk +++ b/plat/qemu/qemu/platform.mk @@ -45,7 +45,7 @@ PLAT_INCLUDES := -Iinclude/plat/arm/common/ \ -Iinclude/common/tbbr

ifeq (${ARM_ARCH_MAJOR},8) -PLAT_INCLUDES += -Iinclude/plat/arm/common/${ARCH} +PLAT_INCLUDES += -Iinclude/plat/arm/common/aarch64

While the aarch32 counterpart doesn't exist, using aarch64 for aarch32 here will not work at all. In fact, I believe it's wrong for plat/qemu to use this include directory, to begin with. I see that we use the macro arm_print_gic_regs from there and possibly more. It would be good if we could avoid reaching into the internals of plat/arm.

endif

PLAT_BL_COMMON_SOURCES := ${PLAT_QEMU_COMMON_PATH}/qemu_common.c \ @@ -138,11 +138,11 @@ BL1_SOURCES += drivers/io/io_semihosting.c \ ${PLAT_QEMU_COMMON_PATH}/qemu_bl1_setup.c

ifeq (${ARM_ARCH_MAJOR},8) -BL1_SOURCES += lib/cpus/aarch64/aem_generic.S \

•                           lib/cpus/aarch64/cortex_a53.S           \
  

•                           lib/cpus/aarch64/cortex_a57.S           \
  

•                           lib/cpus/aarch64/cortex_a72.S           \
  

•                           lib/cpus/aarch64/qemu_max.S             \
  

+BL1_SOURCES += lib/cpus/${ARCH}/aem_generic.S \

•                           lib/cpus/${ARCH}/cortex_a53.S           \
  

•                           lib/cpus/${ARCH}/cortex_a57.S           \
  

•                           lib/cpus/${ARCH}/cortex_a72.S           \
  

•                           lib/cpus/${ARCH}/qemu_max.S             \
  
  

Makes sense.

else BL1_SOURCES += lib/cpus/${ARCH}/cortex_a15.S diff --git a/plat/qemu/qemu_sbsa/platform.mk b/plat/qemu/qemu_sbsa/platform.mk index 5a6b1e11e812..05a66f81314f 100644 --- a/plat/qemu/qemu_sbsa/platform.mk +++ b/plat/qemu/qemu_sbsa/platform.mk @@ -30,7 +30,7 @@ PLAT_INCLUDES := -Iinclude/plat/arm/common/ \ -I${PLAT_QEMU_PATH}/include \ -Iinclude/common/tbbr

-PLAT_INCLUDES += -Iinclude/plat/arm/common/${ARCH} +PLAT_INCLUDES += -Iinclude/plat/arm/common/aarch64

Same problem here as above in the other platform.mk file.

Cheers, Jens

PLAT_BL_COMMON_SOURCES := ${PLAT_QEMU_COMMON_PATH}/qemu_common.c \ ${PLAT_QEMU_COMMON_PATH}/qemu_console.c \ -- 2.30.2