Fixup parameter to match plusargs

stffrdhrn commented 3 years ago

Due to recent changes in fusesoc/edalize it seems that the parameter name elf-load no longer works. Changing it to elf_load fixes the issue also when running fusesoc we must pass the argument --elf_load instead of --elf-load.

Without this we get errors:

$ fusesoc run --target mor1kx_tb mor1kx-generic --elf-load
./native/build/or1k/or1k-jmp
INFO: Preparing ::adv_debug_sys:3.1.0-r1
INFO: Preparing ::cdc_utils:0.1
INFO: Preparing ::elf-loader:1.0.2
INFO: Preparing ::intgen:0
INFO: Preparing ::jtag_tap:1.13-r1
INFO: Preparing ::jtag_vpi:0-r4
INFO: Preparing ::mor1kx:5.0-r3
INFO: Preparing ::uart16550:1.5.5-r1
INFO: Preparing ::verilog-arbiter:0-r2
INFO: Preparing ::vlog_tb_utils:1.1
INFO: Preparing ::wb_common:1.0.3
INFO: Preparing ::wb_bfm:1.2.1
INFO: Preparing ::wb_intercon:1.2.2
INFO: Preparing ::wb_ram:1.1
INFO: Preparing ::mor1kx-generic:1.1
ERROR: Setup failed : Unknown parameter elf_load

There are many projects that use the elf_load plusarg, so I think changing it here is better than every project.

$ grep -r plus.*elf.load *
de0_nano-multicore/bench/orpsoc_tb.v:   if($value$plusargs("elf_load=%s", elf_file)) begin
micron-cores/dram/mt48lc16m16a2_wrapper.v:      if($value$plusargs("elf_load=%s", elf_file)) begin
mor1kx-generic/bench/verilog/orpsoc_tb.v:      if($value$plusargs("elf_load=%s", elf_file)) begin
wb_streamer/build/wb_sdram_ctrl/src/mt48lc16m16a2/mt48lc16m16a2_wrapper.v:      if($value$plusargs("elf_load=%s", elf_file)) begin

stffrdhrn commented 3 years ago

Hi @olofk do you want to have a look at this?

olofk commented 3 years ago

The change is fine. Would you be up for converting it to CAPI2 as well? I'm trying to get rid of the last CAPI1 cores. If you don't have the time and energy for it I can take the fix as is.

Another thing to keep in mind is that we probably want to update the one in fusesoc-cores as well

stffrdhrn commented 3 years ago

Sure I will get around to updating to CAPI2 in a few days.

stffrdhrn commented 3 years ago

I forgot about this one. Will get back to it.

stffrdhrn commented 3 years ago

So, I converted this to CAPI2, it works in iverilog but in verilator I get:

fusesoc run --target mor1kx_tb --tool verilator ::mor1kx-generic:1.1 --elf_load ../../hello.elf 

../src/verilator_tb_utils_0/verilator_tb_utils.cpp:6:10: fatal error: elf-loader.h: No such file or directory
    6 | #include <elf-loader.h>
      |          ^~~~~~~~~~~~~~

Is there something extra to do in verilator? or maybe my version of verilator has a bug?

stffrdhrn commented 3 years ago

ping @olofk

stffrdhrn commented 3 years ago

Pinging @olofk again on this. I'll email if I have to.

olofk commented 3 years ago

Sorry for taking so long. Can you check if this works for you? If it does, just update the PR and I'll get it merged ASAP

CAPI=2:

name : ::elf-loader:1.0.3
description : Generic ELF loader

filesets :
  elf_loader:
    files : [elf-loader.c, elf-loader.h : {is_include_file : true}]
    file_type : cSource

  vpi :
    files : [vpi_wrapper.c]
    file_type : cSource

  check_libelf:
    files: [check_libelf.sh : {file_type: user, copyto: check_libelf.sh}]

vpi:
  elf_loader_vpi:
    filesets: [elf_loader, vpi]
    libs: [elf]

targets:
  default:
    hooks:
      pre_build: [check_libelf]
    filesets: ["tool_verilator? (elf_loader)", check_libelf]
    parameters : [elf_load]
    tools :
      verilator:
        libs : [-lelf]
    vpi: [elf_loader_vpi]

parameters :
  elf_load :
    datatype    : file
    description : ELF file to preload to memory
    paramtype   : plusarg
    scope       : public

scripts:
  check_libelf:
    cmd: [/bin/sh, check_libelf.sh]

stffrdhrn commented 3 years ago

This does seem to fix the compiler issue, however, the plusarg for elf_load doesn't seem to work with verilator. The verilator simulation expects and argument --elf-load which is setup by verilator_tb_utils. Oh, well

Instead I was able to fallback to using --run_options. It seems there is a strange issue where the --run_options are passed 2 times too.

fusesoc --verbose  run --target mor1kx_tb --tool verilator ::mor1kx-generic:1.1 --run_options "--elf-load $PWD/../../hello.elf"

make[1]: Leaving directory '/home/shorne/work/openrisc/fuse-build/mor1kx-generic_1.1/mor1kx_tb-verilator'
INFO: Running
INFO: Running simulation
DEBUG: Running ./Vorpsoc_top
DEBUG: args  : --elf-load /home/shorne/work/openrisc/local-cores/elf-loader/../../hello.elf --elf-load /home/shorne/work/openrisc/local-cores/elf-loader/../../hello.elf
Tracing on
Loading /home/shorne/work/openrisc/local-cores/elf-loader/../../hello.elf
Program header 0: addr 0x00000000, size 0x000067E2
Program header 1: addr 0x000087E4, size 0x00000C7C
Loading /home/shorne/work/openrisc/local-cores/elf-loader/../../hello.elf
Program header 0: addr 0x00000000, size 0x000067E2
Program header 1: addr 0x000087E4, size 0x00000C7C
Success! Got NOP_EXIT. Exiting (28226)
Simulation ended at PC = 0000679c (28227)

stffrdhrn commented 3 years ago

For now, I have disabled elf_load plusarg for verilator as I think that is the right thing to do, verilator doesn't use that plusarg. I think we can define the arguments as needed in verilator_tb_utils.

olofk commented 3 years ago

If we just change the conditional parameter I think this is ready to go in.

Note: I'm cleaning up fusesoc-cores a bit and will remove elf-loader there so you will have to add this repo as a library in the future if you're using elf-loader

stffrdhrn commented 3 years ago

OK, updated now, note also changed to verilator tb which I have a PR for @ https://github.com/fusesoc/fusesoc-cores/pull/8

olofk commented 3 years ago

Awesome. Thanks! Sorry for taking such a long time to fix it

fusesoc / elf-loader

Fixup parameter to match plusargs #2