search

OpenAMP / open-amp

The main OpenAMP library implementing RPMSG, Virtio, and Remoteproc for RTOS etc
https://www.openampproject.org/
Other
678 stars 278 forks source link

OpenAMP library v2023.10 build failure #532

Open edmooring opened 8 months ago

edmooring commented 8 months ago

cd /OpenAMP-Project/gh3/open-amp/build_r5/lib && /opt/arm/gcc-arm-none-eabi-10.3-2021.10/bin/arm-none-eabi-gcc -I/OpenAMP-Project/gh3/libmetal/build_r5/lib/include -I/OpenAMP-Project/gh3/open-amp/lib/include -I/OpenAMP-Project/gh3/open-amp/build_r5/include/generated/openamp -mfloat-abi=hard -mcpu=cortex-r5 -Os -mfpu=vfpv3-d16 -DUNDEFINE_FILE_OPS -Wall -Wextra -Wcast-align -I/OpenAMP-Project/gh3/open-amp/../libmetal/usr/local/include -I/home/mooring/build-oa/psu_cortexr5_0/include -Wall -Wextra -MD -MT lib/CMakeFiles/open_amp-static.dir/virtio/virtio.c.obj -MF CMakeFiles/open_amp-static.dir/virtio/virtio.c.obj.d -o CMakeFiles/open_amp-static.dir/virtio/virtio.c.obj -c /OpenAMP-Project/gh3/open-amp/lib/virtio/virtio.c In file included from /OpenAMP-Project/gh3/libmetal/build_r5/lib/include/metal/sys.h:85, from /OpenAMP-Project/gh3/libmetal/build_r5/lib/include/metal/io.h:22, from /OpenAMP-Project/gh3/open-amp/lib/include/openamp/virtqueue.h:22, from /OpenAMP-Project/gh3/open-amp/lib/include/openamp/virtio.h:10, from /OpenAMP-Project/gh3/open-amp/lib/virtio/virtio.c:7: /OpenAMP-Project/gh3/libmetal/build_r5/lib/include/metal/system/generic/sys.h:29:10: fatal error: ./zynqmp_r5/sys.h: No such file or directory 29 | #include "./zynqmp_r5/sys.h" | ^~~~~~~ compilation terminated. make[2]: [lib/CMakeFiles/open_amp-static.dir/build.make:90: lib/CMakeFiles/open_amp-static.dir/virtio/virtio.c.obj] Error 1 make[2]: Leaving directory '/OpenAMP-Project/gh3/open-amp/build_r5' make[1]: [CMakeFiles/Makefile2:187: lib/CMakeFiles/open_amp-static.dir/all] Error 2 make[1]: Leaving directory '/OpenAMP-Project/gh3/open-amp/build_r5'

The relevant code is: '''

ifdef XLNX_PLATFORM

include <metal/system/generic/xlnx/sys.h>

else

include "./@PROJECT_MACHINE@/sys.h"

endif

'''

The problem is that XLNX_PLATFORM is defined as a compiler option in libmetal. There is no mechanism to pass this definition along to clients of the libmetal library such as open-amp. Therefore XLNX_PLATFORM is always undefined when building the open-amp library. It appears this only affects Xilinx platforms that are built using an external toolchain.

github-actions[bot] commented 6 months ago

This issue has been marked as a stale issue because it has been open (more than) 45 days with no activity.

tnmysh commented 6 months ago

For Now, Clients are expected to pass XLNX_PLATFORM definition during compile command with -D option. For AMD-Xilinx petalinux builds, it will be passed via Yocto recipe.

However, this should be fixed in libmetal in a way where XLNX_PLATFORM flag should not be required.

tnmysh commented 6 months ago

@edmooring could you please post steps to reproduce this error ?

edmooring commented 6 months ago

@tnmysh The process is a bit complicated. @bentheredonethat showed me how to do it 4+ years ago.

  1. Install an external cross-toolchain. I used the gcc-10.3 tarball from ARM.
  2. Install a Xilinx/AMD BSP. This is kind of tricky and where I needed most of the help from Ben.
  3. Create toolchain files to reflect the locations of the gcc install and the BSP.
  4. Create CMake incantations.
  5. Build libmetal
  6. Build open-amp (which will fail as shown above)

I have attached the toolchain files and and CMake commands I use. r5-build-lm.sa.txt r5-build-oa.sa.txt toolchain.r5-lm.sa.txt toolchain.r5-oa.sa.txt

bentheredonethat commented 6 months ago

Hi @edmooring can you pass the definition in your toolchain file so that it is present in the CMake cache?

Or as Tanmay suggested pass the command in the cmake configure step.

in the future one scalable approach we can take is to instead add cmake logic to set/get a property based on the machine since a property can be used at multiple scopes. CC @tnmysh

edmooring commented 6 months ago

Hi @edmooring can you pass the definition in your toolchain file so that it is present in the CMake cache?

Or as Tanmay suggested pass the command in the cmake configure step.

in the future one scalable approach we can take is to instead add cmake logic to set/get a property based on the machine since a property can be used at multiple scopes. CC @tnmysh

Adding it to the toolchain file is a usable workaround for now. I don't like introducing additional coupling between the liberal and open-amp library builds. It requires additional documentation and sets a bad precedent.

tnmysh commented 5 months ago

@edmooring how about if we have these toolchain files in respective upstream repositories ? Then in README, we can provide steps how to get necessary library from Xilinx's BSP and then people can build libmetal and openamp libs only using cmake and any toolchain they want.

bentheredonethat commented 5 months ago

can we also instead move it into a CFLAG? that way @tnmysh it can be done on the command line or in toolchain file

we already do this to denote the processor for example (-DARMR5)

github-actions[bot] commented 3 months ago

This issue has been marked as a stale issue because it has been open (more than) 45 days with no activity.