Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External Registers Issue #90

Open
davidp135 opened this issue Nov 11, 2021 · 18 comments
Open

External Registers Issue #90

davidp135 opened this issue Nov 11, 2021 · 18 comments

Comments

@davidp135
Copy link

Hi Taichi,

I am looking into Rggen, it looks great! Thank you for spending the time on creating these repositories. Unfortunately I am having issues with creating external register interfaces to connect RAMs to the register maps. I am using Questa so have also been testing the compatibility as I see you have raised ticket #79.

Firstly working with Rggen without external registers:
I can get the simulations to work with with 1 file change. Using Rggen as it is I get the following error:
-- Compiling package rggen_ral_backdoor_pkg ** Error: ..../rggen-sample-testbench/ral/rggen-sv-ral/rggen_ral_backdoor_pkg.sv(72): (vlog-2163) Macro `uvm_fatal is undefined.

I can avoid this error by going to ral/rggen-sv-ral/rggen_ral_backdoor_pkg.sv and going to line 72 andcommenting out:
`uvm_fatal( "RegModel", $sformatf( "Casting failed, '%s' is neither uvm_reg nor uvm_reg_field", element.get_full_name() ) ) `uvm_fatal( "RegModel", $sformatf("Casting failed, '%s' is neither uvm_reg nor uvm_reg_field", element.get_full_name()))
I am not sure of another way around this or why I am getting an error unfortunately.

Secondly working with external registers
The issue here I don't think is tool related as I have got your sample test bench to start simulating which does have external registers. The test bench that I have written I get the following errors:
** Error: (vsim-8268) No Default value for formal 'has_coverage' in task/function new. Time: 0 ps Iteration: 0 Region: /test_register_block_ral_pkg::test_register_block_block_model #(rggen_ral_pkg::rggen_ral_block, 1) File: ../tb/registers/test_register_block_ral_pkg.sv Line: 37
and
** Error: (vsim-8268) No Default value for formal 'n_bytes' in task/function new. Time: 0 ps Iteration: 0 Region: /test_register_block_ral_pkg::test_register_block_block_model #(rggen_ral_pkg::rggen_ral_block, 1) File: ../tb/registers/test_register_block_ral_pkg.sv Line: 37

Line 37 there is where the external register is created using `rggen_ral_create_block Going back through the new functions in the class structure it looks like I am not setting the has_coverage and n_bytes values. I can't see how this is done on your sample test bench.

I have spent quite a long time trying to work this out with no success, so I thought I would ask for your help if that is ok? Have you an idea on where I could be going wrong? I have attached a cut-down sample test bench where I get this error if that is useful.

Thanks

David
rggen_example.zip

@taichi-ishitani
Copy link
Member

taichi-ishitani commented Nov 12, 2021

Hi @davidp135 ,
Thank you for using RgGen and opening this issue!

For first error:
I think we can avoid this error by changing rggen_ral_backdoor_pkg.sv file like below.

`ifndef RGGEN_RAL_BACKDOOR_PKG_SV
`define RGGEN_RAL_BACKDOOR_PKG_SV
package rggen_ral_backdoor_pkg;
  import  uvm_pkg::*;
  `include "uvm_macros.svh" // add

`ifdef RGGEN_ENABLE_BACKDOOR

Can you confirm if the above change works well?

For second error:
The rggen_ral_block class which is default value of the TEST_RAM_INTERFACE parameter is just a base class of block models generated by RgGen and you need to set an actual register block model to the parameter.
For example:
https://github.com/rggen/rggen-sample-testbench/blob/0971e14a44c1845dac340218e9fcea4faecac16c/env/env_ral_model.svh#L2

@davidp135
Copy link
Author

Hi,

The first error is fixed. Thanks!

With the external registers, are you saying that another register block/file has to be passed into an external register?
I want to do something like this:
image

Basically to reserve a bunch of register addresses and store their contents in RAM rather than discrete registers to save space on the FPGA.

Is there another way to extend that class with the has_coverage and n_bytes?

@taichi-ishitani
Copy link
Member

The first error is fixed. Thanks!

Thank you for your confirmation. I will update the rggen_ral_backdoor_pkg.sv file.

I want to do something like this:

To do this, you need to RAM model by extending uvm_reg_block or rggen_ral_block class.

taichi-ishitani added a commit to rggen/rggen-sv-ral that referenced this issue Nov 12, 2021
@davidp135
Copy link
Author

Ok thanks, Il give it a go.

@taichi-ishitani
Copy link
Member

https://github.com/rggen/rggen-sample-testbench

This is a sample TB to demonstrate integration of RAM model generated by RgGen.
Could please you add a makefile for Questa?

@taichi-ishitani
Copy link
Member

Thank you for your confirmation. I will update the rggen_ral_backdoor_pkg.sv file.

I've updated.
rggen/rggen-sv-ral@cba385d

Can you try to use the latest version?

@davidp135
Copy link
Author

I can confirm rggen/rggen-sv-ral@cba385d works.

I have made a Questa makefile (attached) but am unfortunately getting some errors with the uvm_reg_predictor:

# ** Error: (vsim-3043) Unresolved reference to 'BUSTYPE' in BUSTYPE::type_id::create.
#    Time: 0 ps  Iteration: 0  Region: /uvm_pkg::uvm_reg_predictor #(int) File: verilog_src/uvm-1.2/src/reg/uvm_reg_predictor.svh Line: 120
# ** Error: (vsim-3701) 'get_type_name' is not a field or method in 't'.
#    Time: 0 ps  Iteration: 0  Region: /uvm_pkg::uvm_reg_predictor #(int) File: verilog_src/uvm-1.2/src/reg/uvm_reg_predictor.svh Line: 121
# ** Error: (vsim-3044) Usage of 'get_type_name' inconsistent with 'dynamic type' object.
#    Time: 0 ps  Iteration: 0  Region: /uvm_pkg::uvm_reg_predictor #(int) File: verilog_src/uvm-1.2/src/reg/uvm_reg_predictor.svh Line: 121
# ** Error: (vsim-3701) 'get_type_name' is not a field or method in 't'.
#    Time: 0 ps  Iteration: 0  Region: /uvm_pkg::uvm_reg_predictor #(int) File: .verilog_src/uvm-1.2/src/reg/uvm_reg_predictor.svh Line: 121
# ** Error: (vsim-3701) '$$' is not a field or method in 'get_type_name'.
#    Time: 0 ps  Iteration: 0  Region: /uvm_pkg::uvm_reg_predictor #(int) File: verilog_src/uvm-1.2/src/reg/uvm_reg_predictor.svh Line: 121

Im not sure what the issue is but am looking into it. Let me know if you have any ideas
questa.zip
.

@taichi-ishitani
Copy link
Member

am unfortunately getting some errors with the uvm_reg_predictor

Can you apply the following patch to env/tue/src/reg/tue_reg_predictor.svh and try again?

diff --git a/src/reg/tue_reg_predictor.svh b/src/reg/tue_reg_predictor.svh
index a4bf8bb..8f32e34 100644
--- a/src/reg/tue_reg_predictor.svh
+++ b/src/reg/tue_reg_predictor.svh
@@ -15,7 +15,14 @@
 //------------------------------------------------------------------------------
 `ifndef TUE_REG_PREDICTOR_SVH
 `define TUE_REG_PREDICTOR_SVH
-class tue_reg_predictor #(type BUSTYPE = int) extends uvm_reg_predictor #(BUSTYPE);
+class tue_reg_bustype_dummy extends uvm_sequence_item;
+  `tue_object_default_constructor(tue_reg_bustype_dummy)
+  `uvm_object_utils(tue_reg_bustype_dummy)
+endclass
+
+class tue_reg_predictor #(
+  type  BUSTYPE = tue_reg_bustype_dummy
+) extends uvm_reg_predictor #(BUSTYPE);
   function new(string name = "tue_reg_predictor", uvm_component parent = null);
     super.new(name, parent);
   endfunction

@davidp135
Copy link
Author

Hi, unfortunately that gives the same error as above.

@taichi-ishitani
Copy link
Member

Thank you for your trial.
Can you apply the additional patch below to env/env_ral_monitor.svh b/env/env_ral_monitor.svh?

diff --git a/env/env_ral_monitor.svh b/env/env_ral_monitor.svh
index ad47683..8a2cf15 100644
--- a/env/env_ral_monitor.svh
+++ b/env/env_ral_monitor.svh
@@ -1,7 +1,7 @@
 class env_ral_monitor #(
   type  ITEM      = uvm_sequence_item,
   type  ADAPTER   = uvm_reg_adapter,
-  type  PREDICTOR = uvm_reg_predictor
+  type  PREDICTOR = uvm_reg_predictor #(uvm_reg_item)
 ) extends tue_component #(
   .CONFIGURATION  (env_configuration  )
 );

@davidp135
Copy link
Author

Great! I think it is working. Also I don't need the tue_reg_bustype_dummy patch

image

@davidp135
Copy link
Author

That was with apb, verilog. Il cycle through the other options now

taichi-ishitani added a commit to rggen/rggen-sample-testbench that referenced this issue Nov 15, 2021
@taichi-ishitani
Copy link
Member

Thank you for confirmation.
I've committed the workaround.
rggen/rggen-sample-testbench@a4cae57

Can you open a new PR to add makefile for Questa?
https://github.com/rggen/rggen-sample-testbench/pulls

@davidp135
Copy link
Author

Sure, I just need to work out the VHDL side of it.

@davidp135
Copy link
Author

I have had made a feature branch with the questa changes (feature/questa_support) but I don't have permission to write it to remote. Could you give me the permissions? Otherwise is there another way to do this?

ERROR: Permission to rggen/rggen-sample-testbench.git denied to davidp135.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

@taichi-ishitani
Copy link
Member

taichi-ishitani commented Nov 17, 2021

Thank you for your help for Questa support!
Could you please do:

  1. Fork the repository
  2. Commit your change to your forked repository
  3. Create a new PR

@davidp135
Copy link
Author

Sure ok.

Test 1 passes for VHDL, but I am having issues with the other two.

They both say they pass in the UVM Report catcher Summary but actually have UVM_ERRORs above.

# --- UVM Report catcher Summary ---
# 
# 
# Number of demoted UVM_FATAL reports  :    0
# Number of demoted UVM_ERROR reports  :    0
# Number of demoted UVM_WARNING reports:    0
# Number of caught UVM_FATAL reports   :    0
# Number of caught UVM_ERROR reports   :    0
# Number of caught UVM_WARNING reports :    0

ral_bit_bash test fails on what looks like all the reg_single_bit bash_seq tests for example:

# UVM_ERROR verilog_src/uvm-1.1d/src/reg/sequences/uvm_reg_bit_bash_seq.svh(174) @ 22079500: uvm_test_top.env.sequencer@@<unknown>.ral_sequence.reg_single_bit_bash_seq [uvm_reg_bit_bash_seq] Writing a 1 in bit #23 of register "model.register_14.register_file_2[1].register_file_0.register_0[1][2]" with initial value 'h0000000000000000 yielded 'h0000000000000000 instead of 'h0000000000800000

ral_access_test fails on the tue_reg_access_seq for what looks like all registers:

# UVM_INFO /home/david/Repositories/rggen_vhdl/rggen-sample-testbench/env/tue/src/reg/sequences/tue_reg_access_seq.svh(102) @ 715500: uvm_test_top.env.sequencer@@<unknown>.ral_sequence.single_reg_access_seq [tue_reg_single_access_seq] Verifying access of register 'model.register_14.register_file_2[1].register_file_0.register_0[1][2]' in map 'model.register_14.register_file_2[1].register_file_0.default_map' ...
# UVM_ERROR verilog_src/uvm-1.1d/src/reg/uvm_reg.svh(2889) @ 718500: reporter [RegModel] Register "model.register_14.register_file_2[1].register_file_0.register_0[1][2]" value read from DUT (0x0000000000000000) does not match mirrored value (0x0000000000ff00ff)
# UVM_INFO verilog_src/uvm-1.1d/src/reg/uvm_reg.svh(2902) @ 718500: reporter [RegModel] Field bit_field_0[0] (model.register_14.register_file_2[1].register_file_0.register_0[1][2][3:0]) mismatch read=4'h0 mirrored=4'hf 
# UVM_INFO verilog_src/uvm-1.1d/src/reg/uvm_reg.svh(2902) @ 718500: reporter [RegModel] Field bit_field_0[1] (model.register_14.register_file_2[1].register_file_0.register_0[1][2][7:4]) mismatch read=4'h0 mirrored=4'hf 
# UVM_INFO verilog_src/uvm-1.1d/src/reg/uvm_reg.svh(2902) @ 718500: reporter [RegModel] Field bit_field_2[0] (model.register_14.register_file_2[1].register_file_0.register_0[1][2][19:16]) mismatch read=4'h0 mirrored=4'hf 
# UVM_INFO verilog_src/uvm-1.1d/src/reg/uvm_reg.svh(2902) @ 718500: reporter [RegModel] Field bit_field_2[1] (model.register_14.register_file_2[1].register_file_0.register_0[1][2][23:20]) mismatch read=4'h0 mirrored=4'hf 

I assume that you don't have these failures?

Also VHDL block 0 was not compatible with sv apb_top.sv and axi4_lite_top.sv for registers 9 and 10. I assume this is because the VHDL output vector can not be passed directly into a multidimensional packed array. I added a conversion from logic vector to packed arrays in the _top.sv files. Eg:

logic [3:0][3:0][3:0]       register_9_bit_field_0;
logic [63:0]                register_9_bit_field_0_vector;
...
.o_register_9_bit_field_0           (register_9_bit_field_0_vector    ),
...
assign  register_9_bit_field_0        = register_9_bit_field_0_vector;

Is this acceptable to you, or would you rather leave it out?

@taichi-ishitani
Copy link
Member

taichi-ishitani commented Nov 18, 2021

They both say they pass in the UVM Report catcher Summary but actually have UVM_ERRORs above.

In the case of VCS, all tests are passed.
Can you put log files?

Are these errors raised only when DUT is VHDL?

Is this acceptable to you, or would you rather leave it out?

Thank you for your feedback!
Output signals from register 9/10 are not used so I'd prefer to change definition of these signals from packed array to vector.

logic [63:0] register_9_bit_field_0;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants