stnolting / neorv32

:desktop_computer: A small, customizable and extensible MCU-class 32-bit RISC-V soft-core CPU and microcontroller-like SoC written in platform-independent VHDL.
https://neorv32.org
BSD 3-Clause "New" or "Revised" License
1.55k stars 215 forks source link

Multi-driven Pin Vivado Error: what causes it and how to solve it? #655

Closed Luperior closed 1 year ago

Luperior commented 1 year ago

Hello everyone, I am currently working on a multicore version of the NEORV32 processor. I am using Vivado 2022.2 to generate the bitstream for the XILINX PYNQ-Z2 FPGA and the last stable version of this processor (v 1.8.7). My approach consists in replicating the core instantiation, and consequently their sub-components, along with those that I need for each core, in the neorv32_top.vhd file. Everything seems to work fine but then the Implemented Design is shown empty in Vivado and in fact I do not achieve the desired behaviour. It looks like everything is due to these critical warnings (repeated multiple times, all referring to the same code line) since only duplicating some elements triggers them and when they're not triggered I have zero issues with my results. I would also like to point out that each component has its own signals and there are no superpositions, except when needed. Could you help me figure out what may be causing them, and/or how to solve them? Or even where to look or ask for help? I can't seem to find anything on Reddit, Xilinx Support, ChatGPT and so on... the only thing they mention is a Netlist function in Vivado Synthesized Design but I can't get deep enough in my design to analyze these signals (https://support.xilinx.com/s/question/0D52E00006hplltSAA/synth-86859-multi-driven-net-on-pin-output-error?language=en_US). Thank you in advance!

MultiDrivenPin_GitHub

stnolting commented 1 year ago

Strange. I have never see this specific warning before. However, I do not think this is a problem with the or_reduce_f function itself. Maybe the function is just being used in the wrong way somewhere...

Did you made any modifications to the rtl code base? What processor configuration are you using (generics)?

Luperior commented 1 year ago

Since I am using the _neorv32_test_setupbootloader the generic processor configuration is the one provided, with the exception of IMEM and DMEM sizes changed according to my needs (in the linker file too) and the _INT_BOOTLOADEREN set to false to hardcode the application in the bitstream.
When it comes to the rtl files the only changes I made were duplicating the Core Complex and Memory System sections, making sure that each component had its own name and signals (except for the clock and reset). However what triggers these critical warnings is only duplicating either the core itself (neorv32_cpu) or the busswitch (neorv32_busswitch) or both, but I can't figure out why.

stnolting commented 1 year ago

IMEM and DMEM sizes changed

What sizes did you configure?

When it comes to the rtl files the only changes I made were duplicating the Core Complex and Memory System sections

You also duplicated the memory system? Could you show your modified rtl file(s)?

Luperior commented 1 year ago

What sizes did you configure?

IMEM is set at 512K, whereas DMEM at 64K.

You also duplicated the memory system? Could you show your modified rtl file(s)?

This is the part of the code I modified the most, the others are just copies of components like IMEM, DMEM, busswitch, bootloaders, caches etc... it is also what triggers the warnings. HART_ID, firq_i and dbi_i changes are inspired by this neorv32 multicore project (https://github.com/NikLeberg/neorv32_soc) which you recommended me in this open discussion (https://github.com/stnolting/neorv32/discussions/644#discussioncomment-6553225). These changes though are applied only to this second core, to both start with a simpler project and to keep the first core as main (for now).

neorv32_cpu_2_inst: entity neorv32.neorv32_cpu
    generic map (
      -- General --
      HART_ID                      => STD_ULOGIC_VECTOR(to_unsigned(2, 32)),   -- each core has its own ID 
      VENDOR_ID                    => VENDOR_ID,
      CPU_BOOT_ADDR                => cpu_boot_addr_c,
      CPU_DEBUG_PARK_ADDR          => dm_park_entry_c,
      CPU_DEBUG_EXC_ADDR           => dm_exc_entry_c,
      -- RISC-V CPU Extensions --
      CPU_EXTENSION_RISCV_A        => CPU_EXTENSION_RISCV_A,
      CPU_EXTENSION_RISCV_B        => CPU_EXTENSION_RISCV_B,
      CPU_EXTENSION_RISCV_C        => CPU_EXTENSION_RISCV_C,
      CPU_EXTENSION_RISCV_E        => CPU_EXTENSION_RISCV_E,
      CPU_EXTENSION_RISCV_M        => CPU_EXTENSION_RISCV_M,
      CPU_EXTENSION_RISCV_U        => CPU_EXTENSION_RISCV_U,
      CPU_EXTENSION_RISCV_Zfinx    => CPU_EXTENSION_RISCV_Zfinx,
      CPU_EXTENSION_RISCV_Zicntr   => CPU_EXTENSION_RISCV_Zicntr,
      CPU_EXTENSION_RISCV_Zicond   => CPU_EXTENSION_RISCV_Zicond,
      CPU_EXTENSION_RISCV_Zihpm    => CPU_EXTENSION_RISCV_Zihpm,
      CPU_EXTENSION_RISCV_Zifencei => CPU_EXTENSION_RISCV_Zifencei,
      CPU_EXTENSION_RISCV_Zmmul    => CPU_EXTENSION_RISCV_Zmmul,
      CPU_EXTENSION_RISCV_Zxcfu    => CPU_EXTENSION_RISCV_Zxcfu,
      CPU_EXTENSION_RISCV_Sdext    => ON_CHIP_DEBUGGER_EN,
      CPU_EXTENSION_RISCV_Sdtrig   => ON_CHIP_DEBUGGER_EN,
      -- Extension Options --
      FAST_MUL_EN                  => FAST_MUL_EN,
      FAST_SHIFT_EN                => FAST_SHIFT_EN,
      CPU_IPB_ENTRIES              => CPU_IPB_ENTRIES,
      -- Physical Memory Protection (PMP) --
      PMP_NUM_REGIONS              => PMP_NUM_REGIONS,
      PMP_MIN_GRANULARITY          => PMP_MIN_GRANULARITY,
      -- Hardware Performance Monitors (HPM) --
      HPM_NUM_CNTS                 => HPM_NUM_CNTS,
      HPM_CNT_WIDTH                => HPM_CNT_WIDTH
    )
    port map (
      -- global control --
      clk_i      => clk_i,
      rstn_i     => rstn_int,
      sleep_o    => cpu_sleep2,
      debug_o    => cpu_debug2,
      ifence_o   => i_fence2,
      dfence_o   => d_fence2

      -- interrupts --
      msi_i      => msw_irq_i2,
      mei_i      => mext_irq_i2,
      mti_i      => mtime_irq2,
      firq_i     => (OTHERS => '0'),
      dbi_i      => '0',

      -- instruction bus interface --
      ibus_req_o => cpu_i_req2,
      ibus_rsp_i => cpu_i_rsp2,
      -- data bus interface --
      dbus_req_o => cpu_d_req2,
      dbus_rsp_i => cpu_d_rsp2
    );

    -- advanced memory control --
    fence_o2  <= d_fence2;
    fencei_o2 <= i_fence2;
stnolting commented 1 year ago

IMEM is set at 512K, whereas DMEM at 64K.

Wow, that's a lot! What FPGA do you use??

This is the part of the code I modified the most, the others are just copies of components like IMEM, DMEM, busswitch, bootloaders, caches etc... it is also what triggers the warnings.

Hmm, that is hard to analyze from here... If you use no caches at all does the problem remain? I think it might be a problem with the bus MUX... Did you set PORT_*_READ_ONLY accordingly (read-only for the I interfaces, read/write for the D interfaces)?

Luperior commented 1 year ago

Wow, that's a lot! What FPGA do you use??

I am working on a XILINX PYNQ Z2, do you think that lowering these values could solve the problem?

If you use no caches at all does the problem remain?

Yes, I tried disabling caches but noticed no improvements sadly.

Did you set PORT_*_READ_ONLY accordingly (read-only for the I interfaces, read/write for the D interfaces)?

Yes I did since I did not change the generic parameters for the busswitch.

stnolting commented 1 year ago

I am working on a XILINX PYNQ Z2, do you think that lowering these values could solve the problem?

According to the board's product page the FPGA has 630kB of blockRAM. If you have two cores with 512kB each then synthesis should fail as you are running out of resources?!?! 🤔

Luperior commented 1 year ago

According to the board's product page the FPGA has 630kB of blockRAM. If you have two cores with 512kB each then synthesis should fail as you are running out of resources?!?! 🤔

I can't believe I missed this, I guess I got so distracted by the error description that I didn't check the basis. Thank you so much for noticing that, I have already taken care of it. The issue however doesn't seem to disappear, so I guess something else must be causing it. Where else could I investigate?

stnolting commented 1 year ago

The issue however doesn't seem to disappear, so I guess something else must be causing it. Where else could I investigate?

That is a very good question... Can you pack the entire project and make it public somewhere so I/we can test it locally?

Luperior commented 1 year ago

Can you pack the entire project and make it public somewhere so I/we can test it locally?

This is the link to the Dropbox folder (https://www.dropbox.com/scl/fo/dm87qxhbtzp8o1lwwo5lo/h?rlkey=e8gevk8e9u047ezshsodnq3xv&dl=0) to download 1) my modified version of the neorv32_top.vhd file which contains all my main changes, 2) the application code to be written in the IMEM (a simple 3X3 integer matrix multiplication app that prints out the results) 3) the bootloader test setup and last but not least 4) the linker file tuned according to the memory size. Hopefully it should be enough, I really can't thank you enough for your kindness and helpfulness.

stnolting commented 1 year ago

I had a quick look at your sources (neorv32_top.vhd).

  1. Your second core complex isn't connected to a bus system. core_req2 and core_rsp2 are dangling.
  2. As far as I can see you are not using the caches, right? ICACHE_EN and DCACHE_EN are false by default. In this case no cache modules are instantiated and the neorv32_*cache_*_inst_false sections are implemented. But that means that both core complexes are wired to the same bus lines. Synthesis cannot resolve that (and should issue a warning). Here are the failing assignments:
    neorv32_icache_inst_false:
    if (ICACHE_EN = false) generate
      icache_req <= cpu_i_req;
      cpu_i_rsp  <= icache_rsp;
    end generate;

...

    neorv32_icache_2_inst_false:
    if (ICACHE_EN = false) generate
      icache_req <= cpu_i_req2;
      cpu_i_rsp  <= icache_rsp2;
    end generate;
    neorv32_dcache_inst_false:
    if (DCACHE_EN = false) generate
      dcache_req <= cpu_d_req;
      cpu_d_rsp  <= dcache_rsp;
    end generate;

...

    neorv32_dcache_2_inst_false:
    if (DCACHE_EN = false) generate
      dcache_req <= cpu_d_req2;
      cpu_d_rsp  <= dcache_rsp2;
    end generate;

icache_req and cpu_i_rsp as well as dcache_req and cpu_d_rsp are multi-driven signals.

Luperior commented 1 year ago

As far as I can see you are not using the caches, right? ICACHE_EN and DCACHE_EN are false by default. In this case no cache modules are instantiated and the neorv32_*cache_*_inst_false sections are implemented. But that means that both core complexes are wired to the same bus lines.

Thank you, you are absolutely right. By enabling both caches I fixed both this Vivado warning and the empty implemented design.

Your second core complex isn't connected to a bus system. core_req2 and core_rsp2 are dangling.

Then enabling the caches should have solved this too I guess, since it looks like everything is working fine right now. Thank you so much, you really saved me.