Skip to content

Commit

Permalink
Use vendored-in primitives from OpenTitan
Browse files Browse the repository at this point in the history
Instead of using copies of primitives from OpenTitan, vendor the files
in directly from OpenTitan, and use them.

Benefits:

- Less potential for diverging code between OpenTitan and Ibex, causing
  problems when importing Ibex into OT.

- Use of the abstract primitives instead of the generic ones. The
  abstract primitives are replaced during synthesis time with
  target-dependent implementations. For simulation, nothing changes. For
  synthesis for a given target technology (e.g. a specific ASIC or FPGA
  technology), the primitives system can be instructed to choose
  optimized versions (if available).

  This is most relevant for the icache, which hard-coded the generic
  SRAM primitive before. This primitive is always implemented as
  registers. By using the abstract primitive (prim_ram_1p) instead, the
  RAMs can be replaced with memory-compiler-generated ones if necessary.

There are no real draw-backs, but a couple points to be aware of:

- Our ram_1p and ram_2p implementations are kept as wrapper around the
  primitives, since their interface deviates slightly from the one in
  prim_ram*. This also includes a rather unfortunate naming confusion
  around rvalid, which means "read data valid" in the OpenTitan advanced
  RAM primitives (prim_ram_1p_adv for example), but means "ack" in
  PULP-derived IP and in our bus implementation.

- The core_ibex UVM DV doesn't use FuseSoC to generate its file list,
  but uses a hard-coded list in `ibex_files.f` instead. Since the
  dynamic primitives system requires the use of FuseSoC we need to
  provide a stop-gap until this file is removed. Issue lowRISC#893 tracks
  progress on that.

- Dynamic primitives depend no a not-yet-merged feature of FuseSoC
  (olofk/fusesoc#391). We depend on the same
  functionality in OpenTitan and have instructed users to use a patched
  branch of FuseSoC for a long time through `python-requirements.txt`,
  so no action is needed for users which are either successfully
  interacting with the OpenTitan source code, or have followed our
  instructions. All other users will see a reasonably descriptive error
  message during a FuseSoC run.

- This commit is massive, but there are no good ways to split it into
  bisectable, yet small, chunks. I'm sorry. Reviewers can safely ignore
  all code in `vendor/lowrisc_ip`, it's an import from OpenTitan.

- The check_tool_requirements tooling isn't easily vendor-able from
  OpenTitan at the moment. I've filed
  lowRISC/opentitan#2309 to get that sorted.

- The LFSR primitive doesn't have a own core file, forcing us to include
  the catch-all `lowrisc:prim:all` core. I've filed
  lowRISC/opentitan#2310 to get that sorted.
  • Loading branch information
imphil committed May 27, 2020
1 parent 3f4e706 commit 8b42024
Show file tree
Hide file tree
Showing 190 changed files with 11,808 additions and 252 deletions.
2 changes: 1 addition & 1 deletion check_tool_requirements.core
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ CAPI=2:
# Copyright lowRISC contributors.
# Licensed under the Apache License, Version 2.0, see LICENSE for details.
# SPDX-License-Identifier: Apache-2.0
name: "lowrisc:ibex:check_tool_requirements:0.1"
name: "lowrisc:tool:check_tool_requirements:0.1"
description: "Check tool requirements"

filesets:
Expand Down
4 changes: 3 additions & 1 deletion dv/riscv_compliance/ibex_riscv_compliance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ int main(int argc, char **argv) {
simctrl.SetTop(&top, &top.IO_CLK, &top.IO_RST_N,
VerilatorSimCtrlFlags::ResetPolarityNegative);

memutil.RegisterMemoryArea("ram", "TOP.ibex_riscv_compliance.u_ram.u_ram");
memutil.RegisterMemoryArea(
"ram",
"TOP.ibex_riscv_compliance.u_ram.u_ram.gen_generic.u_impl_generic");
simctrl.RegisterExtension(&memutil);

return simctrl.Exec(argc, argv);
Expand Down
35 changes: 35 additions & 0 deletions dv/uvm/core_ibex/common/prim/prim_clock_gating.sv
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright lowRISC contributors.
// Licensed under the Apache License, Version 2.0, see LICENSE for details.
// SPDX-License-Identifier: Apache-2.0

// Abstract primitives wrapper.
//
// This file is a stop-gap until the DV file list is generated by FuseSoC.
// Its contents are taken from the file which would be generated by FuseSoC.
// https://github.com/lowRISC/ibex/issues/893

`ifndef PRIM_DEFAULT_IMPL
`define PRIM_DEFAULT_IMPL prim_pkg::ImplGeneric
`endif

module prim_clock_gating (
input clk_i,
input en_i,
input test_en_i,
output logic clk_o
);
parameter prim_pkg::impl_e Impl = `PRIM_DEFAULT_IMPL;

if (Impl == prim_pkg::ImplGeneric) begin : gen_generic
prim_generic_clock_gating u_impl_generic (
.*
);
end else if (Impl == prim_pkg::ImplXilinx) begin : gen_xilinx
prim_xilinx_clock_gating u_impl_xilinx (
.*
);
end else begin : gen_failure
// TODO: Find code that works across tools and causes a compile failure
end

endmodule
18 changes: 18 additions & 0 deletions dv/uvm/core_ibex/common/prim/prim_pkg.sv
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright lowRISC contributors.
// Licensed under the Apache License, Version 2.0, see LICENSE for details.
// SPDX-License-Identifier: Apache-2.0
//
// Constants for use in primitives
//
// This file is a stop-gap until the DV file list is generated by FuseSoC.
// Its contents are taken from the file which would be generated by FuseSoC.
// https://github.com/lowRISC/ibex/issues/893

package prim_pkg;

// Implementation target specialization
typedef enum integer {
ImplGeneric,
ImplXilinx
} impl_e;
endpackage : prim_pkg
40 changes: 40 additions & 0 deletions dv/uvm/core_ibex/common/prim/prim_ram_1p.sv
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright lowRISC contributors.
// Licensed under the Apache License, Version 2.0, see LICENSE for details.
// SPDX-License-Identifier: Apache-2.0

// Abstract primitives wrapper.
//
// This file is a stop-gap until the DV file list is generated by FuseSoC.
// Its contents are taken from the file which would be generated by FuseSoC.
// https://github.com/lowRISC/ibex/issues/893

`ifndef PRIM_DEFAULT_IMPL
`define PRIM_DEFAULT_IMPL prim_pkg::ImplGeneric
`endif

module prim_ram_1p #(
parameter int Width = 32, // bit
parameter int Depth = 128,
parameter int DataBitsPerMask = 1, // Number of data bits per bit of write mask
localparam int Aw = $clog2(Depth) // derived parameter
) (
input logic clk_i,

input logic req_i,
input logic write_i,
input logic [Aw-1:0] addr_i,
input logic [Width-1:0] wdata_i,
input logic [Width-1:0] wmask_i,
output logic [Width-1:0] rdata_o // Read data. Data is returned one cycle after req_i is high.
);
parameter prim_pkg::impl_e Impl = `PRIM_DEFAULT_IMPL;

if (Impl == prim_pkg::ImplGeneric) begin : gen_generic
prim_generic_ram_1p u_impl_generic (
.*
);
end else begin : gen_failure
// TODO: Find code that works across tools and causes a compile failure
end

endmodule
26 changes: 17 additions & 9 deletions dv/uvm/core_ibex/ibex_dv.f
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,27 @@
+define+BOOT_ADDR=2147483648 // 32'h8000_0000
+define+TRACE_EXECUTION
+define+RVFI
+incdir+${PRJ_DIR}/ibex/shared/rtl

${PRJ_DIR}/ibex/shared/rtl/prim_clock_gating.sv
// Shared lowRISC code
+incdir+${PRJ_DIR}/ibex/vendor/lowrisc_ip/prim/rtl
${PRJ_DIR}/ibex/vendor/lowrisc_ip/prim/rtl/prim_assert.sv
${PRJ_DIR}/ibex/vendor/lowrisc_ip/prim/rtl/prim_lfsr.sv
${PRJ_DIR}/ibex/vendor/lowrisc_ip/prim/rtl/prim_secded_28_22_enc.sv
${PRJ_DIR}/ibex/vendor/lowrisc_ip/prim/rtl/prim_secded_28_22_dec.sv
${PRJ_DIR}/ibex/vendor/lowrisc_ip/prim/rtl/prim_secded_72_64_enc.sv
${PRJ_DIR}/ibex/vendor/lowrisc_ip/prim/rtl/prim_secded_72_64_dec.sv

// Until this list is generated by FuseSoC, we have to use manually generated
// wrappers around the prim_* modules to instantiate the prim_generic_* ones,
// see https://github.com/lowRISC/ibex/issues/893.
${PRJ_DIR}/ibex/dv/uvm/core_ibex/common/prim/prim_pkg.sv
${PRJ_DIR}/ibex/vendor/lowrisc_ip/prim_generic/rtl/prim_generic_ram_1p.sv
${PRJ_DIR}/ibex/dv/uvm/core_ibex/common/prim/prim_ram_1p.sv
${PRJ_DIR}/ibex/vendor/lowrisc_ip/prim_generic/rtl/prim_generic_clock_gating.sv
${PRJ_DIR}/ibex/dv/uvm/core_ibex/common/prim/prim_clock_gating.sv

// ibex CORE RTL files
+incdir+${PRJ_DIR}/ibex/rtl
${PRJ_DIR}/ibex/shared/rtl/prim_assert.sv
${PRJ_DIR}/ibex/shared/rtl/prim_generic_ram_1p.sv
${PRJ_DIR}/ibex/shared/rtl/prim_lfsr.sv
${PRJ_DIR}/ibex/shared/rtl/prim_secded_28_22_enc.sv
${PRJ_DIR}/ibex/shared/rtl/prim_secded_28_22_dec.sv
${PRJ_DIR}/ibex/shared/rtl/prim_secded_72_64_enc.sv
${PRJ_DIR}/ibex/shared/rtl/prim_secded_72_64_dec.sv
${PRJ_DIR}/ibex/rtl/ibex_pkg.sv
${PRJ_DIR}/ibex/rtl/ibex_tracer_pkg.sv
${PRJ_DIR}/ibex/rtl/ibex_tracer.sv
Expand Down
3 changes: 2 additions & 1 deletion examples/simple_system/ibex_simple_system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ int main(int argc, char **argv) {
simctrl.SetTop(&top, &top.IO_CLK, &top.IO_RST_N,
VerilatorSimCtrlFlags::ResetPolarityNegative);

memutil.RegisterMemoryArea("ram", "TOP.ibex_simple_system.u_ram");
memutil.RegisterMemoryArea(
"ram", "TOP.ibex_simple_system.u_ram.u_ram.gen_generic.u_impl_generic");
simctrl.RegisterExtension(&memutil);

std::cout << "Simulation of Ibex" << std::endl
Expand Down
6 changes: 4 additions & 2 deletions ibex_core.core
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ filesets:
files_rtl:
depend:
- lowrisc:prim:assert
- lowrisc:prim:lfsr
# TODO: Only lfsr is needed. Replace with a more specific dependency
# once available.
- lowrisc:prim:all
- lowrisc:ibex:ibex_pkg
- lowrisc:ibex:ibex_icache
files:
Expand Down Expand Up @@ -48,7 +50,7 @@ filesets:

files_check_tool_requirements:
depend:
- lowrisc:ibex:check_tool_requirements
- lowrisc:tool:check_tool_requirements

parameters:
RVFI:
Expand Down
4 changes: 4 additions & 0 deletions lint/verilator_waiver.vlt
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,7 @@ lint_off -rule UNUSED -file "*/rtl/ibex_decoder.sv" -match "*instr_alu*"
// ibex_core.cs_registers_i.mie_q
// Issue lowrisc/ibex#212
lint_off -rule UNOPTFLAT -file "*/rtl/ibex_cs_registers.sv" -match "*ibex_core.cs_registers_i.mie_q*"

// Temporary waivers until OpenTitan primitives are lint-clean
// https://github.com/lowRISC/opentitan/issues/2313
lint_off -file "*/lowrisc_prim_*/rtl/*.sv"
1 change: 1 addition & 0 deletions python-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ git+https://github.com/lowRISC/edalize.git@ot
git+https://github.com/lowRISC/fusesoc.git@ot

pyyaml
mako
8 changes: 2 additions & 6 deletions rtl/ibex_icache.sv
Original file line number Diff line number Diff line change
Expand Up @@ -300,33 +300,29 @@ module ibex_icache #(

for (genvar way = 0; way < NumWays; way++) begin : gen_rams
// Tag RAM instantiation
prim_generic_ram_1p #(
prim_ram_1p #(
.Width (TAG_SIZE_ECC),
.Depth (NUM_LINES)
) tag_bank (
.clk_i (clk_i),
.rst_ni (rst_ni),
.req_i (tag_req_ic0 & tag_banks_ic0[way]),
.write_i (tag_write_ic0),
.wmask_i ({TAG_SIZE_ECC{1'b1}}),
.addr_i (tag_index_ic0),
.wdata_i (tag_wdata_ic0),
.rvalid_o (),
.rdata_o (tag_rdata_ic1[way])
);
// Data RAM instantiation
prim_generic_ram_1p #(
prim_ram_1p #(
.Width (LINE_SIZE_ECC),
.Depth (NUM_LINES)
) data_bank (
.clk_i (clk_i),
.rst_ni (rst_ni),
.req_i (data_req_ic0 & data_banks_ic0[way]),
.write_i (data_write_ic0),
.wmask_i ({LINE_SIZE_ECC{1'b1}}),
.addr_i (data_index_ic0),
.wdata_i (data_wdata_ic0),
.rvalid_o (),
.rdata_o (data_rdata_ic1[way])
);
end
Expand Down
3 changes: 2 additions & 1 deletion shared/fpga_xilinx.core
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ name: "lowrisc:ibex:fpga_xilinx_shared"
description: "Collection of useful RTL for Xilinx based examples"
filesets:
files_sv:
depend:
- lowrisc:prim:clock_gating
files:
- rtl/fpga/xilinx/prim_clock_gating.sv
- rtl/fpga/xilinx/clkgen_xil7series.sv
- rtl/ram_1p.sv
file_type: systemVerilogSource
Expand Down
24 changes: 0 additions & 24 deletions shared/rtl/prim_clock_gating.sv

This file was deleted.

109 changes: 0 additions & 109 deletions shared/rtl/prim_generic_ram_1p.sv

This file was deleted.

Loading

0 comments on commit 8b42024

Please sign in to comment.