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.61k stars 228 forks source link

TRNG needs to be enabled in SytemTop_axi4lite #227

Closed AWenzel83 closed 2 years ago

AWenzel83 commented 3 years ago

I started a new design today and Vivado won't let me insert the neorv32_SystemTop_axi4lite-template into a block design.

I could track the issue down to the new TRNG-Module: when not enabled there is a problem with the neoTRNG_cell shown in the source-view .

When enabled, the problem disappears, and I can insert the module into the block design. After that I can disable it again, without any problems. Thats why an older project still worked, although I updated the processor.

stnolting commented 3 years ago

Just so I understand correctly: There is a problem with the TRNG when it is not enabled? So Vivado shows a problem even before synthesis?

AWenzel83 commented 3 years ago

Exactly,

TRNG_problem

when not activated there is this "Question-mark"-Symbol.

Since this is in an inactive module, synthesis works. It just won't let me insert the module into a block design.

stnolting commented 3 years ago

I can reproduce this behavior for simple file-based synthesis (no block design). However, Vivado does not complain.

grafik

The TRNG is the only thing that has three in-file entities. Seems like Vivado has problems with the last (lowest) entity level...

At least the question mark symbol vanishes when I comment the instantiation of the lowest TRNG entity. https://github.com/stnolting/neorv32/blob/602b471a384c04671094fab9980b6c385265faa5/rtl/core/neorv32_trng.vhd#L308-L322

It just won't let me insert the module into a block design.

So you cannot synthesize the block design when the TRNG is enabled?

AWenzel83 commented 2 years ago

I now know, that the questionmark means, the source is not found, but when I set the default value of IO_TRNG_EN to true (line 106 in the axi4-lite-template-file), Vivado finds the source for the neoTRNG_cell, and the questionmark disappears:

TRNG_enabled )

Now I can insert the template into a block Design, and disable the TRNG again: TRNG_Blockdesign

everything after that works, synthesis, implementation and so on

stnolting commented 2 years ago

everything after that works, synthesis, implementation and so on

So this is more like a Vivado "feature" that requires an inconvenient GUI work-around? Block design-based synthesis with TRNG enabled/disabled works, right?

AWenzel83 commented 2 years ago

Yes, just another "feature"...

As I said, everything after that works.

stnolting commented 2 years ago

Do you think we should add a note to the according User Guide section regarding this feature?

AWenzel83 commented 2 years ago

Yes, I think that should be ok, like the note regarding the Tri-State Drivers.

... the Block-designer is really a thing on its own... But when working with AXI-IPs its the most convenient way.

... or maybe a "pitfalls"-Section. During the update to the newest core, the on-chip-debugger stopped working for me, and I had to dig a bit to find the note that the Zifencei-extension has to be activated for the debug-mode, which wasn't the case when I last started a new design. At first I thought of a hardware problem, since it worked before...

stnolting commented 2 years ago

Yes, I think that should be ok, like the note regarding the Tri-State Drivers.

Any proposals? :wink:

During the update to the newest core, the on-chip-debugger stopped working for me, and I had to dig a bit to find the note that the Zifencei-extension has to be activated for the debug-mode, which wasn't the case when I last started a new design. At first I thought of a hardware problem, since it worked before...

That also broke some of my Vivado designs... 😅 That's why I have added "Sanity checks" to the VHDL code (using assert ... report) like this: https://github.com/stnolting/neorv32/blob/602b471a384c04671094fab9980b6c385265faa5/rtl/core/neorv32_cpu.vhd#L229 However, Vivado conveniently ignores that...

At least Xilinx is aware of this: https://support.xilinx.com/s/question/0D52E00006hpgpDSAQ/bug-report-vivado-vhdl-assertion-still-broken?language=en_US

AWenzel83 commented 2 years ago

But Vivado conveniently ignores that...

would it be an option to 'or' both 'CPU_EXTENSION_RISCV_Zifencei' and 'ON_CHIP_DEBUGGER_EN' inside the CPU-top-module to either implement the extension when explicitly activated by 'CPU_EXTENSION_RISCV_Zifencei := true' or inderectly when needed if 'ON_CHIP_DEBUGGER_EN := true'

Any proposals?

let me think about that

AWenzel83 commented 2 years ago

However, Vivado conveniently ignores that...

At least for simulations you can choose to stop at an error

stnolting commented 2 years ago

would it be an option to 'or' both [...]

Sure, but I like the "you get what you see" concept - so only the actually enabled extensions do get synthesized. Furthermore, some extension combinations have side effect on other extensions and there even are extension combinations that are illegal... I think the current approach using those sanity checks is the best trade-off here.

AWenzel83 commented 2 years ago

Do you think we should add a note to the according User Guide section regarding this feature?

the more I think about that, the more I think the TRNG should be activated in the AXI4 template from the beginning, since AXI is mainly a Xilinx/Arm-thing, isn't it. So most users using the AXI-template would run into this issue.

That doesn't mean there shouldn't be a note about that somewhere...

AWenzel83 commented 2 years ago

I think the current approach using those sanity checks is the best trade-off here

thats true

stnolting commented 2 years ago

the more I think about that, the more I think the TRNG should be activated in the AXI4 template from the beginning, since AXI is mainly a Xilinx/Arm-thing, isn't it. So most users using the AXI-template would run into this issue.

Good idea. I'm OK with that. :+1:

That doesn't mean there shouldn't be a note about that somewhere...

True. I cannot think about a good wording...

AWenzel83 commented 2 years ago

You could include the following note (right below the one regarding TWI Tri-State-Drivers):

TRNG For the neorv32_top_axi4lite-template the TRNG-Periphal is activated by default. Otherwise Vivado cannot insert this module into a block design.

If you don't need the TRNG, you can disable it by double-clicking on the module's block and deselecting 'Io Trng En', after inserting the module.

or something like this...

stnolting commented 2 years ago

Sounds good!