Skip to content

Commit

Permalink
[spi_device] Add support for 1r1w RAMs and parity init
Browse files Browse the repository at this point in the history
Add 1r1w RAM configuration as an option for spi_device for tech nodes
where the 2p RAM configuration is not available. Make the 2p RAM have
the same access controls as the 1r1w RAM, so the two behave the same
way.

Also add word initialization circuitry on the SPI side, to init parity.
The SPI -> core buffer for the payload uses parity and SW has no way of
initializing it since the the write port is in the SPI domain. Since the SPI
side writes the payload byte by byte, we need to guard against partially
initialized 32bit wordd, because these could cause TL-UL bus errors upon
readout. Unfortunately, an initialization circuit that initializes the entire
SRAM on the SPI clock domain is infeasible since that clock is only
intermittently available. Hence, we keep track of uninitialized words using a
valid bit array, and upon the first write to a word, uninitialized bytes are
set to zero if the write operation is a sub-word write op.

Note that in this commit, DV tests have focused much more on the 2p
variant.

Signed-off-by: Alexander Williams <[email protected]>
Co-authored-by: Michael Schaffner <[email protected]>
  • Loading branch information
a-will and msfschaffner committed Jan 24, 2024
1 parent 1ffc916 commit f554d45
Show file tree
Hide file tree
Showing 29 changed files with 599 additions and 240 deletions.
52 changes: 44 additions & 8 deletions hw/ip/spi_device/data/spi_device.hjson
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,31 @@
scan: "true", // Enable `scanmode_i` port
scan_reset: "true", // Enable `scan_rst_ni` port
param_list: [
{ name: "SramType"
desc: "Sram Entries. Word size is 32bit width."
type: "spi_device_pkg::sram_type_e"
default: "spi_device_pkg::DefaultSramType"
local: "false"
expose: "true"
}
{ name: "SramDepth"
desc: "Sram Entries. Word size is 32bit width."
type: "int unsigned"
default: "1024"
local: "true"
}
{ name: "SramEgressDepth"
desc: "Sram Egress Entries. Word size is 32bit width."
type: "int unsigned"
default: "832"
local: "true"
}
{ name: "SramIngressDepth"
desc: "Sram Ingress Entries. Word size is 32bit width."
type: "int unsigned"
default: "96"
local: "true"
}
{ name: "NumCmdInfo"
desc: "Define the number of Command Info slots."
type: "int unsigned"
Expand Down Expand Up @@ -1317,18 +1336,35 @@
//---------------------------------------------------------------
{ skipto: "0x1000" }
{ window: {
name: "buffer",
items: "SramDepth",
name: "egress_buffer",
items: "SramEgressDepth",
validbits: "32",
byte-write: "false",
unusual: "true"
swaccess: "wo",
desc: '''
SPI internal egress buffer.

The lower 2 kB is for Read content emulating eFlash.
The next 1 kB is for the Mailbox buffer.
'''
},
},
{ window: {
name: "ingress_buffer",
items: "SramIngressDepth",
validbits: "32",
byte-write: "false",
unusual: "false"
swaccess: "rw",
unusual: "true"
swaccess: "ro",
desc: '''
SPI internal buffer.
SPI internal ingress buffer.

The lower 2kB is for Read content emulating eFlash. The next 1 kB is
for Mailbox read/write buffer. The rest is 256B SFDP buffer, 32B of
CmdFIFO, 32B of AddrFIFO, and 256B of payload FIFO.
The layout is as follows (starting from offset 0):
- 256 B SFDP buffer
- 32 B CmdFIFO
- 32 B AddrFIFO
- 256 B payload FIFO
'''
},
},
Expand Down
32 changes: 23 additions & 9 deletions hw/ip/spi_device/doc/registers.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@
| spi_device.[`TPM_CMD_ADDR`](#tpm_cmd_addr) | 0x830 | 4 | TPM Command and Address buffer |
| spi_device.[`TPM_READ_FIFO`](#tpm_read_fifo) | 0x834 | 4 | TPM Read command return data FIFO. |
| spi_device.[`TPM_WRITE_FIFO`](#tpm_write_fifo) | 0x838 | 4 | TPM Write command received data FIFO. |
| spi_device.[`buffer`](#buffer) | 0x1000 | 4096 | SPI internal buffer. |
| spi_device.[`egress_buffer`](#egress_buffer) | 0x1000 | 3328 | SPI internal egress buffer. |
| spi_device.[`ingress_buffer`](#ingress_buffer) | 0x1e00 | 384 | SPI internal ingress buffer. |

## INTR_STATE
Interrupt State Register
Expand Down Expand Up @@ -1574,16 +1575,29 @@ TPM Write command received data FIFO.
| 31:8 | | | | Reserved |
| 7:0 | ro | x | value | Read only port of the write FIFO |

## buffer
SPI internal buffer.
## egress_buffer
SPI internal egress buffer.

The lower 2kB is for Read content emulating eFlash. The next 1 kB is
for Mailbox read/write buffer. The rest is 256B SFDP buffer, 32B of
CmdFIFO, 32B of AddrFIFO, and 256B of payload FIFO.
The lower 2 kB is for Read content emulating eFlash.
The next 1 kB is for the Mailbox buffer.

- Word Aligned Offset Range: `0x1000`to`0x1ffc`
- Size (words): `1024`
- Access: `rw`
- Word Aligned Offset Range: `0x1000`to`0x1cfc`
- Size (words): `832`
- Access: `wo`
- Byte writes are *not* supported.

## ingress_buffer
SPI internal ingress buffer.

The layout is as follows (starting from offset 0):
- 256 B SFDP buffer
- 32 B CmdFIFO
- 32 B AddrFIFO
- 256 B payload FIFO

- Word Aligned Offset Range: `0x1e00`to`0x1f7c`
- Size (words): `96`
- Access: `ro`
- Byte writes are *not* supported.


Expand Down
23 changes: 17 additions & 6 deletions hw/ip/spi_device/dv/env/seq_lib/spi_device_mem_parity_vseq.sv
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,31 @@ class spi_device_mem_parity_vseq extends spi_device_common_vseq;
}

virtual task body();
bit [TL_AW-1:0] offset = $urandom_range(0, (SRAM_SIZE / SRAM_WORD_SIZE) - 1);
bit [TL_AW-1:0] ingress_sram_offset =
(cfg.sram_ingress_start_addr - cfg.sram_egress_start_addr) >> 2;
bit [TL_AW-1:0] offset = $urandom_range(0, (SRAM_INGRESS_SIZE / SRAM_WORD_SIZE) - 1);
bit [TL_DW-1:0] data;
logic [BitPerByte*BytePerWord-1:0] mem_data;
string path = $sformatf("tb.dut.u_memory_2p.u_mem.gen_generic.u_impl_generic.mem[%0d]", offset);
// TODO: Add support for the gen_ram1r1w variant.
string path_fmt =
"tb.dut.u_spid_dpram.gen_ram2p.u_memory_2p.u_mem.gen_generic.u_impl_generic.mem[%0d]";
string egress_path = $sformatf(path_fmt, offset);
string ingress_path = $sformatf(path_fmt, ingress_sram_offset + offset);

// Disable comparisons with the mirror values, since the egress area is
// write-only, and the ingress area is read-only.
cfg.en_scb_mem_chk = 0;

repeat (100) begin
mem_wr(.ptr(ral.buffer), .offset(offset), .data($urandom));
// Write to the egress buffer to get the word with correct parity bits.
mem_wr(.ptr(ral.egress_buffer), .offset(offset), .data($urandom));

`DV_CHECK_MEMBER_RANDOMIZE_FATAL(flip_bits)
// backdoor read and inject parity errors
`DV_CHECK(uvm_hdl_read(path, mem_data));
`DV_CHECK(uvm_hdl_deposit(path, mem_data ^ flip_bits));
`DV_CHECK(uvm_hdl_read(egress_path, mem_data));
`DV_CHECK(uvm_hdl_deposit(ingress_path, mem_data ^ flip_bits));
// frontdoor read to check it returns d_error
tl_access(.addr(ral.buffer.get_offset() + (offset << 2)),
tl_access(.addr(ral.ingress_buffer.get_offset() + (offset << 2)),
.write(0),
.data(data),
.mask(get_rand_contiguous_mask()),
Expand Down
9 changes: 5 additions & 4 deletions hw/ip/spi_device/dv/env/seq_lib/spi_device_pass_base_vseq.sv
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,8 @@ class spi_device_pass_base_vseq extends spi_device_base_vseq;

config_all_cmd_infos();

random_write_spi_mem(.start_addr(0), .end_addr(SRAM_SIZE - 1));
// TODO: Randomize the ingress buffer too.
random_write_spi_mem(.start_addr(0), .end_addr(SRAM_EGRESS_SIZE - 1));
randomize_all_cmd_filters();
endtask : spi_device_flash_pass_init

Expand Down Expand Up @@ -695,12 +696,12 @@ class spi_device_pass_base_vseq extends spi_device_base_vseq;
if (payload_depth_val > PAYLOAD_FIFO_SIZE) payload_depth_val = PAYLOAD_FIFO_SIZE;

// need to shift by 2 for the offset used at mem_rd
payload_base_offset = (READ_BUFFER_SIZE + MAILBOX_BUFFER_SIZE + SFDP_SIZE) / 4;
payload_base_offset = 0;
payload_depth_val = payload_depth_val / 4;
for (int i = 0; i < payload_depth_val; i++) begin
bit [TL_DW-1:0] val;
int offset = i + payload_base_offset;
mem_rd(.ptr(ral.buffer), .offset(offset), .data(val));
mem_rd(.ptr(ral.ingress_buffer), .offset(offset), .data(val));
`uvm_info(`gfn, $sformatf("read upload_payloadfifo: idx: %0d, data: 0x%0x", i, val),
UVM_MEDIUM)
end
Expand Down Expand Up @@ -763,7 +764,7 @@ class spi_device_pass_base_vseq extends spi_device_base_vseq;
end
for (int i = start_addr / 4; i <= end_addr / 4; i++) begin
bit [TL_DW-1:0] data = $urandom();
mem_wr(.ptr(ral.buffer), .offset(i), .data(data), .blocking(!zero_delay_write));
mem_wr(.ptr(ral.egress_buffer), .offset(i), .data(data), .blocking(!zero_delay_write));
`uvm_info(`gfn, $sformatf("write %s offset 0x%0x: 0x%0x", msg_region, i << 2, data),
UVM_MEDIUM)
end
Expand Down
17 changes: 13 additions & 4 deletions hw/ip/spi_device/dv/env/seq_lib/spi_device_ram_cfg_vseq.sv
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,25 @@ class spi_device_ram_cfg_vseq extends spi_device_base_vseq;
`uvm_object_new

virtual task body();
prim_ram_2p_pkg::ram_2p_cfg_t src_ram_cfg, dst_ram_cfg;
prim_ram_2p_pkg::ram_2p_cfg_t src_ram_cfg, dst_ram_cfg, egress_ram_cfg, ingress_ram_cfg;
string src_path = "tb.dut.ram_cfg_i";
string dst_path = "tb.dut.u_memory_2p.u_mem.cfg_i";
string dst_path = "tb.dut.u_spid_dpram.gen_ram2p.u_memory_2p.u_mem.cfg_i";
string egress_path = "tb.dut.u_spid_dpram.gen_ram1r1w.u_sys2spi_mem.u_mem.cfg_i";
string ingress_path = "tb.dut.u_spid_dpram.gen_ram1r1w.u_spi2sys_mem.u_mem.cfg_i";

repeat (100) begin
`DV_CHECK_STD_RANDOMIZE_FATAL(src_ram_cfg)
`DV_CHECK(uvm_hdl_deposit(src_path, src_ram_cfg))
#($urandom_range(1, 100) * 1ns);
`DV_CHECK(uvm_hdl_read(dst_path, dst_ram_cfg))
`DV_CHECK_CASE_EQ(src_ram_cfg, dst_ram_cfg)
if (spi_device_env_pkg::SRAM_TYPE == spi_device_pkg::SramType2p) begin
`DV_CHECK(uvm_hdl_read(dst_path, dst_ram_cfg))
`DV_CHECK_CASE_EQ(src_ram_cfg, dst_ram_cfg)
end else begin // 1r1w
`DV_CHECK(uvm_hdl_read(egress_path, egress_ram_cfg))
`DV_CHECK_CASE_EQ(src_ram_cfg, egress_ram_cfg)
`DV_CHECK(uvm_hdl_read(ingress_path, ingress_ram_cfg))
`DV_CHECK_CASE_EQ(src_ram_cfg, ingress_ram_cfg)
end
end
endtask
endclass : spi_device_ram_cfg_vseq
1 change: 1 addition & 0 deletions hw/ip/spi_device/dv/env/spi_device_env.core
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ filesets:
- lowrisc:dv:ralgen
- lowrisc:dv:cip_lib
- lowrisc:dv:spi_agent
- lowrisc:ip:spi_device_pkg:0.1
files:
- spi_device_env_pkg.sv
- spi_device_env_cfg.sv: {is_include_file: true}
Expand Down
13 changes: 9 additions & 4 deletions hw/ip/spi_device/dv/env/spi_device_env_cfg.sv
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
class spi_device_env_cfg extends cip_base_env_cfg #(.RAL_T(spi_device_reg_block));
rand spi_agent_cfg spi_host_agent_cfg;
rand spi_agent_cfg spi_device_agent_cfg;
bit [TL_AW-1:0] sram_start_addr;
bit [TL_AW-1:0] sram_end_addr;
bit [TL_AW-1:0] sram_egress_start_addr;
bit [TL_AW-1:0] sram_egress_end_addr;
bit [TL_AW-1:0] sram_ingress_start_addr;
bit [TL_AW-1:0] sram_ingress_end_addr;

// read buffer needs to be read with incremental address, otherwise, watermark/fip won't work
// this needs to be kept across sequences and cleared when reset occurs. Only seq uses it.
Expand Down Expand Up @@ -54,8 +56,11 @@ class spi_device_env_cfg extends cip_base_env_cfg #(.RAL_T(spi_device_reg_block)
// set num_interrupts & num_alerts which will be used to create coverage and more
num_interrupts = ral.intr_state.get_n_used_bits();

sram_start_addr = ral.get_addr_from_offset(SRAM_OFFSET);
sram_end_addr = sram_start_addr + SRAM_SIZE - 1;
sram_egress_start_addr = ral.get_addr_from_offset(ral.egress_buffer.get_offset());
sram_egress_end_addr = sram_egress_start_addr + SRAM_EGRESS_SIZE - 1;
sram_ingress_start_addr = ral.get_addr_from_offset(ral.ingress_buffer.get_offset());
sram_ingress_end_addr = sram_ingress_start_addr + SRAM_INGRESS_SIZE - 1;
$display("SRAM ingress start = 0x%h", sram_ingress_start_addr);

// only support 1 outstanding TL item
m_tl_agent_cfg.max_outstanding_req = 1;
Expand Down
16 changes: 0 additions & 16 deletions hw/ip/spi_device/dv/env/spi_device_env_cov.sv
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,6 @@ class spi_device_env_cov extends cip_base_env_cov #(.CFG_T(spi_device_env_cfg));
cr_all: cross tx_order, rx_order, cp_cpol, cp_cpha;
endgroup

covergroup fw_tx_fifo_size_cg with function sample(int tx_size);
cp_tx_size: coverpoint tx_size {
bins sizes[5] = {[0:SRAM_SIZE]};
bins specific_sizes[] = {SRAM_WORD_SIZE, SRAM_SIZE / 2, SRAM_SIZE - SRAM_WORD_SIZE};
}
endgroup

covergroup fw_rx_fifo_size_cg with function sample(int rx_size);
cp_rx_size: coverpoint rx_size {
bins sizes[5] = {[0:SRAM_SIZE]};
bins specific_sizes[] = {SRAM_WORD_SIZE, SRAM_SIZE / 2, SRAM_SIZE - SRAM_WORD_SIZE};
}
endgroup

// TPM related
covergroup tpm_cfg_cg with function sample(
// CSR cfg
Expand Down Expand Up @@ -296,8 +282,6 @@ class spi_device_env_cov extends cip_base_env_cov #(.CFG_T(spi_device_env_cfg));
end
all_modes_cg = new();
bit_order_clk_cfg_cg = new();
fw_tx_fifo_size_cg = new();
fw_rx_fifo_size_cg = new();
// tpm
tpm_cfg_cg = new();
tpm_transfer_size_cg = new();
Expand Down
7 changes: 5 additions & 2 deletions hw/ip/spi_device/dv/env/spi_device_env_pkg.sv
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,12 @@ package spi_device_env_pkg;
parameter uint NUM_ALERTS = 1;
parameter string LIST_OF_ALERTS[] = {"fatal_fault"};

// SPI SRAM is 2kB
parameter spi_device_pkg::sram_type_e SRAM_TYPE = spi_device_pkg::SramType2p;
// SPI SRAM is 4kB
parameter uint SRAM_OFFSET = 'h1000;
parameter uint SRAM_SIZE = 4096; // 672 depth
parameter uint SRAM_SIZE = 4096;
parameter uint SRAM_EGRESS_SIZE = 2048 + 1024 + 256; // 832 depth
parameter uint SRAM_INGRESS_SIZE = 256 + 64 + 64; // 96 depth
parameter uint SRAM_MSB = $clog2(SRAM_SIZE) - 1;
parameter uint SRAM_PTR_PHASE_BIT = SRAM_MSB + 1;
parameter uint SRAM_WORD_SIZE = 4;
Expand Down
8 changes: 6 additions & 2 deletions hw/ip/spi_device/dv/env/spi_device_scoreboard.sv
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ class spi_device_scoreboard extends cip_base_scoreboard #(.CFG_T (spi_device_env
endfunction

virtual function void process_upload_cmd(spi_item item);
bit [31:0] payload_start_addr = get_converted_addr(PAYLOAD_FIFO_START_ADDR);
bit [31:0] payload_start_addr = get_converted_addr(ral.ingress_buffer.get_offset());
int payload_depth_exp;
upload_cmd_q.push_back(item.opcode);
update_pending_intr_w_delay(.intr(CmdFifoNotEmpty), .delay_cyc(4));
Expand Down Expand Up @@ -953,7 +953,11 @@ class spi_device_scoreboard extends cip_base_scoreboard #(.CFG_T (spi_device_env
csr = cfg.ral_models[ral_name].default_map.get_reg_by_offset(csr_addr);
`DV_CHECK_NE_FATAL(csr, null)
end
else if (csr_addr inside {[cfg.sram_start_addr:cfg.sram_end_addr]}) begin
else if (csr_addr inside {[cfg.sram_egress_start_addr:cfg.sram_egress_end_addr]}) begin
// TODO: Anything to do here?
return;
end
else if (csr_addr inside {[cfg.sram_ingress_start_addr:cfg.sram_ingress_end_addr]}) begin
if (!write) begin
// cip_base_scoreboard compares the mem read only when the address exists
// just need to ensure address exists here and mem check is done at process_mem_read
Expand Down
4 changes: 3 additions & 1 deletion hw/ip/spi_device/dv/tb/tb.sv
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ module tb;
`DV_ALERT_IF_CONNECT()

// dut
spi_device dut (
spi_device #(
.SramType(SRAM_TYPE)
) dut (
.clk_i (clk ),
.rst_ni (rst_n ),

Expand Down
6 changes: 6 additions & 0 deletions hw/ip/spi_device/lint/spi_device.waiver
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ waive -rules CONST_FF -location {spi_p2s.sv} \
-regexp {Flip-flop 'tx_state' is driven} \
-comment "Intended behavior"

waive -rules TWO_STATE_TYPE -location {spi_device_pkg.sv} \
-regexp {'sram_type_e' is of two state type} \
-comment "Enum int unsigned is used as a generate selection. OK to be two state"
waive -rules TWO_STATE_TYPE -location {spi_device.sv} \
-regexp {'sys_sram_e' is of two state type} \
-comment "Enum int unsigned is used as a index. OK to be two state"
Expand Down Expand Up @@ -107,6 +110,9 @@ waive -rules CLOCK_MUX -location {spi_device.sv} \
waive -rules {NOT_USED NOT_READ} -location {spi_device.sv} \
-regexp {'sub_(sram|p2s)_.*\[0\]' is not (used|read)} \
-comment "CmdParse does not have SRAM intf"
waive -rules {NOT_USED NOT_READ} -location {spid_dpram.sv} \
-regexp {'.*_unused' is not (used|read)} \
-comment "1r1w control signals are not used in the 2p config"

#### Intented Terminal States
waive -rules {TERMINAL_STATE} -location {spi_cmdparse.sv} \
Expand Down
Loading

0 comments on commit f554d45

Please sign in to comment.