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.

Sign up for GitHub

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

Jump to bottom

[dv,mem_bkdr_util] Move memory specific code to the corresponding IP #25891

Open
4 tasks
matutem opened this issue Jan 15, 2025 · 1 comment
Open
4 tasks

[dv,mem_bkdr_util] Move memory specific code to the corresponding IP #25891

matutem opened this issue Jan 15, 2025 · 1 comment
Assignees
@rswarbrick @matutem
Labels
Component:DV DV issue: testbench, test case, etc. Component:MultiTop
Milestone
Multi-Top

Comments

@matutem
Copy link
Contributor

matutem commented Jan 15, 2025 •
edited
Loading

Description

The mem_bkdr_util class includes some specific code that is heavily dependent on the IP. For example, mem_kkdr_util.sv includes mem_bkdr_util__flash.sv (and similar files for sram and rom). This is undesirable since it makes the mem_bkdr_util less generic and can cause trouble for code that ends up being top-specific. This was the case for the otp_ctrl code, and it created problems for multi-top.

The solution for otp_ctrl is to move the code to hw/ip_templates/otp_ctrl/dv/env and create a separate core file for it, so chip_level environments can use it. The functions in that code used to be member functions of mem_bkdr_util, but they were simply change to package level functions that take a mem_bkdr_util handle (in this case, the handle to the otp_ctrl instance).

This code needs to be a template since the otp_ctrl is ipgen and the functions can access specific partitions.

It is quite possible the same solution could work for other types of memories.

Some tasks:

Move other memory specific code out of mem_bkdr_util using a similar mechanism:

  • sram_ctrl
  • rom_ctrl
  • flash_ctrl
@matutem matutem added Component:DV DV issue: testbench, test case, etc. Component:MultiTop labels Jan 15, 2025
@matutem matutem added this to the Multi-Top milestone Jan 15, 2025
matutem added a commit to matutem/opentitan that referenced this issue Jan 16, 2025
@matutem
[dv,mem_bkdr_util] Move otp specific code to otp_ctrl
9c24f15 
Change the code to take a handle to the OTP mem_bkdr_util instance,
and use that to perform plain reads and writes to the underlying
memory.

The otp_ctrl code in mem_bkdr_util was making this utility to become
top-specific, which is wrong for a generic utility, and caused build
failures.

Change the uses of the affected functions in top_earlgrey dv code.

Part of lowRISC#25891

Signed-off-by: Guillermo Maturana <[email protected]>
matutem added a commit to matutem/opentitan that referenced this issue Jan 16, 2025
@matutem
[dv,mem_bkdr_util] Move otp specific code to otp_ctrl
31d2685 
Change the code to take a handle to the OTP mem_bkdr_util instance,
and use that to perform plain reads and writes to the underlying
memory.

The otp_ctrl code in mem_bkdr_util was making this utility to become
top-specific, which is wrong for a generic utility, and caused build
failures.

Change the uses of the affected functions in top_earlgrey dv code.

The main change is the move of hw/dv/sv/mem_bkdr_util/mem_bkdr_util__otp.sv
to hw/ip_templates/otp_ctrl/dv/ev/otp_ctrl_mem_bkdr_util_pkg.sv, and
changing the functions to take a mem_bkdr_util handle. For some reason
git didn't detect this as a move.

Part of lowRISC#25891

Signed-off-by: Guillermo Maturana <[email protected]>
matutem added a commit to matutem/opentitan that referenced this issue Jan 16, 2025
@matutem
[dv,mem_bkdr_util] Move otp specific code to otp_ctrl
05ace4b 
Change the code to take a handle to the OTP mem_bkdr_util instance,
and use that to perform plain reads and writes to the underlying
memory.

The otp_ctrl code in mem_bkdr_util was making this utility to become
top-specific, which is wrong for a generic utility, and caused build
failures.

Change the uses of the affected functions in top_earlgrey dv code.

The main change is the move of hw/dv/sv/mem_bkdr_util/mem_bkdr_util__otp.sv
to hw/ip_templates/otp_ctrl/dv/ev/otp_ctrl_mem_bkdr_util_pkg.sv, and
changing the functions to take a mem_bkdr_util handle. For some reason
git didn't detect this as a move.

Part of lowRISC#25891

Signed-off-by: Guillermo Maturana <[email protected]>
@matutem matutem mentioned this issue Jan 16, 2025
Merged
matutem added a commit to matutem/opentitan that referenced this issue Jan 16, 2025
@matutem
[dv,mem_bkdr_util] Move otp specific code to otp_ctrl
79057cd 
Change the code to take a handle to the OTP mem_bkdr_util instance,
and use that to perform plain reads and writes to the underlying
memory.

The otp_ctrl code in mem_bkdr_util was making this utility to become
top-specific, which is wrong for a generic utility, and caused build
failures.

Change the uses of the affected functions in top_earlgrey dv code.

The main change is the move of hw/dv/sv/mem_bkdr_util/mem_bkdr_util__otp.sv
to hw/ip_templates/otp_ctrl/dv/ev/otp_ctrl_mem_bkdr_util_pkg.sv, and
changing the functions to take a mem_bkdr_util handle. For some reason
git didn't detect this as a move.

Part of lowRISC#25891

Signed-off-by: Guillermo Maturana <[email protected]>
@vogelpi vogelpi mentioned this issue Jan 16, 2025
Open
@vogelpi
Copy link
Contributor

vogelpi commented Jan 16, 2025

Thanks @matutem for creating the issue and for working on this. @rswarbrick filed an issue specific for OTP here: #25883. In the meantime, we've also merged a hotfix to unblock things. But your proper fix in #25895 is still required (currently fails CI).

matutem added a commit to matutem/opentitan that referenced this issue Jan 16, 2025
@matutem
[dv,mem_bkdr_util] Move otp specific code to otp_ctrl
94fe0e1 
Change the code to take a handle to the OTP mem_bkdr_util instance,
and use that to perform plain reads and writes to the underlying
memory.

The otp_ctrl code in mem_bkdr_util was making this utility to become
top-specific, which is wrong for a generic utility, and caused build
failures.

Change the uses of the affected functions in top_earlgrey dv code.

The main change is the move of hw/dv/sv/mem_bkdr_util/mem_bkdr_util__otp.sv
to hw/ip_templates/otp_ctrl/dv/ev/otp_ctrl_mem_bkdr_util_pkg.sv, and
changing the functions to take a mem_bkdr_util handle. For some reason
git didn't detect this as a move.

Part of lowRISC#25891

Signed-off-by: Guillermo Maturana <[email protected]>
matutem added a commit to matutem/opentitan that referenced this issue Jan 16, 2025
@matutem
[dv,mem_bkdr_util] Move otp specific code to otp_ctrl
2c6114e 
Change the code to take a handle to the OTP mem_bkdr_util instance,
and use that to perform plain reads and writes to the underlying
memory.

The otp_ctrl code in mem_bkdr_util was making this utility to become
top-specific, which is wrong for a generic utility, and caused build
failures.

Change the uses of the affected functions in top_earlgrey dv code.

The main change is the move of hw/dv/sv/mem_bkdr_util/mem_bkdr_util__otp.sv
to hw/ip_templates/otp_ctrl/dv/ev/otp_ctrl_mem_bkdr_util_pkg.sv, and
changing the functions to take a mem_bkdr_util handle. For some reason
git didn't detect this as a move.

Part of lowRISC#25891

Signed-off-by: Guillermo Maturana <[email protected]>
matutem added a commit that referenced this issue Jan 17, 2025
@matutem
[dv,mem_bkdr_util] Move otp specific code to otp_ctrl
d14ce98 
Change the code to take a handle to the OTP mem_bkdr_util instance,
and use that to perform plain reads and writes to the underlying
memory.

The otp_ctrl code in mem_bkdr_util was making this utility to become
top-specific, which is wrong for a generic utility, and caused build
failures.

Change the uses of the affected functions in top_earlgrey dv code.

The main change is the move of hw/dv/sv/mem_bkdr_util/mem_bkdr_util__otp.sv
to hw/ip_templates/otp_ctrl/dv/ev/otp_ctrl_mem_bkdr_util_pkg.sv, and
changing the functions to take a mem_bkdr_util handle. For some reason
git didn't detect this as a move.

Part of #25891

Signed-off-by: Guillermo Maturana <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rswarbrick rswarbrick

@matutem matutem

Labels
Component:DV DV issue: testbench, test case, etc. Component:MultiTop
Projects
None yet
Milestone
Multi-Top
Development

No branches or pull requests
3 participants
@rswarbrick @vogelpi @matutem