pulp-platform / cheshire

A minimal Linux-capable 64-bit RISC-V SoC built around CVA6
Other
190 stars 42 forks source link

Vivado's -tclargs does not use quotes #134

Closed krabo0om closed 2 months ago

krabo0om commented 3 months ago

The quotes around the arguments for -tclargs is wrong. Vivado thinks it is one argument and the tcl script stops. Remove the quotes -> -tclargs $(subst ., ,$*)

https://github.com/pulp-platform/cheshire/blob/8c38f78105c84584a8a5ea2603ca70bae60b6ccb/target/xilinx/xilinx.mk#L32 https://github.com/pulp-platform/cheshire/blob/8c38f78105c84584a8a5ea2603ca70bae60b6ccb/target/xilinx/xilinx.mk#L55 https://github.com/pulp-platform/cheshire/blob/8c38f78105c84584a8a5ea2603ca70bae60b6ccb/target/xilinx/xilinx.mk#L80

paulsc96 commented 3 months ago

Hi,

Thanks for opening this issue. Could you detail what versions of Vivado and GNU Make you are using and how they compare to those listed in our Getting Started guide? (EDIT: The Vivado version requirements are actually documented here, my bad)

The reason I ask is that we run these targets in internal CI for every commit on main from a bare checkout and check each resulting bitstream on real hardware, so we evidently cannot reproduce this issue. The only way I see this would go unnoticed is a mismatch in software versions.

krabo0om commented 3 months ago

Vivado: tested with 2022.1 and 2023.2 (same issue)
GNU Make: 4.3 (required >= 3.82) GNU bash, version 5.1.16(1) bender 0.28.1 (required >= 0.27.1)

In the logs/bash output for make chs-xilinx-genesys2 VIVADO=vivado I see the following line:

cd ..../target/xilinx/build/genesys2.clkwiz/ && vivado -mode batch -log ../genesys2.clkwiz.log -jou ../genesys2.clkwiz.jou -source ..../target/xilinx/scripts/impl_ip.tcl -tclargs "genesys2 clkwiz"

Note the quotation marks at the end. According to Xilinx, there should not be any quotation marks. Removing them solves the following error:

Error: Insufficient implementation arguments (1): {genesys2 clkwiz}.

    while executing
"init_impl $xilinx_root $argc $argv"
    (file "..../target/xilinx/scripts/impl_ip.tcl" line 12)
paulsc96 commented 3 months ago

This is strange; I checked the logs for the current head, and it works fine in Vivado 2022.1 with the quotes on our end.

Could you tell me what OS you are running?

I will try removing the quotes and see if this works on our setup.

paulsc96 commented 3 months ago

On my end, the targets work on Vivado 2022.1 with and without the quotes. I also tried 2023.2, but am facing an unrelated issue with our install there (missing board parts). In any case, this version also does not mind the quotes (the failing command you report is run earlier).

Since it makes no difference to us, we can remove the quotes. Although I am still curious what causes the different behavior in our setups.

krabo0om commented 3 months ago

Thanks for testing, indeed very interesting. We are using Ubuntu 22.04 with kernel 6.5.0. Maybe something with the locale, but that is just a guess. We use en_US.UTF-8 (echo $LANG)